fix: improvements after code review to avoid tests with big scopes an…

…d avoid repeated issues with the same range
terraform-linters · Jul 3, 2022 · 8063659 · 8063659
1 parent 2095371
commit 8063659
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 26 deletions.
diff --git a/rules/terraformrules/terraform_empty_list_equality.go b/rules/terraformrules/terraform_empty_list_equality.go
@@ -10,6 +10,7 @@ import (
 
 // TerraformEmptyListEqualityRule checks whether is there a comparison with an empty list
 type TerraformEmptyListEqualityRule struct{}
+type void struct{}
 
 // NewTerraformCommentSyntaxRule returns a new rule
 func NewTerraformEmptyListEqualityRule() *TerraformEmptyListEqualityRule {
@@ -55,33 +56,37 @@ func (r *TerraformEmptyListEqualityRule) Check(runner *tflint.Runner) error {
 func (r *TerraformEmptyListEqualityRule) checkEmptyList(runner *tflint.Runner) error {
 	return runner.WalkExpressions(func(expr hcl.Expression) error {
 		if conditionalExpr, ok := expr.(*hclsyntax.ConditionalExpr); ok {
-			searchEmptyList(conditionalExpr.Condition, runner, r, conditionalExpr.Range())
+			var issuesRangeSet = make(map[hcl.Range]void)
+			searchEmptyList(conditionalExpr.Condition, runner, r, conditionalExpr.Range(), issuesRangeSet)
+			emitEmptyListEqualityIssues(issuesRangeSet, runner, r)
 		}
 		return nil
 	})
 }
 
-func searchEmptyList(expr hcl.Expression, runner *tflint.Runner, r *TerraformEmptyListEqualityRule, exprRange hcl.Range) {
+func searchEmptyList(expr hcl.Expression, runner *tflint.Runner, r *TerraformEmptyListEqualityRule, exprRange hcl.Range, issuesRangeSet map[hcl.Range]void) {
 	if binaryOpExpr, ok := expr.(*hclsyntax.BinaryOpExpr); ok {
 		if binaryOpExpr.Op.Type.FriendlyName() == "bool" {
-			searchEmptyList(binaryOpExpr.RHS, runner, r, binaryOpExpr.Range())
-			searchEmptyList(binaryOpExpr.LHS, runner, r, binaryOpExpr.Range())
+			searchEmptyList(binaryOpExpr.RHS, runner, r, binaryOpExpr.Range(), issuesRangeSet)
+			searchEmptyList(binaryOpExpr.LHS, runner, r, binaryOpExpr.Range(), issuesRangeSet)
 		}
 	} else if binaryOpExpr, ok := expr.(*hclsyntax.BinaryOpExpr); ok {
-		searchEmptyList(binaryOpExpr, runner, r, binaryOpExpr.Range())
+		searchEmptyList(binaryOpExpr, runner, r, binaryOpExpr.Range(), issuesRangeSet)
 	} else if parenthesesExpr, ok := expr.(*hclsyntax.ParenthesesExpr); ok {
-		searchEmptyList(parenthesesExpr.Expression, runner, r, parenthesesExpr.Range())
+		searchEmptyList(parenthesesExpr.Expression, runner, r, parenthesesExpr.Range(), issuesRangeSet)
 	} else if tupleConsExpr, ok := expr.(*hclsyntax.TupleConsExpr); ok {
 		if len(tupleConsExpr.Exprs) == 0 {
-			emitIssue(exprRange, runner, r)
+			issuesRangeSet[exprRange] = void{}
 		}
 	}
 }
 
-func emitIssue(exprRange hcl.Range, runner *tflint.Runner, r *TerraformEmptyListEqualityRule) {
-	runner.EmitIssue(
-		r,
-		"Comparing a collection with an empty list is invalid. To detect an empty collection, check its length.",
-		exprRange,
-	)
+func emitEmptyListEqualityIssues(exprRanges map[hcl.Range]void, runner *tflint.Runner, r *TerraformEmptyListEqualityRule) {
+	for exprRange := range exprRanges {
+		runner.EmitIssue(
+			r,
+			"Comparing a collection with an empty list is invalid. To detect an empty collection, check its length.",
+			exprRange,
+		)
+	}
 }
diff --git a/rules/terraformrules/terraform_empty_list_equality_test.go b/rules/terraformrules/terraform_empty_list_equality_test.go
@@ -30,6 +30,25 @@ resource "aws_db_instance" "mysql" {
 						End:      hcl.Pos{Line: 3, Column: 18},
 					},
 				},
+			},
+		},
+		{
+			Name: "comparing with [] multiple times in the same statement is not recommended",
+			Content: `
+resource "aws_db_instance" "mysql" {
+	count = [] == [] || [] == [] ? 1 : 0
+	instance_class = "m4.2xlarge"
+}`,
+			Expected: tflint.Issues{
+				{
+					Rule:    NewTerraformEmptyListEqualityRule(),
+					Message: "Comparing a collection with an empty list is invalid. To detect an empty collection, check its length.",
+					Range: hcl.Range{
+						Filename: "resource.tf",
+						Start:    hcl.Pos{Line: 3, Column: 22},
+						End:      hcl.Pos{Line: 3, Column: 30},
+					},
+				},
 				{
 					Rule:    NewTerraformEmptyListEqualityRule(),
 					Message: "Comparing a collection with an empty list is invalid. To detect an empty collection, check its length.",
@@ -42,10 +61,10 @@ resource "aws_db_instance" "mysql" {
 			},
 		},
 		{
-			Name: "comparing with [] is not recommended (mixed with other conditions)",
+			Name: "comparing with [] inside parenthesis is not recommended",
 			Content: `
 resource "aws_db_instance" "mysql" {
-	count = true == true || false != true && (false == false || [] == []) ? 1 : 0
+	count = ([] == []) ? 1 : 0
 	instance_class = "m4.2xlarge"
 }`,
 			Expected: tflint.Issues{
@@ -54,17 +73,8 @@ resource "aws_db_instance" "mysql" {
 					Message: "Comparing a collection with an empty list is invalid. To detect an empty collection, check its length.",
 					Range: hcl.Range{
 						Filename: "resource.tf",
-						Start:    hcl.Pos{Line: 3, Column: 62},
-						End:      hcl.Pos{Line: 3, Column: 70},
-					},
-				},
-				{
-					Rule:    NewTerraformEmptyListEqualityRule(),
-					Message: "Comparing a collection with an empty list is invalid. To detect an empty collection, check its length.",
-					Range: hcl.Range{
-						Filename: "resource.tf",
-						Start:    hcl.Pos{Line: 3, Column: 62},
-						End:      hcl.Pos{Line: 3, Column: 70},
+						Start:    hcl.Pos{Line: 3, Column: 11},
+						End:      hcl.Pos{Line: 3, Column: 19},
 					},
 				},
 			},