Skip to content

Commit 77fb708

Browse files
pgavlinpgavlin
authored
ast: incorporate PR feedback (#230)
- improve property access diagnostics - add a comment
1 parent 482f9dc commit 77fb708

File tree

4 files changed

+33
-54
lines changed

4 files changed

+33
-54
lines changed

‎CHANGELOG_PENDING.md

+3
Original file line numberDiff line numberDiff line change
@@ -6,4 +6,7 @@
66
- Return a properly-merged root schema from environment evaluation.
77
[#229](https://github.com/pulumi/esc/pull/229)
88

9+
- Improve property accessor diagnostics.
10+
[#230](https://github.com/pulumi/esc/pull/230)
11+
912
### Bug Fixes

‎ast/property.go

+11-12
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,8 @@ func (p *propertyAccessParser) errorf(f string, args ...any) {
9393
p.error(fmt.Sprintf(f, args...))
9494
}
9595

96+
// Returns true if the given byte terminates a property name. Also used during error recovery for
97+
// unterminated numeric subscipts.
9698
func (p *propertyAccessParser) terminatesName(c byte) bool {
9799
return c == '.' || c == '[' || c == '}' || unicode.IsSpace(rune(c))
98100
}
@@ -121,7 +123,7 @@ func (p *propertyAccessParser) parse() (string, *PropertyAccess, syntax.Diagnost
121123
for {
122124
c, ok := p.peek()
123125
if !ok {
124-
p.error("unterminated interpolation")
126+
p.error("missing closing brace '}' in interpolation")
125127
return p.finish()
126128
}
127129

@@ -130,17 +132,14 @@ func (p *propertyAccessParser) parse() (string, *PropertyAccess, syntax.Diagnost
130132
p.next()
131133
return p.finish()
132134
case '.':
133-
if len(p.accessors) == 0 {
134-
p.error("the root property must be a string subscript or a name")
135-
}
136135
p.next()
137136
p.append(p.parseName())
138137
case '[':
139138
p.next()
140139
p.append(p.parseSubscript())
141140
default:
142141
if unicode.IsSpace(rune(c)) {
143-
p.error("unterminated interpolation")
142+
p.error("missing closing brace '}' in interpolation")
144143
return p.finish()
145144
}
146145
p.append(p.parseName())
@@ -172,7 +171,7 @@ func (p *propertyAccessParser) parseName() *PropertyName {
172171
b.WriteByte(c)
173172
}
174173
if b.Len() == 0 {
175-
p.errorf("missing property name")
174+
p.errorf("property name must not be empty")
176175
}
177176
return &PropertyName{Name: b.String()}
178177
}
@@ -183,7 +182,7 @@ func (p *propertyAccessParser) parseName() *PropertyName {
183182
func (p *propertyAccessParser) parseSubscript() *PropertySubscript {
184183
c, ok := p.peek()
185184
if !ok {
186-
p.error("missing closing bracket in subscript")
185+
p.error("subscript is missing closing bracket ']'")
187186
return &PropertySubscript{Index: ""}
188187
}
189188

@@ -197,7 +196,7 @@ func (p *propertyAccessParser) parseSubscript() *PropertySubscript {
197196

198197
c, ok = p.peek()
199198
if !ok || c != ']' {
200-
p.error("missing closing bracket in subscript")
199+
p.error("subscript is missing closing bracket ']'")
201200
} else {
202201
p.next()
203202
}
@@ -213,14 +212,14 @@ func (p *propertyAccessParser) parseStringSubscript() string {
213212
for {
214213
c, ok := p.peek()
215214
if !ok {
216-
p.error("missing closing quote in subscript")
215+
p.error("key subscript is missing closing quote '\"'")
217216
return propertyKey.String()
218217
}
219218
p.next()
220219

221220
if c == '"' {
222221
if propertyKey.Len() == 0 {
223-
p.error("property key must not be empty")
222+
p.error("key subscript must not be empty")
224223
}
225224
return propertyKey.String()
226225
}
@@ -254,12 +253,12 @@ func (p *propertyAccessParser) parseIndexSubscript() any {
254253
indexStr := index.String()
255254
num, err := strconv.ParseInt(indexStr, 10, 0)
256255
if err != nil {
257-
p.error("invalid list index")
256+
p.error("numeric subscript must be a positive base-10 integer")
258257
return indexStr
259258
}
260259

261260
if len(p.accessors) == 0 {
262-
p.error("the root property must be a string subscript or a name")
261+
p.error("the first accessor must be a property name or key subscript, not a numeric subscript")
263262
}
264263
return int(num)
265264
}

‎ast/testdata/parse/invalid-interpolations/expected.json

+9-9
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@
127127
"diags": [
128128
{
129129
"Severity": 1,
130-
"Summary": "unterminated interpolation",
130+
"Summary": "missing closing brace '}' in interpolation",
131131
"Detail": "",
132132
"Subject": {
133133
"Filename": "invalid-interpolations",
@@ -150,7 +150,7 @@
150150
},
151151
{
152152
"Severity": 1,
153-
"Summary": "unterminated interpolation",
153+
"Summary": "missing closing brace '}' in interpolation",
154154
"Detail": "",
155155
"Subject": {
156156
"Filename": "invalid-interpolations",
@@ -173,7 +173,7 @@
173173
},
174174
{
175175
"Severity": 1,
176-
"Summary": "missing property name",
176+
"Summary": "property name must not be empty",
177177
"Detail": "",
178178
"Subject": {
179179
"Filename": "invalid-interpolations",
@@ -196,7 +196,7 @@
196196
},
197197
{
198198
"Severity": 1,
199-
"Summary": "invalid list index",
199+
"Summary": "numeric subscript must be a positive base-10 integer",
200200
"Detail": "",
201201
"Subject": {
202202
"Filename": "invalid-interpolations",
@@ -219,7 +219,7 @@
219219
},
220220
{
221221
"Severity": 1,
222-
"Summary": "missing closing bracket in subscript",
222+
"Summary": "subscript is missing closing bracket ']'",
223223
"Detail": "",
224224
"Subject": {
225225
"Filename": "invalid-interpolations",
@@ -242,7 +242,7 @@
242242
},
243243
{
244244
"Severity": 1,
245-
"Summary": "missing closing quote in subscript",
245+
"Summary": "key subscript is missing closing quote '\"'",
246246
"Detail": "",
247247
"Subject": {
248248
"Filename": "invalid-interpolations",
@@ -265,7 +265,7 @@
265265
},
266266
{
267267
"Severity": 1,
268-
"Summary": "missing closing bracket in subscript",
268+
"Summary": "subscript is missing closing bracket ']'",
269269
"Detail": "",
270270
"Subject": {
271271
"Filename": "invalid-interpolations",
@@ -288,7 +288,7 @@
288288
},
289289
{
290290
"Severity": 1,
291-
"Summary": "unterminated interpolation",
291+
"Summary": "missing closing brace '}' in interpolation",
292292
"Detail": "",
293293
"Subject": {
294294
"Filename": "invalid-interpolations",
@@ -311,7 +311,7 @@
311311
},
312312
{
313313
"Severity": 1,
314-
"Summary": "invalid list index",
314+
"Summary": "numeric subscript must be a positive base-10 integer",
315315
"Detail": "",
316316
"Subject": {
317317
"Filename": "invalid-interpolations",

‎eval/testdata/eval/invalid-access-load/expected.json

+10-33
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
"loadDiags": [
33
{
44
"Severity": 1,
5-
"Summary": "unterminated interpolation",
5+
"Summary": "missing closing brace '}' in interpolation",
66
"Detail": "",
77
"Subject": {
88
"Filename": "invalid-access-load",
@@ -25,7 +25,7 @@
2525
},
2626
{
2727
"Severity": 1,
28-
"Summary": "missing closing bracket in subscript",
28+
"Summary": "subscript is missing closing bracket ']'",
2929
"Detail": "",
3030
"Subject": {
3131
"Filename": "invalid-access-load",
@@ -48,7 +48,7 @@
4848
},
4949
{
5050
"Severity": 1,
51-
"Summary": "missing closing quote in subscript",
51+
"Summary": "key subscript is missing closing quote '\"'",
5252
"Detail": "",
5353
"Subject": {
5454
"Filename": "invalid-access-load",
@@ -71,7 +71,7 @@
7171
},
7272
{
7373
"Severity": 1,
74-
"Summary": "missing closing bracket in subscript",
74+
"Summary": "subscript is missing closing bracket ']'",
7575
"Detail": "",
7676
"Subject": {
7777
"Filename": "invalid-access-load",
@@ -94,7 +94,7 @@
9494
},
9595
{
9696
"Severity": 1,
97-
"Summary": "unterminated interpolation",
97+
"Summary": "missing closing brace '}' in interpolation",
9898
"Detail": "",
9999
"Subject": {
100100
"Filename": "invalid-access-load",
@@ -117,7 +117,7 @@
117117
},
118118
{
119119
"Severity": 1,
120-
"Summary": "unterminated interpolation",
120+
"Summary": "missing closing brace '}' in interpolation",
121121
"Detail": "",
122122
"Subject": {
123123
"Filename": "invalid-access-load",
@@ -140,7 +140,7 @@
140140
},
141141
{
142142
"Severity": 1,
143-
"Summary": "missing closing bracket in subscript",
143+
"Summary": "subscript is missing closing bracket ']'",
144144
"Detail": "",
145145
"Subject": {
146146
"Filename": "invalid-access-load",
@@ -163,7 +163,7 @@
163163
},
164164
{
165165
"Severity": 1,
166-
"Summary": "invalid list index",
166+
"Summary": "numeric subscript must be a positive base-10 integer",
167167
"Detail": "",
168168
"Subject": {
169169
"Filename": "invalid-access-load",
@@ -186,7 +186,7 @@
186186
},
187187
{
188188
"Severity": 1,
189-
"Summary": "the root property must be a string subscript or a name",
189+
"Summary": "the first accessor must be a property name or key subscript, not a numeric subscript",
190190
"Detail": "",
191191
"Subject": {
192192
"Filename": "invalid-access-load",
@@ -209,7 +209,7 @@
209209
},
210210
{
211211
"Severity": 1,
212-
"Summary": "missing property name",
212+
"Summary": "property name must not be empty",
213213
"Detail": "",
214214
"Subject": {
215215
"Filename": "invalid-access-load",
@@ -229,29 +229,6 @@
229229
"EvalContext": null,
230230
"Extra": null,
231231
"Path": "values.propertyAccessTest[8]"
232-
},
233-
{
234-
"Severity": 1,
235-
"Summary": "the root property must be a string subscript or a name",
236-
"Detail": "",
237-
"Subject": {
238-
"Filename": "invalid-access-load",
239-
"Start": {
240-
"Line": 13,
241-
"Column": 7,
242-
"Byte": 207
243-
},
244-
"End": {
245-
"Line": 13,
246-
"Column": 14,
247-
"Byte": 214
248-
}
249-
},
250-
"Context": null,
251-
"Expression": null,
252-
"EvalContext": null,
253-
"Extra": null,
254-
"Path": "values.propertyAccessTest[10]"
255232
}
256233
],
257234
"checkDiags": [

0 commit comments

Comments
 (0)