From 55d612422428f35da6e9e043b7ba4476d6d09360 Mon Sep 17 00:00:00 2001 From: Kazuma Watanabe Date: Tue, 2 May 2023 13:08:24 +0000 Subject: [PATCH] Introduce autofix --- cmd/inspect.go | 117 ++++++++++++++++++++++++++++++++------- cmd/option.go | 1 + formatter/formatter.go | 1 + formatter/pretty.go | 18 +++++- go.mod | 2 +- go.sum | 4 +- plugin/server.go | 46 +++++++++++---- plugin/server_test.go | 16 +++--- terraform/module.go | 46 +++++++++++++-- terraform/parser.go | 10 ++-- terraform/parser_test.go | 65 +++++++++++----------- tflint/issue.go | 6 ++ tflint/runner.go | 67 +++++++++++++++++++--- tflint/runner_test.go | 2 +- 14 files changed, 305 insertions(+), 96 deletions(-) diff --git a/cmd/inspect.go b/cmd/inspect.go index 90706becf..1a12e002c 100644 --- a/cmd/inspect.go +++ b/cmd/inspect.go @@ -2,10 +2,12 @@ package cmd import ( "fmt" + "io" "os" "path/filepath" "strings" + "github.com/hashicorp/go-version" "github.com/hashicorp/hcl/v2" "github.com/spf13/afero" "github.com/terraform-linters/tflint-plugin-sdk/hclext" @@ -27,6 +29,7 @@ func (cli *CLI) inspect(opts Options, args []string) int { } issues := tflint.Issues{} + changes := map[string][]byte{} for _, wd := range workingDirs { err := cli.withinChangedDir(wd, func() error { @@ -62,11 +65,16 @@ func (cli *CLI) inspect(opts Options, args []string) int { for i, file := range filterFiles { filterFiles[i] = filepath.Join(wd, file) } - moduleIssues, err := cli.inspectModule(opts, targetDir, filterFiles) + + moduleIssues, moduleChanges, err := cli.inspectModule(opts, targetDir, filterFiles) if err != nil { return err } issues = append(issues, moduleIssues...) + for path, source := range moduleChanges { + changes[path] = source + } + return nil }) if err != nil { @@ -91,8 +99,16 @@ func (cli *CLI) inspect(opts Options, args []string) int { force = cli.config.Force } + cli.formatter.Fix = opts.Fix cli.formatter.Print(issues, nil, cli.sources) + if opts.Fix { + if err := writeChanges(changes); err != nil { + cli.formatter.Print(tflint.Issues{}, err, cli.sources) + return ExitCodeError + } + } + if len(issues) > 0 && !force && exceedsMinimumFailure(issues, opts.MinimumFailureSeverity) { return ExitCodeIssuesFound } @@ -143,75 +159,113 @@ func processArgs(args []string) (string, []string, error) { return dir, filterFiles, nil } -func (cli *CLI) inspectModule(opts Options, dir string, filterFiles []string) (tflint.Issues, error) { +func (cli *CLI) inspectModule(opts Options, dir string, filterFiles []string) (tflint.Issues, map[string][]byte, error) { issues := tflint.Issues{} + changes := map[string][]byte{} var err error // Setup config cli.config, err = tflint.LoadConfig(afero.Afero{Fs: afero.NewOsFs()}, opts.Config) if err != nil { - return tflint.Issues{}, fmt.Errorf("Failed to load TFLint config; %w", err) + return issues, changes, fmt.Errorf("Failed to load TFLint config; %w", err) } cli.config.Merge(opts.toConfig()) // Setup loader cli.loader, err = terraform.NewLoader(afero.Afero{Fs: afero.NewOsFs()}, cli.originalWorkingDir) if err != nil { - return tflint.Issues{}, fmt.Errorf("Failed to prepare loading; %w", err) + return issues, changes, fmt.Errorf("Failed to prepare loading; %w", err) } if opts.Recursive && !cli.loader.IsConfigDir(dir) { // Ignore non-module directories in recursive mode - return tflint.Issues{}, nil + return issues, changes, nil } // Setup runners runners, err := cli.setupRunners(opts, dir) if err != nil { - return tflint.Issues{}, err + return issues, changes, err } rootRunner := runners[len(runners)-1] // Launch plugin processes - rulesetPlugin, err := launchPlugins(cli.config) + rulesetPlugin, err := launchPlugins(cli.config, opts.Fix) if rulesetPlugin != nil { defer rulesetPlugin.Clean() } if err != nil { - return tflint.Issues{}, err + return issues, changes, err } - // Run inspection + // Check preconditions + sdkVersions := map[string]*version.Version{} for name, ruleset := range rulesetPlugin.RuleSets { sdkVersion, err := ruleset.SDKVersion() if err != nil { if st, ok := status.FromError(err); ok && st.Code() == codes.Unimplemented { // SDKVersion endpoint is available in tflint-plugin-sdk v0.14+. - return tflint.Issues{}, fmt.Errorf(`Plugin "%s" SDK version is incompatible. Compatible versions: %s`, name, plugin.SDKVersionConstraints) + return issues, changes, fmt.Errorf(`Plugin "%s" SDK version is incompatible. Compatible versions: %s`, name, plugin.SDKVersionConstraints) } else { - return tflint.Issues{}, fmt.Errorf(`Failed to get plugin "%s" SDK version; %w`, name, err) + return issues, changes, fmt.Errorf(`Failed to get plugin "%s" SDK version; %w`, name, err) } } if !plugin.SDKVersionConstraints.Check(sdkVersion) { - return tflint.Issues{}, fmt.Errorf(`Plugin "%s" SDK version (%s) is incompatible. Compatible versions: %s`, name, sdkVersion, plugin.SDKVersionConstraints) + return issues, changes, fmt.Errorf(`Plugin "%s" SDK version (%s) is incompatible. Compatible versions: %s`, name, sdkVersion, plugin.SDKVersionConstraints) + } + sdkVersions[name] = sdkVersion + } + + // Run inspection + // + // Repeat an inspection until there are no more changes or the limit is reached, + // in case an autofix introduces new issues. + for loop := 1; ; loop++ { + if loop > 10 { + return issues, changes, fmt.Errorf(`Reached the limit of autofix attempts, and the changes made by the autofix will not be applied. This may be due to the following reasons: + +1. The autofix is making changes that do not fix the issue. +2. The autofix is continuing to introduce new issues. + +By setting TFLINT_LOG=trace, you can confirm the changes made by the autofix and start troubleshooting.`) + } + + for name, ruleset := range rulesetPlugin.RuleSets { + for _, runner := range runners { + err = ruleset.Check(plugin.NewGRPCServer(runner, rootRunner, cli.loader.Files(), sdkVersions[name])) + if err != nil { + return issues, changes, fmt.Errorf("Failed to check ruleset; %w", err) + } + } } + changesInAttempt := map[string][]byte{} for _, runner := range runners { - err = ruleset.Check(plugin.NewGRPCServer(runner, rootRunner, cli.loader.Files(), sdkVersion)) - if err != nil { - return tflint.Issues{}, fmt.Errorf("Failed to check ruleset; %w", err) + for _, issue := range runner.LookupIssues(filterFiles...) { + // On the second attempt, only fixable issues are appended to avoid duplicates. + if loop == 1 || issue.Fixable { + issues = append(issues, issue) + } + } + runner.Issues = tflint.Issues{} + + for path, source := range runner.LookupChanges(filterFiles...) { + changesInAttempt[path] = source + changes[path] = source } + runner.ClearChanges() } - } - for _, runner := range runners { - issues = append(issues, runner.LookupIssues(filterFiles...)...) + if !opts.Fix || len(changesInAttempt) == 0 { + break + } } + // Set module sources to CLI for path, source := range cli.loader.Sources() { cli.sources[path] = source } - return issues, nil + return issues, changes, nil } func (cli *CLI) setupRunners(opts Options, dir string) ([]*tflint.Runner, error) { @@ -260,7 +314,7 @@ func (cli *CLI) setupRunners(opts Options, dir string) ([]*tflint.Runner, error) return append(runners, runner), nil } -func launchPlugins(config *tflint.Config) (*plugin.Plugin, error) { +func launchPlugins(config *tflint.Config, fix bool) (*plugin.Plugin, error) { // Lookup plugins rulesetPlugin, err := plugin.Discovery(config) if err != nil { @@ -269,6 +323,7 @@ func launchPlugins(config *tflint.Config) (*plugin.Plugin, error) { rulesets := []tflint.RuleSet{} pluginConf := config.ToPluginConfig() + pluginConf.Fix = fix // Check version constraints and apply a config to plugins for name, ruleset := range rulesetPlugin.RuleSets { @@ -316,6 +371,28 @@ func launchPlugins(config *tflint.Config) (*plugin.Plugin, error) { return rulesetPlugin, nil } +func writeChanges(changes map[string][]byte) error { + fs := afero.NewOsFs() + for path, source := range changes { + f, err := fs.OpenFile(path, os.O_WRONLY|os.O_TRUNC, 0644) + if err != nil { + return fmt.Errorf("Failed to apply autofixes; failed to open %s: %w", path, err) + } + + n, err := f.Write(source) + if err == nil && n < len(source) { + err = io.ErrShortWrite + } + if err1 := f.Close(); err == nil { + err = err1 + } + if err != nil { + return fmt.Errorf("Failed to apply autofixes; failed to write source code to %s: %w", path, err) + } + } + return nil +} + // Checks if the given issues contain severities above or equal to the given minimum failure opt. Defaults to true if an error occurs func exceedsMinimumFailure(issues tflint.Issues, minimumFailureOpt string) bool { if minimumFailureOpt != "" { diff --git a/cmd/option.go b/cmd/option.go index 7129e05f0..620eb92ef 100644 --- a/cmd/option.go +++ b/cmd/option.go @@ -30,6 +30,7 @@ type Options struct { MinimumFailureSeverity string `long:"minimum-failure-severity" description:"Sets minimum severity level for exiting with a non-zero error code" choice:"error" choice:"warning" choice:"notice"` Color bool `long:"color" description:"Enable colorized output"` NoColor bool `long:"no-color" description:"Disable colorized output"` + Fix bool `long:"fix" description:"Fix issues automatically"` ActAsBundledPlugin bool `long:"act-as-bundled-plugin" hidden:"true"` } diff --git a/formatter/formatter.go b/formatter/formatter.go index 065f136d3..e8aed2ab5 100644 --- a/formatter/formatter.go +++ b/formatter/formatter.go @@ -14,6 +14,7 @@ type Formatter struct { Stdout io.Writer Stderr io.Writer Format string + Fix bool NoColor bool } diff --git a/formatter/pretty.go b/formatter/pretty.go index f3ed2b6af..74b76a810 100644 --- a/formatter/pretty.go +++ b/formatter/pretty.go @@ -35,14 +35,28 @@ func (f *Formatter) prettyPrint(issues tflint.Issues, err error, sources map[str } func (f *Formatter) prettyPrintIssueWithSource(issue *tflint.Issue, sources map[string][]byte) { + message := issue.Message + if issue.Fixable { + if f.Fix { + message = "[Fixed] " + message + } else { + message = "[Fixable] " + message + } + } + fmt.Fprintf( f.Stdout, "%s: %s (%s)\n\n", - colorSeverity(issue.Rule.Severity()), colorBold(issue.Message), issue.Rule.Name(), + colorSeverity(issue.Rule.Severity()), colorBold(message), issue.Rule.Name(), ) fmt.Fprintf(f.Stdout, " on %s line %d:\n", issue.Range.Filename, issue.Range.Start.Line) - src := sources[issue.Range.Filename] + var src []byte + if issue.Source != nil { + src = issue.Source + } else { + src = sources[issue.Range.Filename] + } if src == nil { fmt.Fprintf(f.Stdout, " (source code not available)\n") diff --git a/go.mod b/go.mod index b0f371c64..2c3c062eb 100644 --- a/go.mod +++ b/go.mod @@ -24,7 +24,7 @@ require ( github.com/sourcegraph/go-lsp v0.0.0-20200429204803-219e11d77f5d github.com/sourcegraph/jsonrpc2 v0.2.0 github.com/spf13/afero v1.9.5 - github.com/terraform-linters/tflint-plugin-sdk v0.16.1 + github.com/terraform-linters/tflint-plugin-sdk v0.16.2-0.20230506154420-4f23c16c6801 github.com/terraform-linters/tflint-ruleset-terraform v0.2.2 github.com/xeipuuv/gojsonschema v1.2.0 github.com/zclconf/go-cty v1.13.1 diff --git a/go.sum b/go.sum index 454b35d80..2e865c297 100644 --- a/go.sum +++ b/go.sum @@ -457,8 +457,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.20230506154420-4f23c16c6801 h1:4s6pIfOOaiF89JClxNeRHTzYHM+0XU4U73mm5LxPJ0o= +github.com/terraform-linters/tflint-plugin-sdk v0.16.2-0.20230506154420-4f23c16c6801/go.mod h1:ltxVy04PRwptL6P/Ugz2ZeTNclYapClrLn/kVFXJGzo= github.com/terraform-linters/tflint-ruleset-terraform v0.2.2 h1:iTE09KkaZ0DE29xvp6IIM1/gmas9V0h8CER28SyBmQ8= github.com/terraform-linters/tflint-ruleset-terraform v0.2.2/go.mod h1:bCkvH8Vqzr16bWEE3e6Q3hvdZlmSAOR8i6G3M5y+M+k= github.com/ulikunitz/xz v0.5.10 h1:t92gobL9l3HE202wg3rlk19F6X+JOxl9BBrCCMYEYd8= diff --git a/plugin/server.go b/plugin/server.go index c491bb016..58fbd681e 100644 --- a/plugin/server.go +++ b/plugin/server.go @@ -8,6 +8,7 @@ import ( "github.com/hashicorp/go-version" hcl "github.com/hashicorp/hcl/v2" "github.com/terraform-linters/tflint-plugin-sdk/hclext" + "github.com/terraform-linters/tflint-plugin-sdk/plugin/plugin2host" sdk "github.com/terraform-linters/tflint-plugin-sdk/tflint" "github.com/terraform-linters/tflint/terraform" "github.com/terraform-linters/tflint/tflint" @@ -22,6 +23,8 @@ type GRPCServer struct { clientSDKVersion *version.Version } +var _ plugin2host.Server = (*GRPCServer)(nil) + // NewGRPCServer initializes a gRPC server for plugins. func NewGRPCServer(runner *tflint.Runner, rootRunner *tflint.Runner, files map[string]*hcl.File, sdkVersion *version.Version) *GRPCServer { return &GRPCServer{runner: runner, rootRunner: rootRunner, files: files, clientSDKVersion: sdkVersion} @@ -70,6 +73,7 @@ func (s *GRPCServer) GetModuleContent(bodyS *hclext.BodySchema, opts sdk.GetModu // GetFile returns the hcl.File based on passed the file name. func (s *GRPCServer) GetFile(name string) (*hcl.File, error) { + // TODO: return a file from runner.Sources() return s.files[name], nil } @@ -180,22 +184,40 @@ func (s *GRPCServer) EvaluateExpr(expr hcl.Expression, opts sdk.EvaluateExprOpti } // EmitIssue stores an issue in the server based on passed rule, message, and location. -// If the range associated with the issue is an expression, it propagates to the runner -// that the issue found in that expression. This allows you to determine if the issue was caused -// by a module argument in the case of module inspection. -func (s *GRPCServer) EmitIssue(rule sdk.Rule, message string, location hcl.Range) error { +func (s *GRPCServer) EmitIssue(rule sdk.Rule, message string, location hcl.Range, fixable bool) (bool, error) { + // If the issue range represents an expression, it is emitted based on that context. + // This is important for module inspection that emits issues for module arguments included in the expression. + expr, err := s.getExprFromRange(location) + if err != nil { + // If the range does not represent an expression, just emit it without context. + return s.runner.EmitIssue(rule, message, location, fixable), nil + } + + var applied bool + err = s.runner.WithExpressionContext(expr, func() error { + applied = s.runner.EmitIssue(rule, message, location, fixable) + return nil + }) + return applied, err +} + +func (s *GRPCServer) getExprFromRange(location hcl.Range) (hcl.Expression, error) { file := s.runner.File(location.Filename) if file == nil { - s.runner.EmitIssue(rule, message, location) - return nil + return nil, errors.New("file not found") } expr, diags := hclext.ParseExpression(location.SliceBytes(file.Bytes), location.Filename, location.Start) if diags.HasErrors() { - s.runner.EmitIssue(rule, message, location) - return nil + return nil, diags } - return s.runner.WithExpressionContext(expr, func() error { - s.runner.EmitIssue(rule, message, location) - return nil - }) + return expr, nil +} + +// ApplyChanges applies the autofix changes to the runner. +func (s *GRPCServer) ApplyChanges(changes map[string][]byte) error { + diags := s.runner.ApplyChanges(changes) + if diags.HasErrors() { + return diags + } + return nil } diff --git a/plugin/server_test.go b/plugin/server_test.go index 44ae7b820..dd7f2ab99 100644 --- a/plugin/server_test.go +++ b/plugin/server_test.go @@ -707,27 +707,27 @@ resource "aws_instance" "foo" { tests := []struct { Name string - Args func() (sdk.Rule, string, hcl.Range) + Args func() (sdk.Rule, string, hcl.Range, bool) Want int }{ { Name: "on expr", - Args: func() (sdk.Rule, string, hcl.Range) { - return &testRule{}, "error", exprRange + Args: func() (sdk.Rule, string, hcl.Range, bool) { + return &testRule{}, "error", exprRange, false }, Want: 1, }, { Name: "on non-expr", - Args: func() (sdk.Rule, string, hcl.Range) { - return &testRule{}, "error", resourceDefRange + Args: func() (sdk.Rule, string, hcl.Range, bool) { + return &testRule{}, "error", resourceDefRange, false }, Want: 1, }, { Name: "on another file", - Args: func() (sdk.Rule, string, hcl.Range) { - return &testRule{}, "error", hcl.Range{Filename: "not_found.tf"} + Args: func() (sdk.Rule, string, hcl.Range, bool) { + return &testRule{}, "error", hcl.Range{Filename: "not_found.tf"}, false }, Want: 1, }, @@ -739,7 +739,7 @@ resource "aws_instance" "foo" { server := NewGRPCServer(runner, nil, runner.Files(), SDKVersion) - err := server.EmitIssue(test.Args()) + _, err := server.EmitIssue(test.Args()) if err != nil { t.Fatalf("failed to call EmitIssue: %s", err) } diff --git a/terraform/module.go b/terraform/module.go index 1c197d1f8..e9f90eac4 100644 --- a/terraform/module.go +++ b/terraform/module.go @@ -5,6 +5,8 @@ import ( "strings" "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/hcl/v2/hclsyntax" + hcljson "github.com/hashicorp/hcl/v2/json" "github.com/terraform-linters/tflint-plugin-sdk/hclext" ) @@ -19,8 +21,8 @@ type Module struct { Sources map[string][]byte Files map[string]*hcl.File - primaries []*hcl.File - overrides []*hcl.File + primaries map[string]*hcl.File + overrides map[string]*hcl.File } func NewEmptyModule() *Module { @@ -35,8 +37,8 @@ func NewEmptyModule() *Module { Sources: map[string][]byte{}, Files: map[string]*hcl.File{}, - primaries: []*hcl.File{}, - overrides: []*hcl.File{}, + primaries: map[string]*hcl.File{}, + overrides: map[string]*hcl.File{}, } } @@ -73,6 +75,42 @@ func (m *Module) build() hcl.Diagnostics { return diags } +// Rebuild rebuilds the module from the passed sources. +// The main purpose of this is to apply autofixes in the module. +func (m *Module) Rebuild(sources map[string][]byte) hcl.Diagnostics { + if len(sources) == 0 { + return nil + } + var diags hcl.Diagnostics + + for path, source := range sources { + var file *hcl.File + var d hcl.Diagnostics + if strings.HasSuffix(path, ".json") { + file, d = hcljson.Parse(source, path) + } else { + file, d = hclsyntax.ParseConfig(source, path, hcl.InitialPos) + } + if d.HasErrors() { + diags = diags.Extend(d) + continue + } + + m.Sources[path] = source + m.Files[path] = file + if _, exists := m.primaries[path]; exists { + m.primaries[path] = file + } + if _, exists := m.overrides[path]; exists { + m.overrides[path] = file + } + } + + d := m.build() + diags = diags.Extend(d) + return diags +} + // PartialContent extracts body content from Terraform configurations based on the passed schema. // Basically, this function is a wrapper for hclext.PartialContent, but in some ways it reproduces // Terraform language semantics. diff --git a/terraform/parser.go b/terraform/parser.go index 0efa4de9d..513095766 100644 --- a/terraform/parser.go +++ b/terraform/parser.go @@ -65,10 +65,8 @@ func (p *Parser) LoadConfigDir(baseDir, dir string) (*Module, hcl.Diagnostics) { } mod := NewEmptyModule() - mod.primaries = make([]*hcl.File, len(primaries)) - mod.overrides = make([]*hcl.File, len(overrides)) - for i, path := range primaries { + for _, path := range primaries { f, loadDiags := p.loadHCLFile(baseDir, path) diags = diags.Extend(loadDiags) if loadDiags.HasErrors() { @@ -76,11 +74,11 @@ func (p *Parser) LoadConfigDir(baseDir, dir string) (*Module, hcl.Diagnostics) { } realPath := filepath.Join(baseDir, path) - mod.primaries[i] = f + mod.primaries[realPath] = f mod.Sources[realPath] = f.Bytes mod.Files[realPath] = f } - for i, path := range overrides { + for _, path := range overrides { f, loadDiags := p.loadHCLFile(baseDir, path) diags = diags.Extend(loadDiags) if loadDiags.HasErrors() { @@ -88,7 +86,7 @@ func (p *Parser) LoadConfigDir(baseDir, dir string) (*Module, hcl.Diagnostics) { } realPath := filepath.Join(baseDir, path) - mod.overrides[i] = f + mod.overrides[realPath] = f mod.Sources[realPath] = f.Bytes mod.Files[realPath] = f } diff --git a/terraform/parser_test.go b/terraform/parser_test.go index 1e30d2025..03b8b746c 100644 --- a/terraform/parser_test.go +++ b/terraform/parser_test.go @@ -32,12 +32,12 @@ func TestLoadConfigDir(t *testing.T) { dir: ".", want: &Module{ SourceDir: ".", - primaries: []*hcl.File{ - {Body: hcltest.MockBody(&hcl.BodyContent{MissingItemRange: hcl.Range{Filename: "main.tf"}})}, + primaries: map[string]*hcl.File{ + "main.tf": {Body: hcltest.MockBody(&hcl.BodyContent{MissingItemRange: hcl.Range{Filename: "main.tf"}})}, }, - overrides: []*hcl.File{ - {Body: hcltest.MockBody(&hcl.BodyContent{MissingItemRange: hcl.Range{Filename: "main_override.tf"}})}, - {Body: hcltest.MockBody(&hcl.BodyContent{MissingItemRange: hcl.Range{Filename: "override.tf"}})}, + overrides: map[string]*hcl.File{ + "main_override.tf": {Body: hcltest.MockBody(&hcl.BodyContent{MissingItemRange: hcl.Range{Filename: "main_override.tf"}})}, + "override.tf": {Body: hcltest.MockBody(&hcl.BodyContent{MissingItemRange: hcl.Range{Filename: "override.tf"}})}, }, Sources: map[string][]byte{ "main.tf": {}, @@ -62,12 +62,12 @@ func TestLoadConfigDir(t *testing.T) { dir: ".", want: &Module{ SourceDir: ".", - primaries: []*hcl.File{ - {Body: hcltest.MockBody(&hcl.BodyContent{MissingItemRange: hcl.Range{Filename: "main.tf.json"}})}, + primaries: map[string]*hcl.File{ + "main.tf.json": {Body: hcltest.MockBody(&hcl.BodyContent{MissingItemRange: hcl.Range{Filename: "main.tf.json"}})}, }, - overrides: []*hcl.File{ - {Body: hcltest.MockBody(&hcl.BodyContent{MissingItemRange: hcl.Range{Filename: "main_override.tf.json"}})}, - {Body: hcltest.MockBody(&hcl.BodyContent{MissingItemRange: hcl.Range{Filename: "override.tf.json"}})}, + overrides: map[string]*hcl.File{ + "main_override.tf.json": {Body: hcltest.MockBody(&hcl.BodyContent{MissingItemRange: hcl.Range{Filename: "main_override.tf.json"}})}, + "override.tf.json": {Body: hcltest.MockBody(&hcl.BodyContent{MissingItemRange: hcl.Range{Filename: "override.tf.json"}})}, }, Sources: map[string][]byte{ "main.tf.json": []byte("{}"), @@ -92,12 +92,12 @@ func TestLoadConfigDir(t *testing.T) { dir: ".", want: &Module{ SourceDir: ".", - primaries: []*hcl.File{ - {Body: hcltest.MockBody(&hcl.BodyContent{MissingItemRange: hcl.Range{Filename: filepath.Join("foo", "main.tf")}})}, + primaries: map[string]*hcl.File{ + filepath.Join("foo", "main.tf"): {Body: hcltest.MockBody(&hcl.BodyContent{MissingItemRange: hcl.Range{Filename: filepath.Join("foo", "main.tf")}})}, }, - overrides: []*hcl.File{ - {Body: hcltest.MockBody(&hcl.BodyContent{MissingItemRange: hcl.Range{Filename: filepath.Join("foo", "main_override.tf")}})}, - {Body: hcltest.MockBody(&hcl.BodyContent{MissingItemRange: hcl.Range{Filename: filepath.Join("foo", "override.tf")}})}, + overrides: map[string]*hcl.File{ + filepath.Join("foo", "main_override.tf"): {Body: hcltest.MockBody(&hcl.BodyContent{MissingItemRange: hcl.Range{Filename: filepath.Join("foo", "main_override.tf")}})}, + filepath.Join("foo", "override.tf"): {Body: hcltest.MockBody(&hcl.BodyContent{MissingItemRange: hcl.Range{Filename: filepath.Join("foo", "override.tf")}})}, }, Sources: map[string][]byte{ filepath.Join("foo", "main.tf"): {}, @@ -122,12 +122,12 @@ func TestLoadConfigDir(t *testing.T) { dir: "bar", want: &Module{ SourceDir: "bar", - primaries: []*hcl.File{ - {Body: hcltest.MockBody(&hcl.BodyContent{MissingItemRange: hcl.Range{Filename: filepath.Join("bar", "main.tf")}})}, + primaries: map[string]*hcl.File{ + filepath.Join("bar", "main.tf"): {Body: hcltest.MockBody(&hcl.BodyContent{MissingItemRange: hcl.Range{Filename: filepath.Join("bar", "main.tf")}})}, }, - overrides: []*hcl.File{ - {Body: hcltest.MockBody(&hcl.BodyContent{MissingItemRange: hcl.Range{Filename: filepath.Join("bar", "main_override.tf")}})}, - {Body: hcltest.MockBody(&hcl.BodyContent{MissingItemRange: hcl.Range{Filename: filepath.Join("bar", "override.tf")}})}, + overrides: map[string]*hcl.File{ + filepath.Join("bar", "main_override.tf"): {Body: hcltest.MockBody(&hcl.BodyContent{MissingItemRange: hcl.Range{Filename: filepath.Join("bar", "main_override.tf")}})}, + filepath.Join("bar", "override.tf"): {Body: hcltest.MockBody(&hcl.BodyContent{MissingItemRange: hcl.Range{Filename: filepath.Join("bar", "override.tf")}})}, }, Sources: map[string][]byte{ filepath.Join("bar", "main.tf"): {}, @@ -152,12 +152,12 @@ func TestLoadConfigDir(t *testing.T) { dir: "bar", want: &Module{ SourceDir: "bar", - primaries: []*hcl.File{ - {Body: hcltest.MockBody(&hcl.BodyContent{MissingItemRange: hcl.Range{Filename: filepath.Join("foo", "bar", "main.tf")}})}, + primaries: map[string]*hcl.File{ + filepath.Join("foo", "bar", "main.tf"): {Body: hcltest.MockBody(&hcl.BodyContent{MissingItemRange: hcl.Range{Filename: filepath.Join("foo", "bar", "main.tf")}})}, }, - overrides: []*hcl.File{ - {Body: hcltest.MockBody(&hcl.BodyContent{MissingItemRange: hcl.Range{Filename: filepath.Join("foo", "bar", "main_override.tf")}})}, - {Body: hcltest.MockBody(&hcl.BodyContent{MissingItemRange: hcl.Range{Filename: filepath.Join("foo", "bar", "override.tf")}})}, + overrides: map[string]*hcl.File{ + filepath.Join("foo", "bar", "main_override.tf"): {Body: hcltest.MockBody(&hcl.BodyContent{MissingItemRange: hcl.Range{Filename: filepath.Join("foo", "bar", "main_override.tf")}})}, + filepath.Join("foo", "bar", "override.tf"): {Body: hcltest.MockBody(&hcl.BodyContent{MissingItemRange: hcl.Range{Filename: filepath.Join("foo", "bar", "override.tf")}})}, }, Sources: map[string][]byte{ filepath.Join("foo", "bar", "main.tf"): {}, @@ -192,29 +192,27 @@ func TestLoadConfigDir(t *testing.T) { t.Errorf("SourceDir: want=%s, got=%s", test.want.SourceDir, mod.SourceDir) } - opt := cmpopts.SortSlices(func(x, y string) bool { return x > y }) - - primaries := make([]string, len(mod.primaries)) + primaries := map[string]string{} for i, f := range mod.primaries { primaries[i] = f.Body.MissingItemRange().Filename } - primariesWant := make([]string, len(test.want.primaries)) + primariesWant := map[string]string{} for i, f := range test.want.primaries { primariesWant[i] = f.Body.MissingItemRange().Filename } - if diff := cmp.Diff(primaries, primariesWant, opt); diff != "" { + if diff := cmp.Diff(primaries, primariesWant); diff != "" { t.Errorf(diff) } - overrides := make([]string, len(mod.overrides)) + overrides := map[string]string{} for i, f := range mod.overrides { overrides[i] = f.Body.MissingItemRange().Filename } - overridesWant := make([]string, len(test.want.overrides)) + overridesWant := map[string]string{} for i, f := range test.want.overrides { overridesWant[i] = f.Body.MissingItemRange().Filename } - if diff := cmp.Diff(overrides, overridesWant, opt); diff != "" { + if diff := cmp.Diff(overrides, overridesWant); diff != "" { t.Errorf(diff) } @@ -230,6 +228,7 @@ func TestLoadConfigDir(t *testing.T) { for name := range test.want.Files { filesWant = append(filesWant, name) } + opt := cmpopts.SortSlices(func(x, y string) bool { return x > y }) if diff := cmp.Diff(files, filesWant, opt); diff != "" { t.Errorf(diff) } diff --git a/tflint/issue.go b/tflint/issue.go index 70c1dcc53..49a767eae 100644 --- a/tflint/issue.go +++ b/tflint/issue.go @@ -13,7 +13,13 @@ type Issue struct { Rule Rule Message string Range hcl.Range + Fixable bool Callers []hcl.Range + + // Source is the source code of the file where the issue was found. + // Usually this is the same as the originally loaded source, + // but it may be a different if rewritten by autofixes. + Source []byte } // Issues is an alias for the map of Issue diff --git a/tflint/runner.go b/tflint/runner.go index 498412cc0..c71476063 100644 --- a/tflint/runner.go +++ b/tflint/runner.go @@ -25,6 +25,7 @@ type Runner struct { config *Config currentExpr hcl.Expression modVars map[string]*moduleVariable + changes map[string][]byte } // Rule is interface for building the issue @@ -66,6 +67,7 @@ func NewRunner(originalWorkingDir string, c *Config, ants map[string]Annotations Ctx: ctx, annotations: ants, config: c, + changes: map[string][]byte{}, } return runner, nil @@ -186,6 +188,28 @@ func (r *Runner) LookupIssues(files ...string) Issues { return issues } +// LookupChanges returns changes according to the received files +func (r *Runner) LookupChanges(files ...string) map[string][]byte { + if len(files) == 0 { + return r.changes + } + + changes := make(map[string][]byte) + for path, source := range r.changes { + for _, file := range files { + if filepath.Clean(file) == filepath.Clean(path) { + changes[path] = source + } + } + } + return changes +} + +// ClearChanges clears changes +func (r *Runner) ClearChanges() { + r.changes = map[string][]byte{} +} + // File returns the raw *hcl.File representation of a Terraform configuration at the specified path, // or nil if there path does not match any configuration. func (r *Runner) File(path string) *hcl.File { @@ -208,23 +232,35 @@ func (r *Runner) Sources() map[string][]byte { return r.TFConfig.Module.Sources } -// EmitIssue builds an issue and accumulates it -func (r *Runner) EmitIssue(rule Rule, message string, location hcl.Range) { +// EmitIssue builds an issue and accumulates it. +// Returns true if the issue was not ignored by annotations. +func (r *Runner) EmitIssue(rule Rule, message string, location hcl.Range, fixable bool) bool { if r.TFConfig.Path.IsRoot() { - r.emitIssue(&Issue{ + return r.emitIssue(&Issue{ Rule: rule, Message: message, Range: location, + Fixable: fixable, + Source: r.Sources()[location.Filename], }) } else { - for _, modVar := range r.listModuleVars(r.currentExpr) { - r.emitIssue(&Issue{ + modVars := r.listModuleVars(r.currentExpr) + // Returns true only if all issues have not been ignored in module inspection. + allApplied := len(modVars) > 0 + for _, modVar := range modVars { + applied := r.emitIssue(&Issue{ Rule: rule, Message: message, Range: modVar.DeclRange, + Fixable: false, // Issues for module inspection are always not fixable. Callers: append(modVar.callers(), location), + Source: r.Sources()[location.Filename], }) + if !applied { + allApplied = false + } } + return allApplied } } @@ -246,16 +282,33 @@ func (r *Runner) ConfigSources() map[string][]byte { return r.config.Sources() } -func (r *Runner) emitIssue(issue *Issue) { +// ApplyChanges saves the changes and applies them to the Terraform module. +func (r *Runner) ApplyChanges(changes map[string][]byte) hcl.Diagnostics { + if len(changes) == 0 { + return nil + } + + diags := r.TFConfig.Module.Rebuild(changes) + if diags.HasErrors() { + return diags + } + for path, source := range changes { + r.changes[path] = source + } + return nil +} + +func (r *Runner) emitIssue(issue *Issue) bool { if annotations, ok := r.annotations[issue.Range.Filename]; ok { for _, annotation := range annotations { if annotation.IsAffected(issue) { log.Printf("[INFO] %s (%s) is ignored by %s", issue.Range.String(), issue.Rule.Name(), annotation.String()) - return + return false } } } r.Issues = append(r.Issues, issue) + return true } func (r *Runner) listModuleVars(expr hcl.Expression) []*moduleVariable { diff --git a/tflint/runner_test.go b/tflint/runner_test.go index 2659c1773..b50039f7a 100644 --- a/tflint/runner_test.go +++ b/tflint/runner_test.go @@ -458,7 +458,7 @@ func Test_EmitIssue(t *testing.T) { for _, tc := range cases { runner := testRunnerWithAnnotations(t, map[string]string{}, tc.Annotations) - runner.EmitIssue(tc.Rule, tc.Message, tc.Location) + runner.EmitIssue(tc.Rule, tc.Message, tc.Location, false) if !cmp.Equal(runner.Issues, tc.Expected) { t.Fatalf("Failed `%s` test: diff=%s", tc.Name, cmp.Diff(runner.Issues, tc.Expected))