chore: simplify comparators (#4499)

This commit is contained in:
Ludovic Fernandez 2024-03-13 21:10:08 +01:00 committed by GitHub
parent 78d2118c0b
commit 51a963fa5b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 89 additions and 154 deletions

View File

@ -25,21 +25,21 @@ const (
var _ Processor = (*SortResults)(nil)
type SortResults struct {
cmps map[string][]comparator
cmps map[string]*comparator
cfg *config.Output
}
func NewSortResults(cfg *config.Config) *SortResults {
return &SortResults{
cmps: map[string][]comparator{
cmps: map[string]*comparator{
// For sorting we are comparing (in next order):
// file names, line numbers, position, and finally - giving up.
orderNameFile: {&byFileName{}, &byLine{}, &byColumn{}},
orderNameFile: byFileName().SetNext(byLine().SetNext(byColumn())),
// For sorting we are comparing: linter name
orderNameLinter: {&byLinter{}},
orderNameLinter: byLinter(),
// For sorting we are comparing: severity
orderNameSeverity: {&bySeverity{}},
orderNameSeverity: bySeverity(),
},
cfg: &cfg.Output,
}
@ -55,10 +55,10 @@ func (sr SortResults) Process(issues []result.Issue) ([]result.Issue, error) {
sr.cfg.SortOrder = []string{orderNameFile}
}
var cmps []comparator
var cmps []*comparator
for _, name := range sr.cfg.SortOrder {
if c, ok := sr.cmps[name]; ok {
cmps = append(cmps, c...)
cmps = append(cmps, c)
} else {
return nil, fmt.Errorf("unsupported sort-order name %q", name)
}
@ -108,32 +108,30 @@ func (c compareResult) String() string {
}
// comparator describes how to implement compare for two "issues".
type comparator interface {
Compare(a, b *result.Issue) compareResult
Next() comparator
AddNext(comparator) comparator
fmt.Stringer
type comparator struct {
name string
compare func(a, b *result.Issue) compareResult
next *comparator
}
var (
_ comparator = (*byFileName)(nil)
_ comparator = (*byLine)(nil)
_ comparator = (*byColumn)(nil)
_ comparator = (*byLinter)(nil)
_ comparator = (*bySeverity)(nil)
)
func (cmp *comparator) Next() *comparator { return cmp.next }
type byFileName struct{ next comparator }
func (cmp *byFileName) Next() comparator { return cmp.next }
func (cmp *byFileName) AddNext(c comparator) comparator {
func (cmp *comparator) SetNext(c *comparator) *comparator {
cmp.next = c
return cmp
}
func (cmp *byFileName) Compare(a, b *result.Issue) compareResult {
res := compareResult(strings.Compare(a.FilePath(), b.FilePath()))
func (cmp *comparator) String() string {
s := cmp.name
if cmp.Next() != nil {
s += " > " + cmp.Next().String()
}
return s
}
func (cmp *comparator) Compare(a, b *result.Issue) compareResult {
res := cmp.compare(a, b)
if !res.isNeutral() {
return res
}
@ -145,126 +143,71 @@ func (cmp *byFileName) Compare(a, b *result.Issue) compareResult {
return res
}
func (cmp *byFileName) String() string {
return comparatorToString("byFileName", cmp)
}
type byLine struct{ next comparator }
func (cmp *byLine) Next() comparator { return cmp.next }
func (cmp *byLine) AddNext(c comparator) comparator {
cmp.next = c
return cmp
}
func (cmp *byLine) Compare(a, b *result.Issue) compareResult {
res := numericCompare(a.Line(), b.Line())
if !res.isNeutral() {
return res
func byFileName() *comparator {
return &comparator{
name: "byFileName",
compare: func(a, b *result.Issue) compareResult {
return compareResult(strings.Compare(a.FilePath(), b.FilePath()))
},
}
}
if next := cmp.Next(); next != nil {
return next.Compare(a, b)
func byLine() *comparator {
return &comparator{
name: "byLine",
compare: func(a, b *result.Issue) compareResult {
return numericCompare(a.Line(), b.Line())
},
}
return res
}
func (cmp *byLine) String() string {
return comparatorToString("byLine", cmp)
}
type byColumn struct{ next comparator }
func (cmp *byColumn) Next() comparator { return cmp.next }
func (cmp *byColumn) AddNext(c comparator) comparator {
cmp.next = c
return cmp
}
func (cmp *byColumn) Compare(a, b *result.Issue) compareResult {
res := numericCompare(a.Column(), b.Column())
if !res.isNeutral() {
return res
func byColumn() *comparator {
return &comparator{
name: "byColumn",
compare: func(a, b *result.Issue) compareResult {
return numericCompare(a.Column(), b.Column())
},
}
}
if next := cmp.Next(); next != nil {
return next.Compare(a, b)
func byLinter() *comparator {
return &comparator{
name: "byLinter",
compare: func(a, b *result.Issue) compareResult {
return compareResult(strings.Compare(a.FromLinter, b.FromLinter))
},
}
return res
}
func (cmp *byColumn) String() string {
return comparatorToString("byColumn", cmp)
}
type byLinter struct{ next comparator }
func (cmp *byLinter) Next() comparator { return cmp.next }
func (cmp *byLinter) AddNext(c comparator) comparator {
cmp.next = c
return cmp
}
func (cmp *byLinter) Compare(a, b *result.Issue) compareResult {
res := compareResult(strings.Compare(a.FromLinter, b.FromLinter))
if !res.isNeutral() {
return res
func bySeverity() *comparator {
return &comparator{
name: "bySeverity",
compare: func(a, b *result.Issue) compareResult {
return severityCompare(a.Severity, b.Severity)
},
}
if next := cmp.Next(); next != nil {
return next.Compare(a, b)
}
return res
}
func (cmp *byLinter) String() string {
return comparatorToString("byLinter", cmp)
}
type bySeverity struct{ next comparator }
func (cmp *bySeverity) Next() comparator { return cmp.next }
func (cmp *bySeverity) AddNext(c comparator) comparator {
cmp.next = c
return cmp
}
func (cmp *bySeverity) Compare(a, b *result.Issue) compareResult {
res := severityCompare(a.Severity, b.Severity)
if !res.isNeutral() {
return res
}
if next := cmp.Next(); next != nil {
return next.Compare(a, b)
}
return res
}
func (cmp *bySeverity) String() string {
return comparatorToString("bySeverity", cmp)
}
func mergeComparators(cmps []comparator) (comparator, error) {
func mergeComparators(cmps []*comparator) (*comparator, error) {
if len(cmps) == 0 {
return nil, errors.New("no comparator")
}
for i := 0; i < len(cmps)-1; i++ {
cmps[i].AddNext(cmps[i+1])
findComparatorTip(cmps[i]).SetNext(cmps[i+1])
}
return cmps[0], nil
}
func findComparatorTip(cmp *comparator) *comparator {
if cmp.Next() != nil {
return findComparatorTip(cmp.Next())
}
return cmp
}
func severityCompare(a, b string) compareResult {
// The position inside the slice define the importance (lower to higher).
classic := []string{"low", "medium", "high", "warning", "error"}
@ -313,12 +256,3 @@ func numericCompare(a, b int) compareResult {
return equal
}
func comparatorToString(name string, c comparator) string {
s := name
if c.Next() != nil {
s += " > " + c.Next().String()
}
return s
}

View File

@ -76,7 +76,7 @@ type compareTestCase struct {
expected compareResult
}
func testCompareValues(t *testing.T, cmp comparator, name string, tests []compareTestCase) {
func testCompareValues(t *testing.T, cmp *comparator, name string, tests []compareTestCase) {
t.Parallel()
for i := 0; i < len(tests); i++ {
@ -89,7 +89,7 @@ func testCompareValues(t *testing.T, cmp comparator, name string, tests []compar
}
func TestCompareByLine(t *testing.T) {
testCompareValues(t, &byLine{}, "Compare By Line", []compareTestCase{
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
@ -105,7 +105,7 @@ func TestCompareByLine(t *testing.T) {
}
func TestCompareByFileName(t *testing.T) { //nolint:dupl
testCompareValues(t, &byFileName{}, "Compare By File Name", []compareTestCase{
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
@ -120,7 +120,7 @@ func TestCompareByFileName(t *testing.T) { //nolint:dupl
}
func TestCompareByColumn(t *testing.T) { //nolint:dupl
testCompareValues(t, &byColumn{}, "Compare By Column", []compareTestCase{
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
@ -135,7 +135,7 @@ func TestCompareByColumn(t *testing.T) { //nolint:dupl
}
func TestCompareByLinter(t *testing.T) { //nolint:dupl
testCompareValues(t, &byLinter{}, "Compare By Linter", []compareTestCase{
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
@ -150,7 +150,7 @@ func TestCompareByLinter(t *testing.T) { //nolint:dupl
}
func TestCompareBySeverity(t *testing.T) {
testCompareValues(t, &bySeverity{}, "Compare By Severity", []compareTestCase{
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
@ -167,11 +167,7 @@ func TestCompareBySeverity(t *testing.T) {
}
func TestCompareNested(t *testing.T) {
var cmp = &byFileName{
next: &byLine{
next: &byColumn{},
},
}
cmp := byFileName().SetNext(byLine().SetNext(byColumn()))
testCompareValues(t, cmp, "Nested Comparing", []compareTestCase{
{issues[1], issues[0], less}, // file_linux.go vs file_windows.go
@ -188,7 +184,7 @@ func TestCompareNested(t *testing.T) {
}
func TestNumericCompare(t *testing.T) {
var tests = []struct {
tests := []struct {
a, b int
expected compareResult
}{
@ -214,10 +210,10 @@ func TestNumericCompare(t *testing.T) {
}
func TestNoSorting(t *testing.T) {
var tests = make([]result.Issue, len(issues))
tests := make([]result.Issue, len(issues))
copy(tests, issues)
var sr = NewSortResults(&config.Config{})
sr := NewSortResults(&config.Config{})
results, err := sr.Process(tests)
require.NoError(t, err)
@ -225,12 +221,12 @@ func TestNoSorting(t *testing.T) {
}
func TestSorting(t *testing.T) {
var tests = make([]result.Issue, len(issues))
tests := make([]result.Issue, len(issues))
copy(tests, issues)
var cfg = config.Config{}
cfg := config.Config{}
cfg.Output.SortResults = true
var sr = NewSortResults(&cfg)
sr := NewSortResults(&cfg)
results, err := sr.Process(tests)
require.NoError(t, err)
@ -240,27 +236,32 @@ func TestSorting(t *testing.T) {
func Test_mergeComparators(t *testing.T) {
testCases := []struct {
desc string
cmps []comparator
cmps []*comparator
expected string
}{
{
desc: "one",
cmps: []comparator{&byLinter{}},
cmps: []*comparator{byLinter()},
expected: "byLinter",
},
{
desc: "two",
cmps: []comparator{&byLinter{}, &byFileName{}},
cmps: []*comparator{byLinter(), byFileName()},
expected: "byLinter > byFileName",
},
{
desc: "all",
cmps: []comparator{&bySeverity{}, &byLinter{}, &byFileName{}, &byLine{}, &byColumn{}},
cmps: []*comparator{bySeverity(), byLinter(), byFileName(), byLine(), byColumn()},
expected: "bySeverity > byLinter > byFileName > byLine > byColumn",
},
{
desc: "nested",
cmps: []*comparator{bySeverity(), byFileName().SetNext(byLine().SetNext(byColumn())), byLinter()},
expected: "bySeverity > byFileName > byLine > byColumn > byLinter",
},
{
desc: "all reverse",
cmps: []comparator{&byColumn{}, &byLine{}, &byFileName{}, &byLinter{}, &bySeverity{}},
cmps: []*comparator{byColumn(), byLine(), byFileName(), byLinter(), bySeverity()},
expected: "byColumn > byLine > byFileName > byLinter > bySeverity",
},
}