mirror of
https://github.com/nushell/nushell.git
synced 2026-10-28 08:54:52 +08:00
fix(match): allow .. and ..$rest anywhere in a match list pattern, like Rust (#19172)
## Description
In a `match` list pattern, `..` and `..$rest` only worked as the last
item. The parser stopped reading the list at the first rest pattern and
dropped everything after it without an error:
- `[.., 2]` parsed as `[..]`, so it matched every list.
- `[.., 2, ..]` also parsed as `[..]`.
- `[$a, ..$rest, $b]` never declared `$b`, so the arm body failed with
"Variable not found".
The matcher made the same assumption and only handled a rest pattern at
the end.
Rest patterns in a list now follow Rust's slice-pattern rules:
- A list pattern can hold one `..` or `..$rest`, at any position. Items
before it match the start of the list, items after it match the end, and
`..$rest` binds the values in between (an empty list when there are
none).
- A second `..` or `..$rest` in the same list pattern is a parse error,
as in Rust ("`..` can only be used once per slice pattern"). Each nested
list gets its own, so `[[.., 2], ..]` is fine.
- Without a rest pattern, the list must have as many values as the
pattern has items. This is unchanged.
### Before and after
```nushell
match [2] { [.., 1] => "one", [.., 2] => "two" }
match [1 2 3] { [$a, ..$rest, $b] => [$a $rest $b] }
match [2] { [.., 1, ..] => "one", [.., 2, ..] => "two" }
```
Before (0.116.2):
```text
one
Error: nu::parser::variable_not_found (points at `$b` in the arm body)
one
```
After:
```text
two
[1, [2], 3]
Error: nu::parser::error
× `..` can only be used once per list pattern
╭─[source:1:21]
1 │ match [2] { [.., 1, ..] => "one", [.., 2, ..] => "two" }
· ─┬
· ╰── extra rest pattern
╰────
help: Keep one `..` or `..$name` and remove the others.
```
## User-facing changes (Release notes)
`match` list patterns now accept `..` and `..$rest` anywhere in the
list, as in Rust, not only at
the end:
```nushell
match [1 2 3 4] { [$first, ..$middle, $last] => $middle } # [2 3]
match $steps { [.., "done"] => "finished", _ => "running" }
```
Before, every item after `..` was silently ignored, so `[.., "done"]`
matched any list. Using `..`
or `..$rest` more than once in the same list pattern is now a parse
error.
## Additional notes
closes https://github.com/nushell/nushell/issues/19169
This commit is contained in:
1 parent
c14a6f3836
commit
043ae4e425
5 files changed
+145
-68
No files matched your search
@@ -86,6 +86,14 @@ 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 the first and last items of a list, capturing the items between them.",
|
||||
example: "match [1, 2, 3, 4] { [$first, ..$middle, $last] => $middle }",
|
||||
result: Some(Value::test_list(vec![
|
||||
Value::test_int(2),
|
||||
Value::test_int(3),
|
||||
])),
|
||||
},
|
||||
Example {
|
||||
description: "Match against pipeline input.",
|
||||
example: "{a: {b: 3}} | match $in {{a: { $b }} => ($b + 10) }",
|
||||
|
||||
@@ -96,6 +96,46 @@ fn list_rest_empty() -> Result {
|
||||
.expect_value_eq(true)
|
||||
}
|
||||
|
||||
#[rstest]
|
||||
#[case::at_start("match [2] { [1, ..] => false, [2, ..] => true }")]
|
||||
#[case::at_end("match [2] { [.., 1] => false, [.., 2] => true }")]
|
||||
#[case::in_middle("match [1 2 3] { [1, .., 2] => false, [1, .., 3] => true }")]
|
||||
#[case::empty_in_middle("match [1 2] { [1, .., 2] => true }")]
|
||||
#[case::list_too_short("match [1 2] { [1, .., 1, 2] => false, _ => true }")]
|
||||
#[case::list_too_short_capture("match [1 2] { [1, ..$rest, 1, 2] => false, _ => true }")]
|
||||
#[case::empty_list("match [] { [.., 2] => false, _ => true }")]
|
||||
#[case::nested("match [[1 2] 3] { [[.., 1], ..] => false, [[.., 2], ..] => true }")]
|
||||
fn list_rest_position(#[case] code: &str) -> Result {
|
||||
test().run(code).expect_value_eq(true)
|
||||
}
|
||||
|
||||
#[rstest]
|
||||
#[case::in_middle(
|
||||
"match [1 2 3 4] { [$a, ..$rest, $b] => [$a $rest $b] }",
|
||||
test_value!([1, [2, 3], 4])
|
||||
)]
|
||||
#[case::empty_in_middle(
|
||||
"match [1 2] { [$a, ..$rest, $b] => [$a $rest $b] }",
|
||||
test_value!([1, [], 2])
|
||||
)]
|
||||
#[case::at_start(
|
||||
"match [1 2 3] { [..$rest, $b] => [$rest $b] }",
|
||||
test_value!([[1, 2], 3])
|
||||
)]
|
||||
fn list_rest_capture_position(#[case] code: &str, #[case] expected: Value) -> Result {
|
||||
test().run(code).expect_value_eq(expected)
|
||||
}
|
||||
|
||||
#[rstest]
|
||||
#[case::ignore("match [2] { [.., 2, ..] => true }")]
|
||||
#[case::capture("match [2] { [..$a, 2, ..$b] => true }")]
|
||||
#[case::mixed("match [2] { [..$a, 2, ..] => true }")]
|
||||
fn list_rest_used_twice(#[case] code: &str) -> Result {
|
||||
let err = test().run(code).expect_parse_error()?;
|
||||
assert_contains("can only be used once per list pattern", err.to_string());
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[rstest]
|
||||
#[case::int(["1", "2", "3"])]
|
||||
#[case::float(["1.4", "2.3", "3"])]
|
||||
|
||||
@@ -143,48 +143,53 @@ pub fn parse_list_pattern(working_set: &mut StateWorkingSet, span: Span) -> Matc
|
||||
}
|
||||
|
||||
let mut args = vec![];
|
||||
// Like a Rust slice pattern, a list pattern may hold one `..` or `..$name`, at any position.
|
||||
let mut has_rest = false;
|
||||
|
||||
if !output.block.is_empty() {
|
||||
for command in &output.block[0].commands {
|
||||
let mut spans_idx = 0;
|
||||
for &part in &command.parts {
|
||||
let contents = working_set.get_span_contents(part);
|
||||
|
||||
while spans_idx < command.parts.len() {
|
||||
let contents = working_set.get_span_contents(command.parts[spans_idx]);
|
||||
if contents == b".." {
|
||||
args.push(MatchPattern {
|
||||
let arg = if contents == b".." {
|
||||
MatchPattern {
|
||||
pattern: Pattern::IgnoreRest,
|
||||
guard: None,
|
||||
span: command.parts[spans_idx],
|
||||
});
|
||||
break;
|
||||
span: part,
|
||||
}
|
||||
} 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,
|
||||
),
|
||||
Span::new(part.start + 2, part.end),
|
||||
) {
|
||||
args.push(MatchPattern {
|
||||
MatchPattern {
|
||||
pattern: Pattern::Rest(var_id),
|
||||
guard: None,
|
||||
span: command.parts[spans_idx],
|
||||
});
|
||||
break;
|
||||
span: part,
|
||||
}
|
||||
} else {
|
||||
args.push(garbage(command.parts[spans_idx]));
|
||||
working_set.error(ParseError::Expected(
|
||||
"valid variable name",
|
||||
command.parts[spans_idx],
|
||||
));
|
||||
working_set.error(ParseError::Expected("valid variable name", part));
|
||||
garbage(part)
|
||||
}
|
||||
} else {
|
||||
let arg = parse_pattern(working_set, command.parts[spans_idx]);
|
||||
|
||||
args.push(arg);
|
||||
parse_pattern(working_set, part)
|
||||
};
|
||||
|
||||
spans_idx += 1;
|
||||
let is_rest = matches!(arg.pattern, Pattern::IgnoreRest | Pattern::Rest(_));
|
||||
if is_rest && has_rest {
|
||||
// An extra `..$name` was still parsed above, so its variable is declared
|
||||
// and the arm body reports only this error, not also "Variable not found".
|
||||
working_set.error(ParseError::LabeledErrorWithHelp {
|
||||
error: "`..` can only be used once per list pattern".into(),
|
||||
label: "extra rest pattern".into(),
|
||||
help: "Keep one `..` or `..$name` and remove the others.".into(),
|
||||
span: part,
|
||||
});
|
||||
args.push(garbage(part));
|
||||
} else {
|
||||
has_rest |= is_rest;
|
||||
args.push(arg);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -3890,6 +3890,35 @@ fn accept_valid_list_pattern_semicolons(#[case] pattern: &str) {
|
||||
);
|
||||
}
|
||||
|
||||
/// A second `..` or `..$name` in one list pattern gives one error at the extra rest. The arm
|
||||
/// body can still use the extra rest's variable without also getting "Variable not found".
|
||||
#[rstest]
|
||||
#[case::ignore("[.., 2, ..] => true", "..")]
|
||||
#[case::capture("[..$a, 2, ..$b] => $b", "..$b")]
|
||||
fn reject_second_list_pattern_rest(#[case] arm: &str, #[case] extra_rest: &str) {
|
||||
let engine_state = EngineState::new();
|
||||
let mut working_set = StateWorkingSet::new(&engine_state);
|
||||
working_set.add_decl(Box::new(MatchMocked));
|
||||
let source = format!("match [] {{ {arm}, _ => false }}");
|
||||
let _ = parse(&mut working_set, None, source.as_bytes(), false);
|
||||
|
||||
let [error] = working_set.parse_errors.as_slice() else {
|
||||
panic!(
|
||||
"expected one parse error, got {:?}",
|
||||
working_set.parse_errors
|
||||
);
|
||||
};
|
||||
assert!(
|
||||
matches!(error, ParseError::LabeledErrorWithHelp { error, .. }
|
||||
if error == "`..` can only be used once per list pattern"),
|
||||
"unexpected diagnostic: {error:?}"
|
||||
);
|
||||
assert_eq!(
|
||||
working_set.get_span_contents(error.span()),
|
||||
extra_rest.as_bytes()
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn record_semicolon_gives_separator_help() {
|
||||
let engine_state = EngineState::new();
|
||||
|
||||
@@ -47,50 +47,45 @@ impl Matcher for Pattern {
|
||||
}
|
||||
Pattern::List(items) => match &value {
|
||||
Value::List { vals, .. } => {
|
||||
if items.len() > vals.len() {
|
||||
// We only 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::list(Vec::new(), 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) {
|
||||
match &pattern.pattern {
|
||||
Pattern::IgnoreRest => {
|
||||
break;
|
||||
}
|
||||
Pattern::Rest(var_id) => {
|
||||
let rest_vals = vals[val_idx..].to_vec();
|
||||
matches.push((*var_id, Value::list(rest_vals, pattern.span)));
|
||||
break;
|
||||
}
|
||||
_ => {
|
||||
if !pattern.match_value(val, matches) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
}
|
||||
} else {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
// The parser allows at most one `..` or `..$name` per list pattern, at any
|
||||
// position. Items before it match the start of the list, items after it
|
||||
// match the end, and the rest pattern takes whatever is left in between.
|
||||
let rest_idx = items.iter().position(|item| {
|
||||
matches!(item.pattern, Pattern::IgnoreRest | Pattern::Rest(_))
|
||||
});
|
||||
let (before, rest, after) = match rest_idx {
|
||||
Some(idx) => (&items[..idx], Some(&items[idx]), &items[idx + 1..]),
|
||||
None => (&items[..], None, &[][..]),
|
||||
};
|
||||
|
||||
true
|
||||
// Without a rest pattern the lengths must be equal; with one the list may be longer.
|
||||
let fixed_len = before.len() + after.len();
|
||||
if vals.len() < fixed_len || (rest.is_none() && vals.len() > fixed_len) {
|
||||
return false;
|
||||
}
|
||||
let after_start = vals.len() - after.len();
|
||||
|
||||
// Bind left to right, so a variable used twice keeps its rightmost value.
|
||||
if !before
|
||||
.iter()
|
||||
.zip(vals)
|
||||
.all(|(pattern, val)| pattern.match_value(val, matches))
|
||||
{
|
||||
return false;
|
||||
}
|
||||
if let Some(MatchPattern {
|
||||
pattern: Pattern::Rest(var_id),
|
||||
span,
|
||||
..
|
||||
}) = rest
|
||||
{
|
||||
let rest_vals = vals[before.len()..after_start].to_vec();
|
||||
matches.push((*var_id, Value::list(rest_vals, *span)));
|
||||
}
|
||||
after
|
||||
.iter()
|
||||
.zip(&vals[after_start..])
|
||||
.all(|(pattern, val)| pattern.match_value(val, matches))
|
||||
}
|
||||
_ => false,
|
||||
},
|
||||
|
||||
Reference in new issue
Block a user