Merge pull request #1439 from github/fixes/1435-comment-no-diff-lines

Ignore inline comment if it contains no diff line context

Author: grokys

src/GitHub.InlineReviews/Models/InlineCommentThreadModel.cs

@@ -39,6 +39,7 @@ public InlineCommentThreadModel(
 
             Comments = comments.ToList();
             DiffMatch = diffMatch;
+            DiffLineType = diffMatch[0].Type;
             OriginalCommitSha = originalCommitSha;
             OriginalPosition = originalPosition;
             RelativePath = relativePath;
@@ -51,7 +52,7 @@ public InlineCommentThreadModel(
         public IList<DiffLine> DiffMatch { get; }
 
         /// <inheritdoc/>
-        public DiffChangeType DiffLineType => DiffMatch.First().Type;
+        public DiffChangeType DiffLineType { get; }
 
         /// <inheritdoc/>
         public bool IsStale

src/GitHub.InlineReviews/Services/PullRequestSessionService.cs

@@ -11,11 +11,13 @@
 using GitHub.InlineReviews.Models;
 using GitHub.Models;
 using GitHub.Primitives;
+using GitHub.Logging;
 using GitHub.Services;
 using LibGit2Sharp;
 using Microsoft.VisualStudio.Text;
 using Microsoft.VisualStudio.Text.Projection;
 using ReactiveUI;
+using Serilog;
 
 namespace GitHub.InlineReviews.Services
 {
@@ -25,6 +27,8 @@ namespace GitHub.InlineReviews.Services
     [Export(typeof(IPullRequestSessionService))]
     public class PullRequestSessionService : IPullRequestSessionService
     {
+        static readonly ILogger log = LogManager.ForContext<PullRequestSessionService>();
+
         readonly IGitService gitService;
         readonly IGitClient gitClient;
         readonly IDiffService diffService;
@@ -88,6 +92,13 @@ public IReadOnlyList<IInlineCommentThreadModel> BuildCommentThreads(
                 var chunks = DiffUtilities.ParseFragment(hunk);
                 var chunk = chunks.Last();
                 var diffLines = chunk.Lines.Reverse().Take(5).ToList();
+                var firstLine = diffLines.FirstOrDefault();
+                if (firstLine == null)
+                {
+                    log.Warning("Ignoring in-line comment in {RelativePath} with no diff line context", relativePath);
+                    continue;
+                }
+
                 var thread = new InlineCommentThreadModel(
                     relativePath,
                     comments.Key.Item1,

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

@@ -53,6 +53,29 @@ Line 2
                 }
             }
 
+            [Test]
+            public async Task IgnoreCommentsWithNoDiffLineContext()
+            {
+                var baseContents = "Line 1";
+                var headContents = "Line 1";
+
+                var comment = CreateComment(@"@@ -10,7 +10,6 @@ class Program");
+
+                using (var diffService = new FakeDiffService(FilePath, baseContents))
+                {
+                    var diff = await diffService.Diff(FilePath, headContents);
+                    var pullRequest = CreatePullRequest(FilePath, comment);
+                    var target = CreateTarget(diffService);
+
+                    var result = target.BuildCommentThreads(
+                        pullRequest,
+                        FilePath,
+                        diff);
+
+                    Assert.That(result, Is.Empty);
+                }
+            }
+
             [Test]
             public async Task MatchesReviewCommentOnDifferentLine()
             {
@@ -212,7 +235,7 @@ Line 2
 
                     Assert.That(3, Is.EqualTo(threads[0].LineNumber));
                     Assert.That(
-                        new[] 
+                        new[]
                         {
                             Tuple.Create(2, DiffSide.Right),
                             Tuple.Create(3, DiffSide.Right)