Skip to content

Commit 003656b

Browse files
prymitiveprymitive
authored
Merge pull request #1433 from cloudflare/gitlab
Fix incorrect line number when reporting problems on deleted lines
2 parents 40d0a35 + dd5609e commit 003656b

File tree

6 files changed

+139
-10
lines changed

6 files changed

+139
-10
lines changed

‎cmd/pint/tests/0162_ci_deleted_dependency.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ cmp stderr ../stderr.txt
2323

2424
-- stderr.txt --
2525
Warning: rule results used by another rule (rule/dependency)
26-
---> rules.yml:4-5 (deleted) -> `up:sum`
26+
---> rules.yml:4 (deleted) -> `up:sum`
2727

2828
-- src/alert.yml --
2929
groups:

‎cmd/pint/tests/0213_ci_removed_rule.txt‎

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
http method gitlab GET /api/v4/user 200 {"id": 555}
2+
http method gitlab GET /api/v4/projects/1234/merge_requests/1/versions 200 [{"id": 2, "head_commit_sha": "head", "base_commit_sha": "base", "start_commit_sha": "start"},{"id": 1, "head_commit_sha": "head", "base_commit_sha": "base", "start_commit_sha": "start"}]
3+
http method gitlab GET /api/v4/projects/1234/merge_requests/1/diffs 200 [{"old_path":"rules.yml","new_path":"rules.yml","diff":"@@ -1,7 +1,7 @@\n groups:\n - name: foo\n rules:\n- - record: rule1\n+ - record: rule3\n expr: sum(foo) by(job)\n - record: rule2\n expr: sum(rule1) by(job)\n"}]
4+
http method gitlab GET /api/v4/projects/1234/merge_requests/1/discussions 200 []
5+
http method gitlab GET /api/v4/projects/1234/merge_requests 200 [{"iid": 1}]
6+
http method gitlab POST /api/v4/projects/1234/merge_requests/1/discussions 200 {}
7+
http start gitlab 127.0.0.1:6213
8+
9+
mkdir testrepo
10+
cd testrepo
11+
exec git init --initial-branch=main .
12+
13+
cp ../src/v1.yml rules.yml
14+
cp ../src/.pint.hcl .
15+
env GIT_AUTHOR_NAME=pint
16+
env GIT_AUTHOR_EMAIL=pint@example.com
17+
env GIT_COMMITTER_NAME=pint
18+
env GIT_COMMITTER_EMAIL=pint@example.com
19+
exec git add .
20+
exec git commit -am 'import rules and config'
21+
22+
exec git checkout -b v2
23+
cp ../src/v2.yml rules.yml
24+
exec git commit -am 'v2'
25+
26+
env GITLAB_AUTH_TOKEN=secret
27+
exec pint -l debug --offline --no-color ci
28+
! stdout .
29+
cmp gitlab.got ../gitlab.expected
30+
31+
-- src/v1.yml --
32+
groups:
33+
- name: foo
34+
rules:
35+
- record: rule1
36+
expr: sum(foo) by(job)
37+
- record: rule2
38+
expr: sum(rule1) by(job)
39+
40+
-- src/v2.yml --
41+
groups:
42+
- name: foo
43+
rules:
44+
- record: rule3
45+
expr: sum(foo) by(job)
46+
- record: rule2
47+
expr: sum(rule1) by(job)
48+
49+
-- src/.pint.hcl --
50+
ci {
51+
baseBranch = "main"
52+
}
53+
repository {
54+
gitlab {
55+
uri = "http://127.0.0.1:6213"
56+
timeout = "30s"
57+
project = "1234"
58+
}
59+
}
60+
61+
-- gitlab.expected --
62+
GET /api/v4/user
63+
Accept: application/json
64+
Accept-Encoding: gzip
65+
Private-Token: secret
66+
67+
GET /api/v4/projects/1234/merge_requests
68+
Accept: application/json
69+
Accept-Encoding: gzip
70+
Private-Token: secret
71+
72+
GET /api/v4/projects/1234/merge_requests/1/versions
73+
Accept: application/json
74+
Accept-Encoding: gzip
75+
Private-Token: secret
76+
77+
GET /api/v4/projects/1234/merge_requests/1/diffs
78+
Accept: application/json
79+
Accept-Encoding: gzip
80+
Private-Token: secret
81+
82+
GET /api/v4/projects/1234/merge_requests/1/discussions
83+
Accept: application/json
84+
Accept-Encoding: gzip
85+
Private-Token: secret
86+
87+
POST /api/v4/projects/1234/merge_requests/1/discussions
88+
Accept: application/json
89+
Accept-Encoding: gzip
90+
Content-Type: application/json
91+
Private-Token: secret
92+
--- BODY ---
93+
body: |
94+
:warning: **Warning** reported by [pint](https://cloudflare.github.io/pint/) **rule/dependency** check.
95+
96+
------
97+
98+
rule results used by another rule
99+
100+
<details>
101+
<summary>More information</summary>
102+
If you remove the recording rule generating `rule1`, and there is no other source of this metric, then any other rule depending on it will break.
103+
List of found rules that are using `rule1`:
104+
105+
- `rule2` at `rules.yml:7`
106+
107+
</details>
108+
109+
------
110+
111+
:information_source: To see documentation covering this check and instructions on how to resolve it [click here](https://cloudflare.github.io/pint/checks/rule/dependency.html).
112+
position:
113+
base_sha: base
114+
head_sha: head
115+
start_sha: start
116+
new_path: rules.yml
117+
old_path: rules.yml
118+
position_type: text
119+
old_line: 4
120+
--- END ---
121+

‎docs/changelog.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
# Changelog
22

3+
## v0.73.7
4+
5+
### Fixed
6+
7+
- Fixed a bug in GitLab integration causing incorrect line numbers to be used for comments.
8+
39
## v0.73.6
410

511
### Fixed

‎internal/checks/rule_dependency.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ func (c RuleDependencyCheck) Check(_ context.Context, entry discovery.Entry, ent
115115
name := entry.Rule.NameNode()
116116
problems = append(problems, Problem{
117117
Anchor: AnchorBefore,
118-
Lines: entry.Rule.Lines,
118+
Lines: name.Pos.Lines(),
119119
Reporter: c.Reporter(),
120120
Summary: "rule results used by another rule",
121121
Details: details.String(),

‎internal/checks/rule_dependency_test.snap‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
lastcolumn: 3
3030
lines:
3131
first: 1
32-
last: 2
32+
last: 1
3333
severity: 1
3434
anchor: 1
3535

@@ -101,7 +101,7 @@
101101
lastcolumn: 3
102102
lines:
103103
first: 1
104-
last: 2
104+
last: 1
105105
severity: 1
106106
anchor: 1
107107

@@ -133,7 +133,7 @@
133133
lastcolumn: 12
134134
lines:
135135
first: 1
136-
last: 2
136+
last: 1
137137
severity: 1
138138
anchor: 1
139139

@@ -165,7 +165,7 @@
165165
lastcolumn: 12
166166
lines:
167167
first: 1
168-
last: 2
168+
last: 1
169169
severity: 1
170170
anchor: 1
171171

@@ -197,7 +197,7 @@
197197
lastcolumn: 3
198198
lines:
199199
first: 1
200-
last: 2
200+
last: 1
201201
severity: 1
202202
anchor: 1
203203

@@ -229,7 +229,7 @@
229229
lastcolumn: 3
230230
lines:
231231
first: 1
232-
last: 2
232+
last: 1
233233
severity: 1
234234
anchor: 1
235235

‎internal/reporter/gitlab.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -545,17 +545,19 @@ func parseDiffLines(diff string) (lines []diffLine) {
545545
matches := diffRe.FindStringSubmatch(line)
546546
if len(matches) == 5 {
547547
oldLine, _ = strconv.Atoi(matches[1])
548+
oldLine--
548549
newLine, _ = strconv.Atoi(matches[3])
550+
newLine--
549551
}
550552
case strings.HasPrefix(line, "-"):
551553
oldLine++
552554
case strings.HasPrefix(line, "+"):
553-
lines = append(lines, diffLine{old: oldLine, new: newLine, wasModified: true})
554555
newLine++
556+
lines = append(lines, diffLine{old: oldLine, new: newLine, wasModified: true})
555557
default:
556-
lines = append(lines, diffLine{old: oldLine, new: newLine, wasModified: false})
557558
oldLine++
558559
newLine++
560+
lines = append(lines, diffLine{old: oldLine, new: newLine, wasModified: false})
559561
}
560562
}
561563

0 commit comments

Comments
 (0)