fix existing comments handling

GitHub API returns position/line as end line for multiline comments. # ------------------------ >8 ------------------------ # reviewdog results
reviewdog · Jul 22, 2020 · 12ee008 · 12ee008
1 parent 198d9f3
commit 12ee008
Show file tree

Hide file tree

Showing 2 changed files with 75 additions and 12 deletions.
diff --git a/service/github/github.go b/service/github/github.go
@@ -93,7 +93,7 @@ func (g *GitHubPullRequest) postAsReviewComment(ctx context.Context) error {
 			}
 			continue
 		}
-		if g.postedcs.IsPosted(c, c.Result.LnumDiff) {
+		if g.postedcs.IsPosted(c, githubPostedLine(c)) {
 			continue
 		}
 		// Only posts maxCommentsPerRequest comments per 1 request to avoid spammy
@@ -151,6 +151,16 @@ func buildDraftReviewComment(c *reviewdog.Comment) *github.DraftReviewComment {
 	return r
 }
 
+// line represents end line if it's a multiline comment in GitHub.
+// Document: https://docs.github.com/en/rest/reference/pulls#create-a-review-comment-for-a-pull-request
+func githubPostedLine(c *reviewdog.Comment) int {
+	line := c.Result.Diagnostic.GetLocation().GetRange().GetEnd().GetLine()
+	if line == 0 {
+		line = c.Result.Diagnostic.GetLocation().GetRange().GetStart().GetLine()
+	}
+	return int(line)
+}
+
 func (g *GitHubPullRequest) remainingCommentsSummary(remaining []*reviewdog.Comment) string {
 	if len(remaining) == 0 {
 		return ""
@@ -182,12 +192,10 @@ func (g *GitHubPullRequest) setPostedComment(ctx context.Context) error {
 		return err
 	}
 	for _, c := range cs {
-		if c.Position == nil || c.Path == nil || c.Body == nil {
-			// skip resolved comments. Or comments which do not have "path" nor
-			// "body".
+		if c.Line == nil || c.Path == nil || c.Body == nil {
 			continue
 		}
-		g.postedcs.AddPostedComment(c.GetPath(), c.GetPosition(), c.GetBody())
+		g.postedcs.AddPostedComment(c.GetPath(), c.GetLine(), c.GetBody())
 	}
 	return nil
 }

diff --git a/service/github/github_test.go b/service/github/github_test.go
@@ -212,9 +212,9 @@ func TestGitHubPullRequest_Post_Flush_review_api(t *testing.T) {
 		default:
 			cs := []*github.PullRequestComment{
 				{
-					Path:     github.String("reviewdog.go"),
-					Position: github.Int(1),
-					Body:     github.String(commentutil.BodyPrefix + "\nalready commented"),
+					Path: github.String("reviewdog.go"),
+					Line: github.Int(2),
+					Body: github.String(commentutil.BodyPrefix + "\nalready commented"),
 				},
 			}
 			w.Header().Add("Link", `<https://api.github.com/repos/o/r/pulls/14/comments?page=2>; rel="next"`)
@@ -224,9 +224,15 @@ func TestGitHubPullRequest_Post_Flush_review_api(t *testing.T) {
 		case "2":
 			cs := []*github.PullRequestComment{
 				{
-					Path:     github.String("reviewdog.go"),
-					Position: github.Int(14),
-					Body:     github.String(commentutil.BodyPrefix + "\nalready commented 2"),
+					Path: github.String("reviewdog.go"),
+					Line: github.Int(15),
+					Body: github.String(commentutil.BodyPrefix + "\nalready commented 2"),
+				},
+				{
+					Path:      github.String("reviewdog.go"),
+					StartLine: github.Int(15),
+					Line:      github.Int(16),
+					Body:      github.String(commentutil.BodyPrefix + "\nmultiline existing comment"),
 				},
 			}
 			if err := json.NewEncoder(w).Encode(cs); err != nil {
@@ -259,6 +265,14 @@ func TestGitHubPullRequest_Post_Flush_review_api(t *testing.T) {
 				Line: github.Int(15),
 				Body: github.String(commentutil.BodyPrefix + "\nnew comment"),
 			},
+			{
+				Path:      github.String("reviewdog.go"),
+				Side:      github.String("RIGHT"),
+				StartSide: github.String("RIGHT"),
+				StartLine: github.Int(15),
+				Line:      github.Int(16),
+				Body:      github.String(commentutil.BodyPrefix + "\nmultiline new comment"),
+			},
 		}
 		if diff := pretty.Compare(want, req.Comments); diff != "" {
 			t.Errorf("req.Comments diff: (-got +want)\n%s", diff)
@@ -328,6 +342,48 @@ func TestGitHubPullRequest_Post_Flush_review_api(t *testing.T) {
 			},
 			Body: "new comment",
 		},
+		{
+			Result: &reviewdog.FilteredCheck{
+				CheckResult: &reviewdog.CheckResult{
+					Diagnostic: &rdf.Diagnostic{
+						Location: &rdf.Location{
+							Path: "reviewdog.go",
+							Range: &rdf.Range{
+								Start: &rdf.Position{
+									Line: 15,
+								},
+								End: &rdf.Position{
+									Line: 16,
+								},
+							},
+						},
+					},
+				},
+				LnumDiff: 14,
+			},
+			Body: "multiline existing comment",
+		},
+		{
+			Result: &reviewdog.FilteredCheck{
+				CheckResult: &reviewdog.CheckResult{
+					Diagnostic: &rdf.Diagnostic{
+						Location: &rdf.Location{
+							Path: "reviewdog.go",
+							Range: &rdf.Range{
+								Start: &rdf.Position{
+									Line: 15,
+								},
+								End: &rdf.Position{
+									Line: 16,
+								},
+							},
+						},
+					},
+				},
+				LnumDiff: 14,
+			},
+			Body: "multiline new comment",
+		},
 		{
 			Result: &reviewdog.FilteredCheck{
 				CheckResult: &reviewdog.CheckResult{
@@ -338,7 +394,6 @@ func TestGitHubPullRequest_Post_Flush_review_api(t *testing.T) {
 						},
 					},
 				},
-				// No LnumDiff.
 			},
 			Body: "should not be reported via GitHub Review API",
 		},