Skip to content

dev: simplify sort results processors #5150

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

Merged
merged 4 commits into from
Nov 23, 2024
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
211 changes: 50 additions & 161 deletions pkg/result/processors/sort_results.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
package processors

import (
"errors"
"cmp"
"fmt"
"slices"
"sort"
"strings"

"github.com/golangci/golangci-lint/pkg/config"
Expand All @@ -22,24 +21,32 @@ const (
orderNameSeverity = "severity"
)

const (
less = iota - 1
equal
greater
)

var _ Processor = (*SortResults)(nil)

type issueComparator func(a, b *result.Issue) int

type SortResults struct {
cmps map[string]*comparator
cmps map[string][]issueComparator

cfg *config.Output
}

func NewSortResults(cfg *config.Config) *SortResults {
return &SortResults{
cmps: map[string]*comparator{
cmps: map[string][]issueComparator{
// For sorting we are comparing (in next order):
// file names, line numbers, position, and finally - giving up.
orderNameFile: byFileName().SetNext(byLine().SetNext(byColumn())),
orderNameFile: {byFileName, byLine, 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,
}
Expand All @@ -57,171 +64,54 @@ func (p SortResults) Process(issues []result.Issue) ([]result.Issue, error) {
p.cfg.SortOrder = []string{orderNameFile}
}

var cmps []*comparator
var cmps []issueComparator

for _, name := range p.cfg.SortOrder {
c, ok := p.cmps[name]
if !ok {
return nil, fmt.Errorf("unsupported sort-order name %q", name)
}

cmps = append(cmps, c)
cmps = append(cmps, c...)
}

cmp, err := mergeComparators(cmps)
if err != nil {
return nil, err
}
comp := mergeComparators(cmps...)

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(&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
next *comparator
}

func (cmp *comparator) Next() *comparator { return cmp.next }

func (cmp *comparator) SetNext(c *comparator) *comparator {
cmp.next = c
return cmp
func byFileName(a, b *result.Issue) int {
return 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 byLine(a, b *result.Issue) int {
return numericCompare(a.Line(), b.Line())
}

func (cmp *comparator) Compare(a, b *result.Issue) compareResult {
res := cmp.compare(a, b)
if !res.isNeutral() {
return res
}

if next := cmp.Next(); next != nil {
return next.Compare(a, b)
}

return res
func byColumn(a, b *result.Issue) int {
return numericCompare(a.Column(), b.Column())
}

func byFileName() *comparator {
return &comparator{
name: "byFileName",
compare: func(a, b *result.Issue) compareResult {
return compareResult(strings.Compare(a.FilePath(), b.FilePath()))
},
}
func byLinter(a, b *result.Issue) int {
return strings.Compare(a.FromLinter, b.FromLinter)
}

func byLine() *comparator {
return &comparator{
name: "byLine",
compare: func(a, b *result.Issue) compareResult {
return numericCompare(a.Line(), b.Line())
},
}
func bySeverity(a, b *result.Issue) int {
return severityCompare(a.Severity, b.Severity)
}

func byColumn() *comparator {
return &comparator{
name: "byColumn",
compare: func(a, b *result.Issue) compareResult {
return numericCompare(a.Column(), b.Column())
},
}
}

func byLinter() *comparator {
return &comparator{
name: "byLinter",
compare: func(a, b *result.Issue) compareResult {
return compareResult(strings.Compare(a.FromLinter, b.FromLinter))
},
}
}

func bySeverity() *comparator {
return &comparator{
name: "bySeverity",
compare: func(a, b *result.Issue) compareResult {
return severityCompare(a.Severity, b.Severity)
},
}
}

func mergeComparators(cmps []*comparator) (*comparator, error) {
if len(cmps) == 0 {
return nil, errors.New("no comparator")
}

for i := range len(cmps) - 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 {
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) {
Expand All @@ -232,28 +122,27 @@ 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 values and zeros are skipped (equal) because they either invalid or "neutral" (default int value).
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)
}

func mergeComparators(comps ...issueComparator) issueComparator {
return func(a, b *result.Issue) int {
for _, comp := range comps {
i := comp(a, b)
if i != equal {
return i
}
}

return equal
}
}
Loading
Loading