Repository: reviewdog/reviewdog
Pull request #545 (2020-05-05 10:09:59)

github-check reporter: Deprecate OutsideDiff flag and use filter-mode=nofilter instead
ref: #536


Patch:
diff --git a/.github/workflows/reviewdog.yml b/.github/workflows/reviewdog.yml
index 39fdae32..45a75ee2 100644
--- a/.github/workflows/reviewdog.yml
+++ b/.github/workflows/reviewdog.yml
@@ -39,7 +39,7 @@ jobs:
         env:
           REVIEWDOG_GITHUB_API_TOKEN: ${{ secrets.GITHUB_TOKEN }}
         run: |
-          cd ./_testdata/ && golint ./... | reviewdog -f=golint -name=golint-check-subdir -reporter=github-check -level=info
+          cd ./_testdata/ && golint ./... | reviewdog -f=golint -name=golint-check-subdir -reporter=github-check -level=info -filter-mode=nofilter
 
   reviewdog-pr:
     if: github.event_name == 'pull_request'
diff --git a/cmd/reviewdog/doghouse.go b/cmd/reviewdog/doghouse.go
index 126321e4..1f955ad8 100644
--- a/cmd/reviewdog/doghouse.go
+++ b/cmd/reviewdog/doghouse.go
@@ -41,7 +41,7 @@ func runDoghouse(ctx context.Context, r io.Reader, w io.Writer, opt *option, isP
 	if err != nil {
 		return err
 	}
-	filteredResultSet, err := postResultSet(ctx, resultSet, ghInfo, cli, forPr, opt)
+	filteredResultSet, err := postResultSet(ctx, resultSet, ghInfo, cli, opt)
 	if err != nil {
 		return err
 	}
@@ -112,7 +112,7 @@ func checkResultSet(ctx context.Context, r io.Reader, opt *option, isProject boo
 }
 
 func postResultSet(ctx context.Context, resultSet *reviewdog.ResultMap,
-	ghInfo *cienv.BuildInfo, cli client.DogHouseClientInterface, forPr bool, opt *option) (*reviewdog.FilteredResultMap, error) {
+	ghInfo *cienv.BuildInfo, cli client.DogHouseClientInterface, opt *option) (*reviewdog.FilteredResultMap, error) {
 	var g errgroup.Group
 	wd, _ := os.Getwd()
 	gitRelWd, err := serviceutil.GitRelWorkdir()
@@ -135,8 +135,6 @@ func postResultSet(ctx context.Context, resultSet *reviewdog.ResultMap,
 			Branch:      ghInfo.Branch,
 			Annotations: as,
 			Level:       result.Level,
-			// If it's only for PR, do not report results outside diff.
-			OutsideDiff: !forPr,
 			FilterMode:  opt.filterMode,
 		}
 		g.Go(func() error {
diff --git a/cmd/reviewdog/main.go b/cmd/reviewdog/main.go
index 1d1b151e..ec69ea73 100644
--- a/cmd/reviewdog/main.go
+++ b/cmd/reviewdog/main.go
@@ -90,9 +90,9 @@ const (
 		Report results to stdout.
 
 	"github-check" (experimental)
-		Report results to GitHub Check. It works both for Pull Requests and commits
-		and this reporter reports all results regardless of it's new result or not.
-		You can see report results in GitHub PullRequest Check tab for Pull Request.
+		Report results to GitHub Check. It works both for Pull Requests and commits.
+		For Pull Request, you can see report results in GitHub PullRequest Check
+		tab and can control filtering mode by -filter-mode flag.
 
 		There are two options to use this reporter.
 
@@ -113,8 +113,7 @@ const (
 			Note: Token is not required if you run reviewdog in Travis CI.
 
 	"github-pr-check" (experimental)
-		Same as github-check reporter but it only supports Pull Requests and
-		reports only new results which is in diff.
+		Same as github-check reporter but it only supports Pull Requests.
 
 	"github-pr-review"
 		Report results to GitHub review comments.
diff --git a/doghouse/server/doghouse.go b/doghouse/server/doghouse.go
index 28b59671..64ce7127 100644
--- a/doghouse/server/doghouse.go
+++ b/doghouse/server/doghouse.go
@@ -12,6 +12,7 @@ import (
 
 	"github.com/reviewdog/reviewdog"
 	"github.com/reviewdog/reviewdog/diff"
+	"github.com/reviewdog/reviewdog/difffilter"
 	"github.com/reviewdog/reviewdog/doghouse"
 	"github.com/reviewdog/reviewdog/service/github/githubutils"
 )
@@ -49,7 +50,13 @@ func (ch *Checker) Check(ctx context.Context) (*doghouse.CheckResponse, error) {
 	}
 
 	results := annotationsToCheckResults(ch.req.Annotations)
-	filtered := reviewdog.FilterCheck(results, filediffs, 1, "", ch.req.FilterMode)
+	filterMode := ch.req.FilterMode
+	if ch.req.PullRequest == 0 || ch.req.OutsideDiff {
+		// If it's not Pull Request run, do not filter results by diff regardless
+		// of the filter mode.
+		filterMode = difffilter.ModeNoFilter
+	}
+	filtered := reviewdog.FilterCheck(results, filediffs, 1, "", filterMode)
 	check, err := ch.createCheck(ctx)
 	if err != nil {
 		// If this error is StatusForbidden (403) here, it means reviewdog is
@@ -76,10 +83,9 @@ func (ch *Checker) Check(ctx context.Context) (*doghouse.CheckResponse, error) {
 }
 
 func (ch *Checker) postCheck(ctx context.Context, checkID int64, checks []*reviewdog.FilteredCheck) (*github.CheckRun, string, error) {
-	filterByDiff := ch.req.PullRequest != 0 && !ch.req.OutsideDiff
 	var annotations []*github.CheckRunAnnotation
 	for _, c := range checks {
-		if !c.ShouldReport && filterByDiff {
+		if !c.ShouldReport {
 			continue
 		}
 		annotations = append(annotations, ch.toCheckRunAnnotation(c))
diff --git a/doghouse/service.go b/doghouse/service.go
index 825b878e..c67a9c22 100644
--- a/doghouse/service.go
+++ b/doghouse/service.go
@@ -37,6 +37,8 @@ type CheckRequest struct {
 	// Optional.
 	Level string `json:"level"`
 
+	// Deprecatd: Use FilterMode == diffilter.NoFilter instead.
+	//
 	// OutsideDiff represents whether it report results in outside diff or not as
 	// annotations. It's useful only when PullRequest != 0. If PullRequest is
 	// empty, it will always report results all resutls including outside diff


Test patch:
diff --git a/cmd/reviewdog/doghouse_test.go b/cmd/reviewdog/doghouse_test.go
index 355b208d..0c93bf0f 100644
--- a/cmd/reviewdog/doghouse_test.go
+++ b/cmd/reviewdog/doghouse_test.go
@@ -278,7 +278,7 @@ func TestPostResultSet_withReportURL(t *testing.T) {
 	}
 
 	opt := &option{filterMode: difffilter.ModeAdded}
-	if _, err := postResultSet(context.Background(), &resultSet, ghInfo, fakeCli, true, opt); err != nil {
+	if _, err := postResultSet(context.Background(), &resultSet, ghInfo, fakeCli, opt); err != nil {
 		t.Fatal(err)
 	}
 }
@@ -303,7 +303,7 @@ func TestPostResultSet_withoutReportURL(t *testing.T) {
 	ghInfo := &cienv.BuildInfo{Owner: owner, Repo: repo, PullRequest: prNum, SHA: sha}
 
 	opt := &option{filterMode: difffilter.ModeAdded}
-	resp, err := postResultSet(context.Background(), &resultSet, ghInfo, fakeCli, true, opt)
+	resp, err := postResultSet(context.Background(), &resultSet, ghInfo, fakeCli, opt)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -351,7 +351,7 @@ func TestPostResultSet_conclusion(t *testing.T) {
 		}
 		opt := &option{filterMode: difffilter.ModeAdded, failOnError: tt.failOnError}
 		id := fmt.Sprintf("[conclusion=%s, failOnError=%v]", tt.conclusion, tt.failOnError)
-		_, err := postResultSet(context.Background(), &resultSet, ghInfo, fakeCli, true, opt)
+		_, err := postResultSet(context.Background(), &resultSet, ghInfo, fakeCli, opt)
 		if tt.wantErr && err == nil {
 			t.Errorf("[%s] want err, but got nil.", id)
 		} else if !tt.wantErr && err != nil {
@@ -379,7 +379,7 @@ func TestPostResultSet_withEmptyResponse(t *testing.T) {
 	ghInfo := &cienv.BuildInfo{Owner: owner, Repo: repo, PullRequest: prNum, SHA: sha}
 
 	opt := &option{filterMode: difffilter.ModeAdded}
-	if _, err := postResultSet(context.Background(), &resultSet, ghInfo, fakeCli, true, opt); err == nil {
+	if _, err := postResultSet(context.Background(), &resultSet, ghInfo, fakeCli, opt); err == nil {
 		t.Error("got no error but want report missing error")
 	}
 }
diff --git a/doghouse/server/doghouse_test.go b/doghouse/server/doghouse_test.go
index 32968dd5..0a4bbf32 100644
--- a/doghouse/server/doghouse_test.go
+++ b/doghouse/server/doghouse_test.go
@@ -9,6 +9,7 @@ import (
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-github/v31/github"
 
+	"github.com/reviewdog/reviewdog/difffilter"
 	"github.com/reviewdog/reviewdog/doghouse"
 )
 
@@ -141,7 +142,7 @@ func TestCheck_OK(t *testing.T) {
 	}
 }
 
-func TestCheck_OK_outsidediff(t *testing.T) {
+func testOutsideDiff(t *testing.T, outsideDiff bool, filterMode difffilter.Mode) {
 	const (
 		name        = "haya14busa-linter"
 		owner       = "haya14busa"
@@ -174,7 +175,8 @@ func TestCheck_OK_outsidediff(t *testing.T) {
 			},
 		},
 		Level:       "warning",
-		OutsideDiff: true,
+		OutsideDiff: outsideDiff,
+		FilterMode:  filterMode,
 	}
 
 	cli := &fakeCheckerGitHubCli{}
@@ -223,6 +225,15 @@ func TestCheck_OK_outsidediff(t *testing.T) {
 	}
 }
 
+func TestCheck_OK_deprecated_outsidediff(t *testing.T) {
+	t.Run("deprecated: outside_diff", func(t *testing.T) {
+		testOutsideDiff(t, true, difffilter.ModeDefault)
+	})
+	t.Run("filter-mode=NoFilter", func(t *testing.T) {
+		testOutsideDiff(t, false, difffilter.ModeNoFilter)
+	})
+}
+
 func TestCheck_OK_multiple_update_runs(t *testing.T) {
 	const (
 		name        = "haya14busa-linter"