From 12a2b260a21864027d08f9d218d3507bb385ea18 Mon Sep 17 00:00:00 2001 From: haya14busa Date: Mon, 1 Jul 2024 23:17:08 +0900 Subject: [PATCH 01/19] Update go-github version to v62 --- cmd/reviewdog/main.go | 2 +- doghouse/appengine/github.go | 2 +- doghouse/appengine/github_webhook.go | 2 +- doghouse/server/doghouse.go | 2 +- doghouse/server/doghouse_test.go | 2 +- doghouse/server/github.go | 2 +- scripts/trigger-depup/main.go | 2 +- service/github/check.go | 2 +- service/github/check_test.go | 2 +- service/github/diff.go | 2 +- service/github/diff_test.go | 2 +- service/github/github.go | 2 +- service/github/github_test.go | 2 +- 13 files changed, 13 insertions(+), 13 deletions(-) diff --git a/cmd/reviewdog/main.go b/cmd/reviewdog/main.go index d5c2a5a8..452fac21 100644 --- a/cmd/reviewdog/main.go +++ b/cmd/reviewdog/main.go @@ -20,7 +20,7 @@ import ( "golang.org/x/build/gerrit" "golang.org/x/oauth2" - "github.com/google/go-github/v60/github" + "github.com/google/go-github/v62/github" "github.com/mattn/go-shellwords" "github.com/reviewdog/errorformat/fmts" "github.com/xanzy/go-gitlab" diff --git a/doghouse/appengine/github.go b/doghouse/appengine/github.go index c9601a84..5a9e0765 100644 --- a/doghouse/appengine/github.go +++ b/doghouse/appengine/github.go @@ -12,7 +12,7 @@ import ( "net/url" "strings" - "github.com/google/go-github/v60/github" + "github.com/google/go-github/v62/github" "github.com/justinas/nosurf" "github.com/vvakame/sdlog/aelog" "golang.org/x/oauth2" diff --git a/doghouse/appengine/github_webhook.go b/doghouse/appengine/github_webhook.go index 7538c307..8044c051 100644 --- a/doghouse/appengine/github_webhook.go +++ b/doghouse/appengine/github_webhook.go @@ -6,7 +6,7 @@ import ( "fmt" "net/http" - "github.com/google/go-github/v60/github" + "github.com/google/go-github/v62/github" "github.com/reviewdog/reviewdog/doghouse/server/storage" ) diff --git a/doghouse/server/doghouse.go b/doghouse/server/doghouse.go index 33230665..5931b871 100644 --- a/doghouse/server/doghouse.go +++ b/doghouse/server/doghouse.go @@ -6,7 +6,7 @@ import ( "errors" "fmt" - "github.com/google/go-github/v60/github" + "github.com/google/go-github/v62/github" "github.com/reviewdog/reviewdog" "github.com/reviewdog/reviewdog/diff" diff --git a/doghouse/server/doghouse_test.go b/doghouse/server/doghouse_test.go index 4922e28a..146b5859 100644 --- a/doghouse/server/doghouse_test.go +++ b/doghouse/server/doghouse_test.go @@ -9,7 +9,7 @@ import ( "testing" "github.com/google/go-cmp/cmp" - "github.com/google/go-github/v60/github" + "github.com/google/go-github/v62/github" "github.com/reviewdog/reviewdog/doghouse" "github.com/reviewdog/reviewdog/proto/rdf" ) diff --git a/doghouse/server/github.go b/doghouse/server/github.go index e0a1fae0..a20fb193 100644 --- a/doghouse/server/github.go +++ b/doghouse/server/github.go @@ -6,7 +6,7 @@ import ( "net/http" "github.com/bradleyfalzon/ghinstallation/v2" - "github.com/google/go-github/v60/github" + "github.com/google/go-github/v62/github" ) type NewGitHubClientOption struct { diff --git a/scripts/trigger-depup/main.go b/scripts/trigger-depup/main.go index d5d09961..b42da679 100644 --- a/scripts/trigger-depup/main.go +++ b/scripts/trigger-depup/main.go @@ -10,7 +10,7 @@ import ( "os" "strings" - "github.com/google/go-github/v60/github" + "github.com/google/go-github/v62/github" "golang.org/x/oauth2" ) diff --git a/service/github/check.go b/service/github/check.go index ad01f225..d39c8ea3 100644 --- a/service/github/check.go +++ b/service/github/check.go @@ -8,7 +8,7 @@ import ( "sync" "time" - "github.com/google/go-github/v60/github" + "github.com/google/go-github/v62/github" "github.com/reviewdog/reviewdog" "github.com/reviewdog/reviewdog/cienv" "github.com/reviewdog/reviewdog/filter" diff --git a/service/github/check_test.go b/service/github/check_test.go index 713392a6..9288a01e 100644 --- a/service/github/check_test.go +++ b/service/github/check_test.go @@ -10,7 +10,7 @@ import ( "testing" "github.com/google/go-cmp/cmp" - "github.com/google/go-github/v60/github" + "github.com/google/go-github/v62/github" "github.com/reviewdog/reviewdog" "github.com/reviewdog/reviewdog/filter" "github.com/reviewdog/reviewdog/proto/rdf" diff --git a/service/github/diff.go b/service/github/diff.go index d513d490..81174c2d 100644 --- a/service/github/diff.go +++ b/service/github/diff.go @@ -8,7 +8,7 @@ import ( "os" "os/exec" - "github.com/google/go-github/v60/github" + "github.com/google/go-github/v62/github" "github.com/reviewdog/reviewdog" ) diff --git a/service/github/diff_test.go b/service/github/diff_test.go index 81f02eee..a7b76f1e 100644 --- a/service/github/diff_test.go +++ b/service/github/diff_test.go @@ -9,7 +9,7 @@ import ( "strings" "testing" - "github.com/google/go-github/v60/github" + "github.com/google/go-github/v62/github" ) const sampleDiff = `--- a/sample.old.txt 2016-10-13 05:09:35.820791185 +0900 diff --git a/service/github/github.go b/service/github/github.go index a4a91e43..f25084c2 100644 --- a/service/github/github.go +++ b/service/github/github.go @@ -13,7 +13,7 @@ import ( "strings" "sync" - "github.com/google/go-github/v60/github" + "github.com/google/go-github/v62/github" "google.golang.org/protobuf/proto" "github.com/reviewdog/reviewdog" diff --git a/service/github/github_test.go b/service/github/github_test.go index df25272a..e6f643b4 100644 --- a/service/github/github_test.go +++ b/service/github/github_test.go @@ -12,7 +12,7 @@ import ( "strings" "testing" - "github.com/google/go-github/v60/github" + "github.com/google/go-github/v62/github" "github.com/kylelemons/godebug/pretty" "golang.org/x/oauth2" From 018519ab066aedba9548323c1e840746e4f2f18f Mon Sep 17 00:00:00 2001 From: haya14busa Date: Mon, 1 Jul 2024 23:22:46 +0900 Subject: [PATCH 02/19] Use tip go-github Need https://github.com/google/go-github/pull/3169 --- go.mod | 2 +- go.sum | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 5cbacfeb..4d8b8652 100644 --- a/go.mod +++ b/go.mod @@ -48,7 +48,7 @@ require ( github.com/golang-jwt/jwt/v4 v4.5.0 // indirect github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect github.com/golang/protobuf v1.5.4 // indirect - github.com/google/go-github/v62 v62.0.0 // indirect + github.com/google/go-github/v62 v62.0.1-0.20240626135736-42ae4fe29180 // indirect github.com/google/go-querystring v1.1.0 // indirect github.com/google/s2a-go v0.1.7 // indirect github.com/googleapis/enterprise-certificate-proxy v0.3.2 // indirect diff --git a/go.sum b/go.sum index 677e543e..0d577b21 100644 --- a/go.sum +++ b/go.sum @@ -170,6 +170,8 @@ github.com/google/go-github/v60 v60.0.0 h1:oLG98PsLauFvvu4D/YPxq374jhSxFYdzQGNCy github.com/google/go-github/v60 v60.0.0/go.mod h1:ByhX2dP9XT9o/ll2yXAu2VD8l5eNVg8hD4Cr0S/LmQk= github.com/google/go-github/v62 v62.0.0 h1:/6mGCaRywZz9MuHyw9gD1CwsbmBX8GWsbFkwMmHdhl4= github.com/google/go-github/v62 v62.0.0/go.mod h1:EMxeUqGJq2xRu9DYBMwel/mr7kZrzUOfQmmpYrZn2a4= +github.com/google/go-github/v62 v62.0.1-0.20240626135736-42ae4fe29180 h1:7hwybxRYjKO3kRzxJSW0S+H3b8yBtHF/oUL1ROmCx8Q= +github.com/google/go-github/v62 v62.0.1-0.20240626135736-42ae4fe29180/go.mod h1:EMxeUqGJq2xRu9DYBMwel/mr7kZrzUOfQmmpYrZn2a4= github.com/google/go-querystring v1.1.0 h1:AnCroh3fv4ZBgVIf1Iwtovgjaw/GiKJo8M8yD/fhyJ8= github.com/google/go-querystring v1.1.0/go.mod h1:Kcdr2DB4koayq7X8pmAG4sNG59So17icRSOU623lUBU= github.com/google/martian v2.1.0+incompatible/go.mod h1:9I4somxYTbIHy5NJKHRl3wXiIaQGbYVAs8BPL6v8lEs= From c305c69a9ad8e6991c242fc1b45dec9cc6a2f194 Mon Sep 17 00:00:00 2001 From: haya14busa Date: Mon, 1 Jul 2024 23:27:06 +0900 Subject: [PATCH 03/19] Support posting comment outside diff for github-pr-review reporter --- service/github/github.go | 71 ++++++++++++++++++++++++---------------- 1 file changed, 42 insertions(+), 29 deletions(-) diff --git a/service/github/github.go b/service/github/github.go index f25084c2..8ca72e53 100644 --- a/service/github/github.go +++ b/service/github/github.go @@ -138,14 +138,14 @@ func (g *PullRequest) postAsReviewComment(ctx context.Context) error { rawComments := make([]*reviewdog.Comment, 0, len(postComments)) reviewComments := make([]*github.DraftReviewComment, 0, len(postComments)) remaining := make([]*reviewdog.Comment, 0) - repoBaseHTMLURLForRelatedLoc := "" + repoBaseHTMLURL := "" rootPath, err := serviceutil.GetGitRoot() if err != nil { return err } for _, c := range postComments { - if !c.Result.InDiffContext { - // GitHub Review API cannot report results outside diff. If it's running + if !c.Result.InDiffFile { + // GitHub Review API cannot report results outside diff file. If it's running // in GitHub Actions, fallback to GitHub Actions log as report. if cienv.IsInGitHubAction() { if err := g.logWriter.Post(ctx, c); err != nil { @@ -154,18 +154,17 @@ func (g *PullRequest) postAsReviewComment(ctx context.Context) error { } continue } - if repoBaseHTMLURLForRelatedLoc == "" && len(c.Result.Diagnostic.GetRelatedLocations()) > 0 { + if repoBaseHTMLURL == "" && len(c.Result.Diagnostic.GetRelatedLocations()) > 0 { repo, _, err := g.cli.Repositories.Get(ctx, g.owner, g.repo) if err != nil { return err } - repoBaseHTMLURLForRelatedLoc = repo.GetHTMLURL() + "/blob/" + g.sha + repoBaseHTMLURL = repo.GetHTMLURL() + "/blob/" + g.sha } fprint, err := fingerprint(c.Result.Diagnostic) if err != nil { return err } - body := buildBody(c, repoBaseHTMLURLForRelatedLoc, rootPath, fprint, g.toolName) if g.postedcs.IsPosted(c, githubCommentLine(c), fprint) { // it's already posted. Mark the comment as non-outdated and skip it. delete(g.outdatedComments, fprint) @@ -184,7 +183,8 @@ func (g *PullRequest) postAsReviewComment(ctx context.Context) error { remaining = append(remaining, c) continue } - reviewComments = append(reviewComments, buildDraftReviewComment(c, body)) + reviewComments = append(reviewComments, + buildDraftReviewComment(c, buildBody(c, repoBaseHTMLURL, rootPath, fprint, g.toolName))) } if err := g.logWriter.Flush(ctx); err != nil { return err @@ -247,12 +247,17 @@ func buildDraftReviewComment(c *reviewdog.Comment, body string) *github.DraftRev Path: github.String(loc.GetPath()), Side: github.String("RIGHT"), Body: github.String(body), - Line: github.Int(endLine), } - // GitHub API: Start line must precede the end line. - if startLine < endLine { - r.StartSide = github.String("RIGHT") - r.StartLine = github.Int(startLine) + if endLine > 0 { + r.SubjectType = github.String("line") + r.Line = github.Int(endLine) + // GitHub API: Start line must precede the end line. + if startLine < endLine { + r.StartSide = github.String("RIGHT") + r.StartLine = github.Int(startLine) + } + } else { + r.SubjectType = github.String("file") } return r } @@ -325,15 +330,8 @@ func (g *PullRequest) setPostedComment(ctx context.Context) error { if id := c.GetInReplyTo(); id != 0 { g.prCommentWithReply[id] = true } - if c.Line == nil || c.Path == nil || c.Body == nil || c.SubjectType == nil { - continue - } - var line int - if c.GetSubjectType() == "line" { - line = c.GetLine() - } if meta := extractMetaComment(c.GetBody()); meta != nil { - g.postedcs.AddPostedComment(c.GetPath(), line, meta.GetFingerprint()) + g.postedcs.AddPostedComment(c.GetPath(), c.GetLine(), meta.GetFingerprint()) if meta.SourceName == g.toolName { g.outdatedComments[meta.GetFingerprint()] = c // Remove non-outdated comment later. } @@ -424,27 +422,42 @@ func listAllPullRequestsComments(ctx context.Context, cli *github.Client, return append(comments, restComments...), nil } -func buildBody(c *reviewdog.Comment, baseRelatedLocURL string, gitRootPath string, fprint string, toolName string) string { +func buildBody(c *reviewdog.Comment, baseURL string, gitRootPath string, fprint string, toolName string) string { cbody := commentutil.MarkdownComment(c) - if suggestion := buildSuggestions(c); suggestion != "" { - cbody += "\n" + suggestion + if c.Result.InDiffContext { + if suggestion := buildSuggestions(c); suggestion != "" { + cbody += "\n" + suggestion + } + } else { + if c.Result.Diagnostic.GetLocation().GetRange().GetStart().GetLine() > 0 { + snippetURL := githubCodeSnippetURL(baseURL, gitRootPath, c.Result.Diagnostic.GetLocation()) + cbody = snippetURL + "\n\n" + cbody + } } for _, relatedLoc := range c.Result.Diagnostic.GetRelatedLocations() { loc := relatedLoc.GetLocation() if loc.GetPath() == "" || loc.GetRange().GetStart().GetLine() == 0 { continue } - relPath := pathutil.NormalizePath(loc.GetPath(), gitRootPath, "") - relatedURL := fmt.Sprintf("%s/%s#L%d", baseRelatedLocURL, relPath, loc.GetRange().GetStart().GetLine()) - if endLine := loc.GetRange().GetEnd().GetLine(); endLine > 0 { - relatedURL += fmt.Sprintf("-L%d", endLine) - } - cbody += "\n
\n\n" + relatedLoc.GetMessage() + "\n" + relatedURL + snippetURL := githubCodeSnippetURL(baseURL, gitRootPath, loc) + cbody += "\n
\n\n" + relatedLoc.GetMessage() + "\n" + snippetURL } cbody += fmt.Sprintf("\n\n", BuildMetaComment(fprint, toolName)) return cbody } +func githubCodeSnippetURL(baseURL, gitRootPath string, loc *rdf.Location) string { + relPath := pathutil.NormalizePath(loc.GetPath(), gitRootPath, "") + relatedURL := fmt.Sprintf("%s/%s", baseURL, relPath) + if startLine := loc.GetRange().GetStart().GetLine(); startLine > 0 { + relatedURL += fmt.Sprintf("#L%d", startLine) + } + if endLine := loc.GetRange().GetEnd().GetLine(); endLine > 0 { + relatedURL += fmt.Sprintf("-L%d", endLine) + } + return relatedURL +} + func BuildMetaComment(fprint string, toolName string) string { b, _ := proto.Marshal( &metacomment.MetaComment{ From d939ff8b7823e870c4eb28fb44ebde51c2179a11 Mon Sep 17 00:00:00 2001 From: haya14busa Date: Mon, 1 Jul 2024 23:28:24 +0900 Subject: [PATCH 04/19] use -filter-mode=nofilter for github-pr-review test --- .github/workflows/reviewdog.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/reviewdog.yml b/.github/workflows/reviewdog.yml index 0c549f90..8379d097 100644 --- a/.github/workflows/reviewdog.yml +++ b/.github/workflows/reviewdog.yml @@ -151,7 +151,7 @@ jobs: REVIEWDOG_GITHUB_API_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | cat ./_testdata/custom_rdjson.json | \ - reviewdog -name="custom-rdjson" -f=rdjson -reporter=github-pr-review + reviewdog -name="custom-rdjson" -f=rdjson -reporter=github-pr-review -filter-mode=nofilter - name: gofmt -s with reviewdog env: From 6a43e6a2e8d9d3a72f62a20c9fe659ec78b95183 Mon Sep 17 00:00:00 2001 From: haya14busa Date: Mon, 1 Jul 2024 23:33:09 +0900 Subject: [PATCH 05/19] Add new line to test data text --- _testdata/custom.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/_testdata/custom.txt b/_testdata/custom.txt index 07a3770d..05253f7f 100644 --- a/_testdata/custom.txt +++ b/_testdata/custom.txt @@ -17,3 +17,5 @@ function f() } RelatedLoc + +new line From 5b4d3e103d9f288abb2c85e1492662cec653c0e6 Mon Sep 17 00:00:00 2001 From: haya14busa Date: Tue, 2 Jul 2024 00:46:17 +0900 Subject: [PATCH 06/19] Use a pull request comment as a file comment appropriately --- service/github/github.go | 68 +++++++++++++++++++++++++++++++--------- 1 file changed, 54 insertions(+), 14 deletions(-) diff --git a/service/github/github.go b/service/github/github.go index 8ca72e53..31a78ab9 100644 --- a/service/github/github.go +++ b/service/github/github.go @@ -31,6 +31,8 @@ var _ reviewdog.DiffService = (*PullRequest)(nil) const maxCommentsPerRequest = 30 +const maxFileComments = 10 + const ( invalidSuggestionPre = "
reviewdog suggestion error" invalidSuggestionPost = "
" @@ -137,6 +139,7 @@ func (g *PullRequest) postAsReviewComment(ctx context.Context) error { g.postComments = nil rawComments := make([]*reviewdog.Comment, 0, len(postComments)) reviewComments := make([]*github.DraftReviewComment, 0, len(postComments)) + fileComments := make([]*github.PullRequestComment, 0) remaining := make([]*reviewdog.Comment, 0) repoBaseHTMLURL := "" rootPath, err := serviceutil.GetGitRoot() @@ -171,26 +174,35 @@ func (g *PullRequest) postAsReviewComment(ctx context.Context) error { 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. - // - // > 403 You have triggered an abuse detection mechanism and have been - // > temporarily blocked from content creation. Please retry your request - // > again later. - // https://docs.github.com/en/rest/overview/resources-in-the-rest-api?apiVersion=2022-11-28#rate-limiting - if len(reviewComments) >= maxCommentsPerRequest { - remaining = append(remaining, c) - continue + if c.Result.InDiffContext { + // 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. + // + // > 403 You have triggered an abuse detection mechanism and have been + // > temporarily blocked from content creation. Please retry your request + // > again later. + // https://docs.github.com/en/rest/overview/resources-in-the-rest-api?apiVersion=2022-11-28#rate-limiting + if len(reviewComments) >= maxCommentsPerRequest { + remaining = append(remaining, c) + continue + } + comment := buildDraftReviewComment(c, buildBody(c, repoBaseHTMLURL, rootPath, fprint, g.toolName)) + reviewComments = append(reviewComments, comment) + } else { + if len(fileComments) >= maxFileComments { + remaining = append(remaining, c) + continue + } + comment := buildPullRequestComment(c, buildBody(c, repoBaseHTMLURL, rootPath, fprint, g.toolName)) + fileComments = append(fileComments, comment) } - reviewComments = append(reviewComments, - buildDraftReviewComment(c, buildBody(c, repoBaseHTMLURL, rootPath, fprint, g.toolName))) } if err := g.logWriter.Flush(ctx); err != nil { return err } - if len(reviewComments) > 0 { + if len(reviewComments) > 0 || len(remaining) > 0 { // send review comments to GitHub. review := &github.PullRequestReviewRequest{ CommitID: &g.sha, @@ -209,6 +221,17 @@ func (g *PullRequest) postAsReviewComment(ctx context.Context) error { return err } } + for _, c := range fileComments { + if _, _, err := g.cli.PullRequests.CreateComment(ctx, g.owner, g.repo, g.pr, c); 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 + } + } for _, c := range g.outdatedComments { if ok := g.prCommentWithReply[c.GetID()]; ok { @@ -247,6 +270,23 @@ func buildDraftReviewComment(c *reviewdog.Comment, body string) *github.DraftRev Path: github.String(loc.GetPath()), Side: github.String("RIGHT"), Body: github.String(body), + Line: github.Int(endLine), + } + // GitHub API: Start line must precede the end line. + if startLine < endLine { + r.StartSide = github.String("RIGHT") + r.StartLine = github.Int(startLine) + } + return r +} + +func buildPullRequestComment(c *reviewdog.Comment, body string) *github.PullRequestComment { + loc := c.Result.Diagnostic.GetLocation() + startLine, endLine := githubCommentLineRange(c) + r := &github.PullRequestComment{ + Path: github.String(loc.GetPath()), + Side: github.String("RIGHT"), + Body: github.String(body), } if endLine > 0 { r.SubjectType = github.String("line") From 3aaac1036d7b351380a5620609648076263becc7 Mon Sep 17 00:00:00 2001 From: haya14busa Date: Tue, 2 Jul 2024 00:52:26 +0900 Subject: [PATCH 07/19] Fill in commit_id --- service/github/github.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/service/github/github.go b/service/github/github.go index 31a78ab9..b0891e2c 100644 --- a/service/github/github.go +++ b/service/github/github.go @@ -194,7 +194,7 @@ func (g *PullRequest) postAsReviewComment(ctx context.Context) error { remaining = append(remaining, c) continue } - comment := buildPullRequestComment(c, buildBody(c, repoBaseHTMLURL, rootPath, fprint, g.toolName)) + comment := buildPullRequestComment(c, buildBody(c, repoBaseHTMLURL, rootPath, fprint, g.toolName), g.sha) fileComments = append(fileComments, comment) } } @@ -280,13 +280,14 @@ func buildDraftReviewComment(c *reviewdog.Comment, body string) *github.DraftRev return r } -func buildPullRequestComment(c *reviewdog.Comment, body string) *github.PullRequestComment { +func buildPullRequestComment(c *reviewdog.Comment, body string, sha string) *github.PullRequestComment { loc := c.Result.Diagnostic.GetLocation() startLine, endLine := githubCommentLineRange(c) r := &github.PullRequestComment{ - Path: github.String(loc.GetPath()), - Side: github.String("RIGHT"), - Body: github.String(body), + Path: github.String(loc.GetPath()), + Side: github.String("RIGHT"), + Body: github.String(body), + CommitID: github.String(sha), } if endLine > 0 { r.SubjectType = github.String("line") From 92151d290bbba1efb98706887fc1b2dbe52300a1 Mon Sep 17 00:00:00 2001 From: haya14busa Date: Tue, 2 Jul 2024 00:58:57 +0900 Subject: [PATCH 08/19] fix pull request file comment --- service/github/github.go | 29 ++++++++--------------------- 1 file changed, 8 insertions(+), 21 deletions(-) diff --git a/service/github/github.go b/service/github/github.go index b0891e2c..fa50aed4 100644 --- a/service/github/github.go +++ b/service/github/github.go @@ -194,7 +194,7 @@ func (g *PullRequest) postAsReviewComment(ctx context.Context) error { remaining = append(remaining, c) continue } - comment := buildPullRequestComment(c, buildBody(c, repoBaseHTMLURL, rootPath, fprint, g.toolName), g.sha) + comment := buildPullRequestFileComment(c, buildBody(c, repoBaseHTMLURL, rootPath, fprint, g.toolName), g.sha) fileComments = append(fileComments, comment) } } @@ -280,27 +280,14 @@ func buildDraftReviewComment(c *reviewdog.Comment, body string) *github.DraftRev return r } -func buildPullRequestComment(c *reviewdog.Comment, body string, sha string) *github.PullRequestComment { - loc := c.Result.Diagnostic.GetLocation() - startLine, endLine := githubCommentLineRange(c) - r := &github.PullRequestComment{ - Path: github.String(loc.GetPath()), - Side: github.String("RIGHT"), - Body: github.String(body), - CommitID: github.String(sha), - } - if endLine > 0 { - r.SubjectType = github.String("line") - r.Line = github.Int(endLine) - // GitHub API: Start line must precede the end line. - if startLine < endLine { - r.StartSide = github.String("RIGHT") - r.StartLine = github.Int(startLine) - } - } else { - r.SubjectType = github.String("file") +func buildPullRequestFileComment(c *reviewdog.Comment, body string, sha string) *github.PullRequestComment { + return &github.PullRequestComment{ + Path: github.String(c.Result.Diagnostic.GetLocation().GetPath()), + Side: github.String("RIGHT"), + Body: github.String(body), + CommitID: github.String(sha), + SubjectType: github.String("file"), } - return r } // line represents end line if it's a multiline comment in GitHub, otherwise From 3e1916abc6eb8f6b26ae56773d0acbcb6ddb87c8 Mon Sep 17 00:00:00 2001 From: haya14busa Date: Tue, 2 Jul 2024 01:05:20 +0900 Subject: [PATCH 09/19] fix empty repoBaseHTMLURL --- service/github/github.go | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/service/github/github.go b/service/github/github.go index fa50aed4..3f41fc94 100644 --- a/service/github/github.go +++ b/service/github/github.go @@ -141,11 +141,14 @@ func (g *PullRequest) postAsReviewComment(ctx context.Context) error { reviewComments := make([]*github.DraftReviewComment, 0, len(postComments)) fileComments := make([]*github.PullRequestComment, 0) remaining := make([]*reviewdog.Comment, 0) - repoBaseHTMLURL := "" rootPath, err := serviceutil.GetGitRoot() if err != nil { return err } + repoBaseHTMLURL, err := g.repoBaseHTMLURL(ctx) + if err != nil { + return err + } for _, c := range postComments { if !c.Result.InDiffFile { // GitHub Review API cannot report results outside diff file. If it's running @@ -157,13 +160,6 @@ func (g *PullRequest) postAsReviewComment(ctx context.Context) error { } continue } - if repoBaseHTMLURL == "" && len(c.Result.Diagnostic.GetRelatedLocations()) > 0 { - repo, _, err := g.cli.Repositories.Get(ctx, g.owner, g.repo) - if err != nil { - return err - } - repoBaseHTMLURL = repo.GetHTMLURL() + "/blob/" + g.sha - } fprint, err := fingerprint(c.Result.Diagnostic) if err != nil { return err @@ -414,6 +410,14 @@ func (g *PullRequest) Strip() int { return 1 } +func (g *PullRequest) repoBaseHTMLURL(ctx context.Context) (string, error) { + repo, _, err := g.cli.Repositories.Get(ctx, g.owner, g.repo) + if err != nil { + return "", fmt.Errorf("failed to build repo base HTML URL: %w", err) + } + return repo.GetHTMLURL() + "/blob/" + g.sha, nil +} + func (g *PullRequest) comment(ctx context.Context) ([]*github.PullRequestComment, error) { // https://developer.github.com/v3/guides/traversing-with-pagination/ opts := &github.PullRequestListCommentsOptions{ From c94b589d19813c09eaecf72216b6cf477267b8a9 Mon Sep 17 00:00:00 2001 From: haya14busa Date: Tue, 2 Jul 2024 01:09:05 +0900 Subject: [PATCH 10/19] Write snippet URL below body --- service/github/github.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/service/github/github.go b/service/github/github.go index 3f41fc94..487e2741 100644 --- a/service/github/github.go +++ b/service/github/github.go @@ -463,7 +463,7 @@ func buildBody(c *reviewdog.Comment, baseURL string, gitRootPath string, fprint } else { if c.Result.Diagnostic.GetLocation().GetRange().GetStart().GetLine() > 0 { snippetURL := githubCodeSnippetURL(baseURL, gitRootPath, c.Result.Diagnostic.GetLocation()) - cbody = snippetURL + "\n\n" + cbody + cbody += "\n\n" + snippetURL } } for _, relatedLoc := range c.Result.Diagnostic.GetRelatedLocations() { From 8abd05d691fb3f500526f77bbe19c64640e6376d Mon Sep 17 00:00:00 2001 From: haya14busa Date: Tue, 2 Jul 2024 01:10:11 +0900 Subject: [PATCH 11/19] Add test file comment --- _testdata/custom_rdjson.json | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/_testdata/custom_rdjson.json b/_testdata/custom_rdjson.json index 8ab744db..0c4a0894 100644 --- a/_testdata/custom_rdjson.json +++ b/_testdata/custom_rdjson.json @@ -249,6 +249,12 @@ } } ] + }, + { + "message": "test file comment", + "location": { + "path": "_testdata/custom.txt" + } } ] } From 74524d7158361421ff7f441d27ae9e373bc5d058 Mon Sep 17 00:00:00 2001 From: haya14busa Date: Tue, 2 Jul 2024 01:19:58 +0900 Subject: [PATCH 12/19] Use -filter-mode=file for github-pr-annotations test --- .github/workflows/reviewdog.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/reviewdog.yml b/.github/workflows/reviewdog.yml index 8379d097..e9421b9c 100644 --- a/.github/workflows/reviewdog.yml +++ b/.github/workflows/reviewdog.yml @@ -231,7 +231,7 @@ jobs: REVIEWDOG_GITHUB_API_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | cat ./_testdata/custom_rdjson.json | \ - reviewdog -name="custom-rdjson" -f=rdjson -reporter=github-pr-annotations + reviewdog -name="custom-rdjson" -f=rdjson -reporter=github-pr-annotations -filter-mode=file reviewdog-sarif: permissions: From 215f5979765c1a1c839b28eee1f56c1adbc835e7 Mon Sep 17 00:00:00 2001 From: haya14busa Date: Tue, 2 Jul 2024 19:07:01 +0900 Subject: [PATCH 13/19] Use code snippet for remaining findings report --- service/github/github.go | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/service/github/github.go b/service/github/github.go index 487e2741..10873b63 100644 --- a/service/github/github.go +++ b/service/github/github.go @@ -204,7 +204,7 @@ func (g *PullRequest) postAsReviewComment(ctx context.Context) error { CommitID: &g.sha, Event: github.String("COMMENT"), Comments: reviewComments, - Body: github.String(g.remainingCommentsSummary(remaining)), + Body: github.String(g.remainingCommentsSummary(remaining, repoBaseHTMLURL, rootPath)), } _, _, err := g.cli.PullRequests.CreateReview(ctx, g.owner, g.repo, g.pr, review) if err != nil { @@ -317,7 +317,7 @@ func githubCommentLineRange(c *reviewdog.Comment) (start int, end int) { return int(startLine), int(endLine) } -func (g *PullRequest) remainingCommentsSummary(remaining []*reviewdog.Comment) string { +func (g *PullRequest) remainingCommentsSummary(remaining []*reviewdog.Comment, baseURL string, gitRootPath string) string { if len(remaining) == 0 { return "" } @@ -333,7 +333,14 @@ func (g *PullRequest) remainingCommentsSummary(remaining []*reviewdog.Comment) s sb.WriteString(fmt.Sprintf("%s\n", tool)) sb.WriteString("\n") for _, c := range comments { - sb.WriteString(githubutils.LinkedMarkdownDiagnostic(g.owner, g.repo, g.sha, c.Result.Diagnostic)) + sb.WriteString("
") + sb.WriteString("\n") + sb.WriteString("\n") + sb.WriteString(commentutil.MarkdownComment(c)) + sb.WriteString("\n") + sb.WriteString("\n") + sb.WriteString(githubCodeSnippetURL(baseURL, gitRootPath, c.Result.Diagnostic.GetLocation())) + sb.WriteString("\n") sb.WriteString("\n") } sb.WriteString("\n") From 51684d25677525f115c11ac085577f4dc80eee44 Mon Sep 17 00:00:00 2001 From: haya14busa Date: Tue, 2 Jul 2024 19:14:50 +0900 Subject: [PATCH 14/19] empty commit From f7a37bcb05f5794b6347767bcbf8082bd8d75d44 Mon Sep 17 00:00:00 2001 From: haya14busa Date: Tue, 2 Jul 2024 19:27:30 +0900 Subject: [PATCH 15/19] fix githubCommentLine for FILE comment --- service/github/github.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/service/github/github.go b/service/github/github.go index 10873b63..086dfad7 100644 --- a/service/github/github.go +++ b/service/github/github.go @@ -291,7 +291,7 @@ func buildPullRequestFileComment(c *reviewdog.Comment, body string, sha string) // 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 + return 1 // GitHub returns line==1 for FILE comment. } _, end := githubCommentLineRange(c) return end From 2095c79c7501c33e383675b44e7ad7a343db5848 Mon Sep 17 00:00:00 2001 From: haya14busa Date: Tue, 2 Jul 2024 21:58:52 +0900 Subject: [PATCH 16/19] fix tests --- service/github/github_test.go | 62 +++++++++++++++++++++++++++++++---- 1 file changed, 56 insertions(+), 6 deletions(-) diff --git a/service/github/github_test.go b/service/github/github_test.go index e6f643b4..2cb9569b 100644 --- a/service/github/github_test.go +++ b/service/github/github_test.go @@ -16,6 +16,7 @@ import ( "github.com/kylelemons/godebug/pretty" "golang.org/x/oauth2" + "github.com/google/go-cmp/cmp" "github.com/reviewdog/reviewdog" "github.com/reviewdog/reviewdog/filter" "github.com/reviewdog/reviewdog/proto/rdf" @@ -132,7 +133,8 @@ func TestGitHubPullRequest_Post_Flush_review_api(t *testing.T) { defer setupEnvs()() listCommentsAPICalled := 0 - postCommentsAPICalled := 0 + postReviewCommentAPICalled := 0 + postPullRequestCommentAPICalled := 0 repoAPICalled := 0 delCommentsAPICalled := 0 mux := http.NewServeMux() @@ -193,13 +195,13 @@ func TestGitHubPullRequest_Post_Flush_review_api(t *testing.T) { { Path: github.String("reviewdog.go"), Line: github.Int(1), - Body: github.String(commentutil.BodyPrefix + "existing file comment (no-line)"), + Body: github.String(commentutil.BodyPrefix + "existing file comment (no-line)" + "\n\n"), SubjectType: github.String("file"), }, { Path: github.String("reviewdog.go"), Line: github.Int(1), - Body: github.String(commentutil.BodyPrefix + "existing file comment (outside diff-context)"), + Body: github.String(commentutil.BodyPrefix + "existing file comment (outside diff-context)" + "\n\n"), SubjectType: github.String("file"), }, } @@ -207,12 +209,43 @@ func TestGitHubPullRequest_Post_Flush_review_api(t *testing.T) { t.Fatal(err) } } + case http.MethodPost: + postPullRequestCommentAPICalled++ + var req github.PullRequestComment + if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + t.Error(err) + } + expects := []github.PullRequestComment{ + { + Body: github.String("reported by [reviewdog](https://github.com/reviewdog/reviewdog) :dog:
file comment (no-line)\n\n"), + Path: github.String("reviewdog.go"), + Side: github.String("RIGHT"), + CommitID: github.String("sha"), + SubjectType: github.String("file"), + }, + { + Body: github.String(`reported by [reviewdog](https://github.com/reviewdog/reviewdog) :dog:
file comment (outside diff-context) + +https://test/repo/path/blob/sha/reviewdog.go#L18 + +`), + Path: github.String("reviewdog.go"), + Side: github.String("RIGHT"), + CommitID: github.String("sha"), + SubjectType: github.String("file"), + }, + {}, + } + want := expects[postPullRequestCommentAPICalled-1] + if diff := cmp.Diff(req, want); diff != "" { + t.Errorf("result has diff (API call: %d):\n%s", postPullRequestCommentAPICalled, 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) { - postCommentsAPICalled++ + postReviewCommentAPICalled++ if r.Method != http.MethodPost { t.Errorf("unexpected access: %v %v", r.Method, r.URL) } @@ -1253,8 +1286,11 @@ func TestGitHubPullRequest_Post_Flush_review_api(t *testing.T) { if listCommentsAPICalled != 2 { t.Errorf("GitHub List PullRequest comments API called %v times, want 2 times", listCommentsAPICalled) } - if postCommentsAPICalled != 1 { - t.Errorf("GitHub post PullRequest comments API called %v times, want 1 times", postCommentsAPICalled) + if postReviewCommentAPICalled != 1 { + t.Errorf("GitHub post Review comments API called %v times, want 1 times", postReviewCommentAPICalled) + } + if postPullRequestCommentAPICalled != 2 { + t.Errorf("GitHub post PullRequest comments API called %v times, want 2 times", postPullRequestCommentAPICalled) } if repoAPICalled != 1 { t.Errorf("GitHub Repository API called %v times, want 1 times", repoAPICalled) @@ -1290,6 +1326,13 @@ func TestGitHubPullRequest_Post_toomany(t *testing.T) { t.Errorf("PullRequestReviewRequest.Body is empty but want some summary text") } }) + mux.HandleFunc("/repos/o/r", func(w http.ResponseWriter, r *http.Request) { + if err := json.NewEncoder(w).Encode(&github.Repository{ + HTMLURL: github.String("https://test/repo/path"), + }); err != nil { + t.Fatal(err) + } + }) ts := httptest.NewServer(mux) defer ts.Close() @@ -1392,6 +1435,13 @@ func TestGitHubPullRequest_Post_NoPermission(t *testing.T) { postCommentsAPICalled++ w.WriteHeader(http.StatusNotFound) }) + mux.HandleFunc("/repos/o/r", func(w http.ResponseWriter, r *http.Request) { + if err := json.NewEncoder(w).Encode(&github.Repository{ + HTMLURL: github.String("https://test/repo/path"), + }); err != nil { + t.Fatal(err) + } + }) ts := httptest.NewServer(mux) defer ts.Close() From b8e365aefcacadeb569724a32079cb19a8bd9d91 Mon Sep 17 00:00:00 2001 From: haya14busa Date: Tue, 2 Jul 2024 22:00:15 +0900 Subject: [PATCH 17/19] remove -filter-mode=file for github-pr-annotations test --- .github/workflows/reviewdog.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/reviewdog.yml b/.github/workflows/reviewdog.yml index e9421b9c..8379d097 100644 --- a/.github/workflows/reviewdog.yml +++ b/.github/workflows/reviewdog.yml @@ -231,7 +231,7 @@ jobs: REVIEWDOG_GITHUB_API_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | cat ./_testdata/custom_rdjson.json | \ - reviewdog -name="custom-rdjson" -f=rdjson -reporter=github-pr-annotations -filter-mode=file + reviewdog -name="custom-rdjson" -f=rdjson -reporter=github-pr-annotations reviewdog-sarif: permissions: From 8a2b40e3df97fb91fb87b340472a11575120bb71 Mon Sep 17 00:00:00 2001 From: haya14busa Date: Tue, 2 Jul 2024 22:03:19 +0900 Subject: [PATCH 18/19] update CHANGELOG --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e0adc1f8..e598c92b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,7 +21,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - [#1807](https://github.com/reviewdog/reviewdog/pull/1807) Add -reporter=sarif which outputs SARIF format to stdout. You can upload the output SARIF to GitHub and see code scanning alerts. - +- [#1809](https://github.com/reviewdog/reviewdog/pull/1809) Support posting + comments outside diff context as a file comment in github-pr-review reporter ### :bug: Fixes - ... From b20b457176fe2eaa533f2be9faadd4f72bafa470 Mon Sep 17 00:00:00 2001 From: haya14busa Date: Wed, 3 Jul 2024 21:47:32 +0900 Subject: [PATCH 19/19] fix go-github ver in go.mod --- go.mod | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/go.mod b/go.mod index 4d8b8652..2a994069 100644 --- a/go.mod +++ b/go.mod @@ -9,7 +9,7 @@ require ( contrib.go.opencensus.io/exporter/stackdriver v0.13.12 github.com/bradleyfalzon/ghinstallation/v2 v2.11.0 github.com/google/go-cmp v0.6.0 - github.com/google/go-github/v60 v60.0.0 + github.com/google/go-github/v62 v62.0.0 github.com/haya14busa/go-actions-toolkit v0.0.0-20200105081403-ca0307860f01 github.com/haya14busa/go-sarif v0.0.0-20240630170108-a3ba8d79599f github.com/haya14busa/secretbox v0.0.0-20180525171038-07c7ecf409f5 @@ -48,7 +48,6 @@ require ( github.com/golang-jwt/jwt/v4 v4.5.0 // indirect github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect github.com/golang/protobuf v1.5.4 // indirect - github.com/google/go-github/v62 v62.0.1-0.20240626135736-42ae4fe29180 // indirect github.com/google/go-querystring v1.1.0 // indirect github.com/google/s2a-go v0.1.7 // indirect github.com/googleapis/enterprise-certificate-proxy v0.3.2 // indirect