diff --git a/go.mod b/go.mod index a51c9d0..eb68b7e 100644 --- a/go.mod +++ b/go.mod @@ -9,7 +9,7 @@ require ( github.com/hashicorp/go-version v1.6.0 github.com/hashicorp/hcl/v2 v2.17.0 github.com/hashicorp/terraform-registry-address v0.2.0 - github.com/terraform-linters/tflint-plugin-sdk v0.16.1 + github.com/terraform-linters/tflint-plugin-sdk v0.16.2-0.20230605170513-64de942491dc github.com/zclconf/go-cty v1.13.2 ) diff --git a/go.sum b/go.sum index 54c34cd..03d6672 100644 --- a/go.sum +++ b/go.sum @@ -407,8 +407,8 @@ github.com/stretchr/testify v1.7.2/go.mod h1:R6va5+xMeoiuVRoj+gSkQ7d3FALtqAAGI1F github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= github.com/stretchr/testify v1.8.1 h1:w7B6lhMri9wdJUVmEZPGGhZzrYTPvgJArz7wNPgYKsk= github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= -github.com/terraform-linters/tflint-plugin-sdk v0.16.1 h1:fBfLL8KzP3pkQrNp3iQxaGoKBoMo2sFYoqmhuo6yc+A= -github.com/terraform-linters/tflint-plugin-sdk v0.16.1/go.mod h1:ltxVy04PRwptL6P/Ugz2ZeTNclYapClrLn/kVFXJGzo= +github.com/terraform-linters/tflint-plugin-sdk v0.16.2-0.20230605170513-64de942491dc h1:z0PQWOWfWOYps2Oo7nT/v9XASazFZITnMA+oGWZzB78= +github.com/terraform-linters/tflint-plugin-sdk v0.16.2-0.20230605170513-64de942491dc/go.mod h1:ltxVy04PRwptL6P/Ugz2ZeTNclYapClrLn/kVFXJGzo= github.com/ulikunitz/xz v0.5.10 h1:t92gobL9l3HE202wg3rlk19F6X+JOxl9BBrCCMYEYd8= github.com/ulikunitz/xz v0.5.10/go.mod h1:nbz6k7qbPmH4IRqmfOplQw/tblSgqTqBwxkY0oWt/14= github.com/vmihailenco/msgpack/v5 v5.3.5 h1:5gO0H1iULLWGhs2H5tbAHIZTV8/cYafcFOr9znI5mJU= diff --git a/rules/terraform_comment_syntax.go b/rules/terraform_comment_syntax.go index ff162ef..eeb689f 100644 --- a/rules/terraform_comment_syntax.go +++ b/rules/terraform_comment_syntax.go @@ -79,10 +79,13 @@ func (r *TerraformCommentSyntaxRule) checkComments(runner tflint.Runner, filenam } if strings.HasPrefix(string(token.Bytes), "//") { - if err := runner.EmitIssue( + if err := runner.EmitIssueWithFix( r, "Single line comments should begin with #", token.Range, + func(f tflint.Fixer) error { + return f.ReplaceText(f.RangeTo("//", filename, token.Range.Start), "#") + }, ); err != nil { return err } diff --git a/rules/terraform_comment_syntax_test.go b/rules/terraform_comment_syntax_test.go index a120121..ded48ea 100644 --- a/rules/terraform_comment_syntax_test.go +++ b/rules/terraform_comment_syntax_test.go @@ -13,6 +13,7 @@ func Test_TerraformCommentSyntaxRule(t *testing.T) { Content string JSON bool Expected helper.Issues + Fixed string }{ { Name: "hash comment", @@ -48,6 +49,7 @@ func Test_TerraformCommentSyntaxRule(t *testing.T) { }, }, }, + Fixed: `# foo`, }, { Name: "end-of-line hash comment", @@ -82,6 +84,11 @@ variable "foo" { } helper.AssertIssues(t, tc.Expected, runner.Issues) + want := map[string]string{} + if tc.Fixed != "" { + want[filename] = tc.Fixed + } + helper.AssertChanges(t, want, runner.Changes()) }) } } diff --git a/rules/terraform_deprecated_index.go b/rules/terraform_deprecated_index.go index e302970..c6996a7 100644 --- a/rules/terraform_deprecated_index.go +++ b/rules/terraform_deprecated_index.go @@ -72,15 +72,18 @@ func (r *TerraformDeprecatedIndexRule) Check(runner tflint.Runner) error { r.checkLegacyTraversalIndex(runner, expr.Traversal, file.Bytes) case *hclsyntax.SplatExpr: if strings.HasPrefix(string(expr.MarkerRange.SliceBytes(file.Bytes)), ".") { - if err := runner.EmitIssue( + if err := runner.EmitIssueWithFix( r, "List items should be accessed using square brackets", expr.MarkerRange, + func(f tflint.Fixer) error { + return f.ReplaceText(expr.MarkerRange, "[*]") + }, ); err != nil { return hcl.Diagnostics{ { Severity: hcl.DiagError, - Summary: "failed to call EmitIssue()", + Summary: "failed to call EmitIssueWithFix()", Detail: err.Error(), }, } @@ -98,17 +101,20 @@ func (r *TerraformDeprecatedIndexRule) Check(runner tflint.Runner) error { func (r *TerraformDeprecatedIndexRule) checkLegacyTraversalIndex(runner tflint.Runner, traversal hcl.Traversal, file []byte) hcl.Diagnostics { for _, t := range traversal { - if _, ok := t.(hcl.TraverseIndex); ok { + if tn, ok := t.(hcl.TraverseIndex); ok { if strings.HasPrefix(string(t.SourceRange().SliceBytes(file)), ".") { - if err := runner.EmitIssue( + if err := runner.EmitIssueWithFix( r, "List items should be accessed using square brackets", t.SourceRange(), + func(f tflint.Fixer) error { + return f.ReplaceText(t.SourceRange(), "[", f.ValueText(tn.Key), "]") + }, ); err != nil { return hcl.Diagnostics{ { Severity: hcl.DiagError, - Summary: "failed to call EmitIssue()", + Summary: "failed to call EmitIssueWithFix()", Detail: err.Error(), }, } diff --git a/rules/terraform_deprecated_index_test.go b/rules/terraform_deprecated_index_test.go index c44bc5e..6765f3e 100644 --- a/rules/terraform_deprecated_index_test.go +++ b/rules/terraform_deprecated_index_test.go @@ -13,12 +13,13 @@ func Test_TerraformDeprecatedIndexRule(t *testing.T) { Content string JSON bool Expected helper.Issues + Fixed string }{ { Name: "deprecated dot index style", Content: ` locals { - list = ["a"] + list = ["a"] value = list.0 } `, @@ -39,13 +40,19 @@ locals { }, }, }, + Fixed: ` +locals { + list = ["a"] + value = list[0] +} +`, }, { Name: "deprecated dot splat index style", Content: ` locals { - maplist = [{a = "b"}] - values = maplist.*.a + maplist = [{ a = "b" }] + values = maplist.*.a } `, Expected: helper.Issues{ @@ -56,21 +63,27 @@ locals { Filename: "config.tf", Start: hcl.Pos{ Line: 4, - Column: 19, + Column: 20, }, End: hcl.Pos{ Line: 4, - Column: 21, + Column: 22, }, }, }, }, + Fixed: ` +locals { + maplist = [{ a = "b" }] + values = maplist[*].a +} +`, }, { Name: "attribute access", Content: ` locals { - map = {a = "b"} + map = { a = "b" } value = map.a } `, @@ -90,9 +103,9 @@ locals { Content: ` locals { servers = < 2" + } + }, + "provider": { + "template": {} + } +}`, + JSON: true, + Expected: helper.Issues{ + { + Rule: NewTerraformRequiredProvidersRule(), + Message: "Legacy version constraint for provider \"template\" in `required_providers`", + Range: hcl.Range{ + Filename: "module.tf.json", + Start: hcl.Pos{ + Line: 5, + Column: 19, + }, + End: hcl.Pos{ + Line: 5, + Column: 25, + }, + }, + }, + }, + }, } rule := NewTerraformRequiredProvidersRule() for _, tc := range cases { t.Run(tc.Name, func(t *testing.T) { + filename := "module.tf" + if tc.JSON { + filename += ".json" + } + runner := testRunner(t, map[string]string{ - "module.tf": tc.Content, + filename: tc.Content, ".tflint.hcl": tc.Config, }) @@ -494,6 +567,11 @@ resource "google_compute_instance" "foo" { } helper.AssertIssues(t, tc.Expected, runner.Runner.(*helper.Runner).Issues) + want := map[string]string{} + if tc.Fixed != "" { + want[filename] = tc.Fixed + } + helper.AssertChanges(t, want, runner.Runner.(*helper.Runner).Changes()) }) } } diff --git a/rules/terraform_unused_declarations.go b/rules/terraform_unused_declarations.go index 5387a7c..dc490ce 100644 --- a/rules/terraform_unused_declarations.go +++ b/rules/terraform_unused_declarations.go @@ -74,28 +74,31 @@ func (r *TerraformUnusedDeclarationsRule) Check(rr tflint.Runner) error { } for _, variable := range decl.Variables { - if err := runner.EmitIssue( + if err := runner.EmitIssueWithFix( r, fmt.Sprintf(`variable "%s" is declared but not used`, variable.Labels[0]), variable.DefRange, + func(f tflint.Fixer) error { return f.RemoveExtBlock(variable) }, ); err != nil { return err } } for _, data := range decl.DataResources { - if err := runner.EmitIssue( + if err := runner.EmitIssueWithFix( r, fmt.Sprintf(`data "%s" "%s" is declared but not used`, data.Labels[0], data.Labels[1]), data.DefRange, + func(f tflint.Fixer) error { return f.RemoveExtBlock(data) }, ); err != nil { return err } } for _, local := range decl.Locals { - if err := runner.EmitIssue( + if err := runner.EmitIssueWithFix( r, fmt.Sprintf(`local.%s is declared but not used`, local.Name), local.DefRange, + func(f tflint.Fixer) error { return f.RemoveAttribute(local.Attribute) }, ); err != nil { return err } diff --git a/rules/terraform_unused_declarations_test.go b/rules/terraform_unused_declarations_test.go index 71c3272..f66bdff 100644 --- a/rules/terraform_unused_declarations_test.go +++ b/rules/terraform_unused_declarations_test.go @@ -13,6 +13,7 @@ func Test_TerraformUnusedDeclarationsRule(t *testing.T) { Content string JSON bool Expected helper.Issues + Fixed string }{ { Name: "unused variable", @@ -32,6 +33,10 @@ output "u" { value = var.used } }, }, }, + Fixed: ` +variable "used" {} +output "u" { value = var.used } +`, }, { Name: "unused data source", @@ -51,13 +56,17 @@ output "u" { value = data.null_data_source.used } }, }, }, + Fixed: ` +data "null_data_source" "used" {} +output "u" { value = data.null_data_source.used } +`, }, { Name: "unused local source", Content: ` locals { - not_used = "" - used = "" + not_used = "" + used = "" } output "u" { value = local.used } `, @@ -67,20 +76,26 @@ output "u" { value = local.used } Message: `local.not_used is declared but not used`, Range: hcl.Range{ Filename: "config.tf", - Start: hcl.Pos{Line: 3, Column: 2}, - End: hcl.Pos{Line: 3, Column: 15}, + Start: hcl.Pos{Line: 3, Column: 3}, + End: hcl.Pos{Line: 3, Column: 16}, }, }, }, + Fixed: ` +locals { + used = "" +} +output "u" { value = local.used } +`, }, { Name: "variable used in resource", Content: ` variable "used" {} resource "null_resource" "n" { - triggers = { - u = var.used - } + triggers = { + u = var.used + } } `, Expected: helper.Issues{}, @@ -90,8 +105,8 @@ resource "null_resource" "n" { Content: ` variable "used" {} module "m" { - source = "./module" - u = var.used + source = "./module" + u = var.used } `, Expected: helper.Issues{}, @@ -101,8 +116,8 @@ module "m" { Content: ` variable "used" {} module "m" { - source = "./module" - u = var.used + source = "./module" + u = var.used } `, Expected: helper.Issues{}, @@ -112,8 +127,8 @@ module "m" { Content: ` locals { used = "used" } module "m" { - source = "./module" - u = local.used + source = "./module" + u = local.used } `, Expected: helper.Issues{}, @@ -123,7 +138,7 @@ module "m" { Content: ` variable "aws_region" {} provider "aws" { - region = var.aws_region + region = var.aws_region } `, Expected: helper.Issues{}, @@ -135,8 +150,8 @@ variable "used" {} resource "null_resource" "n" { triggers = { u = var.used - } - + } + lifecycle { ignore_changes = [triggers] } @@ -152,14 +167,14 @@ resource "null_resource" "n" { Name: "additional traversal", Content: ` variable "v" { - type = object({ foo = string }) + type = object({ foo = string }) } output "v" { - value = var.v.foo + value = var.v.foo } data "terraform_remote_state" "d" {} output "d" { - value = data.terraform_remote_state.d.outputs.foo + value = data.terraform_remote_state.d.outputs.foo } `, Expected: helper.Issues{}, @@ -177,13 +192,34 @@ output "d" { }] } } - }, + }, "variable": { "again": {} } }`, Expected: helper.Issues{}, }, + { + Name: "json with unused variable", + JSON: true, + Content: ` +{ + "variable": { + "again": {} + } +}`, + Expected: helper.Issues{ + { + Rule: NewTerraformUnusedDeclarationsRule(), + Message: `variable "again" is declared but not used`, + Range: hcl.Range{ + Filename: "config.tf.json", + Start: hcl.Pos{Line: 4, Column: 14}, + End: hcl.Pos{Line: 4, Column: 15}, + }, + }, + }, + }, } rule := NewTerraformUnusedDeclarationsRule() @@ -201,7 +237,14 @@ output "d" { t.Fatalf("Unexpected error occurred: %s", err) } - helper.AssertIssues(t, tc.Expected, runner.Runner.(*helper.Runner).Issues) + helperRunner := runner.Runner.(*helper.Runner) + + helper.AssertIssues(t, tc.Expected, helperRunner.Issues) + want := map[string]string{} + if tc.Fixed != "" { + want[filename] = tc.Fixed + } + helper.AssertChanges(t, want, helperRunner.Changes()) }) } } diff --git a/terraform/ruleset.go b/terraform/ruleset.go index 4c7d037..9b66d88 100644 --- a/terraform/ruleset.go +++ b/terraform/ruleset.go @@ -107,14 +107,7 @@ func (r *RuleSet) ApplyConfig(body *hclext.BodyContent) error { return nil } -// Check runs inspection for each rule by applying Runner. -func (r *RuleSet) Check(rr tflint.Runner) error { - runner := NewRunner(rr) - - for _, rule := range r.EnabledRules { - if err := rule.Check(runner); err != nil { - return fmt.Errorf("Failed to check `%s` rule: %s", rule.Name(), err) - } - } - return nil +// NewRunner injects a custom runner +func (r *RuleSet) NewRunner(runner tflint.Runner) (tflint.Runner, error) { + return NewRunner(runner), nil } diff --git a/terraform/runner.go b/terraform/runner.go index 48f1ab4..48d41c4 100644 --- a/terraform/runner.go +++ b/terraform/runner.go @@ -93,8 +93,9 @@ func (r *Runner) GetLocals() (map[string]*Local, hcl.Diagnostics) { for name, attr := range attrs { locals[name] = &Local{ - Name: attr.Name, - DefRange: attr.Range, + Name: attr.Name, + Attribute: attr, + DefRange: attr.Range, } } } diff --git a/terraform/runner_test.go b/terraform/runner_test.go index 1f630ea..e7bd206 100644 --- a/terraform/runner_test.go +++ b/terraform/runner_test.go @@ -165,6 +165,7 @@ locals { opts := []cmp.Option{ cmpopts.IgnoreFields(hcl.Pos{}, "Byte"), + cmpopts.IgnoreFields(Local{}, "Attribute"), } if diff := cmp.Diff(got, test.want, opts...); diff != "" { t.Error(diff) diff --git a/terraform/terraform.go b/terraform/terraform.go index a35ec68..c919c5a 100644 --- a/terraform/terraform.go +++ b/terraform/terraform.go @@ -59,8 +59,9 @@ func decodeModuleCall(block *hclext.Block) (*ModuleCall, hcl.Diagnostics) { // Local represents a single entry from a "locals" block. type Local struct { - Name string - DefRange hcl.Range + Name string + Attribute *hcl.Attribute + DefRange hcl.Range } // ProviderRef represents a reference to a provider like `provider = google.europe` in a resource or module.