diff --git a/hclsyntax/expression_ops.go b/hclsyntax/expression_ops.go index 6d9e1af5..da146ef8 100644 --- a/hclsyntax/expression_ops.go +++ b/hclsyntax/expression_ops.go @@ -38,6 +38,13 @@ var ( switch { // if both are unknown, we don't short circuit anything case !lhs.IsKnown() && !rhs.IsKnown(): + // short-circuit left-to-right when encountering a good unknown + // value and both are unknown. + if !lhsDiags.HasErrors() { + return cty.UnknownVal(cty.Bool).RefineNotNull(), lhsDiags + } + // If the LHS has an error, the RHS might too. Don't + // short-circuit so both diags get collected. return cty.NilVal, nil // for ||, a single true is the controlling condition @@ -46,7 +53,7 @@ var ( case rhs.IsKnown() && rhs.True(): return cty.True, rhsDiags - // if the opposing side is false we can't sort-circuit based on + // if the opposing side is false we can't short-circuit based on // boolean logic, so an unknown becomes the controlling condition case !lhs.IsKnown() && rhs.False(): return cty.UnknownVal(cty.Bool).RefineNotNull(), lhsDiags @@ -62,9 +69,16 @@ var ( Type: cty.Bool, ShortCircuit: func(lhs, rhs cty.Value, lhsDiags, rhsDiags hcl.Diagnostics) (cty.Value, hcl.Diagnostics) { + switch { - // if both are unknown, we don't short circuit anything case !lhs.IsKnown() && !rhs.IsKnown(): + // short-circuit left-to-right when encountering a good unknown + // value and both are unknown. + if !lhsDiags.HasErrors() { + return cty.UnknownVal(cty.Bool).RefineNotNull(), lhsDiags + } + // If the LHS has an error, the RHS might too. Don't + // short-circuit so both diags get collected. return cty.NilVal, nil // For &&, a single false is the controlling condition @@ -73,14 +87,13 @@ var ( case rhs.IsKnown() && rhs.False(): return cty.False, rhsDiags - // if the opposing side is true we can't sort-circuit based on + // if the opposing side is true we can't short-circuit based on // boolean logic, so an unknown becomes the controlling condition case !lhs.IsKnown() && rhs.True(): return cty.UnknownVal(cty.Bool).RefineNotNull(), lhsDiags case !rhs.IsKnown() && lhs.True(): return cty.UnknownVal(cty.Bool).RefineNotNull(), rhsDiags } - return cty.NilVal, nil }, } @@ -242,9 +255,6 @@ func (e *BinaryOpExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnostics) lhsVal, lhsMarks := lhsVal.Unmark() rhsVal, rhsMarks := rhsVal.Unmark() - // If we short-circuited above and still passed the type-check of RHS then - // we'll halt here and return the short-circuit result rather than actually - // executing the operation. if e.Op.ShortCircuit != nil { forceResult, diags := e.Op.ShortCircuit(lhsVal, rhsVal, lhsDiags, rhsDiags) if forceResult != cty.NilVal { @@ -256,6 +266,8 @@ func (e *BinaryOpExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnostics) } } + diags = append(diags, lhsDiags...) + diags = append(diags, rhsDiags...) if diags.HasErrors() { // Don't actually try the call if we have errors, since the this will // probably just produce confusing duplicate diagnostics. diff --git a/hclsyntax/expression_test.go b/hclsyntax/expression_test.go index 843847df..636a541d 100644 --- a/hclsyntax/expression_test.go +++ b/hclsyntax/expression_test.go @@ -2616,6 +2616,16 @@ func TestExpressionErrorMessages(t *testing.T) { "Function calls not allowed", `Functions may not be called here.`, }, + { + `map != null || map["key"] == "value"`, + &hcl.EvalContext{ + Variables: map[string]cty.Value{ + "map": cty.NullVal(cty.Map(cty.String)), + }, + }, + "Attempt to index null value", + `This value is null, so it does not have any indices.`, + }, } for _, test := range tests {