Skip to content

Commit bed771a

Browse files
authored
feat: new default sort order (#5465)
1 parent 378dfbd commit bed771a

8 files changed

+26
-93
lines changed

.golangci.next.reference.yml

+1-6
Original file line numberDiff line numberDiff line change
@@ -4102,12 +4102,7 @@ output:
41024102
# Default: ""
41034103
path-prefix: ""
41044104

4105-
# Sort results by the order defined in `sort-order`.
4106-
# Default: false
4107-
sort-results: true
4108-
41094105
# Order to use when sorting results.
4110-
# Require `sort-results` to `true`.
41114106
# Possible values: `file`, `linter`, and `severity`.
41124107
#
41134108
# If the severity values are inside the following list, they are ordered in this order:
@@ -4118,7 +4113,7 @@ output:
41184113
# 5. low
41194114
# Either they are sorted alphabetically.
41204115
#
4121-
# Default: ["file"]
4116+
# Default: ["linter", "file"]
41224117
sort-order:
41234118
- linter
41244119
- severity

jsonschema/golangci.next.jsonschema.json

-5
Original file line numberDiff line numberDiff line change
@@ -3890,11 +3890,6 @@
38903890
"items": {
38913891
"enum": ["linter", "severity", "file"]
38923892
}
3893-
},
3894-
"sort-results": {
3895-
"description": "Sort results by: filepath, line and column.",
3896-
"type": "boolean",
3897-
"default": true
38983893
}
38993894
}
39003895
},

pkg/commands/flagsets.go

-4
Original file line numberDiff line numberDiff line change
@@ -70,10 +70,6 @@ func setupRunFlagSet(v *viper.Viper, fs *pflag.FlagSet) {
7070
}
7171

7272
func setupOutputFlagSet(v *viper.Viper, fs *pflag.FlagSet) {
73-
internal.AddFlagAndBind(v, fs, fs.Bool, "sort-results", "output.sort-results", false,
74-
color.GreenString("Sort linter results"))
75-
internal.AddFlagAndBind(v, fs, fs.StringSlice, "sort-order", "output.sort-order", nil,
76-
color.GreenString("Sort order of linter results"))
7773
internal.AddFlagAndBind(v, fs, fs.String, "path-prefix", "output.path-prefix", "",
7874
color.GreenString("Path prefix to add to output"))
7975
internal.AddFlagAndBind(v, fs, fs.Bool, "show-stats", "output.show-stats", false, color.GreenString("Show statistics per linter"))

pkg/config/output.go

+4-10
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,19 @@
11
package config
22

33
import (
4-
"errors"
54
"fmt"
65
"slices"
76
"strings"
87
)
98

109
type Output struct {
11-
Formats Formats `mapstructure:"formats"`
12-
SortResults bool `mapstructure:"sort-results"`
13-
SortOrder []string `mapstructure:"sort-order"`
14-
PathPrefix string `mapstructure:"path-prefix"`
15-
ShowStats bool `mapstructure:"show-stats"`
10+
Formats Formats `mapstructure:"formats"`
11+
SortOrder []string `mapstructure:"sort-order"`
12+
PathPrefix string `mapstructure:"path-prefix"`
13+
ShowStats bool `mapstructure:"show-stats"`
1614
}
1715

1816
func (o *Output) Validate() error {
19-
if !o.SortResults && len(o.SortOrder) > 0 {
20-
return errors.New("sort-results should be 'true' to use sort-order")
21-
}
22-
2317
validOrders := []string{"linter", "file", "severity"}
2418

2519
all := strings.Join(o.SortOrder, " ")

pkg/config/output_test.go

+6-19
Original file line numberDiff line numberDiff line change
@@ -14,29 +14,25 @@ func TestOutput_Validate(t *testing.T) {
1414
{
1515
desc: "file",
1616
settings: &Output{
17-
SortResults: true,
18-
SortOrder: []string{"file"},
17+
SortOrder: []string{"file"},
1918
},
2019
},
2120
{
2221
desc: "linter",
2322
settings: &Output{
24-
SortResults: true,
25-
SortOrder: []string{"linter"},
23+
SortOrder: []string{"linter"},
2624
},
2725
},
2826
{
2927
desc: "severity",
3028
settings: &Output{
31-
SortResults: true,
32-
SortOrder: []string{"severity"},
29+
SortOrder: []string{"severity"},
3330
},
3431
},
3532
{
3633
desc: "multiple",
3734
settings: &Output{
38-
SortResults: true,
39-
SortOrder: []string{"file", "linter", "severity"},
35+
SortOrder: []string{"file", "linter", "severity"},
4036
},
4137
},
4238
}
@@ -57,26 +53,17 @@ func TestOutput_Validate_error(t *testing.T) {
5753
settings *Output
5854
expected string
5955
}{
60-
{
61-
desc: "sort-results false and sort-order",
62-
settings: &Output{
63-
SortOrder: []string{"file"},
64-
},
65-
expected: "sort-results should be 'true' to use sort-order",
66-
},
6756
{
6857
desc: "invalid sort-order",
6958
settings: &Output{
70-
SortResults: true,
71-
SortOrder: []string{"a"},
59+
SortOrder: []string{"a"},
7260
},
7361
expected: `unsupported sort-order name "a"`,
7462
},
7563
{
7664
desc: "duplicate",
7765
settings: &Output{
78-
SortResults: true,
79-
SortOrder: []string{"file", "linter", "severity", "linter"},
66+
SortOrder: []string{"file", "linter", "severity", "linter"},
8067
},
8168
expected: `the sort-order name "linter" is repeated several times`,
8269
},

pkg/result/processors/sort_results.go

+1-5
Original file line numberDiff line numberDiff line change
@@ -55,12 +55,8 @@ func (SortResults) Name() string { return "sort_results" }
5555

5656
// Process is performing sorting of the result issues.
5757
func (p SortResults) Process(issues []result.Issue) ([]result.Issue, error) {
58-
if !p.cfg.SortResults {
59-
return issues, nil
60-
}
61-
6258
if len(p.cfg.SortOrder) == 0 {
63-
p.cfg.SortOrder = []string{orderNameFile}
59+
p.cfg.SortOrder = []string{orderNameLinter, orderNameFile}
6460
}
6561

6662
var cmps []issueComparator

pkg/result/processors/sort_results_test.go

+3-14
Original file line numberDiff line numberDiff line change
@@ -209,28 +209,17 @@ func Test_numericCompare(t *testing.T) {
209209
}
210210
}
211211

212-
func TestSortResults_Process_noSorting(t *testing.T) {
212+
func TestSortResults_Process(t *testing.T) {
213213
tests := make([]result.Issue, len(issues))
214214
copy(tests, issues)
215215

216-
sr := NewSortResults(&config.Output{})
217-
218-
results, err := sr.Process(tests)
219-
require.NoError(t, err)
220-
assert.Equal(t, tests, results)
221-
}
222-
223-
func TestSortResults_Process_Sorting(t *testing.T) {
224-
tests := make([]result.Issue, len(issues))
225-
copy(tests, issues)
226-
227-
cfg := &config.Output{SortResults: true}
216+
cfg := &config.Output{}
228217

229218
sr := NewSortResults(cfg)
230219

231220
results, err := sr.Process(tests)
232221
require.NoError(t, err)
233-
assert.Equal(t, []result.Issue{issues[3], issues[2], issues[1], issues[0]}, results)
222+
assert.Equal(t, []result.Issue{issues[1], issues[0], issues[3], issues[2]}, results)
234223
}
235224

236225
func compToString(c int) string {

test/run_test.go

+11-30
Original file line numberDiff line numberDiff line change
@@ -367,38 +367,19 @@ func TestUnsafeOk(t *testing.T) {
367367
}
368368

369369
func TestSortedResults(t *testing.T) {
370-
testCases := []struct {
371-
opt string
372-
want string
373-
}{
374-
{
375-
opt: "--sort-results=false",
376-
want: "testdata/sort_results/main.go:15:13: Error return value is not checked (errcheck)" + "\n" +
377-
"testdata/sort_results/main.go:12:5: var `db` is unused (unused)",
378-
},
379-
{
380-
opt: "--sort-results=true",
381-
want: "testdata/sort_results/main.go:12:5: var `db` is unused (unused)" + "\n" +
382-
"testdata/sort_results/main.go:15:13: Error return value is not checked (errcheck)",
383-
},
384-
}
385-
386370
binPath := testshared.InstallGolangciLint(t)
387371

388-
for _, test := range testCases {
389-
t.Run(test.opt, func(t *testing.T) {
390-
t.Parallel()
391-
392-
testshared.NewRunnerBuilder(t).
393-
WithNoConfig().
394-
WithArgs("--output.text.print-issued-lines=false", test.opt).
395-
WithTargetPath(testdataDir, "sort_results").
396-
WithBinPath(binPath).
397-
Runner().
398-
Run().
399-
ExpectExitCode(exitcodes.IssuesFound).ExpectOutputEq(test.want + "\n")
400-
})
401-
}
372+
testshared.NewRunnerBuilder(t).
373+
WithNoConfig().
374+
WithArgs("--output.text.print-issued-lines=false").
375+
WithTargetPath(testdataDir, "sort_results").
376+
WithBinPath(binPath).
377+
Runner().
378+
Run().
379+
ExpectExitCode(exitcodes.IssuesFound).ExpectOutputEq(
380+
"testdata/sort_results/main.go:15:13: Error return value is not checked (errcheck)" + "\n" +
381+
"testdata/sort_results/main.go:12:5: var `db` is unused (unused)" + "\n",
382+
)
402383
}
403384

404385
func TestIdentifierUsedOnlyInTests(t *testing.T) {

0 commit comments

Comments
 (0)