Point at matcher selectors

cloudflare · Mar 10, 2025 · 0046810 · 0046810
1 parent 4c66344
commit 0046810
Show file tree

Hide file tree

Showing 6 changed files with 21 additions and 16 deletions.
diff --git a/cmd/pint/tests/0003_lint_workdir.txt b/cmd/pint/tests/0003_lint_workdir.txt
@@ -21,12 +21,12 @@ Warning: label must be removed in aggregations (promql/aggregate)
 Warning: redundant regexp (promql/regexp)
   ---> rules/0002.yaml:2
 2 |   expr: up{job=~"foo"} == 0
-            ^^^^^^^^^^^^^^ Unnecessary regexp match on static string `job=~"foo"`, use `job="foo"` instead.
+               ^^^^^^^^^^ Unnecessary regexp match on static string `job=~"foo"`, use `job="foo"` instead.
 
 Warning: redundant regexp (promql/regexp)
   ---> rules/0002.yaml:5
 5 |   expr: up{job!~"foo"} == 0
-            ^^^^^^^^^^^^^^ Unnecessary regexp match on static string `job!~"foo"`, use `job!="foo"` instead.
+               ^^^^^^^^^^ Unnecessary regexp match on static string `job!~"foo"`, use `job!="foo"` instead.
 
 Warning: label must be removed in aggregations (promql/aggregate)
   ---> rules/0003.yaml:11

diff --git a/cmd/pint/tests/0069_bitbucket_unmodified.txt b/cmd/pint/tests/0069_bitbucket_unmodified.txt
@@ -33,7 +33,7 @@ Warning: always firing alert (alerts/comparison)
 Warning: redundant regexp (promql/regexp)
   ---> rules.yml:2
 2 |   expr: sum(foo{job=~"xxx"}) by(job)
-                ^^^^^^^^^^^^^^^ Unnecessary regexp match on static string `job=~"xxx"`, use `job="xxx"` instead.
+                    ^^^^^^^^^^ Unnecessary regexp match on static string `job=~"xxx"`, use `job="xxx"` instead.
 
 Information: redundant field with default value (alerts/for)
   ---> rules.yml:3
@@ -48,7 +48,7 @@ Warning: always firing alert (alerts/comparison)
 Warning: redundant regexp (promql/regexp)
   ---> rules.yml:5
 5 |   expr: sum(foo{job=~"xxx"}) by(job)
-                ^^^^^^^^^^^^^^^ Unnecessary regexp match on static string `job=~"xxx"`, use `job="xxx"` instead.
+                    ^^^^^^^^^^ Unnecessary regexp match on static string `job=~"xxx"`, use `job="xxx"` instead.
 
 Information: redundant field with default value (alerts/for)
   ---> rules.yml:6

diff --git a/cmd/pint/tests/0076_ci_group_errors.txt b/cmd/pint/tests/0076_ci_group_errors.txt
@@ -400,7 +400,7 @@ Bug: missing owner (rule/owner)
 Warning: redundant regexp (promql/regexp)
   ---> rules.yml:36
 36 |     expr: sum(no_such_metric{job=~"fake"})
-                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^ Unnecessary regexp match on static string `job=~"fake"`, use `job="fake"` instead.
+                                  ^^^^^^^^^^^ Unnecessary regexp match on static string `job=~"fake"`, use `job="fake"` instead.
 
 Bug: required label is being removed via aggregation (promql/aggregate)
   ---> rules.yml:36

diff --git a/cmd/pint/tests/0125_lint_fail_on_warning.txt b/cmd/pint/tests/0125_lint_fail_on_warning.txt
@@ -20,7 +20,7 @@ Warning: always firing alert (alerts/comparison)
 Warning: redundant regexp (promql/regexp)
   ---> rules/0001.yml:5
 5 |       expr: up{job=~"xxx"}
-                ^^^^^^^^^^^^^^ Unnecessary regexp match on static string `job=~"xxx"`, use `job="xxx"` instead.
+                   ^^^^^^^^^^ Unnecessary regexp match on static string `job=~"xxx"`, use `job="xxx"` instead.
 
 level=INFO msg="Problems found" Warning=2
 level=ERROR msg="Execution completed with error(s)" err="found 1 problem(s) with severity Warning or higher"
diff --git a/internal/checks/promql_regexp.go b/internal/checks/promql_regexp.go
@@ -205,6 +205,7 @@ func (c RegexpCheck) Check(ctx context.Context, _ discovery.Path, rule parser.Ru
 					b.lm, b.lm.Name, b.op, b.lm.Value)
 
 			}
+			pos := findMatcherPos(expr.Value.Value, b.pos, b.lm)
 			problems = append(problems, Problem{
 				Anchor:   AnchorAfter,
 				Lines:    expr.Value.Lines,
@@ -216,8 +217,8 @@ func (c RegexpCheck) Check(ctx context.Context, _ discovery.Path, rule parser.Ru
 					{
 						Message:     text,
 						Pos:         expr.Value.Pos,
-						FirstColumn: int(b.pos.Start) + 1,
-						LastColumn:  int(b.pos.End),
+						FirstColumn: int(pos.Start) + 1,
+						LastColumn:  int(pos.End) + 1,
 					},
 				},
 			})
@@ -270,3 +271,15 @@ func isOpSmelly(a, b syntax.Op) bool {
 	}
 	return false
 }
+
+func findMatcherPos(expr string, within posrange.PositionRange, m *labels.Matcher) posrange.PositionRange {
+	re := regexp.MustCompile("(" + m.Name + ")(?: *)" + m.Type.String() + "(?: *)" + `"` + m.Value + `"`)
+	idx := re.FindStringSubmatchIndex(utils.GetQueryFragment(expr, within))
+	if idx == nil {
+		return within
+	}
+	return posrange.PositionRange{
+		Start: within.Start + posrange.Pos(idx[0]),
+		End:   within.Start + posrange.Pos(idx[1]-1),
+	}
+}
diff --git a/internal/diags/position.go b/internal/diags/position.go
@@ -154,14 +154,6 @@ END:
 	return offsets
 }
 
-func NewPositionForLine(lines []string, line int) (out PositionRanges) {
-	return append(out, PositionRange{
-		Line:        line,
-		FirstColumn: 1,
-		LastColumn:  len(lines[line-1]),
-	})
-}
-
 func countLeadingSpace(line string) (i int) {
 	for _, r := range line {
 		if r != ' ' {