Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

add ExprSyntaxError #668

Merged
merged 4 commits into from
Mar 13, 2024
Merged

add ExprSyntaxError #668

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1cbb0d4
feat: return ExprSyntaxError instead of nil when expression parsing f…
ansgarm Mar 12, 2024
53ee54e
chore: add test from #665
ansgarm Mar 12, 2024
54e4175
add SrcRange to ExprSyntaxError
ansgarm Mar 13, 2024
cc3af98
fix test error message if wrong type
ansgarm Mar 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 23 additions & 0 deletions hclsyntax/expression.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2013,3 +2013,26 @@ func (e *AnonSymbolExpr) Range() hcl.Range {
func (e *AnonSymbolExpr) StartRange() hcl.Range {
return e.SrcRange
}

// ExprSyntaxError is a placeholder for an invalid expression that could not
// be parsed due to syntax errors.
type ExprSyntaxError struct {
Placeholder cty.Value
ParseDiags hcl.Diagnostics
}

func (e *ExprSyntaxError) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnostics) {
return e.Placeholder, e.ParseDiags
}

func (e *ExprSyntaxError) walkChildNodes(w internalWalkFunc) {
// ExprSyntaxError is a leaf node in the tree
}

func (e *ExprSyntaxError) Range() hcl.Range {
return hcl.Range{}
Copy link
Member Author

@ansgarm ansgarm Mar 12, 2024 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a valid Range we could return here? Maybe from the diagnostic?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the simplest approach would be to add SourceRange as a field in ExprSyntaxError and then have the parser decide what range to "blame" for the error.

As you noted, the parser already ought to be picking source ranges to go into the diagnostics anyway, and so I think it shouldn't be a big hardship to choose one of those ranges to also write into that field, and then we can keep it explicit and thus hopefully the ranges are more likely to be useful.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good! Added SrcRange to that struct and set the same range as in the diagnostic 👍

}

func (e *ExprSyntaxError) StartRange() hcl.Range {
return hcl.Range{}
}
22 changes: 22 additions & 0 deletions hclsyntax/expression_template_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -438,6 +438,28 @@ trim`,

}

func TestTemplateExprGracefulValue(t *testing.T) {
// we don't care about diags since we know it's invalid config
expr, _ := ParseTemplate([]byte(`prefix${provider::}`), "", hcl.Pos{Line: 1, Column: 1, Byte: 0})

got, _ := expr.Value(nil) // this should not panic

if !got.RawEquals(cty.UnknownVal(cty.String).RefineNotNull()) {
t.Errorf("wrong result\ngot: %#v\nwant: %#v", got, cty.UnknownVal(cty.String).RefineNotNull())
}
}

func TestTemplateExprWrappedGracefulValue(t *testing.T) {
// we don't care about diags since we know it's invalid config
expr, _ := ParseTemplate([]byte(`${provider::}`), "", hcl.Pos{Line: 1, Column: 1, Byte: 0})

got, _ := expr.Value(nil) // this should not panic

if !got.RawEquals(cty.DynamicVal) {
t.Errorf("wrong result\ngot: %#v\nwant: %#v", got, cty.NilVal)
}
}

func TestTemplateExprIsStringLiteral(t *testing.T) {
tests := map[string]bool{
// A simple string value is a string literal
Expand Down
8 changes: 4 additions & 4 deletions hclsyntax/expression_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -379,8 +379,8 @@ upper(
"double::::upper": stdlib.UpperFunc,
},
},
cty.NilVal,
1,
cty.DynamicVal,
2,
},
{
`missing::("foo")`, // missing name after ::
Expand All @@ -389,8 +389,8 @@ upper(
"missing::": stdlib.UpperFunc,
},
},
cty.NilVal,
1,
cty.DynamicVal,
2,
},
{
`misbehave()`,
Expand Down
6 changes: 5 additions & 1 deletion hclsyntax/expression_vars.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

package hclsyntax

// Generated by expression_vars_get.go. DO NOT EDIT.
// Generated by expression_vars_gen.go. DO NOT EDIT.
// Run 'go generate' on this package to update the set of functions here.

import (
Expand All @@ -22,6 +22,10 @@ func (e *ConditionalExpr) Variables() []hcl.Traversal {
return Variables(e)
}

func (e *ExprSyntaxError) Variables() []hcl.Traversal {
return Variables(e)
}

func (e *ForExpr) Variables() []hcl.Traversal {
return Variables(e)
}
Expand Down
2 changes: 1 addition & 1 deletion hclsyntax/expression_vars_gen.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ const outputPreamble = `// Copyright (c) HashiCorp, Inc.

package hclsyntax

// Generated by expression_vars_get.go. DO NOT EDIT.
// Generated by expression_vars_gen.go. DO NOT EDIT.
// Run 'go generate' on this package to update the set of functions here.

import (
Expand Down
21 changes: 15 additions & 6 deletions hclsyntax/parser.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1161,15 +1161,19 @@ func (p *parser) finishParsingFunctionCall(name Token) (Expression, hcl.Diagnost
for openTok.Type == TokenDoubleColon {
nextName := p.Read()
if nextName.Type != TokenIdent {
diags = append(diags, &hcl.Diagnostic{
diag := hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Missing function name",
Detail: "Function scope resolution symbol :: must be followed by a function name in this scope.",
Subject: &nextName.Range,
Context: hcl.RangeBetween(name.Range, nextName.Range).Ptr(),
})
}
diags = append(diags, &diag)
p.recoverOver(TokenOParen)
return nil, diags
return &ExprSyntaxError{
ParseDiags: hcl.Diagnostics{&diag},
ansgarm marked this conversation as resolved.
Show resolved Hide resolved
Placeholder: cty.DynamicVal,
ansgarm marked this conversation as resolved.
Show resolved Hide resolved
}, diags
}

// Initial versions of HCLv2 didn't support function namespaces, and
Expand All @@ -1192,15 +1196,20 @@ func (p *parser) finishParsingFunctionCall(name Token) (Expression, hcl.Diagnost
}

if openTok.Type != TokenOParen {
diags = append(diags, &hcl.Diagnostic{
diag := hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Missing open parenthesis",
Detail: "Function selector must be followed by an open parenthesis to begin the function call.",
Subject: &openTok.Range,
Context: hcl.RangeBetween(name.Range, openTok.Range).Ptr(),
})
}

diags = append(diags, &diag)
p.recoverOver(TokenOParen)
return nil, diags
return &ExprSyntaxError{
ParseDiags: hcl.Diagnostics{&diag},
Placeholder: cty.DynamicVal,
}, diags
}

var args []Expression
Expand Down
106 changes: 106 additions & 0 deletions hclsyntax/parser_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2559,6 +2559,112 @@ block "valid" {}
},
},
},
{
"a = partial::namespaced\n",
1,
&Body{
Attributes: Attributes{
"a": {
Name: "a",
Expr: &ExprSyntaxError{
Placeholder: cty.DynamicVal,
ParseDiags: hcl.Diagnostics{
{
Severity: hcl.DiagError,
Summary: "Missing open parenthesis",
Detail: "Function selector must be followed by an open parenthesis to begin the function call.",
Subject: &hcl.Range{
Start: hcl.Pos{Line: 1, Column: 24, Byte: 23},
End: hcl.Pos{Line: 2, Column: 1, Byte: 24},
},
Context: &hcl.Range{
Start: hcl.Pos{Line: 1, Column: 5, Byte: 4},
End: hcl.Pos{Line: 2, Column: 1, Byte: 24},
},
},
},
},
SrcRange: hcl.Range{
Filename: "",
Start: hcl.Pos{Line: 1, Column: 1, Byte: 0},
End: hcl.Pos{Line: 2, Column: 1, Byte: 24},
},
NameRange: hcl.Range{
Filename: "",
Start: hcl.Pos{Line: 1, Column: 1, Byte: 0},
End: hcl.Pos{Line: 1, Column: 2, Byte: 1},
},
EqualsRange: hcl.Range{
Filename: "",
Start: hcl.Pos{Line: 1, Column: 3, Byte: 2},
End: hcl.Pos{Line: 1, Column: 4, Byte: 3},
},
},
},
Blocks: Blocks{},
SrcRange: hcl.Range{
Start: hcl.Pos{Line: 1, Column: 1, Byte: 0},
End: hcl.Pos{Line: 2, Column: 1, Byte: 24},
},
EndRange: hcl.Range{
Start: hcl.Pos{Line: 2, Column: 1, Byte: 24},
End: hcl.Pos{Line: 2, Column: 1, Byte: 24},
},
},
},
{
"a = partial::\n",
1,
&Body{
Attributes: Attributes{
"a": {
Name: "a",
Expr: &ExprSyntaxError{
Placeholder: cty.DynamicVal,
ParseDiags: hcl.Diagnostics{
{
Severity: hcl.DiagError,
Summary: "Missing function name",
Detail: "Function scope resolution symbol :: must be followed by a function name in this scope.",
Subject: &hcl.Range{
Start: hcl.Pos{Line: 1, Column: 14, Byte: 13},
End: hcl.Pos{Line: 2, Column: 1, Byte: 14},
},
Context: &hcl.Range{
Start: hcl.Pos{Line: 1, Column: 5, Byte: 4},
End: hcl.Pos{Line: 2, Column: 1, Byte: 14},
},
},
},
},
SrcRange: hcl.Range{
Filename: "",
Start: hcl.Pos{Line: 1, Column: 1, Byte: 0},
End: hcl.Pos{Line: 2, Column: 1, Byte: 14},
},
NameRange: hcl.Range{
Filename: "",
Start: hcl.Pos{Line: 1, Column: 1, Byte: 0},
End: hcl.Pos{Line: 1, Column: 2, Byte: 1},
},
EqualsRange: hcl.Range{
Filename: "",
Start: hcl.Pos{Line: 1, Column: 3, Byte: 2},
End: hcl.Pos{Line: 1, Column: 4, Byte: 3},
},
},
},
Blocks: Blocks{},
SrcRange: hcl.Range{
Start: hcl.Pos{Line: 1, Column: 1, Byte: 0},
End: hcl.Pos{Line: 2, Column: 1, Byte: 14},
},
EndRange: hcl.Range{
Start: hcl.Pos{Line: 2, Column: 1, Byte: 14},
End: hcl.Pos{Line: 2, Column: 1, Byte: 14},
},
},
},
}

for _, test := range tests {
Expand Down