Skip to content

Commit ba07f49

Browse files
zharinovzharinov
committed
Improve grammar code flow
1 parent b5cf2d6 commit ba07f49

7 files changed

Lines changed: 106 additions & 78 deletions

File tree

‎AGENTS.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,3 +182,4 @@ let name = node.name().unwrap_or_else(|| {
182182
- Follow rnix-parser/taplo patterns
183183
- Span-based tokens, no text in intermediate structures
184184
- Don't put AI slop comments in the code
185+
- IMPORTANT: Avoid nesting logic, prefer early exit code flow in functions (return) and loops (continue/break)

‎crates/plotnik-lib/src/ast/parser/grammar.rs‎

Lines changed: 60 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -257,28 +257,23 @@ impl Parser<'_> {
257257
}
258258
}
259259

260-
// Check if there are children
261260
let has_children = self.peek() != SyntaxKind::ParenClose;
262261

263-
if is_ref {
264-
if has_children {
265-
// Reference with children: commit to Tree and emit error
266-
self.start_node_at(checkpoint, SyntaxKind::Tree);
267-
let children_start = self.current_span().start();
268-
self.parse_children(SyntaxKind::ParenClose, TREE_RECOVERY);
269-
let children_end = self.last_non_trivia_end().unwrap_or(children_start);
270-
let children_span = TextRange::new(children_start, children_end);
271-
272-
if let Some(name) = &ref_name {
273-
self.errors.push(super::error::Diagnostic::error(
274-
children_span,
275-
format!("reference `{}` cannot contain children", name),
276-
));
277-
}
278-
} else {
279-
// Valid reference: no children
280-
self.start_node_at(checkpoint, SyntaxKind::Ref);
262+
if is_ref && has_children {
263+
self.start_node_at(checkpoint, SyntaxKind::Tree);
264+
let children_start = self.current_span().start();
265+
self.parse_children(SyntaxKind::ParenClose, TREE_RECOVERY);
266+
let children_end = self.last_non_trivia_end().unwrap_or(children_start);
267+
let children_span = TextRange::new(children_start, children_end);
268+
269+
if let Some(name) = &ref_name {
270+
self.errors.push(super::error::Diagnostic::error(
271+
children_span,
272+
format!("reference `{}` cannot contain children", name),
273+
));
281274
}
275+
} else if is_ref {
276+
self.start_node_at(checkpoint, SyntaxKind::Ref);
282277
} else {
283278
self.parse_children(SyntaxKind::ParenClose, TREE_RECOVERY);
284279
}
@@ -298,13 +293,6 @@ impl Parser<'_> {
298293
/// Parse children until `until` token or recovery set hit.
299294
fn parse_children(&mut self, until: SyntaxKind, recovery: TokenSet) {
300295
loop {
301-
if self.should_stop() {
302-
break;
303-
}
304-
let kind = self.peek();
305-
if kind == until {
306-
break;
307-
}
308296
if self.eof() {
309297
let (construct, delim) = match until {
310298
SyntaxKind::ParenClose => ("tree", "')'"),
@@ -325,23 +313,33 @@ impl Parser<'_> {
325313
self.error_with_related(msg, related);
326314
break;
327315
}
316+
if self.has_fatal_error() {
317+
break;
318+
}
319+
let kind = self.peek();
320+
if kind == until {
321+
break;
322+
}
328323
if SEPARATORS.contains(kind) {
329324
self.error_skip_separator();
330325
continue;
331326
}
332327
if EXPR_FIRST.contains(kind) {
333328
self.parse_expr();
334-
} else if kind == SyntaxKind::Predicate {
329+
continue;
330+
}
331+
if kind == SyntaxKind::Predicate {
335332
self.error_and_bump(
336333
"tree-sitter predicates (#eq?, #match?, #set!, etc.) are not supported",
337334
);
338-
} else if recovery.contains(kind) {
335+
continue;
336+
}
337+
if recovery.contains(kind) {
339338
break;
340-
} else {
341-
self.error_and_bump(
342-
"unexpected token; expected a child expression or closing delimiter",
343-
);
344339
}
340+
self.error_and_bump(
341+
"unexpected token; expected a child expression or closing delimiter",
342+
);
345343
}
346344
}
347345

@@ -361,13 +359,6 @@ impl Parser<'_> {
361359
/// Parse alternation children, handling both tagged `Label: expr` and unlabeled expressions.
362360
fn parse_alt_children(&mut self) {
363361
loop {
364-
if self.has_fatal_error() {
365-
break;
366-
}
367-
let kind = self.peek();
368-
if kind == SyntaxKind::BracketClose {
369-
break;
370-
}
371362
if self.eof() {
372363
let msg = "unclosed alternation; expected ']'";
373364
let open = self.delimiter_stack.last().unwrap_or_else(|| {
@@ -380,6 +371,13 @@ impl Parser<'_> {
380371
self.error_with_related(msg, related);
381372
break;
382373
}
374+
if self.has_fatal_error() {
375+
break;
376+
}
377+
let kind = self.peek();
378+
if kind == SyntaxKind::BracketClose {
379+
break;
380+
}
383381
if SEPARATORS.contains(kind) {
384382
self.error_skip_separator();
385383
continue;
@@ -392,20 +390,22 @@ impl Parser<'_> {
392390
if first_char.is_ascii_uppercase() {
393391
self.parse_branch();
394392
} else {
395-
// Lowercase: likely mistyped branch label
396393
self.parse_branch_lowercase_label();
397394
}
398-
} else if EXPR_FIRST.contains(kind) {
395+
continue;
396+
}
397+
if EXPR_FIRST.contains(kind) {
399398
self.start_node(SyntaxKind::Branch);
400399
self.parse_expr();
401400
self.finish_node();
402-
} else if ALT_RECOVERY.contains(kind) {
401+
continue;
402+
}
403+
if ALT_RECOVERY.contains(kind) {
403404
break;
404-
} else {
405-
self.error_and_bump(
406-
"unexpected token; expected a child expression or closing delimiter",
407-
);
408405
}
406+
self.error_and_bump(
407+
"unexpected token; expected a child expression or closing delimiter",
408+
);
409409
}
410410
}
411411

@@ -520,10 +520,11 @@ impl Parser<'_> {
520520

521521
self.validate_capture_name(name, span);
522522

523-
// Check for single colon (common mistake: @x : Type instead of @x :: Type)
524523
if self.peek() == SyntaxKind::Colon {
525524
self.parse_type_annotation_single_colon();
526-
} else if self.peek() == SyntaxKind::DoubleColon {
525+
return;
526+
}
527+
if self.peek() == SyntaxKind::DoubleColon {
527528
self.parse_type_annotation();
528529
}
529530
}
@@ -547,7 +548,6 @@ impl Parser<'_> {
547548

548549
/// Handle single colon type annotation (common mistake: `@x : Type` instead of `@x :: Type`)
549550
fn parse_type_annotation_single_colon(&mut self) {
550-
// Check if followed by something that looks like a type
551551
if self.peek_nth(1) != SyntaxKind::Id {
552552
return;
553553
}
@@ -558,8 +558,9 @@ impl Parser<'_> {
558558
let fix = Fix::new("::", "use '::'");
559559
self.error_with_fix(span, "single colon is not valid for type annotations", fix);
560560

561-
self.bump();
561+
self.bump(); // colon
562562

563+
// peek() skips whitespace, so this handles `@x : Type` with space
563564
if self.peek() == SyntaxKind::Id {
564565
self.bump();
565566
}
@@ -578,14 +579,17 @@ impl Parser<'_> {
578579
fn parse_negated_field(&mut self) {
579580
self.start_node(SyntaxKind::NegatedField);
580581
self.expect(SyntaxKind::Negation, "'!' for negated field");
581-
if self.peek() == SyntaxKind::Id {
582-
let span = self.current_span();
583-
let text = token_text(self.source, &self.tokens[self.pos]);
584-
self.bump();
585-
self.validate_field_name(text, span);
586-
} else {
582+
583+
if self.peek() != SyntaxKind::Id {
587584
self.error("expected field name after '!' (e.g., !value)");
585+
self.finish_node();
586+
return;
588587
}
588+
589+
let span = self.current_span();
590+
let text = token_text(self.source, &self.tokens[self.pos]);
591+
self.bump();
592+
self.validate_field_name(text, span);
589593
self.finish_node();
590594
}
591595

‎crates/plotnik-lib/src/ast/parser/tests/recovery/coverage.rs‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -391,10 +391,12 @@ fn unclosed_tree_shows_open_location() {
391391
let query = Query::new(input).unwrap();
392392
assert!(!query.is_valid());
393393
insta::assert_snapshot!(query.dump_errors(), @r"
394-
error: expected closing ')' for tree
394+
error: unclosed tree; expected ')'
395395
|
396+
1 | (call
397+
| - tree started here
396398
2 | (identifier)
397-
| ^ expected closing ')' for tree
399+
| ^ unclosed tree; expected ')'
398400
");
399401
}
400402

@@ -430,10 +432,13 @@ fn unclosed_sequence_shows_open_location() {
430432
let query = Query::new(input).unwrap();
431433
assert!(!query.is_valid());
432434
insta::assert_snapshot!(query.dump_errors(), @r"
433-
error: expected closing '}' for sequence
435+
error: unclosed sequence; expected '}'
434436
|
437+
1 | {
438+
| - sequence started here
439+
2 | (a)
435440
3 | (b)
436-
| ^ expected closing '}' for sequence
441+
| ^ unclosed sequence; expected '}'
437442
");
438443
}
439444

‎crates/plotnik-lib/src/ast/parser/tests/recovery/incomplete.rs‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -152,10 +152,12 @@ fn type_annotation_invalid_token_after() {
152152
|
153153
1 | (identifier) @name :: (
154154
| ^ expected type name after '::' (e.g., ::MyType or ::string)
155-
error: expected closing ')' for tree
155+
error: unclosed tree; expected ')'
156156
|
157157
1 | (identifier) @name :: (
158-
| ^ expected closing ')' for tree
158+
| -^ unclosed tree; expected ')'
159+
| |
160+
| tree started here
159161
error: unnamed definition must be last in file; add a name: `Name = (identifier) @name ::`
160162
|
161163
1 | (identifier) @name :: (

‎crates/plotnik-lib/src/ast/parser/tests/recovery/unclosed.rs‎

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,12 @@ fn missing_paren() {
1010
let query = Query::new(input).unwrap();
1111
assert!(!query.is_valid());
1212
insta::assert_snapshot!(query.dump_errors(), @r"
13-
error: expected closing ')' for tree
13+
error: unclosed tree; expected ')'
1414
|
1515
1 | (identifier
16-
| ^ expected closing ')' for tree
16+
| - ^ unclosed tree; expected ')'
17+
| |
18+
| tree started here
1719
");
1820
}
1921

@@ -44,10 +46,12 @@ fn missing_brace() {
4446
let query = Query::new(input).unwrap();
4547
assert!(!query.is_valid());
4648
insta::assert_snapshot!(query.dump_errors(), @r"
47-
error: expected closing '}' for sequence
49+
error: unclosed sequence; expected '}'
4850
|
4951
1 | {(a) (b)
50-
| ^ expected closing '}' for sequence
52+
| - ^ unclosed sequence; expected '}'
53+
| |
54+
| sequence started here
5155
");
5256
}
5357

@@ -60,10 +64,12 @@ fn nested_unclosed() {
6064
let query = Query::new(input).unwrap();
6165
assert!(!query.is_valid());
6266
insta::assert_snapshot!(query.dump_errors(), @r"
63-
error: expected closing ')' for tree
67+
error: unclosed tree; expected ')'
6468
|
6569
1 | (a (b (c)
66-
| ^ expected closing ')' for tree
70+
| - ^ unclosed tree; expected ')'
71+
| |
72+
| tree started here
6773
");
6874
}
6975

@@ -76,10 +82,12 @@ fn deeply_nested_unclosed() {
7682
let query = Query::new(input).unwrap();
7783
assert!(!query.is_valid());
7884
insta::assert_snapshot!(query.dump_errors(), @r"
79-
error: expected closing ')' for tree
85+
error: unclosed tree; expected ')'
8086
|
8187
1 | (a (b (c (d
82-
| ^ expected closing ')' for tree
88+
| - ^ unclosed tree; expected ')'
89+
| |
90+
| tree started here
8391
");
8492
}
8593

@@ -92,10 +100,12 @@ fn unclosed_alternation_nested() {
92100
let query = Query::new(input).unwrap();
93101
assert!(!query.is_valid());
94102
insta::assert_snapshot!(query.dump_errors(), @r"
95-
error: expected closing ')' for tree
103+
error: unclosed tree; expected ')'
96104
|
97105
1 | [(a) (b
98-
| ^ expected closing ')' for tree
106+
| - ^ unclosed tree; expected ')'
107+
| |
108+
| tree started here
99109
");
100110
}
101111

‎crates/plotnik-lib/src/ast/parser/tests/recovery/validation.rs‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1076,10 +1076,12 @@ fn unclosed_double_quote_string() {
10761076
|
10771077
1 | (call "foo)
10781078
| ^^^^^ unexpected token; expected a child expression or closing delimiter
1079-
error: expected closing ')' for tree
1079+
error: unclosed tree; expected ')'
10801080
|
10811081
1 | (call "foo)
1082-
| ^ expected closing ')' for tree
1082+
| - ^ unclosed tree; expected ')'
1083+
| |
1084+
| tree started here
10831085
"#);
10841086
}
10851087

@@ -1094,10 +1096,12 @@ fn unclosed_single_quote_string() {
10941096
|
10951097
1 | (call 'foo)
10961098
| ^^^^^ unexpected token; expected a child expression or closing delimiter
1097-
error: expected closing ')' for tree
1099+
error: unclosed tree; expected ')'
10981100
|
10991101
1 | (call 'foo)
1100-
| ^ expected closing ')' for tree
1102+
| - ^ unclosed tree; expected ')'
1103+
| |
1104+
| tree started here
11011105
");
11021106
}
11031107

‎crates/plotnik-lib/src/query/errors.rs‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -165,10 +165,12 @@ mod tests {
165165
let q = Query::new("(unclosed").unwrap();
166166
let rendered = q.render_diagnostics(RenderOptions::plain());
167167
insta::assert_snapshot!(rendered, @r"
168-
error: expected closing ')' for tree
168+
error: unclosed tree; expected ')'
169169
|
170170
1 | (unclosed
171-
| ^ expected closing ')' for tree
171+
| - ^ unclosed tree; expected ')'
172+
| |
173+
| tree started here
172174
");
173175
}
174176

0 commit comments

Comments
 (0)