From 618463aa79585804de2d5bb43032a43ab008cba3 Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Thu, 18 Jul 2019 20:26:11 -0400
Subject: [PATCH] ensure correct type in conditionals w/ DynamicVal (#118)

If an expression in a conditional contains a DynamicVal, we know that
the opposing condition can pass through with no conversion since
converting to a DynamicPseudoType is a noop. We can also just pass
through the Dynamic val, since it is unknown and can't be converted.
---
 hcl/hclsyntax/expression.go      | 11 +++++++-
 hcl/hclsyntax/expression_test.go | 45 ++++++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+), 1 deletion(-)

diff --git a/hcl/hclsyntax/expression.go b/hcl/hclsyntax/expression.go
index b9bd6ac..d3f7a74 100644
--- a/hcl/hclsyntax/expression.go
+++ b/hcl/hclsyntax/expression.go
@@ -473,7 +473,7 @@ func (e *ConditionalExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnostic
 	falseResult, falseDiags := e.FalseResult.Value(ctx)
 	var diags hcl.Diagnostics
 
-	var resultType cty.Type
+	resultType := cty.DynamicPseudoType
 	convs := make([]convert.Conversion, 2)
 
 	switch {
@@ -481,12 +481,21 @@ func (e *ConditionalExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnostic
 	// 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.
+
+	// If we know that either Type is a DynamicPseudoType, we can be certain
+	// that the other value can convert since it's a pass-through, and we don't
+	// need to unify the types. If the final evaluation results in the dynamic
+	// value being returned, there's no conversion we can do, so we return the
+	// value directly.
 	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)
+	case trueResult.Type() == cty.DynamicPseudoType, falseResult.Type() == cty.DynamicPseudoType:
+		// the final resultType type is still unknown
+		// we don't need to get the conversion, because both are a noop.
 
 	default:
 		// Try to find a type that both results can be converted to.
diff --git a/hcl/hclsyntax/expression_test.go b/hcl/hclsyntax/expression_test.go
index 80ce35c..0332e8e 100644
--- a/hcl/hclsyntax/expression_test.go
+++ b/hcl/hclsyntax/expression_test.go
@@ -1401,6 +1401,51 @@ EOT
 			cty.NullVal(cty.DynamicPseudoType),
 			0,
 		},
+		{
+			`false ? var: {a = "b"}`,
+			&hcl.EvalContext{
+				Variables: map[string]cty.Value{
+					"var": cty.DynamicVal,
+				},
+			},
+			cty.ObjectVal(map[string]cty.Value{
+				"a": cty.StringVal("b"),
+			}),
+			0,
+		},
+		{
+			`true ? ["a", "b"]: var`,
+			&hcl.EvalContext{
+				Variables: map[string]cty.Value{
+					"var": cty.UnknownVal(cty.DynamicPseudoType),
+				},
+			},
+			cty.TupleVal([]cty.Value{
+				cty.StringVal("a"),
+				cty.StringVal("b"),
+			}),
+			0,
+		},
+		{
+			`false ? ["a", "b"]: var`,
+			&hcl.EvalContext{
+				Variables: map[string]cty.Value{
+					"var": cty.DynamicVal,
+				},
+			},
+			cty.DynamicVal,
+			0,
+		},
+		{
+			`false ? ["a", "b"]: var`,
+			&hcl.EvalContext{
+				Variables: map[string]cty.Value{
+					"var": cty.UnknownVal(cty.DynamicPseudoType),
+				},
+			},
+			cty.DynamicVal,
+			0,
+		},
 	}
 
 	for _, test := range tests {