Skip to content
This repository was archived by the owner on Jun 21, 2023. It is now read-only.

Commit 532063e

Browse files
committed
Ignore in-line comments with no diff context
This is unlikely to happen in real life but worth guarding against.
1 parent 45b48dd commit 532063e

File tree

2 files changed

+27
-5
lines changed

2 files changed

+27
-5
lines changed

src/GitHub.InlineReviews/Services/PullRequestSessionService.cs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -93,14 +93,13 @@ public IReadOnlyList<IInlineCommentThreadModel> BuildCommentThreads(
9393
var chunk = chunks.Last();
9494
var diffLines = chunk.Lines.Reverse().Take(5).ToList();
9595
var firstLine = diffLines.FirstOrDefault();
96-
var diffLineType = firstLine != null ? firstLine.Type : DiffChangeType.None;
97-
9896
if (firstLine == null)
9997
{
100-
log.Warning("Couldn't find diff lines for inline comment. RelativePath={RelativePath}, OriginalCommitSha={Sha}, OriginalPosition={Position}, Chunks={Chunks}, DiffHunk={DiffHunk}",
101-
relativePath, comments.Key.Item1, comments.Key.Item2, chunks.Count(), hunk);
98+
log.Warning("Ignoring in-line comment in {RelativePath} with no diff line context", relativePath);
99+
continue;
102100
}
103101

102+
var diffLineType = firstLine.Type;
104103
var thread = new InlineCommentThreadModel(
105104
relativePath,
106105
comments.Key.Item1,

test/GitHub.InlineReviews.UnitTests/Services/PullRequestSessionServiceTests.cs

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,29 @@ Line 2
5353
}
5454
}
5555

56+
[Test]
57+
public async Task IgnoreCommentsWithNoDiffLineContext()
58+
{
59+
var baseContents = "Line 1";
60+
var headContents = "Line 1";
61+
62+
var comment = CreateComment(@"@@ -10,7 +10,6 @@ class Program");
63+
64+
using (var diffService = new FakeDiffService(FilePath, baseContents))
65+
{
66+
var diff = await diffService.Diff(FilePath, headContents);
67+
var pullRequest = CreatePullRequest(FilePath, comment);
68+
var target = CreateTarget(diffService);
69+
70+
var result = target.BuildCommentThreads(
71+
pullRequest,
72+
FilePath,
73+
diff);
74+
75+
Assert.That(result, Is.Empty);
76+
}
77+
}
78+
5679
[Test]
5780
public async Task MatchesReviewCommentOnDifferentLine()
5881
{
@@ -212,7 +235,7 @@ Line 2
212235

213236
Assert.That(3, Is.EqualTo(threads[0].LineNumber));
214237
Assert.That(
215-
new[]
238+
new[]
216239
{
217240
Tuple.Create(2, DiffSide.Right),
218241
Tuple.Create(3, DiffSide.Right)

0 commit comments

Comments
 (0)