Add nogo check annotations to GitHub.

When nogo checks are violated, they will automatically posted
as annotations on the specific GitHub commit. This allows us
to ensure analysis & style rules and have them called out.

PiperOrigin-RevId: 334447285

.github/workflows/build.yml

@@ -21,3 +21,8 @@ jobs:
         restore-keys: |
           ${{ runner.os }}-bazel-
     - run: make
+    - run: make build OPTIONS="--build_tag_filters nogo" TARGETS="//..."
+    - run: make run TARGETS="//tools/github" ARGS="-path=bazel-bin/ nogo"
+      env:
+        GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+        GITHUB_REPOSITORY: ${{ github.repository }}

.github/workflows/issue_reviver.yml

@@ -9,7 +9,7 @@ jobs:
     steps:
     - uses: actions/checkout@v2
       if: github.repository == 'google/gvisor'
-    - run: make run TARGETS="//tools/issue_reviver"
+    - run: make run TARGETS="//tools/github" ARGS="revive"
       if: github.repository == 'google/gvisor'
       env:
         GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

Makefile

@@ -379,3 +379,8 @@ configure: ## Configures a single runtime. Requires sudo. Typically called from
 test-runtime: ## A convenient wrapper around test that provides the runtime argument. Target must still be provided.
 	@$(call submake,test OPTIONS="$(OPTIONS) --test_arg=--runtime=$(RUNTIME)")
 .PHONY: test-runtime
+
+nogo: ## Surfaces all nogo findings.
+	@$(call submake,build OPTIONS="--build_tag_filters nogo" TARGETS="//...")
+	@$(call submake,run TARGETS="//tools/github" ARGS="-path=$(BUILD_ROOT) -dry-run nogo")
+.PHONY: nogo

tools/bazel.mk

@@ -19,6 +19,7 @@ SHELL=/bin/bash -o pipefail
 BRANCH_NAME := $(shell (git branch --show-current 2>/dev/null || \
 			git rev-parse --abbrev-ref HEAD 2>/dev/null) | \
 			xargs -n 1 basename 2>/dev/null)
+BUILD_ROOT := $(CURDIR)/bazel-bin/
 
 # Bazel container configuration (see below).
 USER ?= gvisor

tools/github/BUILD

@@ -0,0 +1,15 @@
+load("//tools:defs.bzl", "go_binary")
+
+package(licenses = ["notice"])
+
+go_binary(
+    name = "github",
+    srcs = ["main.go"],
+    nogo = False,
+    deps = [
+        "//tools/github/nogo",
+        "//tools/github/reviver",
+        "@com_github_google_go_github_v28//github:go_default_library",
+        "@org_golang_x_oauth2//:go_default_library",
+    ],
+)

tools/github/main.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.
+
+// Binary github is the entry point for GitHub utilities.
+package main
+
+import (
+	"context"
+	"flag"
+	"fmt"
+	"io/ioutil"
+	"os"
+	"os/exec"
+	"strings"
+
+	"github.com/google/go-github/github"
+	"golang.org/x/oauth2"
+	"gvisor.dev/gvisor/tools/github/nogo"
+	"gvisor.dev/gvisor/tools/github/reviver"
+)
+
+var (
+	owner     string
+	repo      string
+	tokenFile string
+	path      string
+	commit    string
+	dryRun    bool
+)
+
+// Keep the options simple for now. Supports only a single path and repo.
+func init() {
+	flag.StringVar(&owner, "owner", "", "GitHub project org/owner (required, except nogo dry-run)")
+	flag.StringVar(&repo, "repo", "", "GitHub repo (required, except nogo dry-run)")
+	flag.StringVar(&tokenFile, "oauth-token-file", "", "file containing the GitHub token (or GITHUB_TOKEN is set)")
+	flag.StringVar(&path, "path", ".", "path to scan (required for revive and nogo)")
+	flag.StringVar(&commit, "commit", "", "commit to associated (required for nogo, except dry-run)")
+	flag.BoolVar(&dryRun, "dry-run", false, "just print changes to be made")
+}
+
+func main() {
+	// Set defaults from the environment.
+	repository := os.Getenv("GITHUB_REPOSITORY")
+	if parts := strings.SplitN(repository, "/", 2); len(parts) == 2 {
+		owner = parts[0]
+		repo = parts[1]
+	}
+
+	// Parse flags.
+	flag.Usage = func() {
+		fmt.Fprintf(flag.CommandLine.Output(), "usage: %s [options] <command>\n", os.Args[0])
+		fmt.Fprintf(flag.CommandLine.Output(), "commands: revive, nogo\n")
+		flag.PrintDefaults()
+	}
+	flag.Parse()
+	args := flag.Args()
+	if len(args) != 1 {
+		fmt.Fprintf(flag.CommandLine.Output(), "extra arguments: %s\n", strings.Join(args[1:], ", "))
+		flag.Usage()
+		os.Exit(1)
+	}
+
+	// Check for mandatory parameters.
+	command := args[0]
+	if len(owner) == 0 && (command != "nogo" || !dryRun) {
+		fmt.Fprintln(flag.CommandLine.Output(), "missing --owner option.")
+		flag.Usage()
+		os.Exit(1)
+	}
+	if len(repo) == 0 && (command != "nogo" || !dryRun) {
+		fmt.Fprintln(flag.CommandLine.Output(), "missing --repo option.")
+		flag.Usage()
+		os.Exit(1)
+	}
+	if len(path) == 0 {
+		fmt.Fprintln(flag.CommandLine.Output(), "missing --path option.")
+		flag.Usage()
+		os.Exit(1)
+	}
+
+	// The access token may be passed as a file so it doesn't show up in
+	// command line arguments. It also may be provided through the
+	// environment to faciliate use through GitHub's CI system.
+	token := os.Getenv("GITHUB_TOKEN")
+	if len(tokenFile) != 0 {
+		bytes, err := ioutil.ReadFile(tokenFile)
+		if err != nil {
+			fmt.Println(err.Error())
+			os.Exit(1)
+		}
+		token = string(bytes)
+	}
+	var client *github.Client
+	if len(token) == 0 {
+		// Client is unauthenticated.
+		client = github.NewClient(nil)
+	} else {
+		// Using the above token.
+		ts := oauth2.StaticTokenSource(
+			&oauth2.Token{AccessToken: token},
+		)
+		tc := oauth2.NewClient(context.Background(), ts)
+		client = github.NewClient(tc)
+	}
+
+	switch command {
+	case "revive":
+		// Load existing GitHub bugs.
+		bugger, err := reviver.NewGitHubBugger(client, owner, repo, dryRun)
+		if err != nil {
+			fmt.Fprintf(os.Stderr, "Error getting github issues: %v\n", err)
+			os.Exit(1)
+		}
+		// Scan the provided path.
+		rev := reviver.New([]string{path}, []reviver.Bugger{bugger})
+		if errs := rev.Run(); len(errs) > 0 {
+			fmt.Fprintf(os.Stderr, "Encountered %d errors:\n", len(errs))
+			for _, err := range errs {
+				fmt.Fprintf(os.Stderr, "\t%v\n", err)
+			}
+			os.Exit(1)
+		}
+	case "nogo":
+		// Did we get a commit? Try to extract one.
+		if len(commit) == 0 && !dryRun {
+			cmd := exec.Command("git", "rev-parse", "HEAD")
+			revBytes, err := cmd.Output()
+			if err != nil {
+				fmt.Fprintf(flag.CommandLine.Output(), "missing --commit option, unable to infer: %v\n", err)
+				flag.Usage()
+				os.Exit(1)
+			}
+			commit = strings.TrimSpace(string(revBytes))
+		}
+		// Scan all findings.
+		poster := nogo.NewFindingsPoster(client, owner, repo, commit, dryRun)
+		if err := poster.Walk(path); err != nil {
+			fmt.Fprintln(os.Stderr, "Error finding nogo findings:", err)
+			os.Exit(1)
+		}
+		// Post to GitHub.
+		if err := poster.Post(); err != nil {
+			fmt.Fprintln(os.Stderr, "Error posting nogo findings:", err)
+		}
+	default:
+		// Not a known command.
+		fmt.Fprintf(flag.CommandLine.Output(), "unknown command: %s\n", command)
+		flag.Usage()
+		os.Exit(1)
+	}
+}

tools/github/nogo/BUILD

@@ -0,0 +1,16 @@
+load("//tools:defs.bzl", "go_library")
+
+package(licenses = ["notice"])
+
+go_library(
+    name = "nogo",
+    srcs = ["nogo.go"],
+    nogo = False,
+    visibility = [
+        "//tools/github:__subpackages__",
+    ],
+    deps = [
+        "//tools/nogo/util",
+        "@com_github_google_go_github_v28//github:go_default_library",
+    ],
+)

tools/github/nogo/nogo.go

@@ -0,0 +1,126 @@
+// 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 nogo provides nogo-related utilities.
+package nogo
+
+import (
+	"context"
+	"fmt"
+	"os"
+	"path/filepath"
+	"strings"
+	"time"
+
+	"github.com/google/go-github/github"
+	"gvisor.dev/gvisor/tools/nogo/util"
+)
+
+// FindingsPoster is a simple wrapper around the GitHub api.
+type FindingsPoster struct {
+	owner     string
+	repo      string
+	commit    string
+	dryRun    bool
+	startTime time.Time
+
+	findings map[util.Finding]struct{}
+	client   *github.Client
+}
+
+// NewFindingsPoster returns a object that can post findings.
+func NewFindingsPoster(client *github.Client, owner, repo, commit string, dryRun bool) *FindingsPoster {
+	return &FindingsPoster{
+		owner:     owner,
+		repo:      repo,
+		commit:    commit,
+		dryRun:    dryRun,
+		startTime: time.Now(),
+		findings:  make(map[util.Finding]struct{}),
+		client:    client,
+	}
+}
+
+// Walk walks the given path tree for findings files.
+func (p *FindingsPoster) Walk(path string) error {
+	return filepath.Walk(path, func(filename string, info os.FileInfo, err error) error {
+		if err != nil {
+			return err
+		}
+		// Skip any directories or files not ending in .findings.
+		if !strings.HasSuffix(filename, ".findings") || info.IsDir() {
+			return nil
+		}
+		findings, err := util.ExtractFindingsFromFile(filename)
+		if err != nil {
+			return err
+		}
+		// Add all findings to the list. We use a map to ensure
+		// that each finding is unique.
+		for _, finding := range findings {
+			p.findings[finding] = struct{}{}
+		}
+		return nil
+	})
+}
+
+// Post posts all results to the GitHub API as a check run.
+func (p *FindingsPoster) Post() error {
+	// Just show results?
+	if p.dryRun {
+		for finding, _ := range p.findings {
+			// Pretty print, so that this is useful for debugging.
+			fmt.Printf("%s: (%s+%d) %s\n", finding.Category, finding.Path, finding.Line, finding.Message)
+		}
+		return nil
+	}
+
+	// Construct the message.
+	title := "nogo"
+	count := len(p.findings)
+	status := "completed"
+	conclusion := "success"
+	if count > 0 {
+		conclusion = "failure" // Contains errors.
+	}
+	summary := fmt.Sprintf("%d findings.", count)
+	opts := github.CreateCheckRunOptions{
+		Name:        title,
+		HeadSHA:     p.commit,
+		Status:      &status,
+		Conclusion:  &conclusion,
+		StartedAt:   &github.Timestamp{p.startTime},
+		CompletedAt: &github.Timestamp{time.Now()},
+		Output: &github.CheckRunOutput{
+			Title:            &title,
+			Summary:          &summary,
+			AnnotationsCount: &count,
+		},
+	}
+	annotationLevel := "failure" // Always.
+	for finding, _ := range p.findings {
+		opts.Output.Annotations = append(opts.Output.Annotations, &github.CheckRunAnnotation{
+			Path:            &finding.Path,
+			StartLine:       &finding.Line,
+			EndLine:         &finding.Line,
+			Message:         &finding.Message,
+			Title:           &finding.Category,
+			AnnotationLevel: &annotationLevel,
+		})
+	}
+
+	// Post to GitHub.
+	_, _, err := p.client.Checks.CreateCheckRun(context.Background(), p.owner, p.repo, opts)
+	return err
+}

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,
+)

tools/github/reviver/github.go

@@ -12,8 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-// Package github implements reviver.Bugger interface on top of Github issues.
-package github
+package reviver
 
 import (
 	"context"
@@ -23,12 +22,10 @@ import (
 	"time"
 
 	"github.com/google/go-github/github"
-	"golang.org/x/oauth2"
-	"gvisor.dev/gvisor/tools/issue_reviver/reviver"
 )
 
-// Bugger implements reviver.Bugger interface for github issues.
-type Bugger struct {
+// GitHubBugger implements Bugger interface for github issues.
+type GitHubBugger struct {
 	owner  string
 	repo   string
 	dryRun bool
@@ -37,36 +34,25 @@ type Bugger struct {
 	issues map[int]*github.Issue
 }
 
-// NewBugger creates a new Bugger.
-func NewBugger(token, owner, repo string, dryRun bool) (*Bugger, error) {
-	b := &Bugger{
+// 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(token); err != nil {
+	if err := b.load(); err != nil {
 		return nil, err
 	}
 	return b, nil
 }
 
-func (b *Bugger) load(token string) error {
-	ctx := context.Background()
-	if len(token) == 0 {
-		fmt.Print("No OAUTH token provided, using unauthenticated account.\n")
-		b.client = github.NewClient(nil)
-	} else {
-		ts := oauth2.StaticTokenSource(
-			&oauth2.Token{AccessToken: token},
-		)
-		tc := oauth2.NewClient(ctx, ts)
-		b.client = github.NewClient(tc)
-	}
-
+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(ctx, b.owner, b.repo, opts)
+		tmps, resp, err := b.client.Issues.ListByRepo(context.Background(), b.owner, b.repo, opts)
 		if err != nil {
 			return resp, err
 		}
@@ -83,8 +69,8 @@ func (b *Bugger) load(token string) error {
 	return nil
 }
 
-// Activate implements reviver.Bugger.
-func (b *Bugger) Activate(todo *reviver.Todo) (bool, error) {
+// Activate implements Bugger.Activate.
+func (b *GitHubBugger) Activate(todo *Todo) (bool, error) {
 	id, err := parseIssueNo(todo.Issue)
 	if err != nil {
 		return true, err

tools/github/reviver/github_test.go

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package github
+package reviver
 
 import (
 	"testing"

tools/issue_reviver/BUILD

@@ -1,13 +0,0 @@
-load("//tools:defs.bzl", "go_binary")
-
-package(licenses = ["notice"])
-
-go_binary(
-    name = "issue_reviver",
-    srcs = ["main.go"],
-    nogo = False,
-    deps = [
-        "//tools/issue_reviver/github",
-        "//tools/issue_reviver/reviver",
-    ],
-)

tools/issue_reviver/github/BUILD

@@ -1,25 +0,0 @@
-load("//tools:defs.bzl", "go_library", "go_test")
-
-package(licenses = ["notice"])
-
-go_library(
-    name = "github",
-    srcs = ["github.go"],
-    nogo = False,
-    visibility = [
-        "//tools/issue_reviver:__subpackages__",
-    ],
-    deps = [
-        "//tools/issue_reviver/reviver",
-        "@com_github_google_go_github_v28//github:go_default_library",
-        "@org_golang_x_oauth2//:go_default_library",
-    ],
-)
-
-go_test(
-    name = "github_test",
-    size = "small",
-    srcs = ["github_test.go"],
-    library = ":github",
-    nogo = False,
-)

tools/issue_reviver/main.go

@@ -1,100 +0,0 @@
-// 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 main is the entry point for issue_reviver.
-package main
-
-import (
-	"flag"
-	"fmt"
-	"io/ioutil"
-	"os"
-	"strings"
-
-	"gvisor.dev/gvisor/tools/issue_reviver/github"
-	"gvisor.dev/gvisor/tools/issue_reviver/reviver"
-)
-
-var (
-	owner     string
-	repo      string
-	tokenFile string
-	path      string
-	dryRun    bool
-)
-
-// Keep the options simple for now. Supports only a single path and repo.
-func init() {
-	flag.StringVar(&owner, "owner", "", "Github project org/owner to look for issues")
-	flag.StringVar(&repo, "repo", "", "Github repo to look for issues")
-	flag.StringVar(&tokenFile, "oauth-token-file", "", "Path to file containing the OAUTH token to be used as credential to github")
-	flag.StringVar(&path, "path", ".", "Path to scan for TODOs")
-	flag.BoolVar(&dryRun, "dry-run", false, "If set to true, no changes are made to issues")
-}
-
-func main() {
-	// Set defaults from the environment.
-	repository := os.Getenv("GITHUB_REPOSITORY")
-	if parts := strings.SplitN(repository, "/", 2); len(parts) == 2 {
-		owner = parts[0]
-		repo = parts[1]
-	}
-
-	// Parse flags.
-	flag.Parse()
-
-	// Check for mandatory parameters.
-	if len(owner) == 0 {
-		fmt.Println("missing --owner option.")
-		flag.Usage()
-		os.Exit(1)
-	}
-	if len(repo) == 0 {
-		fmt.Println("missing --repo option.")
-		flag.Usage()
-		os.Exit(1)
-	}
-	if len(path) == 0 {
-		fmt.Println("missing --path option.")
-		flag.Usage()
-		os.Exit(1)
-	}
-
-	// The access token may be passed as a file so it doesn't show up in
-	// command line arguments. It also may be provided through the
-	// environment to faciliate use through GitHub's CI system.
-	token := os.Getenv("GITHUB_TOKEN")
-	if len(tokenFile) != 0 {
-		bytes, err := ioutil.ReadFile(tokenFile)
-		if err != nil {
-			fmt.Println(err.Error())
-			os.Exit(1)
-		}
-		token = string(bytes)
-	}
-
-	bugger, err := github.NewBugger(token, owner, repo, dryRun)
-	if err != nil {
-		fmt.Fprintln(os.Stderr, "Error getting github issues:", err)
-		os.Exit(1)
-	}
-	rev := reviver.New([]string{path}, []reviver.Bugger{bugger})
-	if errs := rev.Run(); len(errs) > 0 {
-		fmt.Fprintf(os.Stderr, "Encountered %d errors:\n", len(errs))
-		for _, err := range errs {
-			fmt.Fprintf(os.Stderr, "\t%v\n", err)
-		}
-		os.Exit(1)
-	}
-}

tools/issue_reviver/reviver/BUILD

@@ -1,18 +0,0 @@
-load("//tools:defs.bzl", "go_library", "go_test")
-
-package(licenses = ["notice"])
-
-go_library(
-    name = "reviver",
-    srcs = ["reviver.go"],
-    visibility = [
-        "//tools/issue_reviver:__subpackages__",
-    ],
-)
-
-go_test(
-    name = "reviver_test",
-    size = "small",
-    srcs = ["reviver_test.go"],
-    library = ":reviver",
-)

tools/nogo/build.go

@@ -26,6 +26,9 @@ var (
 	// and should not have any special prefix applied.
 	internalPrefix = fmt.Sprintf("^")
 
+	// internalDefault is applied when no paths are provided.
+	internalDefault = fmt.Sprintf("%s/.*", notPath("external"))
+
 	// externalPrefix is external workspace packages.
 	externalPrefix = "^external/"
 )

tools/nogo/matchers.go

@@ -16,7 +16,6 @@ package nogo
 
 import (
 	"go/token"
-	"path/filepath"
 	"regexp"
 	"strings"
 
@@ -44,11 +43,30 @@ type pathRegexps struct {
 func buildRegexps(prefix string, args ...string) []*regexp.Regexp {
 	result := make([]*regexp.Regexp, 0, len(args))
 	for _, arg := range args {
-		result = append(result, regexp.MustCompile(filepath.Join(prefix, arg)))
+		result = append(result, regexp.MustCompile(prefix+arg))
 	}
 	return result
 }
 
+// notPath works around the lack of backtracking.
+//
+// It is used to construct a regular expression for non-matching components.
+func notPath(name string) string {
+	sb := strings.Builder{}
+	sb.WriteString("(")
+	for i := range name {
+		if i > 0 {
+			sb.WriteString("|")
+		}
+		sb.WriteString(name[:i])
+		sb.WriteString("[^")
+		sb.WriteByte(name[i])
+		sb.WriteString("/][^/]*")
+	}
+	sb.WriteString(")")
+	return sb.String()
+}
+
 // ShouldReport implements matcher.ShouldReport.
 func (p *pathRegexps) ShouldReport(d analysis.Diagnostic, fs *token.FileSet) bool {
 	fullPos := fs.Position(d.Pos).String()
@@ -79,7 +97,7 @@ func externalExcluded(paths ...string) *pathRegexps {
 // internalMatches returns a path matcher for internal packages.
 func internalMatches() *pathRegexps {
 	return &pathRegexps{
-		expr:    buildRegexps(internalPrefix, ".*"),
+		expr:    buildRegexps(internalPrefix, internalDefault),
 		include: true,
 	}
 }

tools/nogo/util/BUILD

@@ -0,0 +1,9 @@
+load("//tools:defs.bzl", "go_library")
+
+package(licenses = ["notice"])
+
+go_library(
+    name = "util",
+    srcs = ["util.go"],
+    visibility = ["//visibility:public"],
+)