From d6e755b41bf124f92c5cf4990b85250292dfffcb Mon Sep 17 00:00:00 2001 From: Sergey Vilgelm Date: Wed, 8 Mar 2023 23:05:12 -0800 Subject: [PATCH] support suggested fixes by analyzing a diff (#148) Added `GetSuggestedFix` function creates unified diff for `unmodifiedFile` and `formattedFile`. Then analyzes the diff and creates `analysis.SuggestedFix` if needed. The Analyzer checks the result of `GetSuggestedFix` function and reports as `analysis.Diagnostic`. Fix #146 Signed-off-by: Sergey Vilgelm --- go.mod | 2 +- pkg/analyzer/analyzer.go | 47 +++++--------- pkg/analyzer/diff.go | 83 +++++++++++++++++++++++++ pkg/analyzer/diff_test.go | 127 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 225 insertions(+), 34 deletions(-) create mode 100644 pkg/analyzer/diff.go create mode 100644 pkg/analyzer/diff_test.go diff --git a/go.mod b/go.mod index 480f0e0..06092eb 100644 --- a/go.mod +++ b/go.mod @@ -4,6 +4,7 @@ go 1.20 require ( github.com/hexops/gotextdiff v1.0.3 + github.com/pmezard/go-difflib v1.0.0 github.com/spf13/cobra v1.6.1 github.com/stretchr/testify v1.8.1 go.uber.org/zap v1.24.0 @@ -15,7 +16,6 @@ require ( require ( github.com/davecgh/go-spew v1.1.1 // indirect github.com/inconshreveable/mousetrap v1.0.1 // indirect - github.com/pmezard/go-difflib v1.0.0 // indirect github.com/spf13/pflag v1.0.5 // indirect go.uber.org/atomic v1.7.0 // indirect go.uber.org/multierr v1.6.0 // indirect diff --git a/pkg/analyzer/analyzer.go b/pkg/analyzer/analyzer.go index 9500202..aa207ec 100644 --- a/pkg/analyzer/analyzer.go +++ b/pkg/analyzer/analyzer.go @@ -1,13 +1,11 @@ package analyzer import ( - "bytes" "fmt" "go/token" "strings" "golang.org/x/tools/go/analysis" - "golang.org/x/tools/go/analysis/passes/inspect" "github.com/daixiang0/gci/pkg/config" "github.com/daixiang0/gci/pkg/gci" @@ -44,10 +42,9 @@ func init() { } var Analyzer = &analysis.Analyzer{ - Name: "gci", - Doc: "A tool that control golang package import order and make it always deterministic.", - Requires: []*analysis.Analyzer{inspect.Analyzer}, - Run: runAnalysis, + Name: "gci", + Doc: "A tool that control golang package import order and make it always deterministic.", + Run: runAnalysis, } func runAnalysis(pass *analysis.Pass) (interface{}, error) { @@ -77,39 +74,23 @@ func runAnalysis(pass *analysis.Pass) (interface{}, error) { if err != nil { return nil, err } - // search for a difference - fileRunes := bytes.Runes(unmodifiedFile) - formattedRunes := bytes.Runes(formattedFile) - diffIdx := compareRunes(fileRunes, formattedRunes) - switch diffIdx { - case -1: + fix, err := GetSuggestedFix(file, unmodifiedFile, formattedFile) + if err != nil { + return nil, err + } + if fix == nil { // no difference - default: - pass.Reportf(file.Pos(diffIdx), "fix by `%s %s`", generateCmdLine(*gciCfg), filePath) + continue } + pass.Report(analysis.Diagnostic{ + Pos: fix.TextEdits[0].Pos, + Message: fmt.Sprintf("fix by `%s %s`", generateCmdLine(*gciCfg), filePath), + SuggestedFixes: []analysis.SuggestedFix{*fix}, + }) } return nil, nil } -func compareRunes(a, b []rune) (differencePos int) { - // check shorter rune slice first to prevent invalid array access - shorterRune := a - if len(b) < len(a) { - shorterRune = b - } - // check for differences up to where the length is identical - for idx := 0; idx < len(shorterRune); idx++ { - if a[idx] != b[idx] { - return idx - } - } - // check that we have compared two equally long rune arrays - if len(a) != len(b) { - return len(shorterRune) + 1 - } - return -1 -} - func parseGciConfiguration() (*config.Config, error) { fmtCfg := config.BoolConfig{ NoInlineComments: noInlineComments, diff --git a/pkg/analyzer/diff.go b/pkg/analyzer/diff.go new file mode 100644 index 0000000..b283cbf --- /dev/null +++ b/pkg/analyzer/diff.go @@ -0,0 +1,83 @@ +package analyzer + +import ( + "bytes" + "go/token" + "regexp" + "strconv" + "strings" + + "github.com/pmezard/go-difflib/difflib" + "golang.org/x/tools/go/analysis" +) + +var hunkRE = regexp.MustCompile(`@@ -(\d+),(\d+) \+\d+,\d+ @@`) + +func GetSuggestedFix(file *token.File, a, b []byte) (*analysis.SuggestedFix, error) { + d := difflib.UnifiedDiff{ + A: difflib.SplitLines(string(a)), + B: difflib.SplitLines(string(b)), + Context: 1, + } + diff, err := difflib.GetUnifiedDiffString(d) + if err != nil { + return nil, err + } + if diff == "" { + return nil, nil + } + var ( + fix analysis.SuggestedFix + found = false + edit analysis.TextEdit + buf bytes.Buffer + ) + for _, line := range strings.Split(diff, "\n") { + if hunk := hunkRE.FindStringSubmatch(line); len(hunk) > 0 { + if found { + edit.NewText = buf.Bytes() + buf = bytes.Buffer{} + fix.TextEdits = append(fix.TextEdits, edit) + edit = analysis.TextEdit{} + } + found = true + start, err := strconv.Atoi(hunk[1]) + if err != nil { + return nil, err + } + lines, err := strconv.Atoi(hunk[2]) + if err != nil { + return nil, err + } + edit.Pos = file.LineStart(start) + end := start + lines + if end > file.LineCount() { + edit.End = token.Pos(file.Size()) + } else { + edit.End = file.LineStart(end) + } + continue + } + // skip any lines until first hunk found + if !found { + continue + } + if line == "" { + continue + } + switch line[0] { + case '+': + buf.WriteString(line[1:]) + buf.WriteRune('\n') + case '-': + // just skip + default: + buf.WriteString(line) + buf.WriteRune('\n') + } + } + edit.NewText = buf.Bytes() + fix.TextEdits = append(fix.TextEdits, edit) + + return &fix, nil +} diff --git a/pkg/analyzer/diff_test.go b/pkg/analyzer/diff_test.go new file mode 100644 index 0000000..617f1e9 --- /dev/null +++ b/pkg/analyzer/diff_test.go @@ -0,0 +1,127 @@ +package analyzer_test + +import ( + "go/parser" + "go/token" + "testing" + + "github.com/stretchr/testify/assert" + "golang.org/x/tools/go/analysis" + + "github.com/daixiang0/gci/pkg/analyzer" +) + +const formattedFile = `package analyzer + +import ( + "fmt" + "go/token" + "strings" + + "golang.org/x/tools/go/analysis" + + "github.com/daixiang0/gci/pkg/config" + "github.com/daixiang0/gci/pkg/gci" + "github.com/daixiang0/gci/pkg/io" + "github.com/daixiang0/gci/pkg/log" +) +` + +func TestGetSuggestedFix(t *testing.T) { + for _, tt := range []struct { + name string + unformattedFile string + expectedFix *analysis.SuggestedFix + expectedErr string + }{ + { + name: "same files", + unformattedFile: formattedFile, + }, + { + name: "one change", + unformattedFile: `package analyzer + +import ( + "fmt" + "go/token" + "strings" + + "golang.org/x/tools/go/analysis" + + "github.com/daixiang0/gci/pkg/config" + "github.com/daixiang0/gci/pkg/gci" + + "github.com/daixiang0/gci/pkg/io" + "github.com/daixiang0/gci/pkg/log" +) +`, + expectedFix: &analysis.SuggestedFix{ + TextEdits: []analysis.TextEdit{ + { + Pos: 133, + End: 205, + NewText: []byte(` "github.com/daixiang0/gci/pkg/gci" + "github.com/daixiang0/gci/pkg/io" +`, + ), + }, + }, + }, + }, + { + name: "multiple changes", + unformattedFile: `package analyzer + +import ( + "fmt" + "go/token" + + "strings" + + "golang.org/x/tools/go/analysis" + + "github.com/daixiang0/gci/pkg/config" + "github.com/daixiang0/gci/pkg/gci" + + "github.com/daixiang0/gci/pkg/io" + "github.com/daixiang0/gci/pkg/log" +) +`, + expectedFix: &analysis.SuggestedFix{ + TextEdits: []analysis.TextEdit{ + { + Pos: 35, + End: 59, + NewText: []byte(` "go/token" + "strings" +`, + ), + }, + { + Pos: 134, + End: 206, + NewText: []byte(` "github.com/daixiang0/gci/pkg/gci" + "github.com/daixiang0/gci/pkg/io" +`, + ), + }, + }, + }, + }, + } { + t.Run(tt.name, func(t *testing.T) { + fset := token.NewFileSet() + f, err := parser.ParseFile(fset, "analyzer.go", tt.unformattedFile, 0) + assert.NoError(t, err) + + actualFix, err := analyzer.GetSuggestedFix(fset.File(f.Pos()), []byte(tt.unformattedFile), []byte(formattedFile)) + if tt.expectedErr != "" { + assert.ErrorContains(t, err, tt.expectedErr) + return + } + assert.NoError(t, err) + assert.Equal(t, tt.expectedFix, actualFix) + }) + } +}