Repository: reviewdog/reviewdog (go)

Issue:
[github-pr-review] Mark outdated review comments as obsolete or delete such comments
With github-pr-review, if a reported line is changed to fix the reported problem, GitHub marks the comment as outdated. However, if the fix doesn't change the reported line, the previously reported comment still remains as is.

github-pr-review reporter fetches reported comments first to prevent posting the same comments, so it should be possible to detect which comments are fixed by comparing the new reported results.

It should be better to resolve fixed reported results (or mark them as obsolete or delete such comments?).

IMPACT: This problem can happen  for type check based on comment. e.g. phpstan, jsdoc type check of Typescript.

Just for reference  (in Japanese though): https://twitter.com/about_hiroppy/status/1258699418796097537

Pull request description:
Support deleting outdated comments in github-pr-review reporter
- [x] Updated Unreleased section in [CHANGELOG](https://github.com/reviewdog/reviewdog/blob/master/CHANGELOG.md) or it's not notable changes.


Fix #568


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 9c1ad0af..010a9046 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -9,6 +9,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### :sparkles: Release Note <!-- optional -->
 
 ### :rocket: Enhancements
+- [#1804](https://github.com/reviewdog/reviewdog/pull/1804) Support deleting
+  outdated comments in github-pr-review reporter. Note that it won't delete
+  comments if there is a reply considering there can be a meaningful
+  discussion.
 - [#1806](https://github.com/reviewdog/reviewdog/pull/1806) Add -reporter=rdjson/rdjsonl which outputs rdjson/rdjsonl format to stdout. It also changes the default behavior of -diff and -filter-mode for local reporters. If -filter-mode is not provided (-filter-mode=default) and -diff flag is not provided, reviewdog automatically set -filter-mode=nofilter.
 
 ### :bug: Fixes
diff --git a/cmd/reviewdog/main.go b/cmd/reviewdog/main.go
index 35bde682..c3509f11 100644
--- a/cmd/reviewdog/main.go
+++ b/cmd/reviewdog/main.go
@@ -647,7 +647,7 @@ func githubService(ctx context.Context, opt *option) (gs *githubservice.PullRequ
 		g.PullRequest = prID
 	}
 
-	gs, err = githubservice.NewGitHubPullRequest(client, g.Owner, g.Repo, g.PullRequest, g.SHA, opt.level)
+	gs, err = githubservice.NewGitHubPullRequest(client, g.Owner, g.Repo, g.PullRequest, g.SHA, opt.level, toolName(opt))
 	if err != nil {
 		return nil, false, err
 	}
diff --git a/scripts/encode-metacomment/main.go b/scripts/encode-metacomment/main.go
new file mode 100644
index 00000000..5e7e91f6
--- /dev/null
+++ b/scripts/encode-metacomment/main.go
@@ -0,0 +1,21 @@
+package main
+
+import (
+	"flag"
+	"fmt"
+	"os"
+
+	"github.com/reviewdog/reviewdog/service/github"
+)
+
+var fprint = flag.String("fprint", "", "fingerprint")
+var toolName = flag.String("tool-name", "", "tool-name")
+
+func main() {
+	flag.Parse()
+	if *fprint == "" || *toolName == "" {
+		fmt.Println("Set both -fprint and -tool-name flags")
+		os.Exit(1)
+	}
+	fmt.Println(github.BuildMetaComment(*fprint, *toolName))
+}
diff --git a/service/github/github.go b/service/github/github.go
index 51188ee9..a4a91e43 100644
--- a/service/github/github.go
+++ b/service/github/github.go
@@ -52,18 +52,21 @@ func isPermissionError(err error) bool {
 //	https://docs.github.com/en/rest/pulls/comments?apiVersion=2022-11-28#create-a-review-comment-for-a-pull-request
 //	POST /repos/:owner/:repo/pulls/:number/comments
 type PullRequest struct {
-	cli   *github.Client
-	owner string
-	repo  string
-	pr    int
-	sha   string
+	cli      *github.Client
+	owner    string
+	repo     string
+	pr       int
+	sha      string
+	toolName string
 
 	muComments    sync.Mutex
 	postComments  []*reviewdog.Comment
 	logWriter     *githubutils.GitHubActionLogWriter
 	fallbackToLog bool
 
-	postedcs commentutil.PostedComments
+	postedcs           commentutil.PostedComments
+	outdatedComments   map[string]*github.PullRequestComment // fingerprint -> comment
+	prCommentWithReply map[int64]bool                        // review id -> bool
 
 	// wd is working directory relative to root of repository.
 	wd string
@@ -79,7 +82,7 @@ type PullRequest struct {
 //
 // [1]: https://docs.github.com/en/actions/security-guides/automatic-token-authentication#permissions-for-the-github_token
 // [2]: https://docs.github.com/en/actions/reference/workflow-commands-for-github-actions
-func NewGitHubPullRequest(cli *github.Client, owner, repo string, pr int, sha, level string) (*PullRequest, error) {
+func NewGitHubPullRequest(cli *github.Client, owner, repo string, pr int, sha, level, toolName string) (*PullRequest, error) {
 	workDir, err := serviceutil.GitRelWorkdir()
 	if err != nil {
 		return nil, fmt.Errorf("PullRequest needs 'git' command: %w", err)
@@ -90,6 +93,7 @@ func NewGitHubPullRequest(cli *github.Client, owner, repo string, pr int, sha, l
 		repo:      repo,
 		pr:        pr,
 		sha:       sha,
+		toolName:  toolName,
 		logWriter: githubutils.NewGitHubActionLogWriter(level),
 		wd:        workDir,
 	}, nil
@@ -161,9 +165,10 @@ func (g *PullRequest) postAsReviewComment(ctx context.Context) error {
 		if err != nil {
 			return err
 		}
-		body := buildBody(c, repoBaseHTMLURLForRelatedLoc, rootPath, fprint)
+		body := buildBody(c, repoBaseHTMLURLForRelatedLoc, rootPath, fprint, g.toolName)
 		if g.postedcs.IsPosted(c, githubCommentLine(c), fprint) {
-			// it's already posted. skip it.
+			// it's already posted. Mark the comment as non-outdated and skip it.
+			delete(g.outdatedComments, fprint)
 			continue
 		}
 
@@ -205,6 +210,16 @@ func (g *PullRequest) postAsReviewComment(ctx context.Context) error {
 		}
 	}
 
+	for _, c := range g.outdatedComments {
+		if ok := g.prCommentWithReply[c.GetID()]; ok {
+			// Do not remove comment with replies.
+			continue
+		}
+		if _, err := g.cli.PullRequests.DeleteComment(ctx, g.owner, g.repo, c.GetID()); err != nil {
+			return fmt.Errorf("failed to delete comment (id=%d): %w", c.GetID(), err)
+		}
+	}
+
 	return nil
 
 FALLBACK:
@@ -300,11 +315,16 @@ func (g *PullRequest) remainingCommentsSummary(remaining []*reviewdog.Comment) s
 // setPostedComment get posted comments from GitHub.
 func (g *PullRequest) setPostedComment(ctx context.Context) error {
 	g.postedcs = make(commentutil.PostedComments)
+	g.outdatedComments = make(map[string]*github.PullRequestComment)
+	g.prCommentWithReply = make(map[int64]bool)
 	cs, err := g.comment(ctx)
 	if err != nil {
 		return err
 	}
 	for _, c := range cs {
+		if id := c.GetInReplyTo(); id != 0 {
+			g.prCommentWithReply[id] = true
+		}
 		if c.Line == nil || c.Path == nil || c.Body == nil || c.SubjectType == nil {
 			continue
 		}
@@ -314,6 +334,9 @@ func (g *PullRequest) setPostedComment(ctx context.Context) error {
 		}
 		if meta := extractMetaComment(c.GetBody()); meta != nil {
 			g.postedcs.AddPostedComment(c.GetPath(), line, meta.GetFingerprint())
+			if meta.SourceName == g.toolName {
+				g.outdatedComments[meta.GetFingerprint()] = c // Remove non-outdated comment later.
+			}
 		}
 	}
 	return nil
@@ -401,7 +424,7 @@ func listAllPullRequestsComments(ctx context.Context, cli *github.Client,
 	return append(comments, restComments...), nil
 }
 
-func buildBody(c *reviewdog.Comment, baseRelatedLocURL string, gitRootPath string, fprint string) string {
+func buildBody(c *reviewdog.Comment, baseRelatedLocURL string, gitRootPath string, fprint string, toolName string) string {
 	cbody := commentutil.MarkdownComment(c)
 	if suggestion := buildSuggestions(c); suggestion != "" {
 		cbody += "\n" + suggestion
@@ -418,11 +441,11 @@ func buildBody(c *reviewdog.Comment, baseRelatedLocURL string, gitRootPath strin
 		}
 		cbody += "\n<hr>\n\n" + relatedLoc.GetMessage() + "\n" + relatedURL
 	}
-	cbody += fmt.Sprintf("\n<!-- __reviewdog__:%s -->\n", buildMetaComment(fprint, c.ToolName))
+	cbody += fmt.Sprintf("\n<!-- __reviewdog__:%s -->\n", BuildMetaComment(fprint, toolName))
 	return cbody
 }
 
-func buildMetaComment(fprint string, toolName string) string {
+func BuildMetaComment(fprint string, toolName string) string {
 	b, _ := proto.Marshal(
 		&metacomment.MetaComment{
 			Fingerprint: fprint,


Test patch:
diff --git a/_testdata/custom.txt b/_testdata/custom.txt
index 1a0ddc66..07a3770d 100644
--- a/_testdata/custom.txt
+++ b/_testdata/custom.txt
@@ -1,4 +1,4 @@
-Custom text for testing
+Custom text for testing!
 
 Line 3: Counter=1
 
diff --git a/service/github/github_test.go b/service/github/github_test.go
index 82358576..df25272a 100644
--- a/service/github/github_test.go
+++ b/service/github/github_test.go
@@ -69,7 +69,7 @@ func TestGitHubPullRequest_Post(t *testing.T) {
 	pr := 2
 	sha := "cce89afa9ac5519a7f5b1734db2e3aa776b138a7"
 
-	g, err := NewGitHubPullRequest(client, owner, repo, pr, sha, "warning")
+	g, err := NewGitHubPullRequest(client, owner, repo, pr, sha, "warning", "tool-name")
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -106,7 +106,7 @@ func TestGitHubPullRequest_comment(t *testing.T) {
 	owner := "haya14busa"
 	repo := "reviewdog"
 	pr := 2
-	g, err := NewGitHubPullRequest(client, owner, repo, pr, "", "warning")
+	g, err := NewGitHubPullRequest(client, owner, repo, pr, "", "warning", "tool-name")
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -133,7 +133,8 @@ func TestGitHubPullRequest_Post_Flush_review_api(t *testing.T) {
 
 	listCommentsAPICalled := 0
 	postCommentsAPICalled := 0
-	repoCommentsAPICalled := 0
+	repoAPICalled := 0
+	delCommentsAPICalled := 0
 	mux := http.NewServeMux()
 	mux.HandleFunc("/repos/o/r/pulls/14/comments", func(w http.ResponseWriter, r *http.Request) {
 		switch r.Method {
@@ -161,6 +162,20 @@ func TestGitHubPullRequest_Post_Flush_review_api(t *testing.T) {
 						Body:        github.String(commentutil.BodyPrefix + "already commented 2" + "\n<!-- __reviewdog__:ChAxNDgzY2EyNTY0MjU2NmYx -->\n"),
 						SubjectType: github.String("line"),
 					},
+					{
+						ID:          github.Int64(1414),
+						Path:        github.String("reviewdog.go"),
+						Line:        github.Int(15),
+						Body:        github.String(commentutil.BodyPrefix + "already commented [outdated]" + "\n<!-- __reviewdog__:Cg9jY2FlN2NlYTg0M2M0MDISCXRvb2wtbmFtZQ== -->\n"),
+						SubjectType: github.String("line"),
+					},
+					{
+						ID:          github.Int64(1414),
+						Path:        github.String("reviewdog.go"),
+						Line:        github.Int(15),
+						Body:        github.String(commentutil.BodyPrefix + "already commented [different tool]" + "\n<!-- __reviewdog__:CgZ4eHh4eHgSDmRpZmZlcmVudC10b29s -->\n"),
+						SubjectType: github.String("line"),
+					},
 					{
 						Path:        github.String("reviewdog.go"),
 						StartLine:   github.Int(15),
@@ -490,19 +505,22 @@ func TestGitHubPullRequest_Post_Flush_review_api(t *testing.T) {
 		}
 	})
 	mux.HandleFunc("/repos/o/r", func(w http.ResponseWriter, r *http.Request) {
-		repoCommentsAPICalled++
+		repoAPICalled++
 		if err := json.NewEncoder(w).Encode(&github.Repository{
 			HTMLURL: github.String("https://test/repo/path"),
 		}); err != nil {
 			t.Fatal(err)
 		}
 	})
+	mux.HandleFunc("/repos/o/r/pulls/comments/1414", func(w http.ResponseWriter, r *http.Request) {
+		delCommentsAPICalled++
+	})
 	ts := httptest.NewServer(mux)
 	defer ts.Close()
 
 	cli := github.NewClient(nil)
 	cli.BaseURL, _ = url.Parse(ts.URL + "/")
-	g, err := NewGitHubPullRequest(cli, "o", "r", 14, "sha", "warning")
+	g, err := NewGitHubPullRequest(cli, "o", "r", 14, "sha", "warning", "tool-name")
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -1238,6 +1256,12 @@ 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 repoAPICalled != 1 {
+		t.Errorf("GitHub Repository API called %v times, want 1 times", repoAPICalled)
+	}
+	if delCommentsAPICalled != 1 {
+		t.Errorf("GitHub Delete PullRequest comments API called %v times, want 1 times", delCommentsAPICalled)
+	}
 }
 
 func TestGitHubPullRequest_Post_toomany(t *testing.T) {
@@ -1271,7 +1295,7 @@ func TestGitHubPullRequest_Post_toomany(t *testing.T) {
 
 	cli := github.NewClient(nil)
 	cli.BaseURL, _ = url.Parse(ts.URL + "/")
-	g, err := NewGitHubPullRequest(cli, "o", "r", 14, "sha", "warning")
+	g, err := NewGitHubPullRequest(cli, "o", "r", 14, "sha", "warning", "tool-name")
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -1316,7 +1340,7 @@ func TestGitHubPullRequest_workdir(t *testing.T) {
 	moveToRootDir()
 	defer setupEnvs()()
 
-	g, err := NewGitHubPullRequest(nil, "", "", 0, "", "warning")
+	g, err := NewGitHubPullRequest(nil, "", "", 0, "", "warning", "tool-name")
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -1335,7 +1359,7 @@ func TestGitHubPullRequest_workdir(t *testing.T) {
 	if err := os.Chdir(subDir); err != nil {
 		t.Fatal(err)
 	}
-	g, _ = NewGitHubPullRequest(nil, "", "", 0, "", "warning")
+	g, _ = NewGitHubPullRequest(nil, "", "", 0, "", "warning", "tool-name")
 	if g.wd != subDir {
 		t.Fatalf("gitRelWorkdir() = %q, want %q", g.wd, subDir)
 	}
@@ -1373,7 +1397,7 @@ func TestGitHubPullRequest_Post_NoPermission(t *testing.T) {
 
 	cli := github.NewClient(nil)
 	cli.BaseURL, _ = url.Parse(ts.URL + "/")
-	g, err := NewGitHubPullRequest(cli, "o", "r", 14, "sha", "warning")
+	g, err := NewGitHubPullRequest(cli, "o", "r", 14, "sha", "warning", "tool-name")
 	if err != nil {
 		t.Fatal(err)
 	}