From 7ad62c49b570286c4d774a056084d073bcd4b0bc Mon Sep 17 00:00:00 2001 From: Thirukumaran Vaseeharan Date: Thu, 17 Mar 2022 15:23:07 -0700 Subject: [PATCH 1/3] make gci linter fixable --- pkg/golinters/gci.go | 95 +++++++++++++++++++++++++++++------ pkg/golinters/gofmt_common.go | 27 ++++++++++ 2 files changed, 107 insertions(+), 15 deletions(-) diff --git a/pkg/golinters/gci.go b/pkg/golinters/gci.go index c0c606a7b89d..786e268437f8 100644 --- a/pkg/golinters/gci.go +++ b/pkg/golinters/gci.go @@ -1,10 +1,15 @@ package golinters import ( + "bytes" "fmt" - "strings" + "sync" - gci "github.com/daixiang0/gci/pkg/analyzer" + gcicfg "github.com/daixiang0/gci/pkg/configuration" + "github.com/daixiang0/gci/pkg/gci" + gciio "github.com/daixiang0/gci/pkg/io" + "github.com/pkg/errors" + "github.com/shazow/go-diff/difflib" "golang.org/x/tools/go/analysis" "github.com/golangci/golangci-lint/pkg/config" @@ -15,34 +20,94 @@ import ( const gciName = "gci" func NewGci(settings *config.GciSettings) *goanalysis.Linter { - var linterCfg map[string]map[string]interface{} + var cfg *gci.GciConfiguration + var mu sync.Mutex + var resIssues []goanalysis.Issue + differ := difflib.New() if settings != nil { - cfg := map[string]interface{}{ - gci.NoInlineCommentsFlag: settings.NoInlineComments, - gci.NoPrefixCommentsFlag: settings.NoPrefixComments, - gci.SectionsFlag: strings.Join(settings.Sections, gci.SectionDelimiter), - gci.SectionSeparatorsFlag: strings.Join(settings.SectionSeparator, gci.SectionDelimiter), + strcfg := gci.GciStringConfiguration{ + Cfg: gcicfg.FormatterConfiguration{ + NoInlineComments: settings.NoInlineComments, + NoPrefixComments: settings.NoPrefixComments, + }, + SectionStrings: settings.Sections, + SectionSeparatorStrings: settings.SectionSeparator, } - if settings.LocalPrefixes != "" { prefix := []string{"standard", "default", fmt.Sprintf("prefix(%s)", settings.LocalPrefixes)} - cfg[gci.SectionsFlag] = strings.Join(prefix, gci.SectionDelimiter) + strcfg.SectionStrings = prefix } - linterCfg = map[string]map[string]interface{}{ - gci.Analyzer.Name: cfg, - } + cfg, _ = strcfg.Parse() } + analyzer := &analysis.Analyzer{ + Name: gciName, + Doc: goanalysis.TheOnlyanalyzerDoc, + } return goanalysis.NewLinter( gciName, "Gci controls golang package import order and makes it always deterministic.", - []*analysis.Analyzer{gci.Analyzer}, - linterCfg, + []*analysis.Analyzer{analyzer}, + nil, ).WithContextSetter(func(lintCtx *linter.Context) { if settings.LocalPrefixes != "" { lintCtx.Log.Warnf("gci: `local-prefixes` is deprecated, use `sections` and `prefix(%s)` instead.", settings.LocalPrefixes) } + + analyzer.Run = func(pass *analysis.Pass) (interface{}, error) { + var fileNames []string + for _, f := range pass.Files { + pos := pass.Fset.PositionFor(f.Pos(), false) + fileNames = append(fileNames, pos.Filename) + } + + var issues []goanalysis.Issue + for _, f := range fileNames { + fio := gciio.File{FilePath: f} + source, result, err := gci.LoadFormatGoFile(fio, *cfg) + if err != nil { + return nil, err + } + if result == nil { + continue + } + + if !bytes.Equal(source, result) { + diff := bytes.Buffer{} + _, err = diff.WriteString(fmt.Sprintf("--- %[1]s\n+++ %[1]s\n", f)) + if err != nil { + return nil, fmt.Errorf("can't write diff header: %v", err) + } + + err = differ.Diff(&diff, bytes.NewReader(source), bytes.NewReader(result)) + if err != nil { + return nil, fmt.Errorf("can't get gci diff output: %v", err) + } + + is, err := extractIssuesFromPatch(diff.String(), lintCtx.Log, lintCtx, gciName) + if err != nil { + return nil, errors.Wrapf(err, "can't extract issues from gci diff output %q", diff.String()) + } + + for i := range is { + issues = append(issues, goanalysis.NewIssue(&is[i], pass)) + } + } + } + + if len(issues) == 0 { + return nil, nil + } + + mu.Lock() + resIssues = append(resIssues, issues...) + mu.Unlock() + + return nil, nil + } + }).WithIssuesReporter(func(*linter.Context) []goanalysis.Issue { + return resIssues }).WithLoadMode(goanalysis.LoadModeSyntax) } diff --git a/pkg/golinters/gofmt_common.go b/pkg/golinters/gofmt_common.go index 4f63e7bed853..02537494baa0 100644 --- a/pkg/golinters/gofmt_common.go +++ b/pkg/golinters/gofmt_common.go @@ -207,9 +207,36 @@ func (p *hunkChangesParser) parse(h *diffpkg.Hunk) []Change { return p.ret } +func getErrorTextForGci(lintCtx *linter.Context) string { + text := "File is not `gci`-ed" + noInLineComments := lintCtx.Settings().Gci.NoInlineComments + noPrefixComments := lintCtx.Settings().Gci.NoPrefixComments + sections := lintCtx.Settings().Gci.Sections + sectionSeparator := lintCtx.Settings().Gci.SectionSeparator + hasOptions := noInLineComments || noPrefixComments || len(sections) > 0 || len(sectionSeparator) > 0 + if hasOptions { + text += " with" + } + if noInLineComments { + text += " -NoInlineComments" + } + if noPrefixComments { + text += " -NoPrefixComments" + } + if len(sections) > 0 { + text += " -s " + strings.Join(sections, ",") + } + if len(sectionSeparator) > 0 { + text += " -x " + strings.Join(sectionSeparator, ",") + } + return text +} + func getErrorTextForLinter(lintCtx *linter.Context, linterName string) string { text := "File is not formatted" switch linterName { + case gciName: + text = getErrorTextForGci(lintCtx) case gofumptName: text = "File is not `gofumpt`-ed" if lintCtx.Settings().Gofumpt.ExtraRules { From ac822ce86200144ca492a0d0e8bfe01802fbac0f Mon Sep 17 00:00:00 2001 From: Thirukumaran Vaseeharan Date: Fri, 18 Mar 2022 10:35:36 -0700 Subject: [PATCH 2/3] Update gci tests --- test/linters_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/linters_test.go b/test/linters_test.go index a80baa6a511c..f294640b6ce3 100644 --- a/test/linters_test.go +++ b/test/linters_test.go @@ -96,7 +96,7 @@ func TestGciLocal(t *testing.T) { require.NoError(t, err) testshared.NewLintRunner(t).RunWithYamlConfig(string(cfg), args...). - ExpectHasIssue("testdata/gci/gci.go:9:1: Expected '\\n', Found '\\t'") + ExpectHasIssue("testdata/gci/gci.go:8: File is not `gci`-ed") } func TestMultipleOutputs(t *testing.T) { @@ -112,7 +112,7 @@ func TestMultipleOutputs(t *testing.T) { require.NoError(t, err) testshared.NewLintRunner(t).RunWithYamlConfig(string(cfg), args...). - ExpectHasIssue("testdata/gci/gci.go:9:1: Expected '\\n', Found '\\t'"). + ExpectHasIssue("testdata/gci/gci.go:8: File is not `gci`-ed"). ExpectOutputContains(`"Issues":[`) } @@ -129,7 +129,7 @@ func TestStderrOutput(t *testing.T) { require.NoError(t, err) testshared.NewLintRunner(t).RunWithYamlConfig(string(cfg), args...). - ExpectHasIssue("testdata/gci/gci.go:9:1: Expected '\\n', Found '\\t'"). + ExpectHasIssue("testdata/gci/gci.go:8: File is not `gci`-ed"). ExpectOutputContains(`"Issues":[`) } @@ -149,7 +149,7 @@ func TestFileOutput(t *testing.T) { require.NoError(t, err) testshared.NewLintRunner(t).RunWithYamlConfig(string(cfg), args...). - ExpectHasIssue("testdata/gci/gci.go:9:1: Expected '\\n', Found '\\t'"). + ExpectHasIssue("testdata/gci/gci.go:8: File is not `gci`-ed"). ExpectOutputNotContains(`"Issues":[`) b, err := os.ReadFile(resultPath) From 5bd9850dcef63942a59e3b9d99e154112543a0b6 Mon Sep 17 00:00:00 2001 From: Thirukumaran Vaseeharan Date: Fri, 18 Mar 2022 10:48:19 -0700 Subject: [PATCH 3/3] add fix tests for gci --- test/testdata/fix/in/gci.go | 15 +++++++++++++++ test/testdata/fix/out/gci.go | 17 +++++++++++++++++ 2 files changed, 32 insertions(+) create mode 100644 test/testdata/fix/in/gci.go create mode 100644 test/testdata/fix/out/gci.go diff --git a/test/testdata/fix/in/gci.go b/test/testdata/fix/in/gci.go new file mode 100644 index 000000000000..4efb0541063e --- /dev/null +++ b/test/testdata/fix/in/gci.go @@ -0,0 +1,15 @@ +//args: -Egci +//config_path: testdata/configs/gci.yml +package gci + +import ( + "github.com/golangci/golangci-lint/pkg/config" + "github.com/pkg/errors" + "fmt" +) + +func GoimportsLocalTest() { + fmt.Print("x") + _ = config.Config{} + _ = errors.New("") +} diff --git a/test/testdata/fix/out/gci.go b/test/testdata/fix/out/gci.go new file mode 100644 index 000000000000..a9dfbb07d76e --- /dev/null +++ b/test/testdata/fix/out/gci.go @@ -0,0 +1,17 @@ +//args: -Egci +//config_path: testdata/configs/gci.yml +package gci + +import ( + "fmt" + + "github.com/golangci/golangci-lint/pkg/config" + + "github.com/pkg/errors" +) + +func GoimportsLocalTest() { + fmt.Print("x") + _ = config.Config{} + _ = errors.New("") +}