Skip to content

Commit 6e7c317

Browse files
butuzovbutuzov
authored
Sorting result.Issues implementation (#1217) (#1218)
1 parent 65e1b30 commit 6e7c317

File tree

7 files changed

+409
-0
lines changed

7 files changed

+409
-0
lines changed

‎pkg/commands/run.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ func initFlagSet(fs *pflag.FlagSet, cfg *config.Config, m *lintersdb.Manager, is
8080
fs.BoolVar(&oc.PrintIssuedLine, "print-issued-lines", true, wh("Print lines of code with issue"))
8181
fs.BoolVar(&oc.PrintLinterName, "print-linter-name", true, wh("Print linter name in issue line"))
8282
fs.BoolVar(&oc.UniqByLine, "uniq-by-line", true, wh("Make issues output unique by line"))
83+
fs.BoolVar(&oc.SortResults, "sort-results", false, wh("Sort linter results"))
8384
fs.BoolVar(&oc.PrintWelcomeMessage, "print-welcome", false, wh("Print welcome message"))
8485
fs.StringVar(&oc.PathPrefix, "path-prefix", "", wh("Path prefix to add to output"))
8586
hideFlag("print-welcome") // no longer used

‎pkg/config/config.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -527,6 +527,7 @@ type Config struct {
527527
PrintIssuedLine bool `mapstructure:"print-issued-lines"`
528528
PrintLinterName bool `mapstructure:"print-linter-name"`
529529
UniqByLine bool `mapstructure:"uniq-by-line"`
530+
SortResults bool `mapstructure:"sort-results"`
530531
PrintWelcomeMessage bool `mapstructure:"print-welcome"`
531532
PathPrefix string `mapstructure:"path-prefix"`
532533
}

‎pkg/lint/runner.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ func NewRunner(cfg *config.Config, log logutils.Log, goenv *goutil.Env, es *lint
8080
processors.NewPathShortener(),
8181
getSeverityRulesProcessor(&cfg.Severity, log, lineCache),
8282
processors.NewPathPrefixer(cfg.Output.PathPrefix),
83+
processors.NewSortResults(cfg),
8384
},
8485
Log: log,
8586
}, nil

‎pkg/result/processors/sort_results.go

Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,173 @@
1+
package processors
2+
3+
import (
4+
"sort"
5+
"strings"
6+
7+
"github.com/golangci/golangci-lint/pkg/config"
8+
"github.com/golangci/golangci-lint/pkg/result"
9+
)
10+
11+
// Base propose of this functionality to sort results (issues)
12+
// produced by various linters by analyzing code. We achieving this
13+
// by sorting results.Issues using processor step, and chain based
14+
// rules that can compare different properties of the Issues struct.
15+
16+
var _ Processor = (*SortResults)(nil)
17+
18+
type SortResults struct {
19+
cmp comparator
20+
cfg *config.Config
21+
}
22+
23+
func NewSortResults(cfg *config.Config) *SortResults {
24+
// For sorting we are comparing (in next order): file names, line numbers,
25+
// position, and finally - giving up.
26+
return &SortResults{
27+
cmp: ByName{
28+
next: ByLine{
29+
next: ByColumn{},
30+
},
31+
},
32+
cfg: cfg,
33+
}
34+
}
35+
36+
// Process is performing sorting of the result issues.
37+
func (sr SortResults) Process(issues []result.Issue) ([]result.Issue, error) {
38+
if !sr.cfg.Output.SortResults {
39+
return issues, nil
40+
}
41+
42+
sort.Slice(issues, func(i, j int) bool {
43+
return sr.cmp.Compare(&issues[i], &issues[j]) == Less
44+
})
45+
46+
return issues, nil
47+
}
48+
49+
func (sr SortResults) Name() string { return "sort_results" }
50+
func (sr SortResults) Finish() {}
51+
52+
type compareResult int
53+
54+
const (
55+
Less compareResult = iota - 1
56+
Equal
57+
Greater
58+
None
59+
)
60+
61+
func (c compareResult) isNeutral() bool {
62+
// return true if compare result is incomparable or equal.
63+
return c == None || c == Equal
64+
}
65+
66+
//nolint:exhaustive
67+
func (c compareResult) String() string {
68+
switch c {
69+
case Less:
70+
return "Less"
71+
case Equal:
72+
return "Equal"
73+
case Greater:
74+
return "Greater"
75+
}
76+
77+
return "None"
78+
}
79+
80+
// comparator describe how to implement compare for two "issues" lexicographically
81+
type comparator interface {
82+
Compare(a, b *result.Issue) compareResult
83+
Next() comparator
84+
}
85+
86+
var (
87+
_ comparator = (*ByName)(nil)
88+
_ comparator = (*ByLine)(nil)
89+
_ comparator = (*ByColumn)(nil)
90+
)
91+
92+
type ByName struct{ next comparator }
93+
94+
//nolint:golint
95+
func (cmp ByName) Next() comparator { return cmp.next }
96+
97+
//nolint:golint
98+
func (cmp ByName) Compare(a, b *result.Issue) compareResult {
99+
var res compareResult
100+
101+
if res = compareResult(strings.Compare(a.FilePath(), b.FilePath())); !res.isNeutral() {
102+
return res
103+
}
104+
105+
if next := cmp.Next(); next != nil {
106+
return next.Compare(a, b)
107+
}
108+
109+
return res
110+
}
111+
112+
type ByLine struct{ next comparator }
113+
114+
//nolint:golint
115+
func (cmp ByLine) Next() comparator { return cmp.next }
116+
117+
//nolint:golint
118+
func (cmp ByLine) Compare(a, b *result.Issue) compareResult {
119+
var res compareResult
120+
121+
if res = numericCompare(a.Line(), b.Line()); !res.isNeutral() {
122+
return res
123+
}
124+
125+
if next := cmp.Next(); next != nil {
126+
return next.Compare(a, b)
127+
}
128+
129+
return res
130+
}
131+
132+
type ByColumn struct{ next comparator }
133+
134+
//nolint:golint
135+
func (cmp ByColumn) Next() comparator { return cmp.next }
136+
137+
//nolint:golint
138+
func (cmp ByColumn) Compare(a, b *result.Issue) compareResult {
139+
var res compareResult
140+
141+
if res = numericCompare(a.Column(), b.Column()); !res.isNeutral() {
142+
return res
143+
}
144+
145+
if next := cmp.Next(); next != nil {
146+
return next.Compare(a, b)
147+
}
148+
149+
return res
150+
}
151+
152+
func numericCompare(a, b int) compareResult {
153+
var (
154+
isValuesInvalid = a < 0 || b < 0
155+
isZeroValuesBoth = a == 0 && b == 0
156+
isEqual = a == b
157+
isZeroValueInA = b > 0 && a == 0
158+
isZeroValueInB = a > 0 && b == 0
159+
)
160+
161+
switch {
162+
case isZeroValuesBoth || isEqual:
163+
return Equal
164+
case isValuesInvalid || isZeroValueInA || isZeroValueInB:
165+
return None
166+
case a > b:
167+
return Greater
168+
case a < b:
169+
return Less
170+
}
171+
172+
return Equal
173+
}

‎pkg/result/processors/sort_results_test.go

Lines changed: 182 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,182 @@
1+
package processors
2+
3+
import (
4+
"fmt"
5+
"go/token"
6+
"testing"
7+
8+
"github.com/stretchr/testify/assert"
9+
10+
"github.com/golangci/golangci-lint/pkg/config"
11+
"github.com/golangci/golangci-lint/pkg/result"
12+
)
13+
14+
var issues = []result.Issue{
15+
{
16+
Pos: token.Position{
17+
Filename: "file_windows.go",
18+
Column: 80,
19+
Line: 10,
20+
},
21+
},
22+
{
23+
Pos: token.Position{
24+
Filename: "file_linux.go",
25+
Column: 70,
26+
Line: 11,
27+
},
28+
},
29+
{
30+
Pos: token.Position{
31+
Filename: "file_darwin.go",
32+
Line: 12,
33+
},
34+
},
35+
{
36+
Pos: token.Position{
37+
Filename: "file_darwin.go",
38+
Column: 60,
39+
Line: 10,
40+
},
41+
},
42+
}
43+
44+
type compareTestCase struct {
45+
a, b result.Issue
46+
expected compareResult
47+
}
48+
49+
func testCompareValues(t *testing.T, cmp comparator, name string, tests []compareTestCase) {
50+
t.Parallel()
51+
52+
for i := 0; i < len(tests); i++ {
53+
test := tests[i]
54+
t.Run(fmt.Sprintf("%s(%d)", name, i), func(t *testing.T) {
55+
res := cmp.Compare(&test.a, &test.b)
56+
assert.Equal(t, test.expected.String(), res.String())
57+
})
58+
}
59+
}
60+
61+
func TestCompareByLine(t *testing.T) {
62+
testCompareValues(t, ByLine{}, "Compare By Line", []compareTestCase{
63+
{issues[0], issues[1], Less}, // 10 vs 11
64+
{issues[0], issues[0], Equal}, // 10 vs 10
65+
{issues[3], issues[3], Equal}, // 10 vs 10
66+
{issues[0], issues[3], Equal}, // 10 vs 10
67+
{issues[3], issues[2], Less}, // 10 vs 12
68+
{issues[1], issues[1], Equal}, // 11 vs 11
69+
{issues[1], issues[0], Greater}, // 11 vs 10
70+
{issues[1], issues[2], Less}, // 11 vs 12
71+
{issues[2], issues[3], Greater}, // 12 vs 10
72+
{issues[2], issues[1], Greater}, // 12 vs 11
73+
{issues[2], issues[2], Equal}, // 12 vs 12
74+
})
75+
}
76+
77+
func TestCompareByName(t *testing.T) { //nolint:dupl
78+
testCompareValues(t, ByName{}, "Compare By Name", []compareTestCase{
79+
{issues[0], issues[1], Greater}, // file_windows.go vs file_linux.go
80+
{issues[1], issues[2], Greater}, // file_linux.go vs file_darwin.go
81+
{issues[2], issues[3], Equal}, // file_darwin.go vs file_darwin.go
82+
{issues[1], issues[1], Equal}, // file_linux.go vs file_linux.go
83+
{issues[1], issues[0], Less}, // file_linux.go vs file_windows.go
84+
{issues[3], issues[2], Equal}, // file_darwin.go vs file_darwin.go
85+
{issues[2], issues[1], Less}, // file_darwin.go vs file_linux.go
86+
{issues[0], issues[0], Equal}, // file_windows.go vs file_windows.go
87+
{issues[2], issues[2], Equal}, // file_darwin.go vs file_darwin.go
88+
{issues[3], issues[3], Equal}, // file_darwin.go vs file_darwin.go
89+
})
90+
}
91+
92+
func TestCompareByColumn(t *testing.T) { //nolint:dupl
93+
testCompareValues(t, ByColumn{}, "Compare By Column", []compareTestCase{
94+
{issues[0], issues[1], Greater}, // 80 vs 70
95+
{issues[1], issues[2], None}, // 70 vs zero value
96+
{issues[3], issues[3], Equal}, // 60 vs 60
97+
{issues[2], issues[3], None}, // zero value vs 60
98+
{issues[2], issues[1], None}, // zero value vs 70
99+
{issues[1], issues[0], Less}, // 70 vs 80
100+
{issues[1], issues[1], Equal}, // 70 vs 70
101+
{issues[3], issues[2], None}, // vs zero value
102+
{issues[2], issues[2], Equal}, // zero value vs zero value
103+
{issues[1], issues[1], Equal}, // 70 vs 70
104+
})
105+
}
106+
107+
func TestCompareNested(t *testing.T) {
108+
var cmp = ByName{
109+
next: ByLine{
110+
next: ByColumn{},
111+
},
112+
}
113+
114+
testCompareValues(t, cmp, "Nested Comparing", []compareTestCase{
115+
{issues[1], issues[0], Less}, // file_linux.go vs file_windows.go
116+
{issues[2], issues[1], Less}, // file_darwin.go vs file_linux.go
117+
{issues[0], issues[1], Greater}, // file_windows.go vs file_linux.go
118+
{issues[1], issues[2], Greater}, // file_linux.go vs file_darwin.go
119+
{issues[3], issues[2], Less}, // file_darwin.go vs file_darwin.go, 10 vs 12
120+
{issues[0], issues[0], Equal}, // file_windows.go vs file_windows.go
121+
{issues[2], issues[3], Greater}, // file_darwin.go vs file_darwin.go, 12 vs 10
122+
{issues[1], issues[1], Equal}, // file_linux.go vs file_linux.go
123+
{issues[2], issues[2], Equal}, // file_darwin.go vs file_darwin.go
124+
{issues[3], issues[3], Equal}, // file_darwin.go vs file_darwin.go
125+
})
126+
}
127+
128+
func TestNumericCompare(t *testing.T) {
129+
var tests = []struct {
130+
a, b int
131+
expected compareResult
132+
}{
133+
{0, 0, Equal},
134+
{0, 1, None},
135+
{1, 0, None},
136+
{1, -1, None},
137+
{-1, 1, None},
138+
{1, 1, Equal},
139+
{1, 2, Less},
140+
{2, 1, Greater},
141+
}
142+
143+
t.Parallel()
144+
145+
for i := 0; i < len(tests); i++ {
146+
test := tests[i]
147+
t.Run(fmt.Sprintf("%s(%d)", "Numeric Compare", i), func(t *testing.T) {
148+
res := numericCompare(test.a, test.b)
149+
assert.Equal(t, test.expected.String(), res.String())
150+
})
151+
}
152+
}
153+
154+
func TestNoSorting(t *testing.T) {
155+
var tests = make([]result.Issue, len(issues))
156+
copy(tests, issues)
157+
158+
var sr = NewSortResults(&config.Config{})
159+
160+
results, err := sr.Process(tests)
161+
assert.Equal(t, tests, results)
162+
assert.Nil(t, err, nil)
163+
}
164+
165+
func TestSorting(t *testing.T) {
166+
var tests = make([]result.Issue, len(issues))
167+
copy(tests, issues)
168+
169+
var expected = make([]result.Issue, len(issues))
170+
expected[0] = issues[3]
171+
expected[1] = issues[2]
172+
expected[2] = issues[1]
173+
expected[3] = issues[0]
174+
175+
var cfg = config.Config{}
176+
cfg.Output.SortResults = true
177+
var sr = NewSortResults(&cfg)
178+
179+
results, err := sr.Process(tests)
180+
assert.Equal(t, results, expected)
181+
assert.Nil(t, err, nil)
182+
}

0 commit comments

Comments
 (0)