Skip to content

Commit

Permalink
feat: add new rule to check comparisons with empty list
Browse files Browse the repository at this point in the history
  • Loading branch information
bmbferreira committed Apr 18, 2022
1 parent 642c99c commit 2b18257
Show file tree
Hide file tree
Showing 6 changed files with 219 additions and 0 deletions.
2 changes: 2 additions & 0 deletions docs/rules/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ Below is a list of available rules.
|[terraform_deprecated_interpolation](terraform_deprecated_interpolation.md)|Disallow deprecated (0.11-style) interpolation||
|[terraform_documented_outputs](terraform_documented_outputs.md)|Disallow `output` declarations without description||
|[terraform_documented_variables](terraform_documented_variables.md)|Disallow `variable` declarations without description||
|[terraform_empty_list_check](terraform_empty_list_check.md)|Disallow comparisons with `[]` when checking if a list is empty||
|[terraform_module_pinned_source](terraform_module_pinned_source.md)|Disallow specifying a git or mercurial repository as a module source without pinning to a version||
|[terraform_module_version](terraform_module_version.md)|Checks that Terraform modules sourced from a registry specify a version||
|[terraform_naming_convention](terraform_naming_convention.md)|Enforces naming conventions for resources, data sources, etc||
Expand All @@ -21,3 +22,4 @@ Below is a list of available rules.
|[terraform_unused_declarations](terraform_unused_declarations.md)|Disallow variables, data sources, and locals that are declared but never used||
|[terraform_unused_required_providers](terraform_unused_required_providers.md)|Check that all `required_providers` are used in the module||
|[terraform_workspace_remote](terraform_workspace_remote.md)|`terraform.workspace` should not be used with a "remote" backend with remote execution||

36 changes: 36 additions & 0 deletions docs/rules/terraform_empty_list_check.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
# terraform_empty_list_check

Disallow comparisons with `[]` when checking if a list is empty.

## Example

```hcl
variable "my_list" {
type = list(string)
}
resource "aws_db_instance" "mysql" {
count = var.my_list == [] ? 0 : 1
instance_class = "m4.2xlarge"
}
```

```
$ tflint
1 issue(s) found:
Warning: List is compared with [] instead of checking if length is 0. (terraform_empty_list_check)
on test.tf line 5:
5: count = var.my_list == [] ? 0 : 1
Reference: https://github.com/terraform-linters/tflint/blob/master/docs/rules/terraform_empty_list_check.md
```

## Why

The `==` operator can only return true when the two operands have identical types, and the type of `[]` alone (without any further type conversions) is an empty tuple rather than a list of objects, strings, numbers or any other type. Therefore, a comparison with a single `[]` with the goal of checking if a list is empty, will always return false.

## How To Fix

Check if a list is empty by checking its length instead. For example: `length(var.my_list) == 0`.
1 change: 1 addition & 0 deletions rules/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ var DefaultRules = []Rule{
terraformrules.NewTerraformDeprecatedInterpolationRule(),
terraformrules.NewTerraformDocumentedOutputsRule(),
terraformrules.NewTerraformDocumentedVariablesRule(),
terraformrules.NewTerraformEmptyListCheckRule(),
terraformrules.NewTerraformModulePinnedSourceRule(),
terraformrules.NewTerraformModuleVersionRule(),
terraformrules.NewTerraformNamingConventionRule(),
Expand Down
88 changes: 88 additions & 0 deletions rules/terraformrules/terraform_empty_list_check.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
package terraformrules

import (
"log"

"github.com/hashicorp/hcl/v2"
"github.com/hashicorp/hcl/v2/hclsyntax"
"github.com/terraform-linters/tflint/tflint"
)

// TerraformEmptyListCheckRule checks whether is there a comparison with an empty list
type TerraformEmptyListCheckRule struct{}

// NewTerraformCommentSyntaxRule returns a new rule
func NewTerraformEmptyListCheckRule() *TerraformEmptyListCheckRule {
return &TerraformEmptyListCheckRule{}
}

// Name returns the rule name
func (r *TerraformEmptyListCheckRule) Name() string {
return "terraform_empty_list_check"
}

// Enabled returns whether the rule is enabled by default
func (r *TerraformEmptyListCheckRule) Enabled() bool {
return true
}

// Severity returns the rule severity
func (r *TerraformEmptyListCheckRule) Severity() tflint.Severity {
return tflint.WARNING
}

// Link returns the rule reference link
func (r *TerraformEmptyListCheckRule) Link() string {
return tflint.ReferenceLink(r.Name())
}

// Check checks whether the list is being compared with static empty list
func (r *TerraformEmptyListCheckRule) Check(runner *tflint.Runner) error {
if !runner.TFConfig.Path.IsRoot() {
// This rule does not evaluate child modules.
return nil
}

log.Printf("[TRACE] Check `%s` rule for `%s` runner", r.Name(), runner.TFConfigPath())

files := make(map[string]*struct{})
for _, variable := range runner.TFConfig.Module.Variables {
files[variable.DeclRange.Filename] = nil
}

for filename := range files {
if err := r.checkEmptyList(runner, filename); err != nil {
return err
}
}

return nil
}

func (r *TerraformEmptyListCheckRule) checkEmptyList(runner *tflint.Runner, filename string) error {
return runner.WalkExpressions(func(expr hcl.Expression) error {
if conditionalExpr, ok := expr.(*hclsyntax.ConditionalExpr); ok {
if binaryOpExpr, ok := conditionalExpr.Condition.(*hclsyntax.BinaryOpExpr); ok {
if binaryOpExpr.Op.Type.FriendlyName() == "bool" {
if right, ok := binaryOpExpr.RHS.(*hclsyntax.TupleConsExpr); ok {
checkEmptyList(right, runner, r, binaryOpExpr)
}
if left, ok := binaryOpExpr.LHS.(*hclsyntax.TupleConsExpr); ok {
checkEmptyList(left, runner, r, binaryOpExpr)
}
}
}
}
return nil
})
}

func checkEmptyList(tupleConsExpr *hclsyntax.TupleConsExpr, runner *tflint.Runner, r *TerraformEmptyListCheckRule, binaryOpExpr *hclsyntax.BinaryOpExpr) {
if len(tupleConsExpr.Exprs) == 0 {
runner.EmitIssue(
r,
"List is compared with [] instead of checking if length is 0.",
binaryOpExpr.Range(),
)
}
}
85 changes: 85 additions & 0 deletions rules/terraformrules/terraform_empty_list_check_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
package terraformrules

import (
"testing"

hcl "github.com/hashicorp/hcl/v2"
"github.com/terraform-linters/tflint/tflint"
)

func Test_TerraformEmptyListCheckRule(t *testing.T) {
cases := []struct {
Name string
Content string
Expected tflint.Issues
}{
{
Name: "comparing with [] is not recommended",
Content: `
variable "my_list" {
type = list(string)
}
resource "aws_db_instance" "mysql" {
count = var.my_list == [] ? 0 : 1
instance_class = "m4.2xlarge"
}`,
Expected: tflint.Issues{
{
Rule: NewTerraformEmptyListCheckRule(),
Message: "List is compared with [] instead of checking if length is 0.",
Range: hcl.Range{
Filename: "resource.tf",
Start: hcl.Pos{Line: 6, Column: 10},
End: hcl.Pos{Line: 6, Column: 27},
},
},
},
},
{
Name: "negatively comparing with [] is not recommended",
Content: `
variable "my_list" {
type = list(string)
}
resource "aws_db_instance" "mysql" {
count = var.my_list != [] ? 1 : 0
instance_class = "m4.2xlarge"
}`,
Expected: tflint.Issues{
{
Rule: NewTerraformEmptyListCheckRule(),
Message: "List is compared with [] instead of checking if length is 0.",
Range: hcl.Range{
Filename: "resource.tf",
Start: hcl.Pos{Line: 6, Column: 10},
End: hcl.Pos{Line: 6, Column: 27},
},
},
},
},
{
Name: "checking if length is 0 is recommended",
Content: `
variable "my_list" {
type = list(string)
}
resource "aws_db_instance" "mysql" {
count = length(var.my_list) == 0 ? 1 : 0
instance_class = "m4.2xlarge"
}`,
Expected: tflint.Issues{},
},
}

rule := NewTerraformEmptyListCheckRule()

for _, tc := range cases {
runner := tflint.TestRunner(t, map[string]string{"resource.tf": tc.Content})

if err := rule.Check(runner); err != nil {
t.Fatalf("Unexpected error occurred: %s", err)
}

tflint.AssertIssues(t, tc.Expected, runner.Issues)
}
}
7 changes: 7 additions & 0 deletions test.tf
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
variable "my_list" {
type = list(string)
}
resource "aws_db_instance" "mysql" {
count = var.my_list == [] ? 0 : 1
instance_class = "m4.2xlarge"
}

0 comments on commit 2b18257

Please sign in to comment.