From 3db0aed9f739fd929f8dff287fafd60883649c07 Mon Sep 17 00:00:00 2001 From: JT <547158+jntrnr@users.noreply.github.com> Date: Fri, 31 Mar 2023 11:08:53 +1300 Subject: [PATCH] Add rest and ignore-rest patterns (#8681) # Description Adds two more patterns when working with lists: ``` [1, ..$remainder] ``` and ``` [1, ..] ``` The first one collects the remaining items and assigns them into the variable. The second one ignores any remaining values. # User-Facing Changes Adds more capability to list pattern matching. # 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 - `cargo run -- crates/nu-utils/standard_library/tests.nu` to run the tests for the standard library > **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 | 22 ++++++ crates/nu-parser/src/flatten.rs | 6 ++ crates/nu-parser/src/parse_patterns.rs | 75 +++++++++++++------ crates/nu-parser/src/parser.rs | 3 +- crates/nu-protocol/src/ast/match_pattern.rs | 7 +- .../nu-protocol/src/engine/pattern_match.rs | 44 +++++++++-- 6 files changed, 125 insertions(+), 32 deletions(-) diff --git a/crates/nu-command/tests/commands/match_.rs b/crates/nu-command/tests/commands/match_.rs index 7b9ac34c42..0540d3d622 100644 --- a/crates/nu-command/tests/commands/match_.rs +++ b/crates/nu-command/tests/commands/match_.rs @@ -55,6 +55,28 @@ fn match_list() { assert_eq!(actual.out, "double: 1 2"); } +#[test] +fn match_list_rest_ignore() { + 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, "single: 1"); +} + +#[test] +fn match_list_rest() { + let actual = nu!( + cwd: ".", + r#"match [1, 2, 3] { [$a, ..$remainder] => { print $"single: ($a) ($remainder | math sum)" }, [$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, "single: 1 5"); +} + #[test] fn match_constant_1() { let actual = nu!( diff --git a/crates/nu-parser/src/flatten.rs b/crates/nu-parser/src/flatten.rs index cc1e5222d5..82b2306aa0 100644 --- a/crates/nu-parser/src/flatten.rs +++ b/crates/nu-parser/src/flatten.rs @@ -515,6 +515,9 @@ pub fn flatten_pattern(match_pattern: &MatchPattern) -> Vec<(Span, FlatShape)> { Pattern::IgnoreValue => { output.push((match_pattern.span, FlatShape::Nothing)); } + Pattern::IgnoreRest => { + output.push((match_pattern.span, FlatShape::Nothing)); + } Pattern::List(items) => { if let Some(first) = items.first() { if let Some(last) = items.last() { @@ -559,6 +562,9 @@ pub fn flatten_pattern(match_pattern: &MatchPattern) -> Vec<(Span, FlatShape)> { Pattern::Variable(_) => { output.push((match_pattern.span, FlatShape::Variable)); } + Pattern::Rest(_) => { + output.push((match_pattern.span, FlatShape::Variable)); + } Pattern::Or(patterns) => { for pattern in patterns { output.extend(flatten_pattern(pattern)); diff --git a/crates/nu-parser/src/parse_patterns.rs b/crates/nu-parser/src/parse_patterns.rs index eb686fe539..134d493e7e 100644 --- a/crates/nu-parser/src/parse_patterns.rs +++ b/crates/nu-parser/src/parse_patterns.rs @@ -1,7 +1,7 @@ use nu_protocol::{ ast::{Expr, Expression, MatchPattern, Pattern}, engine::StateWorkingSet, - Span, SyntaxShape, Type, + Span, SyntaxShape, Type, VarId, }; use crate::{ @@ -72,32 +72,34 @@ pub fn parse_pattern( } } -pub fn parse_variable_pattern( - working_set: &mut StateWorkingSet, - span: Span, -) -> (MatchPattern, Option) { +fn parse_variable_pattern_helper(working_set: &mut StateWorkingSet, span: Span) -> Option { let bytes = working_set.get_span_contents(span); if is_variable(bytes) { if let Some(var_id) = working_set.find_variable_in_current_frame(bytes) { - ( - MatchPattern { - pattern: Pattern::Variable(var_id), - span, - }, - None, - ) + Some(var_id) } else { let var_id = working_set.add_variable(bytes.to_vec(), span, Type::Any, false); - ( - MatchPattern { - pattern: Pattern::Variable(var_id), - span, - }, - None, - ) + Some(var_id) } + } else { + None + } +} + +pub fn parse_variable_pattern( + working_set: &mut StateWorkingSet, + span: Span, +) -> (MatchPattern, Option) { + if let Some(var_id) = parse_variable_pattern_helper(working_set, span) { + ( + MatchPattern { + pattern: Pattern::Variable(var_id), + span, + }, + None, + ) } else { ( garbage(span), @@ -143,10 +145,39 @@ pub fn parse_list_pattern( if let LiteElement::Command(_, command) = arg { while spans_idx < command.parts.len() { - let (arg, err) = parse_pattern(working_set, command.parts[spans_idx]); - error = error.or(err); + let contents = working_set.get_span_contents(command.parts[spans_idx]); + if contents == b".." { + args.push(MatchPattern { + pattern: Pattern::IgnoreRest, + span: command.parts[spans_idx], + }); + break; + } else if contents.starts_with(b"..$") { + if let Some(var_id) = parse_variable_pattern_helper( + working_set, + Span::new( + command.parts[spans_idx].start + 2, + command.parts[spans_idx].end, + ), + ) { + args.push(MatchPattern { + pattern: Pattern::Rest(var_id), + span: command.parts[spans_idx], + }); + break; + } else { + args.push(garbage(command.parts[spans_idx])); + error = error.or(Some(ParseError::Expected( + "valid variable name".into(), + command.parts[spans_idx], + ))); + } + } else { + let (arg, err) = parse_pattern(working_set, command.parts[spans_idx]); + error = error.or(err); - args.push(arg); + args.push(arg); + }; spans_idx += 1; } diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index e22c61da15..d464c63cee 100644 --- a/crates/nu-parser/src/parser.rs +++ b/crates/nu-parser/src/parser.rs @@ -6237,7 +6237,8 @@ pub fn discover_captures_in_pattern(pattern: &MatchPattern, seen: &mut Vec {} + Pattern::Rest(var_id) => seen.push(*var_id), + Pattern::Value(_) | Pattern::IgnoreValue | Pattern::IgnoreRest | Pattern::Garbage => {} } } diff --git a/crates/nu-protocol/src/ast/match_pattern.rs b/crates/nu-protocol/src/ast/match_pattern.rs index d5ec1013f8..c04bf9e9e3 100644 --- a/crates/nu-protocol/src/ast/match_pattern.rs +++ b/crates/nu-protocol/src/ast/match_pattern.rs @@ -23,6 +23,8 @@ pub enum Pattern { Value(Expression), Variable(VarId), Or(Vec), + Rest(VarId), // the ..$foo pattern + IgnoreRest, // the .. pattern IgnoreValue, // the _ pattern Garbage, } @@ -41,15 +43,14 @@ impl Pattern { 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 => {} + Pattern::Rest(var_id) => output.push(*var_id), + Pattern::Value(_) | Pattern::IgnoreValue | Pattern::Garbage | Pattern::IgnoreRest => {} } output diff --git a/crates/nu-protocol/src/engine/pattern_match.rs b/crates/nu-protocol/src/engine/pattern_match.rs index 448163f1b7..68e59dc2b1 100644 --- a/crates/nu-protocol/src/engine/pattern_match.rs +++ b/crates/nu-protocol/src/engine/pattern_match.rs @@ -18,6 +18,8 @@ impl Matcher for Pattern { match self { Pattern::Garbage => false, Pattern::IgnoreValue => true, + Pattern::IgnoreRest => false, // `..` and `..$foo` only match in specific contexts + Pattern::Rest(_) => false, // so we return false here and handle them elsewhere Pattern::Record(field_patterns) => match value { Value::Record { cols, vals, .. } => { 'top: for field_pattern in field_patterns { @@ -46,17 +48,47 @@ impl Matcher for Pattern { Pattern::List(items) => match &value { Value::List { vals, .. } => { if items.len() > vals.len() { - // We need more items in our pattern than are available in the Value - return false; + // The only we we allow this is to have a rest pattern in the n+1 position + if items.len() == (vals.len() + 1) { + match &items[vals.len()].pattern { + Pattern::IgnoreRest => {} + Pattern::Rest(var_id) => { + matches.push((*var_id, Value::nothing(items[vals.len()].span))) + } + _ => { + // There is a pattern which can't skip missing values, so we fail + return false; + } + } + } else { + // There are patterns that can't be matches, so we fail + return false; + } } - for (val_idx, val) in vals.iter().enumerate() { // We require that the pattern and the value have the same number of items, or the pattern does not match // The only exception is if the pattern includes a `..` pattern - if let Some(pattern) = items.get(val_idx) { - if !pattern.match_value(val, matches) { - return false; + match &pattern.pattern { + Pattern::IgnoreRest => { + break; + } + Pattern::Rest(var_id) => { + let rest_vals = vals[val_idx..].to_vec(); + matches.push(( + *var_id, + Value::List { + vals: rest_vals, + span: pattern.span, + }, + )); + break; + } + _ => { + if !pattern.match_value(val, matches) { + return false; + } + } } } else { return false;