gci: fix issues and re-enable autofix (#2892)

Co-authored-by: Fernandez Ludovic <ldez@users.noreply.github.com>

go.mod

@@ -20,7 +20,7 @@ require (
 	github.com/breml/errchkjson v0.3.0
 	github.com/butuzov/ireturn v0.1.1
 	github.com/charithe/durationcheck v0.0.9
-	github.com/daixiang0/gci v0.3.3
+	github.com/daixiang0/gci v0.3.4
 	github.com/denis-tingaikin/go-header v0.4.3
 	github.com/esimonov/ifshort v1.0.4
 	github.com/fatih/color v1.13.0
@@ -164,6 +164,9 @@ require (
 	github.com/tklauser/numcpus v0.4.0 // indirect
 	github.com/valyala/bytebufferpool v1.0.0 // indirect
 	github.com/yusufpapurcu/wmi v1.2.2 // indirect
+	go.uber.org/atomic v1.7.0 // indirect
+	go.uber.org/multierr v1.6.0 // indirect
+	go.uber.org/zap v1.17.0 // indirect
 	golang.org/x/exp/typeparams v0.0.0-20220218215828-6cf2b201936e // indirect
 	golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4 // indirect
 	golang.org/x/sync v0.0.0-20210220032951-036812b2e83c // indirect

go.sum

@@ -162,8 +162,8 @@ github.com/cpuguy83/go-md2man/v2 v2.0.0/go.mod h1:maD7wRr/U5Z6m/iR4s+kqSMx2CaBsr
 github.com/cpuguy83/go-md2man/v2 v2.0.1/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o=
 github.com/creack/pty v1.1.7/go.mod h1:lj5s0c3V2DBrqTV7llrYr5NG6My20zk30Fl46Y7DoTY=
 github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
-github.com/daixiang0/gci v0.3.3 h1:55xJKH7Gl9Vk6oQ1cMkwrDWjAkT1D+D1G9kNmRcAIY4=
+github.com/daixiang0/gci v0.3.4 h1:+EZ83znNs73C9ZBTM7xhNagMP6gJs5wlptiFiuce5BM=
-github.com/daixiang0/gci v0.3.3/go.mod h1:1Xr2bxnQbDxCqqulUOv8qpGqkgRw9RSCGGjEC2LjF8o=
+github.com/daixiang0/gci v0.3.4/go.mod h1:pB1j339Q+2sv/EyKd4dgvGXcaBGIErim+dlhLDtqeW4=
 github.com/davecgh/go-spew v0.0.0-20161028175848-04cdfd42973b/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
@@ -858,14 +858,17 @@ go.opentelemetry.io/proto/otlp v0.7.0/go.mod h1:PqfVotwruBrMGOCsRd/89rSnXhoiJIqe
 go.uber.org/atomic v1.3.2/go.mod h1:gD2HeocX3+yG+ygLZcrzQJaqmWj9AIm7n08wl/qW/PE=
 go.uber.org/atomic v1.4.0/go.mod h1:gD2HeocX3+yG+ygLZcrzQJaqmWj9AIm7n08wl/qW/PE=
 go.uber.org/atomic v1.5.0/go.mod h1:sABNBOSYdrvTF6hTgEIbc7YasKWGhgEQZyfxyTvoXHQ=
+go.uber.org/atomic v1.7.0 h1:ADUqmZGgLDDfbSL9ZmPxKTybcoEYHgpYfELNoN+7hsw=
 go.uber.org/atomic v1.7.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc=
 go.uber.org/multierr v1.1.0/go.mod h1:wR5kodmAFQ0UK8QlbwjlSNy0Z68gJhDJUG5sjR94q/0=
 go.uber.org/multierr v1.3.0/go.mod h1:VgVr7evmIr6uPjLBxg28wmKNXyqE9akIJ5XnfpiKl+4=
 go.uber.org/multierr v1.4.0/go.mod h1:VgVr7evmIr6uPjLBxg28wmKNXyqE9akIJ5XnfpiKl+4=
+go.uber.org/multierr v1.6.0 h1:y6IPFStTAIT5Ytl7/XYmHvzXQ7S3g/IeZW9hyZ5thw4=
 go.uber.org/multierr v1.6.0/go.mod h1:cdWPpRnG4AhwMwsgIHip0KRBQjJy5kYEpYjJxpXp9iU=
 go.uber.org/tools v0.0.0-20190618225709-2cfd321de3ee/go.mod h1:vJERXedbb3MVM5f9Ejo0C68/HhF8uaILCdgjnY+goOA=
 go.uber.org/zap v1.10.0/go.mod h1:vwi/ZaCAaUcBkycHslxD9B2zi4UTXhF60s6SWpuDF0Q=
 go.uber.org/zap v1.13.0/go.mod h1:zwrFLgMcdUuIBviXEYEH1YKNaOBnKXsx2IPda5bBwHM=
+go.uber.org/zap v1.17.0 h1:MTjgFu6ZLKvY6Pvaqk97GlxNBuMpV4Hy/3P6tRGlI2U=
 go.uber.org/zap v1.17.0/go.mod h1:MXVU+bhUf/A7Xi2HNOnopQOrmycQ5Ih87HtOu4q5SSo=
 golang.org/x/crypto v0.0.0-20180501155221-613d6eafa307/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4=
 golang.org/x/crypto v0.0.0-20180904163835-0709b304e793/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4=

pkg/golinters/gci.go

@@ -3,8 +3,11 @@ package golinters
 import (
 	"fmt"
 	"strings"
+	"sync"
 
-	gci "github.com/daixiang0/gci/pkg/analyzer"
+	gcicfg "github.com/daixiang0/gci/pkg/configuration"
+	"github.com/daixiang0/gci/pkg/gci"
+	"github.com/pkg/errors"
 	"golang.org/x/tools/go/analysis"
 
 	"github.com/golangci/golangci-lint/pkg/config"
@@ -15,33 +18,121 @@ import (
 const gciName = "gci"
 
 func NewGci(settings *config.GciSettings) *goanalysis.Linter {
-	var linterCfg map[string]map[string]interface{}
+	var mu sync.Mutex
+	var resIssues []goanalysis.Issue
+
+	analyzer := &analysis.Analyzer{
+		Name: gciName,
+		Doc:  goanalysis.TheOnlyanalyzerDoc,
+		Run:  goanalysis.DummyRun,
+	}
+
+	var cfg *gci.GciConfiguration
 	if settings != nil {
-		cfg := map[string]interface{}{
+		rawCfg := gci.GciStringConfiguration{
-			gci.NoInlineCommentsFlag:  settings.NoInlineComments,
+			Cfg: gcicfg.FormatterConfiguration{
-			gci.NoPrefixCommentsFlag:  settings.NoPrefixComments,
+				NoInlineComments: settings.NoInlineComments,
-			gci.SectionsFlag:          strings.Join(settings.Sections, gci.SectionDelimiter),
+				NoPrefixComments: settings.NoPrefixComments,
-			gci.SectionSeparatorsFlag: strings.Join(settings.SectionSeparator, gci.SectionDelimiter),
+			},
+			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)
+			rawCfg.SectionStrings = prefix
 		}
 
-		linterCfg = map[string]map[string]interface{}{
+		cfg, _ = rawCfg.Parse()
-			gci.Analyzer.Name: cfg,
-		}
 	}
 
+	var lock sync.Mutex
+
 	return goanalysis.NewLinter(
 		gciName,
 		"Gci controls golang package import order and makes it always deterministic.",
-		[]*analysis.Analyzer{gci.Analyzer},
+		[]*analysis.Analyzer{analyzer},
-		linterCfg,
+		nil,
 	).WithContextSetter(func(lintCtx *linter.Context) {
-		if settings.LocalPrefixes != "" {
+		analyzer.Run = func(pass *analysis.Pass) (interface{}, error) {
-			lintCtx.Log.Warnf("gci: `local-prefixes` is deprecated, use `sections` and `prefix(%s)` instead.", settings.LocalPrefixes)
+			issues, err := runGci(pass, lintCtx, cfg, &lock)
+			if err != nil {
+				return nil, err
 			}
+
+			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)
 }
+
+func runGci(pass *analysis.Pass, lintCtx *linter.Context, cfg *gci.GciConfiguration, lock *sync.Mutex) ([]goanalysis.Issue, error) {
+	var fileNames []string
+	for _, f := range pass.Files {
+		pos := pass.Fset.PositionFor(f.Pos(), false)
+		fileNames = append(fileNames, pos.Filename)
+	}
+
+	var diffs []string
+	err := gci.DiffFormattedFilesToArray(fileNames, *cfg, &diffs, lock)
+	if err != nil {
+		return nil, err
+	}
+
+	var issues []goanalysis.Issue
+
+	for _, diff := range diffs {
+		if diff == "" {
+			continue
+		}
+
+		is, err := extractIssuesFromPatch(diff, lintCtx, gciName)
+		if err != nil {
+			return nil, errors.Wrapf(err, "can't extract issues from gci diff output %s", diff)
+		}
+
+		for i := range is {
+			issues = append(issues, goanalysis.NewIssue(&is[i], pass))
+		}
+	}
+
+	return issues, nil
+}
+
+func getErrorTextForGci(settings config.GciSettings) string {
+	text := "File is not `gci`-ed"
+
+	hasOptions := settings.NoInlineComments || settings.NoPrefixComments || len(settings.Sections) > 0 || len(settings.SectionSeparator) > 0
+	if !hasOptions {
+		return text
+	}
+
+	text += " with"
+
+	if settings.NoInlineComments {
+		text += " -NoInlineComments"
+	}
+
+	if settings.NoPrefixComments {
+		text += " -NoPrefixComments"
+	}
+
+	if len(settings.Sections) > 0 {
+		text += " -s " + strings.Join(settings.Sections, ",")
+	}
+
+	if len(settings.SectionSeparator) > 0 {
+		text += " -x " + strings.Join(settings.SectionSeparator, ",")
+	}
+
+	return text
+}

pkg/golinters/gofmt_common.go

@@ -9,6 +9,7 @@ import (
 	"github.com/pkg/errors"
 	diffpkg "github.com/sourcegraph/go-diff/diff"
 
+	"github.com/golangci/golangci-lint/pkg/config"
 	"github.com/golangci/golangci-lint/pkg/lint/linter"
 	"github.com/golangci/golangci-lint/pkg/logutils"
 	"github.com/golangci/golangci-lint/pkg/result"
@@ -207,23 +208,25 @@ func (p *hunkChangesParser) parse(h *diffpkg.Hunk) []Change {
 	return p.ret
 }
 
-func getErrorTextForLinter(lintCtx *linter.Context, linterName string) string {
+func getErrorTextForLinter(settings *config.LintersSettings, linterName string) string {
 	text := "File is not formatted"
 	switch linterName {
+	case gciName:
+		text = getErrorTextForGci(settings.Gci)
 	case gofumptName:
 		text = "File is not `gofumpt`-ed"
-		if lintCtx.Settings().Gofumpt.ExtraRules {
+		if settings.Gofumpt.ExtraRules {
 			text += " with `-extra`"
 		}
 	case gofmtName:
 		text = "File is not `gofmt`-ed"
-		if lintCtx.Settings().Gofmt.Simplify {
+		if settings.Gofmt.Simplify {
 			text += " with `-s`"
 		}
 	case goimportsName:
 		text = "File is not `goimports`-ed"
-		if lintCtx.Settings().Goimports.LocalPrefixes != "" {
+		if settings.Goimports.LocalPrefixes != "" {
-			text += " with -local " + lintCtx.Settings().Goimports.LocalPrefixes
+			text += " with -local " + settings.Goimports.LocalPrefixes
 		}
 	}
 	return text
@@ -247,9 +250,7 @@ func extractIssuesFromPatch(patch string, lintCtx *linter.Context, linterName st
 		}
 
 		for _, hunk := range d.Hunks {
-			p := hunkChangesParser{
+			p := hunkChangesParser{log: lintCtx.Log}
-				log: lintCtx.Log,
-			}
 
 			changes := p.parse(hunk)
 
@@ -261,7 +262,7 @@ func extractIssuesFromPatch(patch string, lintCtx *linter.Context, linterName st
 						Filename: d.NewName,
 						Line:     change.LineRange.From,
 					},
-					Text:        getErrorTextForLinter(lintCtx, linterName),
+					Text:        getErrorTextForLinter(lintCtx.Settings(), linterName),
 					Replacement: &change.Replacement,
 				}
 				if change.LineRange.From != change.LineRange.To {

test/linters_test.go

@@ -105,7 +105,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) {
@@ -124,7 +124,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":[`)
 }
 
@@ -144,7 +144,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":[`)
 }
 
@@ -167,7 +167,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)

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("")
+}

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("")
+}