Skip to content

Commit 8e56c4f

Browse files
committed
fix: Reject multi-element sequences without internal captures
1 parent b424a15 commit 8e56c4f

7 files changed

Lines changed: 172 additions & 33 deletions

File tree

‎crates/plotnik-compiler/src/analyze/type_check/infer.rs‎

Lines changed: 43 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -581,9 +581,9 @@ impl<'a, 'd> InferenceVisitor<'a, 'd> {
581581
let flow = match quantifier {
582582
QuantifierKind::Optional => self.make_flow_optional(inner_info.flow),
583583
QuantifierKind::ZeroOrMore | QuantifierKind::OneOrMore => {
584-
if !is_row_capture {
585-
self.check_strict_dimensionality(quant, &inner_info);
586-
}
584+
// Always check multi-element sequences (row capture doesn't help)
585+
// Only skip internal capture check when is_row_capture
586+
self.check_strict_dimensionality(quant, &inner_info, is_row_capture);
587587
self.make_flow_array(inner_info.flow, &inner, quantifier.is_non_empty())
588588
}
589589
};
@@ -681,8 +681,46 @@ impl<'a, 'd> InferenceVisitor<'a, 'd> {
681681
}
682682

683683
/// Check strict dimensionality rule for * and + quantifiers.
684-
/// Captures inside a quantifier are forbidden unless marked as a row capture.
685-
fn check_strict_dimensionality(&mut self, quant: &QuantifiedExpr, inner_info: &TermInfo) {
684+
///
685+
/// Two checks:
686+
/// 1. Multi-element patterns (Arity::Many) without captures can't be scalar arrays
687+
/// (applies regardless of is_row_capture - row capture doesn't help here)
688+
/// 2. Internal captures require a row capture on the quantifier
689+
/// (skipped when is_row_capture=true)
690+
fn check_strict_dimensionality(
691+
&mut self,
692+
quant: &QuantifiedExpr,
693+
inner_info: &TermInfo,
694+
is_row_capture: bool,
695+
) {
696+
let op = quant
697+
.operator()
698+
.map(|t| t.text().to_string())
699+
.unwrap_or_else(|| "*".to_string());
700+
701+
// Check 1: Multi-element patterns without captures can't be scalar arrays
702+
// This check applies even with row capture - you can't meaningfully capture
703+
// multiple nodes per iteration as a scalar
704+
if inner_info.arity == Arity::Many && inner_info.flow.is_void() {
705+
self.diag
706+
.report(
707+
self.source_id,
708+
DiagnosticKind::MultiElementScalarCapture,
709+
quant.text_range(),
710+
)
711+
.message(format!(
712+
"sequence with `{}` matches multiple nodes but has no internal captures",
713+
op
714+
))
715+
.emit();
716+
return;
717+
}
718+
719+
// Check 2: Internal captures require row capture (skip if already a row capture)
720+
if is_row_capture {
721+
return;
722+
}
723+
686724
let TypeFlow::Bubble(type_id) = &inner_info.flow else {
687725
return;
688726
};
@@ -695,11 +733,6 @@ impl<'a, 'd> InferenceVisitor<'a, 'd> {
695733
return;
696734
}
697735

698-
let op = quant
699-
.operator()
700-
.map(|t| t.text().to_string())
701-
.unwrap_or_else(|| "*".to_string());
702-
703736
let capture_names: Vec<_> = fields
704737
.keys()
705738
.map(|s| format!("`@{}`", self.interner.resolve(*s)))

‎crates/plotnik-compiler/src/analyze/type_check/type_check_tests.rs‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -664,6 +664,39 @@ fn error_named_node_with_capture_quantified() {
664664
");
665665
}
666666

667+
#[test]
668+
fn error_multi_element_sequence_no_captures() {
669+
let input = "Bad = {(identifier) (number)}* @items";
670+
671+
let res = Query::expect_invalid(input);
672+
673+
insta::assert_snapshot!(res, @r"
674+
error: sequence with `*` matches multiple nodes but has no internal captures
675+
|
676+
1 | Bad = {(identifier) (number)}* @items
677+
| ^^^^^^^^^^^^^^^^^^^^^^^^
678+
|
679+
help: add internal captures: `{(a) @a (b) @b}* @items`
680+
");
681+
}
682+
683+
#[test]
684+
fn error_multi_element_alternation_branch() {
685+
// Alternation where one branch is multi-element
686+
let input = "Bad = [(identifier) {(number) (string)}]* @items";
687+
688+
let res = Query::expect_invalid(input);
689+
690+
insta::assert_snapshot!(res, @r"
691+
error: sequence with `*` matches multiple nodes but has no internal captures
692+
|
693+
1 | Bad = [(identifier) {(number) (string)}]* @items
694+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
695+
|
696+
help: add internal captures: `{(a) @a (b) @b}* @items`
697+
");
698+
}
699+
667700
#[test]
668701
fn recursive_type_with_alternation() {
669702
let input = indoc! {r#"

‎crates/plotnik-compiler/src/diagnostics/message.rs‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ pub enum DiagnosticKind {
7777
MultiCaptureQuantifierNoName,
7878
UnusedBranchLabels,
7979
StrictDimensionalityViolation,
80+
MultiElementScalarCapture,
8081
UncapturedOutputWithCaptures,
8182
AmbiguousUncapturedOutputs,
8283
DuplicateCaptureInScope,
@@ -181,6 +182,9 @@ impl DiagnosticKind {
181182
Self::AmbiguousUncapturedOutputs => {
182183
Some("capture each expression explicitly: `(X) @x (Y) @y`")
183184
}
185+
Self::MultiElementScalarCapture => {
186+
Some("add internal captures: `{(a) @a (b) @b}* @items`")
187+
}
184188
_ => None,
185189
}
186190
}
@@ -253,6 +257,9 @@ impl DiagnosticKind {
253257
Self::StrictDimensionalityViolation => {
254258
"quantifier with captures requires a struct capture"
255259
}
260+
Self::MultiElementScalarCapture => {
261+
"cannot capture multi-element pattern as scalar array"
262+
}
256263
Self::UncapturedOutputWithCaptures => {
257264
"output-producing expression requires capture when siblings have captures"
258265
}
@@ -301,6 +308,7 @@ impl DiagnosticKind {
301308

302309
// Type inference errors with context
303310
Self::StrictDimensionalityViolation => "{}".to_string(),
311+
Self::MultiElementScalarCapture => "{}".to_string(),
304312
Self::DuplicateCaptureInScope => {
305313
"capture `@{}` already defined in this scope".to_string()
306314
}

‎crates/plotnik-compiler/src/emit/emit_tests.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -279,8 +279,9 @@ fn sequences_nested() {
279279

280280
#[test]
281281
fn sequences_in_quantifier() {
282+
// Sequence with internal captures - valid for struct array
282283
snap!(indoc! {r#"
283-
Test = (array {(identifier) (number)}* @items)
284+
Test = (array {(identifier) @id (number) @num}* @items)
284285
"#});
285286
}
286287

Lines changed: 31 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,38 @@
11
---
22
source: crates/plotnik-compiler/src/emit/emit_tests.rs
33
---
4-
Test = (array {(identifier) (number)}* @items)
4+
Test = (array {(identifier) @id (number) @num}* @items)
55
---
66
[strings]
77
S0 "Beauty will save the world"
8-
S1 "items"
9-
S2 "Test"
10-
S3 "array"
11-
S4 "identifier"
12-
S5 "number"
8+
S1 "id"
9+
S2 "num"
10+
S3 "items"
11+
S4 "Test"
12+
S5 "array"
13+
S6 "identifier"
14+
S7 "number"
1315

1416
[type_defs]
1517
T0 = <Node>
16-
T1 = ArrayStar(T0) ; <Node>*
17-
T2 = Struct M0:1 ; { items }
18+
T1 = Struct M0:2 ; { id, num }
19+
T2 = ArrayStar(T1) ; T1*
20+
T3 = Struct M2:1 ; { items }
21+
T4 = Struct M3:1 ; { id }
22+
T5 = Struct M4:1 ; { num }
1823

1924
[type_members]
20-
M0: S1 → T1 ; items: T1
25+
M0: S1 → T0 ; id: <Node>
26+
M1: S2 → T0 ; num: <Node>
27+
M2: S3 → T2 ; items: T2
28+
M3: S1 → T0 ; id: <Node>
29+
M4: S2 → T0 ; num: <Node>
2130

2231
[type_names]
23-
N0: S2 → T2 ; Test
32+
N0: S4 → T3 ; Test
2433

2534
[entrypoints]
26-
Test = 06 :: T2
35+
Test = 06 :: T3
2736

2837
[transitions]
2938
_ObjWrap:
@@ -35,13 +44,15 @@ _ObjWrap:
3544
Test:
3645
06 ! (array) 08
3746
07 ...
38-
08 ε [Arr] 18, 11
47+
08 ε [Arr] 22, 12
3948
10 ▶
40-
11 ε [EndArr Set(M0)] 10
41-
13 ▷ (number) [Node Push] 22, 24
42-
15 ! (identifier) 13
43-
16 ▷ _ 15, 16, 24
44-
18 ▽ _ 15, 16, 24
45-
20 ▷ _ 15, 20, 24
46-
22 ▷ _ 15, 20, 24
47-
24 △ [EndArr Set(M0)] _ 10
49+
11 ε 28
50+
12 ε [EndArr Set(M2)] 10
51+
14 ε [EndObj Push] 26, 11
52+
16 ▷ (number) [Node Set(M1)] 14
53+
18 ! [Obj] (identifier) [Node Set(M0)] 16
54+
20 ▷ _ 18, 20, 28
55+
22 ▽ _ 18, 20, 28
56+
24 ▷ _ 18, 24, 28
57+
26 ▷ _ 18, 24, 28
58+
28 △ [EndArr Set(M2)] _ 10

‎crates/plotnik-compiler/src/parser/tests/grammar/sequences_tests.rs‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -177,11 +177,14 @@ fn sequence_with_captures() {
177177

178178
#[test]
179179
fn sequence_with_quantifier() {
180+
// Note: This tests parser behavior only. The pattern `{(a) (b)}+` is
181+
// semantically invalid (multi-element sequence without captures), but
182+
// we're only testing that the parser produces the correct CST.
180183
let input = indoc! {r#"
181184
Q = {(a) (b)}+
182185
"#};
183186

184-
let res = Query::expect_valid_cst(input);
187+
let res = Query::parse_cst(input);
185188

186189
insta::assert_snapshot!(res, @r#"
187190
Root
@@ -306,11 +309,14 @@ fn sequence_with_alternation() {
306309

307310
#[test]
308311
fn sequence_comma_separated_expression() {
312+
// Note: This tests parser behavior only. The inner pattern `{"," (number)}*`
313+
// is semantically invalid (multi-element sequence without captures), but
314+
// we're only testing that the parser produces the correct CST.
309315
let input = indoc! {r#"
310316
Q = {(number) {"," (number)}*}
311317
"#};
312318

313-
let res = Query::expect_valid_cst(input);
319+
let res = Query::parse_cst(input);
314320

315321
insta::assert_snapshot!(res, @r#"
316322
Root

‎crates/plotnik-compiler/src/query/query_tests.rs‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,47 @@ impl QueryAnalyzed {
3636
query
3737
}
3838

39+
/// Parse and validate syntax only (no semantic analysis).
40+
/// Use this for pure parser/grammar tests.
41+
#[track_caller]
42+
fn parse_syntax_only(src: &str) -> Self {
43+
use crate::diagnostics::DiagnosticKind::*;
44+
let source_map = SourceMap::one_liner(src);
45+
let query = QueryBuilder::new(source_map).parse().unwrap().analyze();
46+
// Only check for parse errors (not semantic errors)
47+
let diag = query.diagnostics();
48+
let has_parse_error = diag.raw().iter().any(|d| {
49+
matches!(
50+
d.kind,
51+
UnclosedTree
52+
| UnclosedSequence
53+
| UnclosedAlternation
54+
| UnclosedRegex
55+
| ExpectedExpression
56+
| ExpectedTypeName
57+
| ExpectedCaptureName
58+
| ExpectedFieldName
59+
| ExpectedSubtype
60+
| ExpectedPredicateValue
61+
| EmptyTree
62+
| EmptyAnonymousNode
63+
| EmptySequence
64+
| EmptyAlternation
65+
| BareIdentifier
66+
| InvalidSeparator
67+
| UnexpectedToken
68+
)
69+
});
70+
71+
if has_parse_error {
72+
panic!(
73+
"Expected valid syntax, got parse error:\n{}",
74+
query.dump_diagnostics()
75+
);
76+
}
77+
query
78+
}
79+
3980
#[track_caller]
4081
pub fn expect(src: &str) -> Self {
4182
let source_map = SourceMap::one_liner(src);
@@ -52,6 +93,12 @@ impl QueryAnalyzed {
5293
Self::parse_and_validate(src).dump_cst()
5394
}
5495

96+
/// Parse-only CST dump (for pure parser tests, no semantic validation).
97+
#[track_caller]
98+
pub fn parse_cst(src: &str) -> String {
99+
Self::parse_syntax_only(src).dump_cst()
100+
}
101+
55102
#[track_caller]
56103
pub fn expect_valid_cst_full(src: &str) -> String {
57104
Self::parse_and_validate(src).dump_cst_full()

0 commit comments

Comments
 (0)