Skip to content

Commit

Permalink
fix: improvements after code review
Browse files Browse the repository at this point in the history
  • Loading branch information
bmbferreira committed May 16, 2022
1 parent 926ceda commit 0fb3337
Show file tree
Hide file tree
Showing 6 changed files with 40 additions and 48 deletions.
2 changes: 1 addition & 1 deletion docs/rules/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +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_empty_list_equality](terraform_empty_list_equality.md)|Disallow comparisons with `[]` when checking if a collection 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 Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# terraform_empty_list_check
# terraform_empty_list_equality

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

## Example

Expand All @@ -18,19 +18,19 @@ resource "aws_db_instance" "mysql" {
$ tflint
1 issue(s) found:
Warning: List is compared with [] instead of checking if length is 0. (terraform_empty_list_check)
Warning: Comparing a collection with an empty list is invalid. To detect an empty collection, check its length. (terraform_empty_list_equality)
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
Reference: https://github.com/terraform-linters/tflint/blob/master/docs/rules/terraform_empty_list_equality.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.
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 collection 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`.
Check if a collection is empty by checking its length instead. For example: `length(var.my_list) == 0`.
2 changes: 1 addition & 1 deletion rules/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ var DefaultRules = []Rule{
terraformrules.NewTerraformDeprecatedInterpolationRule(),
terraformrules.NewTerraformDocumentedOutputsRule(),
terraformrules.NewTerraformDocumentedVariablesRule(),
terraformrules.NewTerraformEmptyListCheckRule(),
terraformrules.NewTerraformEmptyListEqualityRule(),
terraformrules.NewTerraformModulePinnedSourceRule(),
terraformrules.NewTerraformModuleVersionRule(),
terraformrules.NewTerraformNamingConventionRule(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,58 +8,51 @@ import (
"github.com/terraform-linters/tflint/tflint"
)

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

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

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

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

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

// Link returns the rule reference link
func (r *TerraformEmptyListCheckRule) Link() string {
func (r *TerraformEmptyListEqualityRule) 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 {
func (r *TerraformEmptyListEqualityRule) 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
}
if err := r.checkEmptyList(runner); err != nil {
return err
}

return nil
}

func (r *TerraformEmptyListCheckRule) checkEmptyList(runner *tflint.Runner, filename string) error {
func (r *TerraformEmptyListEqualityRule) checkEmptyList(runner *tflint.Runner) error {
return runner.WalkExpressions(func(expr hcl.Expression) error {
if conditionalExpr, ok := expr.(*hclsyntax.ConditionalExpr); ok {
if binaryOpExpr, ok := conditionalExpr.Condition.(*hclsyntax.BinaryOpExpr); ok {
Expand All @@ -77,7 +70,7 @@ func (r *TerraformEmptyListCheckRule) checkEmptyList(runner *tflint.Runner, file
})
}

func checkEmptyList(tupleConsExpr *hclsyntax.TupleConsExpr, runner *tflint.Runner, r *TerraformEmptyListCheckRule, binaryOpExpr *hclsyntax.BinaryOpExpr) {
func checkEmptyList(tupleConsExpr *hclsyntax.TupleConsExpr, runner *tflint.Runner, r *TerraformEmptyListEqualityRule, binaryOpExpr *hclsyntax.BinaryOpExpr) {
if len(tupleConsExpr.Exprs) == 0 {
runner.EmitIssue(
r,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
"github.com/terraform-linters/tflint/tflint"
)

func Test_TerraformEmptyListCheckRule(t *testing.T) {
func Test_TerraformEmptyListEqualityRule(t *testing.T) {
cases := []struct {
Name string
Content string
Expand All @@ -16,21 +16,27 @@ func Test_TerraformEmptyListCheckRule(t *testing.T) {
{
Name: "comparing with [] is not recommended",
Content: `
variable "my_list" {
type = list(string)
}
resource "aws_db_instance" "mysql" {
count = var.my_list == [] ? 0 : 1
count = [] == [] ? 0 : 1
instance_class = "m4.2xlarge"
}`,
Expected: tflint.Issues{
{
Rule: NewTerraformEmptyListCheckRule(),
Message: "List is compared with [] instead of checking if length is 0.",
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: 6, Column: 10},
End: hcl.Pos{Line: 6, Column: 27},
Start: hcl.Pos{Line: 3, Column: 10},
End: hcl.Pos{Line: 3, Column: 18},
},
},
{
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: 10},
End: hcl.Pos{Line: 3, Column: 18},
},
},
},
Expand All @@ -47,8 +53,8 @@ resource "aws_db_instance" "mysql" {
}`,
Expected: tflint.Issues{
{
Rule: NewTerraformEmptyListCheckRule(),
Message: "List is compared with [] instead of checking if length is 0.",
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: 6, Column: 10},
Expand All @@ -71,7 +77,7 @@ resource "aws_db_instance" "mysql" {
},
}

rule := NewTerraformEmptyListCheckRule()
rule := NewTerraformEmptyListEqualityRule()

for _, tc := range cases {
runner := tflint.TestRunner(t, map[string]string{"resource.tf": tc.Content})
Expand Down
7 changes: 0 additions & 7 deletions test.tf

This file was deleted.

0 comments on commit 0fb3337

Please sign in to comment.