Skip to content

Commit 57ed6d5

Browse files
Allow custom linters to auto-fix
This allows custom linters hook into the `--fix` functionality. Custom linters specify the fixes using the Go analysis structures, which allow for arbitrary char offsets for fixes; they get converted into golangci structures, which are line-based. If the conversion is not possible, the fix is dropped on the floor. Signed-off-by: Steve Coffman <[email protected]>
1 parent 6edca92 commit 57ed6d5

File tree

3 files changed

+333
-29
lines changed

3 files changed

+333
-29
lines changed

pkg/golinters/goanalysis/linter.go

+5-1
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,11 @@ type Linter struct {
5151
needUseOriginalPackages bool
5252
}
5353

54-
func NewLinter(name, desc string, analyzers []*analysis.Analyzer, cfg map[string]map[string]interface{}) *Linter {
54+
func NewLinter(
55+
name, desc string,
56+
analyzers []*analysis.Analyzer,
57+
cfg map[string]map[string]interface{},
58+
) *Linter {
5559
return &Linter{name: name, desc: desc, analyzers: analyzers, cfg: cfg}
5660
}
5761

pkg/golinters/goanalysis/runners.go

+83-28
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package goanalysis
22

33
import (
44
"fmt"
5+
"go/token"
56
"runtime"
67
"sort"
78
"strings"
@@ -34,7 +35,14 @@ func runAnalyzers(cfg runAnalyzersConfig, lintCtx *linter.Context) ([]result.Iss
3435
const stagesToPrint = 10
3536
defer sw.PrintTopStages(stagesToPrint)
3637

37-
runner := newRunner(cfg.getName(), log, lintCtx.PkgCache, lintCtx.LoadGuard, cfg.getLoadMode(), sw)
38+
runner := newRunner(
39+
cfg.getName(),
40+
log,
41+
lintCtx.PkgCache,
42+
lintCtx.LoadGuard,
43+
cfg.getLoadMode(),
44+
sw,
45+
)
3846

3947
pkgs := lintCtx.Packages
4048
if cfg.useOriginalPackages() {
@@ -84,38 +92,70 @@ func runAnalyzers(cfg runAnalyzersConfig, lintCtx *linter.Context) ([]result.Iss
8492
return issues, nil
8593
}
8694

87-
func buildIssues(diags []Diagnostic, linterNameBuilder func(diag *Diagnostic) string) []result.Issue {
95+
func buildIssues(
96+
diags []Diagnostic,
97+
linterNameBuilder func(diag *Diagnostic) string,
98+
) []result.Issue {
8899
var issues []result.Issue
89100
for i := range diags {
90101
diag := &diags[i]
91-
linterName := linterNameBuilder(diag)
102+
issues = append(issues, buildSingleIssue(diag, linterNameBuilder(diag)))
103+
}
104+
return issues
105+
}
92106

93-
var text string
94-
if diag.Analyzer.Name == linterName {
95-
text = diag.Message
96-
} else {
97-
text = fmt.Sprintf("%s: %s", diag.Analyzer.Name, diag.Message)
98-
}
107+
func buildSingleIssue(diag *Diagnostic, linterName string) result.Issue {
108+
text := generateIssueText(diag, linterName)
109+
issue := result.Issue{
110+
FromLinter: linterName,
111+
Text: text,
112+
Pos: diag.Position,
113+
Pkg: diag.Pkg,
114+
}
115+
116+
if len(diag.SuggestedFixes) > 0 {
117+
// Don't really have a better way of picking a best fix right now
118+
chosenFix := diag.SuggestedFixes[0]
119+
120+
// It could be confusing to return more than one issue per single diagnostic,
121+
// but if we return a subset it might be a partial application of a fix. Don't
122+
// apply a fix unless there is only one for now
123+
if len(chosenFix.TextEdits) == 1 {
124+
edit := chosenFix.TextEdits[0]
125+
126+
pos := diag.Pkg.Fset.Position(edit.Pos)
127+
end := diag.Pkg.Fset.Position(edit.End)
128+
129+
newLines := strings.Split(string(edit.NewText), "\n")
99130

100-
issues = append(issues, result.Issue{
101-
FromLinter: linterName,
102-
Text: text,
103-
Pos: diag.Position,
104-
Pkg: diag.Pkg,
105-
})
106-
107-
if len(diag.Related) > 0 {
108-
for _, info := range diag.Related {
109-
issues = append(issues, result.Issue{
110-
FromLinter: linterName,
111-
Text: fmt.Sprintf("%s(related information): %s", diag.Analyzer.Name, info.Message),
112-
Pos: diag.Pkg.Fset.Position(info.Pos),
113-
Pkg: diag.Pkg,
114-
})
131+
// This only works if we're only replacing whole lines with brand-new lines
132+
if onlyReplacesWholeLines(pos, end, newLines) {
133+
// both original and new content ends with newline,
134+
// omit to avoid partial line replacement
135+
newLines = newLines[:len(newLines)-1]
136+
137+
issue.Replacement = &result.Replacement{NewLines: newLines}
138+
issue.LineRange = &result.Range{From: pos.Line, To: end.Line - 1}
139+
140+
return issue
115141
}
116142
}
117143
}
118-
return issues
144+
145+
return issue
146+
}
147+
148+
func onlyReplacesWholeLines(oPos, oEnd token.Position, newLines []string) bool {
149+
return oPos.Column == 1 && oEnd.Column == 1 &&
150+
oPos.Line < oEnd.Line && // must be replacing at least one line
151+
newLines[len(newLines)-1] == "" // edit.NewText ended with '\n'
152+
}
153+
154+
func generateIssueText(diag *Diagnostic, linterName string) string {
155+
if diag.Analyzer.Name == linterName {
156+
return diag.Message
157+
}
158+
return fmt.Sprintf("%s: %s", diag.Analyzer.Name, diag.Message)
119159
}
120160

121161
func getIssuesCacheKey(analyzers []*analysis.Analyzer) string {
@@ -160,7 +200,12 @@ func saveIssuesToCache(allPkgs []*packages.Package, pkgsFromCache map[*packages.
160200

161201
atomic.AddInt32(&savedIssuesCount, int32(len(encodedIssues)))
162202
if err := lintCtx.PkgCache.Put(pkg, pkgcache.HashModeNeedAllDeps, lintResKey, encodedIssues); err != nil {
163-
lintCtx.Log.Infof("Failed to save package %s issues (%d) to cache: %s", pkg, len(pkgIssues), err)
203+
lintCtx.Log.Infof(
204+
"Failed to save package %s issues (%d) to cache: %s",
205+
pkg,
206+
len(pkgIssues),
207+
err,
208+
)
164209
} else {
165210
issuesCacheDebugf("Saved package %s issues (%d) to cache", pkg, len(pkgIssues))
166211
}
@@ -178,7 +223,12 @@ func saveIssuesToCache(allPkgs []*packages.Package, pkgsFromCache map[*packages.
178223
close(pkgCh)
179224
wg.Wait()
180225

181-
issuesCacheDebugf("Saved %d issues from %d packages to cache in %s", savedIssuesCount, len(allPkgs), time.Since(startedAt))
226+
issuesCacheDebugf(
227+
"Saved %d issues from %d packages to cache in %s",
228+
savedIssuesCount,
229+
len(allPkgs),
230+
time.Since(startedAt),
231+
)
182232
}
183233

184234
//nolint:gocritic
@@ -206,7 +256,12 @@ func loadIssuesFromCache(pkgs []*packages.Package, lintCtx *linter.Context,
206256
defer wg.Done()
207257
for pkg := range pkgCh {
208258
var pkgIssues []EncodingIssue
209-
err := lintCtx.PkgCache.Get(pkg, pkgcache.HashModeNeedAllDeps, lintResKey, &pkgIssues)
259+
err := lintCtx.PkgCache.Get(
260+
pkg,
261+
pkgcache.HashModeNeedAllDeps,
262+
lintResKey,
263+
&pkgIssues,
264+
)
210265
cacheRes := pkgToCacheRes[pkg]
211266
cacheRes.loadErr = err
212267
if err != nil {

0 commit comments

Comments
 (0)