Skip to content

Commit

Permalink
ast: better handling for empty interpolations (#225)
Browse files Browse the repository at this point in the history
Rather than treat empty interpolations as having no accessors, treat
them as having a single accessor that is the empty string. This is still
invalid--and we issue an error--but allows us to more faithfully
represent the syntactical structure of the document.
  • Loading branch information
pgavlin authored Jan 31, 2024
1 parent 9b88959 commit ba2d384
Show file tree
Hide file tree
Showing 6 changed files with 685 additions and 405 deletions.
9 changes: 9 additions & 0 deletions ast/property.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,11 @@ outer:
switch access[0] {
case '}':
// interpolation terminator

// Handle the case of an empty, terminated access (`${}`)
if len(accessors) == 0 {
accessors = []PropertyAccessor{&PropertyName{Name: ""}}
}
return access[1:], &PropertyAccess{Accessors: accessors}, diags
case '.':
if len(accessors) == 0 {
Expand Down Expand Up @@ -216,6 +221,10 @@ outer:
}
}
}
// Handle the case of an empty, unterminated access (`${`)
if len(accessors) == 0 {
accessors = []PropertyAccessor{&PropertyName{Name: ""}}
}
diags.Extend(syntax.NodeError(node, "unterminated interpolation"))
return access, &PropertyAccess{Accessors: accessors}, diags
}
2 changes: 2 additions & 0 deletions ast/testdata/parse/invalid-interpolations/env.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
values:
interpolations:
- ${
- ${}
- unterminated ${interpolation
- missing ${property.} name
- missing ${property[} subscript
Expand Down
83 changes: 62 additions & 21 deletions ast/testdata/parse/invalid-interpolations/expected.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,24 @@
},
"Value": {
"Elements": [
{
"Property": {
"Accessors": [
{
"Name": ""
}
]
}
},
{
"Property": {
"Accessors": [
{
"Name": ""
}
]
}
},
{
"Parts": [
{
Expand Down Expand Up @@ -114,8 +132,8 @@
},
"End": {
"Line": 3,
"Column": 35,
"Byte": 60
"Column": 9,
"Byte": 34
}
},
"Context": null,
Expand All @@ -126,19 +144,19 @@
},
{
"Severity": 1,
"Summary": "missing closing bracket in list index",
"Summary": "unterminated interpolation",
"Detail": "",
"Subject": {
"Filename": "invalid-interpolations",
"Start": {
"Line": 5,
"Column": 7,
"Byte": 99
"Byte": 51
},
"End": {
"Line": 5,
"Column": 37,
"Byte": 129
"Column": 35,
"Byte": 79
}
},
"Context": null,
Expand All @@ -147,28 +165,51 @@
"Extra": null,
"Path": "values.interpolations[2]"
},
{
"Severity": 1,
"Summary": "missing closing bracket in list index",
"Detail": "",
"Subject": {
"Filename": "invalid-interpolations",
"Start": {
"Line": 7,
"Column": 7,
"Byte": 118
},
"End": {
"Line": 7,
"Column": 37,
"Byte": 148
}
},
"Context": null,
"Expression": null,
"EvalContext": null,
"Extra": null,
"Path": "values.interpolations[4]"
},
{
"Severity": 1,
"Summary": "missing closing quote in property name",
"Detail": "",
"Subject": {
"Filename": "invalid-interpolations",
"Start": {
"Line": 6,
"Line": 8,
"Column": 7,
"Byte": 136
"Byte": 155
},
"End": {
"Line": 6,
"Line": 8,
"Column": 31,
"Byte": 160
"Byte": 179
}
},
"Context": null,
"Expression": null,
"EvalContext": null,
"Extra": null,
"Path": "values.interpolations[3]"
"Path": "values.interpolations[5]"
},
{
"Severity": 1,
Expand All @@ -177,21 +218,21 @@
"Subject": {
"Filename": "invalid-interpolations",
"Start": {
"Line": 6,
"Line": 8,
"Column": 7,
"Byte": 136
"Byte": 155
},
"End": {
"Line": 6,
"Line": 8,
"Column": 31,
"Byte": 160
"Byte": 179
}
},
"Context": null,
"Expression": null,
"EvalContext": null,
"Extra": null,
"Path": "values.interpolations[3]"
"Path": "values.interpolations[5]"
},
{
"Severity": 1,
Expand All @@ -200,21 +241,21 @@
"Subject": {
"Filename": "invalid-interpolations",
"Start": {
"Line": 7,
"Line": 9,
"Column": 7,
"Byte": 167
"Byte": 186
},
"End": {
"Line": 7,
"Line": 9,
"Column": 36,
"Byte": 196
"Byte": 215
}
},
"Context": null,
"Expression": null,
"EvalContext": null,
"Extra": null,
"Path": "values.interpolations[4]"
"Path": "values.interpolations[6]"
}
]
}
12 changes: 0 additions & 12 deletions eval/eval.go
Original file line number Diff line number Diff line change
Expand Up @@ -590,18 +590,6 @@ func (e *evalContext) evaluatePropertyAccess(x *expr, accessors []*propertyAcces
func (e *evalContext) evaluateExprAccess(x *expr, accessors []*propertyAccessor) *value {
receiver := e.root

// This can happen for invalid property accesses. No need to issue an error here--the parser has already done so.
if len(accessors) == 0 {
return &value{
def: &expr{
repr: &literalExpr{node: x.repr.syntax()},
state: exprDone,
},
schema: schema.Always().Schema(),
unknown: true,
}
}

// Check for an imports access.
if k, ok := e.objectKey(x.repr.syntax(), accessors[0].accessor, false); ok && k == "imports" {
accessors[0].value = e.myImports
Expand Down
1 change: 1 addition & 0 deletions eval/testdata/eval/invalid-access-load/env.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
values:
propertyAccessTest:
- ${
- ${open["foo"}
- ${open["foo]}
- ${open["foo"]
Expand Down
Loading

0 comments on commit ba2d384

Please sign in to comment.