refactor: use standard comparison values

Author: ldez

pkg/result/processors/sort_results.go

@@ -1,10 +1,10 @@
 package processors
 
 import (
+	"cmp"
 	"errors"
 	"fmt"
 	"slices"
-	"sort"
 	"strings"
 
 	"github.com/golangci/golangci-lint/pkg/config"
@@ -22,6 +22,12 @@ const (
 	orderNameSeverity = "severity"
 )
 
+const (
+	less = iota - 1
+	equal
+	greater
+)
+
 var _ Processor = (*SortResults)(nil)
 
 type SortResults struct {
@@ -67,77 +73,50 @@ func (p SortResults) Process(issues []result.Issue) ([]result.Issue, error) {
 		cmps = append(cmps, c)
 	}
 
-	cmp, err := mergeComparators(cmps)
+	comp, 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
+	slices.SortFunc(issues, func(a, b result.Issue) int {
+		return comp.Compare(&a, &b)
 	})
 
 	return issues, nil
 }
 
 func (SortResults) Finish() {}
 
-type compareResult int
-
-const (
-	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
-}
-
-func (c compareResult) String() string {
-	switch c {
-	case less:
-		return "less"
-	case equal:
-		return "equal"
-	case greater:
-		return "greater"
-	default:
-		return "none"
-	}
-}
-
 // comparator describes how to implement compare for two "issues".
 type comparator struct {
 	name    string
-	compare func(a, b *result.Issue) compareResult
+	compare func(a, b *result.Issue) int
 	next    *comparator
 }
 
-func (cmp *comparator) Next() *comparator { return cmp.next }
+func (cp *comparator) Next() *comparator { return cp.next }
 
-func (cmp *comparator) SetNext(c *comparator) *comparator {
-	cmp.next = c
-	return cmp
+func (cp *comparator) SetNext(c *comparator) *comparator {
+	cp.next = c
+	return cp
 }
 
-func (cmp *comparator) String() string {
-	s := cmp.name
-	if cmp.Next() != nil {
-		s += " > " + cmp.Next().String()
+func (cp *comparator) String() string {
+	s := cp.name
+	if cp.Next() != nil {
+		s += " > " + cp.Next().String()
 	}
 
 	return s
 }
 
-func (cmp *comparator) Compare(a, b *result.Issue) compareResult {
-	res := cmp.compare(a, b)
-	if !res.isNeutral() {
+func (cp *comparator) Compare(a, b *result.Issue) int {
+	res := cp.compare(a, b)
+	if res != equal {
 		return res
 	}
 
-	if next := cmp.Next(); next != nil {
+	if next := cp.Next(); next != nil {
 		return next.Compare(a, b)
 	}
 
@@ -147,16 +126,16 @@ func (cmp *comparator) Compare(a, b *result.Issue) compareResult {
 func byFileName() *comparator {
 	return &comparator{
 		name: "byFileName",
-		compare: func(a, b *result.Issue) compareResult {
-			return compareResult(strings.Compare(a.FilePath(), b.FilePath()))
+		compare: func(a, b *result.Issue) int {
+			return strings.Compare(a.FilePath(), b.FilePath())
 		},
 	}
 }
 
 func byLine() *comparator {
 	return &comparator{
 		name: "byLine",
-		compare: func(a, b *result.Issue) compareResult {
+		compare: func(a, b *result.Issue) int {
 			return numericCompare(a.Line(), b.Line())
 		},
 	}
@@ -165,7 +144,7 @@ func byLine() *comparator {
 func byColumn() *comparator {
 	return &comparator{
 		name: "byColumn",
-		compare: func(a, b *result.Issue) compareResult {
+		compare: func(a, b *result.Issue) int {
 			return numericCompare(a.Column(), b.Column())
 		},
 	}
@@ -174,16 +153,16 @@ func byColumn() *comparator {
 func byLinter() *comparator {
 	return &comparator{
 		name: "byLinter",
-		compare: func(a, b *result.Issue) compareResult {
-			return compareResult(strings.Compare(a.FromLinter, b.FromLinter))
+		compare: func(a, b *result.Issue) int {
+			return strings.Compare(a.FromLinter, b.FromLinter)
 		},
 	}
 }
 
 func bySeverity() *comparator {
 	return &comparator{
 		name: "bySeverity",
-		compare: func(a, b *result.Issue) compareResult {
+		compare: func(a, b *result.Issue) int {
 			return severityCompare(a.Severity, b.Severity)
 		},
 	}
@@ -209,19 +188,12 @@ func findComparatorTip(cmp *comparator) *comparator {
 	return cmp
 }
 
-func severityCompare(a, b string) compareResult {
+func severityCompare(a, b string) int {
 	// The position inside the slice define the importance (lower to higher).
 	classic := []string{"low", "medium", "high", "warning", "error"}
 
 	if slices.Contains(classic, a) && slices.Contains(classic, b) {
-		switch {
-		case slices.Index(classic, a) > slices.Index(classic, b):
-			return greater
-		case slices.Index(classic, a) < slices.Index(classic, b):
-			return less
-		default:
-			return equal
-		}
+		return cmp.Compare(slices.Index(classic, a), slices.Index(classic, b))
 	}
 
 	if slices.Contains(classic, a) {
@@ -232,28 +204,14 @@ func severityCompare(a, b string) compareResult {
 		return less
 	}
 
-	return compareResult(strings.Compare(a, b))
+	return strings.Compare(a, b)
 }
 
-func numericCompare(a, b int) compareResult {
-	var (
-		isValuesInvalid  = a < 0 || b < 0
-		isZeroValuesBoth = a == 0 && b == 0
-		isEqual          = a == b
-		isZeroValueInA   = b > 0 && a == 0
-		isZeroValueInB   = a > 0 && b == 0
-	)
-
-	switch {
-	case isZeroValuesBoth || isEqual:
+func numericCompare(a, b int) int {
+	// Negative value and 0 are skipped because they either "neutral" (default value) or "invalid.
+	if a <= 0 || b <= 0 {
 		return equal
-	case isValuesInvalid || isZeroValueInA || isZeroValueInB:
-		return none
-	case a > b:
-		return greater
-	case a < b:
-		return less
 	}
 
-	return equal
+	return cmp.Compare(a, b)
 }

pkg/result/processors/sort_results_test.go

@@ -73,7 +73,7 @@ var extraSeverityIssues = []result.Issue{
 
 type compareTestCase struct {
 	a, b     result.Issue
-	expected compareResult
+	expected int
 }
 
 func testCompareValues(t *testing.T, cmp *comparator, name string, tests []compareTestCase) {
@@ -82,7 +82,7 @@ func testCompareValues(t *testing.T, cmp *comparator, name string, tests []compa
 	for i, test := range tests { //nolint:gocritic // To ignore rangeValCopy rule
 		t.Run(fmt.Sprintf("%s(%d)", name, i), func(t *testing.T) {
 			res := cmp.Compare(&test.a, &test.b)
-			assert.Equal(t, test.expected.String(), res.String())
+			assert.Equal(t, compToString(test.expected), compToString(res))
 		})
 	}
 }
@@ -121,13 +121,13 @@ func TestCompareByFileName(t *testing.T) {
 func TestCompareByColumn(t *testing.T) {
 	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[1], issues[2], equal},   // 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[2], issues[3], equal},   // zero value vs 60
+		{issues[2], issues[1], equal},   // 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[3], issues[2], equal},   // vs zero value
 		{issues[2], issues[2], equal},   // zero value vs zero value
 		{issues[1], issues[1], equal},   // 70 vs 70
 	})
@@ -185,13 +185,13 @@ func TestCompareNested(t *testing.T) {
 func TestNumericCompare(t *testing.T) {
 	tests := []struct {
 		a, b     int
-		expected compareResult
+		expected int
 	}{
 		{0, 0, equal},
-		{0, 1, none},
-		{1, 0, none},
-		{1, -1, none},
-		{-1, 1, none},
+		{0, 1, equal},
+		{1, 0, equal},
+		{1, -1, equal},
+		{-1, 1, equal},
 		{1, 1, equal},
 		{1, 2, less},
 		{2, 1, greater},
@@ -202,7 +202,7 @@ func TestNumericCompare(t *testing.T) {
 	for i, test := range tests {
 		t.Run(fmt.Sprintf("%s(%d)", "Numeric Compare", i), func(t *testing.T) {
 			res := numericCompare(test.a, test.b)
-			assert.Equal(t, test.expected.String(), res.String())
+			assert.Equal(t, compToString(test.expected), compToString(res))
 		})
 	}
 }
@@ -280,3 +280,14 @@ func Test_mergeComparators_error(t *testing.T) {
 	_, err := mergeComparators(nil)
 	require.EqualError(t, err, "no comparator")
 }
+
+func compToString(c int) string {
+	switch c {
+	case less:
+		return "less"
+	case greater:
+		return "greater"
+	default:
+		return "equal"
+	}
+}