From 1cbb0d41b1499559bd3dddc37d6873c28f179952 Mon Sep 17 00:00:00 2001 From: Ansgar Mertens Date: Tue, 12 Mar 2024 09:36:49 +0100 Subject: [PATCH 1/4] feat: return ExprSyntaxError instead of nil when expression parsing fails for namespaced functions --- hclsyntax/expression.go | 23 +++++++ hclsyntax/expression_test.go | 8 +-- hclsyntax/expression_vars.go | 6 +- hclsyntax/expression_vars_gen.go | 2 +- hclsyntax/parser.go | 21 ++++-- hclsyntax/parser_test.go | 106 +++++++++++++++++++++++++++++++ 6 files changed, 154 insertions(+), 12 deletions(-) diff --git a/hclsyntax/expression.go b/hclsyntax/expression.go index c4a353c4..63bd24c1 100644 --- a/hclsyntax/expression.go +++ b/hclsyntax/expression.go @@ -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{} +} + +func (e *ExprSyntaxError) StartRange() hcl.Range { + return hcl.Range{} +} diff --git a/hclsyntax/expression_test.go b/hclsyntax/expression_test.go index 4f3f75f7..b18214cd 100644 --- a/hclsyntax/expression_test.go +++ b/hclsyntax/expression_test.go @@ -379,8 +379,8 @@ upper( "double::::upper": stdlib.UpperFunc, }, }, - cty.NilVal, - 1, + cty.DynamicVal, + 2, }, { `missing::("foo")`, // missing name after :: @@ -389,8 +389,8 @@ upper( "missing::": stdlib.UpperFunc, }, }, - cty.NilVal, - 1, + cty.DynamicVal, + 2, }, { `misbehave()`, diff --git a/hclsyntax/expression_vars.go b/hclsyntax/expression_vars.go index ce5a5cb7..6c3e472c 100755 --- a/hclsyntax/expression_vars.go +++ b/hclsyntax/expression_vars.go @@ -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 ( @@ -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) } diff --git a/hclsyntax/expression_vars_gen.go b/hclsyntax/expression_vars_gen.go index efbc2d82..d0888025 100644 --- a/hclsyntax/expression_vars_gen.go +++ b/hclsyntax/expression_vars_gen.go @@ -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 ( diff --git a/hclsyntax/parser.go b/hclsyntax/parser.go index cd9d63d2..e9e84065 100644 --- a/hclsyntax/parser.go +++ b/hclsyntax/parser.go @@ -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}, + Placeholder: cty.DynamicVal, + }, diags } // Initial versions of HCLv2 didn't support function namespaces, and @@ -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 diff --git a/hclsyntax/parser_test.go b/hclsyntax/parser_test.go index 6e226a60..2231fdba 100644 --- a/hclsyntax/parser_test.go +++ b/hclsyntax/parser_test.go @@ -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 { From 53ee54e0da34370e928b8cf6d72f8d25f24e6ee4 Mon Sep 17 00:00:00 2001 From: Ansgar Mertens Date: Tue, 12 Mar 2024 09:37:20 +0100 Subject: [PATCH 2/4] chore: add test from #665 --- hclsyntax/expression_template_test.go | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/hclsyntax/expression_template_test.go b/hclsyntax/expression_template_test.go index c78094f5..aa33c9d9 100644 --- a/hclsyntax/expression_template_test.go +++ b/hclsyntax/expression_template_test.go @@ -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 From 54e4175c12b085892d61aa506b9562a591fde6a7 Mon Sep 17 00:00:00 2001 From: Ansgar Mertens Date: Wed, 13 Mar 2024 09:17:03 +0100 Subject: [PATCH 3/4] add SrcRange to ExprSyntaxError --- hclsyntax/expression.go | 5 +++-- hclsyntax/parser.go | 2 ++ hclsyntax/parser_test.go | 8 ++++++++ 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/hclsyntax/expression.go b/hclsyntax/expression.go index 63bd24c1..81597399 100644 --- a/hclsyntax/expression.go +++ b/hclsyntax/expression.go @@ -2019,6 +2019,7 @@ func (e *AnonSymbolExpr) StartRange() hcl.Range { type ExprSyntaxError struct { Placeholder cty.Value ParseDiags hcl.Diagnostics + SrcRange hcl.Range } func (e *ExprSyntaxError) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnostics) { @@ -2030,9 +2031,9 @@ func (e *ExprSyntaxError) walkChildNodes(w internalWalkFunc) { } func (e *ExprSyntaxError) Range() hcl.Range { - return hcl.Range{} + return e.SrcRange } func (e *ExprSyntaxError) StartRange() hcl.Range { - return hcl.Range{} + return e.SrcRange } diff --git a/hclsyntax/parser.go b/hclsyntax/parser.go index e9e84065..ce96ae35 100644 --- a/hclsyntax/parser.go +++ b/hclsyntax/parser.go @@ -1173,6 +1173,7 @@ func (p *parser) finishParsingFunctionCall(name Token) (Expression, hcl.Diagnost return &ExprSyntaxError{ ParseDiags: hcl.Diagnostics{&diag}, Placeholder: cty.DynamicVal, + SrcRange: hcl.RangeBetween(name.Range, nextName.Range), }, diags } @@ -1209,6 +1210,7 @@ func (p *parser) finishParsingFunctionCall(name Token) (Expression, hcl.Diagnost return &ExprSyntaxError{ ParseDiags: hcl.Diagnostics{&diag}, Placeholder: cty.DynamicVal, + SrcRange: hcl.RangeBetween(name.Range, openTok.Range), }, diags } diff --git a/hclsyntax/parser_test.go b/hclsyntax/parser_test.go index 2231fdba..90a75832 100644 --- a/hclsyntax/parser_test.go +++ b/hclsyntax/parser_test.go @@ -2583,6 +2583,10 @@ block "valid" {} }, }, }, + SrcRange: hcl.Range{ + Start: hcl.Pos{Line: 1, Column: 5, Byte: 4}, + End: hcl.Pos{Line: 2, Column: 1, Byte: 24}, + }, }, SrcRange: hcl.Range{ Filename: "", @@ -2636,6 +2640,10 @@ block "valid" {} }, }, }, + SrcRange: hcl.Range{ + Start: hcl.Pos{Line: 1, Column: 5, Byte: 4}, + End: hcl.Pos{Line: 2, Column: 1, Byte: 14}, + }, }, SrcRange: hcl.Range{ Filename: "", From cc3af98c59dce4f86374165653b0fb2b738a45d3 Mon Sep 17 00:00:00 2001 From: Ansgar Mertens Date: Wed, 13 Mar 2024 11:45:03 +0100 Subject: [PATCH 4/4] fix test error message if wrong type --- hclsyntax/expression_template_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hclsyntax/expression_template_test.go b/hclsyntax/expression_template_test.go index aa33c9d9..31198458 100644 --- a/hclsyntax/expression_template_test.go +++ b/hclsyntax/expression_template_test.go @@ -456,7 +456,7 @@ func TestTemplateExprWrappedGracefulValue(t *testing.T) { got, _ := expr.Value(nil) // this should not panic if !got.RawEquals(cty.DynamicVal) { - t.Errorf("wrong result\ngot: %#v\nwant: %#v", got, cty.NilVal) + t.Errorf("wrong result\ngot: %#v\nwant: %#v", got, cty.DynamicVal) } }