Add rule for finding duplicate keys in map expressions (#194)

Co-authored-by: Ben Drucker <bvdrucker@gmail.com>

Author: tanelso2

rules/terraform_map_duplicate_keys.go

@@ -0,0 +1,115 @@
+package rules
+
+import (
+	"fmt"
+
+	"github.com/hashicorp/hcl/v2"
+	"github.com/hashicorp/hcl/v2/hclsyntax"
+	"github.com/terraform-linters/tflint-plugin-sdk/logger"
+	"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 {
+	objExpr, ok := e.(*hclsyntax.ObjectConsExpr)
+	if !ok {
+		return nil
+	}
+
+	var diags hcl.Diagnostics
+	keys := make(map[string]hcl.Range)
+
+	for _, item := range objExpr.Items {
+		expr := item.KeyExpr.(*hclsyntax.ObjectConsKeyExpr)
+		var val cty.Value
+
+		err := runner.EvaluateExpr(expr, &val, nil)
+		if err != nil {
+			logger.Debug("Failed to evaluate an expression, continuing", "error", err)
+
+			diags = append(diags, &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 value,
+			// runner.evaluateExpr() returns a null value.
+			// Ignore this case since there's nothing we can do.
+			logger.Debug("Unknown key, continuing", "range", expr.Range())
+			continue
+		}
+
+		if declRange, exists := keys[val.AsString()]; exists {
+			if err := runner.EmitIssue(
+				r,
+				fmt.Sprintf("Duplicate key: %q, first defined at %s", val.AsString(), declRange),
+				expr.Range(),
+			); err != nil {
+				diags = append(diags, &hcl.Diagnostic{
+					Severity: hcl.DiagError,
+					Summary:  "failed to call EmitIssue()",
+					Detail:   err.Error(),
+				})
+
+				return diags
+			}
+
+			continue
+		}
+
+		keys[val.AsString()] = expr.Range()
+	}
+
+	return diags
+}

rules/terraform_map_duplicate_keys_test.go

@@ -0,0 +1,172 @@
+package rules
+
+import (
+	"testing"
+
+	"github.com/hashicorp/hcl/v2"
+	"github.com/terraform-linters/tflint-plugin-sdk/helper"
+)
+
+func Test_TerraformMapDuplicateKeys(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", first defined 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: "duplicate keys with quoting",
+			Content: `
+resource "null_resource" "test" {
+    triggers = {
+        a = "b"
+        "a" = "c"
+    }
+}`,
+			Expected: helper.Issues{
+				{
+					Rule:    NewTerraformMapDuplicateKeysRule(),
+					Message: `Duplicate key: "a", first defined 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: 12},
+					},
+				},
+			},
+		},
+		{
+			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", first defined 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", first defined 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", first defined 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)
+		})
+	}
+}