From 02fcc485fbe8947b709d91f8a27b73385311ceaa Mon Sep 17 00:00:00 2001 From: zc he Date: Wed, 26 Mar 2025 22:02:26 +0800 Subject: [PATCH] fix(parser): skip eval_const if parsing errors detected to avoid panic (#15364) Fixes #14972 #15321 #14706 # Description Early returns `NotAConstant` if parsing errors exist in the subexpression. I'm not sure when the span of a block will be None, and whether there're better ways to handle none block spans, like a more suitable ShellError type. # User-Facing Changes # Tests + Formatting +1, but possibly not the easiest way to do it. # After Submitting --- crates/nu-parser/tests/test_parser.rs | 92 ++++++++++++++++++- .../nu-protocol/src/errors/shell_error/mod.rs | 19 ++++ crates/nu-protocol/src/eval_const.rs | 8 ++ 3 files changed, 118 insertions(+), 1 deletion(-) diff --git a/crates/nu-parser/tests/test_parser.rs b/crates/nu-parser/tests/test_parser.rs index 30b4c57728..945190014e 100644 --- a/crates/nu-parser/tests/test_parser.rs +++ b/crates/nu-parser/tests/test_parser.rs @@ -6,7 +6,7 @@ use nu_protocol::{ }; use rstest::rstest; -use mock::{Alias, AttrEcho, Def, Let, Mut, ToCustom}; +use mock::{Alias, AttrEcho, Const, Def, IfMocked, Let, Mut, ToCustom}; fn test_int( test_tag: &str, // name of sub-test @@ -784,6 +784,38 @@ pub fn parse_attributes_external_alias() { assert!(parse_error.contains("Encountered error during parse-time evaluation")); } +#[test] +pub fn parse_if_in_const_expression() { + // https://github.com/nushell/nushell/issues/15321 + let engine_state = EngineState::new(); + let mut working_set = StateWorkingSet::new(&engine_state); + + working_set.add_decl(Box::new(Const)); + working_set.add_decl(Box::new(Def)); + working_set.add_decl(Box::new(IfMocked)); + + let source = b"const foo = if t"; + let _ = parse(&mut working_set, None, source, false); + + assert!(!working_set.parse_errors.is_empty()); + let ParseError::MissingPositional(error, _, _) = &working_set.parse_errors[0] else { + panic!("Expected MissingPositional"); + }; + + assert!(error.contains("cond")); + + working_set.parse_errors = Vec::new(); + let source = b"def a [n= (if ]"; + let _ = parse(&mut working_set, None, source, false); + + assert!(!working_set.parse_errors.is_empty()); + let ParseError::UnexpectedEof(error, _) = &working_set.parse_errors[0] else { + panic!("Expected UnexpectedEof"); + }; + + assert!(error.contains(")")); +} + fn test_external_call(input: &str, tag: &str, f: impl FnOnce(&Expression, &[ExternalArgument])) { let engine_state = EngineState::new(); let mut working_set = StateWorkingSet::new(&engine_state); @@ -1969,6 +2001,51 @@ mod mock { engine::Call, Category, IntoPipelineData, PipelineData, ShellError, Type, Value, }; + #[derive(Clone)] + pub struct Const; + + impl Command for Const { + fn name(&self) -> &str { + "const" + } + + fn description(&self) -> &str { + "Create a parse-time constant." + } + + fn signature(&self) -> nu_protocol::Signature { + Signature::build("const") + .input_output_types(vec![(Type::Nothing, Type::Nothing)]) + .allow_variants_without_examples(true) + .required("const_name", SyntaxShape::VarWithOptType, "Constant name.") + .required( + "initial_value", + SyntaxShape::Keyword(b"=".to_vec(), Box::new(SyntaxShape::MathExpression)), + "Equals sign followed by constant value.", + ) + .category(Category::Core) + } + + fn run( + &self, + _engine_state: &EngineState, + _stack: &mut Stack, + _call: &Call, + _input: PipelineData, + ) -> Result { + todo!() + } + + fn run_const( + &self, + _working_set: &StateWorkingSet, + _call: &Call, + _input: PipelineData, + ) -> Result { + Ok(PipelineData::empty()) + } + } + #[derive(Clone)] pub struct Let; @@ -2422,6 +2499,19 @@ mod mock { ) -> Result { todo!() } + + fn is_const(&self) -> bool { + true + } + + fn run_const( + &self, + _working_set: &StateWorkingSet, + _call: &Call, + _input: PipelineData, + ) -> Result { + panic!("Should not be called!") + } } } diff --git a/crates/nu-protocol/src/errors/shell_error/mod.rs b/crates/nu-protocol/src/errors/shell_error/mod.rs index 627c3679b3..84c23cd208 100644 --- a/crates/nu-protocol/src/errors/shell_error/mod.rs +++ b/crates/nu-protocol/src/errors/shell_error/mod.rs @@ -1140,6 +1140,25 @@ This is an internal Nushell error, please file an issue https://github.com/nushe span: Span, }, + /// TODO: Get rid of this error by moving the check before evaluation + /// + /// Tried evaluating of a subexpression with parsing error + /// + /// ## Resolution + /// + /// Fix the parsing error first. + #[error("Found parsing error in expression.")] + #[diagnostic( + code(nu::shell::parse_error_in_constant), + help( + "This expression is supposed to be evaluated into a constant, which means error-free." + ) + )] + ParseErrorInConstant { + #[label("Parsing error detected in expression")] + span: Span, + }, + /// Tried assigning non-constant value to a constant /// /// ## Resolution diff --git a/crates/nu-protocol/src/eval_const.rs b/crates/nu-protocol/src/eval_const.rs index 86ce4d7385..15ad479e19 100644 --- a/crates/nu-protocol/src/eval_const.rs +++ b/crates/nu-protocol/src/eval_const.rs @@ -508,6 +508,14 @@ impl Eval for EvalConst { block_id: BlockId, span: Span, ) -> Result { + // If parsing errors exist in the subexpression, don't bother to evaluate it. + if working_set + .parse_errors + .iter() + .any(|error| span.contains_span(error.span())) + { + return Err(ShellError::ParseErrorInConstant { span }); + } // TODO: Allow debugging const eval let block = working_set.get_block(block_id); eval_const_subexpression(working_set, block, PipelineData::empty(), span)?.into_value(span)