From 7ec5f2f2eb33e517d2390b1009bae76b5dbb9666 Mon Sep 17 00:00:00 2001 From: JT <547158+jntrnr@users.noreply.github.com> Date: Mon, 27 Mar 2023 11:31:57 +1300 Subject: [PATCH] Add or-patterns, fix var binding scope (#8633) # Description Adds `|` patterns to `match`, allowing you to try multiple patterns for the same case. Example: ``` match {b: 1} { {a: $b} | {b: $b} => { print $b } } ``` Variables that don't bind are set to `$nothing` so that they can be later checked. This PR also: fixes #8631 Creates a set of integration tests for pattern matching also # User-Facing Changes Adds `|` to `match`. Fixes variable binding scope. # 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. --- crates/nu-command/tests/commands/match_.rs | 177 ++++++++++++++++++ crates/nu-command/tests/commands/mod.rs | 1 + crates/nu-parser/src/flatten.rs | 5 + crates/nu-parser/src/parse_patterns.rs | 2 +- crates/nu-parser/src/parser.rs | 75 +++++++- crates/nu-protocol/src/ast/match_pattern.rs | 36 ++++ crates/nu-protocol/src/engine/engine_state.rs | 18 ++ .../nu-protocol/src/engine/pattern_match.rs | 50 ++++- 8 files changed, 355 insertions(+), 9 deletions(-) create mode 100644 crates/nu-command/tests/commands/match_.rs diff --git a/crates/nu-command/tests/commands/match_.rs b/crates/nu-command/tests/commands/match_.rs new file mode 100644 index 000000000..7b9ac34c4 --- /dev/null +++ b/crates/nu-command/tests/commands/match_.rs @@ -0,0 +1,177 @@ +use nu_test_support::nu; + +#[test] +fn match_for_range() { + let actual = nu!( + cwd: ".", + r#"match 3 { 1..10 => { print "success" } }"# + ); + // Make sure we don't see any of these values in the output + // As we do not auto-print loops anymore + assert_eq!(actual.out, "success"); +} + +#[test] +fn match_for_range_unmatched() { + let actual = nu!( + cwd: ".", + r#"match 11 { 1..10 => { print "failure" }, _ => { print "success" }}"# + ); + // Make sure we don't see any of these values in the output + // As we do not auto-print loops anymore + assert_eq!(actual.out, "success"); +} + +#[test] +fn match_for_record() { + let actual = nu!( + cwd: ".", + r#"match {a: 11} { {a: $b} => { print $b }}"# + ); + // Make sure we don't see any of these values in the output + // As we do not auto-print loops anymore + assert_eq!(actual.out, "11"); +} + +#[test] +fn match_for_record_shorthand() { + let actual = nu!( + cwd: ".", + r#"match {a: 12} { {$a} => { print $a }}"# + ); + // Make sure we don't see any of these values in the output + // As we do not auto-print loops anymore + assert_eq!(actual.out, "12"); +} + +#[test] +fn match_list() { + let actual = nu!( + cwd: ".", + r#"match [1, 2] { [$a] => { print $"single: ($a)" }, [$b, $c] => {print $"double: ($b) ($c)"}}"# + ); + // Make sure we don't see any of these values in the output + // As we do not auto-print loops anymore + assert_eq!(actual.out, "double: 1 2"); +} + +#[test] +fn match_constant_1() { + let actual = nu!( + cwd: ".", + r#"match 2 { 1 => { print "failure"}, 2 => { print "success" }, 3 => { print "failure" }}"# + ); + // Make sure we don't see any of these values in the output + // As we do not auto-print loops anymore + assert_eq!(actual.out, "success"); +} + +#[test] +fn match_constant_2() { + let actual = nu!( + cwd: ".", + r#"match 2.3 { 1.4 => { print "failure"}, 2.3 => { print "success" }, 3 => { print "failure" }}"# + ); + // Make sure we don't see any of these values in the output + // As we do not auto-print loops anymore + assert_eq!(actual.out, "success"); +} + +#[test] +fn match_constant_3() { + let actual = nu!( + cwd: ".", + r#"match true { false => { print "failure"}, true => { print "success" }, 3 => { print "failure" }}"# + ); + // Make sure we don't see any of these values in the output + // As we do not auto-print loops anymore + assert_eq!(actual.out, "success"); +} + +#[test] +fn match_constant_4() { + let actual = nu!( + cwd: ".", + r#"match "def" { "abc" => { print "failure"}, "def" => { print "success" }, "ghi" => { print "failure" }}"# + ); + // Make sure we don't see any of these values in the output + // As we do not auto-print loops anymore + assert_eq!(actual.out, "success"); +} + +#[test] +fn match_constant_5() { + let actual = nu!( + cwd: ".", + r#"match 2019-08-23 { 2010-01-01 => { print "failure"}, 2019-08-23 => { print "success" }, 2020-02-02 => { print "failure" }}"# + ); + // Make sure we don't see any of these values in the output + // As we do not auto-print loops anymore + assert_eq!(actual.out, "success"); +} + +#[test] +fn match_constant_6() { + let actual = nu!( + cwd: ".", + r#"match 6sec { 2sec => { print "failure"}, 6sec => { print "success" }, 1min => { print "failure" }}"# + ); + // Make sure we don't see any of these values in the output + // As we do not auto-print loops anymore + assert_eq!(actual.out, "success"); +} + +#[test] +fn match_constant_7() { + let actual = nu!( + cwd: ".", + r#"match 1kib { 1kb => { print "failure"}, 1kib => { print "success" }, 2kb => { print "failure" }}"# + ); + // Make sure we don't see any of these values in the output + // As we do not auto-print loops anymore + assert_eq!(actual.out, "success"); +} + +#[test] +fn match_or_pattern() { + let actual = nu!( + cwd: ".", + r#"match {b: 7} { {a: $a} | {b: $b} => { print $"success: ($b)" }, _ => { print "failure" }}"# + ); + // Make sure we don't see any of these values in the output + // As we do not auto-print loops anymore + assert_eq!(actual.out, "success: 7"); +} + +#[test] +fn match_or_pattern_overlap_1() { + let actual = nu!( + cwd: ".", + r#"match {a: 7} { {a: $b} | {b: $b} => { print $"success: ($b)" }, _ => { print "failure" }}"# + ); + // Make sure we don't see any of these values in the output + // As we do not auto-print loops anymore + assert_eq!(actual.out, "success: 7"); +} + +#[test] +fn match_or_pattern_overlap_2() { + let actual = nu!( + cwd: ".", + r#"match {b: 7} { {a: $b} | {b: $b} => { print $"success: ($b)" }, _ => { print "failure" }}"# + ); + // Make sure we don't see any of these values in the output + // As we do not auto-print loops anymore + assert_eq!(actual.out, "success: 7"); +} + +#[test] +fn match_doesnt_overwrite_variable() { + let actual = nu!( + cwd: ".", + r#"let b = 100; match 55 { $b => {} }; print $b"# + ); + // Make sure we don't see any of these values in the output + // As we do not auto-print loops anymore + assert_eq!(actual.out, "100"); +} diff --git a/crates/nu-command/tests/commands/mod.rs b/crates/nu-command/tests/commands/mod.rs index 870131934..564457c30 100644 --- a/crates/nu-command/tests/commands/mod.rs +++ b/crates/nu-command/tests/commands/mod.rs @@ -48,6 +48,7 @@ mod let_; mod lines; mod loop_; mod ls; +mod match_; mod math; mod merge; mod mkdir; diff --git a/crates/nu-parser/src/flatten.rs b/crates/nu-parser/src/flatten.rs index d9030c37a..cc1e5222d 100644 --- a/crates/nu-parser/src/flatten.rs +++ b/crates/nu-parser/src/flatten.rs @@ -559,6 +559,11 @@ pub fn flatten_pattern(match_pattern: &MatchPattern) -> Vec<(Span, FlatShape)> { Pattern::Variable(_) => { output.push((match_pattern.span, FlatShape::Variable)); } + Pattern::Or(patterns) => { + for pattern in patterns { + output.extend(flatten_pattern(pattern)); + } + } } output } diff --git a/crates/nu-parser/src/parse_patterns.rs b/crates/nu-parser/src/parse_patterns.rs index bec3447a5..eb686fe53 100644 --- a/crates/nu-parser/src/parse_patterns.rs +++ b/crates/nu-parser/src/parse_patterns.rs @@ -79,7 +79,7 @@ pub fn parse_variable_pattern( let bytes = working_set.get_span_contents(span); if is_variable(bytes) { - if let Some(var_id) = working_set.find_variable(bytes) { + if let Some(var_id) = working_set.find_variable_in_current_frame(bytes) { ( MatchPattern { pattern: Pattern::Variable(var_id), diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index cf6e5060c..8f390ddd9 100644 --- a/crates/nu-parser/src/parser.rs +++ b/crates/nu-parser/src/parser.rs @@ -4526,7 +4526,7 @@ pub fn parse_match_block_expression( let source = working_set.get_span_contents(inner_span); - let (output, err) = lex(source, start, &[b' ', b'\r', b'\n', b','], &[], false); + let (output, err) = lex(source, start, &[b' ', b'\r', b'\n', b',', b'|'], &[], false); error = error.or(err); let mut position = 0; @@ -4539,7 +4539,7 @@ pub fn parse_match_block_expression( working_set.enter_scope(); // First parse the pattern - let (pattern, err) = parse_pattern(working_set, output[position].span); + let (mut pattern, err) = parse_pattern(working_set, output[position].span); error = error.or(err); position += 1; @@ -4555,19 +4555,75 @@ pub fn parse_match_block_expression( break; } - // Then the => - let thick_arrow = working_set.get_span_contents(output[position].span); - if thick_arrow != b"=>" { + // Multiple patterns connected by '|' + let mut connector = working_set.get_span_contents(output[position].span); + if connector == b"|" && position < output.len() { + let mut or_pattern = vec![pattern]; + + while connector == b"|" && position < output.len() { + connector = b""; + + position += 1; + + if position >= output.len() { + error = error.or(Some(ParseError::Mismatch( + "pattern".into(), + "end of input".into(), + Span::new(output[position - 1].span.end, output[position - 1].span.end), + ))); + + working_set.exit_scope(); + break; + } + + let (pattern, err) = parse_pattern(working_set, output[position].span); + error = error.or(err); + or_pattern.push(pattern); + + position += 1; + if position >= output.len() { + error = error.or(Some(ParseError::Mismatch( + "=>".into(), + "end of input".into(), + Span::new(output[position - 1].span.end, output[position - 1].span.end), + ))); + + working_set.exit_scope(); + break; + } else { + connector = working_set.get_span_contents(output[position].span); + } + } + + let start = or_pattern + .first() + .expect("internal error: unexpected state of or-pattern") + .span + .start; + let end = or_pattern + .last() + .expect("internal error: unexpected state of or-pattern") + .span + .end; + + pattern = MatchPattern { + pattern: Pattern::Or(or_pattern), + span: Span::new(start, end), + } + } + + // Then the `=>` arrow + if connector != b"=>" { error = error.or(Some(ParseError::Mismatch( "=>".into(), "end of input".into(), Span::new(output[position - 1].span.end, output[position - 1].span.end), ))); + } else { + position += 1; } // Finally, the value/expression/block that we will run to produce the result - position += 1; - if position >= output.len() { error = error.or(Some(ParseError::Mismatch( "match result".into(), @@ -6094,6 +6150,11 @@ pub fn discover_captures_in_pattern(pattern: &MatchPattern, seen: &mut Vec { + for pattern in patterns { + discover_captures_in_pattern(pattern, seen) + } + } Pattern::Value(_) | Pattern::IgnoreValue | Pattern::Garbage => {} } } diff --git a/crates/nu-protocol/src/ast/match_pattern.rs b/crates/nu-protocol/src/ast/match_pattern.rs index 9428eddb0..d5ec1013f 100644 --- a/crates/nu-protocol/src/ast/match_pattern.rs +++ b/crates/nu-protocol/src/ast/match_pattern.rs @@ -10,12 +10,48 @@ pub struct MatchPattern { pub span: Span, } +impl MatchPattern { + pub fn variables(&self) -> Vec { + self.pattern.variables() + } +} + #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub enum Pattern { Record(Vec<(String, MatchPattern)>), List(Vec), Value(Expression), Variable(VarId), + Or(Vec), IgnoreValue, // the _ pattern Garbage, } + +impl Pattern { + pub fn variables(&self) -> Vec { + let mut output = vec![]; + match self { + Pattern::Record(items) => { + for item in items { + output.append(&mut item.1.variables()); + } + } + Pattern::List(items) => { + for item in items { + output.append(&mut item.variables()); + } + } + Pattern::Value(_) => {} + Pattern::Variable(var_id) => output.push(*var_id), + Pattern::Or(patterns) => { + for pattern in patterns { + output.append(&mut pattern.variables()); + } + } + Pattern::IgnoreValue => {} + Pattern::Garbage => {} + } + + output + } +} diff --git a/crates/nu-protocol/src/engine/engine_state.rs b/crates/nu-protocol/src/engine/engine_state.rs index a768db654..2b4dda8c1 100644 --- a/crates/nu-protocol/src/engine/engine_state.rs +++ b/crates/nu-protocol/src/engine/engine_state.rs @@ -1803,6 +1803,24 @@ impl<'a> StateWorkingSet<'a> { None } + pub fn find_variable_in_current_frame(&self, name: &[u8]) -> Option { + let mut removed_overlays = vec![]; + + for scope_frame in self.delta.scope.iter().rev().take(1) { + for overlay_frame in scope_frame + .active_overlays(&mut removed_overlays) + .iter() + .rev() + { + if let Some(var_id) = overlay_frame.vars.get(name) { + return Some(*var_id); + } + } + } + + None + } + pub fn add_variable( &mut self, mut name: Vec, diff --git a/crates/nu-protocol/src/engine/pattern_match.rs b/crates/nu-protocol/src/engine/pattern_match.rs index 2159ac66f..448163f1b 100644 --- a/crates/nu-protocol/src/engine/pattern_match.rs +++ b/crates/nu-protocol/src/engine/pattern_match.rs @@ -1,6 +1,6 @@ use crate::{ ast::{Expr, MatchPattern, Pattern, RangeInclusion}, - Unit, Value, VarId, + Span, Unit, Value, VarId, }; pub trait Matcher { @@ -217,6 +217,54 @@ impl Matcher for Pattern { _ => false, } } + Pattern::Or(patterns) => { + let mut result = false; + + for pattern in patterns { + let mut local_matches = vec![]; + if !result { + if pattern.match_value(value, &mut local_matches) { + // TODO: do we need to replace previous variables that defaulted to nothing? + matches.append(&mut local_matches); + result = true; + } else { + // Create variables that don't match and assign them to null + let vars = pattern.variables(); + for var in &vars { + let mut found = false; + for match_ in matches.iter() { + if match_.0 == *var { + found = true; + } + } + + if !found { + // FIXME: don't use Span::unknown() + matches.push((*var, Value::nothing(Span::unknown()))) + } + } + } + } else { + // We already have a match, so ignore the remaining match variables + // And assign them to null + let vars = pattern.variables(); + for var in &vars { + let mut found = false; + for match_ in matches.iter() { + if match_.0 == *var { + found = true; + } + } + + if !found { + // FIXME: don't use Span::unknown() + matches.push((*var, Value::nothing(Span::unknown()))) + } + } + } + } + result + } } } }