Fix issues caused by multiple terraform blocks (#216)

See also #205

Some rules that reference terraform blocks do not assume that
blocks can be defined multiple times and can lead to inconsistent results in that case.

Author: wata727

rules/terraform_required_providers.go

@@ -151,7 +151,9 @@ func (r *TerraformRequiredProvidersRule) Check(rr tflint.Runner) error {
 	requiredProviders := hclext.Attributes{}
 	for _, terraform := range body.Blocks {
 		for _, requiredProvidersBlock := range terraform.Body.Blocks {
-			requiredProviders = requiredProvidersBlock.Body.Attributes
+			for name, attr := range requiredProvidersBlock.Body.Attributes {
+				requiredProviders[name] = attr
+			}
 		}
 	}
 

rules/terraform_required_providers_test.go

@@ -1,10 +1,14 @@
 package rules
 
 import (
+	"reflect"
 	"testing"
 
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/hashicorp/hcl/v2"
 	"github.com/terraform-linters/tflint-plugin-sdk/helper"
+	"github.com/terraform-linters/tflint-plugin-sdk/tflint"
 )
 
 func Test_TerraformRequiredProvidersRule(t *testing.T) {
@@ -570,6 +574,107 @@ output "foo" {
 				},
 			},
 		},
+		{
+			Name: "multiple required providers",
+			Content: `
+terraform {
+  required_providers {
+    template = "~> 2"
+  }
+
+  required_providers {
+    aws = "~> 5.0"
+  }
+}
+
+provider "template" {}
+provider "aws" {}
+provider "google" {}
+
+terraform {
+  required_providers {
+    google = "~> 6.0"
+  }
+}
+`,
+			Expected: helper.Issues{
+				{
+					Rule:    NewTerraformRequiredProvidersRule(),
+					Message: "Legacy version constraint for provider \"template\" in `required_providers`",
+					Range: hcl.Range{
+						Filename: "module.tf",
+						Start: hcl.Pos{
+							Line:   4,
+							Column: 16,
+						},
+						End: hcl.Pos{
+							Line:   4,
+							Column: 22,
+						},
+					},
+				},
+				{
+					Rule:    NewTerraformRequiredProvidersRule(),
+					Message: "Legacy version constraint for provider \"aws\" in `required_providers`",
+					Range: hcl.Range{
+						Filename: "module.tf",
+						Start: hcl.Pos{
+							Line:   8,
+							Column: 11,
+						},
+						End: hcl.Pos{
+							Line:   8,
+							Column: 19,
+						},
+					},
+				},
+				{
+					Rule:    NewTerraformRequiredProvidersRule(),
+					Message: "Legacy version constraint for provider \"google\" in `required_providers`",
+					Range: hcl.Range{
+						Filename: "module.tf",
+						Start: hcl.Pos{
+							Line:   18,
+							Column: 14,
+						},
+						End: hcl.Pos{
+							Line:   18,
+							Column: 22,
+						},
+					},
+				},
+			},
+			Fixed: `
+terraform {
+  required_providers {
+    template = {
+      source  = "hashicorp/template"
+      version = "~> 2"
+    }
+  }
+
+  required_providers {
+    aws = {
+      source  = "hashicorp/aws"
+      version = "~> 5.0"
+    }
+  }
+}
+
+provider "template" {}
+provider "aws" {}
+provider "google" {}
+
+terraform {
+  required_providers {
+    google = {
+      source  = "hashicorp/google"
+      version = "~> 6.0"
+    }
+  }
+}
+`,
+		},
 	}
 
 	rule := NewTerraformRequiredProvidersRule()
@@ -590,7 +695,35 @@ output "foo" {
 				t.Fatalf("Unexpected error occurred: %s", err)
 			}
 
-			helper.AssertIssues(t, tc.Expected, runner.Runner.(*helper.Runner).Issues)
+			// TODO: replace the following assertions without ordering by AssertIssues
+			// helper.AssertIssues(t, tc.Expected, runner.Runner.(*helper.Runner).Issues)
+			opts := []cmp.Option{
+				cmpopts.IgnoreFields(hcl.Pos{}, "Byte"),
+				cmp.Comparer(func(x, y tflint.Rule) bool {
+					return reflect.TypeOf(x) == reflect.TypeOf(y)
+				}),
+				cmpopts.SortSlices(func(i, j *helper.Issue) bool {
+					if i.Range.Filename != j.Range.Filename {
+						return i.Range.Filename < j.Range.Filename
+					}
+					if i.Range.Start.Line != j.Range.Start.Line {
+						return i.Range.Start.Line < j.Range.Start.Line
+					}
+					if i.Range.Start.Column != j.Range.Start.Column {
+						return i.Range.Start.Column < j.Range.Start.Column
+					}
+					if i.Range.End.Line != j.Range.End.Line {
+						return i.Range.End.Line > j.Range.End.Line
+					}
+					if i.Range.End.Column != j.Range.End.Column {
+						return i.Range.End.Column > j.Range.End.Column
+					}
+					return i.Message < j.Message
+				}),
+			}
+			if diff := cmp.Diff(tc.Expected, runner.Runner.(*helper.Runner).Issues, opts...); diff != "" {
+				t.Fatalf("Expected issues are not matched:\n %s\n", diff)
+			}
 			want := map[string]string{}
 			if tc.Fixed != "" {
 				want[filename] = tc.Fixed

rules/terraform_unused_required_providers.go

@@ -82,8 +82,10 @@ func (r *TerraformUnusedRequiredProvidersRule) Check(rr tflint.Runner) error {
 			}
 
 			for _, block := range content.Blocks {
-				var attrDiags hcl.Diagnostics
-				requiredProviders, attrDiags = block.Body.JustAttributes()
+				attributes, attrDiags := block.Body.JustAttributes()
+				for k, v := range attributes {
+					requiredProviders[k] = v
+				}
 				diags = diags.Extend(attrDiags)
 				if diags.HasErrors() {
 					continue

rules/terraform_unused_required_providers_test.go

@@ -260,6 +260,43 @@ func Test_TerraformUnusedRequiredProvidersRule(t *testing.T) {
 			`,
 			Expected: helper.Issues{},
 		},
+		{
+			Name: "unused - in multiple terraform blocks",
+			Content: `
+				terraform {
+					required_providers {
+						aws = {
+							source = "hashicorp/aws"
+						}
+					}
+				}
+				terraform {
+					required_providers {
+						null = {
+							source = "hashicorp/null"
+						}
+					}
+				}
+				resource "null_resource" "foo" {}
+			`,
+			Expected: helper.Issues{
+				{
+					Rule:    NewTerraformUnusedRequiredProvidersRule(),
+					Message: "provider 'aws' is declared in required_providers but not used by the module",
+					Range: hcl.Range{
+						Filename: "module.tf",
+						Start: hcl.Pos{
+							Line:   4,
+							Column: 7,
+						},
+						End: hcl.Pos{
+							Line:   6,
+							Column: 8,
+						},
+					},
+				},
+			},
+		},
 	}
 
 	rule := NewTerraformUnusedRequiredProvidersRule()

rules/terraform_workspace_remote.go

@@ -94,20 +94,15 @@ func (r *TerraformWorkspaceRemoteRule) Check(runner tflint.Runner) error {
 	}
 
 	var remoteBackend bool
-	var tf10Support bool
+	constraints := version.Constraints{}
 	for _, terraform := range body.Blocks {
 		for _, requiredVersion := range terraform.Body.Attributes {
 			err := runner.EvaluateExpr(requiredVersion.Expr, func(v string) error {
-				constraints, err := version.NewConstraint(v)
+				c, err := version.NewConstraint(v)
 				if err != nil {
 					return err
 				}
-
-				for _, tf10Version := range tf10Versions {
-					if constraints.Check(tf10Version) {
-						tf10Support = true
-					}
-				}
+				constraints = append(constraints, c...)
 				return nil
 			}, nil)
 			if err != nil {
@@ -121,6 +116,14 @@ func (r *TerraformWorkspaceRemoteRule) Check(runner tflint.Runner) error {
 			}
 		}
 	}
+	var tf10Support bool
+	if constraints.Len() > 0 {
+		for _, tf10Version := range tf10Versions {
+			if constraints.Check(tf10Version) {
+				tf10Support = true
+			}
+		}
+	}
 	if !remoteBackend || !tf10Support {
 		return nil
 	}

rules/terraform_workspace_remote_test.go

@@ -296,6 +296,23 @@ resource "kubernetes_secret" "my_secret" {
       data
     ]
   }
+}`,
+			Expected: helper.Issues{},
+		},
+		{
+			Name: "required_version does not support 1.0.x by multiple setting",
+			Content: `
+terraform {
+	required_version = "< 1.9"
+	backend "remote" {}
+}
+terraform {
+	required_version = ">= 1.1"
+}
+resource "null_resource" "a" {
+	triggers = {
+		w = terraform.workspace
+	}
 }`,
 			Expected: helper.Issues{},
 		},