From 8d5fbc6fcb922277120a10e04ccdd47fc92002d9 Mon Sep 17 00:00:00 2001 From: JT <547158+jntrnr@users.noreply.github.com> Date: Fri, 24 Mar 2023 22:50:23 +1300 Subject: [PATCH] Fix closures that use matches. Move 'collect' to core. (#8596) # Description Fix patterns in pattern matching to properly declare their variables when discovering which variables need to be closed over when creating a closure. Also, moves `collect` to core, so that the core language can use `$in`. Fixes #8595 # User-Facing Changes See above # 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. --- .../src/core_commands}/collect.rs | 0 .../nu-cmd-lang/src/core_commands/match_.rs | 5 +++++ crates/nu-cmd-lang/src/core_commands/mod.rs | 2 ++ crates/nu-cmd-lang/src/default_context.rs | 1 + crates/nu-cmd-lang/src/example_test.rs | 3 ++- crates/nu-command/src/default_context.rs | 1 - crates/nu-command/src/filters/mod.rs | 2 -- crates/nu-parser/src/parser.rs | 22 +++++++++++++++++-- 8 files changed, 30 insertions(+), 6 deletions(-) rename crates/{nu-command/src/filters => nu-cmd-lang/src/core_commands}/collect.rs (100%) diff --git a/crates/nu-command/src/filters/collect.rs b/crates/nu-cmd-lang/src/core_commands/collect.rs similarity index 100% rename from crates/nu-command/src/filters/collect.rs rename to crates/nu-cmd-lang/src/core_commands/collect.rs diff --git a/crates/nu-cmd-lang/src/core_commands/match_.rs b/crates/nu-cmd-lang/src/core_commands/match_.rs index 6369d173cf..a7f9f8e41a 100644 --- a/crates/nu-cmd-lang/src/core_commands/match_.rs +++ b/crates/nu-cmd-lang/src/core_commands/match_.rs @@ -111,6 +111,11 @@ impl Command for Match { example: "match [1, 2, 3] { [$a, $b, $c] => { $a + $b + $c }, _ => 0 }", result: Some(Value::test_int(6)), }, + Example { + description: "Match against pipeline input", + example: "{a: {b: 3}} | match $in.a { { $b } => ($b + 10) }", + result: Some(Value::test_int(13)), + }, ] } } diff --git a/crates/nu-cmd-lang/src/core_commands/mod.rs b/crates/nu-cmd-lang/src/core_commands/mod.rs index f969026f3f..3a04492bbc 100644 --- a/crates/nu-cmd-lang/src/core_commands/mod.rs +++ b/crates/nu-cmd-lang/src/core_commands/mod.rs @@ -1,5 +1,6 @@ mod alias; mod break_; +mod collect; mod commandline; mod const_; mod continue_; @@ -41,6 +42,7 @@ mod while_; pub use alias::Alias; pub use break_::Break; +pub use collect::Collect; pub use commandline::Commandline; pub use const_::Const; pub use continue_::Continue; diff --git a/crates/nu-cmd-lang/src/default_context.rs b/crates/nu-cmd-lang/src/default_context.rs index 81cf90163c..aa2b5b0a1c 100644 --- a/crates/nu-cmd-lang/src/default_context.rs +++ b/crates/nu-cmd-lang/src/default_context.rs @@ -18,6 +18,7 @@ pub fn create_default_context() -> EngineState { bind_command! { Alias, Break, + Collect, Commandline, Const, Continue, diff --git a/crates/nu-cmd-lang/src/example_test.rs b/crates/nu-cmd-lang/src/example_test.rs index 4f5bc343b6..c65c0963a7 100644 --- a/crates/nu-cmd-lang/src/example_test.rs +++ b/crates/nu-cmd-lang/src/example_test.rs @@ -13,7 +13,7 @@ mod test_examples { check_example_evaluates_to_expected_output, check_example_input_and_output_types_match_command_signature, }; - use crate::{Break, Describe, Mut}; + use crate::{Break, Collect, Describe, Mut}; use crate::{Echo, If, Let}; use nu_protocol::{ engine::{Command, EngineState, StateWorkingSet}, @@ -66,6 +66,7 @@ mod test_examples { working_set.add_decl(Box::new(If)); working_set.add_decl(Box::new(Let)); working_set.add_decl(Box::new(Mut)); + working_set.add_decl(Box::new(Collect)); // Adding the command that is being tested to the working set working_set.add_decl(cmd); diff --git a/crates/nu-command/src/default_context.rs b/crates/nu-command/src/default_context.rs index 17be5afb29..4fe22cbfe8 100644 --- a/crates/nu-command/src/default_context.rs +++ b/crates/nu-command/src/default_context.rs @@ -36,7 +36,6 @@ pub fn create_default_context() -> EngineState { All, Any, Append, - Collect, Columns, Compact, Default, diff --git a/crates/nu-command/src/filters/mod.rs b/crates/nu-command/src/filters/mod.rs index 00da2428c4..bb7a586539 100644 --- a/crates/nu-command/src/filters/mod.rs +++ b/crates/nu-command/src/filters/mod.rs @@ -1,7 +1,6 @@ mod all; mod any; mod append; -mod collect; mod columns; mod compact; mod default; @@ -58,7 +57,6 @@ mod zip; pub use all::All; pub use any::Any; pub use append::Append; -pub use collect::Collect; pub use columns::Columns; pub use compact::Compact; pub use default::Default; diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index 1e0e445f71..a44148121d 100644 --- a/crates/nu-parser/src/parser.rs +++ b/crates/nu-parser/src/parser.rs @@ -11,8 +11,8 @@ use crate::{ use nu_protocol::{ ast::{ Argument, Assignment, Bits, Block, Boolean, Call, CellPath, Comparison, Expr, Expression, - FullCellPath, ImportPattern, ImportPatternHead, ImportPatternMember, Math, Operator, - PathMember, Pipeline, PipelineElement, RangeInclusion, RangeOperator, + FullCellPath, ImportPattern, ImportPatternHead, ImportPatternMember, MatchPattern, Math, + Operator, PathMember, Pattern, Pipeline, PipelineElement, RangeInclusion, RangeOperator, }, engine::StateWorkingSet, span, BlockId, Flag, PositionalArg, Signature, Span, Spanned, SyntaxShape, Type, Unit, VarId, @@ -6005,6 +6005,23 @@ pub fn discover_captures_in_pipeline_element( } } +pub fn discover_captures_in_pattern(pattern: &MatchPattern, seen: &mut Vec) { + match &pattern.pattern { + Pattern::Variable(var_id) => seen.push(*var_id), + Pattern::List(items) => { + for item in items { + discover_captures_in_pattern(item, seen) + } + } + Pattern::Record(items) => { + for item in items { + discover_captures_in_pattern(&item.1, seen) + } + } + Pattern::Value(_) | Pattern::IgnoreValue | Pattern::Garbage => {} + } +} + // Closes over captured variables pub fn discover_captures_in_expr( working_set: &StateWorkingSet, @@ -6206,6 +6223,7 @@ pub fn discover_captures_in_expr( Expr::MatchPattern(_) => {} Expr::MatchBlock(match_block) => { for match_ in match_block { + discover_captures_in_pattern(&match_.0, seen); let result = discover_captures_in_expr(working_set, &match_.1, seen, seen_blocks)?; output.extend(&result); }