feat: add sort-order option (#4467)

.golangci.reference.yml

@@ -109,14 +109,33 @@ output:
   # Default: ""
   path-prefix: ""
 
-  # Sort results by: filepath, line and column.
+  # Sort results by the order defined in `sort-order`.
   # Default: false
   sort-results: true
 
+  # Order to use when sorting results.
+  # Require `sort-results` to `true`.
+  # Possible values: `file`, `linter`, and `severity`.
+  #
+  # If the severity values are inside the following list, they are ordered in this order:
+  #   1. error
+  #   2. warning
+  #   3. high
+  #   4. medium
+  #   5. low
+  # Either they are sorted alphabetically.
+  #
+  # Default: ["file"]
+  sort-order:
+    - linter
+    - severity
+    - file # filepath, line, and column.
+
   # Show statistics per linter.
   # Default: false
   show-stats: true
 
+
 # All available settings of specific linters.
 linters-settings:
   asasalint:

pkg/commands/flagsets.go

@@ -71,6 +71,8 @@ func setupOutputFlagSet(v *viper.Viper, fs *pflag.FlagSet) {
 		color.GreenString("Make issues output unique by line"))
 	internal.AddFlagAndBind(v, fs, fs.Bool, "sort-results", "output.sort-results", false,
 		color.GreenString("Sort linter results"))
+	internal.AddFlagAndBind(v, fs, fs.StringSlice, "sort-order", "output.sort-order", nil,
+		color.GreenString("Sort order of linter results"))
 	internal.AddFlagAndBind(v, fs, fs.String, "path-prefix", "output.path-prefix", "",
 		color.GreenString("Path prefix to add to output"))
 	internal.AddFlagAndBind(v, fs, fs.Bool, "show-stats", "output.show-stats", false, color.GreenString("Show statistics per linter"))

pkg/config/config.go

@@ -37,6 +37,7 @@ func (c *Config) Validate() error {
 		c.Severity.Validate,
 		c.LintersSettings.Validate,
 		c.Linters.Validate,
+		c.Output.Validate,
 	}
 
 	for _, v := range validators {

pkg/config/output.go

@@ -1,5 +1,11 @@
 package config
 
+import (
+	"errors"
+	"fmt"
+	"slices"
+)
+
 const (
 	OutFormatJSON              = "json"
 	OutFormatLineNumber        = "line-number"
@@ -28,11 +34,26 @@ var OutFormats = []string{
 }
 
 type Output struct {
-	Format          string `mapstructure:"format"`
-	PrintIssuedLine bool   `mapstructure:"print-issued-lines"`
-	PrintLinterName bool   `mapstructure:"print-linter-name"`
-	UniqByLine      bool   `mapstructure:"uniq-by-line"`
-	SortResults     bool   `mapstructure:"sort-results"`
-	PathPrefix      string `mapstructure:"path-prefix"`
-	ShowStats       bool   `mapstructure:"show-stats"`
+	Format          string   `mapstructure:"format"`
+	PrintIssuedLine bool     `mapstructure:"print-issued-lines"`
+	PrintLinterName bool     `mapstructure:"print-linter-name"`
+	UniqByLine      bool     `mapstructure:"uniq-by-line"`
+	SortResults     bool     `mapstructure:"sort-results"`
+	SortOrder       []string `mapstructure:"sort-order"`
+	PathPrefix      string   `mapstructure:"path-prefix"`
+	ShowStats       bool     `mapstructure:"show-stats"`
+}
+
+func (o *Output) Validate() error {
+	if !o.SortResults && len(o.SortOrder) > 0 {
+		return errors.New("sort-results should be 'true' to use sort-order")
+	}
+
+	for _, order := range o.SortOrder {
+		if !slices.Contains([]string{"linter", "file", "severity"}, order) {
+			return fmt.Errorf("unsupported sort-order name %q", order)
+		}
+	}
+
+	return nil
 }

pkg/config/output_test.go

@@ -0,0 +1,80 @@
+package config
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestOutput_Validate(t *testing.T) {
+	testCases := []struct {
+		desc     string
+		settings *Output
+	}{
+		{
+			desc: "file",
+			settings: &Output{
+				SortResults: true,
+				SortOrder:   []string{"file"},
+			},
+		},
+		{
+			desc: "linter",
+			settings: &Output{
+				SortResults: true,
+				SortOrder:   []string{"linter"},
+			},
+		},
+		{
+			desc: "severity",
+			settings: &Output{
+				SortResults: true,
+				SortOrder:   []string{"severity"},
+			},
+		},
+	}
+
+	for _, test := range testCases {
+		test := test
+		t.Run(test.desc, func(t *testing.T) {
+			t.Parallel()
+
+			err := test.settings.Validate()
+			require.NoError(t, err)
+		})
+	}
+}
+
+func TestOutput_Validate_error(t *testing.T) {
+	testCases := []struct {
+		desc     string
+		settings *Output
+		expected string
+	}{
+		{
+			desc: "sort-results false and sort-order",
+			settings: &Output{
+				SortOrder: []string{"file"},
+			},
+			expected: "sort-results should be 'true' to use sort-order",
+		},
+		{
+			desc: "invalid sort-order",
+			settings: &Output{
+				SortResults: true,
+				SortOrder:   []string{"a"},
+			},
+			expected: `unsupported sort-order name "a"`,
+		},
+	}
+
+	for _, test := range testCases {
+		test := test
+		t.Run(test.desc, func(t *testing.T) {
+			t.Parallel()
+
+			err := test.settings.Validate()
+			require.EqualError(t, err, test.expected)
+		})
+	}
+}

pkg/result/processors/sort_results.go

@@ -1,6 +1,9 @@
 package processors
 
 import (
+	"errors"
+	"fmt"
+	"slices"
 	"sort"
 	"strings"
 
@@ -13,41 +16,69 @@ import (
 // by sorting results.Issues using processor step, and chain based
 // rules that can compare different properties of the Issues struct.
 
+const (
+	fileOrderName      = "file"
+	linterOrderName    = "linter"
+	linterSeverityName = "severity"
+)
+
 var _ Processor = (*SortResults)(nil)
 
 type SortResults struct {
-	cmp comparator
-	cfg *config.Config
+	cmps map[string][]comparator
+
+	cfg *config.Output
 }
 
 func NewSortResults(cfg *config.Config) *SortResults {
-	// For sorting we are comparing (in next order): file names, line numbers,
-	// position, and finally - giving up.
 	return &SortResults{
-		cmp: ByName{
-			next: ByLine{
-				next: ByColumn{},
-			},
+		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{}},
+			// For sorting we are comparing: linter name
+			linterOrderName: {&byLinter{}},
+			// For sorting we are comparing: severity
+			linterSeverityName: {&bySeverity{}},
 		},
-		cfg: cfg,
+		cfg: &cfg.Output,
 	}
 }
 
 // Process is performing sorting of the result issues.
 func (sr SortResults) Process(issues []result.Issue) ([]result.Issue, error) {
-	if !sr.cfg.Output.SortResults {
+	if !sr.cfg.SortResults {
 		return issues, nil
 	}
 
+	if len(sr.cfg.SortOrder) == 0 {
+		sr.cfg.SortOrder = []string{fileOrderName}
+	}
+
+	var cmps []comparator
+	for _, name := range sr.cfg.SortOrder {
+		if c, ok := sr.cmps[name]; ok {
+			cmps = append(cmps, c...)
+		} else {
+			return nil, fmt.Errorf("unsupported sort-order name %q", name)
+		}
+	}
+
+	cmp, err := mergeComparator(cmps)
+	if err != nil {
+		return nil, err
+	}
+
 	sort.Slice(issues, func(i, j int) bool {
-		return sr.cmp.Compare(&issues[i], &issues[j]) == Less
+		return cmp.Compare(&issues[i], &issues[j]) == Less
 	})
 
 	return issues, nil
 }
 
 func (sr SortResults) Name() string { return "sort_results" }
-func (sr SortResults) Finish()      {}
+
+func (sr SortResults) Finish() {}
 
 type compareResult int
 
@@ -71,28 +102,37 @@ func (c compareResult) String() string {
 		return "Equal"
 	case Greater:
 		return "Greater"
+	default:
+		return "None"
 	}
-
-	return "None"
 }
 
 // comparator describe how to implement compare for two "issues" lexicographically
 type comparator interface {
 	Compare(a, b *result.Issue) compareResult
 	Next() comparator
+	AddNext(comparator) comparator
+	fmt.Stringer
 }
 
 var (
-	_ comparator = (*ByName)(nil)
-	_ comparator = (*ByLine)(nil)
-	_ comparator = (*ByColumn)(nil)
+	_ comparator = (*byName)(nil)
+	_ comparator = (*byLine)(nil)
+	_ comparator = (*byColumn)(nil)
+	_ comparator = (*byLinter)(nil)
+	_ comparator = (*bySeverity)(nil)
 )
 
-type ByName struct{ next comparator }
+type byName struct{ next comparator }
 
-func (cmp ByName) Next() comparator { return cmp.next }
+func (cmp *byName) Next() comparator { return cmp.next }
 
-func (cmp ByName) Compare(a, b *result.Issue) compareResult {
+func (cmp *byName) 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() {
@@ -106,11 +146,20 @@ func (cmp ByName) Compare(a, b *result.Issue) compareResult {
 	return res
 }
 
-type ByLine struct{ next comparator }
+func (cmp *byName) String() string {
+	return comparatorToString("byName", cmp)
+}
 
-func (cmp ByLine) Next() comparator { return cmp.next }
+type byLine struct{ next comparator }
 
-func (cmp ByLine) Compare(a, b *result.Issue) compareResult {
+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 {
 	var res compareResult
 
 	if res = numericCompare(a.Line(), b.Line()); !res.isNeutral() {
@@ -124,11 +173,20 @@ func (cmp ByLine) Compare(a, b *result.Issue) compareResult {
 	return res
 }
 
-type ByColumn struct{ next comparator }
+func (cmp *byLine) String() string {
+	return comparatorToString("byLine", cmp)
+}
 
-func (cmp ByColumn) Next() comparator { return cmp.next }
+type byColumn struct{ next comparator }
 
-func (cmp ByColumn) Compare(a, b *result.Issue) compareResult {
+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 {
 	var res compareResult
 
 	if res = numericCompare(a.Column(), b.Column()); !res.isNeutral() {
@@ -142,6 +200,94 @@ func (cmp ByColumn) Compare(a, b *result.Issue) compareResult {
 	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 {
+	var res compareResult
+
+	if res = compareResult(strings.Compare(a.FromLinter, b.FromLinter)); !res.isNeutral() {
+		return res
+	}
+
+	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 {
+	var res compareResult
+
+	if res = severityCompare(a.Severity, b.Severity); !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 mergeComparator(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])
+	}
+
+	return cmps[0], nil
+}
+
+func severityCompare(a, b string) compareResult {
+	// 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 compareResult(strings.Compare(a, b))
+}
+
 func numericCompare(a, b int) compareResult {
 	var (
 		isValuesInvalid  = a < 0 || b < 0
@@ -164,3 +310,12 @@ 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
+}

pkg/result/processors/sort_results_test.go

@@ -14,6 +14,8 @@ import (
 
 var issues = []result.Issue{
 	{
+		FromLinter: "b",
+		Severity:   "medium",
 		Pos: token.Position{
 			Filename: "file_windows.go",
 			Column:   80,
@@ -21,6 +23,8 @@ var issues = []result.Issue{
 		},
 	},
 	{
+		FromLinter: "a",
+		Severity:   "low",
 		Pos: token.Position{
 			Filename: "file_linux.go",
 			Column:   70,
@@ -28,12 +32,16 @@ var issues = []result.Issue{
 		},
 	},
 	{
+		FromLinter: "c",
+		Severity:   "high",
 		Pos: token.Position{
 			Filename: "file_darwin.go",
 			Line:     12,
 		},
 	},
 	{
+		FromLinter: "c",
+		Severity:   "high",
 		Pos: token.Position{
 			Filename: "file_darwin.go",
 			Column:   60,
@@ -60,7 +68,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
@@ -76,7 +84,7 @@ func TestCompareByLine(t *testing.T) {
 }
 
 func TestCompareByName(t *testing.T) { //nolint:dupl
-	testCompareValues(t, ByName{}, "Compare By Name", []compareTestCase{
+	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
@@ -91,7 +99,7 @@ func TestCompareByName(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
@@ -105,10 +113,40 @@ func TestCompareByColumn(t *testing.T) { //nolint:dupl
 	})
 }
 
+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
+	})
+}
+
+func TestCompareBySeverity(t *testing.T) { //nolint:dupl
+	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
+	})
+}
+
 func TestCompareNested(t *testing.T) {
-	var cmp = ByName{
-		next: ByLine{
-			next: ByColumn{},
+	var cmp = &byName{
+		next: &byLine{
+			next: &byColumn{},
 		},
 	}
 
@@ -175,3 +213,49 @@ func TestSorting(t *testing.T) {
 	require.NoError(t, err)
 	assert.Equal(t, []result.Issue{issues[3], issues[2], issues[1], issues[0]}, results)
 }
+
+func Test_mergeComparator(t *testing.T) {
+	testCases := []struct {
+		desc     string
+		cmps     []comparator
+		expected string
+	}{
+		{
+			desc:     "one",
+			cmps:     []comparator{&byLinter{}},
+			expected: "byLinter",
+		},
+		{
+			desc:     "two",
+			cmps:     []comparator{&byLinter{}, &byName{}},
+			expected: "byLinter > byName",
+		},
+		{
+			desc:     "all",
+			cmps:     []comparator{&bySeverity{}, &byLinter{}, &byName{}, &byLine{}, &byColumn{}},
+			expected: "bySeverity > byLinter > byName > byLine > byColumn",
+		},
+		{
+			desc:     "all reverse",
+			cmps:     []comparator{&byColumn{}, &byLine{}, &byName{}, &byLinter{}, &bySeverity{}},
+			expected: "byColumn > byLine > byName > byLinter > bySeverity",
+		},
+	}
+
+	for _, test := range testCases {
+		test := test
+		t.Run(test.desc, func(t *testing.T) {
+			t.Parallel()
+
+			cmp, err := mergeComparator(test.cmps)
+			require.NoError(t, err)
+
+			assert.Equal(t, test.expected, cmp.String())
+		})
+	}
+}
+
+func Test_mergeComparator_error(t *testing.T) {
+	_, err := mergeComparator(nil)
+	require.EqualError(t, err, "no comparator")
+}