Add a fallback to git diff command to get diff of a GitHub pull request on doghouse

CHANGELOG.md

@@ -12,7 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - ...
 
 ### :bug: Fixes
-- ...
+- [#1722](https://github.com/reviewdog/reviewdog/pull/1722) Add a fallback to `git diff` command to get diff of a GitHub pull request on doghouse
 
 ### :rotating_light: Breaking changes
 - ...

doghouse/server/github_checker.go

@@ -2,7 +2,12 @@
 
 import (
 	"context"
+	"fmt"
 	"io"
+	"log"
+	"net/http"
+	"os"
+	"os/exec"
 	"time"
 
 	"github.com/google/go-github/v60/github"
@@ -21,10 +26,58 @@
 
 func (c *checkerGitHubClient) GetPullRequestDiff(ctx context.Context, owner, repo string, number int) ([]byte, error) {
 	opt := github.RawOptions{Type: github.Diff}
-	d, _, err := c.PullRequests.GetRaw(ctx, owner, repo, number, opt)
+	d, resp, err := c.PullRequests.GetRaw(ctx, owner, repo, number, opt)
+	if err != nil {
+		if resp != nil && resp.StatusCode == http.StatusNotAcceptable && c.checkInstallGitCommand() {
+			log.Print("fallback to use git command")
+			return c.getPullRequestDiffUsingGitCommand(ctx, owner, repo, number)
+		}
+
+		return nil, err
+	}
 	return []byte(d), err
 }
 
+// checkInstallGitCommand checks if git command is installed.
+func (c *checkerGitHubClient) checkInstallGitCommand() bool {
+	_, err := exec.Command("git", "-v").CombinedOutput()
+	return err == nil
+}
+
+// getPullRequestDiffUsingGitCommand returns a diff of PullRequest using git command.
+func (c *checkerGitHubClient) getPullRequestDiffUsingGitCommand(ctx context.Context, owner, repo string, number int) ([]byte, error) {
+	pr, _, err := c.PullRequests.Get(ctx, owner, repo, number)
+	if err != nil {
+		return nil, err
+	}
+
+	head := pr.GetHead()
+	headSha := head.GetSHA()
+
+	commitsComparison, _, err := c.Repositories.CompareCommits(ctx, owner, repo, headSha, pr.GetBase().GetSHA(), nil)
+	if err != nil {
+		return nil, err
+	}
+
+	mergeBaseSha := commitsComparison.GetMergeBaseCommit().GetSHA()
+
+	if os.Getenv("REVIEWDOG_SKIP_GIT_FETCH") != "true" {
+		for _, sha := range []string{mergeBaseSha, headSha} {
+			_, err := exec.Command("git", "fetch", "--depth=1", head.GetRepo().GetHTMLURL(), sha).CombinedOutput()
+			if err != nil {
+				return nil, fmt.Errorf("failed to run git fetch: %w", err)
+			}
+		}
+	}
+
+	bytes, err := exec.Command("git", "diff", "--find-renames", mergeBaseSha, headSha).CombinedOutput()
+	if err != nil {
+		return nil, fmt.Errorf("failed to run git diff: %w", err)
+	}
+
+	return bytes, nil
+}
+
 func (c *checkerGitHubClient) CreateCheckRun(ctx context.Context, owner, repo string, opt github.CreateCheckRunOptions) (*github.CheckRun, error) {
 	checkRun, _, err := c.Checks.CreateCheckRun(ctx, owner, repo, opt)
 	return checkRun, err

doghouse/server/github_checker_test.go

@@ -0,0 +1,169 @@
+package server
+
+import (
+	"context"
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"os"
+	"strings"
+	"testing"
+
+	"github.com/google/go-github/v60/github"
+	"github.com/reviewdog/reviewdog/doghouse"
+	"golang.org/x/oauth2"
+)
+
+const notokenSkipTestMes = "skipping test (requires actual Personal access tokens. export REVIEWDOG_TEST_GITHUB_API_TOKEN=<GitHub Personal Access Token>)"
+
+func setupGitHubClient() *github.Client {
+	token := os.Getenv("REVIEWDOG_TEST_GITHUB_API_TOKEN")
+	if token == "" {
+		return nil
+	}
+	ts := oauth2.StaticTokenSource(
+		&oauth2.Token{AccessToken: token},
+	)
+	tc := oauth2.NewClient(context.TODO(), ts)
+	return github.NewClient(tc)
+}
+
+func TestChecker_GetPullRequestDiff(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping test which contains actual API requests in short mode")
+	}
+	client := setupGitHubClient()
+	if client == nil {
+		t.Skip(notokenSkipTestMes)
+	}
+
+	want := `diff --git a/.codecov.yml b/.codecov.yml
+index aa49124774..781ee2492f 100644
+--- a/.codecov.yml
++++ b/.codecov.yml
+@@ -7,5 +7,4 @@ coverage:
+       default:
+         target: 0%
+
+-comment:
+-  layout: "header"
++comment: false
+`
+
+	// https://github.com/reviewdog/reviewdog/pull/73
+	owner := "haya14busa"
+	repo := "reviewdog"
+	pr := 73
+	b, err := NewChecker(&doghouse.CheckRequest{}, client).gh.GetPullRequestDiff(context.Background(), owner, repo, pr)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if got := string(b); got != want {
+		t.Errorf("got:\n%v\nwant:\n%v", got, want)
+	}
+}
+
+func TestChecker_GetPullRequestDiff_fake(t *testing.T) {
+	apiCalled := 0
+	mux := http.NewServeMux()
+	mux.HandleFunc("/repos/o/r/pulls/14", func(w http.ResponseWriter, r *http.Request) {
+		apiCalled++
+		if r.Method != http.MethodGet {
+			t.Errorf("unexpected access: %v %v", r.Method, r.URL)
+		}
+		if accept := r.Header.Get("Accept"); !strings.Contains(accept, "diff") {
+			t.Errorf("Accept header doesn't contain 'diff': %v", accept)
+		}
+		w.Write([]byte("Pull Request diff"))
+	})
+	ts := httptest.NewServer(mux)
+	defer ts.Close()
+
+	cli := github.NewClient(nil)
+	cli.BaseURL, _ = url.Parse(ts.URL + "/")
+
+	gh := NewChecker(&doghouse.CheckRequest{}, cli).gh
+
+	if _, err := gh.GetPullRequestDiff(context.Background(), "o", "r", 14); err != nil {
+		t.Fatal(err)
+	}
+	if apiCalled != 1 {
+		t.Errorf("GitHub API should be called once; called %v times", apiCalled)
+	}
+}
+
+func TestChecker_GetPullRequestDiff_fake_fallback(t *testing.T) {
+	apiCalled := 0
+	mux := http.NewServeMux()
+	headSHA := "HEAD^"
+	baseSHA := "HEAD"
+	mux.HandleFunc("/repos/o/r/pulls/14", func(w http.ResponseWriter, r *http.Request) {
+		apiCalled++
+		if r.Method != http.MethodGet {
+			t.Errorf("unexpected access: %v %v", r.Method, r.URL)
+		}
+		if accept := r.Header.Get("Accept"); strings.Contains(accept, "diff") {
+			w.WriteHeader(http.StatusNotAcceptable)
+			return
+		}
+		if accept := r.Header.Get("Accept"); accept != "application/vnd.github.v3+json" {
+			t.Errorf("Accept header doesn't contain 'diff': %v", accept)
+		}
+
+		pullRequestJSON, err := json.Marshal(github.PullRequest{
+			Head: &github.PullRequestBranch{
+				SHA: &headSHA,
+			},
+			Base: &github.PullRequestBranch{
+				SHA: &baseSHA,
+			},
+		})
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		if _, err := w.Write(pullRequestJSON); err != nil {
+			t.Fatal(err)
+		}
+	})
+	mux.HandleFunc("/repos/o/r/compare/"+headSHA+"..."+baseSHA, func(w http.ResponseWriter, r *http.Request) {
+		if r.Method != http.MethodGet {
+			t.Errorf("unexpected access: %v %v", r.Method, r.URL)
+		}
+		if accept := r.Header.Get("Accept"); accept != "application/vnd.github.v3+json" {
+			t.Errorf("Accept header doesn't contain 'diff': %v", accept)
+		}
+
+		mergeBaseSha := "HEAD^"
+
+		commitsComparisonJSON, err := json.Marshal(github.CommitsComparison{
+			MergeBaseCommit: &github.RepositoryCommit{
+				SHA: &mergeBaseSha,
+			},
+		})
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		if _, err := w.Write(commitsComparisonJSON); err != nil {
+			t.Fatal(err)
+		}
+	})
+	ts := httptest.NewServer(mux)
+	defer ts.Close()
+
+	t.Setenv("REVIEWDOG_SKIP_GIT_FETCH", "true")
+
+	cli := github.NewClient(nil)
+	cli.BaseURL, _ = url.Parse(ts.URL + "/")
+
+	gh := NewChecker(&doghouse.CheckRequest{}, cli).gh
+
+	if _, err := gh.GetPullRequestDiff(context.Background(), "o", "r", 14); err != nil {
+		t.Fatal(err)
+	}
+	if apiCalled != 2 {
+		t.Errorf("GitHub API should be called twice; called %v times", apiCalled)
+	}
+}

service/github/github.go

@@ -304,7 +304,7 @@ func (g *PullRequest) Diff(ctx context.Context) ([]byte, error) {
 	opt := github.RawOptions{Type: github.Diff}
 	d, resp, err := g.cli.PullRequests.GetRaw(ctx, g.owner, g.repo, g.pr, opt)
 	if err != nil {
-		if resp != nil && resp.StatusCode == http.StatusNotAcceptable {
+		if resp != nil && resp.StatusCode == http.StatusNotAcceptable && g.checkInstallGitCommand() {
 			log.Print("fallback to use git command")
 			return g.diffUsingGitCommand(ctx)
 		}
@@ -314,6 +314,12 @@ func (g *PullRequest) Diff(ctx context.Context) ([]byte, error) {
 	return []byte(d), nil
 }
 
+// checkInstallGitCommand checks if git command is installed.
+func (g *PullRequest) checkInstallGitCommand() bool {
+	_, err := exec.Command("git", "-v").CombinedOutput()
+	return err == nil
+}
+
 // diffUsingGitCommand returns a diff of PullRequest using git command.
 func (g *PullRequest) diffUsingGitCommand(ctx context.Context) ([]byte, error) {
 	pr, _, err := g.cli.PullRequests.Get(ctx, g.owner, g.repo, g.pr)