From 2f8a52d2566737f5f5f0b23b5e12c749878a8b76 Mon Sep 17 00:00:00 2001 From: JT <547158+jntrnr@users.noreply.github.com> Date: Thu, 23 Mar 2023 09:14:10 +1300 Subject: [PATCH] Switch let/let-env family to init with math expressions (#8545) # Description This is an experiment to see what switching the `let/let-env` family to math expressions for initialisers would be like. # User-Facing Changes This would require any commands you call from `let x = ` (and similar family) to call the command in parentheses. `let x = (foo)` to call `foo`. # Tests + Formatting Don't forget to add tests that cover your changes. Make sure you've run and fixed any issues with these commands: - `cargo fmt --all -- --check` to check standard code formatting (`cargo fmt --all` applies these changes) - `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used -A clippy::needless_collect` to check that you're using the standard code style - `cargo test --workspace` to check that all tests pass > **Note** > from `nushell` you can also use the `toolkit` as follows > ```bash > use toolkit.nu # or use an `env_change` hook to activate it automatically > toolkit check pr > ``` # After Submitting If your PR had any user-facing changes, update [the documentation](https://github.com/nushell/nushell.github.io) after the PR is merged, if necessary. This will help us keep the docs up to date. --- .../nu-cmd-lang/src/core_commands/const_.rs | 2 +- crates/nu-cmd-lang/src/core_commands/if_.rs | 2 +- crates/nu-cmd-lang/src/core_commands/let_.rs | 2 +- crates/nu-cmd-lang/src/core_commands/mut_.rs | 2 +- .../nu-cmd-lang/src/core_commands/while_.rs | 2 +- crates/nu-command/src/env/let_env.rs | 2 +- crates/nu-command/tests/commands/use_.rs | 2 +- crates/nu-engine/src/eval.rs | 23 +++++++++++++ crates/nu-parser/src/parse_keywords.rs | 10 ++++-- crates/nu-parser/src/parser.rs | 34 ++++++++++++++----- crates/nu-parser/tests/test_parser.rs | 4 +-- src/tests/test_engine.rs | 2 +- src/tests/test_modules.rs | 2 +- 13 files changed, 68 insertions(+), 21 deletions(-) diff --git a/crates/nu-cmd-lang/src/core_commands/const_.rs b/crates/nu-cmd-lang/src/core_commands/const_.rs index a051aa567f..35dea1fda8 100644 --- a/crates/nu-cmd-lang/src/core_commands/const_.rs +++ b/crates/nu-cmd-lang/src/core_commands/const_.rs @@ -21,7 +21,7 @@ impl Command for Const { .required("const_name", SyntaxShape::VarWithOptType, "constant name") .required( "initial_value", - SyntaxShape::Keyword(b"=".to_vec(), Box::new(SyntaxShape::Expression)), + SyntaxShape::Keyword(b"=".to_vec(), Box::new(SyntaxShape::MathExpression)), "equals sign followed by constant value", ) .category(Category::Core) diff --git a/crates/nu-cmd-lang/src/core_commands/if_.rs b/crates/nu-cmd-lang/src/core_commands/if_.rs index 2d8f9c1663..d6a2beb82b 100644 --- a/crates/nu-cmd-lang/src/core_commands/if_.rs +++ b/crates/nu-cmd-lang/src/core_commands/if_.rs @@ -20,7 +20,7 @@ impl Command for If { fn signature(&self) -> nu_protocol::Signature { Signature::build("if") .input_output_types(vec![(Type::Any, Type::Any)]) - .required("cond", SyntaxShape::Expression, "condition to check") + .required("cond", SyntaxShape::MathExpression, "condition to check") .required( "then_block", SyntaxShape::Block, diff --git a/crates/nu-cmd-lang/src/core_commands/let_.rs b/crates/nu-cmd-lang/src/core_commands/let_.rs index 4d419dc146..579396b73b 100644 --- a/crates/nu-cmd-lang/src/core_commands/let_.rs +++ b/crates/nu-cmd-lang/src/core_commands/let_.rs @@ -22,7 +22,7 @@ impl Command for Let { .required("var_name", SyntaxShape::VarWithOptType, "variable name") .required( "initial_value", - SyntaxShape::Keyword(b"=".to_vec(), Box::new(SyntaxShape::Expression)), + SyntaxShape::Keyword(b"=".to_vec(), Box::new(SyntaxShape::MathExpression)), "equals sign followed by value", ) .category(Category::Core) diff --git a/crates/nu-cmd-lang/src/core_commands/mut_.rs b/crates/nu-cmd-lang/src/core_commands/mut_.rs index d974dbc5c7..45add079cb 100644 --- a/crates/nu-cmd-lang/src/core_commands/mut_.rs +++ b/crates/nu-cmd-lang/src/core_commands/mut_.rs @@ -22,7 +22,7 @@ impl Command for Mut { .required("var_name", SyntaxShape::VarWithOptType, "variable name") .required( "initial_value", - SyntaxShape::Keyword(b"=".to_vec(), Box::new(SyntaxShape::Expression)), + SyntaxShape::Keyword(b"=".to_vec(), Box::new(SyntaxShape::MathExpression)), "equals sign followed by value", ) .category(Category::Core) diff --git a/crates/nu-cmd-lang/src/core_commands/while_.rs b/crates/nu-cmd-lang/src/core_commands/while_.rs index c3f725a1f5..d5bf71202e 100644 --- a/crates/nu-cmd-lang/src/core_commands/while_.rs +++ b/crates/nu-cmd-lang/src/core_commands/while_.rs @@ -21,7 +21,7 @@ impl Command for While { Signature::build("while") .input_output_types(vec![(Type::Nothing, Type::Nothing)]) .allow_variants_without_examples(true) - .required("cond", SyntaxShape::Expression, "condition to check") + .required("cond", SyntaxShape::MathExpression, "condition to check") .required( "block", SyntaxShape::Block, diff --git a/crates/nu-command/src/env/let_env.rs b/crates/nu-command/src/env/let_env.rs index 6b9bc7b4c8..e3ce4fa33f 100644 --- a/crates/nu-command/src/env/let_env.rs +++ b/crates/nu-command/src/env/let_env.rs @@ -24,7 +24,7 @@ impl Command for LetEnv { .required("var_name", SyntaxShape::String, "variable name") .required( "initial_value", - SyntaxShape::Keyword(b"=".to_vec(), Box::new(SyntaxShape::Expression)), + SyntaxShape::Keyword(b"=".to_vec(), Box::new(SyntaxShape::MathExpression)), "equals sign followed by value", ) .category(Category::Env) diff --git a/crates/nu-command/tests/commands/use_.rs b/crates/nu-command/tests/commands/use_.rs index 5c36fdae2c..c3b35b3385 100644 --- a/crates/nu-command/tests/commands/use_.rs +++ b/crates/nu-command/tests/commands/use_.rs @@ -172,7 +172,7 @@ fn use_export_env_combined() { "spam.nu", r#" alias bar = foo - export-env { let-env FOO = bar } + export-env { let-env FOO = (bar) } def foo [] { 'foo' } "#, )]); diff --git a/crates/nu-engine/src/eval.rs b/crates/nu-engine/src/eval.rs index 57a705148d..c951735d6a 100644 --- a/crates/nu-engine/src/eval.rs +++ b/crates/nu-engine/src/eval.rs @@ -698,6 +698,29 @@ pub fn eval_expression_with_input( input = eval_subexpression(engine_state, stack, block, input)?; } + elem @ Expression { + expr: Expr::FullCellPath(full_cell_path), + .. + } => match &full_cell_path.head { + Expression { + expr: Expr::Subexpression(block_id), + span, + .. + } => { + let block = engine_state.get_block(*block_id); + + // FIXME: protect this collect with ctrl-c + input = eval_subexpression(engine_state, stack, block, input)?; + let value = input.into_value(*span); + input = value + .follow_cell_path(&full_cell_path.tail, false)? + .into_pipeline_data() + } + _ => { + input = eval_expression(engine_state, stack, elem)?.into_pipeline_data(); + } + }, + elem => { input = eval_expression(engine_state, stack, elem)?.into_pipeline_data(); } diff --git a/crates/nu-parser/src/parse_keywords.rs b/crates/nu-parser/src/parse_keywords.rs index 5c6a763249..c575fdfda8 100644 --- a/crates/nu-parser/src/parse_keywords.rs +++ b/crates/nu-parser/src/parse_keywords.rs @@ -3102,7 +3102,10 @@ pub fn parse_let_or_const( working_set, spans, &mut idx, - &SyntaxShape::Keyword(b"=".to_vec(), Box::new(SyntaxShape::Expression)), + &SyntaxShape::Keyword( + b"=".to_vec(), + Box::new(SyntaxShape::MathExpression), + ), expand_aliases_denylist, ); error = error.or(err); @@ -3236,7 +3239,10 @@ pub fn parse_mut( working_set, spans, &mut idx, - &SyntaxShape::Keyword(b"=".to_vec(), Box::new(SyntaxShape::Expression)), + &SyntaxShape::Keyword( + b"=".to_vec(), + Box::new(SyntaxShape::MathExpression), + ), expand_aliases_denylist, ); error = error.or(err); diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index c3bade66fa..4443f77ce0 100644 --- a/crates/nu-parser/src/parser.rs +++ b/crates/nu-parser/src/parser.rs @@ -71,7 +71,12 @@ pub fn is_math_expression_like( return false; } - if bytes == b"true" || bytes == b"false" || bytes == b"null" || bytes == b"not" { + if bytes == b"true" + || bytes == b"false" + || bytes == b"null" + || bytes == b"not" + || bytes == b"if" + { return true; } @@ -1112,12 +1117,12 @@ pub fn parse_call( for word_span in spans[cmd_start..].iter() { // Find the longest group of words that could form a command - if is_math_expression_like(working_set, *word_span, expand_aliases_denylist) { - let bytes = working_set.get_span_contents(*word_span); - if bytes != b"true" && bytes != b"false" && bytes != b"null" && bytes != b"not" { - break; - } - } + // if is_math_expression_like(working_set, *word_span, expand_aliases_denylist) { + // let bytes = working_set.get_span_contents(*word_span); + // if bytes != b"true" && bytes != b"false" && bytes != b"null" && bytes != b"not" { + // break; + // } + // } name_spans.push(*word_span); @@ -4993,7 +4998,20 @@ pub fn parse_math_expression( let first_span = working_set.get_span_contents(spans[0]); - if first_span == b"not" { + if first_span == b"if" { + // If expression + if spans.len() > 1 { + return parse_call(working_set, spans, spans[0], expand_aliases_denylist, false); + } else { + return ( + garbage(spans[0]), + Some(ParseError::Expected( + "expression".into(), + Span::new(spans[0].end, spans[0].end), + )), + ); + } + } else if first_span == b"not" { if spans.len() > 1 { let (remainder, err) = parse_math_expression( working_set, diff --git a/crates/nu-parser/tests/test_parser.rs b/crates/nu-parser/tests/test_parser.rs index 9297302db4..800a9faeb7 100644 --- a/crates/nu-parser/tests/test_parser.rs +++ b/crates/nu-parser/tests/test_parser.rs @@ -27,7 +27,7 @@ impl Command for Let { .required("var_name", SyntaxShape::VarWithOptType, "variable name") .required( "initial_value", - SyntaxShape::Keyword(b"=".to_vec(), Box::new(SyntaxShape::Expression)), + SyntaxShape::Keyword(b"=".to_vec(), Box::new(SyntaxShape::MathExpression)), "equals sign followed by value", ) } @@ -1555,7 +1555,7 @@ mod input_types { fn signature(&self) -> nu_protocol::Signature { Signature::build("if") - .required("cond", SyntaxShape::Expression, "condition to check") + .required("cond", SyntaxShape::MathExpression, "condition to check") .required( "then_block", SyntaxShape::Block, diff --git a/src/tests/test_engine.rs b/src/tests/test_engine.rs index b61399ee6e..e0d6e980b6 100644 --- a/src/tests/test_engine.rs +++ b/src/tests/test_engine.rs @@ -146,7 +146,7 @@ fn date_comparison() -> TestResult { #[test] fn let_sees_input() -> TestResult { run_test( - r#"def c [] { let x = str length; $x }; "hello world" | c"#, + r#"def c [] { let x = (str length); $x }; "hello world" | c"#, "11", ) } diff --git a/src/tests/test_modules.rs b/src/tests/test_modules.rs index 5849b85873..20e279d333 100644 --- a/src/tests/test_modules.rs +++ b/src/tests/test_modules.rs @@ -91,7 +91,7 @@ fn module_def_import_uses_internal_command() -> TestResult { #[test] fn module_env_import_uses_internal_command() -> TestResult { run_test( - r#"module foo { def b [] { "2" }; export-env { let-env a = b } }; use foo; $env.a"#, + r#"module foo { def b [] { "2" }; export-env { let-env a = (b) } }; use foo; $env.a"#, "2", ) }