Skip to content

Commit 9522052

Browse files
authored
More specific errors for missing values in records (#11423)
# Description Currently, when writing a record, if you don't give the value for a field, the syntax error highlights the entire record instead of pinpointing the issue. Here's some examples: ```nushell > { a: 2, 3 } # Missing colon (and value) Error: nu::parser::parse_mismatch × Parse mismatch during operation. ╭─[entry #2:1:1] 1 │ { a: 2, 3 } · ─────┬───── · ╰── expected record ╰──── > { a: 2, 3: } # Missing value Error: nu::parser::parse_mismatch × Parse mismatch during operation. ╭─[entry #3:1:1] 1 │ { a: 2, 3: } · ──────┬───── · ╰── expected record ╰──── > { a: 2, 3 4 } # Missing colon Error: nu::parser::parse_mismatch × Parse mismatch during operation. ╭─[entry #4:1:1] 1 │ { a: 2, 3 4 } · ──────┬────── · ╰── expected record ╰──── ``` In all of them, the entire record is highlighted red because an `Expr::Garbage` is returned covering that whole span: ![image](https://github.com/nushell/nushell/assets/45539777/36660b50-23be-4353-b180-3f84eff3c220) This PR is for highlighting only the part inside the record that could not be parsed. If the record literal is big, an error message pointing to the start of where the parser thinks things went wrong should help people fix their code. # User-Facing Changes Below are screenshots of the new errors: If there's a stray record key right before the record ends, it highlights only that key and tells the user it expected a colon after it: ![image](https://github.com/nushell/nushell/assets/45539777/94503256-8ea2-47dd-b69a-4b520c66f7b6) If the record ends before the value for the last field was given, it highlights the key and colon of that field and tells the user it expected a value after the colon: ![image](https://github.com/nushell/nushell/assets/45539777/2f3837ec-3b35-4b81-8c57-706f8056ac04) If there are two consecutive expressions without a colon between them, it highlights everything from the second expression to the end of the record and tells the user it expected a colon. I was tempted to add a help message suggesting adding a colon in between, but that may not always be the right thing to do. ![image](https://github.com/nushell/nushell/assets/45539777/1abaaaa8-1896-4909-bbb7-9a38cece5250) # Tests + Formatting # After Submitting
1 parent ba88027 commit 9522052

File tree

2 files changed

+46
-7
lines changed

2 files changed

+46
-7
lines changed

crates/nu-parser/src/parser.rs

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5272,15 +5272,43 @@ pub fn parse_record(working_set: &mut StateWorkingSet, span: Span) -> Expression
52725272

52735273
idx += 1;
52745274
if idx == tokens.len() {
5275-
working_set.error(ParseError::Expected("record", span));
5276-
return garbage(span);
5275+
working_set.error(ParseError::Expected(
5276+
"':'",
5277+
Span::new(curr_span.end, curr_span.end),
5278+
));
5279+
output.push(RecordItem::Pair(
5280+
garbage(curr_span),
5281+
garbage(Span::new(curr_span.end, curr_span.end)),
5282+
));
5283+
break;
52775284
}
5278-
let colon = working_set.get_span_contents(tokens[idx].span);
5285+
let colon_span = tokens[idx].span;
5286+
let colon = working_set.get_span_contents(colon_span);
52795287
idx += 1;
5280-
if idx == tokens.len() || colon != b":" {
5281-
//FIXME: need better error
5282-
working_set.error(ParseError::Expected("record", span));
5283-
return garbage(span);
5288+
if colon != b":" {
5289+
working_set.error(ParseError::Expected(
5290+
"':'",
5291+
Span::new(colon_span.start, colon_span.start),
5292+
));
5293+
output.push(RecordItem::Pair(
5294+
field,
5295+
garbage(Span::new(
5296+
colon_span.start,
5297+
tokens[tokens.len() - 1].span.end,
5298+
)),
5299+
));
5300+
break;
5301+
}
5302+
if idx == tokens.len() {
5303+
working_set.error(ParseError::Expected(
5304+
"value for record field",
5305+
Span::new(colon_span.end, colon_span.end),
5306+
));
5307+
output.push(RecordItem::Pair(
5308+
garbage(Span::new(curr_span.start, colon_span.end)),
5309+
garbage(Span::new(colon_span.end, tokens[tokens.len() - 1].span.end)),
5310+
));
5311+
break;
52845312
}
52855313
let value = parse_value(working_set, tokens[idx].span, &SyntaxShape::Any);
52865314
idx += 1;

src/tests/test_parser.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -763,3 +763,14 @@ fn properly_typecheck_rest_param() -> TestResult {
763763
fn implied_collect_has_compatible_type() -> TestResult {
764764
run_test(r#"let idx = 3 | $in; $idx < 1"#, "false")
765765
}
766+
767+
#[test]
768+
fn record_expected_colon() -> TestResult {
769+
fail_test(r#"{ a: 2 b }"#, "expected ':'")?;
770+
fail_test(r#"{ a: 2 b 3 }"#, "expected ':'")
771+
}
772+
773+
#[test]
774+
fn record_missing_value() -> TestResult {
775+
fail_test(r#"{ a: 2 b: }"#, "expected value for record field")
776+
}

0 commit comments

Comments
 (0)