Ensure we don't report problems from unmodified lines

cmd/pint/ci.go

@@ -110,7 +110,7 @@ func actionCI(c *cli.Context) error {
 	}
 
 	foundBugOrHigher := false
-	bySeverity := map[string]interface{}{}
+	bySeverity := map[string]interface{}{} // interface{} is needed for log.Fields()
 	for s, c := range summary.CountBySeverity() {
 		if s >= checks.Bug {
 			foundBugOrHigher = true

cmd/pint/lint.go

@@ -60,7 +60,7 @@ func actionLint(c *cli.Context) error {
 		return err
 	}
 
-	bySeverity := map[string]interface{}{}
+	bySeverity := map[string]interface{}{} // interface{} is needed for log.Fields()
 	var problems int
 	for s, c := range summary.CountBySeverity() {
 		bySeverity[s.String()] = c

cmd/pint/tests/0055_prometheus_failover.txt

@@ -5,6 +5,7 @@ pint.error --no-color lint rules
 ! stdout .
 stderr 'level=error msg="Query failed" error="Post \\"http://127.0.0.1:1055/api/v1/query\\": dial tcp 127.0.0.1:1055: connect: connection refused" query=count\(foo\) uri=http://127.0.0.1:1055'
 stderr 'level=error msg="Failed to query Prometheus configuration" error="Get \\"http://127.0.0.1:1055/api/v1/status/config\\": dial tcp 127.0.0.1:1055: connect: connection refused" uri=http://127.0.0.1:1055'
+! stderr 'query="count\(foo offset '
 stderr 'rules/1.yml:2: prometheus "prom" at http://127.0.0.1:7055 didn''t have any series for "foo" metric in the last 1w \(promql/series\)'
 exec bash -c 'cat prometheus.pid | xargs kill'
 

cmd/pint/tests/0069_bitbucket_skip_unmodified.txt

@@ -0,0 +1,111 @@
+exec bash -x ./webserver.sh &
+exec bash -c 'I=0 ; while [ ! -f server.pid ] && [ $I -lt 30 ]; do sleep 1; I=$((I+1)); done'
+
+mkdir testrepo
+cd testrepo
+exec git init --initial-branch=main .
+
+cp ../src/v1.yml rules.yml
+cp ../src/.pint.hcl .
+env GIT_AUTHOR_NAME=pint
+env GIT_AUTHOR_EMAIL=pint@example.com
+env GIT_COMMITTER_NAME=pint
+env GIT_COMMITTER_EMAIL=pint@example.com
+exec git add .
+exec git commit -am 'import rules and config'
+
+exec git checkout -b v2
+cp ../src/v2.yml rules.yml
+exec git commit -am 'v2'
+
+env BITBUCKET_AUTH_TOKEN="12345"
+pint.ok --no-color ci
+! stdout .
+stderr 'level=info msg="Problems found" Information=1'
+
+exec sh -c 'cat ../server.pid | xargs kill'
+-- src/v1.yml --
+- alert: rule1a
+  expr: sum(foo{job=~"xxx"}) by(job)
+- alert: rule2a
+  expr: sum(foo{job=~"xxx"}) by(job)
+  for: 0s
+
+-- src/v2.yml --
+- alert: rule1b
+  expr: sum(foo{job=~"xxx"}) by(job)
+  for: 0s
+- alert: rule2b
+  expr: sum(foo{job=~"xxx"}) by(job)
+  for: 0s
+
+-- src/.pint.hcl --
+parser {
+  relaxed = [".*"]
+}
+ci {
+  baseBranch = "main"
+}
+repository {
+  bitbucket {
+    uri        = "http://127.0.0.1:6069"
+    timeout    = "10s"
+    project    = "prometheus"
+    repository = "rules"
+  }
+}
+
+-- webserver.go --
+package main
+
+import (
+	"context"
+	"io"
+	"log"
+	"net"
+	"net/http"
+	"os"
+	"os/signal"
+	"strconv"
+	"syscall"
+	"time"
+)
+
+func main() {
+	http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
+		io.WriteString(w, "OK")
+	})
+
+	listener, err := net.Listen("tcp", "127.0.0.1:6069")
+	if err != nil {
+		log.Fatal(err)
+	}
+
+	server := &http.Server{
+		Addr: "127.0.0.1:6069",
+	}
+
+	go func() {
+		_ = server.Serve(listener)
+	}()
+
+	pid := os.Getpid()
+	err = os.WriteFile("server.pid", []byte(strconv.Itoa(pid)), 0644)
+	if err != nil {
+		log.Fatal(err)
+	}
+
+	stop := make(chan os.Signal, 1)
+	signal.Notify(stop, os.Interrupt, syscall.SIGINT, syscall.SIGTERM)
+	go func() {
+		time.Sleep(time.Minute*2)
+		stop <- syscall.SIGTERM
+	}()
+	<-stop
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+	defer cancel()
+	server.Shutdown(ctx)
+}
+
+-- webserver.sh --
+env GOCACHE=$TMPDIR go run webserver.go

docs/changelog.md

@@ -36,6 +36,7 @@
 ### Fixed
 
 - Improved `promql/vector_matching` checks to detect more issues.
+- Fixed reporting of problems detected on unmodified lines when running `pint ci`.
 
 ## v0.16.1
 

internal/discovery/glob.go

@@ -50,11 +50,14 @@ func (f GlobFinder) Find() (entries []Entry, err error) {
 	}
 
 	for _, path := range paths {
-		e, err := readFile(path, !matchesAny(f.relaxed, path))
+		el, err := readFile(path, !matchesAny(f.relaxed, path))
 		if err != nil {
 			return nil, fmt.Errorf("invalid file syntax: %w", err)
 		}
-		entries = append(entries, e...)
+		for _, e := range el {
+			e.ModifiedLines = e.Rule.Lines()
+			entries = append(entries, e)
+		}
 	}
 
 	return entries, nil

internal/discovery/glob_test.go

@@ -66,9 +66,10 @@ func TestGlobPathFinder(t *testing.T) {
 			finder: discovery.NewGlobFinder([]string{"*"}, []*regexp.Regexp{regexp.MustCompile(".*")}),
 			entries: []discovery.Entry{
 				{
-					Path:  "bar.yml",
-					Rule:  testRules[0],
-					Owner: "bob",
+					Path:          "bar.yml",
+					Rule:          testRules[0],
+					ModifiedLines: testRules[0].Lines(),
+					Owner:         "bob",
 				},
 			},
 		},
@@ -77,9 +78,10 @@ func TestGlobPathFinder(t *testing.T) {
 			finder: discovery.NewGlobFinder([]string{"*"}, []*regexp.Regexp{regexp.MustCompile(".*")}),
 			entries: []discovery.Entry{
 				{
-					Path:  "foo/bar.yml",
-					Rule:  testRules[0],
-					Owner: "alice",
+					Path:          "foo/bar.yml",
+					Rule:          testRules[0],
+					ModifiedLines: testRules[0].Lines(),
+					Owner:         "alice",
 				},
 			},
 		},
@@ -88,9 +90,10 @@ func TestGlobPathFinder(t *testing.T) {
 			finder: discovery.NewGlobFinder([]string{"*"}, nil),
 			entries: []discovery.Entry{
 				{
-					Path:      "bar.yml",
-					PathError: strictErr,
-					Owner:     "bob",
+					Path:          "bar.yml",
+					PathError:     strictErr,
+					ModifiedLines: []int{0},
+					Owner:         "bob",
 				},
 			},
 		},

internal/reporter/bitbucket.go

@@ -10,6 +10,7 @@ import (
 
 	"github.com/cloudflare/pint/internal/checks"
 	"github.com/cloudflare/pint/internal/git"
+	"github.com/cloudflare/pint/internal/output"
 
 	"github.com/rs/zerolog/log"
 )
@@ -93,26 +94,16 @@ func (r BitBucketReporter) Submit(summary Summary) (err error) {
 }
 
 func (r BitBucketReporter) makeAnnotation(report Report, pb git.FileBlames) (annotations []BitBucketAnnotation) {
-	reportLine := -1
-	for _, pl := range report.Problem.Lines {
-		for _, ml := range report.ModifiedLines {
-			if pl == ml {
-				reportLine = pl
-			}
-		}
-	}
-	if reportLine < 0 && report.Problem.Severity == checks.Fatal {
-		for _, ml := range report.ModifiedLines {
-			reportLine = ml
-			break
-		}
-	}
-
-	if reportLine < 0 {
-		log.Debug().Str("path", report.Path).Msg("No file line found, skipping")
+	if !shouldReport(report) {
+		log.Debug().
+			Str("path", report.Path).
+			Str("lines", output.FormatLineRangeString(report.Problem.Lines)).
+			Msg("Problem reported on unmodified line, skipping")
 		return
 	}
 
+	reportLine := reportedLine(report)
+
 	var severity, atype string
 	switch report.Problem.Severity {
 	case checks.Fatal:

internal/reporter/console.go

@@ -12,6 +12,7 @@ import (
 	"github.com/rs/zerolog/log"
 
 	"github.com/cloudflare/pint/internal/checks"
+	"github.com/cloudflare/pint/internal/output"
 )
 
 func NewConsoleReporter(output io.Writer) ConsoleReporter {
@@ -49,6 +50,14 @@ func (cr ConsoleReporter) Submit(summary Summary) error {
 
 	perFile := map[string][]string{}
 	for _, report := range reps {
+		if !shouldReport(report) {
+			log.Debug().
+				Str("path", report.Path).
+				Str("lines", output.FormatLineRangeString(report.Problem.Lines)).
+				Msg("Problem reported on unmodified line, skipping")
+			continue
+		}
+
 		if _, ok := perFile[report.Path]; !ok {
 			perFile[report.Path] = []string{}
 		}

internal/reporter/reporter.go

@@ -30,6 +30,9 @@ func (s Summary) HasFatalProblems() bool {
 func (s Summary) CountBySeverity() map[checks.Severity]int {
 	m := map[checks.Severity]int{}
 	for _, report := range s.Reports {
+		if !shouldReport(report) {
+			continue
+		}
 		if _, ok := m[report.Problem.Severity]; !ok {
 			m[report.Problem.Severity] = 0
 		}
@@ -55,3 +58,38 @@ func blameReports(reports []Report, gitCmd git.CommandRunner) (pb git.FileBlames
 	}
 	return
 }
+
+func shouldReport(report Report) bool {
+	if report.Problem.Severity == checks.Fatal {
+		return true
+	}
+
+	for _, pl := range report.Problem.Lines {
+		for _, ml := range report.ModifiedLines {
+			if pl == ml {
+				return true
+			}
+		}
+	}
+
+	return false
+}
+
+func reportedLine(report Report) (l int) {
+	l = -1
+	for _, pl := range report.Problem.Lines {
+		for _, ml := range report.ModifiedLines {
+			if pl == ml {
+				l = pl
+			}
+		}
+	}
+
+	if l < 0 && report.Problem.Severity == checks.Fatal {
+		for _, ml := range report.ModifiedLines {
+			return ml
+		}
+	}
+
+	return
+}