Add rule for finding duplicate keys in map expressions

Author: tanelso2

rules/terraform_map_duplicate_keys.go

@@ -0,0 +1,106 @@
+package rules
+
+import (
+	"fmt"
+
+	"github.com/hashicorp/hcl/v2"
+	"github.com/hashicorp/hcl/v2/hclsyntax"
+	"github.com/terraform-linters/tflint-plugin-sdk/tflint"
+	"github.com/terraform-linters/tflint-ruleset-terraform/project"
+	"github.com/zclconf/go-cty/cty"
+)
+
+// This rule checks for map literals with duplicate keys
+type TerraformMapDuplicateKeysRule struct {
+	tflint.DefaultRule
+}
+
+func NewTerraformMapDuplicateKeysRule() *TerraformMapDuplicateKeysRule {
+	return &TerraformMapDuplicateKeysRule{}
+}
+
+func (r *TerraformMapDuplicateKeysRule) Name() string {
+	return "terraform_map_duplicate_keys"
+}
+
+func (r *TerraformMapDuplicateKeysRule) Enabled() bool {
+	return true
+}
+
+func (r *TerraformMapDuplicateKeysRule) Severity() tflint.Severity {
+	return tflint.WARNING
+}
+
+func (r *TerraformMapDuplicateKeysRule) Link() string {
+	return project.ReferenceLink(r.Name())
+}
+
+func (r *TerraformMapDuplicateKeysRule) Check(runner tflint.Runner) error {
+	path, err := runner.GetModulePath()
+	if err != nil {
+		return err
+	}
+	if !path.IsRoot() {
+		// This rule does not evaluate child modules
+		return nil
+	}
+
+	diags := runner.WalkExpressions(tflint.ExprWalkFunc(func(e hcl.Expression) hcl.Diagnostics {
+		return r.checkObjectConsExpr(e, runner)
+	}))
+	if diags.HasErrors() {
+		return diags
+	}
+
+	return nil
+}
+
+func (r *TerraformMapDuplicateKeysRule) checkObjectConsExpr(e hcl.Expression, runner tflint.Runner) hcl.Diagnostics {
+	exprMap, ok := e.(*hclsyntax.ObjectConsExpr)
+	if !ok {
+		// Ignore everything that isn't an ObjectConsExpr
+		return nil
+	}
+	diags := hcl.Diagnostics{}
+	foundKeys := make(map[string]hcl.Range)
+	for _, item := range exprMap.Items {
+		expr := item.KeyExpr.(*hclsyntax.ObjectConsKeyExpr)
+		val := cty.Value{}
+		err := runner.EvaluateExpr(expr, &val, &tflint.EvaluateExprOption{})
+		if err != nil {
+			fmt.Printf("Failed to evaluate an expression, continuing\n")
+			diags.Append(
+				&hcl.Diagnostic{
+					Severity: hcl.DiagError,
+					Summary:  "failed to evaluate expression",
+					Detail:   err.Error(),
+				},
+			)
+			continue
+		}
+		if !val.IsKnown() || val.IsNull() {
+			// When trying to evaluate an expression
+			// with a variable without a default,
+			// runner.evaluateExpr() returns a null value.
+			// Ignore this case since there's nothing we can do.
+			fmt.Printf("Unknown key, continuing\n")
+			continue
+		}
+
+		if previousRange, exists := foundKeys[val.AsString()]; exists {
+			msg := fmt.Sprintf("Duplicate key: '%s'\nThe previous definition was at %s", val.AsString(), previousRange)
+			if err := runner.EmitIssue(r, msg, expr.Range()); err != nil {
+				diags.Append(
+					&hcl.Diagnostic{
+						Severity: hcl.DiagError,
+						Summary:  "failed to call EmitIssue()",
+						Detail:   err.Error(),
+					},
+				)
+			}
+		} else {
+			foundKeys[val.AsString()] = expr.Range()
+		}
+	}
+	return diags
+}

rules/terraform_map_duplicate_keys_test.go

@@ -0,0 +1,151 @@
+package rules
+
+import (
+	"testing"
+
+	"github.com/hashicorp/hcl/v2"
+	"github.com/terraform-linters/tflint-plugin-sdk/helper"
+)
+
+func Test_TerraformMapLiteralChangeNameLater(t *testing.T) {
+	cases := []struct {
+		Name     string
+		Content  string
+		Expected helper.Issues
+		Fixed    string
+	}{
+		{
+			Name: "No duplicates",
+			Content: `
+resource "null_resource" "test" {
+	test = {
+	  a = 1
+	  b = 2
+	  c = 3
+	}
+}`,
+			Expected: helper.Issues{},
+		},
+		{
+			Name: "duplicate keys in map literal",
+			Content: `
+resource "null_resource" "test" {
+    triggers = {
+        a = "b"
+        a = "c"
+    }
+}`,
+			Expected: helper.Issues{
+				{
+					Rule:    NewTerraformMapDuplicateKeysRule(),
+					Message: "Duplicate key: 'a'\nThe previous definition was at module.tf:4,9-10",
+					Range: hcl.Range{
+						Filename: "module.tf",
+						Start:    hcl.Pos{Line: 5, Column: 9},
+						End:      hcl.Pos{Line: 5, Column: 10},
+					},
+				},
+			},
+		},
+		{
+			Name: "Using variables as keys",
+			Content: `
+variable "a" {
+  type    = string
+  default = "b"
+}
+
+resource "null_resource" "test" {
+	map = {
+	  (var.a) = 5
+	  b       = 8
+	}
+}`,
+			Expected: helper.Issues{
+				{
+					Rule:    NewTerraformMapDuplicateKeysRule(),
+					Message: "Duplicate key: 'b'\nThe previous definition was at module.tf:9,4-11",
+					Range: hcl.Range{
+						Filename: "module.tf",
+						Start:    hcl.Pos{Line: 10, Column: 4},
+						End:      hcl.Pos{Line: 10, Column: 5},
+					},
+				},
+			},
+		},
+		{
+			Name: "Using a variable as a key without a default",
+			Content: `
+variable "unknown" {
+  type    = string
+}
+
+resource "null_resource" "test" {
+	map = {
+	  x             = 8
+	  (var.unknown) = 5
+	}
+}`,
+			Expected: helper.Issues{},
+		},
+		{
+			Name: "Multiple duplicates in same map",
+			Content: `
+resource "null_resource" "test" {
+	map = {
+	  a = 7
+	  a = 8
+	  a = 9
+	}
+}`,
+			Expected: helper.Issues{
+				{
+					Rule:    NewTerraformMapDuplicateKeysRule(),
+					Message: "Duplicate key: 'a'\nThe previous definition was at module.tf:4,4-5",
+					Range: hcl.Range{
+						Filename: "module.tf",
+						Start:    hcl.Pos{Line: 5, Column: 4},
+						End:      hcl.Pos{Line: 5, Column: 5},
+					},
+				},
+				{
+					Rule:    NewTerraformMapDuplicateKeysRule(),
+					Message: "Duplicate key: 'a'\nThe previous definition was at module.tf:4,4-5",
+					Range: hcl.Range{
+						Filename: "module.tf",
+						Start:    hcl.Pos{Line: 6, Column: 4},
+						End:      hcl.Pos{Line: 6, Column: 5},
+					},
+				},
+			},
+		},
+		{
+			Name: "Using same key in different maps is okay",
+			Content: `
+
+resource "null_resource" "test" {
+	map = {
+	  x = 1
+	}
+	map2 = {
+	  x = 2
+	}
+}`,
+			Expected: helper.Issues{},
+		},
+	}
+
+	rule := NewTerraformMapDuplicateKeysRule()
+
+	for _, tc := range cases {
+		t.Run(tc.Name, func(t *testing.T) {
+			runner := testRunner(t, map[string]string{"module.tf": tc.Content})
+
+			if err := rule.Check(runner); err != nil {
+				t.Fatalf("Unexpected error occurred: %s", err)
+			}
+
+			helper.AssertIssues(t, tc.Expected, runner.Runner.(*helper.Runner).Issues)
+		})
+	}
+}