diff --git a/src/builtins/commandline.rs b/src/builtins/commandline.rs index 3338f7174..d505df233 100644 --- a/src/builtins/commandline.rs +++ b/src/builtins/commandline.rs @@ -7,7 +7,7 @@ use crate::input::input_function_get_code; use crate::input_common::{CharEvent, ReadlineCmd}; use crate::operation_context::{OperationContext, no_cancel}; -use crate::parse_constants::{ParseTreeFlags, ParserTestErrorBits}; +use crate::parse_constants::ParseTreeFlags; use crate::parse_util::{ detect_parse_errors, get_job_extent, get_offset_from_line, get_process_extent, get_token_extent, lineno, @@ -705,13 +705,8 @@ pub fn commandline(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) let res = detect_parse_errors(current_buffer, None, /*accept_incomplete=*/ true); return match res { Ok(()) => Ok(SUCCESS), - Err(err) => { - if err.contains(ParserTestErrorBits::INCOMPLETE) { - Err(STATUS_INVALID_ARGS) - } else { - Err(STATUS_CMD_ERROR) - } - } + Err(p) if p.incomplete => Err(STATUS_INVALID_ARGS), + Err(_) => Err(STATUS_CMD_ERROR), }; } diff --git a/src/parse_constants.rs b/src/parse_constants.rs index 65812a829..838a01b81 100644 --- a/src/parse_constants.rs +++ b/src/parse_constants.rs @@ -1,8 +1,8 @@ //! Constants used in the programmatic representation of fish code. use crate::prelude::*; -use bitflags::bitflags; use fish_fallback::{fish_wcswidth, fish_wcwidth}; +use std::ops::{BitOr, BitOrAssign}; pub type SourceOffset = u32; @@ -27,11 +27,40 @@ pub struct ParseTreeFlags { pub show_extra_semis: bool, } -bitflags! { - #[derive(Copy, Clone, Debug, Default, Eq, PartialEq)] - pub struct ParserTestErrorBits: u8 { - const ERROR = 1; - const INCOMPLETE = 2; +/// Represents parse issues found during validation. +/// If this is returned as the error of a Result, then either `error` or `incomplete` (or both) is set. +#[derive(Copy, Clone, Debug, Default, Eq, PartialEq)] +pub struct ParseIssue { + pub error: bool, // An error was found. + pub incomplete: bool, // Incomplete input, such as unclosed block or pipe. +} + +impl ParseIssue { + pub const ERROR: Result<(), Self> = Err(Self { + error: true, + incomplete: false, + }); + + pub const INCOMPLETE: Result<(), Self> = Err(Self { + error: false, + incomplete: true, + }); +} + +// Allow | and |= to combine ParseIssues. +impl BitOr for ParseIssue { + type Output = Self; + fn bitor(self, rhs: Self) -> Self { + Self { + error: self.error | rhs.error, + incomplete: self.incomplete | rhs.incomplete, + } + } +} + +impl BitOrAssign for ParseIssue { + fn bitor_assign(&mut self, rhs: Self) { + *self = *self | rhs; } } diff --git a/src/parse_util.rs b/src/parse_util.rs index 7dd383b0a..c1a437780 100644 --- a/src/parse_util.rs +++ b/src/parse_util.rs @@ -19,8 +19,8 @@ ERROR_BAD_VAR_CHAR1, ERROR_BRACKETED_VARIABLE_QUOTED1, ERROR_BRACKETED_VARIABLE1, ERROR_NO_VAR_NAME, ERROR_NOT_ARGV_AT, ERROR_NOT_ARGV_COUNT, ERROR_NOT_ARGV_STAR, ERROR_NOT_PID, ERROR_NOT_STATUS, INVALID_BREAK_ERR_MSG, INVALID_CONTINUE_ERR_MSG, - INVALID_PIPELINE_CMD_ERR_MSG, ParseError, ParseErrorCode, ParseErrorList, ParseKeyword, - ParseTokenType, ParseTreeFlags, ParserTestErrorBits, PipelinePosition, SourceRange, + INVALID_PIPELINE_CMD_ERR_MSG, ParseError, ParseErrorCode, ParseErrorList, ParseIssue, + ParseKeyword, ParseTokenType, ParseTreeFlags, PipelinePosition, SourceRange, StatementDecoration, UNKNOWN_BUILTIN_ERR_MSG, parse_error_offset_source_start, }; use crate::prelude::*; @@ -1119,14 +1119,14 @@ fn visit(&mut self, node: &'a dyn Node) { } /// Given a string, detect parse errors in it. If allow_incomplete is set, then if the string is -/// incomplete (e.g. an unclosed quote), an error is not returned and the ParserTestErrorBits::INCOMPLETE bit +/// incomplete (e.g. an unclosed quote), an error is not returned and `ParseIssue::incomplete` /// is set in the return value. If allow_incomplete is not set, then incomplete strings result in an /// error. pub fn detect_parse_errors( buff_src: &wstr, mut out_errors: Option<&mut ParseErrorList>, allow_incomplete: bool, /*=false*/ -) -> Result<(), ParserTestErrorBits> { +) -> Result<(), ParseIssue> { // Whether there's an unclosed quote or subshell, and therefore unfinished. This is only set if // allow_incomplete is set. let mut has_unclosed_quote_or_subshell = false; @@ -1162,7 +1162,7 @@ pub fn detect_parse_errors( assert!(!has_unclosed_quote_or_subshell || allow_incomplete); if has_unclosed_quote_or_subshell { // We do not bother to validate the rest of the tree in this case. - return Err(ParserTestErrorBits::INCOMPLETE); + return ParseIssue::INCOMPLETE; } // Early parse error, stop here. @@ -1170,7 +1170,7 @@ pub fn detect_parse_errors( if let Some(errors) = out_errors.as_mut() { errors.extend(parse_errors); } - return Err(ParserTestErrorBits::ERROR); + return ParseIssue::ERROR; } // Defer to the tree-walking version. @@ -1183,11 +1183,10 @@ pub fn detect_parse_errors_in_ast( ast: &Ast, buff_src: &wstr, mut out_errors: Option<&mut ParseErrorList>, -) -> Result<(), ParserTestErrorBits> { - let mut res = ParserTestErrorBits::default(); - - // Whether we encountered a parse error. - let mut errored = false; +) -> Result<(), ParseIssue> { + // The issue to return. + // We break out various reasons for incompleteness to be explicit. + let mut issue = ParseIssue::default(); // Whether we encountered an unclosed block. We detect this via an 'end_command' block without // source. @@ -1217,7 +1216,7 @@ pub fn detect_parse_errors_in_ast( } } Kind::JobConjunction(job_conjunction) => { - errored |= detect_errors_in_job_conjunction(job_conjunction, &mut out_errors); + issue.error |= detect_errors_in_job_conjunction(job_conjunction, &mut out_errors); } Kind::JobConjunctionContinuation(jcc) => { // Somewhat clumsy way of checking for a job without source in a conjunction. @@ -1228,9 +1227,9 @@ pub fn detect_parse_errors_in_ast( } Kind::Argument(arg) => { let arg_src = arg.source(buff_src); - res |= detect_errors_in_argument(arg, arg_src, &mut out_errors) - .err() - .unwrap_or_default(); + if let Err(e) = detect_errors_in_argument(arg, arg_src, &mut out_errors) { + issue |= e; + } } Kind::JobPipeline(job) => { // Disallow background in the following cases: @@ -1241,11 +1240,12 @@ pub fn detect_parse_errors_in_ast( // while foo & ; end // If it's not a background job, nothing to do. if job.bg.is_some() { - errored |= detect_errors_in_backgrounded_job(&traversal, job, &mut out_errors); + issue.error |= + detect_errors_in_backgrounded_job(&traversal, job, &mut out_errors); } } Kind::DecoratedStatement(stmt) => { - errored |= detect_errors_in_decorated_statement( + issue.error |= detect_errors_in_decorated_statement( buff_src, &traversal, stmt, @@ -1257,7 +1257,7 @@ pub fn detect_parse_errors_in_ast( if !block.end.has_source() { has_unclosed_block = true; } - errored |= detect_errors_in_block_redirection_list( + issue.error |= detect_errors_in_block_redirection_list( node, &block.args_or_redirs, &mut out_errors, @@ -1268,7 +1268,7 @@ pub fn detect_parse_errors_in_ast( if !brace_statement.right_brace.has_source() { has_unclosed_block = true; } - errored |= detect_errors_in_block_redirection_list( + issue.error |= detect_errors_in_block_redirection_list( node, &brace_statement.args_or_redirs, &mut out_errors, @@ -1279,7 +1279,7 @@ pub fn detect_parse_errors_in_ast( if !ifs.end.has_source() { has_unclosed_block = true; } - errored |= detect_errors_in_block_redirection_list( + issue.error |= detect_errors_in_block_redirection_list( node, &ifs.args_or_redirs, &mut out_errors, @@ -1290,7 +1290,7 @@ pub fn detect_parse_errors_in_ast( if !switchs.end.has_source() { has_unclosed_block = true; } - errored |= detect_errors_in_block_redirection_list( + issue.error |= detect_errors_in_block_redirection_list( node, &switchs.args_or_redirs, &mut out_errors, @@ -1300,17 +1300,11 @@ pub fn detect_parse_errors_in_ast( } } - if errored { - res |= ParserTestErrorBits::ERROR; - } - - if has_unclosed_block || has_unclosed_pipe || has_unclosed_conjunction { - res |= ParserTestErrorBits::INCOMPLETE; - } - if res == ParserTestErrorBits::default() { - Ok(()) + issue.incomplete |= has_unclosed_block || has_unclosed_pipe || has_unclosed_conjunction; + if issue.error || issue.incomplete { + Err(issue) } else { - Err(res) + Ok(()) } } @@ -1387,16 +1381,17 @@ pub fn detect_errors_in_argument( arg: &ast::Argument, arg_src: &wstr, out_errors: &mut Option<&mut ParseErrorList>, -) -> Result<(), ParserTestErrorBits> { +) -> Result<(), ParseIssue> { let Some(source_range) = arg.try_source_range() else { return Ok(()); }; let source_start = source_range.start(); - let mut err = ParserTestErrorBits::default(); + let mut issue = ParseIssue::default(); + // Check if a subtoken contains errors. Returns true if there is an error, and appends to out_errors if provided. let check_subtoken = - |begin: usize, end: usize, out_errors: &mut Option<&mut ParseErrorList>| { + |begin: usize, end: usize, out_errors: &mut Option<&mut ParseErrorList>| -> bool { let Some(unesc) = unescape_string( &arg_src[begin..end], UnescapeStringStyle::Script(UnescapeFlags::SPECIAL), @@ -1416,7 +1411,7 @@ pub fn detect_errors_in_argument( "Incomplete escape sequence '%s'", arg_src ); - return ParserTestErrorBits::ERROR; + return true; } append_syntax_error!( out_errors, @@ -1426,10 +1421,10 @@ pub fn detect_errors_in_argument( arg_src ); } - return ParserTestErrorBits::ERROR; + return true; }; - let mut err = ParserTestErrorBits::default(); + let mut errored = false; // Check for invalid variable expansions. let unesc = unesc.as_char_slice(); for (idx, c) in unesc.iter().enumerate() { @@ -1437,10 +1432,10 @@ pub fn detect_errors_in_argument( continue; } let next_char = unesc.get(idx + 1).copied().unwrap_or('\0'); - if ![VARIABLE_EXPAND, VARIABLE_EXPAND_SINGLE, '('].contains(&next_char) + if !matches!(next_char, VARIABLE_EXPAND | VARIABLE_EXPAND_SINGLE | '(') && !valid_var_name_char(next_char) { - err = ParserTestErrorBits::ERROR; + errored = true; if let Some(out_errors) = out_errors { let mut first_dollar = idx; while first_dollar > 0 @@ -1454,7 +1449,7 @@ pub fn detect_errors_in_argument( } } - err + errored }; let mut cursor = 0; @@ -1472,25 +1467,24 @@ pub fn detect_errors_in_argument( Some(&mut has_dollar), ) { MaybeParentheses::Error => { - err |= ParserTestErrorBits::ERROR; + issue.error = true; append_syntax_error!(out_errors, source_start, 1, "Mismatched parenthesis"); - return Err(err); + return Err(issue); } MaybeParentheses::None => { do_loop = false; } MaybeParentheses::CommandSubstitution(parens) => { - err |= check_subtoken( + issue.error |= check_subtoken( checked, parens.start() - if has_dollar { 1 } else { 0 }, out_errors, ); let mut subst_errors = ParseErrorList::new(); - if let Err(subst_err) = + issue |= detect_parse_errors(&arg_src[parens.command()], Some(&mut subst_errors), false) - { - err |= subst_err; - } + .err() + .unwrap_or_default(); // Our command substitution produced error offsets relative to its source. Tweak the // offsets of the errors in the command substitution to account for both its offset @@ -1506,9 +1500,12 @@ pub fn detect_errors_in_argument( } } - err |= check_subtoken(checked, arg_src.len(), out_errors); - - if err.is_empty() { Ok(()) } else { Err(err) } + issue.error |= check_subtoken(checked, arg_src.len(), out_errors); + if issue.error || issue.incomplete { + Err(issue) + } else { + Ok(()) + } } fn detect_errors_in_job_conjunction( diff --git a/src/parser.rs b/src/parser.rs index f8bcb50ee..1193f3cb9 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -1506,7 +1506,7 @@ mod tests { use crate::expand::ExpandFlags; use crate::io::{IoBufferfill, IoChain}; use crate::parse_constants::{ - ParseErrorCode, ParseTokenType, ParseTreeFlags, ParserTestErrorBits, StatementDecoration, + ParseErrorCode, ParseIssue, ParseTokenType, ParseTreeFlags, StatementDecoration, }; use crate::parse_util::{detect_errors_in_argument, detect_parse_errors}; use crate::prelude::*; @@ -1526,11 +1526,11 @@ macro_rules! detect_errors { }; } - fn detect_argument_errors(src: &str) -> Result<(), ParserTestErrorBits> { + fn detect_argument_errors(src: &str) -> Result<(), ParseIssue> { let src = str2wcstring(src); let ast = ast::parse_argument_list(&src, ParseTreeFlags::default(), None); if ast.errored() { - return Err(ParserTestErrorBits::ERROR); + return ParseIssue::ERROR; } let args = &ast.top().arguments; let first_arg = args.first().expect("Failed to parse an argument"); @@ -1646,31 +1646,31 @@ fn detect_argument_errors(src: &str) -> Result<(), ParserTestErrorBits> { assert_eq!( detect_errors!("true | "), - Err(ParserTestErrorBits::INCOMPLETE), + ParseIssue::INCOMPLETE, "unterminated pipe not reported properly" ); assert_eq!( detect_errors!("echo (\nfoo\n bar"), - Err(ParserTestErrorBits::INCOMPLETE), + ParseIssue::INCOMPLETE, "unterminated multiline subshell not reported properly" ); assert_eq!( detect_errors!("begin ; true ; end | "), - Err(ParserTestErrorBits::INCOMPLETE), + ParseIssue::INCOMPLETE, "unterminated pipe not reported properly" ); assert_eq!( detect_errors!(" | true "), - Err(ParserTestErrorBits::ERROR), + ParseIssue::ERROR, "leading pipe not reported properly" ); assert_eq!( detect_errors!("true | # comment"), - Err(ParserTestErrorBits::INCOMPLETE), + ParseIssue::INCOMPLETE, "comment after pipe not reported as incomplete" ); @@ -1681,7 +1681,7 @@ fn detect_argument_errors(src: &str) -> Result<(), ParserTestErrorBits> { assert_eq!( detect_errors!("true | ; false "), - Err(ParserTestErrorBits::ERROR), + ParseIssue::ERROR, "semicolon after pipe not detected as error" ); @@ -1696,16 +1696,12 @@ fn detect_argument_errors(src: &str) -> Result<(), ParserTestErrorBits> { ); assert!( - detect_argument_errors("foo$$") - .unwrap_err() - .contains(ParserTestErrorBits::ERROR), + detect_argument_errors("foo$$").unwrap_err().error, "Bad variable expansion not reported as error" ); assert!( - detect_argument_errors("foo$@") - .unwrap_err() - .contains(ParserTestErrorBits::ERROR), + detect_argument_errors("foo$@").unwrap_err().error, "Bad variable expansion not reported as error" ); @@ -1714,7 +1710,7 @@ fn detect_argument_errors(src: &str) -> Result<(), ParserTestErrorBits> { assert!( detect_argument_errors("foo(cat | or cat)") .unwrap_err() - .contains(ParserTestErrorBits::ERROR), + .error, "Bad command substitution not reported as error" ); @@ -1790,7 +1786,7 @@ fn detect_argument_errors(src: &str) -> Result<(), ParserTestErrorBits> { assert_eq!( detect_errors!("true && "), - Err(ParserTestErrorBits::INCOMPLETE), + ParseIssue::INCOMPLETE, "unterminated conjunction not reported properly" ); @@ -1801,24 +1797,24 @@ fn detect_argument_errors(src: &str) -> Result<(), ParserTestErrorBits> { assert_eq!( detect_errors!("true || \n"), - Err(ParserTestErrorBits::INCOMPLETE), + ParseIssue::INCOMPLETE, "unterminated conjunction not reported properly" ); assert_eq!( detect_errors!("begin ; echo hi; }"), - Err(ParserTestErrorBits::ERROR), + ParseIssue::ERROR, "closing of unopened brace statement not reported properly" ); assert_eq!( detect_errors!("begin {"), // } - Err(ParserTestErrorBits::INCOMPLETE), + ParseIssue::INCOMPLETE, "brace after begin not reported properly" ); assert_eq!( detect_errors!("a=b {"), // } - Err(ParserTestErrorBits::INCOMPLETE), + ParseIssue::INCOMPLETE, "brace after variable override not reported properly" ); } diff --git a/src/reader/reader.rs b/src/reader/reader.rs index 1bb02e866..6c37634b5 100644 --- a/src/reader/reader.rs +++ b/src/reader/reader.rs @@ -73,7 +73,7 @@ use crate::pager::{PageRendering, Pager, SelectionMotion}; use crate::panic::AT_EXIT; use crate::parse_constants::SourceRange; -use crate::parse_constants::{ParseTreeFlags, ParserTestErrorBits}; +use crate::parse_constants::{ParseIssue, ParseTreeFlags}; use crate::parse_util::{ MaybeParentheses, SPACES_PER_INDENT, compute_indents, contains_wildcards, detect_parse_errors, escape_string_with_quote, escape_wildcards, get_cmdsubst_extent, get_line_from_offset, @@ -4514,10 +4514,10 @@ fn handle_execute(&mut self) -> bool { // Expand the command line in preparation for execution. // to_exec is the command to execute; the command line itself has the command for history. let test_res = self.expand_for_execute(); - if let Err(err) = test_res { - if err.contains(ParserTestErrorBits::ERROR) { + if let Err(p) = test_res { + if p.error { return false; - } else if err.contains(ParserTestErrorBits::INCOMPLETE) { + } else if p.incomplete { self.insert_char(elt, '\n'); return true; } @@ -4537,7 +4537,7 @@ fn handle_execute(&mut self) -> bool { // Expand abbreviations before execution. // Replace the command line with any abbreviations as needed. // Return the test result, which may be incomplete to insert a newline, or an error. - fn expand_for_execute(&mut self) -> Result<(), ParserTestErrorBits> { + fn expand_for_execute(&mut self) -> Result<(), ParseIssue> { // Expand abbreviations at the cursor. // The first expansion is "user visible" and enters into history. let el = &self.command_line; @@ -4548,7 +4548,7 @@ fn expand_for_execute(&mut self) -> Result<(), ParserTestErrorBits> { // syntactically invalid but become valid after expanding abbreviations. if self.conf.syntax_check_ok { test_res = reader_shell_test(self.parser, el.text()); - if test_res.is_err_and(|err| err.contains(ParserTestErrorBits::ERROR)) { + if test_res.is_err_and(|p| p.error) { return test_res; } } @@ -6378,11 +6378,11 @@ fn reader_run_command(parser: &Parser, cmd: &wstr) -> EvalRes { eval_res } -fn reader_shell_test(parser: &Parser, bstr: &wstr) -> Result<(), ParserTestErrorBits> { +fn reader_shell_test(parser: &Parser, bstr: &wstr) -> Result<(), ParseIssue> { let mut errors = vec![]; let res = detect_parse_errors(bstr, Some(&mut errors), /*accept_incomplete=*/ true); - if res.is_err_and(|err| err.contains(ParserTestErrorBits::ERROR)) { + if res.is_err_and(|p| p.error) { let mut error_desc = parser.get_backtrace(bstr, &errors); // Ensure we end with a newline. Also add an initial newline, because it's likely the user