Skip to content

Commit 3d78f64

Browse files
committed
fix golangci#522: run misspell in text mode
Treat Go source files as plain text files by misspell: it allows detecting issues in strings, variable names, etc. Also, it's the default mode of a standalone misspell tool. Also, implement richer and more stable auto-fix of misspell issues: now it can fix multiple issues in one line.
1 parent 7db400b commit 3d78f64

File tree

15 files changed

+191
-136
lines changed

15 files changed

+191
-136
lines changed

pkg/commands/executor.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -96,9 +96,9 @@ func NewExecutor(version, commit, date string) *Executor {
9696
e.EnabledLintersSet = lintersdb.NewEnabledSet(e.DBManager,
9797
lintersdb.NewValidator(e.DBManager), e.log.Child("lintersdb"), e.cfg)
9898
e.goenv = goutil.NewEnv(e.log.Child("goenv"))
99-
e.contextLoader = lint.NewContextLoader(e.cfg, e.log.Child("loader"), e.goenv)
10099
e.fileCache = fsutils.NewFileCache()
101100
e.lineCache = fsutils.NewLineCache(e.fileCache)
101+
e.contextLoader = lint.NewContextLoader(e.cfg, e.log.Child("loader"), e.goenv, e.lineCache, e.fileCache)
102102

103103
return e
104104
}

pkg/golinters/gocritic.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ func (lint Gocritic) Run(ctx context.Context, lintCtx *linter.Context) ([]result
109109
go func() {
110110
defer func() {
111111
if err := recover(); err != nil {
112-
panicErr = fmt.Errorf("panic occured: %s", err)
112+
panicErr = fmt.Errorf("panic occurred: %s", err)
113113
lintCtx.Log.Warnf("Panic: %s", debug.Stack())
114114
}
115115
}()

pkg/golinters/misspell.go

+45-15
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"context"
55
"fmt"
66
"go/token"
7-
"io/ioutil"
87
"strings"
98

109
"github.com/golangci/misspell"
@@ -15,6 +14,10 @@ import (
1514

1615
type Misspell struct{}
1716

17+
func NewMisspell() *Misspell {
18+
return &Misspell{}
19+
}
20+
1821
func (Misspell) Name() string {
1922
return "misspell"
2023
}
@@ -50,25 +53,52 @@ func (lint Misspell) Run(ctx context.Context, lintCtx *linter.Context) ([]result
5053

5154
var res []result.Issue
5255
for _, f := range getAllFileNames(lintCtx) {
53-
fileContent, err := ioutil.ReadFile(f)
56+
issues, err := lint.runOnFile(f, &r, lintCtx)
5457
if err != nil {
55-
return nil, fmt.Errorf("can't read file %s: %s", f, err)
58+
return nil, err
5659
}
60+
res = append(res, issues...)
61+
}
62+
63+
return res, nil
64+
}
5765

58-
_, diffs := r.ReplaceGo(string(fileContent))
59-
for _, diff := range diffs {
60-
text := fmt.Sprintf("`%s` is a misspelling of `%s`", diff.Original, diff.Corrected)
61-
pos := token.Position{
62-
Filename: f,
63-
Line: diff.Line,
64-
Column: diff.Column + 1,
66+
func (lint Misspell) runOnFile(fileName string, r *misspell.Replacer, lintCtx *linter.Context) ([]result.Issue, error) {
67+
var res []result.Issue
68+
fileContent, err := lintCtx.FileCache.GetFileBytes(fileName)
69+
if err != nil {
70+
return nil, fmt.Errorf("can't get file %s contents: %s", fileName, err)
71+
}
72+
73+
// use r.Replace, not r.ReplaceGo because r.ReplaceGo doesn't find
74+
// issues inside strings: it searches only inside comments. r.Replace
75+
// searches all words: it treats input as a plain text. A standalone misspell
76+
// tool uses r.Replace by default.
77+
_, diffs := r.Replace(string(fileContent))
78+
for _, diff := range diffs {
79+
text := fmt.Sprintf("`%s` is a misspelling of `%s`", diff.Original, diff.Corrected)
80+
pos := token.Position{
81+
Filename: fileName,
82+
Line: diff.Line,
83+
Column: diff.Column + 1,
84+
}
85+
var replacement *result.Replacement
86+
if lintCtx.Cfg.Issues.NeedFix {
87+
replacement = &result.Replacement{
88+
Inline: &result.InlineFix{
89+
StartCol: diff.Column,
90+
Length: len(diff.Original),
91+
NewString: diff.Corrected,
92+
},
6593
}
66-
res = append(res, result.Issue{
67-
Pos: pos,
68-
Text: text,
69-
FromLinter: lint.Name(),
70-
})
7194
}
95+
96+
res = append(res, result.Issue{
97+
Pos: pos,
98+
Text: text,
99+
FromLinter: lint.Name(),
100+
Replacement: replacement,
101+
})
72102
}
73103

74104
return res, nil

pkg/lint/linter/context.go

+7-3
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ import (
55
"golang.org/x/tools/go/packages"
66
"golang.org/x/tools/go/ssa"
77

8+
"github.com/golangci/golangci-lint/pkg/fsutils"
9+
810
"github.com/golangci/golangci-lint/pkg/config"
911
"github.com/golangci/golangci-lint/pkg/lint/astcache"
1012
"github.com/golangci/golangci-lint/pkg/logutils"
@@ -19,9 +21,11 @@ type Context struct {
1921

2022
SSAProgram *ssa.Program // for unparam and interfacer but not for megacheck (it change it)
2123

22-
Cfg *config.Config
23-
ASTCache *astcache.Cache
24-
Log logutils.Log
24+
Cfg *config.Config
25+
ASTCache *astcache.Cache
26+
FileCache *fsutils.FileCache
27+
LineCache *fsutils.LineCache
28+
Log logutils.Log
2529
}
2630

2731
func (c *Context) Settings() *config.LintersSettings {

pkg/lint/load.go

+14-4
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ import (
1111
"strings"
1212
"time"
1313

14+
"github.com/golangci/golangci-lint/pkg/fsutils"
15+
1416
"github.com/pkg/errors"
1517
"golang.org/x/tools/go/loader"
1618
"golang.org/x/tools/go/packages"
@@ -31,15 +33,21 @@ type ContextLoader struct {
3133
debugf logutils.DebugFunc
3234
goenv *goutil.Env
3335
pkgTestIDRe *regexp.Regexp
36+
lineCache *fsutils.LineCache
37+
fileCache *fsutils.FileCache
3438
}
3539

36-
func NewContextLoader(cfg *config.Config, log logutils.Log, goenv *goutil.Env) *ContextLoader {
40+
func NewContextLoader(cfg *config.Config, log logutils.Log, goenv *goutil.Env,
41+
lineCache *fsutils.LineCache, fileCache *fsutils.FileCache) *ContextLoader {
42+
3743
return &ContextLoader{
3844
cfg: cfg,
3945
log: log,
4046
debugf: logutils.Debug("loader"),
4147
goenv: goenv,
4248
pkgTestIDRe: regexp.MustCompile(`^(.*) \[(.*)\.test\]`),
49+
lineCache: lineCache,
50+
fileCache: fileCache,
4351
}
4452
}
4553

@@ -356,9 +364,11 @@ func (cl ContextLoader) Load(ctx context.Context, linters []*linter.Config) (*li
356364
Cwd: "", // used by depguard and fallbacked to os.Getcwd
357365
Build: nil, // used by depguard and megacheck and fallbacked to build.Default
358366
},
359-
Cfg: cl.cfg,
360-
ASTCache: astCache,
361-
Log: cl.log,
367+
Cfg: cl.cfg,
368+
ASTCache: astCache,
369+
Log: cl.log,
370+
FileCache: cl.fileCache,
371+
LineCache: cl.lineCache,
362372
}
363373

364374
separateNotCompilingPackages(ret)

pkg/lint/runner.go

+1-2
Original file line numberDiff line numberDiff line change
@@ -79,13 +79,12 @@ func NewRunner(astCache *astcache.Cache, cfg *config.Config, log logutils.Log, g
7979
processors.NewExcludeRules(excludeRules, lineCache, log.Child("exclude_rules")),
8080
processors.NewNolint(astCache, log.Child("nolint"), dbManager),
8181

82-
processors.NewUniqByLine(),
82+
processors.NewUniqByLine(cfg),
8383
processors.NewDiff(icfg.Diff, icfg.DiffFromRevision, icfg.DiffPatchFilePath),
8484
processors.NewMaxPerFileFromLinter(cfg),
8585
processors.NewMaxSameIssues(icfg.MaxSameIssues, log.Child("max_same_issues"), cfg),
8686
processors.NewMaxFromLinter(icfg.MaxIssuesPerLinter, log.Child("max_from_linter"), cfg),
8787
processors.NewSourceCode(lineCache, log.Child("source_code")),
88-
processors.NewReplacementBuilder(log.Child("replacement_builder")), // must be after source code
8988
processors.NewPathShortener(),
9089
},
9190
Log: log,

pkg/result/issue.go

+7
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,13 @@ type Range struct {
99
type Replacement struct {
1010
NeedOnlyDelete bool // need to delete all lines of the issue without replacement with new lines
1111
NewLines []string // is NeedDelete is false it's the replacement lines
12+
Inline *InlineFix
13+
}
14+
15+
type InlineFix struct {
16+
StartCol int // zero-based
17+
Length int // length of chunk to be replaced
18+
NewString string
1219
}
1320

1421
type Issue struct {

pkg/result/processors/fixer.go

+81-1
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,19 @@ func (f Fixer) fixIssuesInFile(filePath string, issues []result.Issue) error {
7777
return errors.Wrapf(err, "failed to make file %s", tmpFileName)
7878
}
7979

80+
// merge multiple issues per line into one issue
81+
issuesPerLine := map[int][]result.Issue{}
82+
for _, i := range issues {
83+
issuesPerLine[i.Line()] = append(issuesPerLine[i.Line()], i)
84+
}
85+
86+
issues = issues[:0] // reuse the same memory
87+
for line, lineIssues := range issuesPerLine {
88+
if mergedIssue := f.mergeLineIssues(line, lineIssues, origFileLines); mergedIssue != nil {
89+
issues = append(issues, *mergedIssue)
90+
}
91+
}
92+
8093
issues = f.findNotIntersectingIssues(issues)
8194

8295
if err = f.writeFixedFile(origFileLines, issues, tmpOutFile); err != nil {
@@ -94,9 +107,76 @@ func (f Fixer) fixIssuesInFile(filePath string, issues []result.Issue) error {
94107
return nil
95108
}
96109

110+
//nolint:gocyclo
111+
func (f Fixer) mergeLineIssues(lineNum int, lineIssues []result.Issue, origFileLines [][]byte) *result.Issue {
112+
origLine := origFileLines[lineNum-1] // lineNum is 1-based
113+
114+
if len(lineIssues) == 1 && lineIssues[0].Replacement.Inline == nil {
115+
return &lineIssues[0]
116+
}
117+
118+
// check issues first
119+
for _, i := range lineIssues {
120+
if i.LineRange != nil {
121+
f.log.Infof("Line %d has multiple issues but at least one of them is ranged: %#v", lineNum, lineIssues)
122+
return &lineIssues[0]
123+
}
124+
125+
r := i.Replacement
126+
if r.Inline == nil || len(r.NewLines) != 0 || r.NeedOnlyDelete {
127+
f.log.Infof("Line %d has multiple issues but at least one of them isn't inline: %#v", lineNum, lineIssues)
128+
return &lineIssues[0]
129+
}
130+
131+
if r.Inline.StartCol < 0 || r.Inline.Length <= 0 || r.Inline.StartCol+r.Inline.Length > len(origLine) {
132+
f.log.Warnf("Line %d (%q) has invalid inline fix: %#v, %#v", lineNum, origLine, i, r.Inline)
133+
return nil
134+
}
135+
}
136+
137+
return f.applyInlineFixes(lineIssues, origLine, lineNum)
138+
}
139+
140+
func (f Fixer) applyInlineFixes(lineIssues []result.Issue, origLine []byte, lineNum int) *result.Issue {
141+
sort.Slice(lineIssues, func(i, j int) bool {
142+
return lineIssues[i].Replacement.Inline.StartCol < lineIssues[j].Replacement.Inline.StartCol
143+
})
144+
145+
var newLineBuf bytes.Buffer
146+
newLineBuf.Grow(len(origLine))
147+
148+
//nolint:misspell
149+
// example: origLine="it's becouse of them", StartCol=5, Length=7, NewString="because"
150+
151+
curOrigLinePos := 0
152+
for _, i := range lineIssues {
153+
fix := i.Replacement.Inline
154+
if fix.StartCol < curOrigLinePos {
155+
f.log.Warnf("Line %d has multiple intersecting issues: %#v", lineNum, lineIssues)
156+
return nil
157+
}
158+
159+
if curOrigLinePos != fix.StartCol {
160+
newLineBuf.Write(origLine[curOrigLinePos:fix.StartCol])
161+
}
162+
newLineBuf.WriteString(fix.NewString)
163+
curOrigLinePos = fix.StartCol + fix.Length
164+
}
165+
if curOrigLinePos != len(origLine) {
166+
newLineBuf.Write(origLine[curOrigLinePos:])
167+
}
168+
169+
mergedIssue := lineIssues[0] // use text from the first issue (it's not really used)
170+
mergedIssue.Replacement = &result.Replacement{
171+
NewLines: []string{newLineBuf.String()},
172+
}
173+
return &mergedIssue
174+
}
175+
97176
func (f Fixer) findNotIntersectingIssues(issues []result.Issue) []result.Issue {
98177
sort.SliceStable(issues, func(i, j int) bool {
99-
return issues[i].Line() < issues[j].Line() //nolint:scopelint
178+
a, b := issues[i], issues[j] //nolint:scopelint
179+
return a.Line() < b.Line()
100180
})
101181

102182
var ret []result.Issue

pkg/result/processors/max_from_linter.go

+5-5
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,12 @@ func (p *MaxFromLinter) Process(issues []result.Issue) ([]result.Issue, error) {
3333
return issues, nil
3434
}
3535

36-
if p.cfg.Issues.NeedFix {
37-
// we need to fix all issues at once => we need to return all of them
38-
return issues, nil
39-
}
40-
4136
return filterIssues(issues, func(i *result.Issue) bool {
37+
if i.Replacement != nil && p.cfg.Issues.NeedFix {
38+
// we need to fix all issues at once => we need to return all of them
39+
return true
40+
}
41+
4242
p.lc[i.FromLinter]++ // always inc for stat
4343
return p.lc[i.FromLinter] <= p.limit
4444
}), nil

pkg/result/processors/max_same_issues.go

+5-5
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,12 @@ func (p *MaxSameIssues) Process(issues []result.Issue) ([]result.Issue, error) {
3838
return issues, nil
3939
}
4040

41-
if p.cfg.Issues.NeedFix {
42-
// we need to fix all issues at once => we need to return all of them
43-
return issues, nil
44-
}
45-
4641
return filterIssues(issues, func(i *result.Issue) bool {
42+
if i.Replacement != nil && p.cfg.Issues.NeedFix {
43+
// we need to fix all issues at once => we need to return all of them
44+
return true
45+
}
46+
4747
p.tc[i.Text]++ // always inc for stat
4848
return p.tc[i.Text] <= p.limit
4949
}), nil

0 commit comments

Comments
 (0)