From 0794ebf5fae7d5fb3c4ffab8c6a67946f1d844ac Mon Sep 17 00:00:00 2001 From: Fernando Herrera Date: Fri, 10 Sep 2021 08:28:43 +0100 Subject: [PATCH 1/6] error parsing for def, alias and let --- crates/nu-cli/src/errors.rs | 5 +- crates/nu-parser/src/parser.rs | 134 +++++++++++++++++----------- crates/nu-protocol/src/ast/block.rs | 12 +++ 3 files changed, 98 insertions(+), 53 deletions(-) diff --git a/crates/nu-cli/src/errors.rs b/crates/nu-cli/src/errors.rs index 77ddfd8536..1c25e5d2e4 100644 --- a/crates/nu-cli/src/errors.rs +++ b/crates/nu-cli/src/errors.rs @@ -208,8 +208,9 @@ pub fn report_parsing_error( let (diag_file_id, diag_range) = convert_span_to_diag(working_set, span)?; Diagnostic::error() .with_message("Unknown state") - .with_labels(vec![Label::primary(diag_file_id, diag_range) - .with_message(format!("unknown state {}", name))]) + .with_labels(vec![ + Label::primary(diag_file_id, diag_range).with_message(format!("{}", name)) + ]) } ParseError::NonUtf8(span) => { let (diag_file_id, diag_range) = convert_span_to_diag(working_set, span)?; diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index 57506b7fd2..119f14c3e5 100644 --- a/crates/nu-parser/src/parser.rs +++ b/crates/nu-parser/src/parser.rs @@ -26,6 +26,10 @@ fn garbage(span: Span) -> Expression { Expression::garbage(span) } +fn garbage_statement(spans: &[Span]) -> Statement { + Statement::Pipeline(Pipeline::from_vec(vec![garbage(span(spans))])) +} + fn is_identifier_byte(b: u8) -> bool { b != b'.' && b != b'[' && b != b'(' && b != b'{' } @@ -59,6 +63,22 @@ fn check_call(command: Span, sig: &Signature, call: &Call) -> Option } } +fn check_name(working_set: &mut StateWorkingSet, spans: &[Span]) -> Option { + if spans[1..].len() < 2 { + Some(ParseError::UnknownState( + "missing definition name".into(), + span(spans), + )) + } else if working_set.get_span_contents(spans[2]) != b"=" { + Some(ParseError::UnknownState( + "missing equal sign in definition".into(), + spans[2], + )) + } else { + None + } +} + pub fn parse_external_call( working_set: &mut StateWorkingSet, spans: &[Span], @@ -2325,6 +2345,13 @@ pub fn parse_def( error = error.or(err); working_set.exit_scope(); + if error.is_some() { + return ( + Statement::Pipeline(Pipeline::from_vec(vec![garbage(span(spans))])), + error, + ); + } + let name = name_expr.as_string(); let signature = sig.as_signature(); @@ -2364,23 +2391,15 @@ pub fn parse_def( ) } _ => ( - Statement::Pipeline(Pipeline::from_vec(vec![Expression { - expr: Expr::Garbage, - span: span(spans), - ty: Type::Unknown, - }])), + Statement::Pipeline(Pipeline::from_vec(vec![garbage(span(spans))])), error, ), } } else { ( - Statement::Pipeline(Pipeline::from_vec(vec![Expression { - expr: Expr::Garbage, - span: span(spans), - ty: Type::Unknown, - }])), + garbage_statement(spans), Some(ParseError::UnknownState( - "internal error: definition unparseable".into(), + "definition unparseable. Expected structure: def [] {}".into(), span(spans), )), ) @@ -2394,6 +2413,13 @@ pub fn parse_alias( let name = working_set.get_span_contents(spans[0]); if name == b"alias" { + if let Some(err) = check_name(working_set, spans) { + return ( + Statement::Pipeline(Pipeline::from_vec(vec![garbage(span(spans))])), + Some(err), + ); + } + if let Some(decl_id) = working_set.find_decl(b"alias") { let (call, call_span, _) = parse_internal_call(working_set, spans[0], &spans[1..], decl_id); @@ -2430,11 +2456,7 @@ pub fn parse_alias( } ( - Statement::Pipeline(Pipeline::from_vec(vec![Expression { - expr: Expr::Garbage, - span: span(spans), - ty: Type::Unknown, - }])), + garbage_statement(spans), Some(ParseError::UnknownState( "internal error: let statement unparseable".into(), span(spans), @@ -2449,6 +2471,13 @@ pub fn parse_let( let name = working_set.get_span_contents(spans[0]); if name == b"let" { + if let Some(err) = check_name(working_set, spans) { + return ( + Statement::Pipeline(Pipeline::from_vec(vec![garbage(span(spans))])), + Some(err), + ); + } + 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); @@ -2474,11 +2503,7 @@ pub fn parse_let( } } ( - Statement::Pipeline(Pipeline::from_vec(vec![Expression { - expr: Expr::Garbage, - span: span(spans), - ty: Type::Unknown, - }])), + garbage_statement(spans), Some(ParseError::UnknownState( "internal error: let statement unparseable".into(), span(spans), @@ -2490,16 +2515,16 @@ pub fn parse_statement( working_set: &mut StateWorkingSet, spans: &[Span], ) -> (Statement, Option) { - // FIXME: improve errors by checking keyword first - if let (decl, None) = parse_def(working_set, spans) { - (decl, None) - } else if let (stmt, None) = parse_let(working_set, spans) { - (stmt, None) - } else if let (stmt, None) = parse_alias(working_set, spans) { - (stmt, None) - } else { - let (expr, err) = parse_expression(working_set, spans); - (Statement::Pipeline(Pipeline::from_vec(vec![expr])), err) + let name = working_set.get_span_contents(spans[0]); + + match name { + b"def" => parse_def(working_set, spans), + b"let" => parse_let(working_set, spans), + b"alias" => parse_alias(working_set, spans), + _ => { + let (expr, err) = parse_expression(working_set, spans); + (Statement::Pipeline(Pipeline::from_vec(vec![expr])), err) + } } } @@ -2508,13 +2533,10 @@ pub fn parse_block( lite_block: &LiteBlock, scoped: bool, ) -> (Block, Option) { - let mut error = None; if scoped { working_set.enter_scope(); } - let mut block = Block::new(); - // Pre-declare any definition so that definitions // that share the same block can see each other for pipeline in &lite_block.block { @@ -2523,25 +2545,35 @@ pub fn parse_block( } } - for pipeline in &lite_block.block { - if pipeline.commands.len() > 1 { - let mut output = vec![]; - for command in &pipeline.commands { - let (expr, err) = parse_expression(working_set, &command.parts); - error = error.or(err); + let mut error = None; - output.push(expr); + let block: Block = lite_block + .block + .iter() + .map(|pipeline| { + if pipeline.commands.len() > 1 { + let output = pipeline + .commands + .iter() + .map(|command| { + let (expr, err) = parse_expression(working_set, &command.parts); + error = err.map(|err| err); + + expr + }) + .collect::>(); + + Statement::Pipeline(Pipeline { + expressions: output, + }) + } else { + let (stmt, err) = parse_statement(working_set, &pipeline.commands[0].parts); + error = err.map(|err| err); + + stmt } - block.stmts.push(Statement::Pipeline(Pipeline { - expressions: output, - })); - } else { - let (stmt, err) = parse_statement(working_set, &pipeline.commands[0].parts); - error = error.or(err); - - block.stmts.push(stmt); - } - } + }) + .into(); if scoped { working_set.exit_scope(); diff --git a/crates/nu-protocol/src/ast/block.rs b/crates/nu-protocol/src/ast/block.rs index d36e43d8ec..e5db26da36 100644 --- a/crates/nu-protocol/src/ast/block.rs +++ b/crates/nu-protocol/src/ast/block.rs @@ -48,3 +48,15 @@ impl Block { } } } + +impl From for Block +where + T: Iterator, +{ + fn from(stmts: T) -> Self { + Self { + signature: Box::new(Signature::new("")), + stmts: stmts.collect(), + } + } +} From 9a16a8fd0698fea63349710bfcea3dfbdc4cbe29 Mon Sep 17 00:00:00 2001 From: Fernando Herrera Date: Fri, 10 Sep 2021 08:44:31 +0100 Subject: [PATCH 2/6] corrected error check --- crates/nu-parser/src/parser.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index fd8260c2d0..46978a162d 100644 --- a/crates/nu-parser/src/parser.rs +++ b/crates/nu-parser/src/parser.rs @@ -2610,7 +2610,10 @@ pub fn parse_block( .iter() .map(|command| { let (expr, err) = parse_expression(working_set, &command.parts); - error = err.map(|err| err); + + if error.is_none() { + error = err; + } expr }) @@ -2621,7 +2624,10 @@ pub fn parse_block( }) } else { let (stmt, err) = parse_statement(working_set, &pipeline.commands[0].parts); - error = err.map(|err| err); + + if error.is_none() { + error = err; + } stmt } From 198c884158c301e76becc7d631d5ff8e9404f66c Mon Sep 17 00:00:00 2001 From: Fernando Herrera Date: Sat, 11 Sep 2021 08:22:41 +0100 Subject: [PATCH 3/6] change name in error --- crates/nu-parser/src/parser.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index 46978a162d..9fba1a2d87 100644 --- a/crates/nu-parser/src/parser.rs +++ b/crates/nu-parser/src/parser.rs @@ -2511,7 +2511,7 @@ pub fn parse_alias( ( garbage_statement(spans), Some(ParseError::UnknownState( - "internal error: let statement unparseable".into(), + "internal error: alias statement unparseable".into(), span(spans), )), ) From 4b8ba29cdb66039e48aa6bfa46ae935e62686c17 Mon Sep 17 00:00:00 2001 From: Fernando Herrera Date: Sat, 11 Sep 2021 13:07:19 +0100 Subject: [PATCH 4/6] check for = before internal parsing --- crates/nu-parser/src/parser.rs | 39 ++++++++++++++++++++++++---------- 1 file changed, 28 insertions(+), 11 deletions(-) diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index fcfbe7e34f..16ee5aca75 100644 --- a/crates/nu-parser/src/parser.rs +++ b/crates/nu-parser/src/parser.rs @@ -72,7 +72,7 @@ fn check_name(working_set: &mut StateWorkingSet, spans: &[Span]) -> Option 1 { + let test_equal = working_set.get_span_contents(spans[1]); + + if test_equal == &[b'='] { + return ( + garbage(Span::new(0, 0)), + Some(ParseError::UnknownState( + "internal error: incomplete statement".into(), + span(spans), + )), + ); + } + } + // parse internal command let (call, _, err) = parse_internal_call(working_set, span(&spans[0..pos]), &spans[pos..], decl_id); @@ -2568,16 +2585,16 @@ pub fn parse_statement( working_set: &mut StateWorkingSet, spans: &[Span], ) -> (Statement, Option) { - let name = working_set.get_span_contents(spans[0]); - - match name { - b"def" => parse_def(working_set, spans), - b"let" => parse_let(working_set, spans), - b"alias" => parse_alias(working_set, spans), - _ => { - let (expr, err) = parse_expression(working_set, spans); - (Statement::Pipeline(Pipeline::from_vec(vec![expr])), err) - } + // FIXME: improve errors by checking keyword first + if let (decl, None) = parse_def(working_set, spans) { + (decl, None) + } else if let (stmt, None) = parse_let(working_set, spans) { + (stmt, None) + } else if let (stmt, None) = parse_alias(working_set, spans) { + (stmt, None) + } else { + let (expr, err) = parse_expression(working_set, spans); + (Statement::Pipeline(Pipeline::from_vec(vec![expr])), err) } } From 9c98783917ab43a6707d09b18ceb64bb71fce995 Mon Sep 17 00:00:00 2001 From: Fernando Herrera Date: Sat, 11 Sep 2021 13:16:40 +0100 Subject: [PATCH 5/6] clippy correcgtions --- crates/nu-cli/src/errors.rs | 2 +- crates/nu-parser/src/parser.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/nu-cli/src/errors.rs b/crates/nu-cli/src/errors.rs index 1c25e5d2e4..b5470ead13 100644 --- a/crates/nu-cli/src/errors.rs +++ b/crates/nu-cli/src/errors.rs @@ -209,7 +209,7 @@ pub fn report_parsing_error( Diagnostic::error() .with_message("Unknown state") .with_labels(vec![ - Label::primary(diag_file_id, diag_range).with_message(format!("{}", name)) + Label::primary(diag_file_id, diag_range).with_message(name.to_string()) ]) } ParseError::NonUtf8(span) => { diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index 16ee5aca75..5c03e08ddb 100644 --- a/crates/nu-parser/src/parser.rs +++ b/crates/nu-parser/src/parser.rs @@ -606,7 +606,7 @@ pub fn parse_call( if spans.len() > 1 { let test_equal = working_set.get_span_contents(spans[1]); - if test_equal == &[b'='] { + if test_equal == [b'='] { return ( garbage(Span::new(0, 0)), Some(ParseError::UnknownState( From 66c58217af28d652c8fba519830afa1b92abcb27 Mon Sep 17 00:00:00 2001 From: Fernando Herrera Date: Sun, 12 Sep 2021 16:36:16 +0100 Subject: [PATCH 6/6] change message --- crates/nu-parser/src/parser.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index 5c03e08ddb..f4fd1348fd 100644 --- a/crates/nu-parser/src/parser.rs +++ b/crates/nu-parser/src/parser.rs @@ -610,7 +610,7 @@ pub fn parse_call( return ( garbage(Span::new(0, 0)), Some(ParseError::UnknownState( - "internal error: incomplete statement".into(), + "Incomplete statement".into(), span(spans), )), );