Skip to content

Commit 64c56ae

Browse files
prymitiveprymitive
authored
Merge pull request #1471 from cloudflare/gitlab-diff
Fallback to git history if there's no diff
2 parents f464ca4 + 3422563 commit 64c56ae

File tree

8 files changed

+252
-26
lines changed

8 files changed

+252
-26
lines changed

‎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.74.2
4+
5+
### Fixed
6+
7+
- Fixed an issue where pint would fail to create MR comment if GitLab API returned empty diff body.
8+
39
## v0.74.1
410

511
### Fixed

‎internal/reporter/comments.go‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,11 @@ import (
1313
)
1414

1515
type PendingComment struct {
16-
path string
17-
text string
18-
line int
19-
anchor checks.Anchor
16+
path string
17+
text string
18+
line int
19+
anchor checks.Anchor
20+
modifiedLine bool
2021
}
2122

2223
type ExistingComment struct {
@@ -155,10 +156,11 @@ func makeComments(summary Summary, showDuplicates bool) (comments []PendingComme
155156
}
156157

157158
comments = append(comments, PendingComment{
158-
anchor: reports[0].Problem.Anchor,
159-
path: reports[0].Path.SymlinkTarget,
160-
line: line,
161-
text: buf.String(),
159+
anchor: reports[0].Problem.Anchor,
160+
path: reports[0].Path.SymlinkTarget,
161+
line: line,
162+
text: buf.String(),
163+
modifiedLine: slices.Contains(reports[0].ModifiedLines, line),
162164
})
163165
}
164166
return comments

‎internal/reporter/gitlab.go‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -456,6 +456,24 @@ func reportToGitLabDiscussion(pending PendingComment, diffs []*gitlab.MergeReque
456456
renamed = true
457457
}
458458

459+
// Diff is empty, decide if it was modified based on git history.
460+
if diff.Diff == "" {
461+
slog.Debug("Empty diff from GitLab",
462+
slog.String("path", pending.path),
463+
slog.Int("line", pending.line),
464+
slog.Bool("modified", pending.modifiedLine))
465+
switch {
466+
case pending.anchor == checks.AnchorBefore:
467+
d.Position.OldLine = gitlab.Ptr(pending.line)
468+
case pending.modifiedLine:
469+
d.Position.NewLine = gitlab.Ptr(pending.line)
470+
case !pending.modifiedLine:
471+
d.Position.NewLine = gitlab.Ptr(pending.line)
472+
d.Position.OldLine = gitlab.Ptr(pending.line)
473+
}
474+
continue
475+
}
476+
459477
dl, ok := diffLineFor(parseDiffLines(diff.Diff), pending.line)
460478
switch {
461479
case !ok:

‎internal/reporter/gitlab_test.go‎

Lines changed: 200 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -978,6 +978,206 @@ Below is the list of checks that were disabled for each Prometheus server define
978978
return err
979979
},
980980
},
981+
{
982+
description: "rule moved to a different file, old deleted, no diff",
983+
timeout: time.Minute,
984+
maxComments: 1,
985+
summary: reporter.NewSummary([]reporter.Report{
986+
{
987+
Path: discovery.Path{
988+
SymlinkTarget: mockPath,
989+
Name: mockPath,
990+
},
991+
ModifiedLines: []int{2, 3},
992+
Rule: mockFile.Groups[0].Rules[0],
993+
Problem: checks.Problem{
994+
Reporter: "a",
995+
Summary: "foo error1",
996+
Details: "foo details",
997+
Diagnostics: []diags.Diagnostic{
998+
{
999+
Message: "Diagnostic message",
1000+
Pos: diags.PositionRanges{
1001+
{
1002+
Line: 2,
1003+
FirstColumn: 3,
1004+
LastColumn: 24,
1005+
},
1006+
{
1007+
Line: 3,
1008+
FirstColumn: 3,
1009+
LastColumn: 15,
1010+
},
1011+
},
1012+
FirstColumn: 1,
1013+
LastColumn: 24,
1014+
Kind: diags.Issue,
1015+
},
1016+
},
1017+
Lines: diags.LineRange{First: 1, Last: 3},
1018+
Severity: checks.Fatal,
1019+
Anchor: checks.AnchorAfter,
1020+
},
1021+
},
1022+
}),
1023+
mock: httpmock.New(func(s *httpmock.Server) {
1024+
s.ExpectGet(apiUser).ReturnJSON(gitlab.User{ID: 123})
1025+
s.ExpectGet(apiOpenMergeRequests).ReturnJSON([]gitlab.BasicMergeRequest{
1026+
{IID: 1},
1027+
})
1028+
s.ExpectGet(apiVersions(1)).ReturnJSON([]gitlab.MergeRequestDiffVersion{
1029+
{ID: 2, HeadCommitSHA: "head", BaseCommitSHA: "base", StartCommitSHA: "start"},
1030+
{ID: 1, HeadCommitSHA: "head", BaseCommitSHA: "base", StartCommitSHA: "start"},
1031+
})
1032+
s.ExpectGet(apiDiffs(1)).ReturnJSON([]gitlab.MergeRequestDiff{
1033+
{
1034+
Diff: "",
1035+
NewPath: mockPath,
1036+
OldPath: "foo.old",
1037+
AMode: "100644",
1038+
BMode: "100644",
1039+
RenamedFile: true,
1040+
},
1041+
{
1042+
Diff: "",
1043+
NewPath: "foo.txt",
1044+
OldPath: "foo.txt",
1045+
},
1046+
})
1047+
s.ExpectGet(apiDiscussions(1, true)).ReturnJSON([]gitlab.Discussion{})
1048+
s.ExpectPost(apiDiscussions(1, false)).WithBodyJSON(gitlab.CreateMergeRequestDiscussionOptions{
1049+
Body: discBodyWithDiag("a", "foo error1", "foo details", "```yaml\n2 | - record: target is down\n3 | expr: up == 0\n ^^ \n```", "Diagnostic message"),
1050+
Position: gitlab.Ptr(gitlab.PositionOptions{
1051+
BaseSHA: gitlab.Ptr("base"),
1052+
StartSHA: gitlab.Ptr("start"),
1053+
HeadSHA: gitlab.Ptr("head"),
1054+
OldPath: gitlab.Ptr("foo.old"),
1055+
NewPath: gitlab.Ptr(mockPath),
1056+
PositionType: gitlab.Ptr("text"),
1057+
NewLine: gitlab.Ptr(3),
1058+
// Old file is gone so we don't have OldLine here at all
1059+
}),
1060+
}).ReturnJSON(gitlab.Response{})
1061+
}),
1062+
errorHandler: func(err error) error {
1063+
return err
1064+
},
1065+
},
1066+
{
1067+
description: "deleted rule, no diff",
1068+
timeout: time.Minute,
1069+
maxComments: 1,
1070+
summary: reporter.NewSummary([]reporter.Report{
1071+
{
1072+
Path: discovery.Path{
1073+
SymlinkTarget: mockPath,
1074+
Name: mockPath,
1075+
},
1076+
ModifiedLines: []int{2, 3},
1077+
Rule: mockFile.Groups[0].Rules[0],
1078+
Problem: checks.Problem{
1079+
Reporter: "a",
1080+
Summary: "foo error1",
1081+
Details: "foo details",
1082+
Diagnostics: []diags.Diagnostic{},
1083+
Lines: diags.LineRange{First: 1, Last: 3},
1084+
Severity: checks.Fatal,
1085+
Anchor: checks.AnchorBefore,
1086+
},
1087+
},
1088+
}),
1089+
mock: httpmock.New(func(s *httpmock.Server) {
1090+
s.ExpectGet(apiUser).ReturnJSON(gitlab.User{ID: 123})
1091+
s.ExpectGet(apiOpenMergeRequests).ReturnJSON([]gitlab.BasicMergeRequest{
1092+
{IID: 1},
1093+
})
1094+
s.ExpectGet(apiVersions(1)).ReturnJSON([]gitlab.MergeRequestDiffVersion{
1095+
{ID: 2, HeadCommitSHA: "head", BaseCommitSHA: "base", StartCommitSHA: "start"},
1096+
{ID: 1, HeadCommitSHA: "head", BaseCommitSHA: "base", StartCommitSHA: "start"},
1097+
})
1098+
s.ExpectGet(apiDiffs(1)).ReturnJSON([]gitlab.MergeRequestDiff{
1099+
{
1100+
Diff: "",
1101+
NewPath: mockPath,
1102+
OldPath: mockPath,
1103+
},
1104+
})
1105+
s.ExpectGet(apiDiscussions(1, true)).ReturnJSON([]gitlab.Discussion{})
1106+
s.ExpectPost(apiDiscussions(1, false)).WithBodyJSON(gitlab.CreateMergeRequestDiscussionOptions{
1107+
Body: discBody("a", "foo error1", "foo details"),
1108+
Position: gitlab.Ptr(gitlab.PositionOptions{
1109+
BaseSHA: gitlab.Ptr("base"),
1110+
StartSHA: gitlab.Ptr("start"),
1111+
HeadSHA: gitlab.Ptr("head"),
1112+
OldPath: gitlab.Ptr(mockPath),
1113+
NewPath: gitlab.Ptr(mockPath),
1114+
PositionType: gitlab.Ptr("text"),
1115+
OldLine: gitlab.Ptr(3),
1116+
}),
1117+
}).ReturnJSON(gitlab.Response{})
1118+
}),
1119+
errorHandler: func(err error) error {
1120+
return err
1121+
},
1122+
},
1123+
{
1124+
description: "unmodified rule, no diff",
1125+
timeout: time.Minute,
1126+
maxComments: 1,
1127+
summary: reporter.NewSummary([]reporter.Report{
1128+
{
1129+
Path: discovery.Path{
1130+
SymlinkTarget: mockPath,
1131+
Name: mockPath,
1132+
},
1133+
ModifiedLines: []int{4, 5},
1134+
Rule: mockFile.Groups[0].Rules[0],
1135+
Problem: checks.Problem{
1136+
Reporter: "a",
1137+
Summary: "foo error1",
1138+
Details: "foo details",
1139+
Diagnostics: []diags.Diagnostic{},
1140+
Lines: diags.LineRange{First: 1, Last: 3},
1141+
Severity: checks.Fatal,
1142+
Anchor: checks.AnchorAfter,
1143+
},
1144+
},
1145+
}),
1146+
mock: httpmock.New(func(s *httpmock.Server) {
1147+
s.ExpectGet(apiUser).ReturnJSON(gitlab.User{ID: 123})
1148+
s.ExpectGet(apiOpenMergeRequests).ReturnJSON([]gitlab.BasicMergeRequest{
1149+
{IID: 1},
1150+
})
1151+
s.ExpectGet(apiVersions(1)).ReturnJSON([]gitlab.MergeRequestDiffVersion{
1152+
{ID: 2, HeadCommitSHA: "head", BaseCommitSHA: "base", StartCommitSHA: "start"},
1153+
{ID: 1, HeadCommitSHA: "head", BaseCommitSHA: "base", StartCommitSHA: "start"},
1154+
})
1155+
s.ExpectGet(apiDiffs(1)).ReturnJSON([]gitlab.MergeRequestDiff{
1156+
{
1157+
Diff: "",
1158+
NewPath: mockPath,
1159+
OldPath: mockPath,
1160+
},
1161+
})
1162+
s.ExpectGet(apiDiscussions(1, true)).ReturnJSON([]gitlab.Discussion{})
1163+
s.ExpectPost(apiDiscussions(1, false)).WithBodyJSON(gitlab.CreateMergeRequestDiscussionOptions{
1164+
Body: discBody("a", "foo error1", "foo details"),
1165+
Position: gitlab.Ptr(gitlab.PositionOptions{
1166+
BaseSHA: gitlab.Ptr("base"),
1167+
StartSHA: gitlab.Ptr("start"),
1168+
HeadSHA: gitlab.Ptr("head"),
1169+
OldPath: gitlab.Ptr(mockPath),
1170+
NewPath: gitlab.Ptr(mockPath),
1171+
PositionType: gitlab.Ptr("text"),
1172+
OldLine: gitlab.Ptr(3),
1173+
NewLine: gitlab.Ptr(3),
1174+
}),
1175+
}).ReturnJSON(gitlab.Response{})
1176+
}),
1177+
errorHandler: func(err error) error {
1178+
return err
1179+
},
1180+
},
9811181
}
9821182

9831183
for _, tc := range testCases {

‎tools/benchstat/go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,5 +6,5 @@ tool golang.org/x/perf/cmd/benchstat
66

77
require (
88
github.com/aclements/go-moremath v0.0.0-20210112150236-f10218a38794 // indirect
9-
golang.org/x/perf v0.0.0-20250515181355-8f5f3abfb71a // indirect
9+
golang.org/x/perf v0.0.0-20250605212013-b481878a17be // indirect
1010
)

‎tools/benchstat/go.sum‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
github.com/aclements/go-moremath v0.0.0-20210112150236-f10218a38794 h1:xlwdaKcTNVW4PtpQb8aKA4Pjy0CdJHEqvFbAnvR5m2g=
22
github.com/aclements/go-moremath v0.0.0-20210112150236-f10218a38794/go.mod h1:7e+I0LQFUI9AXWxOfsQROs9xPhoJtbsyWcjJqDd4KPY=
3-
golang.org/x/perf v0.0.0-20250515181355-8f5f3abfb71a h1:LZ+rOVDO9PoCrtZeddvdjuVyUKSqFmmhAYn8YSGFs54=
4-
golang.org/x/perf v0.0.0-20250515181355-8f5f3abfb71a/go.mod h1:6pCFTYjTIch4e2WE6CGv6StYZvAl0TIipo3/hIa1y5o=
3+
golang.org/x/perf v0.0.0-20250605212013-b481878a17be h1:Oh/azZqQQCNCOOki6pNDJt6WOp5zqqEjBklun7f8Tqo=
4+
golang.org/x/perf v0.0.0-20250605212013-b481878a17be/go.mod h1:nWcMYnRm5u4K1K1uHna2Dr0Ly7nKHQZa2dpXixO3BZ8=

‎tools/betteralign/go.mod‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,13 @@ go 1.24.0
55
tool github.com/dkorunic/betteralign/cmd/betteralign
66

77
require (
8-
github.com/KimMachineGun/automemlimit v0.7.1 // indirect
9-
github.com/dkorunic/betteralign v0.7.0 // indirect
8+
github.com/KimMachineGun/automemlimit v0.7.2 // indirect
9+
github.com/dkorunic/betteralign v0.7.1 // indirect
1010
github.com/google/renameio/v2 v2.0.0 // indirect
1111
github.com/pbnjay/memory v0.0.0-20210728143218-7b4eea64cf58 // indirect
1212
github.com/sirkon/dst v0.26.4 // indirect
1313
go.uber.org/automaxprocs v1.6.0 // indirect
14-
golang.org/x/mod v0.24.0 // indirect
15-
golang.org/x/sync v0.12.0 // indirect
16-
golang.org/x/tools v0.31.0 // indirect
14+
golang.org/x/mod v0.25.0 // indirect
15+
golang.org/x/sync v0.15.0 // indirect
16+
golang.org/x/tools v0.34.0 // indirect
1717
)

‎tools/betteralign/go.sum‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
1-
github.com/KimMachineGun/automemlimit v0.7.1 h1:QcG/0iCOLChjfUweIMC3YL5Xy9C3VBeNmCZHrZfJMBw=
2-
github.com/KimMachineGun/automemlimit v0.7.1/go.mod h1:QZxpHaGOQoYvFhv/r4u3U0JTC2ZcOwbSr11UZF46UBM=
1+
github.com/KimMachineGun/automemlimit v0.7.2 h1:DyfHI7zLWmZPn2Wqdy2AgTiUvrGPmnYWgwhHXtAegX4=
2+
github.com/KimMachineGun/automemlimit v0.7.2/go.mod h1:QZxpHaGOQoYvFhv/r4u3U0JTC2ZcOwbSr11UZF46UBM=
33
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
44
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
5-
github.com/dkorunic/betteralign v0.7.0 h1:kJ0OkCqZl0ggyL6Zpu80ai7jPlmjcZyju2OTfAWxdDE=
6-
github.com/dkorunic/betteralign v0.7.0/go.mod h1:lGScW+t8uN2ml0di76txaWuUDsolwUvwx8tPMGJfjy0=
5+
github.com/dkorunic/betteralign v0.7.1 h1:/0iScp0+LxeV+9hbSyA4pgN5RkM2O5s7y8J9fNTqSRA=
6+
github.com/dkorunic/betteralign v0.7.1/go.mod h1:r/+o8JOPXl7sPHIIAcIGYp7vDxcQpP0KNEE9l/pxmME=
77
github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
88
github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
99
github.com/google/renameio/v2 v2.0.0 h1:UifI23ZTGY8Tt29JbYFiuyIU3eX+RNFtUwefq9qAhxg=
@@ -22,12 +22,12 @@ github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcU
2222
github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo=
2323
go.uber.org/automaxprocs v1.6.0 h1:O3y2/QNTOdbF+e/dpXNNW7Rx2hZ4sTIPyybbxyNqTUs=
2424
go.uber.org/automaxprocs v1.6.0/go.mod h1:ifeIMSnPZuznNm6jmdzmU3/bfk01Fe2fotchwEFJ8r8=
25-
golang.org/x/mod v0.24.0 h1:ZfthKaKaT4NrhGVZHO1/WDTwGES4De8KtWO0SIbNJMU=
26-
golang.org/x/mod v0.24.0/go.mod h1:IXM97Txy2VM4PJ3gI61r1YEk/gAj6zAHN3AdZt6S9Ww=
27-
golang.org/x/sync v0.12.0 h1:MHc5BpPuC30uJk597Ri8TV3CNZcTLu6B6z4lJy+g6Jw=
28-
golang.org/x/sync v0.12.0/go.mod h1:1dzgHSNfp02xaA81J2MS99Qcpr2w7fw1gpm99rleRqA=
29-
golang.org/x/tools v0.31.0 h1:0EedkvKDbh+qistFTd0Bcwe/YLh4vHwWEkiI0toFIBU=
30-
golang.org/x/tools v0.31.0/go.mod h1:naFTU+Cev749tSJRXJlna0T3WxKvb1kWEx15xA4SdmQ=
25+
golang.org/x/mod v0.25.0 h1:n7a+ZbQKQA/Ysbyb0/6IbB1H/X41mKgbhfv7AfG/44w=
26+
golang.org/x/mod v0.25.0/go.mod h1:IXM97Txy2VM4PJ3gI61r1YEk/gAj6zAHN3AdZt6S9Ww=
27+
golang.org/x/sync v0.15.0 h1:KWH3jNZsfyT6xfAfKiz6MRNmd46ByHDYaZ7KSkCtdW8=
28+
golang.org/x/sync v0.15.0/go.mod h1:1dzgHSNfp02xaA81J2MS99Qcpr2w7fw1gpm99rleRqA=
29+
golang.org/x/tools v0.34.0 h1:qIpSLOxeCYGg9TrcJokLBG4KFA6d795g0xkBkiESGlo=
30+
golang.org/x/tools v0.34.0/go.mod h1:pAP9OwEaY1CAW3HOmg3hLZC5Z0CCmzjAF2UQMSqNARg=
3131
gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
3232
gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
3333
gotest.tools/v3 v3.5.2 h1:7koQfIKdy+I8UTetycgUqXWSDwpgv193Ka+qRsmBY8Q=

0 commit comments

Comments
 (0)