From 1d74d9c5aeb15814fe8bbe0a62a382ba55517e5d Mon Sep 17 00:00:00 2001 From: JT <547158+jntrnr@users.noreply.github.com> Date: Thu, 16 Dec 2021 09:56:12 +1100 Subject: [PATCH] Fix comment issue and shadowing issue (#501) --- crates/nu-parser/src/lex.rs | 13 ++--- crates/nu-parser/src/parse_keywords.rs | 75 ++++++++++++++++++++------ crates/nu-parser/src/parser.rs | 2 +- crates/nu-protocol/src/shell_error.rs | 2 +- src/tests.rs | 17 ++++++ 5 files changed, 80 insertions(+), 29 deletions(-) diff --git a/crates/nu-parser/src/lex.rs b/crates/nu-parser/src/lex.rs index ef2ddde4a1..e374a29042 100644 --- a/crates/nu-parser/src/lex.rs +++ b/crates/nu-parser/src/lex.rs @@ -276,25 +276,18 @@ pub fn lex( let mut start = curr_offset; while let Some(input) = input.get(curr_offset) { - curr_offset += 1; if *input == b'\n' || *input == b'\r' { if !skip_comment { output.push(Token::new( TokenContents::Comment, - Span::new(start, curr_offset - 1), - )); - - // Adding an end of line token after a comment - // This helps during lite_parser to avoid losing a command - // in a statement - output.push(Token::new( - TokenContents::Eol, - Span::new(curr_offset - 1, curr_offset), + Span::new(start, curr_offset), )); } start = curr_offset; break; + } else { + curr_offset += 1; } } if start != curr_offset && !skip_comment { diff --git a/crates/nu-parser/src/parse_keywords.rs b/crates/nu-parser/src/parse_keywords.rs index a3253c1d35..73c15e08a3 100644 --- a/crates/nu-parser/src/parse_keywords.rs +++ b/crates/nu-parser/src/parse_keywords.rs @@ -13,7 +13,8 @@ use crate::{ lex, lite_parse, parser::{ check_call, check_name, garbage, garbage_statement, parse, parse_block_expression, - parse_import_pattern, parse_internal_call, parse_signature, parse_string, trim_quotes, + parse_import_pattern, parse_internal_call, parse_multispan_value, parse_signature, + parse_string, parse_var_with_opt_type, trim_quotes, }, ParseError, }; @@ -956,28 +957,68 @@ pub fn parse_let( } if let Some(decl_id) = working_set.find_decl(b"let") { - let (call, call_span, err) = - parse_internal_call(working_set, spans[0], &spans[1..], decl_id); + if spans.len() >= 4 { + // This is a bit of by-hand parsing to get around the issue where we want to parse in the reverse order + // so that the var-id created by the variable isn't visible in the expression that init it + for span in spans.iter().enumerate() { + let item = working_set.get_span_contents(*span.1); + if item == b"=" && spans.len() > (span.0 + 1) { + let mut error = None; - // Update the variable to the known type if we can. - if err.is_none() { - let var_id = call.positional[0] - .as_var() - .expect("internal error: expected variable"); - let rhs_type = call.positional[1].ty.clone(); + let mut idx = span.0; + let (rvalue, err) = parse_multispan_value( + working_set, + spans, + &mut idx, + &SyntaxShape::Keyword(b"=".to_vec(), Box::new(SyntaxShape::Expression)), + ); + error = error.or(err); - if var_id != CONFIG_VARIABLE_ID { - working_set.set_variable_type(var_id, rhs_type); + let mut idx = 0; + let (lvalue, err) = + parse_var_with_opt_type(working_set, &spans[1..(span.0)], &mut idx); + error = error.or(err); + + let var_id = lvalue.as_var(); + + let rhs_type = rvalue.ty.clone(); + + if let Some(var_id) = var_id { + if var_id != CONFIG_VARIABLE_ID { + working_set.set_variable_type(var_id, rhs_type); + } + } + + let call = Box::new(Call { + decl_id, + head: spans[0], + positional: vec![lvalue, rvalue], + named: vec![], + }); + + return ( + Statement::Pipeline(Pipeline::from_vec(vec![Expression { + expr: Expr::Call(call), + span: nu_protocol::span(spans), + ty: Type::Unknown, + custom_completion: None, + }])), + error, + ); + } } } + let (call, _, err) = parse_internal_call(working_set, spans[0], &spans[1..], decl_id); return ( - Statement::Pipeline(Pipeline::from_vec(vec![Expression { - expr: Expr::Call(call), - span: call_span, - ty: Type::Unknown, - custom_completion: None, - }])), + Statement::Pipeline(Pipeline { + expressions: vec![Expression { + expr: Expr::Call(call), + span: nu_protocol::span(spans), + ty: Type::Unknown, + custom_completion: None, + }], + }), err, ); } diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index d7fe34a6f8..63ccd8eb3a 100644 --- a/crates/nu-parser/src/parser.rs +++ b/crates/nu-parser/src/parser.rs @@ -388,7 +388,7 @@ fn calculate_end_span( } } -fn parse_multispan_value( +pub fn parse_multispan_value( working_set: &mut StateWorkingSet, spans: &[Span], spans_idx: &mut usize, diff --git a/crates/nu-protocol/src/shell_error.rs b/crates/nu-protocol/src/shell_error.rs index 2a719747de..a11e19b7cd 100644 --- a/crates/nu-protocol/src/shell_error.rs +++ b/crates/nu-protocol/src/shell_error.rs @@ -91,7 +91,7 @@ pub enum ShellError { #[diagnostic(code(nu::shell::nushell_failed), url(docsrs))] NushellFailed(String), - #[error("Variable not found!!!")] + #[error("Variable not found")] #[diagnostic(code(nu::shell::variable_not_found), url(docsrs))] VariableNotFoundAtRuntime(#[label = "variable not found"] Span), diff --git a/src/tests.rs b/src/tests.rs index 1077bd811a..b3e4b999ad 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -1249,3 +1249,20 @@ fn command_drop_column_1() -> TestResult { fn chained_operator_typecheck() -> TestResult { run_test("1 != 2 && 3 != 4 && 5 != 6", "true") } + +#[test] +fn proper_shadow() -> TestResult { + run_test("let x = 10; let x = $x + 9; $x", "19") +} + +#[test] +fn comment_multiline() -> TestResult { + run_test( + r#"def foo [] { + let x = 1 + 2 # comment + let y = 3 + 4 # another comment + $x + $y + }; foo"#, + "10", + ) +}