From 1cbb0d41b1499559bd3dddc37d6873c28f179952 Mon Sep 17 00:00:00 2001 From: Ansgar Mertens Date: Tue, 12 Mar 2024 09:36:49 +0100 Subject: [PATCH 01/23] 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 02/23] 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 03/23] 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 04/23] 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) } } From 2a0a3f049ccff74b1d45315d5ba3fa09713e6929 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Tue, 12 Mar 2024 15:24:34 -0700 Subject: [PATCH 05/23] Standardize on only two value dumping/diffing libraries Due to the quite messy heritage of this codebase -- including a large part of it being just a fork of my earlier personal project ZCL -- there were many different conventions for how to pretty-print and diff values in the tests in different parts of the codebase. To reduce the dependency sprawl, this commit now standardizes on: - github.com/davecgh/go-spew for pretty-printing - github.com/google/go-cmp for diffing These two dependencies were already present anyway, are the most general out of all of the candidates, and are also already in use by at least some of HCL's most significant callers, such as HashiCorp Terraform. The version of go-cmp we were previously using seems to have a bug that causes the tests to crash when run under the Go race detector, so I've also upgraded that dependency to latest here to clear that bug. --- go.mod | 9 +-------- go.sum | 14 ++------------ hcldec/spec_test.go | 12 ++++++++++-- hclsyntax/parser_test.go | 37 +++++++++++++++++++++++-------------- hclsyntax/structure_test.go | 30 ++++++++++++------------------ hclsyntax/variables_test.go | 7 +++++-- hclwrite/parser_test.go | 33 ++++++--------------------------- hclwrite/round_trip_test.go | 8 ++------ 8 files changed, 61 insertions(+), 89 deletions(-) diff --git a/go.mod b/go.mod index a8005025..d9d4da45 100644 --- a/go.mod +++ b/go.mod @@ -4,15 +4,11 @@ go 1.18 require ( github.com/agext/levenshtein v1.2.1 - github.com/apparentlymart/go-dump v0.0.0-20180507223929-23540a00eaa3 github.com/apparentlymart/go-textseg/v15 v15.0.0 github.com/davecgh/go-spew v1.1.1 github.com/go-test/deep v1.0.3 - github.com/google/go-cmp v0.3.1 - github.com/kr/pretty v0.1.0 - github.com/kylelemons/godebug v0.0.0-20170820004349-d65d576e9348 + github.com/google/go-cmp v0.6.0 github.com/mitchellh/go-wordwrap v0.0.0-20150314170334-ad45545899c7 - github.com/sergi/go-diff v1.0.0 github.com/spf13/pflag v1.0.2 github.com/zclconf/go-cty v1.13.0 github.com/zclconf/go-cty-debug v0.0.0-20191215020915-b22d67c1ba0b @@ -22,9 +18,6 @@ require ( require ( github.com/apparentlymart/go-textseg/v13 v13.0.0 // indirect - github.com/kr/text v0.1.0 // indirect - github.com/pmezard/go-difflib v1.0.0 // indirect - github.com/stretchr/testify v1.2.2 // indirect golang.org/x/mod v0.8.0 // indirect golang.org/x/sys v0.5.0 // indirect golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1 // indirect diff --git a/go.sum b/go.sum index 62014eb6..08ed23ed 100644 --- a/go.sum +++ b/go.sum @@ -1,7 +1,5 @@ github.com/agext/levenshtein v1.2.1 h1:QmvMAjj2aEICytGiWzmxoE0x2KZvE0fvmqMOfy2tjT8= github.com/agext/levenshtein v1.2.1/go.mod h1:JEDfjyjHDjOF/1e4FlBE/PkbqA9OfWu2ki2W0IB5558= -github.com/apparentlymart/go-dump v0.0.0-20180507223929-23540a00eaa3 h1:ZSTrOEhiM5J5RFxEaFvMZVEAM1KvT1YzbEOwB2EAGjA= -github.com/apparentlymart/go-dump v0.0.0-20180507223929-23540a00eaa3/go.mod h1:oL81AME2rN47vu18xqj1S1jPIPuN7afo62yKTNn3XMM= github.com/apparentlymart/go-textseg v1.0.0/go.mod h1:z96Txxhf3xSFMPmb5X/1W05FF/Nj9VFpLOpjS5yuumk= github.com/apparentlymart/go-textseg/v13 v13.0.0 h1:Y+KvPE1NYz0xl601PVImeQfFyEy6iT90AvPUL1NNfNw= github.com/apparentlymart/go-textseg/v13 v13.0.0/go.mod h1:ZK2fH7c4NqDTLtiYLvIkEghdlcqw7yxLeM89kiTRPUo= @@ -12,25 +10,17 @@ github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSs github.com/go-test/deep v1.0.3 h1:ZrJSEWsXzPOxaZnFteGEfooLba+ju3FYIbOrS+rQd68= github.com/go-test/deep v1.0.3/go.mod h1:wGDj63lr65AM2AQyKZd/NYHGb0R+1RLqB8NKt3aSFNA= github.com/golang/protobuf v1.1.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= -github.com/google/go-cmp v0.3.1 h1:Xye71clBPdm5HgqGwUkwhbynsUJZhDbS20FvLhQ2izg= github.com/google/go-cmp v0.3.1/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU= -github.com/kr/pretty v0.1.0 h1:L/CwN0zerZDmRFUapSPitk6f+Q3+0za1rQkzVuMiMFI= +github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= +github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= -github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= -github.com/kylelemons/godebug v0.0.0-20170820004349-d65d576e9348 h1:MtvEpTB6LX3vkb4ax0b5D2DHbNAUsen0Gx5wZoq3lV4= github.com/kylelemons/godebug v0.0.0-20170820004349-d65d576e9348/go.mod h1:B69LEHPfb2qLo0BaaOLcbitczOKLWTsrBG9LczfCD4k= github.com/mitchellh/go-wordwrap v0.0.0-20150314170334-ad45545899c7 h1:DpOJ2HYzCv8LZP15IdmG+YdwD2luVPHITV96TkirNBM= github.com/mitchellh/go-wordwrap v0.0.0-20150314170334-ad45545899c7/go.mod h1:ZXFpozHsX6DPmq2I0TCekCxypsnAUbP2oI0UX1GXzOo= -github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= -github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= -github.com/sergi/go-diff v1.0.0 h1:Kpca3qRNrduNnOQeazBd0ysaKrUJiIuISHxogkT9RPQ= -github.com/sergi/go-diff v1.0.0/go.mod h1:0CfEIISq7TuYL3j771MWULgwwjU+GofnZX9QAmXWZgo= github.com/spf13/pflag v1.0.2 h1:Fy0orTDgHdbnzHcsOgfCN4LtHf0ec3wwtiwJqwvf3Gc= github.com/spf13/pflag v1.0.2/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnInEg4= -github.com/stretchr/testify v1.2.2 h1:bSDNvY7ZPG5RlJ8otE/7V6gMiyenm9RtJ7IUVIAoJ1w= -github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= github.com/vmihailenco/msgpack v3.3.3+incompatible/go.mod h1:fy3FlTQTDXWkZ7Bh6AcGMlsjHatGryHQYUTf1ShIgkk= github.com/zclconf/go-cty v1.2.0/go.mod h1:hOPWgoHbaTUnI5k4D2ld+GRpFJSCe6bCM7m1q/N4PQ8= github.com/zclconf/go-cty v1.13.0 h1:It5dfKTTZHe9aeppbNOda3mN7Ag7sg6QkBNm6TkyFa0= diff --git a/hcldec/spec_test.go b/hcldec/spec_test.go index 1b0594d1..a88066b5 100644 --- a/hcldec/spec_test.go +++ b/hcldec/spec_test.go @@ -8,7 +8,6 @@ import ( "reflect" "testing" - "github.com/apparentlymart/go-dump/dump" "github.com/google/go-cmp/cmp" "github.com/zclconf/go-cty-debug/ctydebug" "github.com/zclconf/go-cty/cty" @@ -98,7 +97,16 @@ bar = barval }, } if !reflect.DeepEqual(gotVars, wantVars) { - t.Errorf("wrong Variables result\ngot: %s\nwant: %s", dump.Value(gotVars), dump.Value(wantVars)) + t.Errorf( + "wrong Variables result\n%s", + cmp.Diff( + wantVars, gotVars, + cmp.AllowUnexported( + hcl.TraverseRoot{}, + ), + ctydebug.CmpOptions, + ), + ) } ctx := &hcl.EvalContext{ diff --git a/hclsyntax/parser_test.go b/hclsyntax/parser_test.go index 90a75832..10825f15 100644 --- a/hclsyntax/parser_test.go +++ b/hclsyntax/parser_test.go @@ -5,19 +5,16 @@ package hclsyntax import ( "fmt" + "sync" "testing" - "github.com/go-test/deep" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" "github.com/hashicorp/hcl/v2" + "github.com/zclconf/go-cty-debug/ctydebug" "github.com/zclconf/go-cty/cty" ) -func init() { - deep.MaxDepth = 999 -} - func TestParseConfig(t *testing.T) { tests := []struct { input string @@ -2197,6 +2194,7 @@ block "valid" {} "a": { Name: "a", Expr: &LiteralValueExpr{ + Val: cty.DynamicVal, SrcRange: hcl.Range{ Start: hcl.Pos{Line: 1, Column: 5, Byte: 4}, End: hcl.Pos{Line: 1, Column: 6, Byte: 5}, @@ -2235,6 +2233,7 @@ block "valid" {} "a": { Name: "a", Expr: &LiteralValueExpr{ + Val: cty.DynamicVal, SrcRange: hcl.Range{ Start: hcl.Pos{Line: 1, Column: 5, Byte: 4}, End: hcl.Pos{Line: 1, Column: 6, Byte: 5}, @@ -2687,11 +2686,23 @@ block "valid" {} } got := file.Body - - if diff := deep.Equal(got, test.want); diff != nil { - for _, problem := range diff { - t.Errorf(problem) - } + diff := cmp.Diff( + test.want, got, + cmp.AllowUnexported( + Body{}, + AnonSymbolExpr{}, + hcl.TraverseRoot{}, + hcl.TraverseAttr{}, + hcl.TraverseIndex{}, + ), + cmpopts.IgnoreUnexported( + sync.Mutex{}, + sync.RWMutex{}, + ), + ctydebug.CmpOptions, + ) + if diff != "" { + t.Errorf("wrong result\n%s", diff) } }) } @@ -4006,10 +4017,8 @@ func TestParseConfigDiagnostics(t *testing.T) { t.Logf("\n%s", test.input) _, diags := ParseConfig([]byte(test.input), "test.hcl", hcl.InitialPos) - if diff := deep.Equal(diags, test.want); diff != nil { - for _, problem := range diff { - t.Errorf(problem) - } + if diff := cmp.Diff(test.want, diags); diff != "" { + t.Errorf("wrong diagnostics\n%s", diff) } }) } diff --git a/hclsyntax/structure_test.go b/hclsyntax/structure_test.go index c86dc328..180425bf 100644 --- a/hclsyntax/structure_test.go +++ b/hclsyntax/structure_test.go @@ -8,8 +8,10 @@ import ( "reflect" "testing" + "github.com/davecgh/go-spew/spew" + "github.com/google/go-cmp/cmp" "github.com/hashicorp/hcl/v2" - "github.com/kylelemons/godebug/pretty" + "github.com/zclconf/go-cty-debug/ctydebug" "github.com/zclconf/go-cty/cty" ) @@ -390,12 +392,6 @@ func TestBodyContent(t *testing.T) { }, } - prettyConfig := &pretty.Config{ - Diffable: true, - IncludeUnexported: true, - PrintStringers: true, - } - for i, test := range tests { t.Run(fmt.Sprintf("%02d", i), func(t *testing.T) { var got *hcl.BodyContent @@ -416,7 +412,7 @@ func TestBodyContent(t *testing.T) { if !reflect.DeepEqual(got, test.want) { t.Errorf( "wrong result\ndiff: %s", - prettyConfig.Compare(test.want, got), + cmp.Diff(test.want, got), ) } }) @@ -499,7 +495,7 @@ func TestBodyJustAttributes(t *testing.T) { }, }, hiddenAttrs: map[string]struct{}{ - "foo": struct{}{}, + "foo": {}, }, }, hcl.Attributes{}, @@ -507,12 +503,6 @@ func TestBodyJustAttributes(t *testing.T) { }, } - prettyConfig := &pretty.Config{ - Diffable: true, - IncludeUnexported: true, - PrintStringers: true, - } - for i, test := range tests { t.Run(fmt.Sprintf("%02d", i), func(t *testing.T) { got, diags := test.body.JustAttributes() @@ -524,11 +514,15 @@ func TestBodyJustAttributes(t *testing.T) { } } - if !reflect.DeepEqual(got, test.want) { + diff := cmp.Diff( + test.want, got, + ctydebug.CmpOptions, + ) + if diff != "" { t.Errorf( "wrong result\nbody: %s\ndiff: %s", - prettyConfig.Sprint(test.body), - prettyConfig.Compare(test.want, got), + spew.Sdump(test.body), + diff, ) } }) diff --git a/hclsyntax/variables_test.go b/hclsyntax/variables_test.go index c841d94a..9eb3d411 100644 --- a/hclsyntax/variables_test.go +++ b/hclsyntax/variables_test.go @@ -8,8 +8,8 @@ import ( "reflect" "testing" + "github.com/davecgh/go-spew/spew" "github.com/hashicorp/hcl/v2" - "github.com/kr/pretty" "github.com/zclconf/go-cty/cty" ) @@ -300,7 +300,10 @@ func TestVariables(t *testing.T) { got := Variables(test.Expr) if !reflect.DeepEqual(got, test.Want) { - t.Errorf("wrong result\ngot: %s\nwant: %s", pretty.Sprint(got), pretty.Sprint(test.Want)) + t.Errorf( + "wrong result\ngot: %s\nwant: %s", + spew.Sdump(got), spew.Sdump(test.Want), + ) } }) } diff --git a/hclwrite/parser_test.go b/hclwrite/parser_test.go index e557940e..8ccae993 100644 --- a/hclwrite/parser_test.go +++ b/hclwrite/parser_test.go @@ -5,15 +5,11 @@ package hclwrite import ( "fmt" - "reflect" "testing" "github.com/davecgh/go-spew/spew" - "github.com/google/go-cmp/cmp" - "github.com/kylelemons/godebug/pretty" - "github.com/hashicorp/hcl/v2" "github.com/hashicorp/hcl/v2/hclsyntax" ) @@ -1360,12 +1356,6 @@ func TestPartitionTokens(t *testing.T) { }, } - prettyConfig := &pretty.Config{ - Diffable: true, - IncludeUnexported: true, - PrintStringers: true, - } - for i, test := range tests { t.Run(fmt.Sprintf("%02d", i), func(t *testing.T) { gotStart, gotEnd := partitionTokens(test.tokens, test.rng) @@ -1373,7 +1363,7 @@ func TestPartitionTokens(t *testing.T) { if gotStart != test.wantStart || gotEnd != test.wantEnd { t.Errorf( "wrong result\ntokens: %s\nrange: %#v\ngot: %d, %d\nwant: %d, %d", - prettyConfig.Sprint(test.tokens), test.rng, + spew.Sdump(test.tokens), test.rng, gotStart, test.wantStart, gotEnd, test.wantEnd, ) @@ -1437,12 +1427,6 @@ func TestPartitionLeadCommentTokens(t *testing.T) { }, } - prettyConfig := &pretty.Config{ - Diffable: true, - IncludeUnexported: true, - PrintStringers: true, - } - for i, test := range tests { t.Run(fmt.Sprintf("%02d", i), func(t *testing.T) { gotStart := partitionLeadCommentTokens(test.tokens) @@ -1450,7 +1434,7 @@ func TestPartitionLeadCommentTokens(t *testing.T) { if gotStart != test.wantStart { t.Errorf( "wrong result\ntokens: %s\ngot: %d\nwant: %d", - prettyConfig.Sprint(test.tokens), + spew.Sdump(test.tokens), gotStart, test.wantStart, ) } @@ -1589,20 +1573,15 @@ foo "bar" "baz" { }, } - prettyConfig := &pretty.Config{ - Diffable: true, - IncludeUnexported: true, - PrintStringers: true, - } - for _, test := range tests { t.Run(test.input, func(t *testing.T) { got := lexConfig([]byte(test.input)) - if !reflect.DeepEqual(got, test.want) { - diff := prettyConfig.Compare(test.want, got) + diff := cmp.Diff(test.want, got) + if diff != "" { t.Errorf( - "wrong result\ninput: %s\ndiff: %s", test.input, diff, + "wrong result\ninput: %s\ndiff:\n%s", + test.input, diff, ) } }) diff --git a/hclwrite/round_trip_test.go b/hclwrite/round_trip_test.go index bf306de5..9dbb1526 100644 --- a/hclwrite/round_trip_test.go +++ b/hclwrite/round_trip_test.go @@ -7,7 +7,7 @@ import ( "bytes" "testing" - "github.com/sergi/go-diff/diffmatchpatch" + "github.com/google/go-cmp/cmp" "github.com/zclconf/go-cty/cty" "github.com/zclconf/go-cty/cty/function" "github.com/zclconf/go-cty/cty/function/stdlib" @@ -71,12 +71,8 @@ block { } result := wr.Bytes() - if !bytes.Equal(result, src) { - dmp := diffmatchpatch.New() - diffs := dmp.DiffMain(string(src), string(result), false) - // t.Errorf("wrong result\nresult:\n%s\ninput:\n%s", result, src) - t.Errorf("wrong result\ndiff: (red indicates missing lines, and green indicates unexpected lines)\n%s", dmp.DiffPrettyText(diffs)) + t.Errorf("wrong result\ndiff:\n%s", cmp.Diff(string(src), string(result))) } }) } From 303be6113391cd39e05f26423d724e9bc4fb07f5 Mon Sep 17 00:00:00 2001 From: Radek Simko Date: Tue, 26 Mar 2024 14:54:38 +0000 Subject: [PATCH 06/23] Update CHANGELOG for 2.20.1 --- CHANGELOG.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1597a28b..2eebedbc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,15 @@ # HCL Changelog +## v2.20.1 (March 26, 2024) + +### Bugs Fixed + +* Return `ExprSyntaxError` when an invalid namespaced function is encountered during parsing ([#668](https://github.com/hashicorp/hcl/pull/668)) + +### Internal + +* Standardize on only two value dumping/diffing libraries ([#669](https://github.com/hashicorp/hcl/pull/669)) + ## v2.20.0 (February 29, 2024) ### Enhancements From f7cd61ac04cc66dcbb42ba84dfe640c976762021 Mon Sep 17 00:00:00 2001 From: Liam Cervante Date: Mon, 22 Apr 2024 14:20:01 +0200 Subject: [PATCH 07/23] Add additional function for parsing traversals with [*] keys (#673) * Add additional function for parsing traversals with [*] keys * add more context around skipped test cases --- hclsyntax/parse_traversal_test.go | 81 ++++++++++++++++++++++++++++++- hclsyntax/parser_traversal.go | 51 ++++++++++++++++++- hclsyntax/public.go | 31 ++++++++++++ 3 files changed, 161 insertions(+), 2 deletions(-) diff --git a/hclsyntax/parse_traversal_test.go b/hclsyntax/parse_traversal_test.go index 3ca5fc2b..5d8d24ea 100644 --- a/hclsyntax/parse_traversal_test.go +++ b/hclsyntax/parse_traversal_test.go @@ -4,11 +4,13 @@ package hclsyntax import ( + "fmt" "testing" "github.com/go-test/deep" - "github.com/hashicorp/hcl/v2" "github.com/zclconf/go-cty/cty" + + "github.com/hashicorp/hcl/v2" ) func TestParseTraversalAbs(t *testing.T) { @@ -208,10 +210,63 @@ func TestParseTraversalAbs(t *testing.T) { }, 1, // extra junk after traversal }, + + { + "foo[*]", + hcl.Traversal{ + hcl.TraverseRoot{ + Name: "foo", + SrcRange: hcl.Range{ + Start: hcl.Pos{Line: 1, Column: 1, Byte: 0}, + End: hcl.Pos{Line: 1, Column: 4, Byte: 3}, + }, + }, + hcl.TraverseSplat{ + SrcRange: hcl.Range{ + Start: hcl.Pos{Line: 1, Column: 4, Byte: 3}, + End: hcl.Pos{Line: 1, Column: 7, Byte: 6}, + }, + }, + }, + 0, + }, + { + "foo.*", // Still not supporting this. + hcl.Traversal{ + hcl.TraverseRoot{ + Name: "foo", + SrcRange: hcl.Range{ + Start: hcl.Pos{Line: 1, Column: 1, Byte: 0}, + End: hcl.Pos{Line: 1, Column: 4, Byte: 3}, + }, + }, + }, + 1, + }, + { + "foo[*].bar", // Run this through the unsupported function. + hcl.Traversal{ + hcl.TraverseRoot{ + Name: "foo", + SrcRange: hcl.Range{ + Start: hcl.Pos{Line: 1, Column: 1, Byte: 0}, + End: hcl.Pos{Line: 1, Column: 4, Byte: 3}, + }, + }, + }, + 1, + }, } for _, test := range tests { t.Run(test.src, func(t *testing.T) { + if test.src == "foo[*]" { + // The foo[*] test will fail because the function we test in + // this branch does not support the splat syntax. So we will + // skip this test case here. + t.Skip("skipping test for unsupported splat syntax") + } + got, diags := ParseTraversalAbs([]byte(test.src), "", hcl.Pos{Line: 1, Column: 1}) if len(diags) != test.diagCount { for _, diag := range diags { @@ -226,5 +281,29 @@ func TestParseTraversalAbs(t *testing.T) { } } }) + + t.Run(fmt.Sprintf("partial_%s", test.src), func(t *testing.T) { + if test.src == "foo[*].bar" { + // The foo[*].bar test will fail because the function we test in + // this branch does support the splat syntax and this test is + // designed to make sure that the other branch still fails with + // the splat syntax. So we will skip this test case here. + t.Skip("skipping test that fails for splat syntax") + } + + got, diags := ParseTraversalPartial([]byte(test.src), "", hcl.Pos{Line: 1, Column: 1}) + if len(diags) != test.diagCount { + for _, diag := range diags { + t.Logf(" - %s", diag.Error()) + } + t.Errorf("wrong number of diagnostics %d; want %d", len(diags), test.diagCount) + } + + if diff := deep.Equal(got, test.want); diff != nil { + for _, problem := range diff { + t.Error(problem) + } + } + }) } } diff --git a/hclsyntax/parser_traversal.go b/hclsyntax/parser_traversal.go index 3afa6ab0..f7d4062f 100644 --- a/hclsyntax/parser_traversal.go +++ b/hclsyntax/parser_traversal.go @@ -4,8 +4,9 @@ package hclsyntax import ( - "github.com/hashicorp/hcl/v2" "github.com/zclconf/go-cty/cty" + + "github.com/hashicorp/hcl/v2" ) // ParseTraversalAbs parses an absolute traversal that is assumed to consume @@ -13,6 +14,26 @@ import ( // behavior is not supported here because traversals are not expected to // be parsed as part of a larger program. func (p *parser) ParseTraversalAbs() (hcl.Traversal, hcl.Diagnostics) { + return p.parseTraversal(false) +} + +// ParseTraversalPartial parses an absolute traversal that is permitted +// to contain splat ([*]) expressions. Only splat expressions within square +// brackets are permitted ([*]); splat expressions within attribute names are +// not permitted (.*). +// +// The meaning of partial here is that the traversal may be incomplete, in that +// any splat expression indicates reference to a potentially unknown number of +// elements. +// +// Traversals that include splats cannot be automatically traversed by HCL using +// the TraversalAbs or TraversalRel methods. Instead, the caller must handle +// the traversals manually. +func (p *parser) ParseTraversalPartial() (hcl.Traversal, hcl.Diagnostics) { + return p.parseTraversal(true) +} + +func (p *parser) parseTraversal(allowSplats bool) (hcl.Traversal, hcl.Diagnostics) { var ret hcl.Traversal var diags hcl.Diagnostics @@ -127,6 +148,34 @@ func (p *parser) ParseTraversalAbs() (hcl.Traversal, hcl.Diagnostics) { return ret, diags } + case TokenStar: + if allowSplats { + + p.Read() // Eat the star. + close := p.Read() + if close.Type != TokenCBrack { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Unclosed index brackets", + Detail: "Index key must be followed by a closing bracket.", + Subject: &close.Range, + Context: hcl.RangeBetween(open.Range, close.Range).Ptr(), + }) + } + + ret = append(ret, hcl.TraverseSplat{ + SrcRange: hcl.RangeBetween(open.Range, close.Range), + }) + + if diags.HasErrors() { + return ret, diags + } + + continue + } + + // Otherwise, return the error below for the star. + fallthrough default: if next.Type == TokenStar { diags = append(diags, &hcl.Diagnostic{ diff --git a/hclsyntax/public.go b/hclsyntax/public.go index d56f8e50..17dc1ed4 100644 --- a/hclsyntax/public.go +++ b/hclsyntax/public.go @@ -118,6 +118,37 @@ func ParseTraversalAbs(src []byte, filename string, start hcl.Pos) (hcl.Traversa return expr, diags } +// ParseTraversalPartial matches the behavior of ParseTraversalAbs except +// that it allows splat expressions ([*]) to appear in the traversal. +// +// The returned traversals are "partial" in that the splat expression indicates +// an unknown value for the index. +// +// Traversals that include splats cannot be automatically traversed by HCL using +// the TraversalAbs or TraversalRel methods. Instead, the caller must handle +// the traversals manually. +func ParseTraversalPartial(src []byte, filename string, start hcl.Pos) (hcl.Traversal, hcl.Diagnostics) { + tokens, diags := LexExpression(src, filename, start) + peeker := newPeeker(tokens, false) + parser := &parser{peeker: peeker} + + // Bare traverals are always parsed in "ignore newlines" mode, as if + // they were wrapped in parentheses. + parser.PushIncludeNewlines(false) + + expr, parseDiags := parser.ParseTraversalPartial() + diags = append(diags, parseDiags...) + + parser.PopIncludeNewlines() + + // Panic if the parser uses incorrect stack discipline with the peeker's + // newlines stack, since otherwise it will produce confusing downstream + // errors. + peeker.AssertEmptyIncludeNewlinesStack() + + return expr, diags +} + // LexConfig performs lexical analysis on the given buffer, treating it as a // whole HCL config file, and returns the resulting tokens. // From 1c5ae8fc88a656ab7bd46da4ff27a20c5a97497b Mon Sep 17 00:00:00 2001 From: Liam Cervante Date: Mon, 22 Apr 2024 14:24:19 +0200 Subject: [PATCH 08/23] Update CHANGELOG.md --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2eebedbc..a33c5a68 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # HCL Changelog +## v2.21.0 (Unreleased) + +### Enhancements + +* Introduce `ParseTraversalPartial`, which allows traversals that include the splat (`[*]`) index operator. ([#673](https://github.com/hashicorp/hcl/pull/673)) + ## v2.20.1 (March 26, 2024) ### Bugs Fixed From 4521ae92f155259e24118bddbe5fc4ef23d2a7ff Mon Sep 17 00:00:00 2001 From: "hashicorp-tsccr[bot]" <129506189+hashicorp-tsccr[bot]@users.noreply.github.com> Date: Wed, 8 May 2024 12:56:20 +0200 Subject: [PATCH 09/23] github: Pin action refs to latest trusted by TSCCR (#677) Co-authored-by: hashicorp-tsccr[bot] --- .github/workflows/checks.yml | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index 51e279ae..5102728d 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -30,9 +30,9 @@ jobs: run: | git config --global core.autocrlf false - name: "Fetch source code" - uses: actions/checkout@8ade135a41bc03ea155e62e844d188df1ea18608 # v4.1.0 + uses: actions/checkout@0ad4b8fadaa221de15dcec353f45205ec38ea70b # v4.1.4 - name: Install Go - uses: actions/setup-go@93397bea11091df50f3d7e59dc26a7711a8bcfbe # v4.1.0 + uses: actions/setup-go@cdcb36043654635271a94b9a6d1392de5bb323a7 # v5.0.1 with: go-version-file: go.mod - name: Go test @@ -44,9 +44,9 @@ jobs: runs-on: ubuntu-latest steps: - name: "Fetch source code" - uses: actions/checkout@8ade135a41bc03ea155e62e844d188df1ea18608 # v4.1.0 + uses: actions/checkout@0ad4b8fadaa221de15dcec353f45205ec38ea70b # v4.1.4 - name: Install Go - uses: actions/setup-go@93397bea11091df50f3d7e59dc26a7711a8bcfbe # v4.1.0 + uses: actions/setup-go@cdcb36043654635271a94b9a6d1392de5bb323a7 # v5.0.1 with: go-version-file: go.mod - name: "copyright headers check" @@ -58,9 +58,9 @@ jobs: runs-on: ubuntu-latest steps: - name: "Fetch source code" - uses: actions/checkout@8ade135a41bc03ea155e62e844d188df1ea18608 # v4.1.0 + uses: actions/checkout@0ad4b8fadaa221de15dcec353f45205ec38ea70b # v4.1.4 - name: Install Go - uses: actions/setup-go@93397bea11091df50f3d7e59dc26a7711a8bcfbe # v4.1.0 + uses: actions/setup-go@cdcb36043654635271a94b9a6d1392de5bb323a7 # v5.0.1 with: go-version-file: go.mod - name: "go vet" @@ -72,9 +72,9 @@ jobs: runs-on: ubuntu-latest steps: - name: "Fetch source code" - uses: actions/checkout@8ade135a41bc03ea155e62e844d188df1ea18608 # v4.1.0 + uses: actions/checkout@0ad4b8fadaa221de15dcec353f45205ec38ea70b # v4.1.4 - name: Install Go - uses: actions/setup-go@93397bea11091df50f3d7e59dc26a7711a8bcfbe # v4.1.0 + uses: actions/setup-go@cdcb36043654635271a94b9a6d1392de5bb323a7 # v5.0.1 with: go-version-file: go.mod - name: "gofmt" From bf546973d01a1ec4a306cbcc789bd86ea70e44be Mon Sep 17 00:00:00 2001 From: Nara Kasbergen Kwon <855115+xiehan@users.noreply.github.com> Date: Wed, 8 May 2024 13:00:10 +0200 Subject: [PATCH 10/23] github: Set up Dependabot to manage HashiCorp-owned Actions versioning --- .github/dependabot.yml | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 .github/dependabot.yml diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 00000000..61e0aff8 --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,22 @@ +version: 2 +updates: + - package-ecosystem: github-actions + directory: / + schedule: + interval: monthly + labels: + - dependencies + - automated + reviewers: + - hashicorp/terraform-core + # only update HashiCorp actions, external actions managed by TSCCR + allow: + - dependency-name: hashicorp/* + groups: + github-actions-breaking: + update-types: + - major + github-actions-backward-compatible: + update-types: + - minor + - patch From bc757658ca11c5d6d17f328d5672ac447c3efcff Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Wed, 8 May 2024 16:09:28 -0700 Subject: [PATCH 11/23] hclsyntax: Don't panic if splat operand is unknown and marked We were calling .Range() on any unknown sourceVal, without first checking whether it was marked. That method panics if called on a marked value, so we need to strip that off first. While testing this I found some return paths that weren't properly transferring the source value's marks to the output, and so this also addresses those so that all return paths preserve whatever markings are present on the source value. In particular, if a non-list/set/tuple value gets "upgraded" into a tuple then we must transfer its marks onto the tuple, because the decision about constructing that value was based on characteristics of the source value. --- hclsyntax/expression.go | 15 ++++++++------- hclsyntax/expression_test.go | 10 ++++++++++ 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/hclsyntax/expression.go b/hclsyntax/expression.go index 81597399..577a50fa 100644 --- a/hclsyntax/expression.go +++ b/hclsyntax/expression.go @@ -1780,7 +1780,7 @@ func (e *SplatExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnostics) { if sourceVal.IsNull() { if autoUpgrade { - return cty.EmptyTupleVal, diags + return cty.EmptyTupleVal.WithSameMarks(sourceVal), diags } diags = append(diags, &hcl.Diagnostic{ Severity: hcl.DiagError, @@ -1798,7 +1798,7 @@ func (e *SplatExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnostics) { // If we don't even know the _type_ of our source value yet then // we'll need to defer all processing, since we can't decide our // result type either. - return cty.DynamicVal, diags + return cty.DynamicVal.WithSameMarks(sourceVal), diags } upgradedUnknown := false @@ -1813,13 +1813,14 @@ func (e *SplatExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnostics) { // list of a single attribute, but we still need to check if that // attribute actually exists. if !sourceVal.IsKnown() { - sourceRng := sourceVal.Range() + unmarkedVal, _ := sourceVal.Unmark() + sourceRng := unmarkedVal.Range() if sourceRng.CouldBeNull() { upgradedUnknown = true } } - sourceVal = cty.TupleVal([]cty.Value{sourceVal}) + sourceVal = cty.TupleVal([]cty.Value{sourceVal}).WithSameMarks(sourceVal) sourceTy = sourceVal.Type() } @@ -1900,14 +1901,14 @@ func (e *SplatExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnostics) { e.Item.clearValue(ctx) // clean up our temporary value if upgradedUnknown { - return cty.DynamicVal, diags + return cty.DynamicVal.WithMarks(marks), diags } if !isKnown { // We'll ingore the resultTy diagnostics in this case since they // will just be the same errors we saw while iterating above. ty, _ := resultTy() - return cty.UnknownVal(ty), diags + return cty.UnknownVal(ty).WithMarks(marks), diags } switch { @@ -1915,7 +1916,7 @@ func (e *SplatExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnostics) { if len(vals) == 0 { ty, tyDiags := resultTy() diags = append(diags, tyDiags...) - return cty.ListValEmpty(ty.ElementType()), diags + return cty.ListValEmpty(ty.ElementType()).WithMarks(marks), diags } return cty.ListVal(vals).WithMarks(marks), diags default: diff --git a/hclsyntax/expression_test.go b/hclsyntax/expression_test.go index b18214cd..b50dc137 100644 --- a/hclsyntax/expression_test.go +++ b/hclsyntax/expression_test.go @@ -1457,6 +1457,16 @@ upper( cty.UnknownVal(cty.List(cty.Bool)).RefineNotNull().Mark("sensitive"), 0, }, + { // splat with sensitive non-collection that's unknown + `not_a_list.*`, + &hcl.EvalContext{ + Variables: map[string]cty.Value{ + "not_a_list": cty.UnknownVal(cty.EmptyObject).RefineNotNull().Mark("sensitive"), + }, + }, + cty.TupleVal([]cty.Value{cty.UnknownVal(cty.EmptyObject).RefineNotNull().Mark("sensitive")}).Mark("sensitive"), + 0, + }, { // splat with sensitive collection that's unknown and not null `maps.*.enabled`, &hcl.EvalContext{ From 9a64c17c75059d9c8f5d94f2265c00026ac48781 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Wed, 8 May 2024 17:52:51 -0700 Subject: [PATCH 12/23] dynblock: Preserve marks from for_each expression into result Previously if the for_each expression was marked then expansion would fail because marked expressions are never directly iterable. Now instead we'll allow marked for_each and preserve the marks into the values produced by the resulting block as much as we can. This runs into the classic problem that HCL blocks are not values themselves and so cannot carry marks directly, but we can at least make sure that the values of any leaf arguments end up marked. --- ext/dynblock/expand_body.go | 39 +++++++++++++------ ext/dynblock/expand_body_test.go | 64 ++++++++++++++++++++++++++++++++ ext/dynblock/expand_spec.go | 27 +++++++++++++- ext/dynblock/expr_wrap.go | 24 +++++++++++- ext/dynblock/unknown_body.go | 23 +++++++++--- 5 files changed, 157 insertions(+), 20 deletions(-) diff --git a/ext/dynblock/expand_body.go b/ext/dynblock/expand_body.go index 2734e937..e630f184 100644 --- a/ext/dynblock/expand_body.go +++ b/ext/dynblock/expand_body.go @@ -16,6 +16,7 @@ type expandBody struct { original hcl.Body forEachCtx *hcl.EvalContext iteration *iteration // non-nil if we're nested inside another "dynamic" block + valueMarks cty.ValueMarks checkForEach []func(cty.Value, hcl.Expression, *hcl.EvalContext) hcl.Diagnostics @@ -125,7 +126,7 @@ func (b *expandBody) extendSchema(schema *hcl.BodySchema) *hcl.BodySchema { } func (b *expandBody) prepareAttributes(rawAttrs hcl.Attributes) hcl.Attributes { - if len(b.hiddenAttrs) == 0 && b.iteration == nil { + if len(b.hiddenAttrs) == 0 && b.iteration == nil && len(b.valueMarks) == 0 { // Easy path: just pass through the attrs from the original body verbatim return rawAttrs } @@ -142,13 +143,24 @@ func (b *expandBody) prepareAttributes(rawAttrs hcl.Attributes) hcl.Attributes { if b.iteration != nil { attr := *rawAttr // shallow copy so we can mutate it attr.Expr = exprWrap{ - Expression: attr.Expr, - i: b.iteration, + Expression: attr.Expr, + i: b.iteration, + resultMarks: b.valueMarks, } attrs[name] = &attr } else { - // If we have no active iteration then no wrapping is required. - attrs[name] = rawAttr + // If we have no active iteration then no wrapping is required + // unless we have marks to apply. + if len(b.valueMarks) != 0 { + attr := *rawAttr // shallow copy so we can mutate it + attr.Expr = exprWrap{ + Expression: attr.Expr, + resultMarks: b.valueMarks, + } + attrs[name] = &attr + } else { + attrs[name] = rawAttr + } } } return attrs @@ -192,8 +204,9 @@ func (b *expandBody) expandBlocks(schema *hcl.BodySchema, rawBlocks hcl.Blocks, continue } - if spec.forEachVal.IsKnown() { - for it := spec.forEachVal.ElementIterator(); it.Next(); { + forEachVal, marks := spec.forEachVal.Unmark() + if forEachVal.IsKnown() { + for it := forEachVal.ElementIterator(); it.Next(); { key, value := it.Element() i := b.iteration.MakeChild(spec.iteratorName, key, value) @@ -202,7 +215,7 @@ func (b *expandBody) expandBlocks(schema *hcl.BodySchema, rawBlocks hcl.Blocks, if block != nil { // Attach our new iteration context so that attributes // and other nested blocks can refer to our iterator. - block.Body = b.expandChild(block.Body, i) + block.Body = b.expandChild(block.Body, i, marks) blocks = append(blocks, block) } } @@ -214,7 +227,10 @@ func (b *expandBody) expandBlocks(schema *hcl.BodySchema, rawBlocks hcl.Blocks, block, blockDiags := spec.newBlock(i, b.forEachCtx) diags = append(diags, blockDiags...) if block != nil { - block.Body = unknownBody{b.expandChild(block.Body, i)} + block.Body = unknownBody{ + template: b.expandChild(block.Body, i, marks), + valueMarks: marks, + } blocks = append(blocks, block) } } @@ -226,7 +242,7 @@ func (b *expandBody) expandBlocks(schema *hcl.BodySchema, rawBlocks hcl.Blocks, // case it contains expressions that refer to our inherited // iterators, or nested "dynamic" blocks. expandedBlock := *rawBlock // shallow copy - expandedBlock.Body = b.expandChild(rawBlock.Body, b.iteration) + expandedBlock.Body = b.expandChild(rawBlock.Body, b.iteration, nil) blocks = append(blocks, &expandedBlock) } } @@ -235,11 +251,12 @@ func (b *expandBody) expandBlocks(schema *hcl.BodySchema, rawBlocks hcl.Blocks, return blocks, diags } -func (b *expandBody) expandChild(child hcl.Body, i *iteration) hcl.Body { +func (b *expandBody) expandChild(child hcl.Body, i *iteration, valueMarks cty.ValueMarks) hcl.Body { chiCtx := i.EvalContext(b.forEachCtx) ret := Expand(child, chiCtx) ret.(*expandBody).iteration = i ret.(*expandBody).checkForEach = b.checkForEach + ret.(*expandBody).valueMarks = valueMarks return ret } diff --git a/ext/dynblock/expand_body_test.go b/ext/dynblock/expand_body_test.go index bec6c210..3b245ee8 100644 --- a/ext/dynblock/expand_body_test.go +++ b/ext/dynblock/expand_body_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/davecgh/go-spew/spew" + "github.com/google/go-cmp/cmp" "github.com/hashicorp/hcl/v2" "github.com/hashicorp/hcl/v2/hcldec" "github.com/hashicorp/hcl/v2/hcltest" @@ -710,6 +711,69 @@ func TestExpandUnknownBodies(t *testing.T) { } +func TestExpandMarkedForEach(t *testing.T) { + srcBody := hcltest.MockBody(&hcl.BodyContent{ + Blocks: hcl.Blocks{ + { + Type: "dynamic", + Labels: []string{"b"}, + LabelRanges: []hcl.Range{{}}, + Body: hcltest.MockBody(&hcl.BodyContent{ + Attributes: hcltest.MockAttrs(map[string]hcl.Expression{ + "for_each": hcltest.MockExprLiteral(cty.TupleVal([]cty.Value{ + cty.StringVal("hey"), + }).Mark("boop")), + "iterator": hcltest.MockExprTraversalSrc("dyn_b"), + }), + Blocks: hcl.Blocks{ + { + Type: "content", + Body: hcltest.MockBody(&hcl.BodyContent{ + Attributes: hcltest.MockAttrs(map[string]hcl.Expression{ + "val0": hcltest.MockExprLiteral(cty.StringVal("static c 1")), + "val1": hcltest.MockExprTraversalSrc("dyn_b.value"), + }), + }), + }, + }, + }), + }, + }, + }) + + dynBody := Expand(srcBody, nil) + + t.Run("Decode", func(t *testing.T) { + decSpec := &hcldec.BlockListSpec{ + TypeName: "b", + Nested: &hcldec.ObjectSpec{ + "val0": &hcldec.AttrSpec{ + Name: "val0", + Type: cty.String, + }, + "val1": &hcldec.AttrSpec{ + Name: "val1", + Type: cty.String, + }, + }, + } + + want := cty.ListVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "val0": cty.StringVal("static c 1").Mark("boop"), + "val1": cty.StringVal("hey").Mark("boop"), + }), + }) + got, diags := hcldec.Decode(dynBody, decSpec, nil) + if diags.HasErrors() { + t.Fatalf("unexpected errors\n%s", diags.Error()) + } + if diff := cmp.Diff(want, got, ctydebug.CmpOptions); diff != "" { + t.Errorf("wrong result\n%s", diff) + } + }) +} + func TestExpandInvalidIteratorError(t *testing.T) { srcBody := hcltest.MockBody(&hcl.BodyContent{ Blocks: hcl.Blocks{ diff --git a/ext/dynblock/expand_spec.go b/ext/dynblock/expand_spec.go index 0231c4aa..55a69ba9 100644 --- a/ext/dynblock/expand_spec.go +++ b/ext/dynblock/expand_spec.go @@ -77,7 +77,8 @@ func (b *expandBody) decodeSpec(blockS *hcl.BlockHeaderSchema, rawSpec *hcl.Bloc } } - if !eachVal.CanIterateElements() && eachVal.Type() != cty.DynamicPseudoType { + unmarkedEachVal, _ := eachVal.Unmark() + if !unmarkedEachVal.CanIterateElements() && unmarkedEachVal.Type() != cty.DynamicPseudoType { // We skip this error for DynamicPseudoType because that means we either // have a null (which is checked immediately below) or an unknown // (which is handled in the expandBody Content methods). @@ -91,7 +92,7 @@ func (b *expandBody) decodeSpec(blockS *hcl.BlockHeaderSchema, rawSpec *hcl.Bloc }) return nil, diags } - if eachVal.IsNull() { + if unmarkedEachVal.IsNull() { diags = append(diags, &hcl.Diagnostic{ Severity: hcl.DiagError, Summary: "Invalid dynamic for_each value", @@ -212,6 +213,28 @@ func (s *expandSpec) newBlock(i *iteration, ctx *hcl.EvalContext) (*hcl.Block, h }) return nil, diags } + if labelVal.IsMarked() { + // This situation is tricky because HCL just works generically + // with marks and so doesn't have any good language to talk about + // the meaning of specific mark types, but yet we cannot allow + // marked values here because the HCL API guarantees that a block's + // labels are always known static constant Go strings. + // Therefore this is a low-quality error message but at least + // better than panicking below when we call labelVal.AsString. + // If this becomes a problem then we could potentially add a new + // option for the public function [Expand] to allow calling + // applications to specify custom label validation functions that + // could then supersede this generic message. + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid dynamic block label", + Detail: "This value has dynamic marks that make it unsuitable for use as a block label.", + Subject: labelExpr.Range().Ptr(), + Expression: labelExpr, + EvalContext: lCtx, + }) + return nil, diags + } labels = append(labels, labelVal.AsString()) labelRanges = append(labelRanges, labelExpr.Range()) diff --git a/ext/dynblock/expr_wrap.go b/ext/dynblock/expr_wrap.go index 625bf9cc..57636411 100644 --- a/ext/dynblock/expr_wrap.go +++ b/ext/dynblock/expr_wrap.go @@ -11,6 +11,19 @@ import ( type exprWrap struct { hcl.Expression i *iteration + + // resultMarks is a set of marks that must be applied to whatever + // value results from this expression. We do this whenever a + // dynamic block's for_each expression produced a marked result, + // since in that case any nested expressions inside are treated + // as being derived from that for_each expression. + // + // (calling applications might choose to reject marks by passing + // an [OptCheckForEach] to [Expand] and returning an error when + // marks are present, but this mechanism is here to help achieve + // reasonable behavior for situations where marks are permitted, + // which is the default.) + resultMarks cty.ValueMarks } func (e exprWrap) Variables() []hcl.Traversal { @@ -34,8 +47,13 @@ func (e exprWrap) Variables() []hcl.Traversal { } func (e exprWrap) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnostics) { + if e.i == nil { + // If we don't have an active iteration then we can just use the + // given EvalContext directly. + return e.prepareValue(e.Expression.Value(ctx)) + } extCtx := e.i.EvalContext(ctx) - return e.Expression.Value(extCtx) + return e.prepareValue(e.Expression.Value(extCtx)) } // UnwrapExpression returns the expression being wrapped by this instance. @@ -43,3 +61,7 @@ func (e exprWrap) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnostics) { func (e exprWrap) UnwrapExpression() hcl.Expression { return e.Expression } + +func (e exprWrap) prepareValue(val cty.Value, diags hcl.Diagnostics) (cty.Value, hcl.Diagnostics) { + return val.WithMarks(e.resultMarks), diags +} diff --git a/ext/dynblock/unknown_body.go b/ext/dynblock/unknown_body.go index 6cd59c77..b1fdfc07 100644 --- a/ext/dynblock/unknown_body.go +++ b/ext/dynblock/unknown_body.go @@ -5,6 +5,7 @@ package dynblock import ( "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/hcl/v2/hcldec" "github.com/zclconf/go-cty/cty" ) @@ -18,16 +19,26 @@ import ( // we instead arrange for everything _inside_ the block to be unknown instead, // to give the best possible approximation. type unknownBody struct { - template hcl.Body + template hcl.Body + valueMarks cty.ValueMarks } var _ hcl.Body = unknownBody{} -// hcldec.UnkownBody impl +// hcldec.UnknownBody impl func (b unknownBody) Unknown() bool { return true } +// hcldec.MarkedBody impl +func (b unknownBody) BodyValueMarks() cty.ValueMarks { + // We'll pass through to our template if it is a MarkedBody + if t, ok := b.template.(hcldec.MarkedBody); ok { + return t.BodyValueMarks() + } + return nil +} + func (b unknownBody) Content(schema *hcl.BodySchema) (*hcl.BodyContent, hcl.Diagnostics) { content, diags := b.template.Content(schema) content = b.fixupContent(content) @@ -41,7 +52,7 @@ func (b unknownBody) Content(schema *hcl.BodySchema) (*hcl.BodyContent, hcl.Diag func (b unknownBody) PartialContent(schema *hcl.BodySchema) (*hcl.BodyContent, hcl.Body, hcl.Diagnostics) { content, remain, diags := b.template.PartialContent(schema) content = b.fixupContent(content) - remain = unknownBody{remain} // remaining content must also be wrapped + remain = unknownBody{template: remain, valueMarks: b.valueMarks} // remaining content must also be wrapped // We're intentionally preserving the diagnostics reported from the // inner body so that we can still report where the template body doesn't @@ -69,8 +80,8 @@ func (b unknownBody) fixupContent(got *hcl.BodyContent) *hcl.BodyContent { if len(got.Blocks) > 0 { ret.Blocks = make(hcl.Blocks, 0, len(got.Blocks)) for _, gotBlock := range got.Blocks { - new := *gotBlock // shallow copy - new.Body = unknownBody{gotBlock.Body} // nested content must also be marked unknown + new := *gotBlock // shallow copy + new.Body = unknownBody{template: gotBlock.Body, valueMarks: b.valueMarks} // nested content must also be marked unknown ret.Blocks = append(ret.Blocks, &new) } } @@ -85,7 +96,7 @@ func (b unknownBody) fixupAttrs(got hcl.Attributes) hcl.Attributes { ret := make(hcl.Attributes, len(got)) for name, gotAttr := range got { new := *gotAttr // shallow copy - new.Expr = hcl.StaticExpr(cty.DynamicVal, gotAttr.Expr.Range()) + new.Expr = hcl.StaticExpr(cty.DynamicVal.WithMarks(b.valueMarks), gotAttr.Expr.Range()) ret[name] = &new } return ret From 318bbfebb5e1eeea09765b1edb1aec303f75a268 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Wed, 8 May 2024 17:55:39 -0700 Subject: [PATCH 13/23] hcldec: Allow body-derived values to be marked Similar to the previously-added UnknownBody, the new optional interface MarkedBody allows hcl.Body implementations to suggest a set of marks that ought to be applied to any value that's generated to represent the content of that body. The dynblock extension then uses this to get hcldec to mark the whole object representing any block that was generated by a dynamic block whose for_each was marked, for a better representation of the fact that a block's existence was decided based on a marked value. --- ext/dynblock/expand_body.go | 5 +++++ ext/dynblock/expand_body_test.go | 2 +- go.mod | 2 +- go.sum | 4 ++++ hcldec/spec.go | 19 +++++++++++++++++++ 5 files changed, 30 insertions(+), 2 deletions(-) diff --git a/ext/dynblock/expand_body.go b/ext/dynblock/expand_body.go index e630f184..2cafae5f 100644 --- a/ext/dynblock/expand_body.go +++ b/ext/dynblock/expand_body.go @@ -270,3 +270,8 @@ func (b *expandBody) JustAttributes() (hcl.Attributes, hcl.Diagnostics) { func (b *expandBody) MissingItemRange() hcl.Range { return b.original.MissingItemRange() } + +// hcldec.MarkedBody impl +func (b *expandBody) BodyValueMarks() cty.ValueMarks { + return b.valueMarks +} diff --git a/ext/dynblock/expand_body_test.go b/ext/dynblock/expand_body_test.go index 3b245ee8..0941f291 100644 --- a/ext/dynblock/expand_body_test.go +++ b/ext/dynblock/expand_body_test.go @@ -762,7 +762,7 @@ func TestExpandMarkedForEach(t *testing.T) { cty.ObjectVal(map[string]cty.Value{ "val0": cty.StringVal("static c 1").Mark("boop"), "val1": cty.StringVal("hey").Mark("boop"), - }), + }).Mark("boop"), }) got, diags := hcldec.Decode(dynBody, decSpec, nil) if diags.HasErrors() { diff --git a/go.mod b/go.mod index d9d4da45..56c414fa 100644 --- a/go.mod +++ b/go.mod @@ -11,7 +11,7 @@ require ( github.com/mitchellh/go-wordwrap v0.0.0-20150314170334-ad45545899c7 github.com/spf13/pflag v1.0.2 github.com/zclconf/go-cty v1.13.0 - github.com/zclconf/go-cty-debug v0.0.0-20191215020915-b22d67c1ba0b + github.com/zclconf/go-cty-debug v0.0.0-20240509010212-0d6042c53940 golang.org/x/crypto v0.0.0-20220517005047-85d78b3ac167 golang.org/x/tools v0.6.0 ) diff --git a/go.sum b/go.sum index 08ed23ed..53611d83 100644 --- a/go.sum +++ b/go.sum @@ -27,6 +27,10 @@ github.com/zclconf/go-cty v1.13.0 h1:It5dfKTTZHe9aeppbNOda3mN7Ag7sg6QkBNm6TkyFa0 github.com/zclconf/go-cty v1.13.0/go.mod h1:YKQzy/7pZ7iq2jNFzy5go57xdxdWoLLpaEp4u238AE0= github.com/zclconf/go-cty-debug v0.0.0-20191215020915-b22d67c1ba0b h1:FosyBZYxY34Wul7O/MSKey3txpPYyCqVO5ZyceuQJEI= github.com/zclconf/go-cty-debug v0.0.0-20191215020915-b22d67c1ba0b/go.mod h1:ZRKQfBXbGkpdV6QMzT3rU1kSTAnfu1dO8dPKjYprgj8= +github.com/zclconf/go-cty-debug v0.0.0-20240417160409-8c45e122ae1a h1:/o/Emn22dZIQ7AhyA0aLOKo528WG/WRAM5tqzIoQIOs= +github.com/zclconf/go-cty-debug v0.0.0-20240417160409-8c45e122ae1a/go.mod h1:CmBdvvj3nqzfzJ6nTCIwDTPZ56aVGvDrmztiO5g3qrM= +github.com/zclconf/go-cty-debug v0.0.0-20240509010212-0d6042c53940 h1:4r45xpDWB6ZMSMNJFMOjqrGHynW3DIBuR2H9j0ug+Mo= +github.com/zclconf/go-cty-debug v0.0.0-20240509010212-0d6042c53940/go.mod h1:CmBdvvj3nqzfzJ6nTCIwDTPZ56aVGvDrmztiO5g3qrM= golang.org/x/crypto v0.0.0-20220517005047-85d78b3ac167 h1:O8uGbHCqlTp2P6QJSLmCojM4mN6UemYv8K+dCnmHmu0= golang.org/x/crypto v0.0.0-20220517005047-85d78b3ac167/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4= golang.org/x/mod v0.8.0 h1:LUYupSeNrTNCGzR/hVBk2NHZO4hXcVaW1k4Qx7rjPx8= diff --git a/hcldec/spec.go b/hcldec/spec.go index 2bebc433..b07a5534 100644 --- a/hcldec/spec.go +++ b/hcldec/spec.go @@ -73,6 +73,12 @@ type UnknownBody interface { Unknown() bool } +// MarkedBody can be optionally implemented by an hcl.Body instance to +// indicate that a value created from it ought to be marked. +type MarkedBody interface { + BodyValueMarks() cty.ValueMarks +} + func (s ObjectSpec) visitSameBodyChildren(cb visitFunc) { for _, c := range s { cb(c) @@ -473,6 +479,7 @@ func (s *BlockListSpec) decode(content *hcl.BodyContent, blockLabels []blockLabe val, _, childDiags := decode(childBlock.Body, labelsForBlock(childBlock), ctx, s.Nested, false) diags = append(diags, childDiags...) + val = prepareBodyVal(val, childBlock.Body) if u, ok := childBlock.Body.(UnknownBody); ok { if u.Unknown() { @@ -635,6 +642,7 @@ func (s *BlockTupleSpec) decode(content *hcl.BodyContent, blockLabels []blockLab val, _, childDiags := decode(childBlock.Body, labelsForBlock(childBlock), ctx, s.Nested, false) diags = append(diags, childDiags...) + val = prepareBodyVal(val, childBlock.Body) if u, ok := childBlock.Body.(UnknownBody); ok { if u.Unknown() { @@ -758,6 +766,7 @@ func (s *BlockSetSpec) decode(content *hcl.BodyContent, blockLabels []blockLabel val, _, childDiags := decode(childBlock.Body, labelsForBlock(childBlock), ctx, s.Nested, false) diags = append(diags, childDiags...) + val = prepareBodyVal(val, childBlock.Body) if u, ok := childBlock.Body.(UnknownBody); ok { if u.Unknown() { @@ -928,6 +937,7 @@ func (s *BlockMapSpec) decode(content *hcl.BodyContent, blockLabels []blockLabel childLabels := labelsForBlock(childBlock) val, _, childDiags := decode(childBlock.Body, childLabels[len(s.LabelNames):], ctx, s.Nested, false) + val = prepareBodyVal(val, childBlock.Body) targetMap := elems for _, key := range childBlock.Labels[:len(s.LabelNames)-1] { if _, exists := targetMap[key]; !exists { @@ -1082,6 +1092,7 @@ func (s *BlockObjectSpec) decode(content *hcl.BodyContent, blockLabels []blockLa childLabels := labelsForBlock(childBlock) val, _, childDiags := decode(childBlock.Body, childLabels[len(s.LabelNames):], ctx, s.Nested, false) + val = prepareBodyVal(val, childBlock.Body) targetMap := elems for _, key := range childBlock.Labels[:len(s.LabelNames)-1] { if _, exists := targetMap[key]; !exists { @@ -1720,3 +1731,11 @@ func (s noopSpec) sourceRange(content *hcl.BodyContent, blockLabels []blockLabel Filename: "noopSpec", } } + +func prepareBodyVal(decodeResult cty.Value, body hcl.Body) cty.Value { + if m, ok := body.(MarkedBody); ok { + marks := m.BodyValueMarks() + return decodeResult.WithMarks(marks) + } + return decodeResult +} From 212a40e528766634a1aa6dd1e820d7936762196e Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Thu, 9 May 2024 11:50:35 -0700 Subject: [PATCH 14/23] Update CHANGELOG.md --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a33c5a68..e8d06e47 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,11 @@ ### Enhancements * Introduce `ParseTraversalPartial`, which allows traversals that include the splat (`[*]`) index operator. ([#673](https://github.com/hashicorp/hcl/pull/673)) +* ext/dynblock: Now accepts marked values in `for_each`, and will transfer those marks (as much as technically possible) to values in the generated blocks. ([#679](https://github.com/hashicorp/hcl/pull/679)) + +### Bugs Fixed + +* Expression evaluation will no longer panic if the splat operator is applied to an unknown value that has cty marks. ([#678](https://github.com/hashicorp/hcl/pull/678)) ## v2.20.1 (March 26, 2024) From f7e093a382df24049d3ebbaeb059550b2b755b6f Mon Sep 17 00:00:00 2001 From: "hashicorp-tsccr[bot]" <129506189+hashicorp-tsccr[bot]@users.noreply.github.com> Date: Fri, 14 Jun 2024 15:41:56 +0200 Subject: [PATCH 15/23] github: Pin action refs to latest trusted by TSCCR (#683) Co-authored-by: hashicorp-tsccr[bot] --- .github/workflows/checks.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index 5102728d..a5f6b7af 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -30,7 +30,7 @@ jobs: run: | git config --global core.autocrlf false - name: "Fetch source code" - uses: actions/checkout@0ad4b8fadaa221de15dcec353f45205ec38ea70b # v4.1.4 + uses: actions/checkout@a5ac7e51b41094c92402da3b24376905380afc29 # v4.1.6 - name: Install Go uses: actions/setup-go@cdcb36043654635271a94b9a6d1392de5bb323a7 # v5.0.1 with: @@ -44,7 +44,7 @@ jobs: runs-on: ubuntu-latest steps: - name: "Fetch source code" - uses: actions/checkout@0ad4b8fadaa221de15dcec353f45205ec38ea70b # v4.1.4 + uses: actions/checkout@a5ac7e51b41094c92402da3b24376905380afc29 # v4.1.6 - name: Install Go uses: actions/setup-go@cdcb36043654635271a94b9a6d1392de5bb323a7 # v5.0.1 with: @@ -58,7 +58,7 @@ jobs: runs-on: ubuntu-latest steps: - name: "Fetch source code" - uses: actions/checkout@0ad4b8fadaa221de15dcec353f45205ec38ea70b # v4.1.4 + uses: actions/checkout@a5ac7e51b41094c92402da3b24376905380afc29 # v4.1.6 - name: Install Go uses: actions/setup-go@cdcb36043654635271a94b9a6d1392de5bb323a7 # v5.0.1 with: @@ -72,7 +72,7 @@ jobs: runs-on: ubuntu-latest steps: - name: "Fetch source code" - uses: actions/checkout@0ad4b8fadaa221de15dcec353f45205ec38ea70b # v4.1.4 + uses: actions/checkout@a5ac7e51b41094c92402da3b24376905380afc29 # v4.1.6 - name: Install Go uses: actions/setup-go@cdcb36043654635271a94b9a6d1392de5bb323a7 # v5.0.1 with: From 360ae579460fab69e9939599af85c8f255a59007 Mon Sep 17 00:00:00 2001 From: Liam Cervante Date: Wed, 19 Jun 2024 13:34:52 +0200 Subject: [PATCH 16/23] prepare for v2.21.0 release --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e8d06e47..f81110dc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ # HCL Changelog -## v2.21.0 (Unreleased) +## v2.21.0 (June 19, 2024) ### Enhancements From 117baa8bf53bc2050fe5e0f2ab85c4ef9341d20e Mon Sep 17 00:00:00 2001 From: Ansgar Mertens Date: Wed, 21 Aug 2024 15:32:07 +0200 Subject: [PATCH 17/23] feat: return an ExprSyntaxError for invalid references that end in a dot (commonly occurs in editors for completions) Detect value expressions of the ExprSyntaxError type when parsing object constructor expressions and use them to add an item to the result even though we skip parsing the object due to recovery after the invalid expression. This allows the Terraform language server to support completions for object attributes after a dot was typed. --- hclsyntax/parser.go | 23 ++++++++++-- hclsyntax/parser_test.go | 81 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 101 insertions(+), 3 deletions(-) diff --git a/hclsyntax/parser.go b/hclsyntax/parser.go index ce96ae35..fec7861a 100644 --- a/hclsyntax/parser.go +++ b/hclsyntax/parser.go @@ -811,9 +811,16 @@ Traversal: // will probably be misparsed until we hit something that // allows us to re-sync. // - // We will probably need to do something better here eventually - // in order to support autocomplete triggered by typing a - // period. + // Returning an ExprSyntaxError allows us to pass more information + // about the invalid expression to the caller, which can then + // use this for example for completions that happen after typing + // a dot in an editor. + ret = &ExprSyntaxError{ + Placeholder: cty.DynamicVal, + ParseDiags: diags, + SrcRange: hcl.RangeBetween(from.Range(), dot.Range), + } + p.setRecovery() } @@ -1516,6 +1523,16 @@ func (p *parser) parseObjectCons() (Expression, hcl.Diagnostics) { diags = append(diags, valueDiags...) if p.recovery && valueDiags.HasErrors() { + // If the value is an ExprSyntaxError, we can add an item with it, even though we will recover afterwards + // This allows downstream consumers to still retrieve this first invalid item, even though following items + // won't be parsed. This is useful for supplying completions. + if exprSyntaxError, ok := value.(*ExprSyntaxError); ok { + items = append(items, ObjectConsItem{ + KeyExpr: key, + ValueExpr: exprSyntaxError, + }) + } + // If expression parsing failed then we are probably in a strange // place in the token stream, so we'll bail out and try to reset // to after our closing brace to allow parsing to continue. diff --git a/hclsyntax/parser_test.go b/hclsyntax/parser_test.go index 10825f15..66f1308f 100644 --- a/hclsyntax/parser_test.go +++ b/hclsyntax/parser_test.go @@ -2672,6 +2672,87 @@ block "valid" {} }, }, }, + { + "a = { b = c. }", + 1, + &Body{ + Attributes: Attributes{ + "a": { + Name: "a", + Expr: &ObjectConsExpr{ + Items: []ObjectConsItem{ + { + KeyExpr: &ObjectConsKeyExpr{ + Wrapped: &ScopeTraversalExpr{ + Traversal: hcl.Traversal{ + hcl.TraverseRoot{ + Name: "b", + SrcRange: hcl.Range{ + Start: hcl.Pos{Line: 1, Column: 7, Byte: 6}, + End: hcl.Pos{Line: 1, Column: 8, Byte: 7}, + }, + }, + }, + SrcRange: hcl.Range{ + Start: hcl.Pos{Line: 1, Column: 7, Byte: 6}, + End: hcl.Pos{Line: 1, Column: 8, Byte: 7}, + }, + }, + }, + ValueExpr: &ExprSyntaxError{ + Placeholder: cty.DynamicVal, + SrcRange: hcl.Range{ + Start: hcl.Pos{Line: 1, Column: 11, Byte: 10}, + End: hcl.Pos{Line: 1, Column: 13, Byte: 12}, + }, + ParseDiags: hcl.Diagnostics{ + { + Severity: hcl.DiagError, + Summary: "Invalid attribute name", + Detail: "An attribute name is required after a dot.", + Subject: &hcl.Range{ + Start: hcl.Pos{Line: 1, Column: 14, Byte: 13}, + End: hcl.Pos{Line: 1, Column: 15, Byte: 14}, + }, + }, + }, + }, + }, + }, + SrcRange: hcl.Range{ + Start: hcl.Pos{Line: 1, Column: 5, Byte: 4}, + End: hcl.Pos{Line: 1, Column: 15, Byte: 14}, + }, + OpenRange: hcl.Range{ + Start: hcl.Pos{Line: 1, Column: 5, Byte: 4}, + End: hcl.Pos{Line: 1, Column: 6, Byte: 5}, + }, + }, + SrcRange: hcl.Range{ + Start: hcl.Pos{Line: 1, Column: 1, Byte: 0}, + End: hcl.Pos{Line: 1, Column: 15, Byte: 14}, + }, + NameRange: hcl.Range{ + Start: hcl.Pos{Line: 1, Column: 1, Byte: 0}, + End: hcl.Pos{Line: 1, Column: 2, Byte: 1}, + }, + EqualsRange: hcl.Range{ + 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: 1, Column: 15, Byte: 14}, + }, + EndRange: hcl.Range{ + Start: hcl.Pos{Line: 1, Column: 15, Byte: 14}, + End: hcl.Pos{Line: 1, Column: 15, Byte: 14}, + }, + }, + }, } for _, test := range tests { From f68c58fb4d7c83cd334f3eebe10f045f541e435e Mon Sep 17 00:00:00 2001 From: Ansgar Mertens Date: Thu, 22 Aug 2024 11:01:00 +0200 Subject: [PATCH 18/23] Update Changelog.md --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f81110dc..003fbdb0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # HCL Changelog +## v2.22.0 (Unreleased) + +### Enhancements + +* feat: return an ExprSyntaxError for invalid references that end in a dot ([#692](https://github.com/hashicorp/hcl/pull/692)) + ## v2.21.0 (June 19, 2024) ### Enhancements From 141e3dbf5956904ad1920241148b406d69b2aae0 Mon Sep 17 00:00:00 2001 From: Ansgar Mertens Date: Mon, 26 Aug 2024 14:41:43 +0200 Subject: [PATCH 19/23] Prepare for v2.22.0 release --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 003fbdb0..b2ff2631 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ # HCL Changelog -## v2.22.0 (Unreleased) +## v2.22.0 (August 26, 2024) ### Enhancements From e2f43f4c9722c9a1e7c23b0af4c41ed9f7cab2c5 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Tue, 24 Sep 2024 12:45:20 -0400 Subject: [PATCH 20/23] Preserve marks when traversing unknown values When traversing an unknown value or a DynamicVal, the marks from that initial value must be preserved for HCL Index and GetAttr operations. This mirrors the behavior of GetAttr and Index when used directly the underlying cty values. --- ops.go | 16 ++++---- ops_test.go | 109 +++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 116 insertions(+), 9 deletions(-) diff --git a/ops.go b/ops.go index bdf23614..3cd7b205 100644 --- a/ops.go +++ b/ops.go @@ -49,7 +49,7 @@ func Index(collection, key cty.Value, srcRange *Range) (cty.Value, Diagnostics) ty := collection.Type() kty := key.Type() if kty == cty.DynamicPseudoType || ty == cty.DynamicPseudoType { - return cty.DynamicVal, nil + return cty.DynamicVal.WithSameMarks(collection), nil } switch { @@ -87,9 +87,9 @@ func Index(collection, key cty.Value, srcRange *Range) (cty.Value, Diagnostics) has, _ := collection.HasIndex(key).Unmark() if !has.IsKnown() { if ty.IsTupleType() { - return cty.DynamicVal, nil + return cty.DynamicVal.WithSameMarks(collection), nil } else { - return cty.UnknownVal(ty.ElementType()), nil + return cty.UnknownVal(ty.ElementType()).WithSameMarks(collection), nil } } if has.False() { @@ -196,10 +196,10 @@ func Index(collection, key cty.Value, srcRange *Range) (cty.Value, Diagnostics) } } if !collection.IsKnown() { - return cty.DynamicVal, nil + return cty.DynamicVal.WithSameMarks(collection), nil } if !key.IsKnown() { - return cty.DynamicVal, nil + return cty.DynamicVal.WithSameMarks(collection), nil } key, _ = key.Unmark() @@ -291,13 +291,13 @@ func GetAttr(obj cty.Value, attrName string, srcRange *Range) (cty.Value, Diagno } if !obj.IsKnown() { - return cty.UnknownVal(ty.AttributeType(attrName)), nil + return cty.UnknownVal(ty.AttributeType(attrName)).WithSameMarks(obj), nil } return obj.GetAttr(attrName), nil case ty.IsMapType(): if !obj.IsKnown() { - return cty.UnknownVal(ty.ElementType()), nil + return cty.UnknownVal(ty.ElementType()).WithSameMarks(obj), nil } idx := cty.StringVal(attrName) @@ -319,7 +319,7 @@ func GetAttr(obj cty.Value, attrName string, srcRange *Range) (cty.Value, Diagno return obj.Index(idx), nil case ty == cty.DynamicPseudoType: - return cty.DynamicVal, nil + return cty.DynamicVal.WithSameMarks(obj), nil case ty.IsListType() && ty.ElementType().IsObjectType(): // It seems a common mistake to try to access attributes on a whole // list of objects rather than on a specific individual element, so diff --git a/ops_test.go b/ops_test.go index 7aabd7a2..b7b35ca7 100644 --- a/ops_test.go +++ b/ops_test.go @@ -249,6 +249,113 @@ func TestApplyPath(t *testing.T) { cty.NilVal, `Attempt to get attribute from null value: This value is null, so it does not have any attributes.`, }, + + // Marks should be retained during index and getattr ops, even when + // types and values are unknown. This reflects the same behavior when + // using cty to directly call GetAttr and Index methods. + { + cty.DynamicVal.Mark("marked"), + (cty.Path)(nil).GetAttr("foo"), + cty.DynamicVal.Mark("marked"), + ``, + }, + { + cty.ObjectVal(map[string]cty.Value{ + "foo": cty.StringVal("should be marked"), + }).Mark("marked"), + (cty.Path)(nil).GetAttr("foo"), + cty.StringVal("should be marked").Mark("marked"), + ``, + }, + { + cty.UnknownVal(cty.Object(map[string]cty.Type{ + "foo": cty.DynamicPseudoType, + })).Mark("marked"), + (cty.Path)(nil).GetAttr("foo"), + cty.DynamicVal.Mark("marked"), + ``, + }, + { + cty.DynamicVal.Mark("marked"), + (cty.Path)(nil).Index(cty.StringVal("foo")), + cty.DynamicVal.Mark("marked"), + ``, + }, + { + cty.ObjectVal(map[string]cty.Value{ + "foo": cty.StringVal("should be marked"), + }).Mark("marked"), + (cty.Path)(nil).Index(cty.StringVal("foo")), + cty.StringVal("should be marked").Mark("marked"), + ``, + }, + { + cty.UnknownVal(cty.Object(map[string]cty.Type{ + "foo": cty.DynamicPseudoType, + })).Mark("marked"), + (cty.Path)(nil).Index(cty.StringVal("foo")), + cty.DynamicVal.Mark("marked"), + ``, + }, + { + cty.DynamicVal.Mark("marked"), + (cty.Path)(nil).Index(cty.NumberIntVal(0)), + cty.DynamicVal.Mark("marked"), + ``, + }, + { + cty.ListVal([]cty.Value{cty.StringVal("should be marked")}).Mark("marked"), + (cty.Path)(nil).Index(cty.NumberIntVal(0)), + cty.StringVal("should be marked").Mark("marked"), + ``, + }, + { + cty.UnknownVal(cty.List(cty.String)).Mark("marked"), + (cty.Path)(nil).Index(cty.NumberIntVal(0)), + cty.UnknownVal(cty.String).Mark("marked"), + ``, + }, + + { + cty.DynamicVal.Mark("marked"), + (cty.Path)(nil).Index(cty.UnknownVal(cty.String)), + cty.DynamicVal.Mark("marked"), + ``, + }, + { + cty.ObjectVal(map[string]cty.Value{ + "foo": cty.StringVal("should be marked"), + }).Mark("marked"), + (cty.Path)(nil).Index(cty.UnknownVal(cty.String)), + cty.DynamicVal.Mark("marked"), + ``, + }, + { + cty.UnknownVal(cty.Object(map[string]cty.Type{ + "foo": cty.DynamicPseudoType, + })).Mark("marked"), + (cty.Path)(nil).Index(cty.UnknownVal(cty.String)), + cty.DynamicVal.Mark("marked"), + ``, + }, + { + cty.DynamicVal.Mark("marked"), + (cty.Path)(nil).Index(cty.UnknownVal(cty.Number)), + cty.DynamicVal.Mark("marked"), + ``, + }, + { + cty.ListVal([]cty.Value{cty.StringVal("should be marked")}).Mark("marked"), + (cty.Path)(nil).Index(cty.UnknownVal(cty.Number)), + cty.UnknownVal(cty.String).Mark("marked"), + ``, + }, + { + cty.UnknownVal(cty.List(cty.String)).Mark("marked"), + (cty.Path)(nil).Index(cty.UnknownVal(cty.Number)), + cty.UnknownVal(cty.String).Mark("marked"), + ``, + }, } for _, test := range tests { @@ -257,7 +364,7 @@ func TestApplyPath(t *testing.T) { t.Logf("testing ApplyPath\nstart: %#v\npath: %#v", test.Start, test.Path) for _, diag := range diags { - t.Logf(diag.Error()) + t.Log(diag.Error()) } if test.WantErr != "" { From 1dfc778b7de90d03c632055cd1b8b65a3f69f66d Mon Sep 17 00:00:00 2001 From: Bruno Schaatsbergen Date: Tue, 1 Oct 2024 15:24:47 +0200 Subject: [PATCH 21/23] docs: fix typo Signed-off-by: Bruno Schaatsbergen --- spec.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec.md b/spec.md index 97ef6131..3dfe421d 100644 --- a/spec.md +++ b/spec.md @@ -96,7 +96,7 @@ of the implementation language. ### _Dynamic Attributes_ Processing The _schema-driven_ processing model is useful when the expected structure -of a body is known a priori by the calling application. Some blocks are +of a body is known prior to the calling application. Some blocks are instead more free-form, such as a user-provided set of arbitrary key/value pairs. From 65971e8952c195172870bd11cf1bb34be6bd718d Mon Sep 17 00:00:00 2001 From: Bruno Schaatsbergen Date: Tue, 1 Oct 2024 15:30:01 +0200 Subject: [PATCH 22/23] docs: use 'by' instead of 'prior to' --- spec.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec.md b/spec.md index 3dfe421d..d52ed70b 100644 --- a/spec.md +++ b/spec.md @@ -96,7 +96,7 @@ of the implementation language. ### _Dynamic Attributes_ Processing The _schema-driven_ processing model is useful when the expected structure -of a body is known prior to the calling application. Some blocks are +of a body is known by the calling application. Some blocks are instead more free-form, such as a user-provided set of arbitrary key/value pairs. From 3883feb0e06458153e8a1b56566582b6725330d2 Mon Sep 17 00:00:00 2001 From: Calmon Ribeiro Date: Tue, 8 Oct 2024 14:40:59 -0300 Subject: [PATCH 23/23] docs(ext/dynblock): recursive function call typo in detecting variables (#686) --- ext/dynblock/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/dynblock/README.md b/ext/dynblock/README.md index f59ce92e..1cee2b93 100644 --- a/ext/dynblock/README.md +++ b/ext/dynblock/README.md @@ -134,7 +134,7 @@ func walkVariables(node dynblock.WalkVariablesNode, schema *hcl.BodySchema) []hc panic(fmt.Errorf("can't find schema for unknown block type %q", child.BlockTypeName)) } - vars = append(vars, testWalkAndAccumVars(child.Node, childSchema)...) + vars = append(vars, walkVariables(child.Node, childSchema)...) } } ```