From e66fd9104507f7bfcc5926c1fb3f3b9cb1fce1b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Mon, 18 Oct 2021 23:19:25 +0300 Subject: [PATCH 01/20] Move module block parsing into its own function --- crates/nu-parser/src/parse_keywords.rs | 167 +++++++++++++------------ 1 file changed, 84 insertions(+), 83 deletions(-) diff --git a/crates/nu-parser/src/parse_keywords.rs b/crates/nu-parser/src/parse_keywords.rs index b086ed223f..3cad00023d 100644 --- a/crates/nu-parser/src/parse_keywords.rs +++ b/crates/nu-parser/src/parse_keywords.rs @@ -309,6 +309,89 @@ pub fn parse_export( } } +pub fn parse_module_block( + working_set: &mut StateWorkingSet, + spans: &[Span], +) -> (Block, Option) { + let mut error = None; + + working_set.enter_scope(); + + let source = working_set.get_span_contents(spans[0]); + + let (output, err) = lex(source, spans[0].start, &[], &[]); + error = error.or(err); + + let (output, err) = lite_parse(&output); + error = error.or(err); + + for pipeline in &output.block { + if pipeline.commands.len() == 1 { + parse_def_predecl(working_set, &pipeline.commands[0].parts); + } + } + + let mut exports: Vec<(Vec, DeclId)> = vec![]; + + let block: Block = output + .block + .iter() + .map(|pipeline| { + if pipeline.commands.len() == 1 { + // this one here is doing parse_statement() equivalent + // let (stmt, err) = parse_statement(working_set, &pipeline.commands[0].parts); + let name = working_set.get_span_contents(pipeline.commands[0].parts[0]); + + let (stmt, err) = match name { + b"def" => { + let (stmt, err) = parse_def(working_set, &pipeline.commands[0].parts); + + (stmt, err) + } + b"export" => { + let (stmt, err) = + parse_export(working_set, &pipeline.commands[0].parts); + + if err.is_none() { + let decl_name = + // parts[2] is safe since it's checked in parse_export already + working_set.get_span_contents(pipeline.commands[0].parts[2]); + + let decl_id = working_set + .find_decl(decl_name) + .expect("internal error: failed to find added declaration"); + + exports.push((decl_name.into(), decl_id)); + } + + (stmt, err) + } + _ => ( + garbage_statement(&pipeline.commands[0].parts), + Some(ParseError::Expected( + "def or export keyword".into(), + pipeline.commands[0].parts[0], + )), + ), + }; + + if error.is_none() { + error = err; + } + + stmt + } else { + error = Some(ParseError::Expected("not a pipeline".into(), spans[0])); + garbage_statement(spans) + } + }) + .into(); + + working_set.exit_scope(); + + (block.with_exports(exports), error) +} + pub fn parse_module( working_set: &mut StateWorkingSet, spans: &[Span], @@ -359,91 +442,9 @@ pub fn parse_module( let block_span = Span { start, end }; - let source = working_set.get_span_contents(block_span); - - let (output, err) = lex(source, start, &[], &[]); + let (block, err) = parse_module_block(working_set, &[block_span]); error = error.or(err); - working_set.enter_scope(); - - // Do we need block parameters? - - let (output, err) = lite_parse(&output); - error = error.or(err); - - // We probably don't need $it - - // we're doing parse_block() equivalent - // let (mut output, err) = parse_block(working_set, &output, false); - - for pipeline in &output.block { - if pipeline.commands.len() == 1 { - parse_def_predecl(working_set, &pipeline.commands[0].parts); - } - } - - let mut exports: Vec<(Vec, DeclId)> = vec![]; - - let block: Block = output - .block - .iter() - .map(|pipeline| { - if pipeline.commands.len() == 1 { - // this one here is doing parse_statement() equivalent - // let (stmt, err) = parse_statement(working_set, &pipeline.commands[0].parts); - let name = working_set.get_span_contents(pipeline.commands[0].parts[0]); - - let (stmt, err) = match name { - // TODO: Here we can add other stuff that's allowed for modules - b"def" => { - let (stmt, err) = parse_def(working_set, &pipeline.commands[0].parts); - - (stmt, err) - } - b"export" => { - let (stmt, err) = - parse_export(working_set, &pipeline.commands[0].parts); - - if err.is_none() { - let decl_name = - // parts[2] is safe since it's checked in parse_def already - working_set.get_span_contents(pipeline.commands[0].parts[2]); - - let decl_id = working_set - .find_decl(decl_name) - .expect("internal error: failed to find added declaration"); - - exports.push((decl_name.into(), decl_id)); - } - - (stmt, err) - } - _ => ( - garbage_statement(&pipeline.commands[0].parts), - Some(ParseError::Expected( - // TODO: Fill in more keywords as they come - "def or export keyword".into(), - pipeline.commands[0].parts[0], - )), - ), - }; - - if error.is_none() { - error = err; - } - - stmt - } else { - error = Some(ParseError::Expected("not a pipeline".into(), block_span)); - garbage_statement(spans) - } - }) - .into(); - - let block = block.with_exports(exports); - - working_set.exit_scope(); - let block_id = working_set.add_module(&module_name, block); let block_expr = Expression { From cbda1b1650e21d150dfadd6c5261cc53fd776c1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Tue, 19 Oct 2021 22:56:01 +0300 Subject: [PATCH 02/20] Change import pattern delimiter to :: --- crates/nu-parser/src/parse_keywords.rs | 12 ++++++++---- crates/nu-parser/src/parser.rs | 4 ++-- src/tests.rs | 26 +++++++++++++------------- 3 files changed, 23 insertions(+), 19 deletions(-) diff --git a/crates/nu-parser/src/parse_keywords.rs b/crates/nu-parser/src/parse_keywords.rs index 3cad00023d..e68bbc41de 100644 --- a/crates/nu-parser/src/parse_keywords.rs +++ b/crates/nu-parser/src/parse_keywords.rs @@ -513,7 +513,8 @@ pub fn parse_use( .into_iter() .map(|(name, id)| { let mut new_name = import_pattern.head.to_vec(); - new_name.push(b'.'); + new_name.push(b':'); + new_name.push(b':'); new_name.extend(&name); (new_name, id) }) @@ -628,7 +629,8 @@ pub fn parse_hide( .into_iter() .map(|name| { let mut new_name = import_pattern.head.to_vec(); - new_name.push(b'.'); + new_name.push(b':'); + new_name.push(b':'); new_name.extend(&name); new_name }) @@ -639,7 +641,8 @@ pub fn parse_hide( .filter(|n| n == name) .map(|n| { let mut new_name = import_pattern.head.to_vec(); - new_name.push(b'.'); + new_name.push(b':'); + new_name.push(b':'); new_name.extend(&n); new_name }) @@ -660,7 +663,8 @@ pub fn parse_hide( .filter_map(|n| if n == name { Some(n.clone()) } else { None }) .map(|n| { let mut new_name = import_pattern.head.to_vec(); - new_name.push(b'.'); + new_name.push(b':'); + new_name.push(b':'); new_name.extend(n); new_name }) diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index 893c5f492e..7ba36d127f 100644 --- a/crates/nu-parser/src/parser.rs +++ b/crates/nu-parser/src/parser.rs @@ -1698,7 +1698,7 @@ pub fn parse_import_pattern( let source = working_set.get_span_contents(span); let mut error = None; - let (tokens, err) = lex(source, span.start, &[], &[b'.']); + let (tokens, err) = lex(source, span.start, &[], &[b':']); error = error.or(err); if tokens.is_empty() { @@ -1713,7 +1713,7 @@ pub fn parse_import_pattern( let head = working_set.get_span_contents(tokens[0].span).to_vec(); - if let Some(tail) = tokens.get(2) { + if let Some(tail) = tokens.get(3) { // FIXME: expand this to handle deeper imports once we support module imports let tail_span = tail.span; let tail = working_set.get_span_contents(tail.span); diff --git a/src/tests.rs b/src/tests.rs index fa2d323065..30bc49f383 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -389,7 +389,7 @@ fn better_block_types() -> TestResult { #[test] fn module_imports_1() -> TestResult { run_test( - r#"module foo { export def a [] { 1 }; def b [] { 2 } }; use foo; foo.a"#, + r#"module foo { export def a [] { 1 }; def b [] { 2 } }; use foo; foo::a"#, "1", ) } @@ -397,7 +397,7 @@ fn module_imports_1() -> TestResult { #[test] fn module_imports_2() -> TestResult { run_test( - r#"module foo { export def a [] { 1 }; def b [] { 2 } }; use foo.a; a"#, + r#"module foo { export def a [] { 1 }; def b [] { 2 } }; use foo::a; a"#, "1", ) } @@ -405,7 +405,7 @@ fn module_imports_2() -> TestResult { #[test] fn module_imports_3() -> TestResult { run_test( - r#"module foo { export def a [] { 1 }; export def b [] { 2 } }; use foo.*; b"#, + r#"module foo { export def a [] { 1 }; export def b [] { 2 } }; use foo::*; b"#, "2", ) } @@ -413,7 +413,7 @@ fn module_imports_3() -> TestResult { #[test] fn module_imports_4() -> TestResult { fail_test( - r#"module foo { export def a [] { 1 }; export def b [] { 2 } }; use foo.c"#, + r#"module foo { export def a [] { 1 }; export def b [] { 2 } }; use foo::c"#, "not find import", ) } @@ -421,7 +421,7 @@ fn module_imports_4() -> TestResult { #[test] fn module_imports_5() -> TestResult { run_test( - r#"module foo { export def a [] { 1 }; def b [] { 2 }; export def c [] { 3 } }; use foo.[a, c]; c"#, + r#"module foo { export def a [] { 1 }; def b [] { 2 }; export def c [] { 3 } }; use foo::[a, c]; c"#, "3", ) } @@ -429,7 +429,7 @@ fn module_imports_5() -> TestResult { #[test] fn module_import_uses_internal_command() -> TestResult { run_test( - r#"module foo { def b [] { 2 }; export def a [] { b } }; use foo; foo.a"#, + r#"module foo { def b [] { 2 }; export def a [] { b } }; use foo; foo::a"#, "2", ) } @@ -491,7 +491,7 @@ fn hide_twice_not_allowed() -> TestResult { #[test] fn hides_import_1() -> TestResult { fail_test( - r#"module spam { export def foo [] { "foo" } }; use spam; hide spam.foo; foo"#, + r#"module spam { export def foo [] { "foo" } }; use spam; hide spam::foo; foo"#, not_found_msg(), ) } @@ -499,7 +499,7 @@ fn hides_import_1() -> TestResult { #[test] fn hides_import_2() -> TestResult { fail_test( - r#"module spam { export def foo [] { "foo" } }; use spam; hide spam.*; foo"#, + r#"module spam { export def foo [] { "foo" } }; use spam; hide spam::*; foo"#, not_found_msg(), ) } @@ -507,7 +507,7 @@ fn hides_import_2() -> TestResult { #[test] fn hides_import_3() -> TestResult { fail_test( - r#"module spam { export def foo [] { "foo" } }; use spam; hide spam.[foo]; foo"#, + r#"module spam { export def foo [] { "foo" } }; use spam; hide spam::[foo]; foo"#, not_found_msg(), ) } @@ -515,7 +515,7 @@ fn hides_import_3() -> TestResult { #[test] fn hides_import_4() -> TestResult { fail_test( - r#"module spam { export def foo [] { "foo" } }; use spam.foo; hide foo; foo"#, + r#"module spam { export def foo [] { "foo" } }; use spam::foo; hide foo; foo"#, not_found_msg(), ) } @@ -523,7 +523,7 @@ fn hides_import_4() -> TestResult { #[test] fn hides_import_5() -> TestResult { fail_test( - r#"module spam { export def foo [] { "foo" } }; use spam.*; hide foo; foo"#, + r#"module spam { export def foo [] { "foo" } }; use spam::*; hide foo; foo"#, not_found_msg(), ) } @@ -539,7 +539,7 @@ fn def_twice_should_fail() -> TestResult { #[test] fn use_import_after_hide() -> TestResult { run_test( - r#"module spam { export def foo [] { "foo" } }; use spam.foo; hide foo; use spam.foo; foo"#, + r#"module spam { export def foo [] { "foo" } }; use spam::foo; hide foo; use spam::foo; foo"#, "foo", ) } @@ -547,7 +547,7 @@ fn use_import_after_hide() -> TestResult { #[test] fn hide_shadowed_decl() -> TestResult { run_test( - r#"module spam { export def foo [] { "bar" } }; def foo [] { "foo" }; do { use spam.foo; hide foo; foo }"#, + r#"module spam { export def foo [] { "bar" } }; def foo [] { "foo" }; do { use spam::foo; hide foo; foo }"#, "foo", ) } From 5163dbb7a1d6631f6849a7195342693f72b9d9b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Tue, 19 Oct 2021 23:38:49 +0300 Subject: [PATCH 03/20] Add tests and cover edge cases of the :: delim. --- crates/nu-parser/src/errors.rs | 4 ++++ crates/nu-parser/src/parser.rs | 27 +++++++++++++++++++++++++++ src/tests.rs | 16 ++++++++++++++++ 3 files changed, 47 insertions(+) diff --git a/crates/nu-parser/src/errors.rs b/crates/nu-parser/src/errors.rs index 36412e9a7b..9b50440c29 100644 --- a/crates/nu-parser/src/errors.rs +++ b/crates/nu-parser/src/errors.rs @@ -171,6 +171,10 @@ pub enum ParseError { #[diagnostic(code(nu::parser::missing_import_pattern), url(docsrs))] MissingImportPattern(#[label = "needs an import pattern"] Span), + #[error("Wrong import pattern structure.")] + #[diagnostic(code(nu::parser::missing_import_pattern), url(docsrs))] + WrongImportPattern(#[label = "invalid import pattern structure"] Span), + #[error("Module export not found.")] #[diagnostic(code(nu::parser::export_not_found), url(docsrs))] ExportNotFound(#[label = "could not find imports"] Span), diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index 7ba36d127f..3c2c9a63aa 100644 --- a/crates/nu-parser/src/parser.rs +++ b/crates/nu-parser/src/parser.rs @@ -1711,6 +1711,33 @@ pub fn parse_import_pattern( ); } + if (tokens.len() != 1) && (tokens.len() != 4) { + return ( + ImportPattern { + head: vec![], + members: vec![], + }, + Some(ParseError::WrongImportPattern(span)), + ); + } + + let has_second_colon = if let Some(t) = tokens.get(2) { + let potential_colon = working_set.get_span_contents(t.span); + potential_colon == b":" + } else { + false + }; + + if (tokens.len() == 4) && !has_second_colon { + return ( + ImportPattern { + head: vec![], + members: vec![], + }, + Some(ParseError::WrongImportPattern(span)), + ); + } + let head = working_set.get_span_contents(tokens[0].span).to_vec(); if let Some(tail) = tokens.get(3) { diff --git a/src/tests.rs b/src/tests.rs index 30bc49f383..83b1d46038 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -434,6 +434,22 @@ fn module_import_uses_internal_command() -> TestResult { ) } +#[test] +fn module_import_does_not_parse_with_incorrect_delimiter() -> TestResult { + fail_test( + r#"module foo { export def a [] { 1 } }; use foo:.a"#, + "not found", + ) +} + +#[test] +fn module_import_does_not_parse_with_missing_tail() -> TestResult { + fail_test( + r#"module foo { export def a [] { 1 } }; use foo::"#, + "not found", + ) +} + // TODO: Test the use/hide tests also as separate lines in REPL (i.e., with merging the delta in between) #[test] fn hides_def() -> TestResult { From 75b3b3e09095247d02cd915f358c58d7ea8d85d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Tue, 19 Oct 2021 23:41:59 +0300 Subject: [PATCH 04/20] Add comments --- crates/nu-parser/src/parser.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index 3c2c9a63aa..798a89ec6b 100644 --- a/crates/nu-parser/src/parser.rs +++ b/crates/nu-parser/src/parser.rs @@ -1711,6 +1711,7 @@ pub fn parse_import_pattern( ); } + // We can have either "head" or "head::tail" if (tokens.len() != 1) && (tokens.len() != 4) { return ( ImportPattern { @@ -1721,6 +1722,7 @@ pub fn parse_import_pattern( ); } + // Check if the second : of the :: is really a : let has_second_colon = if let Some(t) = tokens.get(2) { let potential_colon = working_set.get_span_contents(t.span); potential_colon == b":" @@ -1729,6 +1731,7 @@ pub fn parse_import_pattern( }; if (tokens.len() == 4) && !has_second_colon { + // Applies only to the "head::tail" structure; "head" only doesn't have : return ( ImportPattern { head: vec![], From a240aead8c454957f727301a46cd9568b4839607 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Wed, 20 Oct 2021 00:23:59 +0300 Subject: [PATCH 05/20] Add loading module from file Currently, `use spam.nu` creates a module `spam`. Therefore, after the first `use`, it is possible to call both `use spam.nu` and `use spam` with the same effect. --- crates/nu-parser/src/parse_keywords.rs | 57 +++++++++++++++++++++----- 1 file changed, 46 insertions(+), 11 deletions(-) diff --git a/crates/nu-parser/src/parse_keywords.rs b/crates/nu-parser/src/parse_keywords.rs index e68bbc41de..beafb70544 100644 --- a/crates/nu-parser/src/parse_keywords.rs +++ b/crates/nu-parser/src/parse_keywords.rs @@ -1,5 +1,5 @@ use nu_protocol::{ - ast::{Block, Call, Expr, Expression, ImportPatternMember, Pipeline, Statement}, + ast::{Block, Call, Expr, Expression, ImportPattern, ImportPatternMember, Pipeline, Statement}, engine::StateWorkingSet, span, DeclId, Span, SyntaxShape, Type, }; @@ -349,8 +349,7 @@ pub fn parse_module_block( (stmt, err) } b"export" => { - let (stmt, err) = - parse_export(working_set, &pipeline.commands[0].parts); + let (stmt, err) = parse_export(working_set, &pipeline.commands[0].parts); if err.is_none() { let decl_name = @@ -499,14 +498,50 @@ pub fn parse_use( let (import_pattern, err) = parse_import_pattern(working_set, spans[1]); error = error.or(err); - let exports = if let Some(block_id) = working_set.find_module(&import_pattern.head) { - working_set.get_block(block_id).exports.clone() - } else { - return ( - garbage_statement(spans), - Some(ParseError::ModuleNotFound(spans[1])), - ); - }; + let (import_pattern, exports) = + if let Some(block_id) = working_set.find_module(&import_pattern.head) { + ( + import_pattern, + working_set.get_block(block_id).exports.clone(), + ) + } else { + // It could be a file + let module_filename = String::from_utf8_lossy(&import_pattern.head).to_string(); + let module_path = Path::new(&module_filename); + let module_name = if let Some(stem) = module_path.file_stem() { + stem.to_string_lossy().to_string() + } else { + return ( + garbage_statement(spans), + Some(ParseError::ModuleNotFound(spans[1])), + ); + }; + + if let Ok(contents) = std::fs::read(module_path) { + let span_start = working_set.next_span_start(); + working_set.add_file(module_filename, &contents); + let span_end = working_set.next_span_start(); + + let (block, err) = + parse_module_block(working_set, &[Span::new(span_start, span_end)]); + error = error.or(err); + + let block_id = working_set.add_module(&module_name, block); + + ( + ImportPattern { + head: module_name.into(), + members: import_pattern.members, + }, + working_set.get_block(block_id).exports.clone(), + ) + } else { + return ( + garbage_statement(spans), + Some(ParseError::ModuleNotFound(spans[1])), + ); + } + }; let exports = if import_pattern.members.is_empty() { exports From 402a4acd7a331f555c02628fdedc4ad447215c00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Wed, 20 Oct 2021 00:40:50 +0300 Subject: [PATCH 06/20] Fix leftover test --- src/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tests.rs b/src/tests.rs index 83b1d46038..c55725dd38 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -571,7 +571,7 @@ fn hide_shadowed_decl() -> TestResult { #[test] fn hides_all_decls_within_scope() -> TestResult { fail_test( - r#"module spam { export def foo [] { "bar" } }; def foo [] { "foo" }; use spam.foo; hide foo; foo"#, + r#"module spam { export def foo [] { "bar" } }; def foo [] { "foo" }; use spam::foo; hide foo; foo"#, not_found_msg(), ) } From 595fc7a7f66da079e19a1f97281ac43d5f5eda05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Wed, 20 Oct 2021 00:43:48 +0300 Subject: [PATCH 07/20] Switch to cross-platform fail message --- src/tests.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tests.rs b/src/tests.rs index c55725dd38..0d51af0547 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -438,7 +438,7 @@ fn module_import_uses_internal_command() -> TestResult { fn module_import_does_not_parse_with_incorrect_delimiter() -> TestResult { fail_test( r#"module foo { export def a [] { 1 } }; use foo:.a"#, - "not found", + not_found_msg(), ) } @@ -446,7 +446,7 @@ fn module_import_does_not_parse_with_incorrect_delimiter() -> TestResult { fn module_import_does_not_parse_with_missing_tail() -> TestResult { fail_test( r#"module foo { export def a [] { 1 } }; use foo::"#, - "not found", + not_found_msg(), ) } From bd6c550470e11e3346cd6d99eeea9bd1a3b8b7f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Wed, 27 Oct 2021 00:06:08 +0300 Subject: [PATCH 08/20] Change import pattern delimiter to space Subcommands and module imports will have the same syntax now. --- crates/nu-command/src/core_commands/use_.rs | 2 +- crates/nu-parser/src/parse_keywords.rs | 16 ++--- crates/nu-parser/src/parser.rs | 80 +++++++++------------ src/tests.rs | 36 ++++------ 4 files changed, 53 insertions(+), 81 deletions(-) diff --git a/crates/nu-command/src/core_commands/use_.rs b/crates/nu-command/src/core_commands/use_.rs index 27635687d5..3b899c96ef 100644 --- a/crates/nu-command/src/core_commands/use_.rs +++ b/crates/nu-command/src/core_commands/use_.rs @@ -15,7 +15,7 @@ impl Command for Use { } fn signature(&self) -> nu_protocol::Signature { - Signature::build("use").required("pattern", SyntaxShape::String, "import pattern") + Signature::build("use").rest("pattern", SyntaxShape::String, "import pattern parts") } fn run( diff --git a/crates/nu-parser/src/parse_keywords.rs b/crates/nu-parser/src/parse_keywords.rs index beafb70544..c5de2f8918 100644 --- a/crates/nu-parser/src/parse_keywords.rs +++ b/crates/nu-parser/src/parse_keywords.rs @@ -495,7 +495,7 @@ pub fn parse_use( let (module_name_expr, err) = parse_string(working_set, spans[1]); error = error.or(err); - let (import_pattern, err) = parse_import_pattern(working_set, spans[1]); + let (import_pattern, err) = parse_import_pattern(working_set, &spans[1..]); error = error.or(err); let (import_pattern, exports) = @@ -548,8 +548,7 @@ pub fn parse_use( .into_iter() .map(|(name, id)| { let mut new_name = import_pattern.head.to_vec(); - new_name.push(b':'); - new_name.push(b':'); + new_name.push(b' '); new_name.extend(&name); (new_name, id) }) @@ -634,7 +633,7 @@ pub fn parse_hide( let (name_expr, err) = parse_string(working_set, spans[1]); error = error.or(err); - let (import_pattern, err) = parse_import_pattern(working_set, spans[1]); + let (import_pattern, err) = parse_import_pattern(working_set, &spans[1..]); error = error.or(err); let exported_names: Vec> = @@ -664,8 +663,7 @@ pub fn parse_hide( .into_iter() .map(|name| { let mut new_name = import_pattern.head.to_vec(); - new_name.push(b':'); - new_name.push(b':'); + new_name.push(b' '); new_name.extend(&name); new_name }) @@ -676,8 +674,7 @@ pub fn parse_hide( .filter(|n| n == name) .map(|n| { let mut new_name = import_pattern.head.to_vec(); - new_name.push(b':'); - new_name.push(b':'); + new_name.push(b' '); new_name.extend(&n); new_name }) @@ -698,8 +695,7 @@ pub fn parse_hide( .filter_map(|n| if n == name { Some(n.clone()) } else { None }) .map(|n| { let mut new_name = import_pattern.head.to_vec(); - new_name.push(b':'); - new_name.push(b':'); + new_name.push(b' '); new_name.extend(n); new_name }) diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index 798a89ec6b..8087ffdc1a 100644 --- a/crates/nu-parser/src/parser.rs +++ b/crates/nu-parser/src/parser.rs @@ -1693,70 +1693,54 @@ pub fn parse_type(_working_set: &StateWorkingSet, bytes: &[u8]) -> Type { pub fn parse_import_pattern( working_set: &mut StateWorkingSet, - span: Span, + spans: &[Span], ) -> (ImportPattern, Option) { - let source = working_set.get_span_contents(span); let mut error = None; - let (tokens, err) = lex(source, span.start, &[], &[b':']); - error = error.or(err); + // return ( + // ImportPattern { + // head: vec![], + // members: vec![], + // }, + // Some(ParseError::MissingImportPattern(span)), + // ); - if tokens.is_empty() { - return ( - ImportPattern { - head: vec![], - members: vec![], - }, - Some(ParseError::MissingImportPattern(span)), - ); - } + // return ( + // ImportPattern { + // head: vec![], + // members: vec![], + // }, + // Some(ParseError::WrongImportPattern(span)), + // ); - // We can have either "head" or "head::tail" - if (tokens.len() != 1) && (tokens.len() != 4) { - return ( - ImportPattern { - head: vec![], - members: vec![], - }, - Some(ParseError::WrongImportPattern(span)), - ); - } - - // Check if the second : of the :: is really a : - let has_second_colon = if let Some(t) = tokens.get(2) { - let potential_colon = working_set.get_span_contents(t.span); - potential_colon == b":" + let head = if let Some(head_span) = spans.get(0) { + let (head_expr, err) = parse_string(working_set, *head_span); + error = error.or(err); + working_set.get_span_contents(head_expr.span).to_vec() } else { - false + return ( + ImportPattern { + head: vec![], + members: vec![], + }, + Some(ParseError::WrongImportPattern(span(spans))), + ); }; - if (tokens.len() == 4) && !has_second_colon { - // Applies only to the "head::tail" structure; "head" only doesn't have : - return ( - ImportPattern { - head: vec![], - members: vec![], - }, - Some(ParseError::WrongImportPattern(span)), - ); - } - - let head = working_set.get_span_contents(tokens[0].span).to_vec(); - - if let Some(tail) = tokens.get(3) { + if let Some(tail_span) = spans.get(1) { // FIXME: expand this to handle deeper imports once we support module imports - let tail_span = tail.span; - let tail = working_set.get_span_contents(tail.span); + let tail = working_set.get_span_contents(*tail_span); if tail == b"*" { ( ImportPattern { head, - members: vec![ImportPatternMember::Glob { span: tail_span }], + members: vec![ImportPatternMember::Glob { span: *tail_span }], }, error, ) } else if tail.starts_with(b"[") { - let (result, err) = parse_list_expression(working_set, tail_span, &SyntaxShape::String); + let (result, err) = + parse_list_expression(working_set, *tail_span, &SyntaxShape::String); error = error.or(err); let mut output = vec![]; @@ -1793,7 +1777,7 @@ pub fn parse_import_pattern( head, members: vec![ImportPatternMember::Name { name: tail.to_vec(), - span: tail_span, + span: *tail_span, }], }, error, diff --git a/src/tests.rs b/src/tests.rs index 0d51af0547..a3146456c8 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -389,7 +389,7 @@ fn better_block_types() -> TestResult { #[test] fn module_imports_1() -> TestResult { run_test( - r#"module foo { export def a [] { 1 }; def b [] { 2 } }; use foo; foo::a"#, + r#"module foo { export def a [] { 1 }; def b [] { 2 } }; use foo; foo a"#, "1", ) } @@ -397,7 +397,7 @@ fn module_imports_1() -> TestResult { #[test] fn module_imports_2() -> TestResult { run_test( - r#"module foo { export def a [] { 1 }; def b [] { 2 } }; use foo::a; a"#, + r#"module foo { export def a [] { 1 }; def b [] { 2 } }; use foo a; a"#, "1", ) } @@ -405,7 +405,7 @@ fn module_imports_2() -> TestResult { #[test] fn module_imports_3() -> TestResult { run_test( - r#"module foo { export def a [] { 1 }; export def b [] { 2 } }; use foo::*; b"#, + r#"module foo { export def a [] { 1 }; export def b [] { 2 } }; use foo *; b"#, "2", ) } @@ -413,7 +413,7 @@ fn module_imports_3() -> TestResult { #[test] fn module_imports_4() -> TestResult { fail_test( - r#"module foo { export def a [] { 1 }; export def b [] { 2 } }; use foo::c"#, + r#"module foo { export def a [] { 1 }; export def b [] { 2 } }; use foo c"#, "not find import", ) } @@ -421,7 +421,7 @@ fn module_imports_4() -> TestResult { #[test] fn module_imports_5() -> TestResult { run_test( - r#"module foo { export def a [] { 1 }; def b [] { 2 }; export def c [] { 3 } }; use foo::[a, c]; c"#, + r#"module foo { export def a [] { 1 }; def b [] { 2 }; export def c [] { 3 } }; use foo [a, c]; c"#, "3", ) } @@ -429,7 +429,7 @@ fn module_imports_5() -> TestResult { #[test] fn module_import_uses_internal_command() -> TestResult { run_test( - r#"module foo { def b [] { 2 }; export def a [] { b } }; use foo; foo::a"#, + r#"module foo { def b [] { 2 }; export def a [] { b } }; use foo; foo a"#, "2", ) } @@ -442,14 +442,6 @@ fn module_import_does_not_parse_with_incorrect_delimiter() -> TestResult { ) } -#[test] -fn module_import_does_not_parse_with_missing_tail() -> TestResult { - fail_test( - r#"module foo { export def a [] { 1 } }; use foo::"#, - not_found_msg(), - ) -} - // TODO: Test the use/hide tests also as separate lines in REPL (i.e., with merging the delta in between) #[test] fn hides_def() -> TestResult { @@ -507,7 +499,7 @@ fn hide_twice_not_allowed() -> TestResult { #[test] fn hides_import_1() -> TestResult { fail_test( - r#"module spam { export def foo [] { "foo" } }; use spam; hide spam::foo; foo"#, + r#"module spam { export def foo [] { "foo" } }; use spam; hide spam foo; foo"#, not_found_msg(), ) } @@ -515,7 +507,7 @@ fn hides_import_1() -> TestResult { #[test] fn hides_import_2() -> TestResult { fail_test( - r#"module spam { export def foo [] { "foo" } }; use spam; hide spam::*; foo"#, + r#"module spam { export def foo [] { "foo" } }; use spam; hide spam *; foo"#, not_found_msg(), ) } @@ -523,7 +515,7 @@ fn hides_import_2() -> TestResult { #[test] fn hides_import_3() -> TestResult { fail_test( - r#"module spam { export def foo [] { "foo" } }; use spam; hide spam::[foo]; foo"#, + r#"module spam { export def foo [] { "foo" } }; use spam; hide spam [foo]; foo"#, not_found_msg(), ) } @@ -531,7 +523,7 @@ fn hides_import_3() -> TestResult { #[test] fn hides_import_4() -> TestResult { fail_test( - r#"module spam { export def foo [] { "foo" } }; use spam::foo; hide foo; foo"#, + r#"module spam { export def foo [] { "foo" } }; use spam foo; hide foo; foo"#, not_found_msg(), ) } @@ -539,7 +531,7 @@ fn hides_import_4() -> TestResult { #[test] fn hides_import_5() -> TestResult { fail_test( - r#"module spam { export def foo [] { "foo" } }; use spam::*; hide foo; foo"#, + r#"module spam { export def foo [] { "foo" } }; use spam *; hide foo; foo"#, not_found_msg(), ) } @@ -555,7 +547,7 @@ fn def_twice_should_fail() -> TestResult { #[test] fn use_import_after_hide() -> TestResult { run_test( - r#"module spam { export def foo [] { "foo" } }; use spam::foo; hide foo; use spam::foo; foo"#, + r#"module spam { export def foo [] { "foo" } }; use spam foo; hide foo; use spam foo; foo"#, "foo", ) } @@ -563,7 +555,7 @@ fn use_import_after_hide() -> TestResult { #[test] fn hide_shadowed_decl() -> TestResult { run_test( - r#"module spam { export def foo [] { "bar" } }; def foo [] { "foo" }; do { use spam::foo; hide foo; foo }"#, + r#"module spam { export def foo [] { "bar" } }; def foo [] { "foo" }; do { use spam foo; hide foo; foo }"#, "foo", ) } @@ -571,7 +563,7 @@ fn hide_shadowed_decl() -> TestResult { #[test] fn hides_all_decls_within_scope() -> TestResult { fail_test( - r#"module spam { export def foo [] { "bar" } }; def foo [] { "foo" }; use spam::foo; hide foo; foo"#, + r#"module spam { export def foo [] { "bar" } }; def foo [] { "foo" }; use spam foo; hide foo; foo"#, not_found_msg(), ) } From 78256b49231d9bb5b6b6f4c99b7042b1ea49da4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Wed, 27 Oct 2021 00:30:39 +0300 Subject: [PATCH 09/20] Fix syntax highlighting for new import patterns --- crates/nu-parser/src/parse_keywords.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/crates/nu-parser/src/parse_keywords.rs b/crates/nu-parser/src/parse_keywords.rs index c5de2f8918..be6366bc72 100644 --- a/crates/nu-parser/src/parse_keywords.rs +++ b/crates/nu-parser/src/parse_keywords.rs @@ -492,9 +492,15 @@ pub fn parse_use( let bytes = working_set.get_span_contents(spans[0]); if bytes == b"use" && spans.len() >= 2 { - let (module_name_expr, err) = parse_string(working_set, spans[1]); - error = error.or(err); + let mut import_pattern_exprs: Vec = vec![]; + for span in spans[1..].iter() { + let (expr, err) = parse_string(working_set, *span); + import_pattern_exprs.push(expr); + error = error.or(err); + } + // TODO: Add checking for importing too long import patterns, e.g.: + // > use spam foo non existent names here do not throw error let (import_pattern, err) = parse_import_pattern(working_set, &spans[1..]); error = error.or(err); @@ -598,7 +604,7 @@ pub fn parse_use( let call = Box::new(Call { head: spans[0], decl_id: use_decl_id, - positional: vec![module_name_expr], + positional: import_pattern_exprs, named: vec![], }); From b5329fe4ec4b5da75e3eb8b5e4f3f24ff8119ede Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Wed, 27 Oct 2021 00:34:39 +0300 Subject: [PATCH 10/20] Cleanup; Remove redundant UTF-8 check --- crates/nu-parser/src/parser.rs | 20 +------------------- 1 file changed, 1 insertion(+), 19 deletions(-) diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index 8087ffdc1a..17c9d76ac7 100644 --- a/crates/nu-parser/src/parser.rs +++ b/crates/nu-parser/src/parser.rs @@ -1697,26 +1697,8 @@ pub fn parse_import_pattern( ) -> (ImportPattern, Option) { let mut error = None; - // return ( - // ImportPattern { - // head: vec![], - // members: vec![], - // }, - // Some(ParseError::MissingImportPattern(span)), - // ); - - // return ( - // ImportPattern { - // head: vec![], - // members: vec![], - // }, - // Some(ParseError::WrongImportPattern(span)), - // ); - let head = if let Some(head_span) = spans.get(0) { - let (head_expr, err) = parse_string(working_set, *head_span); - error = error.or(err); - working_set.get_span_contents(head_expr.span).to_vec() + working_set.get_span_contents(*head_span).to_vec() } else { return ( ImportPattern { From 4fc533340b8a8056e216b65418581b97b6471496 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Thu, 28 Oct 2021 00:52:59 +0300 Subject: [PATCH 11/20] Add function that searches for multi-word commands It doesn't do anything right now. --- crates/nu-parser/src/parse_keywords.rs | 1 + crates/nu-parser/src/parser.rs | 87 ++++++++++++++++++++++++-- 2 files changed, 83 insertions(+), 5 deletions(-) diff --git a/crates/nu-parser/src/parse_keywords.rs b/crates/nu-parser/src/parse_keywords.rs index be6366bc72..3ebf61bbdc 100644 --- a/crates/nu-parser/src/parse_keywords.rs +++ b/crates/nu-parser/src/parse_keywords.rs @@ -511,6 +511,7 @@ pub fn parse_use( working_set.get_block(block_id).exports.clone(), ) } else { + //TODO: Fix this // It could be a file let module_filename = String::from_utf8_lossy(&import_pattern.head).to_string(); let module_path = Path::new(&module_filename); diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index 17c9d76ac7..deec9834a8 100644 --- a/crates/nu-parser/src/parser.rs +++ b/crates/nu-parser/src/parser.rs @@ -11,7 +11,7 @@ use nu_protocol::{ Operator, PathMember, Pipeline, RangeInclusion, RangeOperator, Statement, }, engine::StateWorkingSet, - span, Flag, PositionalArg, Signature, Span, Spanned, SyntaxShape, Type, Unit, VarId, + span, DeclId, Flag, PositionalArg, Signature, Span, Spanned, SyntaxShape, Type, Unit, VarId, }; use crate::parse_keywords::{ @@ -39,6 +39,26 @@ fn is_identifier_byte(b: u8) -> bool { b != b'.' && b != b'[' && b != b'(' && b != b'{' } +fn is_math_expression_byte(b: u8) -> bool { + b == b'0' + || b == b'1' + || b == b'2' + || b == b'3' + || b == b'4' + || b == b'5' + || b == b'6' + || b == b'7' + || b == b'8' + || b == b'9' + || b == b'(' + || b == b'{' + || b == b'[' + || b == b'$' + || b == b'"' + || b == b'\'' + || b == b'-' +} + fn is_identifier(bytes: &[u8]) -> bool { bytes.iter().all(|x| is_identifier_byte(*x)) } @@ -590,6 +610,62 @@ pub fn parse_internal_call( (Box::new(call), span(spans), error) } +pub fn parse_command_name( + working_set: &mut StateWorkingSet, + spans: &[Span], +) -> (Option, Option) { + if spans.len() == 0 { + ( + None, + Some(ParseError::UnknownState( + "Encountered command with zero spans".into(), + span(spans), + )), + ) + } else if spans.len() == 1 { + let bytes = working_set.get_span_contents(spans[0]); + (working_set.find_decl(bytes), None) + } else { + // Find the longest group of words that could form a command + let mut longest_name = working_set.get_span_contents(spans[0]).to_vec(); + let mut indices = vec![0]; + for span in spans[1..].iter() { + let bytes = working_set.get_span_contents(*span); + if is_math_expression_byte(bytes[0]) { + break; + } + indices.push(longest_name.len()); + longest_name.push(b' '); + longest_name.extend_from_slice(bytes); + } + + // Now, try if it matches a command and if not, peel off the last word and try again + let mut decl_id = working_set.find_decl(&longest_name); + let mut err = None; + while decl_id.is_none() { + let split_idx = if let Some(i) = indices.pop() { + i + } else { + decl_id = None; + err = Some(ParseError::UnknownState( + "Command has no words".into(), + span(spans), + )); + break; + }; + + if split_idx == 0 { + // This is the first word, we reached the end + break; + } + + decl_id = working_set.find_decl(&longest_name[..split_idx]); + } + + (decl_id, err) + } +} + pub fn parse_call( working_set: &mut StateWorkingSet, spans: &[Span], @@ -618,6 +694,7 @@ pub fn parse_call( ); } + parse_command_name(working_set, &spans[pos..]); let name = working_set.get_span_contents(spans[pos]); let cmd_start = pos; @@ -2931,10 +3008,10 @@ pub fn parse_expression( ) -> (Expression, Option) { let bytes = working_set.get_span_contents(spans[0]); - match bytes[0] { - b'0' | b'1' | b'2' | b'3' | b'4' | b'5' | b'6' | b'7' | b'8' | b'9' | b'(' | b'{' - | b'[' | b'$' | b'"' | b'\'' | b'-' => parse_math_expression(working_set, spans, None), - _ => parse_call(working_set, spans, expand_aliases), + if is_math_expression_byte(bytes[0]) { + parse_math_expression(working_set, spans, None) + } else { + parse_call(working_set, spans, expand_aliases) } } From 751595e72e575c0052c5cbf3086c22565261a088 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Fri, 29 Oct 2021 23:50:28 +0300 Subject: [PATCH 12/20] Add multi-word name calling support --- crates/nu-parser/src/parser.rs | 214 ++++++++++++--------------------- 1 file changed, 75 insertions(+), 139 deletions(-) diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index deec9834a8..1046149e5f 100644 --- a/crates/nu-parser/src/parser.rs +++ b/crates/nu-parser/src/parser.rs @@ -11,7 +11,7 @@ use nu_protocol::{ Operator, PathMember, Pipeline, RangeInclusion, RangeOperator, Statement, }, engine::StateWorkingSet, - span, DeclId, Flag, PositionalArg, Signature, Span, Spanned, SyntaxShape, Type, Unit, VarId, + span, Flag, PositionalArg, Signature, Span, Spanned, SyntaxShape, Type, Unit, VarId, }; use crate::parse_keywords::{ @@ -610,68 +610,21 @@ pub fn parse_internal_call( (Box::new(call), span(spans), error) } -pub fn parse_command_name( - working_set: &mut StateWorkingSet, - spans: &[Span], -) -> (Option, Option) { - if spans.len() == 0 { - ( - None, - Some(ParseError::UnknownState( - "Encountered command with zero spans".into(), - span(spans), - )), - ) - } else if spans.len() == 1 { - let bytes = working_set.get_span_contents(spans[0]); - (working_set.find_decl(bytes), None) - } else { - // Find the longest group of words that could form a command - let mut longest_name = working_set.get_span_contents(spans[0]).to_vec(); - let mut indices = vec![0]; - for span in spans[1..].iter() { - let bytes = working_set.get_span_contents(*span); - if is_math_expression_byte(bytes[0]) { - break; - } - indices.push(longest_name.len()); - longest_name.push(b' '); - longest_name.extend_from_slice(bytes); - } - - // Now, try if it matches a command and if not, peel off the last word and try again - let mut decl_id = working_set.find_decl(&longest_name); - let mut err = None; - while decl_id.is_none() { - let split_idx = if let Some(i) = indices.pop() { - i - } else { - decl_id = None; - err = Some(ParseError::UnknownState( - "Command has no words".into(), - span(spans), - )); - break; - }; - - if split_idx == 0 { - // This is the first word, we reached the end - break; - } - - decl_id = working_set.find_decl(&longest_name[..split_idx]); - } - - (decl_id, err) - } -} - pub fn parse_call( working_set: &mut StateWorkingSet, spans: &[Span], expand_aliases: bool, ) -> (Expression, Option) { - // assume spans.len() > 0? + if spans.is_empty() { + return ( + garbage(Span::unknown()), + Some(ParseError::UnknownState( + "Encountered command with zero spans".into(), + span(spans), + )), + ); + } + let mut pos = 0; let mut shorthand = vec![]; @@ -689,105 +642,82 @@ pub fn parse_call( if pos == spans.len() { return ( - Expression::garbage(span(spans)), + garbage(span(spans)), Some(ParseError::UnknownCommand(spans[0])), ); } - parse_command_name(working_set, &spans[pos..]); - let name = working_set.get_span_contents(spans[pos]); - let cmd_start = pos; + let mut name_spans = vec![]; - if expand_aliases { - if let Some(expansion) = working_set.find_alias(name) { - let orig_span = spans[pos]; - //let mut spans = spans.to_vec(); - let mut new_spans: Vec = vec![]; - new_spans.extend(&spans[0..pos]); - new_spans.extend(expansion); - if spans.len() > pos { - new_spans.extend(&spans[(pos + 1)..]); - } + for word_span in spans[cmd_start..].iter() { + // Find the longest group of words that could form a command + let bytes = working_set.get_span_contents(*word_span); - let (result, err) = parse_expression(working_set, &new_spans, false); + if is_math_expression_byte(bytes[0]) { + break; + } - let expression = match result { - Expression { - expr: Expr::Call(mut call), - span, - ty, - custom_completion: None, - } => { - call.head = orig_span; + name_spans.push(*word_span); + + let name = working_set.get_span_contents(span(&name_spans)); + + if expand_aliases { + // If the word is an alias, expand it and re-parse the expression + if let Some(expansion) = working_set.find_alias(name) { + let orig_span = spans[pos]; + let mut new_spans: Vec = vec![]; + new_spans.extend(&spans[0..pos]); + new_spans.extend(expansion); + if spans.len() > pos { + new_spans.extend(&spans[(pos + 1)..]); + } + + let (result, err) = parse_expression(working_set, &new_spans, false); + + let expression = match result { Expression { - expr: Expr::Call(call), + expr: Expr::Call(mut call), span, ty, custom_completion: None, - } - } - x => x, - }; - - return (expression, err); - } - } - - pos += 1; - - if let Some(mut decl_id) = working_set.find_decl(name) { - let mut name = name.to_vec(); - while pos < spans.len() { - // look to see if it's a subcommand - let mut new_name = name.to_vec(); - new_name.push(b' '); - new_name.extend(working_set.get_span_contents(spans[pos])); - - if expand_aliases { - if let Some(expansion) = working_set.find_alias(&new_name) { - let orig_span = span(&spans[cmd_start..pos + 1]); - //let mut spans = spans.to_vec(); - let mut new_spans: Vec = vec![]; - new_spans.extend(&spans[0..cmd_start]); - new_spans.extend(expansion); - if spans.len() > pos { - new_spans.extend(&spans[(pos + 1)..]); - } - - let (result, err) = parse_expression(working_set, &new_spans, false); - - let expression = match result { + } => { + call.head = orig_span; Expression { - expr: Expr::Call(mut call), + expr: Expr::Call(call), span, ty, custom_completion: None, - } => { - call.head = orig_span; - Expression { - expr: Expr::Call(call), - span, - ty, - custom_completion: None, - } } - x => x, - }; + } + x => x, + }; - return (expression, err); - } + return (expression, err); } - - if let Some(did) = working_set.find_decl(&new_name) { - decl_id = did; - } else { - break; - } - name = new_name; - pos += 1; } + pos += 1; + } + + let name = working_set.get_span_contents(span(&name_spans)); + let mut maybe_decl_id = working_set.find_decl(name); + + while maybe_decl_id.is_none() { + // Find the longest command match + if name_spans.len() <= 1 { + // Keep the first word even if it does not match -- could be external command + break; + } + + name_spans.pop(); + pos -= 1; + + let name = working_set.get_span_contents(span(&name_spans)); + maybe_decl_id = working_set.find_decl(name); + } + + if let Some(decl_id) = maybe_decl_id { // Before the internal parsing we check if there is no let or alias declarations // that are missing their name, e.g.: let = 1 or alias = 2 if spans.len() > 1 { @@ -795,7 +725,7 @@ pub fn parse_call( if test_equal == [b'='] { return ( - garbage(Span::new(0, 0)), + garbage(Span::unknown()), Some(ParseError::UnknownState( "Incomplete statement".into(), span(spans), @@ -805,8 +735,12 @@ pub fn parse_call( } // parse internal command - let (call, _, err) = - parse_internal_call(working_set, span(&spans[0..pos]), &spans[pos..], decl_id); + let (call, _, err) = parse_internal_call( + working_set, + span(&spans[cmd_start..pos]), + &spans[pos..], + decl_id, + ); ( Expression { expr: Expr::Call(call), @@ -825,6 +759,8 @@ pub fn parse_call( return (range_expr, range_err); } } + + // Otherwise, try external command parse_external_call(working_set, spans) } } From 2dcfecbbd7f97f2bfe9901c4d364dddaf25b420c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Fri, 29 Oct 2021 23:57:33 +0300 Subject: [PATCH 13/20] Add test for multi-word alias --- src/tests.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/tests.rs b/src/tests.rs index a3146456c8..4640ce4d9d 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -247,6 +247,11 @@ fn alias_2() -> TestResult { ) } +#[test] +fn alias_2_multi_word() -> TestResult { + run_test(r#"def "foo bar" [$x $y] { $x + $y + 10 }; alias f = foo bar 33; f 100"#, "143") +} + #[test] fn block_param1() -> TestResult { run_test("[3] | each { $it + 10 } | get 0", "13") From da515b1c9d4c8bcf475cc7d27eb687303da0d805 Mon Sep 17 00:00:00 2001 From: Michael Angerman Date: Sat, 30 Oct 2021 10:51:20 -0700 Subject: [PATCH 14/20] port the filter command range from nushell --- crates/nu-command/src/default_context.rs | 1 + crates/nu-command/src/filters/mod.rs | 2 + crates/nu-command/src/filters/range.rs | 127 +++++++++++++++++++++++ 3 files changed, 130 insertions(+) create mode 100644 crates/nu-command/src/filters/range.rs diff --git a/crates/nu-command/src/default_context.rs b/crates/nu-command/src/default_context.rs index 77993b7e55..32cb38aa89 100644 --- a/crates/nu-command/src/default_context.rs +++ b/crates/nu-command/src/default_context.rs @@ -58,6 +58,7 @@ pub fn create_default_context() -> EngineState { Mv, ParEach, Ps, + Range, Rm, Select, Size, diff --git a/crates/nu-command/src/filters/mod.rs b/crates/nu-command/src/filters/mod.rs index 595fe396cd..f322ca3092 100644 --- a/crates/nu-command/src/filters/mod.rs +++ b/crates/nu-command/src/filters/mod.rs @@ -4,6 +4,7 @@ mod last; mod length; mod lines; mod par_each; +mod range; mod select; mod where_; mod wrap; @@ -14,6 +15,7 @@ pub use last::Last; pub use length::Length; pub use lines::Lines; pub use par_each::ParEach; +pub use range::Range; pub use select::Select; pub use where_::Where; pub use wrap::Wrap; diff --git a/crates/nu-command/src/filters/range.rs b/crates/nu-command/src/filters/range.rs new file mode 100644 index 0000000000..0280f51822 --- /dev/null +++ b/crates/nu-command/src/filters/range.rs @@ -0,0 +1,127 @@ +use nu_engine::CallExt; + +use nu_protocol::ast::Call; +use nu_protocol::engine::{Command, EngineState, Stack}; +use nu_protocol::{ + Example, IntoInterruptiblePipelineData, PipelineData, ShellError, Signature, Span, SyntaxShape, + Value, +}; + +#[derive(Clone)] +pub struct Range; + +impl Command for Range { + fn name(&self) -> &str { + "range" + } + + fn signature(&self) -> Signature { + Signature::build("range").optional( + "rows", + SyntaxShape::Range, + "range of rows to return: Eg) 4..7 (=> from 4 to 7)", + ) + } + + fn usage(&self) -> &str { + "Return only the selected rows." + } + + fn examples(&self) -> Vec { + vec![ + Example { + example: "[0,1,2,3,4,5] | range 4..5", + description: "Get the last 2 items", + result: Some(Value::List { + vals: vec![Value::test_int(4), Value::test_int(5)], + span: Span::unknown(), + }), + }, + Example { + example: "[0,1,2,3,4,5] | range (-2)..", + description: "Get the last 2 items", + result: Some(Value::List { + vals: vec![Value::test_int(4), Value::test_int(5)], + span: Span::unknown(), + }), + }, + Example { + example: "[0,1,2,3,4,5] | range (-3)..-2", + description: "Get the next to last 2 items", + result: Some(Value::List { + vals: vec![Value::test_int(3), Value::test_int(4)], + span: Span::unknown(), + }), + }, + ] + } + + fn run( + &self, + engine_state: &EngineState, + stack: &mut Stack, + call: &Call, + input: PipelineData, + ) -> Result { + let rows: nu_protocol::Range = call.req(engine_state, stack, 0)?; + + let rows_from = get_range_val(rows.from); + let rows_to = get_range_val(rows.to); + + // only collect the input if we have any negative indices + if rows_from < 0 || rows_to < 0 { + let v: Vec<_> = input.into_iter().collect(); + let vlen: i64 = v.len() as i64; + + let from = if rows_from < 0 { + (vlen + rows_from) as usize + } else { + rows_from as usize + }; + + let to = if rows_to < 0 { + (vlen + rows_to) as usize + } else if rows_to > v.len() as i64 { + v.len() + } else { + rows_to as usize + }; + + if from > to { + Ok(PipelineData::Value(Value::Nothing { span: call.head })) + } else { + let iter = v.into_iter().skip(from).take(to - from + 1); + Ok(iter.into_pipeline_data(engine_state.ctrlc.clone())) + } + } else { + let from = rows_from as usize; + let to = rows_to as usize; + + if from > to { + Ok(PipelineData::Value(Value::Nothing { span: call.head })) + } else { + let iter = input.into_iter().skip(from).take(to - from + 1); + Ok(iter.into_pipeline_data(engine_state.ctrlc.clone())) + } + } + } +} + +fn get_range_val(rows_val: Value) -> i64 { + match rows_val { + Value::Int { val: x, .. } => x, + _ => 0, + } +} + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn test_examples() { + use crate::test_examples; + + test_examples(Range {}) + } +} From 5add6035a4da6b3d109de7d0acb8e144e708e448 Mon Sep 17 00:00:00 2001 From: Luccas Mateus de Medeiros Gomes Date: Fri, 29 Oct 2021 16:02:42 -0300 Subject: [PATCH 15/20] Added math and min commands typo Added op span --- crates/nu-command/src/default_context.rs | 2 + crates/nu-command/src/math/avg.rs | 1 + crates/nu-command/src/math/max.rs | 57 ++++++++++++++++++++ crates/nu-command/src/math/min.rs | 57 ++++++++++++++++++++ crates/nu-command/src/math/mod.rs | 4 ++ crates/nu-command/src/math/reducers.rs | 66 +++++++++++++++++++++--- 6 files changed, 181 insertions(+), 6 deletions(-) create mode 100644 crates/nu-command/src/math/max.rs create mode 100644 crates/nu-command/src/math/min.rs diff --git a/crates/nu-command/src/default_context.rs b/crates/nu-command/src/default_context.rs index b1f8fd929c..97abc1e0e9 100644 --- a/crates/nu-command/src/default_context.rs +++ b/crates/nu-command/src/default_context.rs @@ -51,6 +51,8 @@ pub fn create_default_context() -> EngineState { Math, MathAbs, MathAvg, + MathMax, + MathMin, Mkdir, Module, Mv, diff --git a/crates/nu-command/src/math/avg.rs b/crates/nu-command/src/math/avg.rs index 384d3493a5..f7de5be527 100644 --- a/crates/nu-command/src/math/avg.rs +++ b/crates/nu-command/src/math/avg.rs @@ -50,6 +50,7 @@ pub fn average(values: &[Value], head: &Span) -> Result { span: Span::unknown(), }, values.to_vec(), + *head, )?; match total { Value::Filesize { val, span } => Ok(Value::Filesize { diff --git a/crates/nu-command/src/math/max.rs b/crates/nu-command/src/math/max.rs new file mode 100644 index 0000000000..81d146502b --- /dev/null +++ b/crates/nu-command/src/math/max.rs @@ -0,0 +1,57 @@ +use crate::math::reducers::{reducer_for, Reduce}; +use crate::math::utils::run_with_function; +use nu_protocol::ast::Call; +use nu_protocol::engine::{Command, EngineState, Stack}; +use nu_protocol::{Example, PipelineData, ShellError, Signature, Span, Value}; + +#[derive(Clone)] +pub struct SubCommand; + +impl Command for SubCommand { + fn name(&self) -> &str { + "math max" + } + + fn signature(&self) -> Signature { + Signature::build("math max") + } + + fn usage(&self) -> &str { + "Finds the maximum within a list of numbers or tables" + } + + fn run( + &self, + _engine_state: &EngineState, + _stack: &mut Stack, + call: &Call, + input: PipelineData, + ) -> Result { + run_with_function(call, input, maximum) + } + + fn examples(&self) -> Vec { + vec![Example { + description: "Find the maximum of list of numbers", + example: "[-50 100 25] | math max", + result: Some(Value::test_int(100)), + }] + } +} + +pub fn maximum(values: &[Value], head: &Span) -> Result { + let max_func = reducer_for(Reduce::Maximum); + max_func(Value::nothing(), values.to_vec(), *head) +} + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn test_examples() { + use crate::test_examples; + + test_examples(SubCommand {}) + } +} diff --git a/crates/nu-command/src/math/min.rs b/crates/nu-command/src/math/min.rs new file mode 100644 index 0000000000..b232d13074 --- /dev/null +++ b/crates/nu-command/src/math/min.rs @@ -0,0 +1,57 @@ +use crate::math::reducers::{reducer_for, Reduce}; +use crate::math::utils::run_with_function; +use nu_protocol::ast::Call; +use nu_protocol::engine::{Command, EngineState, Stack}; +use nu_protocol::{Example, PipelineData, ShellError, Signature, Span, Value}; + +#[derive(Clone)] +pub struct SubCommand; + +impl Command for SubCommand { + fn name(&self) -> &str { + "math min" + } + + fn signature(&self) -> Signature { + Signature::build("math min") + } + + fn usage(&self) -> &str { + "Finds the minimum within a list of numbers or tables" + } + + fn run( + &self, + _engine_state: &EngineState, + _stack: &mut Stack, + call: &Call, + input: PipelineData, + ) -> Result { + run_with_function(call, input, minimum) + } + + fn examples(&self) -> Vec { + vec![Example { + description: "Get the minimum of a list of numbers", + example: "[-50 100 25] | math min", + result: Some(Value::test_int(-50)), + }] + } +} + +pub fn minimum(values: &[Value], head: &Span) -> Result { + let min_func = reducer_for(Reduce::Minimum); + min_func(Value::nothing(), values.to_vec(), *head) +} + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn test_examples() { + use crate::test_examples; + + test_examples(SubCommand {}) + } +} diff --git a/crates/nu-command/src/math/mod.rs b/crates/nu-command/src/math/mod.rs index c3f4a42934..6b2bcffc76 100644 --- a/crates/nu-command/src/math/mod.rs +++ b/crates/nu-command/src/math/mod.rs @@ -1,9 +1,13 @@ mod abs; mod avg; pub mod command; +mod max; +mod min; mod reducers; mod utils; pub use abs::SubCommand as MathAbs; pub use avg::SubCommand as MathAvg; pub use command::MathCommand as Math; +pub use max::SubCommand as MathMax; +pub use min::SubCommand as MathMin; diff --git a/crates/nu-command/src/math/reducers.rs b/crates/nu-command/src/math/reducers.rs index 2339fd97f8..9e64602ebe 100644 --- a/crates/nu-command/src/math/reducers.rs +++ b/crates/nu-command/src/math/reducers.rs @@ -1,19 +1,73 @@ use nu_protocol::{ShellError, Span, Value}; +use std::cmp::Ordering; #[allow(dead_code)] pub enum Reduce { Summation, + Minimum, + Maximum, } -pub fn reducer_for( - command: Reduce, -) -> Box) -> Result + Send + Sync + 'static> { +pub type ReducerFunction = + Box, Span) -> Result + Send + Sync + 'static>; + +pub fn reducer_for(command: Reduce) -> ReducerFunction { match command { - Reduce::Summation => Box::new(|_, values| sum(values)), + Reduce::Summation => Box::new(|_, values, head| sum(values, head)), + Reduce::Minimum => Box::new(|_, values, head| min(values, head)), + Reduce::Maximum => Box::new(|_, values, head| max(values, head)), } } -pub fn sum(data: Vec) -> Result { +pub fn max(data: Vec, head: Span) -> Result { + let mut biggest = data + .first() + .ok_or_else(|| ShellError::UnsupportedInput("Empty input".to_string(), Span::unknown()))? + .clone(); + + for value in &data { + if let Some(result) = value.partial_cmp(&biggest) { + if result == Ordering::Greater { + biggest = value.clone(); + } + } else { + return Err(ShellError::OperatorMismatch { + op_span: head, + lhs_ty: biggest.get_type(), + lhs_span: biggest.span()?, + rhs_ty: value.get_type(), + rhs_span: value.span()?, + }); + } + } + Ok(biggest) +} + +pub fn min(data: Vec, head: Span) -> Result { + let mut smallest = data + .first() + .ok_or_else(|| ShellError::UnsupportedInput("Empty input".to_string(), Span::unknown()))? + .clone(); + + for value in &data { + if let Some(result) = value.partial_cmp(&smallest) { + if result == Ordering::Less { + smallest = value.clone(); + } + } else { + return Err(ShellError::OperatorMismatch { + op_span: head, + lhs_ty: smallest.get_type(), + lhs_span: smallest.span()?, + rhs_ty: value.get_type(), + rhs_span: value.span()?, + }); + } + } + Ok(smallest) +} + +pub fn sum(data: Vec, head: Span) -> Result { let initial_value = data.get(0); let mut acc = match initial_value { @@ -42,7 +96,7 @@ pub fn sum(data: Vec) -> Result { | Value::Float { .. } | Value::Filesize { .. } | Value::Duration { .. } => { - let new_value = acc.add(acc.span().unwrap_or_else(|_| Span::unknown()), value); + let new_value = acc.add(head, value); if new_value.is_err() { return new_value; } From 7112664b3fb9caceb72bb6d302d33a5637ee0910 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Sun, 31 Oct 2021 17:22:10 +0200 Subject: [PATCH 16/20] Fix wrong spans of multiple files The introduction of `use ` added the possibility of calling `working_set.add_file()` more than once per parse pass. Some of the logic handling the file contents offsets prevented it from working and hopefully, this commit fixes it. --- crates/nu-parser/src/parse_keywords.rs | 16 +++++++--------- crates/nu-protocol/src/engine/engine_state.rs | 4 ++-- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/crates/nu-parser/src/parse_keywords.rs b/crates/nu-parser/src/parse_keywords.rs index 3ebf61bbdc..e145d4cd90 100644 --- a/crates/nu-parser/src/parse_keywords.rs +++ b/crates/nu-parser/src/parse_keywords.rs @@ -218,8 +218,6 @@ pub fn parse_alias( let replacement = spans[3..].to_vec(); - //println!("{:?} {:?}", alias_name, replacement); - working_set.add_alias(alias_name, replacement); } @@ -311,15 +309,15 @@ pub fn parse_export( pub fn parse_module_block( working_set: &mut StateWorkingSet, - spans: &[Span], + span: Span, ) -> (Block, Option) { let mut error = None; working_set.enter_scope(); - let source = working_set.get_span_contents(spans[0]); + let source = working_set.get_span_contents(span); - let (output, err) = lex(source, spans[0].start, &[], &[]); + let (output, err) = lex(source, span.start, &[], &[]); error = error.or(err); let (output, err) = lite_parse(&output); @@ -380,8 +378,8 @@ pub fn parse_module_block( stmt } else { - error = Some(ParseError::Expected("not a pipeline".into(), spans[0])); - garbage_statement(spans) + error = Some(ParseError::Expected("not a pipeline".into(), span)); + garbage_statement(&[span]) } }) .into(); @@ -441,7 +439,7 @@ pub fn parse_module( let block_span = Span { start, end }; - let (block, err) = parse_module_block(working_set, &[block_span]); + let (block, err) = parse_module_block(working_set, block_span); error = error.or(err); let block_id = working_set.add_module(&module_name, block); @@ -530,7 +528,7 @@ pub fn parse_use( let span_end = working_set.next_span_start(); let (block, err) = - parse_module_block(working_set, &[Span::new(span_start, span_end)]); + parse_module_block(working_set, Span::new(span_start, span_end)); error = error.or(err); let block_id = working_set.add_module(&module_name, block); diff --git a/crates/nu-protocol/src/engine/engine_state.rs b/crates/nu-protocol/src/engine/engine_state.rs index de240af11c..1f8de20c93 100644 --- a/crates/nu-protocol/src/engine/engine_state.rs +++ b/crates/nu-protocol/src/engine/engine_state.rs @@ -501,7 +501,7 @@ impl<'a> StateWorkingSet<'a> { let permanent_span_start = self.permanent_state.next_span_start(); if let Some((_, _, last)) = self.delta.file_contents.last() { - permanent_span_start + *last + *last } else { permanent_span_start } @@ -561,7 +561,7 @@ impl<'a> StateWorkingSet<'a> { if permanent_end <= span.start { for (contents, start, finish) in &self.delta.file_contents { if (span.start >= *start) && (span.end <= *finish) { - return &contents[(span.start - permanent_end)..(span.end - permanent_end)]; + return &contents[(span.start - start)..(span.end - start)]; } } } else { From b7c0ba104f5ebc4df1a0352c730ab2a2a4de2ca0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Sun, 31 Oct 2021 17:38:00 +0200 Subject: [PATCH 17/20] Fix hiding module; Fmt This fixes the case when you call `hide spam`. It will now hide all commands you'd call like `spam foo` etc. --- crates/nu-parser/src/parse_keywords.rs | 115 ++++++++++++++----------- src/tests.rs | 13 ++- 2 files changed, 77 insertions(+), 51 deletions(-) diff --git a/crates/nu-parser/src/parse_keywords.rs b/crates/nu-parser/src/parse_keywords.rs index e145d4cd90..7606a1a486 100644 --- a/crates/nu-parser/src/parse_keywords.rs +++ b/crates/nu-parser/src/parse_keywords.rs @@ -502,51 +502,51 @@ pub fn parse_use( let (import_pattern, err) = parse_import_pattern(working_set, &spans[1..]); error = error.or(err); - let (import_pattern, exports) = - if let Some(block_id) = working_set.find_module(&import_pattern.head) { + let (import_pattern, exports) = if let Some(block_id) = + working_set.find_module(&import_pattern.head) + { + ( + import_pattern, + working_set.get_block(block_id).exports.clone(), + ) + } else { + // TODO: Handle invalid UTF-8 conversion + // It could be a file + let module_filename = String::from_utf8_lossy(&import_pattern.head).to_string(); + let module_path = Path::new(&module_filename); + let module_name = if let Some(stem) = module_path.file_stem() { + stem.to_string_lossy().to_string() + } else { + return ( + garbage_statement(spans), + Some(ParseError::ModuleNotFound(spans[1])), + ); + }; + + if let Ok(contents) = std::fs::read(module_path) { + let span_start = working_set.next_span_start(); + working_set.add_file(module_filename, &contents); + let span_end = working_set.next_span_start(); + + let (block, err) = parse_module_block(working_set, Span::new(span_start, span_end)); + error = error.or(err); + + let block_id = working_set.add_module(&module_name, block); + ( - import_pattern, + ImportPattern { + head: module_name.into(), + members: import_pattern.members, + }, working_set.get_block(block_id).exports.clone(), ) } else { - //TODO: Fix this - // It could be a file - let module_filename = String::from_utf8_lossy(&import_pattern.head).to_string(); - let module_path = Path::new(&module_filename); - let module_name = if let Some(stem) = module_path.file_stem() { - stem.to_string_lossy().to_string() - } else { - return ( - garbage_statement(spans), - Some(ParseError::ModuleNotFound(spans[1])), - ); - }; - - if let Ok(contents) = std::fs::read(module_path) { - let span_start = working_set.next_span_start(); - working_set.add_file(module_filename, &contents); - let span_end = working_set.next_span_start(); - - let (block, err) = - parse_module_block(working_set, Span::new(span_start, span_end)); - error = error.or(err); - - let block_id = working_set.add_module(&module_name, block); - - ( - ImportPattern { - head: module_name.into(), - members: import_pattern.members, - }, - working_set.get_block(block_id).exports.clone(), - ) - } else { - return ( - garbage_statement(spans), - Some(ParseError::ModuleNotFound(spans[1])), - ); - } - }; + return ( + garbage_statement(spans), + Some(ParseError::ModuleNotFound(spans[1])), + ); + } + }; let exports = if import_pattern.members.is_empty() { exports @@ -641,17 +641,20 @@ pub fn parse_hide( let (import_pattern, err) = parse_import_pattern(working_set, &spans[1..]); error = error.or(err); - let exported_names: Vec> = + let (is_module, exported_names): (bool, Vec>) = if let Some(block_id) = working_set.find_module(&import_pattern.head) { - working_set - .get_block(block_id) - .exports - .iter() - .map(|(name, _)| name.clone()) - .collect() + ( + true, + working_set + .get_block(block_id) + .exports + .iter() + .map(|(name, _)| name.clone()) + .collect(), + ) } else if import_pattern.members.is_empty() { // The pattern head can be e.g. a function name, not just a module - vec![import_pattern.head.clone()] + (false, vec![import_pattern.head.clone()]) } else { return ( garbage_statement(spans), @@ -661,7 +664,19 @@ pub fn parse_hide( // This kind of inverts the import pattern matching found in parse_use() let names_to_hide = if import_pattern.members.is_empty() { - exported_names + if is_module { + exported_names + .into_iter() + .map(|name| { + let mut new_name = import_pattern.head.to_vec(); + new_name.push(b' '); + new_name.extend(&name); + new_name + }) + .collect() + } else { + exported_names + } } else { match &import_pattern.members[0] { ImportPatternMember::Glob { .. } => exported_names diff --git a/src/tests.rs b/src/tests.rs index 4640ce4d9d..d057a48da0 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -249,7 +249,10 @@ fn alias_2() -> TestResult { #[test] fn alias_2_multi_word() -> TestResult { - run_test(r#"def "foo bar" [$x $y] { $x + $y + 10 }; alias f = foo bar 33; f 100"#, "143") + run_test( + r#"def "foo bar" [$x $y] { $x + $y + 10 }; alias f = foo bar 33; f 100"#, + "143", + ) } #[test] @@ -541,6 +544,14 @@ fn hides_import_5() -> TestResult { ) } +#[test] +fn hides_import_6() -> TestResult { + fail_test( + r#"module spam { export def foo [] { "foo" } }; use spam; hide spam; foo"#, + not_found_msg(), + ) +} + #[test] fn def_twice_should_fail() -> TestResult { fail_test( From f18252429838c78e08f37f6f1c464241604c7a00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Sun, 31 Oct 2021 17:46:37 +0200 Subject: [PATCH 18/20] Add TODO notes --- crates/nu-parser/src/parse_keywords.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/nu-parser/src/parse_keywords.rs b/crates/nu-parser/src/parse_keywords.rs index 7606a1a486..c36a00d724 100644 --- a/crates/nu-parser/src/parse_keywords.rs +++ b/crates/nu-parser/src/parse_keywords.rs @@ -511,6 +511,7 @@ pub fn parse_use( ) } else { // TODO: Handle invalid UTF-8 conversion + // TODO: Do not close over when loading module from file // It could be a file let module_filename = String::from_utf8_lossy(&import_pattern.head).to_string(); let module_path = Path::new(&module_filename); @@ -734,6 +735,8 @@ pub fn parse_hide( }; for name in names_to_hide { + // TODO: `use spam; use spam foo; hide foo` will hide both `foo` and `spam foo` since + // they point to the same DeclId. Do we want to keep it that way? if working_set.hide_decl(&name).is_none() { error = error.or_else(|| Some(ParseError::UnknownCommand(spans[1]))); } From 73ae3daf859859842ee035eef78d1e5b74637930 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Sun, 31 Oct 2021 17:53:53 +0200 Subject: [PATCH 19/20] Add invalid UTF-8 error to use and source Also changed the error message to be more universal. --- crates/nu-parser/src/errors.rs | 4 +- crates/nu-parser/src/parse_keywords.rs | 94 ++++++++++++++------------ 2 files changed, 54 insertions(+), 44 deletions(-) diff --git a/crates/nu-parser/src/errors.rs b/crates/nu-parser/src/errors.rs index 9b50440c29..2f5f2145a6 100644 --- a/crates/nu-parser/src/errors.rs +++ b/crates/nu-parser/src/errors.rs @@ -93,9 +93,9 @@ pub enum ParseError { )] UnknownCommand(#[label = "unknown command"] Span), - #[error("Non-UTF8 code.")] + #[error("Non-UTF8 string.")] #[diagnostic(code(nu::parser::non_utf8), url(docsrs))] - NonUtf8(#[label = "non-UTF8 code"] Span), + NonUtf8(#[label = "non-UTF8 string"] Span), #[error("The `{0}` command doesn't have flag `{1}`.")] #[diagnostic(code(nu::parser::unknown_flag), url(docsrs))] diff --git a/crates/nu-parser/src/parse_keywords.rs b/crates/nu-parser/src/parse_keywords.rs index c36a00d724..ab2c14d7bc 100644 --- a/crates/nu-parser/src/parse_keywords.rs +++ b/crates/nu-parser/src/parse_keywords.rs @@ -502,52 +502,57 @@ pub fn parse_use( let (import_pattern, err) = parse_import_pattern(working_set, &spans[1..]); error = error.or(err); - let (import_pattern, exports) = if let Some(block_id) = - working_set.find_module(&import_pattern.head) - { - ( - import_pattern, - working_set.get_block(block_id).exports.clone(), - ) - } else { - // TODO: Handle invalid UTF-8 conversion - // TODO: Do not close over when loading module from file - // It could be a file - let module_filename = String::from_utf8_lossy(&import_pattern.head).to_string(); - let module_path = Path::new(&module_filename); - let module_name = if let Some(stem) = module_path.file_stem() { - stem.to_string_lossy().to_string() - } else { - return ( - garbage_statement(spans), - Some(ParseError::ModuleNotFound(spans[1])), - ); - }; - - if let Ok(contents) = std::fs::read(module_path) { - let span_start = working_set.next_span_start(); - working_set.add_file(module_filename, &contents); - let span_end = working_set.next_span_start(); - - let (block, err) = parse_module_block(working_set, Span::new(span_start, span_end)); - error = error.or(err); - - let block_id = working_set.add_module(&module_name, block); - + let (import_pattern, exports) = + if let Some(block_id) = working_set.find_module(&import_pattern.head) { ( - ImportPattern { - head: module_name.into(), - members: import_pattern.members, - }, + import_pattern, working_set.get_block(block_id).exports.clone(), ) } else { - return ( - garbage_statement(spans), - Some(ParseError::ModuleNotFound(spans[1])), - ); - } - }; + // TODO: Do not close over when loading module from file + // It could be a file + if let Ok(module_filename) = String::from_utf8(import_pattern.head) { + let module_path = Path::new(&module_filename); + let module_name = if let Some(stem) = module_path.file_stem() { + stem.to_string_lossy().to_string() + } else { + return ( + garbage_statement(spans), + Some(ParseError::ModuleNotFound(spans[1])), + ); + }; + + if let Ok(contents) = std::fs::read(module_path) { + let span_start = working_set.next_span_start(); + working_set.add_file(module_filename, &contents); + let span_end = working_set.next_span_start(); + + let (block, err) = + parse_module_block(working_set, Span::new(span_start, span_end)); + error = error.or(err); + + let block_id = working_set.add_module(&module_name, block); + + ( + ImportPattern { + head: module_name.into(), + members: import_pattern.members, + }, + working_set.get_block(block_id).exports.clone(), + ) + } else { + return ( + garbage_statement(spans), + Some(ParseError::ModuleNotFound(spans[1])), + ); + } + } else { + return ( + garbage_statement(spans), + Some(ParseError::NonUtf8(spans[1])), + ); + } + }; let exports = if import_pattern.members.is_empty() { exports @@ -892,6 +897,11 @@ pub fn parse_source( ); } } + } else { + return ( + garbage_statement(spans), + Some(ParseError::NonUtf8(spans[1])), + ); } } return ( From b340672331d2387c29b40549027f7f0a086b2cb9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Sun, 31 Oct 2021 18:01:15 +0200 Subject: [PATCH 20/20] Remove leftover test from previous iteration --- src/tests.rs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/tests.rs b/src/tests.rs index d057a48da0..b3852b1c31 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -442,14 +442,6 @@ fn module_import_uses_internal_command() -> TestResult { ) } -#[test] -fn module_import_does_not_parse_with_incorrect_delimiter() -> TestResult { - fail_test( - r#"module foo { export def a [] { 1 } }; use foo:.a"#, - not_found_msg(), - ) -} - // TODO: Test the use/hide tests also as separate lines in REPL (i.e., with merging the delta in between) #[test] fn hides_def() -> TestResult {