Repository: reviewdog/reviewdog
Pull request #417 (2020-01-05 15:00:08)

github-check: Use logging commands of GitHub Actions as Graceful degradation for Pull Request from forked repository
ref: #411


Patch:
diff --git a/.github/workflows/reviewdog_on_push.yml b/.github/workflows/reviewdog_github_check.yml
similarity index 92%
rename from .github/workflows/reviewdog_on_push.yml
rename to .github/workflows/reviewdog_github_check.yml
index 67a7ec65..f7e24ff4 100644
--- a/.github/workflows/reviewdog_on_push.yml
+++ b/.github/workflows/reviewdog_github_check.yml
@@ -1,5 +1,10 @@
-name: reviewdog on Push
-on: [push]
+name: reviewdog (github-check)
+on:
+  push:
+    branches:
+      - master
+  pull_request:
+
 jobs:
   reviewdog:
     name: reviewdog
diff --git a/README.md b/README.md
index a5a78a90..2f76dae0 100644
--- a/README.md
+++ b/README.md
@@ -52,6 +52,7 @@ by diff.
 - [Reporters](#reporters)
   * [Reporter: Local (-reporter=local) [default]](#reporter-local--reporterlocal-default)
   * [Reporter: GitHub Checks (-reporter=github-pr-check)](#reporter-github-checks--reportergithub-pr-check)
+  * [Reporter: GitHub Checks (-reporter=github-check)](#reporter-github-checks--reportergithub-check)
   * [Reporter: GitHub PullRequest review comment (-reporter=github-pr-review)](#reporter-github-pullrequest-review-comment--reportergithub-pr-review)
   * [Reporter: GitLab MergeRequest discussions (-reporter=gitlab-mr-discussion)](#reporter-gitlab-mergerequest-discussions--reportergitlab-mr-discussion)
   * [Reporter: GitLab MergeRequest commit (-reporter=gitlab-mr-commit)](#reporter-gitlab-mergerequest-commit--reportergitlab-mr-commit)
@@ -309,11 +310,11 @@ mind the above caution and please use it on your own risk.
 You can use github-pr-review reporter if you don't want to depend on reviewdog
 server.
 
-#### Reporter: GitHub Checks (-reporter=github-check)
+### Reporter: GitHub Checks (-reporter=github-check)
 
 It's basically same as `-reporter=github-pr-check` except it works not only for
-Pull Request but also for commit.
-Also, it reports results outside Pull Request diff too.
+Pull Request but also for commit and it reports results outside Pull Request
+diff too.
 
 [![sample comment outside diff](https://user-images.githubusercontent.com/3797062/69917921-e0680580-14ae-11ea-9a56-de9e3cbac005.png)](https://github.com/reviewdog/reviewdog/pull/364/files)
 
@@ -399,8 +400,40 @@ jobs:
           REVIEWDOG_GITHUB_API_TOKEN: ${{ secrets.GITHUB_TOKEN }}
         run: |
           reviewdog -reporter=github-pr-check -runners=golint,govet
+          # or
+          reviewdog -reporter=github-pr-review -runners=golint,govet
 ```
 
+<details>
+<summary><strong>Example (github-check reporter):</strong></summary>
+
+[.github/workflows/reviewdog_github_check.yml](.github/workflows/reviewdog_github_check.yml)
+
+Only `github-check` reporter can run on push event too.
+
+```yaml
+name: reviewdog (github-check)
+on:
+  push:
+    branches:
+      - master
+  pull_request:
+
+jobs:
+  reviewdog:
+    name: reviewdog
+    runs-on: ubuntu-latest
+    steps:
+      # ...
+      - name: Run reviewdog
+        env:
+          REVIEWDOG_GITHUB_API_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+        run: |
+          reviewdog -reporter=github-check -runners=golint,govet
+```
+
+</details>
+
 #### Public Reviewdog GitHub Actions
 You can also use public GitHub Actions to start using reviewdog with ease! :tada: :arrow_forward: :tada:
 
diff --git a/cmd/reviewdog/doghouse.go b/cmd/reviewdog/doghouse.go
index 4bb5d3cd..8dc02042 100644
--- a/cmd/reviewdog/doghouse.go
+++ b/cmd/reviewdog/doghouse.go
@@ -31,9 +31,6 @@ func runDoghouse(ctx context.Context, r io.Reader, w io.Writer, opt *option, isP
 		fmt.Fprintln(os.Stderr, "reviewdog: this is not PullRequest build.")
 		return nil
 	}
-	if !forPr {
-		ghInfo.PullRequest = 0
-	}
 	resultSet, err := checkResultSet(ctx, r, opt, isProject)
 	if err != nil {
 		return err
@@ -42,7 +39,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)
+	filteredResultSet, err := postResultSet(ctx, resultSet, ghInfo, cli, forPr)
 	if err != nil {
 		return err
 	}
@@ -111,7 +108,8 @@ func checkResultSet(ctx context.Context, r io.Reader, opt *option, isProject boo
 	return resultSet, nil
 }
 
-func postResultSet(ctx context.Context, resultSet *reviewdog.ResultMap, ghInfo *cienv.BuildInfo, cli client.DogHouseClientInterface) (*reviewdog.FilteredResultMap, error) {
+func postResultSet(ctx context.Context, resultSet *reviewdog.ResultMap,
+	ghInfo *cienv.BuildInfo, cli client.DogHouseClientInterface, forPr bool) (*reviewdog.FilteredResultMap, error) {
 	var g errgroup.Group
 	wd, _ := os.Getwd()
 	filteredResultSet := new(reviewdog.FilteredResultMap)
@@ -130,6 +128,8 @@ func postResultSet(ctx context.Context, resultSet *reviewdog.ResultMap, ghInfo *
 			Branch:      ghInfo.Branch,
 			Annotations: as,
 			Level:       result.Level,
+			// If it's only for PR, do not report results outside diff.
+			OutsideDiff: !forPr,
 		}
 		g.Go(func() error {
 			res, err := cli.Check(ctx, req)
diff --git a/doghouse/server/doghouse.go b/doghouse/server/doghouse.go
index 42d12e83..326e3b2c 100644
--- a/doghouse/server/doghouse.go
+++ b/doghouse/server/doghouse.go
@@ -39,9 +39,6 @@ func NewChecker(req *doghouse.CheckRequest, gh *github.Client) *Checker {
 }
 
 func (ch *Checker) Check(ctx context.Context) (*doghouse.CheckResponse, error) {
-
-	filterByDiff := ch.req.PullRequest != 0
-
 	var filediffs []*diff.FileDiff
 	if ch.req.PullRequest != 0 {
 		var err error
@@ -68,7 +65,7 @@ func (ch *Checker) Check(ctx context.Context) (*doghouse.CheckResponse, error) {
 		return nil, fmt.Errorf("failed to create check: %v", err)
 	}
 
-	checkRun, err := ch.postCheck(ctx, check.GetID(), filtered, filterByDiff)
+	checkRun, err := ch.postCheck(ctx, check.GetID(), filtered)
 	if err != nil {
 		return nil, fmt.Errorf("failed to post result: %v", err)
 	}
@@ -78,7 +75,8 @@ func (ch *Checker) Check(ctx context.Context) (*doghouse.CheckResponse, error) {
 	return res, nil
 }
 
-func (ch *Checker) postCheck(ctx context.Context, checkID int64, checks []*reviewdog.FilteredCheck, filterByDiff bool) (*github.CheckRun, error) {
+func (ch *Checker) postCheck(ctx context.Context, checkID int64, checks []*reviewdog.FilteredCheck) (*github.CheckRun, error) {
+	filterByDiff := ch.req.PullRequest != 0 && !ch.req.OutsideDiff
 	var annotations []*github.CheckRunAnnotation
 	for _, c := range checks {
 		if !c.InDiff && filterByDiff {
diff --git a/doghouse/service.go b/doghouse/service.go
index 43820de6..4c41f65e 100644
--- a/doghouse/service.go
+++ b/doghouse/service.go
@@ -31,7 +31,15 @@ type CheckRequest struct {
 
 	// Level is report level for this request.
 	// One of ["info", "warning", "error"]. Default is "error".
+	// Optional.
 	Level string `json:"level"`
+
+	// 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
+	// (because there are no diff!).
+	// Optional.
+	OutsideDiff bool `json:"outside_diff"`
 }
 
 // CheckResponse represents doghouse GitHub check response.


Test patch:
diff --git a/cmd/reviewdog/doghouse_test.go b/cmd/reviewdog/doghouse_test.go
index 62478bcc..ca50d5d3 100644
--- a/cmd/reviewdog/doghouse_test.go
+++ b/cmd/reviewdog/doghouse_test.go
@@ -273,7 +273,7 @@ func TestPostResultSet_withReportURL(t *testing.T) {
 		SHA:         sha,
 	}
 
-	if _, err := postResultSet(context.Background(), &resultSet, ghInfo, fakeCli); err != nil {
+	if _, err := postResultSet(context.Background(), &resultSet, ghInfo, fakeCli, true); err != nil {
 		t.Fatal(err)
 	}
 }
@@ -297,7 +297,7 @@ func TestPostResultSet_withoutReportURL(t *testing.T) {
 
 	ghInfo := &cienv.BuildInfo{Owner: owner, Repo: repo, PullRequest: prNum, SHA: sha}
 
-	resp, err := postResultSet(context.Background(), &resultSet, ghInfo, fakeCli)
+	resp, err := postResultSet(context.Background(), &resultSet, ghInfo, fakeCli, true)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -331,14 +331,15 @@ func TestPostResultSet_withEmptyResponse(t *testing.T) {
 
 	ghInfo := &cienv.BuildInfo{Owner: owner, Repo: repo, PullRequest: prNum, SHA: sha}
 
-	if _, err := postResultSet(context.Background(), &resultSet, ghInfo, fakeCli); err == nil {
+	if _, err := postResultSet(context.Background(), &resultSet, ghInfo, fakeCli, true); err == nil {
 		t.Error("got no error but want report missing error")
 	}
 }
 
 func TestReportResults(t *testing.T) {
 	cleanup := setupEnvs(map[string]string{
-		"GITHUB_ACTION": "",
+		"GITHUB_ACTION":     "",
+		"GITHUB_EVENT_PATH": "",
 	})
 	defer cleanup()
 	filteredResultSet := new(reviewdog.FilteredResultMap)
@@ -386,7 +387,8 @@ reviewdog: No results found for "name2". 1 results found outside diff.
 
 func TestReportResults_inGitHubAction(t *testing.T) {
 	cleanup := setupEnvs(map[string]string{
-		"GITHUB_ACTION": "xxx",
+		"GITHUB_ACTION":     "xxx",
+		"GITHUB_EVENT_PATH": "",
 	})
 	defer cleanup()
 	filteredResultSet := new(reviewdog.FilteredResultMap)
@@ -411,7 +413,8 @@ func TestReportResults_inGitHubAction(t *testing.T) {
 
 func TestReportResults_noResultsInDiff(t *testing.T) {
 	cleanup := setupEnvs(map[string]string{
-		"GITHUB_ACTION": "",
+		"GITHUB_ACTION":     "",
+		"GITHUB_EVENT_PATH": "",
 	})
 	defer cleanup()
 	filteredResultSet := new(reviewdog.FilteredResultMap)
diff --git a/doghouse/server/doghouse_test.go b/doghouse/server/doghouse_test.go
index 70e7b82a..d5acfe99 100644
--- a/doghouse/server/doghouse_test.go
+++ b/doghouse/server/doghouse_test.go
@@ -141,6 +141,88 @@ func TestCheck_OK(t *testing.T) {
 	}
 }
 
+func TestCheck_OK_outsidediff(t *testing.T) {
+	const (
+		name        = "haya14busa-linter"
+		owner       = "haya14busa"
+		repo        = "reviewdog"
+		prNum       = 14
+		sha         = "1414"
+		reportURL   = "http://example.com/report_url"
+		conclusion  = "neutral"
+		wantCheckID = 1414
+	)
+
+	req := &doghouse.CheckRequest{
+		Name:        name,
+		Owner:       owner,
+		Repo:        repo,
+		PullRequest: prNum,
+		SHA:         sha,
+		Annotations: []*doghouse.Annotation{
+			{
+				Path:       "sample.new.txt",
+				Line:       2,
+				Message:    "test message",
+				RawMessage: "raw test message",
+			},
+			{
+				Path:       "sample.new.txt",
+				Line:       14,
+				Message:    "test message outside diff",
+				RawMessage: "raw test message outside diff",
+			},
+		},
+		Level:       "warning",
+		OutsideDiff: true,
+	}
+
+	cli := &fakeCheckerGitHubCli{}
+	cli.FakeGetPullRequestDiff = func(ctx context.Context, owner, repo string, number int) ([]byte, error) {
+		return []byte(sampleDiff), nil
+	}
+	cli.FakeCreateCheckRun = func(ctx context.Context, owner, repo string, opt github.CreateCheckRunOptions) (*github.CheckRun, error) {
+		return &github.CheckRun{ID: github.Int64(wantCheckID)}, nil
+	}
+	cli.FakeUpdateCheckRun = func(ctx context.Context, owner, repo string, checkID int64, opt github.UpdateCheckRunOptions) (*github.CheckRun, error) {
+		annotations := opt.Output.Annotations
+		if len(annotations) == 0 {
+			if *opt.Conclusion != conclusion {
+				t.Errorf("UpdateCheckRunOptions.Conclusion = %q, want %q", *opt.Conclusion, conclusion)
+			}
+		} else {
+			wantAnnotations := []*github.CheckRunAnnotation{
+				{
+					Path:            github.String("sample.new.txt"),
+					StartLine:       github.Int(2),
+					EndLine:         github.Int(2),
+					AnnotationLevel: github.String("warning"),
+					Message:         github.String("test message"),
+					Title:           github.String("[haya14busa-linter] sample.new.txt#L2"),
+					RawDetails:      github.String("raw test message"),
+				},
+				{
+					Path:            github.String("sample.new.txt"),
+					StartLine:       github.Int(14),
+					EndLine:         github.Int(14),
+					AnnotationLevel: github.String("warning"),
+					Message:         github.String("test message outside diff"),
+					Title:           github.String("[haya14busa-linter] sample.new.txt#L14"),
+					RawDetails:      github.String("raw test message outside diff"),
+				},
+			}
+			if d := cmp.Diff(annotations, wantAnnotations); d != "" {
+				t.Errorf("Annotation diff found:\n%s", d)
+			}
+		}
+		return &github.CheckRun{HTMLURL: github.String(reportURL)}, nil
+	}
+	checker := &Checker{req: req, gh: cli}
+	if _, err := checker.Check(context.Background()); err != nil {
+		t.Fatal(err)
+	}
+}
+
 func TestCheck_OK_multiple_update_runs(t *testing.T) {
 	const (
 		name        = "haya14busa-linter"