From 987fe04c7f2fc7a9babe7f112c3d7ecffffb3365 Mon Sep 17 00:00:00 2001 From: Liam Cervante Date: Mon, 31 Oct 2022 17:11:07 +0100 Subject: [PATCH 1/2] Update go-cty and improve documentation for optional and default attributes --- ext/typeexpr/README.md | 21 ++++++++++++++++++++ ext/typeexpr/defaults.go | 6 +++--- ext/typeexpr/public.go | 4 ---- go.mod | 4 ++-- go.sum | 23 ++++------------------ hclsyntax/expression_test.go | 38 ++++++++++++++++++++++++++++-------- 6 files changed, 60 insertions(+), 36 deletions(-) diff --git a/ext/typeexpr/README.md b/ext/typeexpr/README.md index 058f1e3d..c0fa6ab8 100644 --- a/ext/typeexpr/README.md +++ b/ext/typeexpr/README.md @@ -66,6 +66,27 @@ types with weird attributes generally show up only from arbitrary object constructors in configuration files, which are usually treated either as maps or as the dynamic pseudo-type. +### Optional Object Attributes + +As part of object expressions attributes can be marked as optional. Missing +object attributes would typically result in an error when type constraints are +validated or used. Optional missing attributes, however, would not result in an +error. The `cty` ["convert" function](#the-convert-cty-function) will populate +missing optional attributes with null values. + +For example: + +* `object({name=string,age=optional(number)})` + +Optional attributes can also be specified with default values. The +`TypeConstraintWithDefaults` function will return a `Defaults` object that can +be used to populate missing optional attributes with defaults in a given +`cty.Value`. + +For example: + +* `object({name=string,age=optional(number, 0)})` + ## Type Constraints as Values Along with defining a convention for writing down types using HCL expression diff --git a/ext/typeexpr/defaults.go b/ext/typeexpr/defaults.go index 851c72fb..4e960579 100644 --- a/ext/typeexpr/defaults.go +++ b/ext/typeexpr/defaults.go @@ -6,7 +6,7 @@ import ( // Defaults represents a type tree which may contain default values for // optional object attributes at any level. This is used to apply nested -// defaults to an input value before converting it to the concrete type. +// defaults to a given cty.Value. type Defaults struct { // Type of the node for which these defaults apply. This is necessary in // order to determine how to inspect the Defaults and Children collections. @@ -28,8 +28,8 @@ type Defaults struct { // Apply walks the given value, applying specified defaults wherever optional // attributes are missing. The input and output values may have different -// types, and the result may still require type conversion to the final desired -// type. +// types, to avoid this the input value should be converted into the desired +// type first. // // This function is permissive and does not report errors, assuming that the // caller will have better context to report useful type conversion failure diff --git a/ext/typeexpr/public.go b/ext/typeexpr/public.go index 82f215c0..f2e187ef 100644 --- a/ext/typeexpr/public.go +++ b/ext/typeexpr/public.go @@ -35,10 +35,6 @@ func TypeConstraint(expr hcl.Expression) (cty.Type, hcl.Diagnostics) { // constraint which may include default values for object attributes. If // successful both the resulting type and corresponding defaults are returned. // If unsuccessful, error diagnostics are returned. -// -// When using this function, defaults should be applied to the input value -// before type conversion, to ensure that objects with missing attributes have -// default values populated. func TypeConstraintWithDefaults(expr hcl.Expression) (cty.Type, *Defaults, hcl.Diagnostics) { return getType(expr, true, true) } diff --git a/go.mod b/go.mod index 8f7c79fc..0d22ec3d 100644 --- a/go.mod +++ b/go.mod @@ -14,7 +14,7 @@ require ( 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.8.0 + github.com/zclconf/go-cty v1.12.0 github.com/zclconf/go-cty-debug v0.0.0-20191215020915-b22d67c1ba0b golang.org/x/crypto v0.0.0-20220517005047-85d78b3ac167 ) @@ -25,5 +25,5 @@ require ( github.com/stretchr/testify v1.2.2 // indirect golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1 // indirect golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1 // indirect - golang.org/x/text v0.3.6 // indirect + golang.org/x/text v0.3.7 // indirect ) diff --git a/go.sum b/go.sum index 2d2ddb33..ac1a7146 100644 --- a/go.sum +++ b/go.sum @@ -2,7 +2,6 @@ github.com/agext/levenshtein v1.2.1 h1:QmvMAjj2aEICytGiWzmxoE0x2KZvE0fvmqMOfy2tj 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 h1:rRmlIsPEEhUTIKQb7T++Nz/A5Q6C9IuX2wFoYVvnCs0= 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= @@ -11,8 +10,6 @@ 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/golang/protobuf v1.3.1/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= -github.com/golang/protobuf v1.3.4/go.mod h1:vzj43D7+SQXF/4pzW/hwtAqwc6iTitCiVSaWz5lYuqw= 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= @@ -33,34 +30,22 @@ github.com/spf13/pflag v1.0.2/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnIn 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/vmihailenco/msgpack/v4 v4.3.12/go.mod h1:gborTTJjAo/GWTqqRjrLCn9pgNN+NXzzngzBKDPIqw4= -github.com/vmihailenco/tagparser v0.1.1/go.mod h1:OeAg3pn3UbLjkWt+rN9oFYB6u/cQgqMEUPoW2WPyhdI= github.com/zclconf/go-cty v1.2.0/go.mod h1:hOPWgoHbaTUnI5k4D2ld+GRpFJSCe6bCM7m1q/N4PQ8= -github.com/zclconf/go-cty v1.8.0 h1:s4AvqaeQzJIu3ndv4gVIhplVD0krU+bgrcLSVUnaWuA= -github.com/zclconf/go-cty v1.8.0/go.mod h1:vVKLxnk3puL4qRAv72AO+W99LUD4da90g3uUAzyuvAk= +github.com/zclconf/go-cty v1.12.0 h1:F5E/vbilcrCtat9sYcEjlwwg1mDqbRTjyXR57nnx5sc= +github.com/zclconf/go-cty v1.12.0/go.mod h1:s9IfD1LK5ccNMSWCVFCE2rJfHiZgi7JijgeWIMfhLvA= 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= -golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= 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/net v0.0.0-20180811021610-c39426892332/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= -golang.org/x/net v0.0.0-20190603091049-60506f45cf65/go.mod h1:HSz+uSET+XFnRR8LxR5pz3Of3rY3CfYBVs4xY44aLks= -golang.org/x/net v0.0.0-20200301022130-244492dfa37a/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= -golang.org/x/net v0.0.0-20211112202133-69e39bad7dc2/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20210423082822-04245dca01da/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1 h1:SrN+KX8Art/Sf4HNj6Zcz06G7VEz+7w9tdXTPOZ7+l4= golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1 h1:v+OssWQX+hTHEmOBgwxdZxK4zHq3yOs8F9J7mk0PY8E= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= -golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk= -golang.org/x/text v0.3.5/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= -golang.org/x/text v0.3.6 h1:aRYxNxv6iGQlyVaZmk6ZgYEDa+Jg18DxebPSrd6bg1M= -golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= -golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= +golang.org/x/text v0.3.7 h1:olpwvP2KacW1ZWvsR7uQhoyTYvKAupfQrRGBFM352Gk= +golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= google.golang.org/appengine v1.1.0/go.mod h1:EbEs0AVv82hx2wNQdGPgUI5lhzA/G0D9YwlJXL52JkM= -google.golang.org/appengine v1.6.5/go.mod h1:8WjMMxjGQR8xUklV/ARdw2HLXBOI7O7uCIDZVag1xfc= gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= diff --git a/hclsyntax/expression_test.go b/hclsyntax/expression_test.go index 77d959fb..d22406fc 100644 --- a/hclsyntax/expression_test.go +++ b/hclsyntax/expression_test.go @@ -1847,6 +1847,28 @@ EOT cty.NumberIntVal(1), 0, }, + { // auto-converts collection types + `true ? listOf1Tuple : listOf0Tuple`, + &hcl.EvalContext{ + Variables: map[string]cty.Value{ + "listOf1Tuple": cty.ListVal([]cty.Value{cty.TupleVal([]cty.Value{cty.True})}), + "listOf0Tuple": cty.ListVal([]cty.Value{cty.EmptyTupleVal}), + }, + }, + cty.ListVal([]cty.Value{cty.ListVal([]cty.Value{cty.True})}), + 0, + }, + { + `true ? setOf1Tuple : setOf0Tuple`, + &hcl.EvalContext{ + Variables: map[string]cty.Value{ + "setOf1Tuple": cty.SetVal([]cty.Value{cty.TupleVal([]cty.Value{cty.True})}), + "setOf0Tuple": cty.SetVal([]cty.Value{cty.EmptyTupleVal}), + }, + }, + cty.SetVal([]cty.Value{cty.ListVal([]cty.Value{cty.True})}), + 0, + }, { // marked argument expansion `min(xs...)`, &hcl.EvalContext{ @@ -1937,26 +1959,26 @@ func TestExpressionErrorMessages(t *testing.T) { "The true and false result expressions must have consistent types. The 'false' value includes object attribute \"b\", which is absent in the 'true' value.", }, { - "true ? listOf1Tuple : listOf0Tuple", + "true ? listOf2Tuple : listOf1Tuple", &hcl.EvalContext{ Variables: map[string]cty.Value{ + "listOf2Tuple": cty.ListVal([]cty.Value{cty.TupleVal([]cty.Value{cty.True, cty.Zero})}), "listOf1Tuple": cty.ListVal([]cty.Value{cty.TupleVal([]cty.Value{cty.True})}), - "listOf0Tuple": cty.ListVal([]cty.Value{cty.EmptyTupleVal}), }, }, "Inconsistent conditional result types", - "The true and false result expressions must have consistent types. Mismatched list element types: The 'true' tuple has length 1, but the 'false' tuple has length 0.", + "The true and false result expressions must have consistent types. Mismatched list element types: The 'true' tuple has length 2, but the 'false' tuple has length 1.", }, { - "true ? setOf1Tuple : setOf0Tuple", + "true ? setOf2Tuple : setOf1Tuple", &hcl.EvalContext{ Variables: map[string]cty.Value{ + "setOf2Tuple": cty.SetVal([]cty.Value{cty.TupleVal([]cty.Value{cty.True, cty.Zero})}), "setOf1Tuple": cty.SetVal([]cty.Value{cty.TupleVal([]cty.Value{cty.True})}), - "setOf0Tuple": cty.SetVal([]cty.Value{cty.EmptyTupleVal}), }, }, "Inconsistent conditional result types", - "The true and false result expressions must have consistent types. Mismatched set element types: The 'true' tuple has length 1, but the 'false' tuple has length 0.", + "The true and false result expressions must have consistent types. Mismatched set element types: The 'true' tuple has length 2, but the 'false' tuple has length 1.", }, { "true ? mapOf1Tuple : mapOf2Tuple", @@ -1970,11 +1992,11 @@ func TestExpressionErrorMessages(t *testing.T) { "The true and false result expressions must have consistent types. Mismatched map element types: The 'true' tuple has length 1, but the 'false' tuple has length 2.", }, { - "true ? listOfListOf1Tuple : listOfListOf0Tuple", + "true ? listOfListOf2Tuple : listOfListOf1Tuple", &hcl.EvalContext{ Variables: map[string]cty.Value{ + "listOfListOf2Tuple": cty.ListVal([]cty.Value{cty.ListVal([]cty.Value{cty.TupleVal([]cty.Value{cty.True, cty.Zero})})}), "listOfListOf1Tuple": cty.ListVal([]cty.Value{cty.ListVal([]cty.Value{cty.TupleVal([]cty.Value{cty.True})})}), - "listOfListOf0Tuple": cty.ListVal([]cty.Value{cty.ListVal([]cty.Value{cty.EmptyTupleVal})}), }, }, "Inconsistent conditional result types", From dc40a38475ecd775adfa53c262bf8fcebc8a3ecb Mon Sep 17 00:00:00 2001 From: Liam Cervante Date: Wed, 2 Nov 2022 09:12:56 +0100 Subject: [PATCH 2/2] Add comment clarifying why the given test cases are failing --- hclsyntax/expression_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/hclsyntax/expression_test.go b/hclsyntax/expression_test.go index d22406fc..8a15d115 100644 --- a/hclsyntax/expression_test.go +++ b/hclsyntax/expression_test.go @@ -1959,6 +1959,10 @@ func TestExpressionErrorMessages(t *testing.T) { "The true and false result expressions must have consistent types. The 'false' value includes object attribute \"b\", which is absent in the 'true' value.", }, { + // Failing cases for automatic collection conversions. HCL and cty + // will attempt to unify tuples into lists. We have to make sure + // the tuple inner types have no common base type, so we mix and + // match booleans and numbers and validate the error messages. "true ? listOf2Tuple : listOf1Tuple", &hcl.EvalContext{ Variables: map[string]cty.Value{