Skip to content

Commit

Permalink
Introduce autofix
Browse files Browse the repository at this point in the history
  • Loading branch information
wata727 committed May 2, 2023
1 parent 9b802b1 commit 751afa9
Show file tree
Hide file tree
Showing 10 changed files with 169 additions and 70 deletions.
59 changes: 47 additions & 12 deletions cmd/inspect.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package cmd

import (
"fmt"
"io"
"os"
"path/filepath"
"strings"
Expand All @@ -27,6 +28,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 {
Expand Down Expand Up @@ -62,11 +64,14 @@ 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 {
Expand All @@ -91,8 +96,33 @@ func (cli *CLI) inspect(opts Options, args []string) int {
force = cli.config.Force
}

// TODO: Autofixed issues should contain the previous source code
// TODO: Show "fixed" mark in pretty format
cli.formatter.Print(issues, nil, cli.sources)

if opts.Fix {
fs := afero.NewOsFs()
for path, source := range changes {
f, err := fs.OpenFile(path, os.O_WRONLY|os.O_TRUNC, 0644)
if err != nil {
cli.formatter.Print(tflint.Issues{}, fmt.Errorf("Failed to apply autofix; failed to open %s: %w", path, err), cli.sources)
return ExitCodeError
}

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 {
cli.formatter.Print(tflint.Issues{}, fmt.Errorf("Failed to apply autofix; failed to write source code to %s: %w", path, err), cli.sources)
return ExitCodeError
}
}
}

if len(issues) > 0 && !force && exceedsMinimumFailure(issues, opts.MinimumFailureSeverity) {
return ExitCodeIssuesFound
}
Expand Down Expand Up @@ -143,31 +173,32 @@ 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]

Expand All @@ -177,7 +208,7 @@ func (cli *CLI) inspectModule(opts Options, dir string, filterFiles []string) (t
defer rulesetPlugin.Clean()
}
if err != nil {
return tflint.Issues{}, err
return issues, changes, err
}

// Run inspection
Expand All @@ -186,32 +217,35 @@ func (cli *CLI) inspectModule(opts Options, dir string, filterFiles []string) (t
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)
}

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)
return issues, changes, fmt.Errorf("Failed to check ruleset; %w", err)
}
}
}

for _, runner := range runners {
issues = append(issues, runner.LookupIssues(filterFiles...)...)
for path, source := range runner.Changes {
changes[path] = source
}
}
// 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) {
Expand Down Expand Up @@ -285,6 +319,7 @@ func launchPlugins(config *tflint.Config) (*plugin.Plugin, error) {
return rulesetPlugin, fmt.Errorf("Failed to satisfy version constraints; tflint-ruleset-%s requires %s, but TFLint version is %s", name, constraints, tflint.Version)
}

// TODO: Apply fix option to plugins
if err := ruleset.ApplyGlobalConfig(pluginConf); err != nil {
return rulesetPlugin, fmt.Errorf("Failed to apply global config to `%s` plugin; %w", name, err)
}
Expand Down
1 change: 1 addition & 0 deletions cmd/option.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
}

Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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.20230502084831-d07f1427f697
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
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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.20230502084831-d07f1427f697 h1:9T4wKOqLj6m03SCD/MiazysAYjQH0PCwyWYFOmg5+ps=
github.com/terraform-linters/tflint-plugin-sdk v0.16.2-0.20230502084831-d07f1427f697/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=
Expand Down
26 changes: 21 additions & 5 deletions plugin/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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}
Expand Down Expand Up @@ -183,19 +186,32 @@ func (s *GRPCServer) EvaluateExpr(expr hcl.Expression, opts sdk.EvaluateExprOpti
// 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, fixed map[string][]byte) error {
file := s.runner.File(location.Filename)
if file == nil {
s.runner.EmitIssue(rule, message, location)
return nil
}

expr, diags := hclext.ParseExpression(location.SliceBytes(file.Bytes), location.Filename, location.Start)
if diags.HasErrors() {
s.runner.EmitIssue(rule, message, location)
return nil
} else {
err := s.runner.WithExpressionContext(expr, func() error {
s.runner.EmitIssue(rule, message, location)
return nil
})
if err != nil {
return err
}
}
return s.runner.WithExpressionContext(expr, func() error {
s.runner.EmitIssue(rule, message, location)

if len(fixed) == 0 {
return nil
})
}
diags = s.runner.ApplyChanges(fixed)
if diags.HasErrors() {
return diags
}
return nil
}
14 changes: 7 additions & 7 deletions plugin/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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, map[string][]byte)
Want int
}{
{
Name: "on expr",
Args: func() (sdk.Rule, string, hcl.Range) {
return &testRule{}, "error", exprRange
Args: func() (sdk.Rule, string, hcl.Range, map[string][]byte) {
return &testRule{}, "error", exprRange, map[string][]byte{}
},
Want: 1,
},
{
Name: "on non-expr",
Args: func() (sdk.Rule, string, hcl.Range) {
return &testRule{}, "error", resourceDefRange
Args: func() (sdk.Rule, string, hcl.Range, map[string][]byte) {
return &testRule{}, "error", resourceDefRange, map[string][]byte{}
},
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, map[string][]byte) {
return &testRule{}, "error", hcl.Range{Filename: "not_found.tf"}, map[string][]byte{}
},
Want: 1,
},
Expand Down
44 changes: 40 additions & 4 deletions terraform/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand All @@ -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 {
Expand All @@ -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{},
}
}

Expand Down Expand Up @@ -73,6 +75,40 @@ func (m *Module) build() hcl.Diagnostics {
return diags
}

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.
Expand Down
10 changes: 4 additions & 6 deletions terraform/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,30 +65,28 @@ 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() {
continue
}
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() {
continue
}
realPath := filepath.Join(baseDir, path)

mod.overrides[i] = f
mod.overrides[realPath] = f
mod.Sources[realPath] = f.Bytes
mod.Files[realPath] = f
}
Expand Down
Loading

0 comments on commit 751afa9

Please sign in to comment.