fix(parser): reject unexpected semicolons in list literals and patterns (#19063)

## Description

List literals such as `[1; 2]` previously evaluated to `[1]`, silently
discarding content after the semicolon. The same issue affected `match`
list patterns and could cause incorrect matches.

In this PR, I added an error check that reports parse error for
unexpected semicolon separators in both list literals and list patterns,
with guidance to use commas or whitespace instead.

It also aligns the NUON table grammar with the documented requirement
for at least one data row and marks the header-only table parsing bug as
fixed.

Note: this version also rejects `[;1]`, `[1;]`, and `[;]`. I can't find
rules in the NUON spec that allow leading or trailing semicolons. The
parser now rejects all semicolon separators in list literals but I think
this should be discussed further in #11631 before merging, since it may
break existing code that relies on the current behavior.

## User-facing changes (Release notes)

List literals and `match` list patterns now reject unexpected semicolon
separators instead of silently accepting malformed input or discarding
subsequent content.

Header-only table syntax such as `[[a b];]` now also produces a parse
error.

## Additional notes

Fixes #11631.

Related: the semicolon discussion in #14188 and the NUON specification
added in #18925.

AI assistance (GPT-6 Astra) was used to investigate related issues and
add regression tests. The final review and writing of this PR were done
by myself.
This commit is contained in:
Charlie authored and GitHub committed 2026-09-22 12:21:42 -05:00
1 parent 3e080262cc
commit 9c9f93c727
5 files changed
+177 -6

No files matched your search

+22 -2
View File
@@ -130,6 +130,18 @@ pub fn parse_list_expression(
working_set.error(err)
}
if let Some(token) = output
.iter()
.find(|token| token.contents == TokenContents::Semicolon)
{
working_set.error(ParseError::LabeledErrorWithHelp {
error: "Unexpected semicolon in list".into(),
label: "not a valid list separator".into(),
help: "Use commas or whitespace to separate list items.".into(),
span: token.span,
});
}
let (mut output, err) = lite_parse(&output, working_set);
if let Some(err) = err {
working_set.error(err)
@@ -261,18 +273,26 @@ pub(crate) fn parse_table_expression(
let [first, second, rest @ ..] = &tokens[..] else {
return parse_list_expression(working_set, span, list_element_shape);
};
if !working_set.get_span_contents(first.span).starts_with(b"[")
|| second.contents != TokenContents::Semicolon
|| rest.is_empty()
{
return parse_list_expression(working_set, span, list_element_shape);
};
}
let head = parse_table_row(working_set, first.span);
let errors = working_set.parse_errors.len();
let (head, rows) = match head {
Ok((head, _)) => {
if rest.is_empty() {
working_set.error(ParseError::Expected(
"table row",
Span::new(second.span.end, second.span.end),
));
}
let rows = rest
.iter()
.filter_map(|it| {
+13 -1
View File
@@ -1,7 +1,7 @@
#![allow(clippy::byte_char_slices)]
use crate::{
lex, lite_parse,
TokenContents, lex, lite_parse,
parse_helpers::is_variable,
parser::{ensure_not_reserved_variable_name, parse_value},
};
@@ -125,6 +125,18 @@ pub fn parse_list_pattern(working_set: &mut StateWorkingSet, span: Span) -> Matc
working_set.error(err);
}
if let Some(token) = output
.iter()
.find(|token| token.contents == TokenContents::Semicolon)
{
working_set.error(ParseError::LabeledErrorWithHelp {
error: "Unexpected semicolon in list pattern".into(),
label: "not a valid list separator".into(),
help: "Use commas or whitespace to separate list items.".into(),
span: token.span,
});
}
let (output, err) = lite_parse(&output, working_set);
if let Some(err) = err {
working_set.error(err);
+139
View File
@@ -3751,6 +3751,145 @@ fn conditional_branch_types(#[case] code: &str, #[case] expected_tys: &[Type]) {
assert_eq!(out_ty, expected_ty);
}
#[rstest]
#[case::malformed_table("[a b c; [1 2 3]]")]
#[case::between_items("[1; 2]")]
#[case::before_newline("[1;\n2]")]
#[case::before_variable("[1; $undefined]")]
#[case::leading("[;1]")]
#[case::trailing("[1;]")]
#[case::empty("[;]")]
#[case::repeated("[1;; 2]")]
#[case::nested_list("[[1; 2]]")]
#[case::external_argument("^foo [1; 2]")]
#[case::table_header("[[a; b]; [1]]")]
#[case::table_row("[[a b]; [1 2; 3 4]]")]
fn reject_invalid_list_semicolons(#[case] source: &str) {
let engine_state = EngineState::new();
let mut working_set = StateWorkingSet::new(&engine_state);
let _ = parse(&mut working_set, None, source.as_bytes(), false);
let error = working_set
.parse_errors
.first()
.expect("a semicolon in a list must produce a parse error");
assert!(
matches!(error, ParseError::LabeledErrorWithHelp { error, .. }
if error == "Unexpected semicolon in list"),
"unexpected diagnostic: {error:?}"
);
assert_eq!(working_set.get_span_contents(error.span()), b";");
}
#[rstest]
#[case::plain("[[a b];]")]
#[case::whitespace("[[a b]; \n]")]
#[case::comment("[[a b]; # no rows\n]")]
#[case::empty_header("[[];]")]
fn table_without_rows_reports_missing_row(#[case] source: &str) {
let engine_state = EngineState::new();
let mut working_set = StateWorkingSet::new(&engine_state);
let _ = parse(&mut working_set, None, source.as_bytes(), false);
assert!(
matches!(
working_set.parse_errors.as_slice(),
[ParseError::Expected("table row", _)]
),
"unexpected diagnostics: {:?}",
working_set.parse_errors
);
let span = working_set.parse_errors[0].span();
assert_eq!(span.start, span.end);
assert!(span.start > 0);
assert_eq!(
working_set.get_span_contents(Span::new(span.start - 1, span.start)),
b";"
);
}
#[rstest]
#[case::double_quoted(r#"["a;b" 2]"#)]
#[case::single_quoted("['a;b' 2]")]
#[case::backtick_quoted("[`a;b` 2]")]
#[case::raw_string("[r#'a;b'# 2]")]
#[case::interpolation(r#"[$"(1; 2)" 3]"#)]
#[case::comment("[1 # ; ignored\n 2]")]
#[case::subexpression("[(1; 2) 3]")]
#[case::closure("[{1; 2} 3]")]
#[case::nested_table("[[[a]; [1]] 2]")]
#[case::table_separator("[[a]; [1]]")]
#[case::table_quoted_header(r#"[["a;b"]; [1]]"#)]
#[case::table_subexpression_cell("[[a]; [(1; 2)]]")]
#[case::table_nested_table_cell("[[a]; [[[b]; [1]]]]")]
fn accept_valid_list_semicolons(#[case] source: &str) {
let engine_state = EngineState::new();
let mut working_set = StateWorkingSet::new(&engine_state);
let _ = parse(&mut working_set, None, source.as_bytes(), false);
assert!(
working_set.parse_errors.is_empty(),
"{:?}",
working_set.parse_errors
);
}
#[rstest]
#[case::between_items("[1; 2]")]
#[case::before_newline("[1;\n2]")]
#[case::leading("[;1]")]
#[case::trailing("[1;]")]
#[case::empty("[;]")]
#[case::repeated("[1;; 2]")]
#[case::nested_list("[[1; 2]]")]
#[case::nested_record("[{a: [1; 2]}]")]
#[case::ignore_rest("[1 ..; 2]")]
#[case::capture_rest("[1 ..$rest; 2]")]
fn reject_invalid_list_pattern_semicolons(#[case] pattern: &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 [] {{ {pattern} => true, _ => false }}");
let _ = parse(&mut working_set, None, source.as_bytes(), false);
let error = working_set
.parse_errors
.first()
.expect("a semicolon in a list pattern must produce a parse error");
assert!(
matches!(error, ParseError::LabeledErrorWithHelp { error, .. }
if error == "Unexpected semicolon in list pattern"),
"unexpected diagnostic: {error:?}"
);
assert_eq!(working_set.get_span_contents(error.span()), b";");
}
#[rstest]
#[case::double_quoted(r#"["a;b" 2]"#)]
#[case::single_quoted("['a;b' 2]")]
#[case::backtick_quoted("[`a;b` 2]")]
#[case::raw_string("[r#'a;b'# 2]")]
#[case::comment("[1 # ; ignored\n 2]")]
#[case::constant_subexpression("[(1; 2)]")]
#[case::nested_list(r#"[["a;b"]]"#)]
#[case::nested_record(r#"[{a: "b;c"}]"#)]
#[case::ignore_rest("[1 ..]")]
#[case::capture_rest("[1 ..$rest]")]
fn accept_valid_list_pattern_semicolons(#[case] pattern: &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 [] {{ {pattern} => true, _ => false }}");
let _ = parse(&mut working_set, None, source.as_bytes(), false);
assert!(
working_set.parse_errors.is_empty(),
"{:?}",
working_set.parse_errors
);
}
#[test]
fn record_semicolon_gives_separator_help() {
let engine_state = EngineState::new();
+2 -2
View File
@@ -9,7 +9,7 @@ the list is empty, this file goes away and so does the todo at the top of the sp
- [ ] bug 1 - a leading utf-8 bom is rejected. strip one instead.
- [ ] bug 2 - the empty document decodes to `null`. error instead.
- [ ] bug 3 - `[[a b];]` is a list containing the header, not an empty table. error instead.
- [x] bug 3 - `[[a b];]` is a list containing the header, not an empty table. error instead.
- [ ] bug 4 - an empty container under indentation emits a blank line. do not emit it.
- [ ] bug 5 - `--raw --no-commas` emits no separator and loses data. make the two exclusive.
- [ ] bug 6 - table width is measured in bytes but padded in runes. use one measure for both.
@@ -51,7 +51,7 @@ byte for byte still has to reproduce these until they are fixed.
```
- not an empty table - a list containing the header list. an empty table is not expressible
in the table form; `[]` must be used.
- **reproduce**, for compatibility. documenting it as an intended feature would be dishonest.
- **fixed**: the parser now recognizes this as an incomplete table and reports the missing row.
- bug 4 - an empty container under indentation emits a blank line.
```nushell
@@ -86,7 +86,7 @@
list := "[" ws* (value (sep value)* sep?)? ws* "]"
record := "{" ws* (pair (sep pair)* sep?)? ws* "}"
pair := key ws* ":" ws* value
table := "[" ws* list ws* ";" ws* (list (sep list)* sep?)? ws* "]"
table := "[" ws* list ws* ";" ws* list (sep list)* sep? ws* "]"
binary := "0x[" (hex | sep)* "]" | "0b[" (bit | sep)* "]"
cell_path := "$" ("." member)*
member := (bare | '"' esc* '"' | "'" any* "'" | int) "?"?