Skip to content

Commit 192363e

Browse files
committed
chore: late review
1 parent 2a1afc1 commit 192363e

File tree

4 files changed

+177
-122
lines changed

4 files changed

+177
-122
lines changed

Diff for: pkg/config/output.go

+10-1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"errors"
55
"fmt"
66
"slices"
7+
"strings"
78
)
89

910
const (
@@ -49,8 +50,16 @@ func (o *Output) Validate() error {
4950
return errors.New("sort-results should be 'true' to use sort-order")
5051
}
5152

53+
validOrders := []string{"linter", "file", "severity"}
54+
55+
all := strings.Join(o.SortOrder, " ")
56+
5257
for _, order := range o.SortOrder {
53-
if !slices.Contains([]string{"linter", "file", "severity"}, order) {
58+
if strings.Count(all, order) > 1 {
59+
return fmt.Errorf("the sort-order name %q is repeated several times", order)
60+
}
61+
62+
if !slices.Contains(validOrders, order) {
5463
return fmt.Errorf("unsupported sort-order name %q", order)
5564
}
5665
}

Diff for: pkg/config/output_test.go

+15
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,13 @@ func TestOutput_Validate(t *testing.T) {
3232
SortOrder: []string{"severity"},
3333
},
3434
},
35+
{
36+
desc: "multiple",
37+
settings: &Output{
38+
SortResults: true,
39+
SortOrder: []string{"file", "linter", "severity"},
40+
},
41+
},
3542
}
3643

3744
for _, test := range testCases {
@@ -66,6 +73,14 @@ func TestOutput_Validate_error(t *testing.T) {
6673
},
6774
expected: `unsupported sort-order name "a"`,
6875
},
76+
{
77+
desc: "duplicate",
78+
settings: &Output{
79+
SortResults: true,
80+
SortOrder: []string{"file", "linter", "severity", "linter"},
81+
},
82+
expected: `the sort-order name "linter" is repeated several times`,
83+
},
6984
}
7085

7186
for _, test := range testCases {

Diff for: pkg/result/processors/sort_results.go

+46-38
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,9 @@ import (
1717
// rules that can compare different properties of the Issues struct.
1818

1919
const (
20-
fileOrderName = "file"
21-
linterOrderName = "linter"
22-
linterSeverityName = "severity"
20+
orderNameFile = "file"
21+
orderNameLinter = "linter"
22+
orderNameSeverity = "severity"
2323
)
2424

2525
var _ Processor = (*SortResults)(nil)
@@ -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-
fileOrderName: {&byName{}, &byLine{}, &byColumn{}},
38+
orderNameFile: {&byFileName{}, &byLine{}, &byColumn{}},
3939
// For sorting we are comparing: linter name
40-
linterOrderName: {&byLinter{}},
40+
orderNameLinter: {&byLinter{}},
4141
// For sorting we are comparing: severity
42-
linterSeverityName: {&bySeverity{}},
42+
orderNameSeverity: {&bySeverity{}},
4343
},
4444
cfg: &cfg.Output,
4545
}
@@ -52,7 +52,7 @@ func (sr SortResults) Process(issues []result.Issue) ([]result.Issue, error) {
5252
}
5353

5454
if len(sr.cfg.SortOrder) == 0 {
55-
sr.cfg.SortOrder = []string{fileOrderName}
55+
sr.cfg.SortOrder = []string{orderNameFile}
5656
}
5757

5858
var cmps []comparator
@@ -64,13 +64,13 @@ func (sr SortResults) Process(issues []result.Issue) ([]result.Issue, error) {
6464
}
6565
}
6666

67-
cmp, err := mergeComparator(cmps)
67+
cmp, err := mergeComparators(cmps)
6868
if err != nil {
6969
return nil, err
7070
}
7171

7272
sort.Slice(issues, func(i, j int) bool {
73-
return cmp.Compare(&issues[i], &issues[j]) == Less
73+
return cmp.Compare(&issues[i], &issues[j]) == less
7474
})
7575

7676
return issues, nil
@@ -83,31 +83,31 @@ func (sr SortResults) Finish() {}
8383
type compareResult int
8484

8585
const (
86-
Less compareResult = iota - 1
87-
Equal
88-
Greater
89-
None
86+
less compareResult = iota - 1
87+
equal
88+
greater
89+
none
9090
)
9191

9292
func (c compareResult) isNeutral() bool {
9393
// return true if compare result is incomparable or equal.
94-
return c == None || c == Equal
94+
return c == none || c == equal
9595
}
9696

9797
func (c compareResult) String() string {
9898
switch c {
99-
case Less:
100-
return "Less"
101-
case Equal:
102-
return "Equal"
103-
case Greater:
104-
return "Greater"
99+
case less:
100+
return "less"
101+
case equal:
102+
return "equal"
103+
case greater:
104+
return "greater"
105105
default:
106-
return "None"
106+
return "none"
107107
}
108108
}
109109

110-
// comparator describe how to implement compare for two "issues" lexicographically
110+
// comparator describes how to implement compare for two "issues".
111111
type comparator interface {
112112
Compare(a, b *result.Issue) compareResult
113113
Next() comparator
@@ -116,23 +116,23 @@ type comparator interface {
116116
}
117117

118118
var (
119-
_ comparator = (*byName)(nil)
119+
_ comparator = (*byFileName)(nil)
120120
_ comparator = (*byLine)(nil)
121121
_ comparator = (*byColumn)(nil)
122122
_ comparator = (*byLinter)(nil)
123123
_ comparator = (*bySeverity)(nil)
124124
)
125125

126-
type byName struct{ next comparator }
126+
type byFileName struct{ next comparator }
127127

128-
func (cmp *byName) Next() comparator { return cmp.next }
128+
func (cmp *byFileName) Next() comparator { return cmp.next }
129129

130-
func (cmp *byName) AddNext(c comparator) comparator {
130+
func (cmp *byFileName) AddNext(c comparator) comparator {
131131
cmp.next = c
132132
return cmp
133133
}
134134

135-
func (cmp *byName) Compare(a, b *result.Issue) compareResult {
135+
func (cmp *byFileName) Compare(a, b *result.Issue) compareResult {
136136
var res compareResult
137137

138138
if res = compareResult(strings.Compare(a.FilePath(), b.FilePath())); !res.isNeutral() {
@@ -146,8 +146,8 @@ func (cmp *byName) Compare(a, b *result.Issue) compareResult {
146146
return res
147147
}
148148

149-
func (cmp *byName) String() string {
150-
return comparatorToString("byName", cmp)
149+
func (cmp *byFileName) String() string {
150+
return comparatorToString("byFileName", cmp)
151151
}
152152

153153
type byLine struct{ next comparator }
@@ -258,7 +258,7 @@ func (cmp *bySeverity) String() string {
258258
return comparatorToString("bySeverity", cmp)
259259
}
260260

261-
func mergeComparator(cmps []comparator) (comparator, error) {
261+
func mergeComparators(cmps []comparator) (comparator, error) {
262262
if len(cmps) == 0 {
263263
return nil, errors.New("no comparator")
264264
}
@@ -277,14 +277,22 @@ func severityCompare(a, b string) compareResult {
277277
if slices.Contains(classic, a) && slices.Contains(classic, b) {
278278
switch {
279279
case slices.Index(classic, a) > slices.Index(classic, b):
280-
return Greater
280+
return greater
281281
case slices.Index(classic, a) < slices.Index(classic, b):
282-
return Less
282+
return less
283283
default:
284-
return Equal
284+
return equal
285285
}
286286
}
287287

288+
if slices.Contains(classic, a) {
289+
return greater
290+
}
291+
292+
if slices.Contains(classic, b) {
293+
return less
294+
}
295+
288296
return compareResult(strings.Compare(a, b))
289297
}
290298

@@ -299,16 +307,16 @@ func numericCompare(a, b int) compareResult {
299307

300308
switch {
301309
case isZeroValuesBoth || isEqual:
302-
return Equal
310+
return equal
303311
case isValuesInvalid || isZeroValueInA || isZeroValueInB:
304-
return None
312+
return none
305313
case a > b:
306-
return Greater
314+
return greater
307315
case a < b:
308-
return Less
316+
return less
309317
}
310318

311-
return Equal
319+
return equal
312320
}
313321

314322
func comparatorToString(name string, c comparator) string {

0 commit comments

Comments
 (0)