dev: clean up (#4495)

This commit is contained in:
Ludovic Fernandez 2024-03-13 15:49:29 +01:00 committed by GitHub
parent 236e0e5cb8
commit 78d2118c0b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 187 additions and 137 deletions

View File

@ -4,6 +4,7 @@ import (
"errors"
"fmt"
"slices"
"strings"
)
const (
@ -49,8 +50,16 @@ func (o *Output) Validate() error {
return errors.New("sort-results should be 'true' to use sort-order")
}
validOrders := []string{"linter", "file", "severity"}
all := strings.Join(o.SortOrder, " ")
for _, order := range o.SortOrder {
if !slices.Contains([]string{"linter", "file", "severity"}, order) {
if strings.Count(all, order) > 1 {
return fmt.Errorf("the sort-order name %q is repeated several times", order)
}
if !slices.Contains(validOrders, order) {
return fmt.Errorf("unsupported sort-order name %q", order)
}
}

View File

@ -32,6 +32,13 @@ func TestOutput_Validate(t *testing.T) {
SortOrder: []string{"severity"},
},
},
{
desc: "multiple",
settings: &Output{
SortResults: true,
SortOrder: []string{"file", "linter", "severity"},
},
},
}
for _, test := range testCases {
@ -66,6 +73,14 @@ func TestOutput_Validate_error(t *testing.T) {
},
expected: `unsupported sort-order name "a"`,
},
{
desc: "duplicate",
settings: &Output{
SortResults: true,
SortOrder: []string{"file", "linter", "severity", "linter"},
},
expected: `the sort-order name "linter" is repeated several times`,
},
}
for _, test := range testCases {

View File

@ -17,9 +17,9 @@ import (
// rules that can compare different properties of the Issues struct.
const (
fileOrderName = "file"
linterOrderName = "linter"
linterSeverityName = "severity"
orderNameFile = "file"
orderNameLinter = "linter"
orderNameSeverity = "severity"
)
var _ Processor = (*SortResults)(nil)
@ -35,11 +35,11 @@ func NewSortResults(cfg *config.Config) *SortResults {
cmps: map[string][]comparator{
// For sorting we are comparing (in next order):
// file names, line numbers, position, and finally - giving up.
fileOrderName: {&byName{}, &byLine{}, &byColumn{}},
orderNameFile: {&byFileName{}, &byLine{}, &byColumn{}},
// For sorting we are comparing: linter name
linterOrderName: {&byLinter{}},
orderNameLinter: {&byLinter{}},
// For sorting we are comparing: severity
linterSeverityName: {&bySeverity{}},
orderNameSeverity: {&bySeverity{}},
},
cfg: &cfg.Output,
}
@ -52,7 +52,7 @@ func (sr SortResults) Process(issues []result.Issue) ([]result.Issue, error) {
}
if len(sr.cfg.SortOrder) == 0 {
sr.cfg.SortOrder = []string{fileOrderName}
sr.cfg.SortOrder = []string{orderNameFile}
}
var cmps []comparator
@ -64,13 +64,13 @@ func (sr SortResults) Process(issues []result.Issue) ([]result.Issue, error) {
}
}
cmp, err := mergeComparator(cmps)
cmp, err := mergeComparators(cmps)
if err != nil {
return nil, err
}
sort.Slice(issues, func(i, j int) bool {
return cmp.Compare(&issues[i], &issues[j]) == Less
return cmp.Compare(&issues[i], &issues[j]) == less
})
return issues, nil
@ -83,31 +83,31 @@ func (sr SortResults) Finish() {}
type compareResult int
const (
Less compareResult = iota - 1
Equal
Greater
None
less compareResult = iota - 1
equal
greater
none
)
func (c compareResult) isNeutral() bool {
// return true if compare result is incomparable or equal.
return c == None || c == Equal
return c == none || c == equal
}
func (c compareResult) String() string {
switch c {
case Less:
return "Less"
case Equal:
return "Equal"
case Greater:
return "Greater"
case less:
return "less"
case equal:
return "equal"
case greater:
return "greater"
default:
return "None"
return "none"
}
}
// comparator describe how to implement compare for two "issues" lexicographically
// comparator describes how to implement compare for two "issues".
type comparator interface {
Compare(a, b *result.Issue) compareResult
Next() comparator
@ -116,26 +116,25 @@ type comparator interface {
}
var (
_ comparator = (*byName)(nil)
_ comparator = (*byFileName)(nil)
_ comparator = (*byLine)(nil)
_ comparator = (*byColumn)(nil)
_ comparator = (*byLinter)(nil)
_ comparator = (*bySeverity)(nil)
)
type byName struct{ next comparator }
type byFileName struct{ next comparator }
func (cmp *byName) Next() comparator { return cmp.next }
func (cmp *byFileName) Next() comparator { return cmp.next }
func (cmp *byName) AddNext(c comparator) comparator {
func (cmp *byFileName) AddNext(c comparator) comparator {
cmp.next = c
return cmp
}
func (cmp *byName) Compare(a, b *result.Issue) compareResult {
var res compareResult
if res = compareResult(strings.Compare(a.FilePath(), b.FilePath())); !res.isNeutral() {
func (cmp *byFileName) Compare(a, b *result.Issue) compareResult {
res := compareResult(strings.Compare(a.FilePath(), b.FilePath()))
if !res.isNeutral() {
return res
}
@ -146,8 +145,8 @@ func (cmp *byName) Compare(a, b *result.Issue) compareResult {
return res
}
func (cmp *byName) String() string {
return comparatorToString("byName", cmp)
func (cmp *byFileName) String() string {
return comparatorToString("byFileName", cmp)
}
type byLine struct{ next comparator }
@ -160,9 +159,8 @@ func (cmp *byLine) AddNext(c comparator) comparator {
}
func (cmp *byLine) Compare(a, b *result.Issue) compareResult {
var res compareResult
if res = numericCompare(a.Line(), b.Line()); !res.isNeutral() {
res := numericCompare(a.Line(), b.Line())
if !res.isNeutral() {
return res
}
@ -187,9 +185,8 @@ func (cmp *byColumn) AddNext(c comparator) comparator {
}
func (cmp *byColumn) Compare(a, b *result.Issue) compareResult {
var res compareResult
if res = numericCompare(a.Column(), b.Column()); !res.isNeutral() {
res := numericCompare(a.Column(), b.Column())
if !res.isNeutral() {
return res
}
@ -214,9 +211,8 @@ func (cmp *byLinter) AddNext(c comparator) comparator {
}
func (cmp *byLinter) Compare(a, b *result.Issue) compareResult {
var res compareResult
if res = compareResult(strings.Compare(a.FromLinter, b.FromLinter)); !res.isNeutral() {
res := compareResult(strings.Compare(a.FromLinter, b.FromLinter))
if !res.isNeutral() {
return res
}
@ -241,9 +237,8 @@ func (cmp *bySeverity) AddNext(c comparator) comparator {
}
func (cmp *bySeverity) Compare(a, b *result.Issue) compareResult {
var res compareResult
if res = severityCompare(a.Severity, b.Severity); !res.isNeutral() {
res := severityCompare(a.Severity, b.Severity)
if !res.isNeutral() {
return res
}
@ -258,7 +253,7 @@ func (cmp *bySeverity) String() string {
return comparatorToString("bySeverity", cmp)
}
func mergeComparator(cmps []comparator) (comparator, error) {
func mergeComparators(cmps []comparator) (comparator, error) {
if len(cmps) == 0 {
return nil, errors.New("no comparator")
}
@ -277,14 +272,22 @@ func severityCompare(a, b string) compareResult {
if slices.Contains(classic, a) && slices.Contains(classic, b) {
switch {
case slices.Index(classic, a) > slices.Index(classic, b):
return Greater
return greater
case slices.Index(classic, a) < slices.Index(classic, b):
return Less
return less
default:
return Equal
return equal
}
}
if slices.Contains(classic, a) {
return greater
}
if slices.Contains(classic, b) {
return less
}
return compareResult(strings.Compare(a, b))
}
@ -299,16 +302,16 @@ func numericCompare(a, b int) compareResult {
switch {
case isZeroValuesBoth || isEqual:
return Equal
return equal
case isValuesInvalid || isZeroValueInA || isZeroValueInB:
return None
return none
case a > b:
return Greater
return greater
case a < b:
return Less
return less
}
return Equal
return equal
}
func comparatorToString(name string, c comparator) string {

View File

@ -50,6 +50,27 @@ var issues = []result.Issue{
},
}
var extraSeverityIssues = []result.Issue{
{
FromLinter: "c",
Severity: "error",
Pos: token.Position{
Filename: "file_darwin.go",
Column: 60,
Line: 10,
},
},
{
FromLinter: "c",
Severity: "aaaa",
Pos: token.Position{
Filename: "file_darwin.go",
Column: 60,
Line: 10,
},
},
}
type compareTestCase struct {
a, b result.Issue
expected compareResult
@ -69,98 +90,100 @@ func testCompareValues(t *testing.T, cmp comparator, name string, tests []compar
func TestCompareByLine(t *testing.T) {
testCompareValues(t, &byLine{}, "Compare By Line", []compareTestCase{
{issues[0], issues[1], Less}, // 10 vs 11
{issues[0], issues[0], Equal}, // 10 vs 10
{issues[3], issues[3], Equal}, // 10 vs 10
{issues[0], issues[3], Equal}, // 10 vs 10
{issues[3], issues[2], Less}, // 10 vs 12
{issues[1], issues[1], Equal}, // 11 vs 11
{issues[1], issues[0], Greater}, // 11 vs 10
{issues[1], issues[2], Less}, // 11 vs 12
{issues[2], issues[3], Greater}, // 12 vs 10
{issues[2], issues[1], Greater}, // 12 vs 11
{issues[2], issues[2], Equal}, // 12 vs 12
{issues[0], issues[1], less}, // 10 vs 11
{issues[0], issues[0], equal}, // 10 vs 10
{issues[3], issues[3], equal}, // 10 vs 10
{issues[0], issues[3], equal}, // 10 vs 10
{issues[3], issues[2], less}, // 10 vs 12
{issues[1], issues[1], equal}, // 11 vs 11
{issues[1], issues[0], greater}, // 11 vs 10
{issues[1], issues[2], less}, // 11 vs 12
{issues[2], issues[3], greater}, // 12 vs 10
{issues[2], issues[1], greater}, // 12 vs 11
{issues[2], issues[2], equal}, // 12 vs 12
})
}
func TestCompareByName(t *testing.T) { //nolint:dupl
testCompareValues(t, &byName{}, "Compare By Name", []compareTestCase{
{issues[0], issues[1], Greater}, // file_windows.go vs file_linux.go
{issues[1], issues[2], Greater}, // file_linux.go vs file_darwin.go
{issues[2], issues[3], Equal}, // file_darwin.go vs file_darwin.go
{issues[1], issues[1], Equal}, // file_linux.go vs file_linux.go
{issues[1], issues[0], Less}, // file_linux.go vs file_windows.go
{issues[3], issues[2], Equal}, // file_darwin.go vs file_darwin.go
{issues[2], issues[1], Less}, // file_darwin.go vs file_linux.go
{issues[0], issues[0], Equal}, // file_windows.go vs file_windows.go
{issues[2], issues[2], Equal}, // file_darwin.go vs file_darwin.go
{issues[3], issues[3], Equal}, // file_darwin.go vs file_darwin.go
func TestCompareByFileName(t *testing.T) { //nolint:dupl
testCompareValues(t, &byFileName{}, "Compare By File Name", []compareTestCase{
{issues[0], issues[1], greater}, // file_windows.go vs file_linux.go
{issues[1], issues[2], greater}, // file_linux.go vs file_darwin.go
{issues[2], issues[3], equal}, // file_darwin.go vs file_darwin.go
{issues[1], issues[1], equal}, // file_linux.go vs file_linux.go
{issues[1], issues[0], less}, // file_linux.go vs file_windows.go
{issues[3], issues[2], equal}, // file_darwin.go vs file_darwin.go
{issues[2], issues[1], less}, // file_darwin.go vs file_linux.go
{issues[0], issues[0], equal}, // file_windows.go vs file_windows.go
{issues[2], issues[2], equal}, // file_darwin.go vs file_darwin.go
{issues[3], issues[3], equal}, // file_darwin.go vs file_darwin.go
})
}
func TestCompareByColumn(t *testing.T) { //nolint:dupl
testCompareValues(t, &byColumn{}, "Compare By Column", []compareTestCase{
{issues[0], issues[1], Greater}, // 80 vs 70
{issues[1], issues[2], None}, // 70 vs zero value
{issues[3], issues[3], Equal}, // 60 vs 60
{issues[2], issues[3], None}, // zero value vs 60
{issues[2], issues[1], None}, // zero value vs 70
{issues[1], issues[0], Less}, // 70 vs 80
{issues[1], issues[1], Equal}, // 70 vs 70
{issues[3], issues[2], None}, // vs zero value
{issues[2], issues[2], Equal}, // zero value vs zero value
{issues[1], issues[1], Equal}, // 70 vs 70
{issues[0], issues[1], greater}, // 80 vs 70
{issues[1], issues[2], none}, // 70 vs zero value
{issues[3], issues[3], equal}, // 60 vs 60
{issues[2], issues[3], none}, // zero value vs 60
{issues[2], issues[1], none}, // zero value vs 70
{issues[1], issues[0], less}, // 70 vs 80
{issues[1], issues[1], equal}, // 70 vs 70
{issues[3], issues[2], none}, // vs zero value
{issues[2], issues[2], equal}, // zero value vs zero value
{issues[1], issues[1], equal}, // 70 vs 70
})
}
func TestCompareByLinter(t *testing.T) { //nolint:dupl
testCompareValues(t, &byLinter{}, "Compare By Linter", []compareTestCase{
{issues[0], issues[1], Greater}, // b vs a
{issues[1], issues[2], Less}, // a vs c
{issues[2], issues[3], Equal}, // c vs c
{issues[1], issues[1], Equal}, // a vs a
{issues[1], issues[0], Less}, // a vs b
{issues[3], issues[2], Equal}, // c vs c
{issues[2], issues[1], Greater}, // c vs a
{issues[0], issues[0], Equal}, // b vs b
{issues[2], issues[2], Equal}, // a vs a
{issues[3], issues[3], Equal}, // c vs c
{issues[0], issues[1], greater}, // b vs a
{issues[1], issues[2], less}, // a vs c
{issues[2], issues[3], equal}, // c vs c
{issues[1], issues[1], equal}, // a vs a
{issues[1], issues[0], less}, // a vs b
{issues[3], issues[2], equal}, // c vs c
{issues[2], issues[1], greater}, // c vs a
{issues[0], issues[0], equal}, // b vs b
{issues[2], issues[2], equal}, // a vs a
{issues[3], issues[3], equal}, // c vs c
})
}
func TestCompareBySeverity(t *testing.T) { //nolint:dupl
func TestCompareBySeverity(t *testing.T) {
testCompareValues(t, &bySeverity{}, "Compare By Severity", []compareTestCase{
{issues[0], issues[1], Greater}, // medium vs low
{issues[1], issues[2], Less}, // low vs high
{issues[2], issues[3], Equal}, // high vs high
{issues[1], issues[1], Equal}, // low vs low
{issues[1], issues[0], Less}, // low vs medium
{issues[3], issues[2], Equal}, // high vs high
{issues[2], issues[1], Greater}, // high vs low
{issues[0], issues[0], Equal}, // medium vs medium
{issues[2], issues[2], Equal}, // low vs low
{issues[3], issues[3], Equal}, // high vs high
{issues[0], issues[1], greater}, // medium vs low
{issues[1], issues[2], less}, // low vs high
{issues[2], issues[3], equal}, // high vs high
{issues[1], issues[1], equal}, // low vs low
{issues[1], issues[0], less}, // low vs medium
{issues[3], issues[2], equal}, // high vs high
{issues[2], issues[1], greater}, // high vs low
{issues[0], issues[0], equal}, // medium vs medium
{issues[2], issues[2], equal}, // low vs low
{issues[3], issues[3], equal}, // high vs high
{extraSeverityIssues[0], extraSeverityIssues[1], greater}, // classic vs unknown
{extraSeverityIssues[1], extraSeverityIssues[0], less}, // unknown vs classic
})
}
func TestCompareNested(t *testing.T) {
var cmp = &byName{
var cmp = &byFileName{
next: &byLine{
next: &byColumn{},
},
}
testCompareValues(t, cmp, "Nested Comparing", []compareTestCase{
{issues[1], issues[0], Less}, // file_linux.go vs file_windows.go
{issues[2], issues[1], Less}, // file_darwin.go vs file_linux.go
{issues[0], issues[1], Greater}, // file_windows.go vs file_linux.go
{issues[1], issues[2], Greater}, // file_linux.go vs file_darwin.go
{issues[3], issues[2], Less}, // file_darwin.go vs file_darwin.go, 10 vs 12
{issues[0], issues[0], Equal}, // file_windows.go vs file_windows.go
{issues[2], issues[3], Greater}, // file_darwin.go vs file_darwin.go, 12 vs 10
{issues[1], issues[1], Equal}, // file_linux.go vs file_linux.go
{issues[2], issues[2], Equal}, // file_darwin.go vs file_darwin.go
{issues[3], issues[3], Equal}, // file_darwin.go vs file_darwin.go
{issues[1], issues[0], less}, // file_linux.go vs file_windows.go
{issues[2], issues[1], less}, // file_darwin.go vs file_linux.go
{issues[0], issues[1], greater}, // file_windows.go vs file_linux.go
{issues[1], issues[2], greater}, // file_linux.go vs file_darwin.go
{issues[3], issues[2], less}, // file_darwin.go vs file_darwin.go, 10 vs 12
{issues[0], issues[0], equal}, // file_windows.go vs file_windows.go
{issues[2], issues[3], greater}, // file_darwin.go vs file_darwin.go, 12 vs 10
{issues[1], issues[1], equal}, // file_linux.go vs file_linux.go
{issues[2], issues[2], equal}, // file_darwin.go vs file_darwin.go
{issues[3], issues[3], equal}, // file_darwin.go vs file_darwin.go
})
}
@ -169,14 +192,14 @@ func TestNumericCompare(t *testing.T) {
a, b int
expected compareResult
}{
{0, 0, Equal},
{0, 1, None},
{1, 0, None},
{1, -1, None},
{-1, 1, None},
{1, 1, Equal},
{1, 2, Less},
{2, 1, Greater},
{0, 0, equal},
{0, 1, none},
{1, 0, none},
{1, -1, none},
{-1, 1, none},
{1, 1, equal},
{1, 2, less},
{2, 1, greater},
}
t.Parallel()
@ -214,7 +237,7 @@ func TestSorting(t *testing.T) {
assert.Equal(t, []result.Issue{issues[3], issues[2], issues[1], issues[0]}, results)
}
func Test_mergeComparator(t *testing.T) {
func Test_mergeComparators(t *testing.T) {
testCases := []struct {
desc string
cmps []comparator
@ -227,18 +250,18 @@ func Test_mergeComparator(t *testing.T) {
},
{
desc: "two",
cmps: []comparator{&byLinter{}, &byName{}},
expected: "byLinter > byName",
cmps: []comparator{&byLinter{}, &byFileName{}},
expected: "byLinter > byFileName",
},
{
desc: "all",
cmps: []comparator{&bySeverity{}, &byLinter{}, &byName{}, &byLine{}, &byColumn{}},
expected: "bySeverity > byLinter > byName > byLine > byColumn",
cmps: []comparator{&bySeverity{}, &byLinter{}, &byFileName{}, &byLine{}, &byColumn{}},
expected: "bySeverity > byLinter > byFileName > byLine > byColumn",
},
{
desc: "all reverse",
cmps: []comparator{&byColumn{}, &byLine{}, &byName{}, &byLinter{}, &bySeverity{}},
expected: "byColumn > byLine > byName > byLinter > bySeverity",
cmps: []comparator{&byColumn{}, &byLine{}, &byFileName{}, &byLinter{}, &bySeverity{}},
expected: "byColumn > byLine > byFileName > byLinter > bySeverity",
},
}
@ -247,7 +270,7 @@ func Test_mergeComparator(t *testing.T) {
t.Run(test.desc, func(t *testing.T) {
t.Parallel()
cmp, err := mergeComparator(test.cmps)
cmp, err := mergeComparators(test.cmps)
require.NoError(t, err)
assert.Equal(t, test.expected, cmp.String())
@ -255,7 +278,7 @@ func Test_mergeComparator(t *testing.T) {
}
}
func Test_mergeComparator_error(t *testing.T) {
_, err := mergeComparator(nil)
func Test_mergeComparators_error(t *testing.T) {
_, err := mergeComparators(nil)
require.EqualError(t, err, "no comparator")
}