Skip to content

Commit 13aeaa0

Browse files
chancancodechancancode
committed
Second attempt at landing #1552 (part 2/2)
The previous/existing "parsing" code tries to piggy back on letting the HTML tokenizer/parser parse out the block params syntax into attribute nodes and tries to recover the information after the fact This is fundamentally broken and suffers from some wild edge cases like: ``` <div as="" |foo="bar|>...</div> // => <div as |foo|>...</div> ``` This is especially problematic in light of wanting to retain the source spans of the block params nodes. This commit rework the block params parsing and integrate it into the normal parsing pass in the appropiate time.
1 parent 3104daf commit 13aeaa0

File tree

10 files changed

+731
-196
lines changed

10 files changed

+731
-196
lines changed

‎packages/@glimmer-workspace/integration-tests/test/syntax/general-errors-test.ts

+150-29
Original file line numberDiff line numberDiff line change
@@ -51,18 +51,34 @@ class SyntaxErrors extends RenderTest {
5151
);
5252
}
5353

54+
@test
55+
'Block params in HTML syntax - requires a space between as and pipes'() {
56+
this.assert.throws(
57+
() => {
58+
preprocess('<x-bar as|foo|>foo</x-bar>', { meta: { moduleName: 'test-module' } });
59+
},
60+
syntaxErrorFor(
61+
'Invalid block parameters syntax: expecting at least one space character between "as" and "|"',
62+
'as|',
63+
'test-module',
64+
1,
65+
7
66+
)
67+
);
68+
}
69+
5470
@test
5571
'Block params in HTML syntax - Throws exception if given zero parameters'() {
5672
this.assert.throws(
5773
() => {
5874
preprocess('<x-bar as ||>foo</x-bar>', { meta: { moduleName: 'test-module' } });
5975
},
6076
syntaxErrorFor(
61-
'Cannot use zero block parameters',
62-
'<x-bar as ||>foo</x-bar>',
77+
'Invalid block parameters syntax: empty parameters list, expecting at least one identifier',
78+
'as ||',
6379
'test-module',
6480
1,
65-
0
81+
7
6682
)
6783
);
6884

@@ -71,11 +87,108 @@ class SyntaxErrors extends RenderTest {
7187
preprocess('<x-bar as | |>foo</x-bar>', { meta: { moduleName: 'test-module' } });
7288
},
7389
syntaxErrorFor(
74-
'Cannot use zero block parameters',
75-
'<x-bar as | |>foo</x-bar>',
90+
'Invalid block parameters syntax: empty parameters list, expecting at least one identifier',
91+
'as | |',
7692
'test-module',
7793
1,
78-
0
94+
7
95+
)
96+
);
97+
}
98+
99+
@test
100+
'Block params in HTML syntax - invalid mustaches in block params list'() {
101+
this.assert.throws(
102+
() => {
103+
preprocess('<x-bar as |{{foo}}|>foo</x-bar>', { meta: { moduleName: 'test-module' } });
104+
},
105+
syntaxErrorFor(
106+
'Invalid block parameters syntax: mustaches cannot be used inside parameters list',
107+
'{{foo}}',
108+
'test-module',
109+
1,
110+
11
111+
)
112+
);
113+
114+
this.assert.throws(
115+
() => {
116+
preprocess('<x-bar as |foo{{bar}}|>foo</x-bar>', { meta: { moduleName: 'test-module' } });
117+
},
118+
syntaxErrorFor(
119+
'Invalid block parameters syntax: mustaches cannot be used inside parameters list',
120+
'{{bar}}',
121+
'test-module',
122+
1,
123+
14
124+
)
125+
);
126+
127+
this.assert.throws(
128+
() => {
129+
preprocess('<x-bar as |foo {{bar}}|>foo</x-bar>', { meta: { moduleName: 'test-module' } });
130+
},
131+
syntaxErrorFor(
132+
'Invalid block parameters syntax: mustaches cannot be used inside parameters list',
133+
'{{bar}}',
134+
'test-module',
135+
1,
136+
15
137+
)
138+
);
139+
140+
this.assert.throws(
141+
() => {
142+
preprocess('<x-bar as |foo| {{bar}}>foo</x-bar>', { meta: { moduleName: 'test-module' } });
143+
},
144+
syntaxErrorFor(
145+
'Invalid block parameters syntax: modifiers cannot follow parameters list',
146+
'{{bar}}',
147+
'test-module',
148+
1,
149+
16
150+
)
151+
);
152+
}
153+
154+
@test
155+
'Block params in HTML syntax - EOF in block params list'() {
156+
this.assert.throws(
157+
() => {
158+
preprocess('<x-bar as |', { meta: { moduleName: 'test-module' } });
159+
},
160+
syntaxErrorFor(
161+
'Invalid block parameters syntax: expecting the tag to be closed with ">" or "/>" after parameters list',
162+
'as |',
163+
'test-module',
164+
1,
165+
7
166+
)
167+
);
168+
169+
this.assert.throws(
170+
() => {
171+
preprocess('<x-bar as |foo', { meta: { moduleName: 'test-module' } });
172+
},
173+
syntaxErrorFor(
174+
'Invalid block parameters syntax: expecting the tag to be closed with ">" or "/>" after parameters list',
175+
'as |foo',
176+
'test-module',
177+
1,
178+
7
179+
)
180+
);
181+
182+
this.assert.throws(
183+
() => {
184+
preprocess('<x-bar as |foo|', { meta: { moduleName: 'test-module' } });
185+
},
186+
syntaxErrorFor(
187+
'Invalid block parameters syntax: expecting the tag to be closed with ">" or "/>" after parameters list',
188+
'as |foo|',
189+
'test-module',
190+
1,
191+
7
79192
)
80193
);
81194
}
@@ -87,24 +200,26 @@ class SyntaxErrors extends RenderTest {
87200
preprocess('<x-bar as |x y>{{x}},{{y}}</x-bar>', { meta: { moduleName: 'test-module' } });
88201
},
89202
syntaxErrorFor(
90-
"Invalid block parameters syntax, 'as |x y'",
91-
'<x-bar as |x y>{{x}},{{y}}</x-bar>',
203+
'Invalid block parameters syntax: expecting "|" but the tag was closed prematurely',
204+
'as |x y>',
92205
'test-module',
93206
1,
94-
0
207+
7
95208
)
96209
);
97210

98211
this.assert.throws(
99212
() => {
100-
preprocess('<x-bar as |x| y>{{x}},{{y}}</x-bar>', { meta: { moduleName: 'test-module' } });
213+
preprocess('<x-bar as |x| wat>{{x}},{{y}}</x-bar>', {
214+
meta: { moduleName: 'test-module' },
215+
});
101216
},
102217
syntaxErrorFor(
103-
"Invalid block parameters syntax, 'as |x| y'",
104-
'<x-bar as |x| y>{{x}},{{y}}</x-bar>',
218+
'Invalid block parameters syntax: expecting the tag to be closed with ">" or "/>" after parameters list',
219+
'wat',
105220
'test-module',
106221
1,
107-
0
222+
14
108223
)
109224
);
110225

@@ -113,11 +228,11 @@ class SyntaxErrors extends RenderTest {
113228
preprocess('<x-bar as |x| y|>{{x}},{{y}}</x-bar>', { meta: { moduleName: 'test-module' } });
114229
},
115230
syntaxErrorFor(
116-
"Invalid block parameters syntax, 'as |x| y|'",
117-
'<x-bar as |x| y|>{{x}},{{y}}</x-bar>',
231+
'Invalid block parameters syntax: expecting the tag to be closed with ">" or "/>" after parameters list',
232+
'y|',
118233
'test-module',
119234
1,
120-
0
235+
14
121236
)
122237
);
123238
}
@@ -129,31 +244,37 @@ class SyntaxErrors extends RenderTest {
129244
preprocess('<x-bar as |x foo.bar|></x-bar>', { meta: { moduleName: 'test-module' } });
130245
},
131246
syntaxErrorFor(
132-
"Invalid identifier for block parameters, 'foo.bar'",
133-
'<x-bar as |x foo.bar|></x-bar>',
247+
'Invalid block parameters syntax: invalid identifier name `foo.bar`',
248+
'foo.bar',
134249
'test-module',
135250
1,
136-
0
251+
13
137252
)
138253
);
139254

140255
this.assert.throws(
141256
() => {
142257
preprocess('<x-bar as |x "foo"|></x-bar>', { meta: { moduleName: 'test-module' } });
143258
},
144-
syntaxErrorFor('" is not a valid character within attribute names', '', 'test-module', 1, 17)
259+
syntaxErrorFor(
260+
'Invalid block parameters syntax: invalid identifier name `"foo"`',
261+
'"foo"',
262+
'test-module',
263+
1,
264+
13
265+
)
145266
);
146267

147268
this.assert.throws(
148269
() => {
149270
preprocess('<x-bar as |foo[bar]|></x-bar>', { meta: { moduleName: 'test-module' } });
150271
},
151272
syntaxErrorFor(
152-
"Invalid identifier for block parameters, 'foo[bar]'",
153-
'<x-bar as |foo[bar]|></x-bar>',
273+
'Invalid block parameters syntax: invalid identifier name `foo[bar]`',
274+
'foo[bar]',
154275
'test-module',
155276
1,
156-
0
277+
11
157278
)
158279
);
159280
}
@@ -165,11 +286,11 @@ class SyntaxErrors extends RenderTest {
165286
preprocess('<x-bar |x|></x-bar>', { meta: { moduleName: 'test-module' } });
166287
},
167288
syntaxErrorFor(
168-
'Block parameters must be preceded by the `as` keyword, detected block parameters without `as`',
169-
'<x-bar |x|></x-bar>',
289+
'Invalid block parameters syntax: block parameters must be preceded by the `as` keyword',
290+
'|x|',
170291
'test-module',
171292
1,
172-
0
293+
7
173294
)
174295
);
175296

@@ -180,11 +301,11 @@ class SyntaxErrors extends RenderTest {
180301
});
181302
},
182303
syntaxErrorFor(
183-
'Block parameters must be preceded by the `as` keyword, detected block parameters without `as`',
184-
'<:baz |x|></:baz>',
304+
'Invalid block parameters syntax: block parameters must be preceded by the `as` keyword',
305+
'|x|',
185306
'test-module',
186307
1,
187-
7
308+
13
188309
)
189310
);
190311
}

‎packages/@glimmer/syntax/lib/parser.ts

+14-7
Original file line numberDiff line numberDiff line change
@@ -14,16 +14,23 @@ export type ParserNodeBuilder<N extends { loc: src.SourceSpan }> = Omit<N, 'loc'
1414
start: src.SourceOffset;
1515
};
1616

17-
export interface Tag<T extends 'StartTag' | 'EndTag'> {
18-
readonly type: T;
17+
export interface StartTag {
18+
readonly type: 'StartTag';
1919
name: string;
2020
readonly attributes: ASTv1.AttrNode[];
2121
readonly modifiers: ASTv1.ElementModifierStatement[];
2222
readonly comments: ASTv1.MustacheCommentStatement[];
23+
readonly params: ASTv1.VarHead[];
2324
selfClosing: boolean;
2425
readonly loc: src.SourceSpan;
2526
}
2627

28+
export interface EndTag {
29+
readonly type: 'EndTag';
30+
name: string;
31+
readonly loc: src.SourceSpan;
32+
}
33+
2734
export interface Attribute {
2835
name: string;
2936
currentPart: ASTv1.TextNode | null;
@@ -43,8 +50,8 @@ export abstract class Parser {
4350
Readonly<
4451
| ParserNodeBuilder<ASTv1.CommentStatement>
4552
| ParserNodeBuilder<ASTv1.TextNode>
46-
| ParserNodeBuilder<Tag<'StartTag'>>
47-
| ParserNodeBuilder<Tag<'EndTag'>>
53+
| ParserNodeBuilder<StartTag>
54+
| ParserNodeBuilder<EndTag>
4855
>
4956
> = null;
5057
public tokenizer: EventedTokenizer;
@@ -121,19 +128,19 @@ export abstract class Parser {
121128
return expect(this.currentAttribute, 'expected attribute');
122129
}
123130

124-
get currentTag(): ParserNodeBuilder<Tag<'StartTag' | 'EndTag'>> {
131+
get currentTag(): ParserNodeBuilder<StartTag> | ParserNodeBuilder<EndTag> {
125132
let node = this.currentNode;
126133
assert(node && (node.type === 'StartTag' || node.type === 'EndTag'), 'expected tag');
127134
return node;
128135
}
129136

130-
get currentStartTag(): ParserNodeBuilder<Tag<'StartTag'>> {
137+
get currentStartTag(): ParserNodeBuilder<StartTag> {
131138
let node = this.currentNode;
132139
assert(node && node.type === 'StartTag', 'expected start tag');
133140
return node;
134141
}
135142

136-
get currentEndTag(): ParserNodeBuilder<Tag<'EndTag'>> {
143+
get currentEndTag(): ParserNodeBuilder<EndTag> {
137144
let node = this.currentNode;
138145
assert(node && node.type === 'EndTag', 'expected end tag');
139146
return node;

0 commit comments

Comments
 (0)