From 774224a2dd072ce3742dabcfa5d6f05e1e8b9425 Mon Sep 17 00:00:00 2001 From: Diogenes Fernandes Date: Tue, 23 Sep 2025 07:16:18 -0300 Subject: [PATCH] Early return when having errors and additional tests (#3042) Co-authored-by: Andrei Ciobanu Signed-off-by: Diogenes Fernandes --- internal/configs/parser_config_test.go | 6 +-- internal/configs/resource.go | 4 +- .../lang/evalchecks/eval_lifecycle_enabled.go | 4 ++ .../evalchecks/eval_lifecycle_enabled_test.go | 45 +++++++++++-------- 4 files changed, 36 insertions(+), 23 deletions(-) diff --git a/internal/configs/parser_config_test.go b/internal/configs/parser_config_test.go index 3b3079a03a..e20f3d1f83 100644 --- a/internal/configs/parser_config_test.go +++ b/internal/configs/parser_config_test.go @@ -128,7 +128,7 @@ func TestParserLoadConfigFileFailureMessages(t *testing.T) { "invalid-files/ephemeral-count-and-for_each-and-enabled.tf", hcl.DiagError, `Invalid combination of "count", "enabled", and "for_each"`, - `The "count", "enabled", and "for_each" meta-arguments are mutually-exclusive. Only one may be used to be explicit about the number of resources to be created.`, + `The "count", "enabled", and "for_each" meta-arguments are mutually-exclusive. Only one may be used to be explicit about the number of ephemeral resources to be created.`, }, { "invalid-files/resource-count-and-for_each.tf", @@ -146,13 +146,13 @@ func TestParserLoadConfigFileFailureMessages(t *testing.T) { "invalid-files/data-count-and-for_each.tf", hcl.DiagError, `Invalid combination of "count" and "for_each"`, - `The "count" and "for_each" meta-arguments are mutually-exclusive. Only one may be used to be explicit about the number of resources to be created.`, + `The "count" and "for_each" meta-arguments are mutually-exclusive. Only one may be used to be explicit about the number of data sources to be created.`, }, { "invalid-files/data-count-and-for_each-and-enabled.tf", hcl.DiagError, `Invalid combination of "count", "enabled", and "for_each"`, - `The "count", "enabled", and "for_each" meta-arguments are mutually-exclusive. Only one may be used to be explicit about the number of resources to be created.`, + `The "count", "enabled", and "for_each" meta-arguments are mutually-exclusive. Only one may be used to be explicit about the number of data sources to be created.`, }, { "invalid-files/resource-lifecycle-badbool.tf", diff --git a/internal/configs/resource.go b/internal/configs/resource.go index e666fbdddc..ca386994b6 100644 --- a/internal/configs/resource.go +++ b/internal/configs/resource.go @@ -622,7 +622,7 @@ func decodeDataBlock(block *hcl.Block, override, nested bool) (*Resource, hcl.Di diags = append(diags, &hcl.Diagnostic{ Severity: hcl.DiagError, Summary: fmt.Sprintf(`Invalid combination of %s`, complainMsg), - Detail: fmt.Sprintf(`The %s meta-arguments are mutually-exclusive. Only one may be used to be explicit about the number of resources to be created.`, complainMsg), + Detail: fmt.Sprintf(`The %s meta-arguments are mutually-exclusive. Only one may be used to be explicit about the number of data sources to be created.`, complainMsg), Subject: complainRng, }) } @@ -806,7 +806,7 @@ func decodeEphemeralBlock(block *hcl.Block, override bool) (*Resource, hcl.Diagn diags = append(diags, &hcl.Diagnostic{ Severity: hcl.DiagError, Summary: fmt.Sprintf(`Invalid combination of %s`, complainMsg), - Detail: fmt.Sprintf(`The %s meta-arguments are mutually-exclusive. Only one may be used to be explicit about the number of resources to be created.`, complainMsg), + Detail: fmt.Sprintf(`The %s meta-arguments are mutually-exclusive. Only one may be used to be explicit about the number of ephemeral resources to be created.`, complainMsg), Subject: complainRng, }) } diff --git a/internal/lang/evalchecks/eval_lifecycle_enabled.go b/internal/lang/evalchecks/eval_lifecycle_enabled.go index 80ae7c5909..48a8d22d6a 100644 --- a/internal/lang/evalchecks/eval_lifecycle_enabled.go +++ b/internal/lang/evalchecks/eval_lifecycle_enabled.go @@ -83,6 +83,10 @@ func EvaluateEnabledExpression(expr hcl.Expression, hclCtxFunc ContextFunc) (boo }) } + if diags.HasErrors() { + return false, diags + } + enabledVal, err := convert.Convert(rawEnabledVal, cty.Bool) if err != nil { diags = diags.Append(&hcl.Diagnostic{ diff --git a/internal/lang/evalchecks/eval_lifecycle_enabled_test.go b/internal/lang/evalchecks/eval_lifecycle_enabled_test.go index 87e20b4375..8c11ff5f6a 100644 --- a/internal/lang/evalchecks/eval_lifecycle_enabled_test.go +++ b/internal/lang/evalchecks/eval_lifecycle_enabled_test.go @@ -81,24 +81,20 @@ type WantedError struct { func TestEvaluateEnabledExpression_errors(t *testing.T) { tests := map[string]struct { - val cty.Value + expr hcl.Expression Wanted []WantedError }{ "null": { - cty.NullVal(cty.Number), + hcltest.MockExprLiteral(cty.NullVal(cty.Number)), []WantedError{ { "Invalid enabled argument", `The given "enabled" argument value is unsuitable: the given value is null.`, }, - { - "Invalid enabled argument", - `The given "enabled" argument value is unsuitable: bool required, but have number.`, - }, }, }, "positive number": { - cty.NumberIntVal(1), + hcltest.MockExprLiteral(cty.NumberIntVal(1)), []WantedError{ { "Invalid enabled argument", @@ -107,7 +103,7 @@ func TestEvaluateEnabledExpression_errors(t *testing.T) { }, }, "negative number": { - cty.NumberIntVal(-1), + hcltest.MockExprLiteral(cty.NumberIntVal(-1)), []WantedError{ { "Invalid enabled argument", @@ -116,7 +112,7 @@ func TestEvaluateEnabledExpression_errors(t *testing.T) { }, }, "list": { - cty.ListVal([]cty.Value{cty.StringVal("a"), cty.StringVal("a")}), + hcltest.MockExprLiteral(cty.ListVal([]cty.Value{cty.StringVal("a"), cty.StringVal("a")})), []WantedError{ { "Invalid enabled argument", @@ -125,7 +121,7 @@ func TestEvaluateEnabledExpression_errors(t *testing.T) { }, }, "tuple": { - cty.TupleVal([]cty.Value{cty.StringVal("a"), cty.StringVal("b")}), + hcltest.MockExprLiteral(cty.TupleVal([]cty.Value{cty.StringVal("a"), cty.StringVal("b")})), []WantedError{ { "Invalid enabled argument", @@ -134,20 +130,16 @@ func TestEvaluateEnabledExpression_errors(t *testing.T) { }, }, "unknown": { - cty.UnknownVal(cty.Number), + hcltest.MockExprLiteral(cty.UnknownVal(cty.Number)), []WantedError{ { "Invalid enabled argument", `The given "enabled" argument value is derived from a value that won't be known until the apply phase, so OpenTofu cannot determine whether an instance of this object is declared or not.`, }, - { - "Invalid enabled argument", - "The given \"enabled\" argument value is unsuitable: bool required, but have number.", - }, }, }, "sensitive": { - cty.StringVal("1").Mark(marks.Sensitive), + hcltest.MockExprLiteral(cty.StringVal("1").Mark(marks.Sensitive)), []WantedError{ { "Invalid enabled argument", @@ -156,7 +148,7 @@ func TestEvaluateEnabledExpression_errors(t *testing.T) { }, }, "ephemeral": { - cty.StringVal("1").Mark(marks.Ephemeral), + hcltest.MockExprLiteral(cty.StringVal("1").Mark(marks.Ephemeral)), []WantedError{ { "Invalid enabled argument", @@ -164,11 +156,28 @@ func TestEvaluateEnabledExpression_errors(t *testing.T) { }, }, }, + "bool conversion": { + &hclsyntax.BinaryOpExpr{ + LHS: &hclsyntax.LiteralValueExpr{ + Val: cty.StringVal("3"), + }, + RHS: &hclsyntax.LiteralValueExpr{ + Val: cty.StringVal("5"), + }, + Op: hclsyntax.OpSubtract, + }, + []WantedError{ + { + "Invalid enabled argument", + `The given "enabled" argument value is unsuitable: bool required, but have number.`, + }, + }, + }, } for name, test := range tests { t.Run(name, func(t *testing.T) { - _, diags := EvaluateEnabledExpression(hcltest.MockExprLiteral(test.val), mockRefsFunc()) + _, diags := EvaluateEnabledExpression(test.expr, mockRefsFunc()) if len(diags) != len(test.Wanted) { t.Fatalf("wrong diagnostics size: (want %d, got %d):\n", len(test.Wanted), len(diags))