Add open issues check in checkdocs (#4258)

Closes #4171. Co-authored-by: Patryk Kwiatek <patryk.kwiatek@ferretdb.io>
FerretDB · Jul 31, 2024 · 9011dbe · 9011dbe
1 parent 22c0906
commit 9011dbe
Show file tree

Hide file tree

Showing 11 changed files with 487 additions and 198 deletions.
diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml
@@ -67,6 +67,9 @@ jobs:
         run: go generate -x
         working-directory: tools
 
+      - name: Run init
+        run: bin/task init
+
       - name: Build tools
         run: bin/task env-pull
 

diff --git a/Taskfile.yml b/Taskfile.yml
@@ -884,6 +884,8 @@ tasks:
       - docker compose run --rm textlint --fix --config build/.textlintrc --ignore-path .prettierignore "**/*.md" ".github/**/*.md"
       - docker compose run --rm prettier --write --parser markdown --no-semi --single-quote --trailing-comma none "**/*.md"
       - docker compose run --rm markdownlint --config "build/.markdownlint.yml" "**/*.md"
+      - bin/envtool{{exeExt}} shell rmdir tmp/githubcache
+      - bin/envtool{{exeExt}} shell mkdir tmp/githubcache
       - bin/checkdocs
 
   pngcrush:

diff --git a/tools/checkcomments/checkcomments.go b/tools/checkcomments/checkcomments.go
@@ -17,19 +17,21 @@ package main
 
 import (
 	"context"
+	"errors"
 	"flag"
 	"log"
 	"regexp"
-	"strconv"
 	"strings"
 
 	"github.com/FerretDB/gh"
 	"golang.org/x/tools/go/analysis"
 	"golang.org/x/tools/go/analysis/singlechecker"
+
+	"github.com/FerretDB/FerretDB/tools/github"
 )
 
-// todoRE represents correct // TODO comment format.
-var todoRE = regexp.MustCompile(`^// TODO (\Qhttps://github.com/FerretDB/\E([-\w]+)/issues/(\d+))$`)
+// todoRE represents correct "// TODO" comment format.
+var todoRE = regexp.MustCompile(`^// TODO (\Qhttps://github.com/FerretDB/\E[-\w]+/issues/\d+)$`)
 
 // analyzer represents the checkcomments analyzer.
 var analyzer = &analysis.Analyzer{
@@ -53,10 +55,10 @@ func main() {
 
 // run analyses TODO comments.
 func run(pass *analysis.Pass) (any, error) {
-	var client *client
+	var client *github.Client
 
 	if !pass.Analyzer.Flags.Lookup("offline").Value.(flag.Getter).Get().(bool) {
-		p, err := cacheFilePath()
+		p, err := github.CacheFilePath()
 		if err != nil {
 			log.Panic(err)
 		}
@@ -71,7 +73,7 @@ func run(pass *analysis.Pass) (any, error) {
 			clientDebugF = log.New(log.Writer(), "client-debug: ", log.Flags()).Printf
 		}
 
-		if client, err = newClient(p, log.Printf, cacheDebugF, clientDebugF); err != nil {
+		if client, err = github.NewClient(p, log.Printf, cacheDebugF, clientDebugF); err != nil {
 			log.Panic(err)
 		}
 	}
@@ -92,38 +94,31 @@ func run(pass *analysis.Pass) (any, error) {
 
 				match := todoRE.FindStringSubmatch(line)
 
-				if len(match) != 4 {
+				if len(match) != 2 {
 					pass.Reportf(c.Pos(), "invalid TODO: incorrect format")
 					continue
 				}
 
 				url := match[1]
-				repo := match[2]
-				num, err := strconv.Atoi(match[3])
-				if err != nil {
-					log.Panic(err)
-				}
-
-				if num <= 0 {
-					pass.Reportf(c.Pos(), "invalid TODO: incorrect issue number")
-					continue
-				}
 
-				if client == nil {
-					continue
-				}
+				status, err := client.IssueStatus(context.TODO(), url)
 
-				status, err := client.IssueStatus(context.TODO(), url, repo, num)
-				if err != nil {
+				switch {
+				case err == nil:
+					// nothing
+				case errors.Is(err, github.ErrIncorrectURL),
+					errors.Is(err, github.ErrIncorrectIssueNumber):
+					log.Print(err.Error())
+				default:
 					log.Panic(err)
 				}
 
 				switch status {
-				case issueOpen:
+				case github.IssueOpen:
 					// nothing
-				case issueClosed:
+				case github.IssueClosed:
 					pass.Reportf(c.Pos(), "invalid TODO: linked issue %s is closed", url)
-				case issueNotFound:
+				case github.IssueNotFound:
 					pass.Reportf(c.Pos(), "invalid TODO: linked issue %s is not found", url)
 				default:
 					log.Panicf("unknown issue status: %s", status)

diff --git a/tools/checkcomments/checkcomments_test.go b/tools/checkcomments/checkcomments_test.go
@@ -15,102 +15,24 @@
 package main
 
 import (
-	"context"
 	"os"
 	"path/filepath"
 	"testing"
 
-	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"golang.org/x/tools/go/analysis/analysistest"
+
+	"github.com/FerretDB/FerretDB/tools/github"
 )
 
 func TestCheckCommentIssue(t *testing.T) {
 	t.Parallel()
 
-	path, err := cacheFilePath()
+	path, err := github.CacheFilePath()
 	require.NoError(t, err)
 
 	err = os.MkdirAll(filepath.Dir(path), 0o777)
 	require.NoError(t, err)
 
 	analysistest.Run(t, analysistest.TestData(), analyzer)
 }
-
-func TestCacheFilePath(t *testing.T) {
-	t.Parallel()
-
-	wd, err := os.Getwd()
-	require.NoError(t, err)
-	expected := filepath.Join(wd, "..", "..", "tmp", "githubcache", "cache.json")
-
-	actual, err := cacheFilePath()
-	require.NoError(t, err)
-	assert.Equal(t, expected, actual)
-}
-
-func TestClient(t *testing.T) {
-	t.Parallel()
-
-	cacheFilePath := filepath.Join(t.TempDir(), "cache.json")
-	ctx := context.Background()
-
-	t.Run("CheckIssueStatus", func(t *testing.T) {
-		t.Parallel()
-
-		c, err := newClient(cacheFilePath, t.Logf, t.Logf, t.Logf)
-		require.NoError(t, err)
-
-		actual, err := c.checkIssueStatus(ctx, "FerretDB", 10)
-		require.NoError(t, err)
-		assert.Equal(t, issueOpen, actual)
-
-		actual, err = c.checkIssueStatus(ctx, "FerretDB", 1)
-		require.NoError(t, err)
-		assert.Equal(t, issueClosed, actual)
-
-		actual, err = c.checkIssueStatus(ctx, "FerretDB", 999999)
-		require.NoError(t, err)
-		assert.Equal(t, issueNotFound, actual)
-	})
-
-	t.Run("IssueStatus", func(t *testing.T) {
-		t.Parallel()
-
-		c, err := newClient(cacheFilePath, t.Logf, t.Logf, t.Logf)
-		require.NoError(t, err)
-
-		actual, err := c.IssueStatus(ctx, "https://github.com/FerretDB/FerretDB/issues/10", "FerretDB", 10)
-		require.NoError(t, err)
-		assert.Equal(t, issueOpen, actual)
-
-		actual, err = c.IssueStatus(ctx, "https://github.com/FerretDB/FerretDB/issues/1", "FerretDB", 1)
-		require.NoError(t, err)
-		assert.Equal(t, issueClosed, actual)
-
-		actual, err = c.IssueStatus(ctx, "https://github.com/FerretDB/FerretDB/issues/999999", "FerretDB", 999999)
-		require.NoError(t, err)
-		assert.Equal(t, issueNotFound, actual)
-
-		// The following tests should use cache and not the client,
-		// but it may be empty if tests above failed for some reason.
-
-		if t.Failed() {
-			return
-		}
-
-		c.c = nil
-
-		actual, err = c.IssueStatus(ctx, "https://github.com/FerretDB/FerretDB/issues/10", "FerretDB", 10)
-		require.NoError(t, err)
-		assert.Equal(t, issueOpen, actual)
-
-		actual, err = c.IssueStatus(ctx, "https://github.com/FerretDB/FerretDB/issues/1", "FerretDB", 1)
-		require.NoError(t, err)
-		assert.Equal(t, issueClosed, actual)
-
-		actual, err = c.IssueStatus(ctx, "https://github.com/FerretDB/FerretDB/issues/999999", "FerretDB", 999999)
-		require.NoError(t, err)
-		assert.Equal(t, issueNotFound, actual)
-	})
-}