From 5b39d9ff3a9a746a3cdfcda7c97c3bb070a960f9 Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Tue, 2 Jul 2019 14:56:34 -0400
Subject: [PATCH] Expect correct type from a conditional with nulls (#116)

The type unification done when evaluating a conditional normally needs
to return a DynamicPseudoType when either condition is dynamic. However,
a null dynamic value represents a known value of the desired type rather
than an unknown type, and we can be certain that it is convertible to
the desired type during the final evaluation.  Rather than unifying
types against nulls, directly assign the needed conversion when
evaluating the conditional.
---
 hcl/hclsyntax/expression.go      | 22 ++++++++++++--
 hcl/hclsyntax/expression_test.go | 50 ++++++++++++++++++++++++++++++++
 2 files changed, 70 insertions(+), 2 deletions(-)

diff --git a/hcl/hclsyntax/expression.go b/hcl/hclsyntax/expression.go
index 26819a2..b9bd6ac 100644
--- a/hcl/hclsyntax/expression.go
+++ b/hcl/hclsyntax/expression.go
@@ -473,8 +473,26 @@ func (e *ConditionalExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnostic
 	falseResult, falseDiags := e.FalseResult.Value(ctx)
 	var diags hcl.Diagnostics
 
-	// Try to find a type that both results can be converted to.
-	resultType, convs := convert.UnifyUnsafe([]cty.Type{trueResult.Type(), falseResult.Type()})
+	var resultType cty.Type
+	convs := make([]convert.Conversion, 2)
+
+	switch {
+	// If either case is a dynamic null value (which would result from a
+	// literal null in the config), we know that it can convert to the expected
+	// type of the opposite case, and we don't need to speculatively reduce the
+	// final result type to DynamicPseudoType.
+	case trueResult.RawEquals(cty.NullVal(cty.DynamicPseudoType)):
+		resultType = falseResult.Type()
+		convs[0] = convert.GetConversionUnsafe(cty.DynamicPseudoType, resultType)
+	case falseResult.RawEquals(cty.NullVal(cty.DynamicPseudoType)):
+		resultType = trueResult.Type()
+		convs[1] = convert.GetConversionUnsafe(cty.DynamicPseudoType, resultType)
+
+	default:
+		// Try to find a type that both results can be converted to.
+		resultType, convs = convert.UnifyUnsafe([]cty.Type{trueResult.Type(), falseResult.Type()})
+	}
+
 	if resultType == cty.NilType {
 		return cty.DynamicVal, hcl.Diagnostics{
 			{
diff --git a/hcl/hclsyntax/expression_test.go b/hcl/hclsyntax/expression_test.go
index 2b0005a..80ce35c 100644
--- a/hcl/hclsyntax/expression_test.go
+++ b/hcl/hclsyntax/expression_test.go
@@ -1351,6 +1351,56 @@ EOT
 			cty.False,
 			0,
 		},
+		{
+			`true ? var : null`,
+			&hcl.EvalContext{
+				Variables: map[string]cty.Value{
+					"var": cty.ObjectVal(map[string]cty.Value{"a": cty.StringVal("A")}),
+				},
+			},
+			cty.ObjectVal(map[string]cty.Value{"a": cty.StringVal("A")}),
+			0,
+		},
+		{
+			`true ? var : null`,
+			&hcl.EvalContext{
+				Variables: map[string]cty.Value{
+					"var": cty.UnknownVal(cty.DynamicPseudoType),
+				},
+			},
+			cty.UnknownVal(cty.DynamicPseudoType),
+			0,
+		},
+		{
+			`true ? ["a", "b"] : null`,
+			nil,
+			cty.TupleVal([]cty.Value{cty.StringVal("a"), cty.StringVal("b")}),
+			0,
+		},
+		{
+			`true ? null: ["a", "b"]`,
+			nil,
+			cty.NullVal(cty.Tuple([]cty.Type{cty.String, cty.String})),
+			0,
+		},
+		{
+			`false ? ["a", "b"] : null`,
+			nil,
+			cty.NullVal(cty.Tuple([]cty.Type{cty.String, cty.String})),
+			0,
+		},
+		{
+			`false ? null: ["a", "b"]`,
+			nil,
+			cty.TupleVal([]cty.Value{cty.StringVal("a"), cty.StringVal("b")}),
+			0,
+		},
+		{
+			`false ? null: null`,
+			nil,
+			cty.NullVal(cty.DynamicPseudoType),
+			0,
+		},
 	}
 
 	for _, test := range tests {