diff --git a/internal/tofu/node_module_variable.go b/internal/tofu/node_module_variable.go index 37ed93c251..82340ea062 100644 --- a/internal/tofu/node_module_variable.go +++ b/internal/tofu/node_module_variable.go @@ -9,13 +9,18 @@ import ( "context" "fmt" "log" + "maps" + "slices" "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/hcl/v2/hclsyntax" "github.com/zclconf/go-cty/cty" + "github.com/zclconf/go-cty/cty/convert" "github.com/opentofu/opentofu/internal/addrs" "github.com/opentofu/opentofu/internal/configs" "github.com/opentofu/opentofu/internal/dag" + "github.com/opentofu/opentofu/internal/didyoumean" "github.com/opentofu/opentofu/internal/instances" "github.com/opentofu/opentofu/internal/lang" "github.com/opentofu/opentofu/internal/tfdiags" @@ -160,6 +165,15 @@ func (n *nodeModuleVariable) Execute(ctx context.Context, evalCtx EvalContext, o return diags } + // We might generate some "linter-like" warnings for situations that + // have a high likelihood of being a mistake even though they are + // technically valid. We check these only in the validate walk because + // that always happens before any other walk and so we'd generate + // duplicate diagnostics if we produced this in later walks too. + if op == walkValidate { + diags = diags.Append(n.warningDiags(ctx)) + } + // Set values for arguments of a child module call, for later retrieval // during expression evaluation. _, call := n.Addr.Module.CallInstance() @@ -241,3 +255,69 @@ func (n *nodeModuleVariable) evalModuleVariable(ctx context.Context, evalCtx Eva return finalVal, diags.ErrWithWarnings() } + +// warningDiags detects "lint-like" problems with a variable's definition, where +// the input is technically valid but nonetheless seems highly likely to be +// a mistake. +// +// This function never returns error diagnostics. +func (n *nodeModuleVariable) warningDiags(_ context.Context) tfdiags.Diagnostics { + var diags tfdiags.Diagnostics + + // If the definition directly uses object constructor syntax, meaning that + // the resulting object is definitely for this input variable and not + // also used in any other place, then any attributes that aren't included + // in the variable's object type constrant are very likely to be mistakes. + // (They would just be discarded during type conversion, and so there's + // no useful reason to include them.) + // + // We only do this for direct object constructor syntax because it's more + // reasonable to use an object defined elsewhere that intentionally has + // a superset of the expected attributes but is also used in a different + // place that relies on a different subset of its attributes. + if consExpr, ok := n.Expr.(*hclsyntax.ObjectConsExpr); ok { + ty := n.Config.ConstraintType + if ty != cty.NilType && ty.IsObjectType() { + for _, item := range consExpr.Items { + // The following is a cut-down version of the logic that + // HCL itself uses to evaluate keys in an object constructor + // expression during evaluation, from + // [hclsyntax.ObjectConsExpr.Value]. + // + // This is a best-effort thing which only works for + // valid and literally-defined keys, since that's the common + // case we're trying to lint-check. We'll ignore anything that + // we can't trivially evaluate. + key, keyDiags := item.KeyExpr.Value(nil) + if keyDiags.HasErrors() || !key.IsKnown() || key.IsNull() { + continue + } + key, _ = key.Unmark() + var err error + key, err = convert.Convert(key, cty.String) + if err != nil { + continue + } + keyStr := key.AsString() + if !ty.HasAttribute(keyStr) { + attrs := slices.Collect(maps.Keys(ty.AttributeTypes())) + suggestion := "" + if similarName := didyoumean.NameSuggestion(keyStr, attrs); similarName != "" { + suggestion = fmt.Sprintf(" Did you mean to set attribute %q instead?", similarName) + } + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagWarning, + Summary: "Object attribute is ignored", + Detail: fmt.Sprintf( + "The object type for input variable %q does not include an attribute named %q, so this definition is unused.%s", + n.Addr.Variable.Name, keyStr, suggestion, + ), + Subject: item.KeyExpr.Range().Ptr(), + }) + } + } + } + } + + return diags +} diff --git a/internal/tofu/node_module_variable_test.go b/internal/tofu/node_module_variable_test.go index d0e10fdff4..26d7939450 100644 --- a/internal/tofu/node_module_variable_test.go +++ b/internal/tofu/node_module_variable_test.go @@ -12,8 +12,10 @@ import ( "testing" "github.com/go-test/deep" + "github.com/google/go-cmp/cmp" "github.com/hashicorp/hcl/v2" "github.com/hashicorp/hcl/v2/hclsyntax" + "github.com/zclconf/go-cty-debug/ctydebug" "github.com/zclconf/go-cty/cty" "github.com/opentofu/opentofu/internal/addrs" @@ -307,3 +309,51 @@ func TestNodeModuleVariableConstraints(t *testing.T) { } }) } + +func TestNodeModuleVariable_warningDiags(t *testing.T) { + t.Run("unused object attribute", func(t *testing.T) { + n := &nodeModuleVariable{ + Addr: addrs.InputVariable{Name: "foo"}.Absolute(addrs.RootModuleInstance), + Config: &configs.Variable{ + Name: "foo", + ConstraintType: cty.Object(map[string]cty.Type{ + "foo": cty.String, + "bar": cty.String, + }), + }, + Expr: &hclsyntax.ObjectConsExpr{ + Items: []hclsyntax.ObjectConsItem{ + { + KeyExpr: &hclsyntax.LiteralValueExpr{ + Val: cty.StringVal("baz"), + SrcRange: hcl.Range{ + Filename: "test.tofu", + }, + }, + ValueExpr: &hclsyntax.LiteralValueExpr{ + Val: cty.StringVal("..."), + }, + }, + }, + }, + ModuleInstance: addrs.RootModuleInstance, + } + // We use the "ForRPC" representation of the diagnostics just because + // it's more friendly for comparison and we care only about the + // user-facing information in the diagnostics, not their concrete types. + gotDiags := n.warningDiags(t.Context()).ForRPC() + var wantDiags tfdiags.Diagnostics + wantDiags = wantDiags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagWarning, + Summary: "Object attribute is ignored", + Detail: `The object type for input variable "foo" does not include an attribute named "baz", so this definition is unused. Did you mean to set attribute "bar" instead?`, + Subject: &hcl.Range{ + Filename: "test.tofu", + }, + }) + wantDiags = wantDiags.ForRPC() + if diff := cmp.Diff(wantDiags, gotDiags, ctydebug.CmpOptions); diff != "" { + t.Error("wrong diagnostics\n" + diff) + } + }) +}