Skip to content

Commit 8772d83

Browse files
daixiang0ldez
authored and
SeigeC
committed
gci: fix issues and re-enable autofix (golangci#2892)
Co-authored-by: Fernandez Ludovic <ldez@users.noreply.github.com>
1 parent 6d29b39 commit 8772d83

File tree

7 files changed

+161
-31
lines changed

7 files changed

+161
-31
lines changed

‎go.mod‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ require (
2020
github.com/breml/errchkjson v0.3.0
2121
github.com/butuzov/ireturn v0.1.1
2222
github.com/charithe/durationcheck v0.0.9
23-
github.com/daixiang0/gci v0.3.3
23+
github.com/daixiang0/gci v0.3.4
2424
github.com/denis-tingaikin/go-header v0.4.3
2525
github.com/esimonov/ifshort v1.0.4
2626
github.com/fatih/color v1.13.0
@@ -164,6 +164,9 @@ require (
164164
github.com/tklauser/numcpus v0.4.0 // indirect
165165
github.com/valyala/bytebufferpool v1.0.0 // indirect
166166
github.com/yusufpapurcu/wmi v1.2.2 // indirect
167+
go.uber.org/atomic v1.7.0 // indirect
168+
go.uber.org/multierr v1.6.0 // indirect
169+
go.uber.org/zap v1.17.0 // indirect
167170
golang.org/x/exp/typeparams v0.0.0-20220218215828-6cf2b201936e // indirect
168171
golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4 // indirect
169172
golang.org/x/sync v0.0.0-20210220032951-036812b2e83c // indirect

‎go.sum‎

Lines changed: 5 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎pkg/golinters/gci.go‎

Lines changed: 106 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,11 @@ package golinters
33
import (
44
"fmt"
55
"strings"
6+
"sync"
67

7-
gci "github.com/daixiang0/gci/pkg/analyzer"
8+
gcicfg "github.com/daixiang0/gci/pkg/configuration"
9+
"github.com/daixiang0/gci/pkg/gci"
10+
"github.com/pkg/errors"
811
"golang.org/x/tools/go/analysis"
912

1013
"github.com/golangci/golangci-lint/pkg/config"
@@ -15,33 +18,121 @@ import (
1518
const gciName = "gci"
1619

1720
func NewGci(settings *config.GciSettings) *goanalysis.Linter {
18-
var linterCfg map[string]map[string]interface{}
21+
var mu sync.Mutex
22+
var resIssues []goanalysis.Issue
23+
24+
analyzer := &analysis.Analyzer{
25+
Name: gciName,
26+
Doc: goanalysis.TheOnlyanalyzerDoc,
27+
Run: goanalysis.DummyRun,
28+
}
29+
30+
var cfg *gci.GciConfiguration
1931
if settings != nil {
20-
cfg := map[string]interface{}{
21-
gci.NoInlineCommentsFlag: settings.NoInlineComments,
22-
gci.NoPrefixCommentsFlag: settings.NoPrefixComments,
23-
gci.SectionsFlag: strings.Join(settings.Sections, gci.SectionDelimiter),
24-
gci.SectionSeparatorsFlag: strings.Join(settings.SectionSeparator, gci.SectionDelimiter),
32+
rawCfg := gci.GciStringConfiguration{
33+
Cfg: gcicfg.FormatterConfiguration{
34+
NoInlineComments: settings.NoInlineComments,
35+
NoPrefixComments: settings.NoPrefixComments,
36+
},
37+
SectionStrings: settings.Sections,
38+
SectionSeparatorStrings: settings.SectionSeparator,
2539
}
2640

2741
if settings.LocalPrefixes != "" {
2842
prefix := []string{"standard", "default", fmt.Sprintf("prefix(%s)", settings.LocalPrefixes)}
29-
cfg[gci.SectionsFlag] = strings.Join(prefix, gci.SectionDelimiter)
43+
rawCfg.SectionStrings = prefix
3044
}
3145

32-
linterCfg = map[string]map[string]interface{}{
33-
gci.Analyzer.Name: cfg,
34-
}
46+
cfg, _ = rawCfg.Parse()
3547
}
3648

49+
var lock sync.Mutex
50+
3751
return goanalysis.NewLinter(
3852
gciName,
3953
"Gci controls golang package import order and makes it always deterministic.",
40-
[]*analysis.Analyzer{gci.Analyzer},
41-
linterCfg,
54+
[]*analysis.Analyzer{analyzer},
55+
nil,
4256
).WithContextSetter(func(lintCtx *linter.Context) {
43-
if settings.LocalPrefixes != "" {
44-
lintCtx.Log.Warnf("gci: `local-prefixes` is deprecated, use `sections` and `prefix(%s)` instead.", settings.LocalPrefixes)
57+
analyzer.Run = func(pass *analysis.Pass) (interface{}, error) {
58+
issues, err := runGci(pass, lintCtx, cfg, &lock)
59+
if err != nil {
60+
return nil, err
61+
}
62+
63+
if len(issues) == 0 {
64+
return nil, nil
65+
}
66+
67+
mu.Lock()
68+
resIssues = append(resIssues, issues...)
69+
mu.Unlock()
70+
71+
return nil, nil
4572
}
73+
}).WithIssuesReporter(func(*linter.Context) []goanalysis.Issue {
74+
return resIssues
4675
}).WithLoadMode(goanalysis.LoadModeSyntax)
4776
}
77+
78+
func runGci(pass *analysis.Pass, lintCtx *linter.Context, cfg *gci.GciConfiguration, lock *sync.Mutex) ([]goanalysis.Issue, error) {
79+
var fileNames []string
80+
for _, f := range pass.Files {
81+
pos := pass.Fset.PositionFor(f.Pos(), false)
82+
fileNames = append(fileNames, pos.Filename)
83+
}
84+
85+
var diffs []string
86+
err := gci.DiffFormattedFilesToArray(fileNames, *cfg, &diffs, lock)
87+
if err != nil {
88+
return nil, err
89+
}
90+
91+
var issues []goanalysis.Issue
92+
93+
for _, diff := range diffs {
94+
if diff == "" {
95+
continue
96+
}
97+
98+
is, err := extractIssuesFromPatch(diff, lintCtx, gciName)
99+
if err != nil {
100+
return nil, errors.Wrapf(err, "can't extract issues from gci diff output %s", diff)
101+
}
102+
103+
for i := range is {
104+
issues = append(issues, goanalysis.NewIssue(&is[i], pass))
105+
}
106+
}
107+
108+
return issues, nil
109+
}
110+
111+
func getErrorTextForGci(settings config.GciSettings) string {
112+
text := "File is not `gci`-ed"
113+
114+
hasOptions := settings.NoInlineComments || settings.NoPrefixComments || len(settings.Sections) > 0 || len(settings.SectionSeparator) > 0
115+
if !hasOptions {
116+
return text
117+
}
118+
119+
text += " with"
120+
121+
if settings.NoInlineComments {
122+
text += " -NoInlineComments"
123+
}
124+
125+
if settings.NoPrefixComments {
126+
text += " -NoPrefixComments"
127+
}
128+
129+
if len(settings.Sections) > 0 {
130+
text += " -s " + strings.Join(settings.Sections, ",")
131+
}
132+
133+
if len(settings.SectionSeparator) > 0 {
134+
text += " -x " + strings.Join(settings.SectionSeparator, ",")
135+
}
136+
137+
return text
138+
}

‎pkg/golinters/gofmt_common.go‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"github.com/pkg/errors"
1010
diffpkg "github.com/sourcegraph/go-diff/diff"
1111

12+
"github.com/golangci/golangci-lint/pkg/config"
1213
"github.com/golangci/golangci-lint/pkg/lint/linter"
1314
"github.com/golangci/golangci-lint/pkg/logutils"
1415
"github.com/golangci/golangci-lint/pkg/result"
@@ -207,23 +208,25 @@ func (p *hunkChangesParser) parse(h *diffpkg.Hunk) []Change {
207208
return p.ret
208209
}
209210

210-
func getErrorTextForLinter(lintCtx *linter.Context, linterName string) string {
211+
func getErrorTextForLinter(settings *config.LintersSettings, linterName string) string {
211212
text := "File is not formatted"
212213
switch linterName {
214+
case gciName:
215+
text = getErrorTextForGci(settings.Gci)
213216
case gofumptName:
214217
text = "File is not `gofumpt`-ed"
215-
if lintCtx.Settings().Gofumpt.ExtraRules {
218+
if settings.Gofumpt.ExtraRules {
216219
text += " with `-extra`"
217220
}
218221
case gofmtName:
219222
text = "File is not `gofmt`-ed"
220-
if lintCtx.Settings().Gofmt.Simplify {
223+
if settings.Gofmt.Simplify {
221224
text += " with `-s`"
222225
}
223226
case goimportsName:
224227
text = "File is not `goimports`-ed"
225-
if lintCtx.Settings().Goimports.LocalPrefixes != "" {
226-
text += " with -local " + lintCtx.Settings().Goimports.LocalPrefixes
228+
if settings.Goimports.LocalPrefixes != "" {
229+
text += " with -local " + settings.Goimports.LocalPrefixes
227230
}
228231
}
229232
return text
@@ -247,9 +250,7 @@ func extractIssuesFromPatch(patch string, lintCtx *linter.Context, linterName st
247250
}
248251

249252
for _, hunk := range d.Hunks {
250-
p := hunkChangesParser{
251-
log: lintCtx.Log,
252-
}
253+
p := hunkChangesParser{log: lintCtx.Log}
253254

254255
changes := p.parse(hunk)
255256

@@ -261,7 +262,7 @@ func extractIssuesFromPatch(patch string, lintCtx *linter.Context, linterName st
261262
Filename: d.NewName,
262263
Line: change.LineRange.From,
263264
},
264-
Text: getErrorTextForLinter(lintCtx, linterName),
265+
Text: getErrorTextForLinter(lintCtx.Settings(), linterName),
265266
Replacement: &change.Replacement,
266267
}
267268
if change.LineRange.From != change.LineRange.To {

‎test/linters_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ func TestGciLocal(t *testing.T) {
105105
require.NoError(t, err)
106106

107107
testshared.NewLintRunner(t).RunWithYamlConfig(string(cfg), args...).
108-
ExpectHasIssue("testdata/gci/gci.go:9:1: Expected '\\n', Found '\\t'")
108+
ExpectHasIssue("testdata/gci/gci.go:8: File is not `gci`-ed")
109109
}
110110

111111
func TestMultipleOutputs(t *testing.T) {
@@ -124,7 +124,7 @@ func TestMultipleOutputs(t *testing.T) {
124124
require.NoError(t, err)
125125

126126
testshared.NewLintRunner(t).RunWithYamlConfig(string(cfg), args...).
127-
ExpectHasIssue("testdata/gci/gci.go:9:1: Expected '\\n', Found '\\t'").
127+
ExpectHasIssue("testdata/gci/gci.go:8: File is not `gci`-ed").
128128
ExpectOutputContains(`"Issues":[`)
129129
}
130130

@@ -144,7 +144,7 @@ func TestStderrOutput(t *testing.T) {
144144
require.NoError(t, err)
145145

146146
testshared.NewLintRunner(t).RunWithYamlConfig(string(cfg), args...).
147-
ExpectHasIssue("testdata/gci/gci.go:9:1: Expected '\\n', Found '\\t'").
147+
ExpectHasIssue("testdata/gci/gci.go:8: File is not `gci`-ed").
148148
ExpectOutputContains(`"Issues":[`)
149149
}
150150

@@ -167,7 +167,7 @@ func TestFileOutput(t *testing.T) {
167167
require.NoError(t, err)
168168

169169
testshared.NewLintRunner(t).RunWithYamlConfig(string(cfg), args...).
170-
ExpectHasIssue("testdata/gci/gci.go:9:1: Expected '\\n', Found '\\t'").
170+
ExpectHasIssue("testdata/gci/gci.go:8: File is not `gci`-ed").
171171
ExpectOutputNotContains(`"Issues":[`)
172172

173173
b, err := os.ReadFile(resultPath)

‎test/testdata/fix/in/gci.go‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
//args: -Egci
2+
//config_path: testdata/configs/gci.yml
3+
package gci
4+
5+
import (
6+
"github.com/golangci/golangci-lint/pkg/config"
7+
"github.com/pkg/errors"
8+
"fmt"
9+
)
10+
11+
func GoimportsLocalTest() {
12+
fmt.Print("x")
13+
_ = config.Config{}
14+
_ = errors.New("")
15+
}

‎test/testdata/fix/out/gci.go‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
//args: -Egci
2+
//config_path: testdata/configs/gci.yml
3+
package gci
4+
5+
import (
6+
"fmt"
7+
8+
"github.com/golangci/golangci-lint/pkg/config"
9+
10+
"github.com/pkg/errors"
11+
)
12+
13+
func GoimportsLocalTest() {
14+
fmt.Print("x")
15+
_ = config.Config{}
16+
_ = errors.New("")
17+
}

0 commit comments

Comments
 (0)