Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: new default sort order #5465

Merged
merged 3 commits into from
Feb 24, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 1 addition & 6 deletions .golangci.next.reference.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4102,12 +4102,7 @@ output:
# Default: ""
path-prefix: ""

# 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:
Expand All @@ -4118,7 +4113,7 @@ output:
# 5. low
# Either they are sorted alphabetically.
#
# Default: ["file"]
# Default: ["linter", "file"]
sort-order:
- linter
- severity
Expand Down
5 changes: 0 additions & 5 deletions jsonschema/golangci.next.jsonschema.json
Original file line number Diff line number Diff line change
Expand Up @@ -3890,11 +3890,6 @@
"items": {
"enum": ["linter", "severity", "file"]
}
},
"sort-results": {
"description": "Sort results by: filepath, line and column.",
"type": "boolean",
"default": true
}
}
},
Expand Down
4 changes: 0 additions & 4 deletions pkg/commands/flagsets.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,6 @@ func setupRunFlagSet(v *viper.Viper, fs *pflag.FlagSet) {
}

func setupOutputFlagSet(v *viper.Viper, fs *pflag.FlagSet) {
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"))
Expand Down
14 changes: 4 additions & 10 deletions pkg/config/output.go
Original file line number Diff line number Diff line change
@@ -1,25 +1,19 @@
package config

import (
"errors"
"fmt"
"slices"
"strings"
)

type Output struct {
Formats Formats `mapstructure:"formats"`
SortResults bool `mapstructure:"sort-results"`
SortOrder []string `mapstructure:"sort-order"`
PathPrefix string `mapstructure:"path-prefix"`
ShowStats bool `mapstructure:"show-stats"`
Formats Formats `mapstructure:"formats"`
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")
}

validOrders := []string{"linter", "file", "severity"}

all := strings.Join(o.SortOrder, " ")
Expand Down
25 changes: 6 additions & 19 deletions pkg/config/output_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,29 +14,25 @@ func TestOutput_Validate(t *testing.T) {
{
desc: "file",
settings: &Output{
SortResults: true,
SortOrder: []string{"file"},
SortOrder: []string{"file"},
},
},
{
desc: "linter",
settings: &Output{
SortResults: true,
SortOrder: []string{"linter"},
SortOrder: []string{"linter"},
},
},
{
desc: "severity",
settings: &Output{
SortResults: true,
SortOrder: []string{"severity"},
SortOrder: []string{"severity"},
},
},
{
desc: "multiple",
settings: &Output{
SortResults: true,
SortOrder: []string{"file", "linter", "severity"},
SortOrder: []string{"file", "linter", "severity"},
},
},
}
Expand All @@ -57,26 +53,17 @@ func TestOutput_Validate_error(t *testing.T) {
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"},
SortOrder: []string{"a"},
},
expected: `unsupported sort-order name "a"`,
},
{
desc: "duplicate",
settings: &Output{
SortResults: true,
SortOrder: []string{"file", "linter", "severity", "linter"},
SortOrder: []string{"file", "linter", "severity", "linter"},
},
expected: `the sort-order name "linter" is repeated several times`,
},
Expand Down
6 changes: 1 addition & 5 deletions pkg/result/processors/sort_results.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,8 @@ func (SortResults) Name() string { return "sort_results" }

// Process is performing sorting of the result issues.
func (p SortResults) Process(issues []result.Issue) ([]result.Issue, error) {
if !p.cfg.SortResults {
return issues, nil
}

if len(p.cfg.SortOrder) == 0 {
p.cfg.SortOrder = []string{orderNameFile}
p.cfg.SortOrder = []string{orderNameLinter, orderNameFile}
}

var cmps []issueComparator
Expand Down
17 changes: 3 additions & 14 deletions pkg/result/processors/sort_results_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -209,28 +209,17 @@ func Test_numericCompare(t *testing.T) {
}
}

func TestSortResults_Process_noSorting(t *testing.T) {
func TestSortResults_Process(t *testing.T) {
tests := make([]result.Issue, len(issues))
copy(tests, issues)

sr := NewSortResults(&config.Output{})

results, err := sr.Process(tests)
require.NoError(t, err)
assert.Equal(t, tests, results)
}

func TestSortResults_Process_Sorting(t *testing.T) {
tests := make([]result.Issue, len(issues))
copy(tests, issues)

cfg := &config.Output{SortResults: true}
cfg := &config.Output{}

sr := NewSortResults(cfg)

results, err := sr.Process(tests)
require.NoError(t, err)
assert.Equal(t, []result.Issue{issues[3], issues[2], issues[1], issues[0]}, results)
assert.Equal(t, []result.Issue{issues[1], issues[0], issues[3], issues[2]}, results)
}

func compToString(c int) string {
Expand Down
41 changes: 11 additions & 30 deletions test/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -367,38 +367,19 @@ func TestUnsafeOk(t *testing.T) {
}

func TestSortedResults(t *testing.T) {
testCases := []struct {
opt string
want string
}{
{
opt: "--sort-results=false",
want: "testdata/sort_results/main.go:15:13: Error return value is not checked (errcheck)" + "\n" +
"testdata/sort_results/main.go:12:5: var `db` is unused (unused)",
},
{
opt: "--sort-results=true",
want: "testdata/sort_results/main.go:12:5: var `db` is unused (unused)" + "\n" +
"testdata/sort_results/main.go:15:13: Error return value is not checked (errcheck)",
},
}

binPath := testshared.InstallGolangciLint(t)

for _, test := range testCases {
t.Run(test.opt, func(t *testing.T) {
t.Parallel()

testshared.NewRunnerBuilder(t).
WithNoConfig().
WithArgs("--output.text.print-issued-lines=false", test.opt).
WithTargetPath(testdataDir, "sort_results").
WithBinPath(binPath).
Runner().
Run().
ExpectExitCode(exitcodes.IssuesFound).ExpectOutputEq(test.want + "\n")
})
}
testshared.NewRunnerBuilder(t).
WithNoConfig().
WithArgs("--output.text.print-issued-lines=false").
WithTargetPath(testdataDir, "sort_results").
WithBinPath(binPath).
Runner().
Run().
ExpectExitCode(exitcodes.IssuesFound).ExpectOutputEq(
"testdata/sort_results/main.go:15:13: Error return value is not checked (errcheck)" + "\n" +
"testdata/sort_results/main.go:12:5: var `db` is unused (unused)" + "\n",
)
}

func TestIdentifierUsedOnlyInTests(t *testing.T) {
Expand Down
Loading