Skip to content

Commit 6963aef

Browse files
ldezldez
committed
chore: simplify comparators
1 parent 78d2118 commit 6963aef

File tree

2 files changed

+66
-135
lines changed

2 files changed

+66
-135
lines changed

‎pkg/result/processors/sort_results.go

Lines changed: 50 additions & 115 deletions
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,11 @@ func NewSortResults(cfg *config.Config) *SortResults {
3535
cmps: map[string][]comparator{
3636
// For sorting we are comparing (in next order):
3737
// file names, line numbers, position, and finally - giving up.
38-
orderNameFile: {&byFileName{}, &byLine{}, &byColumn{}},
38+
orderNameFile: {byFileName(), byLine(), byColumn()},
3939
// For sorting we are comparing: linter name
40-
orderNameLinter: {&byLinter{}},
40+
orderNameLinter: {byLinter()},
4141
// For sorting we are comparing: severity
42-
orderNameSeverity: {&bySeverity{}},
42+
orderNameSeverity: {bySeverity()},
4343
},
4444
cfg: &cfg.Output,
4545
}
@@ -115,77 +115,32 @@ type comparator interface {
115115
fmt.Stringer
116116
}
117117

118-
var (
119-
_ comparator = (*byFileName)(nil)
120-
_ comparator = (*byLine)(nil)
121-
_ comparator = (*byColumn)(nil)
122-
_ comparator = (*byLinter)(nil)
123-
_ comparator = (*bySeverity)(nil)
124-
)
125-
126-
type byFileName struct{ next comparator }
127-
128-
func (cmp *byFileName) Next() comparator { return cmp.next }
118+
var _ comparator = (*baseComparator)(nil)
129119

130-
func (cmp *byFileName) AddNext(c comparator) comparator {
131-
cmp.next = c
132-
return cmp
120+
type baseComparator struct {
121+
name string
122+
compare func(a, b *result.Issue) compareResult
123+
next comparator
133124
}
134125

135-
func (cmp *byFileName) Compare(a, b *result.Issue) compareResult {
136-
res := compareResult(strings.Compare(a.FilePath(), b.FilePath()))
137-
if !res.isNeutral() {
138-
return res
139-
}
140-
141-
if next := cmp.Next(); next != nil {
142-
return next.Compare(a, b)
143-
}
144-
145-
return res
146-
}
147-
148-
func (cmp *byFileName) String() string {
149-
return comparatorToString("byFileName", cmp)
150-
}
151-
152-
type byLine struct{ next comparator }
153-
154-
func (cmp *byLine) Next() comparator { return cmp.next }
126+
func (cmp *baseComparator) Next() comparator { return cmp.next }
155127

156-
func (cmp *byLine) AddNext(c comparator) comparator {
128+
func (cmp *baseComparator) AddNext(c comparator) comparator {
157129
cmp.next = c
158130
return cmp
159131
}
160132

161-
func (cmp *byLine) Compare(a, b *result.Issue) compareResult {
162-
res := numericCompare(a.Line(), b.Line())
163-
if !res.isNeutral() {
164-
return res
165-
}
166-
167-
if next := cmp.Next(); next != nil {
168-
return next.Compare(a, b)
133+
func (cmp *baseComparator) String() string {
134+
s := cmp.name
135+
if cmp.Next() != nil {
136+
s += " > " + cmp.Next().String()
169137
}
170138

171-
return res
172-
}
173-
174-
func (cmp *byLine) String() string {
175-
return comparatorToString("byLine", cmp)
176-
}
177-
178-
type byColumn struct{ next comparator }
179-
180-
func (cmp *byColumn) Next() comparator { return cmp.next }
181-
182-
func (cmp *byColumn) AddNext(c comparator) comparator {
183-
cmp.next = c
184-
return cmp
139+
return s
185140
}
186141

187-
func (cmp *byColumn) Compare(a, b *result.Issue) compareResult {
188-
res := numericCompare(a.Column(), b.Column())
142+
func (cmp *baseComparator) Compare(a, b *result.Issue) compareResult {
143+
res := cmp.compare(a, b)
189144
if !res.isNeutral() {
190145
return res
191146
}
@@ -197,60 +152,49 @@ func (cmp *byColumn) Compare(a, b *result.Issue) compareResult {
197152
return res
198153
}
199154

200-
func (cmp *byColumn) String() string {
201-
return comparatorToString("byColumn", cmp)
202-
}
203-
204-
type byLinter struct{ next comparator }
205-
206-
func (cmp *byLinter) Next() comparator { return cmp.next }
207-
208-
func (cmp *byLinter) AddNext(c comparator) comparator {
209-
cmp.next = c
210-
return cmp
211-
}
212-
213-
func (cmp *byLinter) Compare(a, b *result.Issue) compareResult {
214-
res := compareResult(strings.Compare(a.FromLinter, b.FromLinter))
215-
if !res.isNeutral() {
216-
return res
217-
}
218-
219-
if next := cmp.Next(); next != nil {
220-
return next.Compare(a, b)
155+
func byFileName() *baseComparator {
156+
return &baseComparator{
157+
name: "byFileName",
158+
compare: func(a, b *result.Issue) compareResult {
159+
return compareResult(strings.Compare(a.FilePath(), b.FilePath()))
160+
},
221161
}
222-
223-
return res
224162
}
225163

226-
func (cmp *byLinter) String() string {
227-
return comparatorToString("byLinter", cmp)
228-
}
229-
230-
type bySeverity struct{ next comparator }
231-
232-
func (cmp *bySeverity) Next() comparator { return cmp.next }
233-
234-
func (cmp *bySeverity) AddNext(c comparator) comparator {
235-
cmp.next = c
236-
return cmp
164+
func byLine() *baseComparator {
165+
return &baseComparator{
166+
name: "byLine",
167+
compare: func(a, b *result.Issue) compareResult {
168+
return numericCompare(a.Line(), b.Line())
169+
},
170+
}
237171
}
238172

239-
func (cmp *bySeverity) Compare(a, b *result.Issue) compareResult {
240-
res := severityCompare(a.Severity, b.Severity)
241-
if !res.isNeutral() {
242-
return res
173+
func byColumn() *baseComparator {
174+
return &baseComparator{
175+
name: "byColumn",
176+
compare: func(a, b *result.Issue) compareResult {
177+
return numericCompare(a.Column(), b.Column())
178+
},
243179
}
180+
}
244181

245-
if next := cmp.Next(); next != nil {
246-
return next.Compare(a, b)
182+
func byLinter() *baseComparator {
183+
return &baseComparator{
184+
name: "byLinter",
185+
compare: func(a, b *result.Issue) compareResult {
186+
return compareResult(strings.Compare(a.FromLinter, b.FromLinter))
187+
},
247188
}
248-
249-
return res
250189
}
251190

252-
func (cmp *bySeverity) String() string {
253-
return comparatorToString("bySeverity", cmp)
191+
func bySeverity() *baseComparator {
192+
return &baseComparator{
193+
name: "bySeverity",
194+
compare: func(a, b *result.Issue) compareResult {
195+
return severityCompare(a.Severity, b.Severity)
196+
},
197+
}
254198
}
255199

256200
func mergeComparators(cmps []comparator) (comparator, error) {
@@ -313,12 +257,3 @@ func numericCompare(a, b int) compareResult {
313257

314258
return equal
315259
}
316-
317-
func comparatorToString(name string, c comparator) string {
318-
s := name
319-
if c.Next() != nil {
320-
s += " > " + c.Next().String()
321-
}
322-
323-
return s
324-
}

‎pkg/result/processors/sort_results_test.go

Lines changed: 16 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ func testCompareValues(t *testing.T, cmp comparator, name string, tests []compar
8989
}
9090

9191
func TestCompareByLine(t *testing.T) {
92-
testCompareValues(t, &byLine{}, "Compare By Line", []compareTestCase{
92+
testCompareValues(t, byLine(), "Compare By Line", []compareTestCase{
9393
{issues[0], issues[1], less}, // 10 vs 11
9494
{issues[0], issues[0], equal}, // 10 vs 10
9595
{issues[3], issues[3], equal}, // 10 vs 10
@@ -105,7 +105,7 @@ func TestCompareByLine(t *testing.T) {
105105
}
106106

107107
func TestCompareByFileName(t *testing.T) { //nolint:dupl
108-
testCompareValues(t, &byFileName{}, "Compare By File Name", []compareTestCase{
108+
testCompareValues(t, byFileName(), "Compare By File Name", []compareTestCase{
109109
{issues[0], issues[1], greater}, // file_windows.go vs file_linux.go
110110
{issues[1], issues[2], greater}, // file_linux.go vs file_darwin.go
111111
{issues[2], issues[3], equal}, // file_darwin.go vs file_darwin.go
@@ -120,7 +120,7 @@ func TestCompareByFileName(t *testing.T) { //nolint:dupl
120120
}
121121

122122
func TestCompareByColumn(t *testing.T) { //nolint:dupl
123-
testCompareValues(t, &byColumn{}, "Compare By Column", []compareTestCase{
123+
testCompareValues(t, byColumn(), "Compare By Column", []compareTestCase{
124124
{issues[0], issues[1], greater}, // 80 vs 70
125125
{issues[1], issues[2], none}, // 70 vs zero value
126126
{issues[3], issues[3], equal}, // 60 vs 60
@@ -135,7 +135,7 @@ func TestCompareByColumn(t *testing.T) { //nolint:dupl
135135
}
136136

137137
func TestCompareByLinter(t *testing.T) { //nolint:dupl
138-
testCompareValues(t, &byLinter{}, "Compare By Linter", []compareTestCase{
138+
testCompareValues(t, byLinter(), "Compare By Linter", []compareTestCase{
139139
{issues[0], issues[1], greater}, // b vs a
140140
{issues[1], issues[2], less}, // a vs c
141141
{issues[2], issues[3], equal}, // c vs c
@@ -150,7 +150,7 @@ func TestCompareByLinter(t *testing.T) { //nolint:dupl
150150
}
151151

152152
func TestCompareBySeverity(t *testing.T) {
153-
testCompareValues(t, &bySeverity{}, "Compare By Severity", []compareTestCase{
153+
testCompareValues(t, bySeverity(), "Compare By Severity", []compareTestCase{
154154
{issues[0], issues[1], greater}, // medium vs low
155155
{issues[1], issues[2], less}, // low vs high
156156
{issues[2], issues[3], equal}, // high vs high
@@ -167,11 +167,7 @@ func TestCompareBySeverity(t *testing.T) {
167167
}
168168

169169
func TestCompareNested(t *testing.T) {
170-
var cmp = &byFileName{
171-
next: &byLine{
172-
next: &byColumn{},
173-
},
174-
}
170+
cmp := byFileName().AddNext(byLine().AddNext(byColumn()))
175171

176172
testCompareValues(t, cmp, "Nested Comparing", []compareTestCase{
177173
{issues[1], issues[0], less}, // file_linux.go vs file_windows.go
@@ -188,7 +184,7 @@ func TestCompareNested(t *testing.T) {
188184
}
189185

190186
func TestNumericCompare(t *testing.T) {
191-
var tests = []struct {
187+
tests := []struct {
192188
a, b int
193189
expected compareResult
194190
}{
@@ -214,23 +210,23 @@ func TestNumericCompare(t *testing.T) {
214210
}
215211

216212
func TestNoSorting(t *testing.T) {
217-
var tests = make([]result.Issue, len(issues))
213+
tests := make([]result.Issue, len(issues))
218214
copy(tests, issues)
219215

220-
var sr = NewSortResults(&config.Config{})
216+
sr := NewSortResults(&config.Config{})
221217

222218
results, err := sr.Process(tests)
223219
require.NoError(t, err)
224220
assert.Equal(t, tests, results)
225221
}
226222

227223
func TestSorting(t *testing.T) {
228-
var tests = make([]result.Issue, len(issues))
224+
tests := make([]result.Issue, len(issues))
229225
copy(tests, issues)
230226

231-
var cfg = config.Config{}
227+
cfg := config.Config{}
232228
cfg.Output.SortResults = true
233-
var sr = NewSortResults(&cfg)
229+
sr := NewSortResults(&cfg)
234230

235231
results, err := sr.Process(tests)
236232
require.NoError(t, err)
@@ -245,22 +241,22 @@ func Test_mergeComparators(t *testing.T) {
245241
}{
246242
{
247243
desc: "one",
248-
cmps: []comparator{&byLinter{}},
244+
cmps: []comparator{byLinter()},
249245
expected: "byLinter",
250246
},
251247
{
252248
desc: "two",
253-
cmps: []comparator{&byLinter{}, &byFileName{}},
249+
cmps: []comparator{byLinter(), byFileName()},
254250
expected: "byLinter > byFileName",
255251
},
256252
{
257253
desc: "all",
258-
cmps: []comparator{&bySeverity{}, &byLinter{}, &byFileName{}, &byLine{}, &byColumn{}},
254+
cmps: []comparator{bySeverity(), byLinter(), byFileName(), byLine(), byColumn()},
259255
expected: "bySeverity > byLinter > byFileName > byLine > byColumn",
260256
},
261257
{
262258
desc: "all reverse",
263-
cmps: []comparator{&byColumn{}, &byLine{}, &byFileName{}, &byLinter{}, &bySeverity{}},
259+
cmps: []comparator{byColumn(), byLine(), byFileName(), byLinter(), bySeverity()},
264260
expected: "byColumn > byLine > byFileName > byLinter > bySeverity",
265261
},
266262
}

0 commit comments

Comments
 (0)