Merge branch 'master' into fix/sarif_range

reviewdog · Jan 30, 2024 · 0c4cae2 · 0c4cae2
2 parents f241461 + b728eea
commit 0c4cae2
Show file tree

Hide file tree

Showing 4 changed files with 5 additions and 68 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - ...
 
 ### :bug: Fixes
+- [#1651](https://github.com/reviewdog/reviewdog/pull/1651) Revert #1576: Support `--filter-mode=file` in `github-pr-review`. Reasons: #1645
 - [#1653](https://github.com/reviewdog/reviewdog/pull/1653) fix: SARIF parser: parse with no region result. fix originalOutput field
 - ...
 

diff --git a/README.md b/README.md
@@ -931,17 +931,16 @@ $ reviewdog -reporter=github-pr-review -filter-mode=nofilter -fail-on-error
 ### Filter Mode Support Table
 Note that not all reporters provide full support for filter mode due to API limitation.
 e.g. `github-pr-review` reporter uses [GitHub Review
-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 comments outside diff file,
+API](https://docs.github.com/en/rest/pulls/reviews) but this API don't support posting comments outside diff context,
 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` |
 | ---------------------------- | ------- | -------------- | ----------------------- | ---------- |
 | **`local`**                  | OK      | OK             | OK                      | OK |
 | **`github-check`**           | OK      | OK             | OK                      | OK |
 | **`github-pr-check`**        | OK      | OK             | OK                      | OK |
-| **`github-pr-review`**       | OK      | OK             | OK                      | Partially Supported [1] |
-| **`github-pr-annotations`**   | OK      | OK             | OK                      | OK |
+| **`github-pr-review`**       | OK      | OK             | Partially Supported [1] | Partially Supported [1] |
+| **`github-pr-annotations`**  | OK      | OK             | OK                      | OK |
 | **`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] |

diff --git a/service/github/github.go b/service/github/github.go
@@ -127,11 +127,10 @@ func (g *PullRequest) postAsReviewComment(ctx context.Context) error {
 	postComments := g.postComments
 	g.postComments = nil
 	rawComments := make([]*reviewdog.Comment, 0, len(postComments))
-	plainComments := make([]*github.PullRequestComment, 0, len(postComments))
 	reviewComments := make([]*github.DraftReviewComment, 0, len(postComments))
 	remaining := make([]*reviewdog.Comment, 0)
 	for _, c := range postComments {
-		if !c.Result.InDiffFile {
+		if !c.Result.InDiffContext {
 			// 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() {
@@ -147,14 +146,6 @@ func (g *PullRequest) postAsReviewComment(ctx context.Context) error {
 			continue
 		}
 
-		rawComments = append(rawComments, c)
-		if !c.Result.InDiffContext {
-			// If the result is outside of diff context, fallback to GitHub Review
-			// Comment API.
-			comment := buildPullRequestComment(c, body, g.sha)
-			plainComments = append(plainComments, comment)
-			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.
@@ -173,22 +164,6 @@ func (g *PullRequest) postAsReviewComment(ctx context.Context) error {
 		return err
 	}
 
-	if len(plainComments) > 0 {
-		// send pull request comments to GitHub.
-		for _, c := range plainComments {
-			_, _, err := g.cli.PullRequests.CreateComment(ctx, g.owner, g.repo, g.pr, c)
-			if err != nil {
-				log.Printf("reviewdog: failed to post a pull request comment: %v", err)
-				// GitHub returns 403 or 404 if we don't have permission to post a review comment.
-				// fallback to log message in this case.
-				if isPermissionError(err) && cienv.IsInGitHubAction() {
-					goto FALLBACK
-				}
-				return err
-			}
-		}
-	}
-
 	if len(reviewComments) > 0 {
 		// send review comments to GitHub.
 		review := &github.PullRequestReviewRequest{
@@ -246,17 +221,6 @@ 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

diff --git a/service/github/github_test.go b/service/github/github_test.go
@@ -202,7 +202,6 @@ 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) {
 		switch r.Method {
@@ -261,29 +260,6 @@ func TestGitHubPullRequest_Post_Flush_review_api(t *testing.T) {
 					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)
-			}
-			want := []*github.PullRequestComment{
-				{
-					Body:        github.String(commentutil.BodyPrefix + "file comment (no-line)"),
-					CommitID:    github.String("sha"),
-					Path:        github.String("reviewdog.go"),
-					SubjectType: github.String("file"),
-				},
-				{
-					Body:        github.String(commentutil.BodyPrefix + "file comment (outside diff-context)"),
-					CommitID:    github.String("sha"),
-					Path:        github.String("reviewdog.go"),
-					SubjectType: github.String("file"),
-				},
-			}
-			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)
 		}
@@ -1218,9 +1194,6 @@ 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) {