From cac2875c964ff7ce90122b943a98535f4ac94236 Mon Sep 17 00:00:00 2001 From: JT Date: Fri, 25 Jun 2021 17:18:38 +1200 Subject: [PATCH] Improve def parse errors (#3681) --- crates/nu-errors/src/lib.rs | 25 +++--- crates/nu-parser/src/parse.rs | 9 +- crates/nu-parser/src/parse/def.rs | 143 +++++++++++++++++++----------- 3 files changed, 109 insertions(+), 68 deletions(-) diff --git a/crates/nu-errors/src/lib.rs b/crates/nu-errors/src/lib.rs index 22c25e20ae..6d78484ed2 100644 --- a/crates/nu-errors/src/lib.rs +++ b/crates/nu-errors/src/lib.rs @@ -32,7 +32,10 @@ pub enum ParseErrorReason { Unclosed { delimiter: String, span: Span }, /// An unexpected internal error has occurred - InternalError { message: Spanned }, + GeneralError { + message: String, + label: Spanned, + }, /// The parser tried to parse an argument for a command, but it failed for /// some reason @@ -83,11 +86,15 @@ impl ParseError { } } - /// Construct a [ParseErrorReason::InternalError](ParseErrorReason::InternalError) - pub fn internal_error(message: Spanned>) -> ParseError { + /// Construct a [ParseErrorReason::GeneralError](ParseErrorReason::GeneralError) + pub fn general_error( + message: impl Into, + label: Spanned>, + ) -> ParseError { ParseError { - reason: ParseErrorReason::InternalError { - message: message.item.into().spanned(message.span), + reason: ParseErrorReason::GeneralError { + message: message.into(), + label: label.item.into().spanned(label.span), }, } } @@ -119,11 +126,9 @@ impl From for ShellError { ParseErrorReason::Mismatch { actual, expected } => { ShellError::type_error(expected, actual) } - ParseErrorReason::InternalError { message } => ShellError::labeled_error( - format!("Internal error: {}", message.item), - &message.item, - &message.span, - ), + ParseErrorReason::GeneralError { message, label } => { + ShellError::labeled_error(&message, &label.item, &label.span) + } ParseErrorReason::ArgumentError { command, error } => { ShellError::argument_error(command, error) } diff --git a/crates/nu-parser/src/parse.rs b/crates/nu-parser/src/parse.rs index 35af79b031..ae1db7cc94 100644 --- a/crates/nu-parser/src/parse.rs +++ b/crates/nu-parser/src/parse.rs @@ -1853,13 +1853,8 @@ fn parse_call( // Check if it's an internal command if let Some(signature) = scope.get_signature(&lite_cmd.parts[0].item) { if lite_cmd.parts[0].item == "def" { - let error = parse_definition(&lite_cmd, scope); - if error.is_some() { - return ( - Some(ClassifiedCommand::Expr(Box::new(garbage(lite_cmd.span())))), - error, - ); - } + let err = parse_definition(&lite_cmd, scope); + error = error.or(err); } let (mut internal_command, err) = parse_internal_command(&lite_cmd, scope, &signature, 0); diff --git a/crates/nu-parser/src/parse/def.rs b/crates/nu-parser/src/parse/def.rs index 06151c66da..bf8d134949 100644 --- a/crates/nu-parser/src/parse/def.rs +++ b/crates/nu-parser/src/parse/def.rs @@ -8,7 +8,7 @@ use crate::{ use indexmap::IndexMap; use nu_errors::ParseError; use nu_protocol::hir::Block; -use nu_source::{HasSpan, SpannedItem}; +use nu_source::{HasSpan, Span, SpannedItem}; //use crate::errors::{ParseError, ParseResult}; use crate::lex::lexer::{lex, parse_block}; @@ -28,59 +28,100 @@ pub(crate) fn parse_definition(call: &LiteCommand, scope: &dyn ParserScope) -> O // So our main goal here is to parse the block now that the names and // prototypes of adjacent commands are also available - if call.parts.len() == 4 { - if call.parts[0].item != "def" { - return Some(ParseError::mismatch("definition", call.parts[0].clone())); - } - - let name = trim_quotes(&call.parts[1].item); - let (mut signature, err) = parse_signature(&name, &call.parts[2]); - - //Add commands comments to signature usage - signature.usage = call.comments_joined(); - - if err.is_some() { - return err; - }; - - let mut chars = call.parts[3].chars(); - match (chars.next(), chars.next_back()) { - (Some('{'), Some('}')) => { - // We have a literal block - let string: String = chars.collect(); - - scope.enter_scope(); - - let (tokens, err) = lex(&string, call.parts[3].span.start() + 1); - if err.is_some() { - return err; - }; - let (lite_block, err) = parse_block(tokens); - if err.is_some() { - return err; - }; - - let (mut block, err) = classify_block(&lite_block, scope); - scope.exit_scope(); - - if let Some(block) = std::sync::Arc::::get_mut(&mut block) - { - block.params = signature; - block.params.name = name; - } - - scope.add_definition(block); - - err + match call.parts.len() { + 4 => { + if call.parts[0].item != "def" { + return Some(ParseError::mismatch("definition", call.parts[0].clone())); + } + + let name = trim_quotes(&call.parts[1].item); + let (mut signature, err) = parse_signature(&name, &call.parts[2]); + + //Add commands comments to signature usage + signature.usage = call.comments_joined(); + + if err.is_some() { + return err; + }; + + let mut chars = call.parts[3].chars(); + match (chars.next(), chars.next_back()) { + (Some('{'), Some('}')) => { + // We have a literal block + let string: String = chars.collect(); + + scope.enter_scope(); + + let (tokens, err) = lex(&string, call.parts[3].span.start() + 1); + if err.is_some() { + return err; + }; + let (lite_block, err) = parse_block(tokens); + if err.is_some() { + return err; + }; + + let (mut block, err) = classify_block(&lite_block, scope); + scope.exit_scope(); + + if let Some(block) = + std::sync::Arc::::get_mut(&mut block) + { + block.params = signature; + block.params.name = name; + } + + scope.add_definition(block); + + err + } + _ => Some(ParseError::mismatch("body", call.parts[3].clone())), } - _ => Some(ParseError::mismatch("body", call.parts[3].clone())), } - } else { - Some(ParseError::internal_error( - "Wrong shape. Expected def name [signature] {body}." + + 3 => Some(ParseError::general_error( + "wrong shape. Expected: def name [signature] {body}", + "expected definition body".to_string().spanned(Span::new( + call.parts[2].span.end(), + call.parts[2].span.end(), + )), + )), + 2 => Some(ParseError::general_error( + "wrong shape. Expected: def name [signature] {body}", + "expected definition parameters" .to_string() - .spanned(call.span()), - )) + .spanned(Span::new( + call.parts[1].span.end(), + call.parts[1].span.end(), + )), + )), + 1 => Some(ParseError::general_error( + "wrong shape. Expected: def name [signature] {body}", + "expected definition name".to_string().spanned(Span::new( + call.parts[0].span.end(), + call.parts[0].span.end(), + )), + )), + 0 => Some(ParseError::general_error( + "wrong shape. Expected: def name [signature] {body}", + "expected 'def' keyword'".to_string().spanned(call.span()), + )), + + x if x < 4 => Some(ParseError::general_error( + "wrong shape. Expected: def name [signature] {body}", + "expected: def name [signature] {body}" + .to_string() + .spanned(Span::new( + call.parts[x - 1].span.end(), + call.parts[x - 1].span.end(), + )), + )), + _ => Some(ParseError::general_error( + "extra arguments given. Expected: def name [signature] {body}.", + "extra argument given" + .to_string() + .spanned(call.parts[4].span()), + )), } }