Repository: reviewdog/reviewdog (go)

Issue:
Support --filter-mode=[file/nofilter] in github-pr-review
GitHub has introduced [a new feature that allows commenting files in a pull requests](https://github.blog/changelog/2023-04-11-commenting-on-files-in-a-pull-request-is-now-generally-available/) by [Create a review comment for a pull request](https://docs.github.com/en/rest/pulls/comments?apiVersion=2022-11-28#create-a-review-comment-for-a-pull-request). With this feature, it's now possible to comment on a file displayed in the diff context, even without specifying a line number.

With this new enhancement, could reviewdog support file and nofilter modes in github-pr-review?

Pull request:
Support `--filter-mode=file` in `github-pr-review`
closes #1572

Support `--filter-mode=file` in `github-pr-review` by falling back to file comment when outside of diff context.

- [x] Updated Unreleased section in [CHANGELOG](https://github.com/reviewdog/reviewdog/blob/master/CHANGELOG.md) or it's not notable changes.



Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 75e87a04..d4100d50 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### :sparkles: Release Note <!-- optional -->
 
 ### :rocket: Enhancements
-- ...
+- [#1576](https://github.com/reviewdog/reviewdog/pull/1576) Support `--filter-mode=file` in `github-pr-review`
 
 ### :bug: Fixes
 - ...
diff --git a/README.md b/README.md
index 4b3accb5..482e5f50 100644
--- a/README.md
+++ b/README.md
@@ -917,7 +917,8 @@ $ reviewdog -reporter=github-pr-review -filter-mode=nofilter -fail-on-error
 ### Filter Mode Support Table
 Note that not all reporters provide full support of filter mode due to API limitation.
 e.g. `github-pr-review` reporter uses [GitHub Review
-API](https://docs.github.com/en/rest/pulls/reviews) but it doesn't support posting comment outside diff (`diff_context`),
+API](https://docs.github.com/en/rest/pulls/reviews) and [GitHub Review
+Comment API](https://docs.github.com/en/rest/pulls/comments) but these APIs don't support posting comment outside diff file,
 so reviewdog will use [Check annotation](https://docs.github.com/en/rest/checks/runs) as fallback to post those comments [1]. 
 
 | `-reporter` \ `-filter-mode` | `added` | `diff_context` | `file`                  | `nofilter` |
@@ -925,13 +926,13 @@ so reviewdog will use [Check annotation](https://docs.github.com/en/rest/checks/
 | **`local`**                  | OK      | OK             | OK                      | OK |
 | **`github-check`**           | OK      | OK             | OK                      | OK |
 | **`github-pr-check`**        | OK      | OK             | OK                      | OK |
-| **`github-pr-review`**       | OK      | OK             | Partially Supported [1] | Partially Supported [1] |
+| **`github-pr-review`**       | OK      | OK             | OK                      | Partially Supported [1] |
 | **`gitlab-mr-discussion`**   | OK      | OK             | OK                      | Partially Supported [2] |
 | **`gitlab-mr-commit`**       | OK      | Partially Supported [2] | Partially Supported [2] | Partially Supported [2] |
 | **`gerrit-change-review`**   | OK      | OK? [3]        | OK? [3]                 | Partially Supported? [2][3] |
 | **`bitbucket-code-report`**  | NO [4]  | NO [4]         | NO [4]                  | OK |
 
-- [1] Report results which is outside diff context with Check annotation as fallback if it's running in GitHub actions instead of Review API (comments). All results will be reported to console as well.
+- [1] Report results which is outside diff file with Check annotation as fallback if it's running in GitHub actions instead of Review API (comments). All results will be reported to console as well.
 - [2] Report results which is outside diff file to console.
 - [3] It should work, but not verified yet.
 - [4] Not implemented at the moment
diff --git a/service/github/github.go b/service/github/github.go
index c93b1d3b..86f60756 100644
--- a/service/github/github.go
+++ b/service/github/github.go
@@ -93,7 +93,7 @@ func (g *PullRequest) postAsReviewComment(ctx context.Context) error {
 	comments := make([]*github.DraftReviewComment, 0, len(g.postComments))
 	remaining := make([]*reviewdog.Comment, 0)
 	for _, c := range g.postComments {
-		if !c.Result.InDiffContext {
+		if !c.Result.InDiffFile {
 			// GitHub Review API cannot report results outside diff. If it's running
 			// in GitHub Actions, fallback to GitHub Actions log as report .
 			if cienv.IsInGitHubAction() {
@@ -105,6 +105,16 @@ func (g *PullRequest) postAsReviewComment(ctx context.Context) error {
 		if g.postedcs.IsPosted(c, githubCommentLine(c), body) {
 			continue
 		}
+		if !c.Result.InDiffContext {
+			// If the result is outside of diff context, fallback to GitHub Review
+			// Comment API.
+			comment := buildPullRequestComment(c, body, g.sha)
+			_, _, err := g.cli.PullRequests.CreateComment(ctx, g.owner, g.repo, g.pr, comment)
+			if err != nil {
+				return err
+			}
+			continue
+		}
 		// Only posts maxCommentsPerRequest comments per 1 request to avoid spammy
 		// review comments. An example GitHub error if we don't limit the # of
 		// review comments.
@@ -152,10 +162,24 @@ func buildDraftReviewComment(c *reviewdog.Comment, body string) *github.DraftRev
 	return r
 }
 
+// Document: https://docs.github.com/en/rest/pulls/comments?apiVersion=2022-11-28#create-a-review-comment-for-a-pull-request
+func buildPullRequestComment(c *reviewdog.Comment, body, commitID string) *github.PullRequestComment {
+	loc := c.Result.Diagnostic.GetLocation()
+	return &github.PullRequestComment{
+		Body:        github.String(body),
+		CommitID:    github.String(commitID),
+		Path:        github.String(loc.GetPath()),
+		SubjectType: github.String("file"),
+	}
+}
+
 // line represents end line if it's a multiline comment in GitHub, otherwise
 // it's start line.
 // Document: https://docs.github.com/en/rest/reference/pulls#create-a-review-comment-for-a-pull-request
 func githubCommentLine(c *reviewdog.Comment) int {
+	if !c.Result.InDiffContext {
+		return 0
+	}
 	_, end := githubCommentLineRange(c)
 	return end
 }
@@ -213,10 +237,14 @@ func (g *PullRequest) setPostedComment(ctx context.Context) error {
 		return err
 	}
 	for _, c := range cs {
-		if c.Line == nil || c.Path == nil || c.Body == nil {
+		if c.Line == nil || c.Path == nil || c.Body == nil || c.SubjectType == nil {
 			continue
 		}
-		g.postedcs.AddPostedComment(c.GetPath(), c.GetLine(), c.GetBody())
+		var line int
+		if c.GetSubjectType() == "line" {
+			line = c.GetLine()
+		}
+		g.postedcs.AddPostedComment(c.GetPath(), line, c.GetBody())
 	}
 	return nil
 }


Test patch:
diff --git a/service/github/github_test.go b/service/github/github_test.go
index 053f0e03..3f5fba94 100644
--- a/service/github/github_test.go
+++ b/service/github/github_test.go
@@ -79,6 +79,7 @@ func TestGitHubPullRequest_Post(t *testing.T) {
 				},
 				Message: "[reviewdog] test",
 			},
+			InDiffFile:    true,
 			InDiffContext: true,
 		},
 	}
@@ -201,48 +202,90 @@ func TestGitHubPullRequest_Post_Flush_review_api(t *testing.T) {
 
 	listCommentsAPICalled := 0
 	postCommentsAPICalled := 0
+	postCommentAPICalled := 0
 	mux := http.NewServeMux()
 	mux.HandleFunc("/repos/o/r/pulls/14/comments", func(w http.ResponseWriter, r *http.Request) {
-		listCommentsAPICalled++
-		if r.Method != http.MethodGet {
-			t.Errorf("unexpected access: %v %v", r.Method, r.URL)
-		}
-		switch r.URL.Query().Get("page") {
-		default:
-			cs := []*github.PullRequestComment{
-				{
-					Path: github.String("reviewdog.go"),
-					Line: github.Int(2),
-					Body: github.String(commentutil.BodyPrefix + "already commented"),
-				},
+		switch r.Method {
+		case http.MethodGet:
+			listCommentsAPICalled++
+			switch r.URL.Query().Get("page") {
+			default:
+				cs := []*github.PullRequestComment{
+					{
+						Path:        github.String("reviewdog.go"),
+						Line:        github.Int(2),
+						Body:        github.String(commentutil.BodyPrefix + "already commented"),
+						SubjectType: github.String("line"),
+					},
+				}
+				w.Header().Add("Link", `<https://api.github.com/repos/o/r/pulls/14/comments?page=2>; rel="next"`)
+				if err := json.NewEncoder(w).Encode(cs); err != nil {
+					t.Fatal(err)
+				}
+			case "2":
+				cs := []*github.PullRequestComment{
+					{
+						Path:        github.String("reviewdog.go"),
+						Line:        github.Int(15),
+						Body:        github.String(commentutil.BodyPrefix + "already commented 2"),
+						SubjectType: github.String("line"),
+					},
+					{
+						Path:        github.String("reviewdog.go"),
+						StartLine:   github.Int(15),
+						Line:        github.Int(16),
+						Body:        github.String(commentutil.BodyPrefix + "multiline existing comment"),
+						SubjectType: github.String("line"),
+					},
+					{
+						Path:        github.String("reviewdog.go"),
+						StartLine:   github.Int(15),
+						Line:        github.Int(17),
+						Body:        github.String(commentutil.BodyPrefix + "multiline existing comment (line-break)"),
+						SubjectType: github.String("line"),
+					},
+					{
+						Path:        github.String("reviewdog.go"),
+						Line:        github.Int(1),
+						Body:        github.String(commentutil.BodyPrefix + "existing file comment (no-line)"),
+						SubjectType: github.String("file"),
+					},
+					{
+						Path:        github.String("reviewdog.go"),
+						Line:        github.Int(1),
+						Body:        github.String(commentutil.BodyPrefix + "existing file comment (outside diff-context)"),
+						SubjectType: github.String("file"),
+					},
+				}
+				if err := json.NewEncoder(w).Encode(cs); err != nil {
+					t.Fatal(err)
+				}
 			}
-			w.Header().Add("Link", `<https://api.github.com/repos/o/r/pulls/14/comments?page=2>; rel="next"`)
-			if err := json.NewEncoder(w).Encode(cs); err != nil {
-				t.Fatal(err)
+		case http.MethodPost:
+			defer func() { postCommentAPICalled++ }()
+			var req github.PullRequestComment
+			if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
+				t.Error(err)
 			}
-		case "2":
-			cs := []*github.PullRequestComment{
-				{
-					Path: github.String("reviewdog.go"),
-					Line: github.Int(15),
-					Body: github.String(commentutil.BodyPrefix + "already commented 2"),
-				},
+			want := []*github.PullRequestComment{
 				{
-					Path:      github.String("reviewdog.go"),
-					StartLine: github.Int(15),
-					Line:      github.Int(16),
-					Body:      github.String(commentutil.BodyPrefix + "multiline existing comment"),
+					Body:        github.String(commentutil.BodyPrefix + "file comment (no-line)"),
+					CommitID:    github.String("sha"),
+					Path:        github.String("reviewdog.go"),
+					SubjectType: github.String("file"),
 				},
 				{
-					Path:      github.String("reviewdog.go"),
-					StartLine: github.Int(15),
-					Line:      github.Int(17),
-					Body:      github.String(commentutil.BodyPrefix + "multiline existing comment (line-break)"),
+					Body:        github.String(commentutil.BodyPrefix + "file comment (outside diff-context)"),
+					CommitID:    github.String("sha"),
+					Path:        github.String("reviewdog.go"),
+					SubjectType: github.String("file"),
 				},
 			}
-			if err := json.NewEncoder(w).Encode(cs); err != nil {
-				t.Fatal(err)
+			if diff := pretty.Compare(want[postCommentAPICalled], req); diff != "" {
+				t.Errorf("req.Comments diff: (-got +want)\n%s", diff)
 			}
+		default:
+			t.Errorf("unexpected access: %v %v", r.Method, r.URL)
 		}
 	})
 	mux.HandleFunc("/repos/o/r/pulls/14/reviews", func(w http.ResponseWriter, r *http.Request) {
@@ -504,6 +547,7 @@ func TestGitHubPullRequest_Post_Flush_review_api(t *testing.T) {
 					},
 					Message: "already commented",
 				},
+				InDiffFile:    true,
 				InDiffContext: true,
 			},
 		},
@@ -520,6 +564,7 @@ func TestGitHubPullRequest_Post_Flush_review_api(t *testing.T) {
 					},
 					Message: "already commented 2",
 				},
+				InDiffFile:    true,
 				InDiffContext: true,
 			},
 		},
@@ -536,6 +581,7 @@ func TestGitHubPullRequest_Post_Flush_review_api(t *testing.T) {
 					},
 					Message: "new comment",
 				},
+				InDiffFile:    true,
 				InDiffContext: true,
 			},
 		},
@@ -555,6 +601,7 @@ func TestGitHubPullRequest_Post_Flush_review_api(t *testing.T) {
 					},
 					Message: "multiline existing comment",
 				},
+				InDiffFile:    true,
 				InDiffContext: true,
 			},
 		},
@@ -576,6 +623,7 @@ func TestGitHubPullRequest_Post_Flush_review_api(t *testing.T) {
 					},
 					Message: "multiline existing comment (line-break)",
 				},
+				InDiffFile:    true,
 				InDiffContext: true,
 			},
 		},
@@ -595,6 +643,7 @@ func TestGitHubPullRequest_Post_Flush_review_api(t *testing.T) {
 					},
 					Message: "multiline new comment",
 				},
+				InDiffFile:    true,
 				InDiffContext: true,
 			},
 		},
@@ -607,6 +656,68 @@ func TestGitHubPullRequest_Post_Flush_review_api(t *testing.T) {
 					},
 					Message: "should not be reported via GitHub Review API",
 				},
+				InDiffFile:    false,
+				InDiffContext: false,
+			},
+		},
+		{
+			Result: &filter.FilteredDiagnostic{
+				Diagnostic: &rdf.Diagnostic{
+					Location: &rdf.Location{
+						Path: "reviewdog.go",
+						// No Line
+					},
+					Message: "file comment (no-line)",
+				},
+				InDiffFile:    true,
+				InDiffContext: false,
+			},
+		},
+		{
+			Result: &filter.FilteredDiagnostic{
+				Diagnostic: &rdf.Diagnostic{
+					Location: &rdf.Location{
+						Path: "reviewdog.go",
+						// No Line
+					},
+					Message: "existing file comment (no-line)",
+				},
+				InDiffFile:    true,
+				InDiffContext: false,
+			},
+		},
+		{
+			Result: &filter.FilteredDiagnostic{
+				Diagnostic: &rdf.Diagnostic{
+					Location: &rdf.Location{
+						Path: "reviewdog.go",
+						Range: &rdf.Range{
+							Start: &rdf.Position{
+								Line: 18,
+							},
+						},
+					},
+					Message: "file comment (outside diff-context)",
+				},
+				InDiffFile:    true,
+				InDiffContext: false,
+			},
+		},
+		{
+			Result: &filter.FilteredDiagnostic{
+				Diagnostic: &rdf.Diagnostic{
+					Location: &rdf.Location{
+						Path: "reviewdog.go",
+						Range: &rdf.Range{
+							Start: &rdf.Position{
+								Line: 18,
+							},
+						},
+					},
+					Message: "existing file comment (outside diff-context)",
+				},
+				InDiffFile:    true,
+				InDiffContext: false,
 			},
 		},
 		{
@@ -638,6 +749,7 @@ func TestGitHubPullRequest_Post_Flush_review_api(t *testing.T) {
 					},
 					Message: "multiline suggestion comment",
 				},
+				InDiffFile:    true,
 				InDiffContext: true,
 			},
 		},
@@ -664,6 +776,7 @@ func TestGitHubPullRequest_Post_Flush_review_api(t *testing.T) {
 					},
 					Message: "singleline suggestion comment",
 				},
+				InDiffFile:    true,
 				InDiffContext: true,
 			},
 		},
@@ -696,6 +809,7 @@ func TestGitHubPullRequest_Post_Flush_review_api(t *testing.T) {
 					},
 					Message: "invalid lines suggestion comment",
 				},
+				InDiffFile:                   true,
 				InDiffContext:                true,
 				FirstSuggestionInDiffContext: false,
 			},
@@ -731,6 +845,7 @@ func TestGitHubPullRequest_Post_Flush_review_api(t *testing.T) {
 					},
 					Message: "Use suggestion range as GitHub comment range if the suggestion is in diff context",
 				},
+				InDiffFile:                   true,
 				InDiffContext:                true,
 				FirstSuggestionInDiffContext: true,
 			},
@@ -777,6 +892,7 @@ func TestGitHubPullRequest_Post_Flush_review_api(t *testing.T) {
 					},
 					Message: "Partially invalid suggestions",
 				},
+				InDiffFile:                   true,
 				InDiffContext:                true,
 				FirstSuggestionInDiffContext: true,
 			},
@@ -812,6 +928,7 @@ func TestGitHubPullRequest_Post_Flush_review_api(t *testing.T) {
 					},
 					Message: "non-line based suggestion comment (no source lines)",
 				},
+				InDiffFile:    true,
 				InDiffContext: true,
 			},
 		},
@@ -837,6 +954,7 @@ func TestGitHubPullRequest_Post_Flush_review_api(t *testing.T) {
 					},
 					Message: "range suggestion (single line)",
 				},
+				InDiffFile:    true,
 				InDiffContext: true,
 			},
 		},
@@ -865,6 +983,7 @@ func TestGitHubPullRequest_Post_Flush_review_api(t *testing.T) {
 					},
 					Message: "range suggestion (multi-line)",
 				},
+				InDiffFile:    true,
 				InDiffContext: true,
 			},
 		},
@@ -894,6 +1013,7 @@ func TestGitHubPullRequest_Post_Flush_review_api(t *testing.T) {
 					},
 					Message: "range suggestion (line-break, remove)",
 				},
+				InDiffFile:    true,
 				InDiffContext: true,
 			},
 		},
@@ -921,6 +1041,7 @@ func TestGitHubPullRequest_Post_Flush_review_api(t *testing.T) {
 					},
 					Message: "range suggestion (insert)",
 				},
+				InDiffFile:    true,
 				InDiffContext: true,
 			},
 		},
@@ -960,6 +1081,7 @@ func TestGitHubPullRequest_Post_Flush_review_api(t *testing.T) {
 					},
 					Message: "multiple suggestions",
 				},
+				InDiffFile:    true,
 				InDiffContext: true,
 			},
 		},
@@ -984,6 +1106,7 @@ func TestGitHubPullRequest_Post_Flush_review_api(t *testing.T) {
 					},
 					Message: "range suggestion with start only location",
 				},
+				InDiffFile:    true,
 				InDiffContext: true,
 			},
 		},
@@ -1016,6 +1139,7 @@ func TestGitHubPullRequest_Post_Flush_review_api(t *testing.T) {
 					},
 					Message: "multiline suggestion comment including a code fence block",
 				},
+				InDiffFile:    true,
 				InDiffContext: true,
 			},
 		},
@@ -1042,6 +1166,7 @@ func TestGitHubPullRequest_Post_Flush_review_api(t *testing.T) {
 					},
 					Message: "singleline suggestion comment including a code fence block",
 				},
+				InDiffFile:    true,
 				InDiffContext: true,
 			},
 		},
@@ -1074,6 +1199,7 @@ func TestGitHubPullRequest_Post_Flush_review_api(t *testing.T) {
 					},
 					Message: "multiline suggestion comment including an empty code fence block",
 				},
+				InDiffFile:    true,
 				InDiffContext: true,
 			},
 		},
@@ -1092,6 +1218,9 @@ func TestGitHubPullRequest_Post_Flush_review_api(t *testing.T) {
 	if postCommentsAPICalled != 1 {
 		t.Errorf("GitHub post PullRequest comments API called %v times, want 1 times", postCommentsAPICalled)
 	}
+	if postCommentAPICalled != 2 {
+		t.Errorf("GitHub post PullRequest comment API called %v times, want 2 times", postCommentAPICalled)
+	}
 }
 
 func TestGitHubPullRequest_Post_toomany(t *testing.T) {
@@ -1142,6 +1271,7 @@ func TestGitHubPullRequest_Post_toomany(t *testing.T) {
 					},
 					Message: "comment",
 				},
+				InDiffFile:    true,
 				InDiffContext: true,
 			},
 			ToolName: "tool",