From b704144c779172d316f2b6c6f24842e58016b0b4 Mon Sep 17 00:00:00 2001
From: Lukasz Mierzwa <lukasz@cloudflare.com>
Date: Fri, 25 Oct 2024 15:29:11 +0100
Subject: [PATCH] Sort reports

---
 cmd/pint/ci.go                                |  1 +
 cmd/pint/lint.go                              |  1 +
 cmd/pint/scan.go                              |  1 -
 cmd/pint/tests/0007_alerts.txt                |  8 ++---
 .../tests/0008_recording_rule_prometheus.txt  |  4 +--
 cmd/pint/tests/0160_ci_comment_edit.txt       |  4 +--
 cmd/pint/watch.go                             |  4 +--
 internal/checks/promql_vector_matching.go     |  3 +-
 internal/checks/rule_dependency.go            | 17 +++++-----
 internal/output/ranges.go                     |  4 +--
 internal/promapi/range_normalize.go           |  3 +-
 internal/reporter/console.go                  | 33 ++-----------------
 internal/reporter/json.go                     |  2 +-
 internal/reporter/reporter.go                 | 26 +++++++--------
 14 files changed, 40 insertions(+), 71 deletions(-)

diff --git a/cmd/pint/ci.go b/cmd/pint/ci.go
index 8f18cb26..c0a41b4e 100644
--- a/cmd/pint/ci.go
+++ b/cmd/pint/ci.go
@@ -257,6 +257,7 @@ func actionCI(c *cli.Context) error {
 		slog.Info("Problems found", logSeverityCounters(bySeverity)...)
 	}
 
+	summary.SortReports()
 	for _, rep := range reps {
 		err = rep.Submit(summary)
 		if err != nil {
diff --git a/cmd/pint/lint.go b/cmd/pint/lint.go
index c02007ef..be1dead2 100644
--- a/cmd/pint/lint.go
+++ b/cmd/pint/lint.go
@@ -139,6 +139,7 @@ func actionLint(c *cli.Context) error {
 		reps = append(reps, reporter.NewJSONReporter(j))
 	}
 
+	summary.SortReports()
 	for _, rep := range reps {
 		err = rep.Submit(summary)
 		if err != nil {
diff --git a/cmd/pint/scan.go b/cmd/pint/scan.go
index 918ff597..f588a79a 100644
--- a/cmd/pint/scan.go
+++ b/cmd/pint/scan.go
@@ -116,7 +116,6 @@ func checkRules(ctx context.Context, workers int, isOffline bool, gen *config.Pr
 	for result := range results {
 		summary.Report(result)
 	}
-	summary.SortReports()
 	summary.Duration = time.Since(start)
 	summary.TotalEntries = len(entries)
 	summary.CheckedEntries = checkedEntriesCount.Load()
diff --git a/cmd/pint/tests/0007_alerts.txt b/cmd/pint/tests/0007_alerts.txt
index 81c2f7dd..8d6bfe7a 100644
--- a/cmd/pint/tests/0007_alerts.txt
+++ b/cmd/pint/tests/0007_alerts.txt
@@ -5,22 +5,22 @@ cmp stderr stderr.txt
 -- stderr.txt --
 level=INFO msg="Loading configuration file" path=.pint.hcl
 level=INFO msg="Finding all rules to check" paths=["rules"]
-rules/0001.yml:1-2 Bug: `url` annotation is required. (alerts/annotation)
+rules/0001.yml:1-2 Warning: `severity` label is required. (rule/label)
  1 | - alert: Always
  2 |   expr: up
 
-rules/0001.yml:1-2 Warning: `severity` label is required. (rule/label)
+rules/0001.yml:1-2 Bug: `url` annotation is required. (alerts/annotation)
  1 | - alert: Always
  2 |   expr: up
 
 rules/0001.yml:2 Warning: Alert query doesn't have any condition, it will always fire if the metric exists. (alerts/comparison)
  2 |   expr: up
 
-rules/0001.yml:9-10 Bug: `url` annotation is required. (alerts/annotation)
+rules/0001.yml:9-10 Warning: `severity` label is required. (rule/label)
   9 | - alert: ServiceIsDown
  10 |   expr: up == 0
 
-rules/0001.yml:9-10 Warning: `severity` label is required. (rule/label)
+rules/0001.yml:9-10 Bug: `url` annotation is required. (alerts/annotation)
   9 | - alert: ServiceIsDown
  10 |   expr: up == 0
 
diff --git a/cmd/pint/tests/0008_recording_rule_prometheus.txt b/cmd/pint/tests/0008_recording_rule_prometheus.txt
index 64bf9cf9..b78654af 100644
--- a/cmd/pint/tests/0008_recording_rule_prometheus.txt
+++ b/cmd/pint/tests/0008_recording_rule_prometheus.txt
@@ -5,10 +5,10 @@ cmp stderr stderr.txt
 -- stderr.txt --
 level=INFO msg="Loading configuration file" path=.pint.hcl
 level=INFO msg="Finding all rules to check" paths=["rules"]
-rules/0001.yml:5 Bug: `instance` label should be removed when aggregating `^colo(?:_.+)?:.+$` rules, remove instance from `by()`. (promql/aggregate)
+rules/0001.yml:5 Warning: `job` label is required and should be preserved when aggregating `^.+$` rules, use `by(job, ...)`. (promql/aggregate)
  5 |       expr: sum by (instance) (http_inprogress_requests)
 
-rules/0001.yml:5 Warning: `job` label is required and should be preserved when aggregating `^.+$` rules, use `by(job, ...)`. (promql/aggregate)
+rules/0001.yml:5 Bug: `instance` label should be removed when aggregating `^colo(?:_.+)?:.+$` rules, remove instance from `by()`. (promql/aggregate)
  5 |       expr: sum by (instance) (http_inprogress_requests)
 
 level=INFO msg="Problems found" Bug=1 Warning=1
diff --git a/cmd/pint/tests/0160_ci_comment_edit.txt b/cmd/pint/tests/0160_ci_comment_edit.txt
index ddfc3e94..d5062806 100644
--- a/cmd/pint/tests/0160_ci_comment_edit.txt
+++ b/cmd/pint/tests/0160_ci_comment_edit.txt
@@ -47,10 +47,10 @@ level=WARN msg="Using dummy Prometheus uptime metric results with no gaps" name=
 level=WARN msg="No results for Prometheus uptime metric, you might have set uptime config option to a missing metric, please check your config" name=prom metric=up
 level=WARN msg="Using dummy Prometheus uptime metric results with no gaps" name=prom metric=up
 level=INFO msg="Problems found" Bug=2 Warning=1
-rules.yml:8 Bug: `prom` Prometheus server at http://127.0.0.1:7160 didn't have any series for `up` metric in the last 1w. (promql/series)
+rules.yml:8 Warning: pint disable comment `promql/series(xxx)` doesn't match any selector in this query (promql/series)
  8 |     expr: up == 0
 
-rules.yml:8 Warning: pint disable comment `promql/series(xxx)` doesn't match any selector in this query (promql/series)
+rules.yml:8 Bug: `prom` Prometheus server at http://127.0.0.1:7160 didn't have any series for `up` metric in the last 1w. (promql/series)
  8 |     expr: up == 0
 
 rules.yml:16 Bug: `prom` Prometheus server at http://127.0.0.1:7160 didn't have any series for `up` metric in the last 1w. (promql/series)
diff --git a/cmd/pint/watch.go b/cmd/pint/watch.go
index cfd58a55..174365a8 100644
--- a/cmd/pint/watch.go
+++ b/cmd/pint/watch.go
@@ -9,7 +9,7 @@ import (
 	_ "net/http/pprof"
 	"os"
 	"os/signal"
-	"sort"
+	"slices"
 	"strings"
 	"sync"
 	"syscall"
@@ -406,7 +406,7 @@ func (c *problemCollector) Collect(ch chan<- prometheus.Metric) {
 
 	ch <- prometheus.MustNewConstMetric(c.problems, prometheus.GaugeValue, float64(len(done)))
 
-	sort.Strings(keys)
+	slices.Sort(keys)
 	var reported int
 	for _, key := range keys {
 		ch <- done[key]
diff --git a/internal/checks/promql_vector_matching.go b/internal/checks/promql_vector_matching.go
index 3d693474..1b62d6db 100644
--- a/internal/checks/promql_vector_matching.go
+++ b/internal/checks/promql_vector_matching.go
@@ -4,7 +4,6 @@ import (
 	"context"
 	"fmt"
 	"slices"
-	"sort"
 	"strings"
 
 	"github.com/prometheus/common/model"
@@ -243,7 +242,7 @@ func (c VectorMatchingCheck) seriesLabels(ctx context.Context, query string, ign
 			ls.add(l.Name)
 		})
 		if len(ls.names) > 1 {
-			sort.Strings(ls.names)
+			slices.Sort(ls.names)
 		}
 		lsets = append(lsets, ls)
 	}
diff --git a/internal/checks/rule_dependency.go b/internal/checks/rule_dependency.go
index 97abc673..160382f0 100644
--- a/internal/checks/rule_dependency.go
+++ b/internal/checks/rule_dependency.go
@@ -1,9 +1,10 @@
 package checks
 
 import (
+	"cmp"
 	"context"
 	"fmt"
-	"sort"
+	"slices"
 	"strconv"
 	"strings"
 
@@ -83,14 +84,12 @@ func (c RuleDependencyCheck) Check(_ context.Context, path discovery.Path, rule
 		return problems
 	}
 
-	sort.Slice(broken, func(i, j int) bool {
-		if broken[i].path != broken[j].path {
-			return broken[i].path < broken[j].path
-		}
-		if broken[i].line != broken[j].line {
-			return broken[i].line < broken[j].line
-		}
-		return broken[i].name < broken[j].name
+	slices.SortFunc(broken, func(a, b *brokenDependency) int {
+		return cmp.Or(
+			cmp.Compare(a.path, b.path),
+			cmp.Compare(a.line, b.line),
+			cmp.Compare(a.name, b.name),
+		)
 	})
 
 	var details strings.Builder
diff --git a/internal/output/ranges.go b/internal/output/ranges.go
index a747ece6..4300e2de 100644
--- a/internal/output/ranges.go
+++ b/internal/output/ranges.go
@@ -2,7 +2,7 @@ package output
 
 import (
 	"fmt"
-	"sort"
+	"slices"
 	"strconv"
 	"strings"
 )
@@ -10,7 +10,7 @@ import (
 func FormatLineRangeString(lines []int) string {
 	ls := make([]int, len(lines))
 	copy(ls, lines)
-	sort.Ints(ls)
+	slices.Sort(ls)
 
 	var ranges []string
 	start := -1
diff --git a/internal/promapi/range_normalize.go b/internal/promapi/range_normalize.go
index 9a4fde8f..a7631933 100644
--- a/internal/promapi/range_normalize.go
+++ b/internal/promapi/range_normalize.go
@@ -2,6 +2,7 @@ package promapi
 
 import (
 	"fmt"
+	"slices"
 	"sort"
 	"strings"
 	"time"
@@ -35,7 +36,7 @@ func labelsBefore(ls, o labels.Labels) bool {
 	o.Range(func(l labels.Label) {
 		lns = append(lns, l.Name)
 	})
-	sort.Strings(lns)
+	slices.Sort(lns)
 	for _, ln := range lns {
 		mlv, ok := labelValue(ls, ln)
 		if !ok {
diff --git a/internal/reporter/console.go b/internal/reporter/console.go
index 25c172c9..3c301541 100644
--- a/internal/reporter/console.go
+++ b/internal/reporter/console.go
@@ -5,7 +5,7 @@ import (
 	"io"
 	"log/slog"
 	"os"
-	"sort"
+	"slices"
 	"strings"
 
 	"github.com/fatih/color"
@@ -23,10 +23,8 @@ type ConsoleReporter struct {
 }
 
 func (cr ConsoleReporter) Submit(summary Summary) (err error) {
-	reports := sortReports(summary.Reports())
-
 	perFile := map[string][]string{}
-	for _, report := range reports {
+	for _, report := range summary.Reports() {
 		if report.Problem.Severity < cr.minSeverity {
 			continue
 		}
@@ -88,7 +86,7 @@ func (cr ConsoleReporter) Submit(summary Summary) (err error) {
 	for path := range perFile {
 		paths = append(paths, path)
 	}
-	sort.Strings(paths)
+	slices.Sort(paths)
 
 	for _, path := range paths {
 		msgs := perFile[path]
@@ -122,28 +120,3 @@ func countDigits(n int) (c int) {
 	}
 	return c
 }
-
-func sortReports(reports []Report) []Report {
-	sort.SliceStable(reports, func(i, j int) bool {
-		if reports[i].Path.Name < reports[j].Path.Name {
-			return true
-		}
-		if reports[i].Path.Name > reports[j].Path.Name {
-			return false
-		}
-		if reports[i].Problem.Lines.First < reports[j].Problem.Lines.First {
-			return true
-		}
-		if reports[i].Problem.Lines.First > reports[j].Problem.Lines.First {
-			return false
-		}
-		if reports[i].Problem.Reporter < reports[j].Problem.Reporter {
-			return true
-		}
-		if reports[i].Problem.Reporter > reports[j].Problem.Reporter {
-			return false
-		}
-		return reports[i].Problem.Text < reports[j].Problem.Text
-	})
-	return reports
-}
diff --git a/internal/reporter/json.go b/internal/reporter/json.go
index 3e431f94..06f420de 100644
--- a/internal/reporter/json.go
+++ b/internal/reporter/json.go
@@ -24,7 +24,7 @@ type JSONReport struct {
 }
 
 func (jr JSONReporter) Submit(summary Summary) (err error) {
-	reports := sortReports(summary.Reports())
+	reports := summary.Reports()
 	out := make([]JSONReport, 0, len(reports))
 
 	for _, report := range reports {
diff --git a/internal/reporter/reporter.go b/internal/reporter/reporter.go
index 8d0d44ef..ae014477 100644
--- a/internal/reporter/reporter.go
+++ b/internal/reporter/reporter.go
@@ -1,7 +1,8 @@
 package reporter
 
 import (
-	"sort"
+	"cmp"
+	"slices"
 	"time"
 
 	"github.com/cloudflare/pint/internal/checks"
@@ -79,20 +80,15 @@ func (s Summary) hasReport(r Report) bool {
 }
 
 func (s *Summary) SortReports() {
-	sort.SliceStable(s.reports, func(i, j int) bool {
-		if s.reports[i].Path.SymlinkTarget != s.reports[j].Path.SymlinkTarget {
-			return s.reports[i].Path.SymlinkTarget < s.reports[j].Path.SymlinkTarget
-		}
-		if s.reports[i].Path.Name != s.reports[j].Path.Name {
-			return s.reports[i].Path.Name < s.reports[j].Path.Name
-		}
-		if s.reports[i].Problem.Lines.First != s.reports[j].Problem.Lines.First {
-			return s.reports[i].Problem.Lines.First < s.reports[j].Problem.Lines.First
-		}
-		if s.reports[i].Problem.Reporter != s.reports[j].Problem.Reporter {
-			return s.reports[i].Problem.Reporter < s.reports[j].Problem.Reporter
-		}
-		return s.reports[i].Problem.Text < s.reports[j].Problem.Text
+	slices.SortFunc(s.reports, func(a, b Report) int {
+		return cmp.Or(
+			cmp.Compare(a.Path.Name, b.Path.Name),
+			cmp.Compare(a.Problem.Lines.First, b.Problem.Lines.First),
+			cmp.Compare(a.Problem.Lines.Last, b.Problem.Lines.Last),
+			cmp.Compare(a.Problem.Severity, b.Problem.Severity),
+			cmp.Compare(a.Problem.Reporter, b.Problem.Reporter),
+			cmp.Compare(a.Problem.Text, b.Problem.Text),
+		)
 	})
 }