Skip to content

Commit

Permalink
Add some small improvements to the linter that checks open issues (#3756
Browse files Browse the repository at this point in the history 
)

Closes #2733.

Co-authored-by: Alexey Palazhchenko <alexey.palazhchenko@ferretdb.io>
  • Loading branch information
@AlekSi
Elena Grahovac and AlekSi authored Dec 25, 2023
1 parent 235a208 commit e36e848
Show file tree
Hide file tree
Showing 9 changed files with 377 additions and 140 deletions.
2 changes: 2 additions & 0 deletions internal/backends/mysql/metadata/registry_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,8 @@ func TestCheckAuth(t *testing.T) {
require.NoError(t, err)
t.Cleanup(r.Close)

t.Skip("https://github.com/FerretDB/FerretDB/issues/3413")

_, err = r.getPool(ctx)

expected := `Error 1045 \(28000\): Access denied for user 'wrong-user*'@'[\d\.]+' \(using password: YES\)`
Expand Down
3 changes: 1 addition & 2 deletions internal/util/ctxutil/ctxutil_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,7 @@ func simulateCompetingClients(clients int) int64 {
}
}

wg := sync.WaitGroup{}

var wg sync.WaitGroup
for i := 0; i < clients; i++ {
wg.Add(1)

Expand Down
172 changes: 45 additions & 127 deletions tools/checkcomments/checkcomments.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,90 +17,57 @@ package main

import (
"context"
"encoding/json"
"errors"
"fmt"
"flag"
"log"
"os"
"path/filepath"
"regexp"
"strconv"
"strings"

"github.com/FerretDB/gh"
"github.com/google/go-github/v56/github"
"github.com/rogpeppe/go-internal/lockedfile"
"golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/analysis/singlechecker"
)

// struct used to hold open status of issues, true if open otherwise false.
type issueCache struct {
Issues map[string]bool `json:"Issues"`
}

// todoRE represents correct // TODO comment format.
var todoRE = regexp.MustCompile(`^// TODO \Qhttps://github.com/FerretDB/FerretDB/issues/\E(\d+)$`)
var todoRE = regexp.MustCompile(`^// TODO (\Qhttps://github.com/FerretDB/FerretDB/issues/\E(\d+))$`)

// analyzer represents the checkcomments analyzer.
var analyzer = &analysis.Analyzer{
Name: "checkcomments",
Doc: "check TODO comments",
Run: run,
Name: "checkcomments",
Doc: "check TODO comments",
Run: run,
Flags: *flag.NewFlagSet("", flag.ExitOnError),
}

// init initializes the analyzer flags.
func init() {
analyzer.Flags.Bool("offline", false, "do not check issues open/closed status")
analyzer.Flags.Bool("client-debug", false, "log GitHub API requests/responses and cache hit/misses")
}

// main runs the analyzer.
func main() {
singlechecker.Main(analyzer)
}

// run analyses TODO comments.
func run(pass *analysis.Pass) (any, error) {
var iCache issueCache

current_path, err := os.Getwd()
if err != nil {
log.Fatal(err)
}

cache_path := getCacheFilePath(current_path)

cf, err := lockedfile.OpenFile(cache_path, os.O_RDWR|os.O_CREATE, 0o666)
if err != nil {
log.Fatal(err)
}

defer func() {
cerr := cf.Close()
if cerr != nil {
log.Fatal(cerr)
}
}()
var client *client

stat, err := cf.Stat()
if err != nil {
log.Fatal(err)
}

if stat.Size() > 0 {
buffer := make([]byte, stat.Size())

_, err = cf.Read(buffer)
if !pass.Analyzer.Flags.Lookup("offline").Value.(flag.Getter).Get().(bool) {
p, err := cacheFilePath()
if err != nil {
log.Fatal(err)
log.Panic(err)
}

err = json.Unmarshal(buffer, &iCache)
if err != nil {
log.Fatal(err)
debugf := gh.NoopPrintf
if pass.Analyzer.Flags.Lookup("client-debug").Value.(flag.Getter).Get().(bool) {
debugf = log.New(log.Writer(), "client-debug: ", log.Flags()).Printf
}
} else {
iCache.Issues = make(map[string]bool)
}

token := os.Getenv("GITHUB_TOKEN")

client, err := gh.NewRESTClient(token, nil)
if err != nil {
log.Fatal(err)
if client, err = newClient(p, log.Printf, debugf); err != nil {
log.Panic(err)
}
}

for _, f := range pass.Files {
Expand All @@ -119,89 +86,40 @@ func run(pass *analysis.Pass) (any, error) {

match := todoRE.FindStringSubmatch(line)

if match == nil {
if len(match) != 3 {
pass.Reportf(c.Pos(), "invalid TODO: incorrect format")
continue
}

iNum := match[1]

_, ok := iCache.Issues[iNum]
if client == nil {
continue
}

if !ok {
n, inErr := strconv.Atoi(iNum)
if inErr != nil {
log.Fatal(inErr)
}
url := match[1]

isOpen, inErr := isIssueOpen(client, n)
if err != nil {
log.Fatal(inErr)
}
num, err := strconv.Atoi(match[2])
if err != nil {
log.Panic(err)
}

iCache.Issues[iNum] = isOpen
status, err := client.IssueStatus(context.TODO(), num)
if err != nil {
log.Panic(err)
}

if !iCache.Issues[iNum] {
message := fmt.Sprintf("invalid TODO: linked issue %s is closed", iNum)
pass.Reportf(c.Pos(), message)
switch status {
case issueOpen:
// nothing
case issueClosed:
pass.Reportf(c.Pos(), "invalid TODO: linked issue %s is closed", url)
case issueNotFound:
pass.Reportf(c.Pos(), "invalid TODO: linked issue %s is not found", url)
default:
log.Panicf("unknown issue status: %s", status)
}
}
}
}

if len(iCache.Issues) > 0 {
jsonb, err := json.Marshal(iCache)
if err != nil {
log.Fatal(err)
}

_, err = cf.WriteAt(jsonb, 0)
if err != nil {
log.Fatal(err)
}
}

return nil, nil
}

func isIssueOpen(client *github.Client, n int) (bool, error) {
issue, _, err := client.Issues.Get(context.TODO(), "FerretDB", "FerretDB", n)
// if error is RateLimitError and token is not set propmt user to provide GITHUB_TOKEN
// else consider issue open
if err != nil {
if errors.As(err, new(*github.RateLimitError)) && os.Getenv("GITHUB_TOKEN") == "" {
log.Println(
"Rate limit reached. Please set a GITHUB_TOKEN as described at",
"https://github.com/FerretDB/FerretDB/blob/main/CONTRIBUTING.md#setting-a-github_token",
)

return false, err
}

return true, nil
}

isOpen := issue.GetState() == "open"

return isOpen, nil
}

/*
* Due to analysis tools changing cwd for each go Package
* we need to find the root of the project in order to get the common cache file.
* this is done by recursively traversing the Path up until README.md is found.
*/
func getCacheFilePath(p string) string {
path := filepath.Dir(p)

readmePath := filepath.Join(path, "README.md")

_, err := os.Stat(readmePath)

if os.IsNotExist(err) {
return getCacheFilePath(path)
}

return filepath.Join(path, "tmp", "checkcomments", "cache.json")
}
96 changes: 96 additions & 0 deletions tools/checkcomments/checkcomments_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,107 @@
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"
)

func TestCheckCommentIssue(t *testing.T) {
t.Parallel()

path, err := 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", "checkcomments", "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)
require.NoError(t, err)

actual, err := c.checkIssueStatus(ctx, 10)
require.NoError(t, err)
assert.Equal(t, issueOpen, actual)

actual, err = c.checkIssueStatus(ctx, 1)
require.NoError(t, err)
assert.Equal(t, issueClosed, actual)

actual, err = c.checkIssueStatus(ctx, 999999)
require.NoError(t, err)
assert.Equal(t, issueNotFound, actual)

actual, err = c.checkIssueStatus(ctx, -1)
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)
require.NoError(t, err)

actual, err := c.IssueStatus(ctx, 10)
require.NoError(t, err)
assert.Equal(t, issueOpen, actual)

actual, err = c.IssueStatus(ctx, 1)
require.NoError(t, err)
assert.Equal(t, issueClosed, actual)

actual, err = c.IssueStatus(ctx, 999999)
require.NoError(t, err)
assert.Equal(t, issueNotFound, actual)

actual, err = c.IssueStatus(ctx, -1)
require.NoError(t, err)
assert.Equal(t, issueNotFound, actual)

c.c = nil

actual, err = c.IssueStatus(ctx, 10)
require.NoError(t, err)
assert.Equal(t, issueOpen, actual)

actual, err = c.IssueStatus(ctx, 1)
require.NoError(t, err)
assert.Equal(t, issueClosed, actual)

actual, err = c.IssueStatus(ctx, 999999)
require.NoError(t, err)
assert.Equal(t, issueNotFound, actual)

actual, err = c.IssueStatus(ctx, -1)
require.NoError(t, err)
assert.Equal(t, issueNotFound, actual)
})
}
Loading

0 comments on commit e36e848

Please sign in to comment.