h/fish-shell
1
Fork 0
mirror of https://github.com/fish-shell/fish-shell.git synced 2026-06-07 18:21:16 -03:00
Code Issues Packages Projects Releases Wiki Activity

Cleanup ParserTestErrorBits

Browse Source 
This was a weird type that made sense in C++ but not so much in Rust.
Let's clean this up.
This commit is contained in:
Peter Ammon
2026-02-16 22:29:22 -08:00
parent 89c2f1bd6b
commit b65649725e
5 changed files with 110 additions and 93 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
src/builtins/commandline.rs
View File

@@ -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),
};
}

41
src/parse_constants.rs
View File

@@ -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;
}
}

97
src/parse_util.rs
View File

@@ -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(

38
src/parser.rs
View File

@@ -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"
);
}

16
src/reader/reader.rs
View File

@@ -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