From 00468104eb2c2bce3c5fde9f1f44e0100998836c Mon Sep 17 00:00:00 2001 From: Lukasz Mierzwa Date: Mon, 10 Mar 2025 11:59:11 +0000 Subject: [PATCH] Point at matcher selectors --- cmd/pint/tests/0003_lint_workdir.txt | 4 ++-- cmd/pint/tests/0069_bitbucket_unmodified.txt | 4 ++-- cmd/pint/tests/0076_ci_group_errors.txt | 2 +- cmd/pint/tests/0125_lint_fail_on_warning.txt | 2 +- internal/checks/promql_regexp.go | 17 +++++++++++++++-- internal/diags/position.go | 8 -------- 6 files changed, 21 insertions(+), 16 deletions(-) diff --git a/cmd/pint/tests/0003_lint_workdir.txt b/cmd/pint/tests/0003_lint_workdir.txt index 6e7076fb..b5b644fd 100644 --- 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 index 15ab8067..b6450f66 100644 --- 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 index 3181f381..b7b0250c 100644 --- 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 index d9a3556f..7bc3d282 100644 --- 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 index a28abf82..3a4d1fd0 100644 --- 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 index 831c773f..1a4a2f3f 100644 --- 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 != ' ' {