Skip to content

Commit

Permalink
Introduce autofix API (#254)
Browse files Browse the repository at this point in the history
wata727 authored Jun 13, 2023

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
1 parent 30d991e commit b875e92
Showing 22 changed files with 4,465 additions and 484 deletions.
31 changes: 30 additions & 1 deletion helper/runner.go
Original file line number Diff line number Diff line change
@@ -9,6 +9,7 @@ import (
"github.com/hashicorp/hcl/v2"
"github.com/hashicorp/hcl/v2/hclsyntax"
"github.com/terraform-linters/tflint-plugin-sdk/hclext"
"github.com/terraform-linters/tflint-plugin-sdk/internal"
"github.com/terraform-linters/tflint-plugin-sdk/terraform/addrs"
"github.com/terraform-linters/tflint-plugin-sdk/tflint"
"github.com/zclconf/go-cty/cty"
@@ -21,8 +22,10 @@ type Runner struct {
Issues Issues

files map[string]*hcl.File
sources map[string][]byte
config Config
variables map[string]*Variable
fixer *internal.Fixer
}

// Variable is an implementation of variables in Terraform language
@@ -318,6 +321,25 @@ func (r *Runner) EmitIssue(rule tflint.Rule, message string, location hcl.Range)
return nil
}

// EmitIssueWithFix adds an issue and invoke fix.
func (r *Runner) EmitIssueWithFix(rule tflint.Rule, message string, location hcl.Range, fixFunc func(f tflint.Fixer) error) error {
r.fixer.StashChanges()
if err := fixFunc(r.fixer); err != nil {
if errors.Is(err, tflint.ErrFixNotSupported) {
r.fixer.PopChangesFromStash()
return r.EmitIssue(rule, message, location)
}
return err
}
return r.EmitIssue(rule, message, location)
}

// Changes returns formatted changes by the fixer.
func (r *Runner) Changes() map[string][]byte {
r.fixer.FormatChanges()
return r.fixer.Changes()
}

// EnsureNoError is a method that simply runs a function if there is no error.
//
// Deprecated: Use EvaluateExpr with a function callback. e.g. EvaluateExpr(expr, func (val T) error {}, ...)
@@ -331,7 +353,12 @@ func (r *Runner) EnsureNoError(err error, proc func() error) error {
// NewLocalRunner initialises a new test runner.
// Internal use only.
func NewLocalRunner(files map[string]*hcl.File, issues Issues) *Runner {
return &Runner{files: map[string]*hcl.File{}, variables: map[string]*Variable{}, Issues: issues}
return &Runner{
files: map[string]*hcl.File{},
sources: map[string][]byte{},
variables: map[string]*Variable{},
Issues: issues,
}
}

// AddLocalFile adds a new file to the current mapped files.
@@ -342,6 +369,7 @@ func (r *Runner) AddLocalFile(name string, file *hcl.File) bool {
}

r.files[name] = file
r.sources[name] = file.Bytes
return true
}

@@ -365,6 +393,7 @@ func (r *Runner) initFromFiles() error {
}
}
}
r.fixer = internal.NewFixer(r.sources)

return nil
}
182 changes: 182 additions & 0 deletions helper/runner_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package helper

import (
"errors"
"testing"

"github.com/google/go-cmp/cmp"
@@ -644,6 +645,187 @@ resource "aws_instance" "foo" {
}
}

func Test_EmitIssueWithFix(t *testing.T) {
// default error check helper
neverHappend := func(err error) bool { return err != nil }

tests := []struct {
name string
src string
rng hcl.Range
fix func(tflint.Fixer) error
want Issues
fixed string
errCheck func(error) bool
}{
{
name: "with fix",
src: `
resource "aws_instance" "foo" {
instance_type = "t2.micro"
}`,
rng: hcl.Range{
Filename: "main.tf",
Start: hcl.Pos{Line: 3, Column: 19, Byte: 51},
End: hcl.Pos{Line: 3, Column: 29, Byte: 61},
},
fix: func(fixer tflint.Fixer) error {
return fixer.ReplaceText(
hcl.Range{
Filename: "main.tf",
Start: hcl.Pos{Line: 3, Column: 19, Byte: 51},
End: hcl.Pos{Line: 3, Column: 29, Byte: 61},
},
`"t3.micro"`,
)
},
want: Issues{
{
Rule: &dummyRule{},
Message: "issue found",
Range: hcl.Range{Filename: "main.tf", Start: hcl.Pos{Line: 3, Column: 19}, End: hcl.Pos{Line: 3, Column: 29}},
},
},
fixed: `
resource "aws_instance" "foo" {
instance_type = "t3.micro"
}`,
errCheck: neverHappend,
},
{
name: "autofix is not supported",
src: `
resource "aws_instance" "foo" {
instance_type = "t2.micro"
}`,
rng: hcl.Range{
Filename: "main.tf",
Start: hcl.Pos{Line: 3, Column: 19, Byte: 51},
End: hcl.Pos{Line: 3, Column: 29, Byte: 61},
},
fix: func(fixer tflint.Fixer) error {
if err := fixer.ReplaceText(
hcl.Range{
Filename: "main.tf",
Start: hcl.Pos{Line: 3, Column: 19, Byte: 51},
End: hcl.Pos{Line: 3, Column: 29, Byte: 61},
},
`"t3.micro"`,
); err != nil {
return err
}
return tflint.ErrFixNotSupported
},
want: Issues{
{
Rule: &dummyRule{},
Message: "issue found",
Range: hcl.Range{Filename: "main.tf", Start: hcl.Pos{Line: 3, Column: 19}, End: hcl.Pos{Line: 3, Column: 29}},
},
},
errCheck: neverHappend,
},
{
name: "other errors",
src: `
resource "aws_instance" "foo" {
instance_type = "t2.micro"
}`,
rng: hcl.Range{
Filename: "main.tf",
Start: hcl.Pos{Line: 3, Column: 19, Byte: 51},
End: hcl.Pos{Line: 3, Column: 29, Byte: 61},
},
fix: func(fixer tflint.Fixer) error {
if err := fixer.ReplaceText(
hcl.Range{
Filename: "main.tf",
Start: hcl.Pos{Line: 3, Column: 19, Byte: 51},
End: hcl.Pos{Line: 3, Column: 29, Byte: 61},
},
`"t3.micro"`,
); err != nil {
return err
}
return errors.New("unexpected error")
},
want: Issues{},
fixed: `
resource "aws_instance" "foo" {
instance_type = "t3.micro"
}`,
errCheck: func(err error) bool {
return err == nil && err.Error() != "unexpected error"
},
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
runner := TestRunner(t, map[string]string{"main.tf": test.src})

err := runner.EmitIssueWithFix(&dummyRule{}, "issue found", test.rng, test.fix)
if test.errCheck(err) {
t.Fatal(err)
}

opt := cmpopts.IgnoreFields(hcl.Pos{}, "Byte")
if diff := cmp.Diff(test.want, runner.Issues, opt); diff != "" {
t.Fatal(diff)
}
if diff := cmp.Diff(test.fixed, string(runner.Changes()["main.tf"]), opt); diff != "" {
t.Fatal(diff)
}
})
}
}

func TestChanges(t *testing.T) {
tests := []struct {
name string
src string
fix func(tflint.Fixer) error
want string
}{
{
name: "changes",
src: `
locals {
foo = "bar"
}`,
fix: func(fixer tflint.Fixer) error {
return fixer.InsertTextBefore(
hcl.Range{
Filename: "main.tf",
Start: hcl.Pos{Byte: 12},
End: hcl.Pos{Byte: 15},
},
"bar = \"baz\"\n",
)
},
want: `
locals {
bar = "baz"
foo = "bar"
}`,
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
runner := TestRunner(t, map[string]string{"main.tf": test.src})

if err := test.fix(runner.fixer); err != nil {
t.Fatal(err)
}

if diff := cmp.Diff(test.want, string(runner.Changes()["main.tf"])); diff != "" {
t.Fatal(diff)
}
})
}
}

func Test_EnsureNoError(t *testing.T) {
runner := TestRunner(t, map[string]string{})

31 changes: 25 additions & 6 deletions helper/testing.go
Original file line number Diff line number Diff line change
@@ -17,6 +17,8 @@ import (
// TestRunner returns a mock Runner for testing.
// You can pass the map of file names and their contents in the second argument.
func TestRunner(t *testing.T, files map[string]string) *Runner {
t.Helper()

runner := NewLocalRunner(map[string]*hcl.File{}, Issues{})
parser := hclparse.NewParser()

@@ -50,25 +52,42 @@ func TestRunner(t *testing.T, files map[string]string) *Runner {
}

// AssertIssues is an assertion helper for comparing issues.
func AssertIssues(t *testing.T, expected Issues, actual Issues) {
func AssertIssues(t *testing.T, want Issues, got Issues) {
t.Helper()

opts := []cmp.Option{
// Byte field will be ignored because it's not important in tests such as positions
cmpopts.IgnoreFields(hcl.Pos{}, "Byte"),
ruleComparer(),
}
if !cmp.Equal(expected, actual, opts...) {
t.Fatalf("Expected issues are not matched:\n %s\n", cmp.Diff(expected, actual, opts...))
if diff := cmp.Diff(want, got, opts...); diff != "" {
t.Fatalf("Expected issues are not matched:\n %s\n", diff)
}
}

// AssertIssuesWithoutRange is an assertion helper for comparing issues except for range.
func AssertIssuesWithoutRange(t *testing.T, expected Issues, actual Issues) {
func AssertIssuesWithoutRange(t *testing.T, want Issues, got Issues) {
t.Helper()

opts := []cmp.Option{
cmpopts.IgnoreFields(Issue{}, "Range"),
ruleComparer(),
}
if !cmp.Equal(expected, actual, opts...) {
t.Fatalf("Expected issues are not matched:\n %s\n", cmp.Diff(expected, actual, opts...))
if diff := cmp.Diff(want, got, opts...); diff != "" {
t.Fatalf("Expected issues are not matched:\n %s\n", diff)
}
}

// AssertChanges is an assertion helper for comparing autofix changes.
func AssertChanges(t *testing.T, want map[string]string, got map[string][]byte) {
t.Helper()

sources := make(map[string]string)
for name, src := range got {
sources[name] = string(src)
}
if diff := cmp.Diff(want, sources); diff != "" {
t.Fatalf("Expected changes are not matched:\n %s\n", diff)
}
}

Loading

0 comments on commit b875e92

Please sign in to comment.