cty Refinements: Unknown values with tighter range

ext/typeexpr/defaults.go

@@ -95,6 +95,9 @@ func (d *Defaults) apply(v cty.Value) cty.Value {
 				}
 				values[key] = defaultValue
 			}
+			if defaultRng := defaultValue.Range(); defaultRng.DefinitelyNotNull() {
+				values[key] = values[key].RefineNotNull()
+			}
 		}
 
 		if v.Type().IsMapType() {

ext/typeexpr/defaults_test.go

@@ -830,6 +830,74 @@ func TestDefaults_Apply(t *testing.T) {
 				}),
 			}),
 		},
+		"optional attribute with a default can never be null": {
+			defaults: &Defaults{
+				Type: cty.ObjectWithOptionalAttrs(map[string]cty.Type{
+					"foo": cty.String,
+				}, []string{"foo"}),
+				DefaultValues: map[string]cty.Value{
+					"foo": cty.StringVal("bar"), // Important: default is non-null
+				},
+			},
+			value: cty.ObjectVal(map[string]cty.Value{
+				"foo": cty.UnknownVal(cty.String), // could potentially be null once known
+			}),
+			want: cty.ObjectVal(map[string]cty.Value{
+				// Because the default isn't null we can guarantee that the
+				// result cannot be null even if the given value turns out to be.
+				"foo": cty.UnknownVal(cty.String).RefineNotNull(),
+			}),
+		},
+		"optional attribute with a null default could be null": {
+			defaults: &Defaults{
+				Type: cty.ObjectWithOptionalAttrs(map[string]cty.Type{
+					"foo": cty.String,
+				}, []string{"foo"}),
+				DefaultValues: map[string]cty.Value{
+					"foo": cty.NullVal(cty.String), // Important: default is null
+				},
+			},
+			value: cty.ObjectVal(map[string]cty.Value{
+				"foo": cty.UnknownVal(cty.String), // could potentially be null once known
+			}),
+			want: cty.ObjectVal(map[string]cty.Value{
+				// The default value is itself null, so this result is nullable.
+				"foo": cty.UnknownVal(cty.String),
+			}),
+		},
+		"optional attribute with no default could be null": {
+			defaults: &Defaults{
+				Type: cty.ObjectWithOptionalAttrs(map[string]cty.Type{
+					"foo": cty.String,
+				}, []string{"foo"}),
+			},
+			value: cty.ObjectVal(map[string]cty.Value{
+				"foo": cty.UnknownVal(cty.String), // could potentially be null once known
+			}),
+			want: cty.ObjectVal(map[string]cty.Value{
+				// The default value is itself null, so this result is nullable.
+				"foo": cty.UnknownVal(cty.String),
+			}),
+		},
+		"optional attribute with non-null unknown value cannot be null": {
+			defaults: &Defaults{
+				Type: cty.ObjectWithOptionalAttrs(map[string]cty.Type{
+					"foo": cty.String,
+				}, []string{"foo"}),
+				DefaultValues: map[string]cty.Value{
+					"foo": cty.NullVal(cty.String), // Important: default is null
+				},
+			},
+			value: cty.ObjectVal(map[string]cty.Value{
+				"foo": cty.UnknownVal(cty.String).RefineNotNull(),
+			}),
+			want: cty.ObjectVal(map[string]cty.Value{
+				// If the input is guaranteed not null then the default
+				// value can't possibly be selected, and so the result can
+				// also not be null.
+				"foo": cty.UnknownVal(cty.String).RefineNotNull(),
+			}),
+		},
 	}
 
 	for name, tc := range testCases {

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.12.1
+	github.com/zclconf/go-cty v1.13.0
 	github.com/zclconf/go-cty-debug v0.0.0-20191215020915-b22d67c1ba0b
 	golang.org/x/crypto v0.0.0-20220517005047-85d78b3ac167
 )
@@ -23,7 +23,7 @@ require (
 	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/sys v0.0.0-20210615035016-665e8c7367d1 // indirect
+	golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f // indirect
 	golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1 // indirect
-	golang.org/x/text v0.3.7 // indirect
+	golang.org/x/text v0.3.8 // indirect
 )

go.sum

@@ -31,23 +31,21 @@ github.com/stretchr/testify v1.2.2 h1:bSDNvY7ZPG5RlJ8otE/7V6gMiyenm9RtJ7IUVIAoJ1
 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.12.0 h1:F5E/vbilcrCtat9sYcEjlwwg1mDqbRTjyXR57nnx5sc=
-github.com/zclconf/go-cty v1.12.0/go.mod h1:s9IfD1LK5ccNMSWCVFCE2rJfHiZgi7JijgeWIMfhLvA=
-github.com/zclconf/go-cty v1.12.1 h1:PcupnljUm9EIvbgSHQnHhUr3fO6oFmkOrvs2BAFNXXY=
-github.com/zclconf/go-cty v1.12.1/go.mod h1:s9IfD1LK5ccNMSWCVFCE2rJfHiZgi7JijgeWIMfhLvA=
+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=
 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/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/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/sys v0.0.0-20220722155257-8c9f86f7a55f h1:v4INt8xihDGvnrfjMDVXGxw9wrfxYyCjk0KbXjhR55s=
+golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/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.7 h1:olpwvP2KacW1ZWvsR7uQhoyTYvKAupfQrRGBFM352Gk=
-golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ=
+golang.org/x/text v0.3.8 h1:nAL+RVCQ9uMn3vJZbV+MRnydTJFPf8qqY42YiA6MrqY=
+golang.org/x/text v0.3.8/go.mod h1:E6s5w1FMmriuDzIBO73fBruAKo1PCIq6d2Q6DHfQ8WQ=
 google.golang.org/appengine v1.1.0/go.mod h1:EbEs0AVv82hx2wNQdGPgUI5lhzA/G0D9YwlJXL52JkM=
 gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=

hcldec/spec.go

@@ -1606,7 +1606,52 @@ func (s *TransformFuncSpec) sourceRange(content *hcl.BodyContent, blockLabels []
 	return s.Wrapped.sourceRange(content, blockLabels)
 }
 
-// ValidateFuncSpec is a spec that allows for extended
+// RefineValueSpec is a spec that wraps another and applies a fixed set of [cty]
+// value refinements to whatever value it produces.
+//
+// Refinements serve to constrain the range of any unknown values, and act as
+// assertions for known values by panicking if the final value does not meet
+// the refinement. Therefore applications using this spec must guarantee that
+// any value passing through the RefineValueSpec will always be consistent with
+// the refinements; if not then that is a bug in the application.
+//
+// The wrapped spec should typically be a [ValidateSpec], a [TransformFuncSpec],
+// or some other adapter that guarantees that the inner result cannot possibly
+// violate the refinements.
+type RefineValueSpec struct {
+	Wrapped Spec
+
+	// Refine is a function which accepts a builder for a refinement in
+	// progress and uses the builder pattern to add extra refinements to it,
+	// finally returning the same builder with those modifications applied.
+	Refine func(*cty.RefinementBuilder) *cty.RefinementBuilder
+}
+
+func (s *RefineValueSpec) visitSameBodyChildren(cb visitFunc) {
+	cb(s.Wrapped)
+}
+
+func (s *RefineValueSpec) decode(content *hcl.BodyContent, blockLabels []blockLabel, ctx *hcl.EvalContext) (cty.Value, hcl.Diagnostics) {
+	wrappedVal, diags := s.Wrapped.decode(content, blockLabels, ctx)
+	if diags.HasErrors() {
+		// We won't try to run our function in this case, because it'll probably
+		// generate confusing additional errors that will distract from the
+		// root cause.
+		return cty.UnknownVal(s.impliedType()), diags
+	}
+
+	return wrappedVal.RefineWith(s.Refine), diags
+}
+
+func (s *RefineValueSpec) impliedType() cty.Type {
+	return s.Wrapped.impliedType()
+}
+
+func (s *RefineValueSpec) sourceRange(content *hcl.BodyContent, blockLabels []blockLabel) hcl.Range {
+	return s.Wrapped.sourceRange(content, blockLabels)
+}
+
+// ValidateSpec is a spec that allows for extended
 // developer-defined validation. The validation function receives the
 // result of the wrapped spec.
 //

hcldec/spec_test.go

@@ -9,6 +9,8 @@ import (
 	"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"
 
 	"github.com/hashicorp/hcl/v2"
@@ -210,3 +212,72 @@ foo = "invalid"
 		})
 	}
 }
+
+func TestRefineValueSpec(t *testing.T) {
+	config := `
+foo = "hello"
+bar = unk
+`
+
+	f, diags := hclsyntax.ParseConfig([]byte(config), "", hcl.InitialPos)
+	if diags.HasErrors() {
+		t.Fatal(diags.Error())
+	}
+
+	attrSpec := func(name string) Spec {
+		return &RefineValueSpec{
+			// RefineValueSpec should typically have a ValidateSpec wrapped
+			// inside it to catch any values that are outside of the required
+			// range and return a helpful error message about it. In this
+			// case our refinement is .NotNull so the validation function
+			// must reject null values.
+			Wrapped: &ValidateSpec{
+				Wrapped: &AttrSpec{
+					Name:     name,
+					Required: true,
+					Type:     cty.String,
+				},
+				Func: func(value cty.Value) hcl.Diagnostics {
+					var diags hcl.Diagnostics
+					if value.IsNull() {
+						diags = diags.Append(&hcl.Diagnostic{
+							Severity: hcl.DiagError,
+							Summary:  "Cannot be null",
+							Detail:   "Argument is required.",
+						})
+					}
+					return diags
+				},
+			},
+			Refine: func(rb *cty.RefinementBuilder) *cty.RefinementBuilder {
+				return rb.NotNull()
+			},
+		}
+	}
+	spec := &ObjectSpec{
+		"foo": attrSpec("foo"),
+		"bar": attrSpec("bar"),
+	}
+
+	got, diags := Decode(f.Body, spec, &hcl.EvalContext{
+		Variables: map[string]cty.Value{
+			"unk": cty.UnknownVal(cty.String),
+		},
+	})
+	if diags.HasErrors() {
+		t.Fatal(diags.Error())
+	}
+
+	want := cty.ObjectVal(map[string]cty.Value{
+		// This argument had a known value, so it's unchanged but the
+		// RefineValueSpec still checks that it isn't null to catch
+		// bugs in the application's validation function.
+		"foo": cty.StringVal("hello"),
+
+		// The final value of bar is unknown but refined as non-null.
+		"bar": cty.UnknownVal(cty.String).RefineNotNull(),
+	})
+	if diff := cmp.Diff(want, got, ctydebug.CmpOptions); diff != "" {
+		t.Errorf("wrong result\n%s", diff)
+	}
+}

hclsyntax/expression.go

@@ -696,7 +696,59 @@ func (e *ConditionalExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnostic
 		return cty.UnknownVal(resultType), diags
 	}
 	if !condResult.IsKnown() {
-		return cty.UnknownVal(resultType), diags
+		// We might be able to offer a refined range for the result based on
+		// the two possible outcomes.
+		if trueResult.Type() == cty.Number && falseResult.Type() == cty.Number {
+			// This case deals with the common case of (predicate ? 1 : 0) and
+			// significantly decreases the range of the result in that case.
+			if !(trueResult.IsNull() || falseResult.IsNull()) {
+				if gt := trueResult.GreaterThan(falseResult); gt.IsKnown() {
+					b := cty.UnknownVal(cty.Number).Refine()
+					if gt.True() {
+						b = b.
+							NumberRangeLowerBound(falseResult, true).
+							NumberRangeUpperBound(trueResult, true)
+					} else {
+						b = b.
+							NumberRangeLowerBound(trueResult, true).
+							NumberRangeUpperBound(falseResult, true)
+					}
+					b = b.NotNull() // If neither of the results is null then the result can't be either
+					return b.NewValue().WithSameMarks(condResult).WithSameMarks(trueResult).WithSameMarks(falseResult), diags
+				}
+			}
+		}
+		if trueResult.Type().IsCollectionType() && falseResult.Type().IsCollectionType() {
+			if trueResult.Type().Equals(falseResult.Type()) {
+				if !(trueResult.IsNull() || falseResult.IsNull()) {
+					trueLen := trueResult.Length()
+					falseLen := falseResult.Length()
+					if gt := trueLen.GreaterThan(falseLen); gt.IsKnown() {
+						b := cty.UnknownVal(resultType).Refine()
+						trueLen, _ := trueLen.AsBigFloat().Int64()
+						falseLen, _ := falseLen.AsBigFloat().Int64()
+						if gt.True() {
+							b = b.
+								CollectionLengthLowerBound(int(falseLen)).
+								CollectionLengthUpperBound(int(trueLen))
+						} else {
+							b = b.
+								CollectionLengthLowerBound(int(trueLen)).
+								CollectionLengthUpperBound(int(falseLen))
+						}
+						b = b.NotNull() // If neither of the results is null then the result can't be either
+						return b.NewValue().WithSameMarks(condResult).WithSameMarks(trueResult).WithSameMarks(falseResult), diags
+					}
+				}
+			}
+		}
+		trueRng := trueResult.Range()
+		falseRng := falseResult.Range()
+		ret := cty.UnknownVal(resultType)
+		if trueRng.DefinitelyNotNull() && falseRng.DefinitelyNotNull() {
+			ret = ret.RefineNotNull()
+		}
+		return ret.WithSameMarks(condResult).WithSameMarks(trueResult).WithSameMarks(falseResult), diags
 	}
 	condResult, err := convert.Convert(condResult, cty.Bool)
 	if err != nil {
@@ -1632,11 +1684,15 @@ func (e *SplatExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnostics) {
 		// example, it is valid to use a splat on a single object to retrieve a
 		// list of a single attribute, but we still need to check if that
 		// attribute actually exists.
-		upgradedUnknown = !sourceVal.IsKnown()
+		if !sourceVal.IsKnown() {
+			sourceRng := sourceVal.Range()
+			if sourceRng.CouldBeNull() {
+				upgradedUnknown = true
+			}
+		}
 
 		sourceVal = cty.TupleVal([]cty.Value{sourceVal})
 		sourceTy = sourceVal.Type()
-
 	}
 
 	// We'll compute our result type lazily if we need it. In the normal case
@@ -1675,7 +1731,20 @@ func (e *SplatExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnostics) {
 		// checking to proceed.
 		ty, tyDiags := resultTy()
 		diags = append(diags, tyDiags...)
-		return cty.UnknownVal(ty), diags
+		ret := cty.UnknownVal(ty)
+		if ty != cty.DynamicPseudoType {
+			ret = ret.RefineNotNull()
+		}
+		if ty.IsListType() && sourceVal.Type().IsCollectionType() {
+			// We can refine the length of an unknown list result based on
+			// the source collection's own length.
+			sourceRng := sourceVal.Range()
+			ret = ret.Refine().
+				CollectionLengthLowerBound(sourceRng.LengthLowerBound()).
+				CollectionLengthUpperBound(sourceRng.LengthUpperBound()).
+				NewValue()
+		}
+		return ret.WithSameMarks(sourceVal), diags
 	}
 
 	// Unmark the collection, and save the marks to apply to the returned