Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce autofix API #254

Merged
merged 1 commit into from
Jun 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 30 additions & 1 deletion helper/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand Down Expand Up @@ -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 {}, ...)
Expand All @@ -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.
Expand All @@ -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
}

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

return nil
}
Expand Down
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"
Expand Down Expand Up @@ -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{})

Expand Down
31 changes: 25 additions & 6 deletions helper/testing.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down Expand Up @@ -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)
}
}

Expand Down
Loading