diff options
Diffstat (limited to 'tools/github/reviver')
| -rw-r--r-- | tools/github/reviver/BUILD | 27 | ||||
| -rw-r--r-- | tools/github/reviver/github.go | 162 | ||||
| -rw-r--r-- | tools/github/reviver/github_test.go | 55 | ||||
| -rw-r--r-- | tools/github/reviver/reviver.go | 192 | ||||
| -rw-r--r-- | tools/github/reviver/reviver_test.go | 88 |
5 files changed, 524 insertions, 0 deletions
diff --git a/tools/github/reviver/BUILD b/tools/github/reviver/BUILD new file mode 100644 index 000000000..7d78480a7 --- /dev/null +++ b/tools/github/reviver/BUILD @@ -0,0 +1,27 @@ +load("//tools:defs.bzl", "go_library", "go_test") + +package(licenses = ["notice"]) + +go_library( + name = "reviver", + srcs = [ + "github.go", + "reviver.go", + ], + nogo = False, + visibility = [ + "//tools/github:__subpackages__", + ], + deps = ["@com_github_google_go_github_v28//github:go_default_library"], +) + +go_test( + name = "reviver_test", + size = "small", + srcs = [ + "github_test.go", + "reviver_test.go", + ], + library = ":reviver", + nogo = False, +) diff --git a/tools/github/reviver/github.go b/tools/github/reviver/github.go new file mode 100644 index 000000000..a95df0fb6 --- /dev/null +++ b/tools/github/reviver/github.go @@ -0,0 +1,162 @@ +// Copyright 2019 The gVisor Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package reviver + +import ( + "context" + "fmt" + "strconv" + "strings" + "time" + + "github.com/google/go-github/github" +) + +// GitHubBugger implements Bugger interface for github issues. +type GitHubBugger struct { + owner string + repo string + dryRun bool + + client *github.Client + issues map[int]*github.Issue +} + +// NewGitHubBugger creates a new GitHubBugger. +func NewGitHubBugger(client *github.Client, owner, repo string, dryRun bool) (*GitHubBugger, error) { + b := &GitHubBugger{ + owner: owner, + repo: repo, + dryRun: dryRun, + issues: map[int]*github.Issue{}, + client: client, + } + if err := b.load(); err != nil { + return nil, err + } + return b, nil +} + +func (b *GitHubBugger) load() error { + err := processAllPages(func(listOpts github.ListOptions) (*github.Response, error) { + opts := &github.IssueListByRepoOptions{State: "open", ListOptions: listOpts} + tmps, resp, err := b.client.Issues.ListByRepo(context.Background(), b.owner, b.repo, opts) + if err != nil { + return resp, err + } + for _, issue := range tmps { + b.issues[issue.GetNumber()] = issue + } + return resp, nil + }) + if err != nil { + return err + } + + fmt.Printf("Loaded %d issues from github.com/%s/%s\n", len(b.issues), b.owner, b.repo) + return nil +} + +// Activate implements Bugger.Activate. +func (b *GitHubBugger) Activate(todo *Todo) (bool, error) { + id, err := parseIssueNo(todo.Issue) + if err != nil { + return true, err + } + if id <= 0 { + return false, nil + } + + // Check against active issues cache. + if _, ok := b.issues[id]; ok { + fmt.Printf("%q is active: OK\n", todo.Issue) + return true, nil + } + + fmt.Printf("%q is not active: reopening issue %d\n", todo.Issue, id) + + // Format comment with TODO locations and search link. + comment := strings.Builder{} + fmt.Fprintln(&comment, "There are TODOs still referencing this issue:") + for _, l := range todo.Locations { + fmt.Fprintf(&comment, + "1. [%s:%d](https://github.com/%s/%s/blob/HEAD/%s#%d): %s\n", + l.File, l.Line, b.owner, b.repo, l.File, l.Line, l.Comment) + } + fmt.Fprintf(&comment, + "\n\nSearch [TODO](https://github.com/%s/%s/search?q=%%22%s%%22)", b.owner, b.repo, todo.Issue) + + if b.dryRun { + fmt.Printf("[dry-run: skipping change to issue %d]\n%s\n=======================\n", id, comment.String()) + return true, nil + } + + ctx := context.Background() + req := &github.IssueRequest{State: github.String("open")} + _, _, err = b.client.Issues.Edit(ctx, b.owner, b.repo, id, req) + if err != nil { + return true, fmt.Errorf("failed to reactivate issue %d: %v", id, err) + } + + cmt := &github.IssueComment{ + Body: github.String(comment.String()), + Reactions: &github.Reactions{Confused: github.Int(1)}, + } + if _, _, err := b.client.Issues.CreateComment(ctx, b.owner, b.repo, id, cmt); err != nil { + return true, fmt.Errorf("failed to add comment to issue %d: %v", id, err) + } + + return true, nil +} + +// parseIssueNo parses the issue number out of the issue url. +func parseIssueNo(url string) (int, error) { + const prefix = "gvisor.dev/issue/" + + // First check if I can handle the TODO. + idStr := strings.TrimPrefix(url, prefix) + if len(url) == len(idStr) { + return 0, nil + } + + id, err := strconv.ParseInt(strings.TrimRight(idStr, "/"), 10, 64) + if err != nil { + return 0, err + } + return int(id), nil +} + +func processAllPages(fn func(github.ListOptions) (*github.Response, error)) error { + opts := github.ListOptions{PerPage: 1000} + for { + resp, err := fn(opts) + if err != nil { + if rateErr, ok := err.(*github.RateLimitError); ok { + duration := rateErr.Rate.Reset.Sub(time.Now()) + if duration > 5*time.Minute { + return fmt.Errorf("Rate limited for too long: %v", duration) + } + fmt.Printf("Rate limited, sleeping for: %v\n", duration) + time.Sleep(duration) + continue + } + return err + } + if resp.NextPage == 0 { + return nil + } + opts.Page = resp.NextPage + } +} diff --git a/tools/github/reviver/github_test.go b/tools/github/reviver/github_test.go new file mode 100644 index 000000000..5df7e3624 --- /dev/null +++ b/tools/github/reviver/github_test.go @@ -0,0 +1,55 @@ +// Copyright 2020 The gVisor Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package reviver + +import ( + "testing" +) + +func TestParseIssueNo(t *testing.T) { + testCases := []struct { + issue string + expectErr bool + expected int + }{ + { + issue: "gvisor.dev/issue/123", + expected: 123, + }, + { + issue: "gvisor.dev/issue/123/", + expected: 123, + }, + { + issue: "not a url", + expected: 0, + }, + { + issue: "gvisor.dev/issue//", + expectErr: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.issue, func(t *testing.T) { + id, err := parseIssueNo(tc.issue) + if err != nil && !tc.expectErr { + t.Errorf("got error: %v", err) + } else if tc.expected != id { + t.Errorf("got: %v, want: %v", id, tc.expected) + } + }) + } +} diff --git a/tools/github/reviver/reviver.go b/tools/github/reviver/reviver.go new file mode 100644 index 000000000..2af7f0d59 --- /dev/null +++ b/tools/github/reviver/reviver.go @@ -0,0 +1,192 @@ +// Copyright 2019 The gVisor Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Package reviver scans the code looking for TODOs and pass them to registered +// Buggers to ensure TODOs point to active issues. +package reviver + +import ( + "bufio" + "fmt" + "io/ioutil" + "os" + "path/filepath" + "regexp" + "sync" +) + +// regexTodo matches a TODO or FIXME comment. +var regexTodo = regexp.MustCompile(`(\/\/|#)\s*(TODO|FIXME)\(([a-zA-Z0-9.\/]+)\):\s*(.+)`) + +// Bugger interface is called for every TODO found in the code. If it can handle +// the TODO, it must return true. If it returns false, the next Bugger is +// called. If no Bugger handles the TODO, it's dropped on the floor. +type Bugger interface { + Activate(todo *Todo) (bool, error) +} + +// Location saves the location where the TODO was found. +type Location struct { + Comment string + File string + Line uint +} + +// Todo represents a unique TODO. There can be several TODOs pointing to the +// same issue in the code. They are all grouped together. +type Todo struct { + Issue string + Locations []Location +} + +// Reviver scans the given paths for TODOs and calls Buggers to handle them. +type Reviver struct { + paths []string + buggers []Bugger + + mu sync.Mutex + todos map[string]*Todo + errs []error +} + +// New create a new Reviver. +func New(paths []string, buggers []Bugger) *Reviver { + return &Reviver{ + paths: paths, + buggers: buggers, + todos: map[string]*Todo{}, + } +} + +// Run runs. It returns all errors found during processing, it doesn't stop +// on errors. +func (r *Reviver) Run() []error { + // Process each directory in parallel. + wg := sync.WaitGroup{} + for _, path := range r.paths { + wg.Add(1) + go func(path string) { + defer wg.Done() + r.processPath(path, &wg) + }(path) + } + + wg.Wait() + + r.mu.Lock() + defer r.mu.Unlock() + + fmt.Printf("Processing %d TODOs (%d errors)...\n", len(r.todos), len(r.errs)) + dropped := 0 + for _, todo := range r.todos { + ok, err := r.processTodo(todo) + if err != nil { + r.errs = append(r.errs, err) + } + if !ok { + dropped++ + } + } + fmt.Printf("Processed %d TODOs, %d were skipped (%d errors)\n", len(r.todos)-dropped, dropped, len(r.errs)) + + return r.errs +} + +func (r *Reviver) processPath(path string, wg *sync.WaitGroup) { + fmt.Printf("Processing dir %q\n", path) + fis, err := ioutil.ReadDir(path) + if err != nil { + r.addErr(fmt.Errorf("error processing dir %q: %v", path, err)) + return + } + + for _, fi := range fis { + childPath := filepath.Join(path, fi.Name()) + switch { + case fi.Mode().IsDir(): + wg.Add(1) + go func() { + defer wg.Done() + r.processPath(childPath, wg) + }() + + case fi.Mode().IsRegular(): + file, err := os.Open(childPath) + if err != nil { + r.addErr(err) + continue + } + + scanner := bufio.NewScanner(file) + lineno := uint(0) + for scanner.Scan() { + lineno++ + line := scanner.Text() + if todo := r.processLine(line, childPath, lineno); todo != nil { + r.addTodo(todo) + } + } + } + } +} + +func (r *Reviver) processLine(line, path string, lineno uint) *Todo { + matches := regexTodo.FindStringSubmatch(line) + if matches == nil { + return nil + } + if len(matches) != 5 { + panic(fmt.Sprintf("regex returned wrong matches for %q: %v", line, matches)) + } + return &Todo{ + Issue: matches[3], + Locations: []Location{ + { + File: path, + Line: lineno, + Comment: matches[4], + }, + }, + } +} + +func (r *Reviver) addTodo(newTodo *Todo) { + r.mu.Lock() + defer r.mu.Unlock() + + if todo := r.todos[newTodo.Issue]; todo == nil { + r.todos[newTodo.Issue] = newTodo + } else { + todo.Locations = append(todo.Locations, newTodo.Locations...) + } +} + +func (r *Reviver) addErr(err error) { + r.mu.Lock() + defer r.mu.Unlock() + r.errs = append(r.errs, err) +} + +func (r *Reviver) processTodo(todo *Todo) (bool, error) { + for _, bugger := range r.buggers { + ok, err := bugger.Activate(todo) + if err != nil { + return false, err + } + if ok { + return true, nil + } + } + return false, nil +} diff --git a/tools/github/reviver/reviver_test.go b/tools/github/reviver/reviver_test.go new file mode 100644 index 000000000..a9fb1f9f1 --- /dev/null +++ b/tools/github/reviver/reviver_test.go @@ -0,0 +1,88 @@ +// Copyright 2019 The gVisor Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package reviver + +import ( + "testing" +) + +func TestProcessLine(t *testing.T) { + for _, tc := range []struct { + line string + want *Todo + }{ + { + line: "// TODO(foobar.com/issue/123): comment, bla. blabla.", + want: &Todo{ + Issue: "foobar.com/issue/123", + Locations: []Location{ + {Comment: "comment, bla. blabla."}, + }, + }, + }, + { + line: "// FIXME(b/123): internal bug", + want: &Todo{ + Issue: "b/123", + Locations: []Location{ + {Comment: "internal bug"}, + }, + }, + }, + { + line: "TODO(issue): not todo", + }, + { + line: "FIXME(issue): not todo", + }, + { + line: "// TODO (issue): not todo", + }, + { + line: "// TODO(issue) not todo", + }, + { + line: "// todo(issue): not todo", + }, + { + line: "// TODO(issue):", + }, + } { + t.Logf("Testing: %s", tc.line) + r := Reviver{} + got := r.processLine(tc.line, "test", 0) + if got == nil { + if tc.want != nil { + t.Errorf("failed to process line, want: %+v", tc.want) + } + } else { + if tc.want == nil { + t.Errorf("expected error, got: %+v", got) + continue + } + if got.Issue != tc.want.Issue { + t.Errorf("wrong issue, got: %v, want: %v", got.Issue, tc.want.Issue) + } + if len(got.Locations) != len(tc.want.Locations) { + t.Errorf("wrong number of locations, got: %v, want: %v, locations: %+v", len(got.Locations), len(tc.want.Locations), got.Locations) + } + for i, wantLoc := range tc.want.Locations { + if got.Locations[i].Comment != wantLoc.Comment { + t.Errorf("wrong comment, got: %v, want: %v", got.Locations[i].Comment, wantLoc.Comment) + } + } + } + } +} |