go/analysis: harmonize flags across all checkers
The -json and -c=N flags, formerly belonging only to the
go/packages-based {single,multi}checkers, are now supported by
unitchecker as well.
The no-op -source, -v, -all, and -tags flags, formerly belonging only
to unitchecker, have moved to the analysisflags package, which is
common to all checkers.
The -flags flag now reports all registered flags (except the
{single,multi}checker-only debugging flags) rather than just those
related to analyzers, allowing one to say: 'go vet -json' or 'go vet -c=1'.
The code for printing diagnostics, either plain or in JSON, has been
factored and moved into the common analysisflags package.
This CL depends on https://go-review.googlesource.com/c/go/+/149960 to
cmd/go, which causes 'go vet' to populate the ID field of the *.cfg.
This field is used as a key in the JSON tree.
Added basic tests of the new -json and -c unitchecker flags.
Change-Id: Ia7a3a9adc86de067de060732d2c200c58be3945a
Reviewed-on: https://go-review.googlesource.com/c/150038
Reviewed-by: Michael Matloob <matloob@golang.org>
diff --git a/go/analysis/cmd/vet-lite/main.go b/go/analysis/cmd/vet-lite/main.go
index cf7441a..259d397 100644
--- a/go/analysis/cmd/vet-lite/main.go
+++ b/go/analysis/cmd/vet-lite/main.go
@@ -7,8 +7,6 @@
package main
import (
- "flag"
-
"golang.org/x/tools/go/analysis/unitchecker"
"golang.org/x/tools/go/analysis/passes/asmdecl"
@@ -34,11 +32,6 @@
"golang.org/x/tools/go/analysis/passes/unusedresult"
)
-// Flags for legacy vet compatibility.
-//
-// These flags, plus the shims in analysisflags, enable
-// existing scripts that run vet to continue to work.
-//
// Legacy vet had the concept of "experimental" checkers. There
// was exactly one, shadow, and it had to be explicitly enabled
// by the -shadow flag, which would of course disable all the
@@ -53,12 +46,6 @@
// Alternatively, one could build a multichecker containing all
// the desired checks (vet's suite + shadow) and run it in a
// single "go vet" command.
-func init() {
- _ = flag.Bool("source", false, "no effect (deprecated)")
- _ = flag.Bool("v", false, "no effect (deprecated)")
- _ = flag.Bool("all", false, "no effect (deprecated)")
- _ = flag.String("tags", "", "no effect (deprecated)")
-}
func main() {
unitchecker.Main(
diff --git a/go/analysis/internal/analysisflags/flags.go b/go/analysis/internal/analysisflags/flags.go
index d915be6..729ac3b 100644
--- a/go/analysis/internal/analysisflags/flags.go
+++ b/go/analysis/internal/analysisflags/flags.go
@@ -11,27 +11,29 @@
"encoding/json"
"flag"
"fmt"
+ "go/token"
"io"
+ "io/ioutil"
"log"
"os"
"strconv"
+ "strings"
"golang.org/x/tools/go/analysis"
)
+// flags common to all {single,multi,unit}checkers.
+var (
+ JSON = false // -json
+ Context = -1 // -c=N: if N>0, display offending line plus N lines of context
+)
+
// Parse creates a flag for each of the analyzer's flags,
// including (in multi mode) a flag named after the analyzer,
// parses the flags, then filters and returns the list of
// analyzers enabled by flags.
func Parse(analyzers []*analysis.Analyzer, multi bool) []*analysis.Analyzer {
// Connect each analysis flag to the command line as -analysis.flag.
- type analysisFlag struct {
- Name string
- Bool bool
- Usage string
- }
- var analysisFlags []analysisFlag
-
enabled := make(map[*analysis.Analyzer]*triState)
for _, a := range analyzers {
var prefix string
@@ -44,7 +46,6 @@
enableUsage := "enable " + a.Name + " analysis"
flag.Var(enable, a.Name, enableUsage)
enabled[a] = enable
- analysisFlags = append(analysisFlags, analysisFlag{a.Name, true, enableUsage})
}
a.Flags.VisitAll(func(f *flag.Flag) {
@@ -55,12 +56,6 @@
name := prefix + f.Name
flag.Var(f.Value, name, f.Usage)
-
- var isBool bool
- if b, ok := f.Value.(interface{ IsBoolFlag() bool }); ok {
- isBool = b.IsBoolFlag()
- }
- analysisFlags = append(analysisFlags, analysisFlag{name, isBool, f.Usage})
})
}
@@ -68,7 +63,16 @@
printflags := flag.Bool("flags", false, "print analyzer flags in JSON")
addVersionFlag()
- // Add shims for legacy vet flags.
+ // flags common to all checkers
+ flag.BoolVar(&JSON, "json", JSON, "emit JSON output")
+ flag.IntVar(&Context, "c", Context, `display offending line with this many lines of context`)
+
+ // Add shims for legacy vet flags to enable existing
+ // scripts that run vet to continue to work.
+ _ = flag.Bool("source", false, "no effect (deprecated)")
+ _ = flag.Bool("v", false, "no effect (deprecated)")
+ _ = flag.Bool("all", false, "no effect (deprecated)")
+ _ = flag.String("tags", "", "no effect (deprecated)")
for old, new := range vetLegacyFlags {
newFlag := flag.Lookup(new)
if newFlag != nil && flag.Lookup(old) == nil {
@@ -80,11 +84,7 @@
// -flags: print flags so that go vet knows which ones are legitimate.
if *printflags {
- data, err := json.MarshalIndent(analysisFlags, "", "\t")
- if err != nil {
- log.Fatal(err)
- }
- os.Stdout.Write(data)
+ printFlags()
os.Exit(0)
}
@@ -122,6 +122,33 @@
return analyzers
}
+func printFlags() {
+ type jsonFlag struct {
+ Name string
+ Bool bool
+ Usage string
+ }
+ var flags []jsonFlag = nil
+ flag.VisitAll(func(f *flag.Flag) {
+ // Don't report {single,multi}checker debugging
+ // flags as these have no effect on unitchecker
+ // (as invoked by 'go vet').
+ switch f.Name {
+ case "debug", "cpuprofile", "memprofile", "trace":
+ return
+ }
+
+ b, ok := f.Value.(interface{ IsBoolFlag() bool })
+ isBool := ok && b.IsBoolFlag()
+ flags = append(flags, jsonFlag{f.Name, isBool, f.Usage})
+ })
+ data, err := json.MarshalIndent(flags, "", "\t")
+ if err != nil {
+ log.Fatal(err)
+ }
+ os.Stdout.Write(data)
+}
+
// addVersionFlag registers a -V flag that, if set,
// prints the executable version and exits 0.
//
@@ -247,3 +274,70 @@
"unusedfuncs": "unusedresult.funcs",
"unusedstringmethods": "unusedresult.stringmethods",
}
+
+// ---- output helpers common to all drivers ----
+
+// PrintPlain prints a diagnostic in plain text form,
+// with context specified by the -c flag.
+func PrintPlain(fset *token.FileSet, diag analysis.Diagnostic) {
+ posn := fset.Position(diag.Pos)
+ fmt.Fprintf(os.Stderr, "%s: %s\n", posn, diag.Message)
+
+ // -c=N: show offending line plus N lines of context.
+ if Context >= 0 {
+ data, _ := ioutil.ReadFile(posn.Filename)
+ lines := strings.Split(string(data), "\n")
+ for i := posn.Line - Context; i <= posn.Line+Context; i++ {
+ if 1 <= i && i <= len(lines) {
+ fmt.Fprintf(os.Stderr, "%d\t%s\n", i, lines[i-1])
+ }
+ }
+ }
+}
+
+// A JSONTree is a mapping from package ID to analysis name to result.
+// Each result is either a jsonError or a list of jsonDiagnostic.
+type JSONTree map[string]map[string]interface{}
+
+// Add adds the result of analysis 'name' on package 'id'.
+// The result is either a list of diagnostics or an error.
+func (tree JSONTree) Add(fset *token.FileSet, id, name string, diags []analysis.Diagnostic, err error) {
+ var v interface{}
+ if err != nil {
+ type jsonError struct {
+ Err string `json:"error"`
+ }
+ v = jsonError{err.Error()}
+ } else if len(diags) > 0 {
+ type jsonDiagnostic struct {
+ Category string `json:"category,omitempty"`
+ Posn string `json:"posn"`
+ Message string `json:"message"`
+ }
+ var diagnostics []jsonDiagnostic
+ for _, f := range diags {
+ diagnostics = append(diagnostics, jsonDiagnostic{
+ Category: f.Category,
+ Posn: fset.Position(f.Pos).String(),
+ Message: f.Message,
+ })
+ }
+ v = diagnostics
+ }
+ if v != nil {
+ m, ok := tree[id]
+ if !ok {
+ m = make(map[string]interface{})
+ tree[id] = m
+ }
+ m[name] = v
+ }
+}
+
+func (tree JSONTree) Print() {
+ data, err := json.MarshalIndent(tree, "", "\t")
+ if err != nil {
+ log.Panicf("internal error: JSON marshalling failed: %v", err)
+ }
+ fmt.Printf("%s\n", data)
+}
diff --git a/go/analysis/internal/checker/checker.go b/go/analysis/internal/checker/checker.go
index 80fa02f..6c65179 100644
--- a/go/analysis/internal/checker/checker.go
+++ b/go/analysis/internal/checker/checker.go
@@ -7,12 +7,10 @@
import (
"bytes"
"encoding/gob"
- "encoding/json"
"flag"
"fmt"
"go/token"
"go/types"
- "io/ioutil"
"log"
"os"
"reflect"
@@ -25,12 +23,11 @@
"time"
"golang.org/x/tools/go/analysis"
+ "golang.org/x/tools/go/analysis/internal/analysisflags"
"golang.org/x/tools/go/packages"
)
var (
- JSON = false
-
// Debug is a set of single-letter flags:
//
// f show [f]acts as they are created
@@ -41,17 +38,16 @@
//
Debug = ""
- Context = -1 // if >=0, display offending line plus this many lines of context
-
// Log files for optional performance tracing.
CPUProfile, MemProfile, Trace string
)
// RegisterFlags registers command-line flags used the analysis driver.
func RegisterFlags() {
- flag.BoolVar(&JSON, "json", JSON, "emit JSON output")
+ // When adding flags here, remember to update
+ // the list of suppressed flags in analysisflags.
+
flag.StringVar(&Debug, "debug", Debug, `debug flags, any subset of "lpsv"`)
- flag.IntVar(&Context, "c", Context, `display offending line with this many lines of context`)
flag.StringVar(&CPUProfile, "cpuprofile", "", "write CPU profile to this file")
flag.StringVar(&MemProfile, "memprofile", "", "write memory profile to this file")
@@ -276,50 +272,18 @@
}
}
- if JSON {
- tree := make(map[string]map[string]interface{}) // ID -> analysis -> result
-
+ if analysisflags.JSON {
+ // JSON output
+ tree := make(analysisflags.JSONTree)
print = func(act *action) {
- m, existing := tree[act.pkg.ID]
- if !existing {
- m = make(map[string]interface{})
- // Insert m into tree later iff non-empty.
+ var diags []analysis.Diagnostic
+ if act.isroot {
+ diags = act.diagnostics
}
- if act.err != nil {
- type jsonError struct {
- Err string `json:"error"`
- }
- m[act.a.Name] = jsonError{act.err.Error()}
- } else if act.isroot {
- type jsonDiagnostic struct {
- Category string `json:"category,omitempty"`
- Posn string `json:"posn"`
- Message string `json:"message"`
- }
- var diagnostics []jsonDiagnostic
- for _, f := range act.diagnostics {
- diagnostics = append(diagnostics, jsonDiagnostic{
- Category: f.Category,
- Posn: act.pkg.Fset.Position(f.Pos).String(),
- Message: f.Message,
- })
- }
- if diagnostics != nil {
- m[act.a.Name] = diagnostics
- }
- }
- if !existing && len(m) > 0 {
- tree[act.pkg.ID] = m
- }
+ tree.Add(act.pkg.Fset, act.pkg.ID, act.a.Name, diags, act.err)
}
visitAll(roots)
-
- data, err := json.MarshalIndent(tree, "", "\t")
- if err != nil {
- log.Panicf("internal error: JSON marshalling failed: %v", err)
- }
- os.Stdout.Write(data)
- fmt.Println()
+ tree.Print()
} else {
// plain text output
@@ -340,30 +304,18 @@
return
}
if act.isroot {
- for _, f := range act.diagnostics {
+ for _, diag := range act.diagnostics {
// We don't display a.Name/f.Category
// as most users don't care.
- posn := act.pkg.Fset.Position(f.Pos)
-
- k := key{posn, act.a, f.Message}
+ posn := act.pkg.Fset.Position(diag.Pos)
+ k := key{posn, act.a, diag.Message}
if seen[k] {
continue // duplicate
}
seen[k] = true
- fmt.Fprintf(os.Stderr, "%s: %s\n", posn, f.Message)
-
- // -c=0: show offending line of code in context.
- if Context >= 0 {
- data, _ := ioutil.ReadFile(posn.Filename)
- lines := strings.Split(string(data), "\n")
- for i := posn.Line - Context; i <= posn.Line+Context; i++ {
- if 1 <= i && i <= len(lines) {
- fmt.Fprintf(os.Stderr, "%d\t%s\n", i, lines[i-1])
- }
- }
- }
+ analysisflags.PrintPlain(act.pkg.Fset, diag)
}
}
}
diff --git a/go/analysis/unitchecker/unitchecker.go b/go/analysis/unitchecker/unitchecker.go
index edfca57..7b8fec9 100644
--- a/go/analysis/unitchecker/unitchecker.go
+++ b/go/analysis/unitchecker/unitchecker.go
@@ -25,7 +25,6 @@
// so we will not get to analyze it. Yet we must in order
// to create base facts for, say, the fmt package for the
// printf checker.
-// - support JSON output, factored with multichecker.
import (
"encoding/gob"
@@ -57,6 +56,7 @@
// It is provided to the tool in a JSON-encoded file
// whose name ends with ".cfg".
type Config struct {
+ ID string // e.g. "fmt [fmt.test]"
Compiler string
Dir string
ImportPath string
@@ -127,16 +127,37 @@
}
fset := token.NewFileSet()
- diags, err := run(fset, cfg, analyzers)
+ results, err := run(fset, cfg, analyzers)
if err != nil {
log.Fatal(err)
}
- if !cfg.VetxOnly && len(diags) > 0 {
- for _, diag := range diags {
- fmt.Fprintf(os.Stderr, "%s: %s\n", fset.Position(diag.Pos), diag.Message)
+ // In VetxOnly mode, the analysis is run only for facts.
+ if !cfg.VetxOnly {
+ if analysisflags.JSON {
+ // JSON output
+ tree := make(analysisflags.JSONTree)
+ for _, res := range results {
+ tree.Add(fset, cfg.ID, res.a.Name, res.diagnostics, res.err)
+ }
+ tree.Print()
+ } else {
+ // plain text
+ exit := 0
+ for _, res := range results {
+ if res.err != nil {
+ log.Println(res.err)
+ exit = 1
+ }
+ }
+ for _, res := range results {
+ for _, diag := range res.diagnostics {
+ analysisflags.PrintPlain(fset, diag)
+ exit = 1
+ }
+ }
+ os.Exit(exit)
}
- os.Exit(1)
}
os.Exit(0)
@@ -160,7 +181,7 @@
return cfg, nil
}
-func run(fset *token.FileSet, cfg *Config, analyzers []*analysis.Analyzer) ([]analysis.Diagnostic, error) {
+func run(fset *token.FileSet, cfg *Config, analyzers []*analysis.Analyzer) ([]result, error) {
// Load, parse, typecheck.
var files []*ast.File
for _, name := range cfg.GoFiles {
@@ -333,14 +354,13 @@
execAll(analyzers)
- // Return diagnostics from root analyzers.
- var diags []analysis.Diagnostic
- for _, a := range analyzers {
+ // Return diagnostics and errors from root analyzers.
+ results := make([]result, len(analyzers))
+ for i, a := range analyzers {
act := actions[a]
- if act.err != nil {
- return nil, act.err // some analysis failed
- }
- diags = append(diags, act.diagnostics...)
+ results[i].a = a
+ results[i].err = act.err
+ results[i].diagnostics = act.diagnostics
}
data := facts.Encode()
@@ -348,7 +368,13 @@
return nil, fmt.Errorf("failed to write analysis facts: %v", err)
}
- return diags, nil
+ return results, nil
+}
+
+type result struct {
+ a *analysis.Analyzer
+ diagnostics []analysis.Diagnostic
+ err error
}
type importerFunc func(path string) (*types.Package, error)
diff --git a/go/analysis/unitchecker/unitchecker_test.go b/go/analysis/unitchecker/unitchecker_test.go
index c6bb2bf..3fc12a5 100644
--- a/go/analysis/unitchecker/unitchecker_test.go
+++ b/go/analysis/unitchecker/unitchecker_test.go
@@ -60,14 +60,29 @@
testdata/src/b/b.go:6:13: call of MyFunc123(...)
testdata/src/b/b.go:7:11: call of MyFunc123(...)
`
+ const wantAJSON = `# a
+{
+ "a": {
+ "findcall": [
+ {
+ "posn": "$GOPATH/src/a/a.go:4:11",
+ "message": "call of MyFunc123(...)"
+ }
+ ]
+ }
+}
+`
for _, test := range []struct {
- args string
- want string
+ args string
+ wantOut string
+ wantExit int
}{
- {args: "a", want: wantA},
- {args: "b", want: wantB},
- {args: "a b", want: wantA + wantB},
+ {args: "a", wantOut: wantA, wantExit: 2},
+ {args: "b", wantOut: wantB, wantExit: 2},
+ {args: "a b", wantOut: wantA + wantB, wantExit: 2},
+ {args: "-json a", wantOut: wantAJSON, wantExit: 0},
+ {args: "-c=0 a", wantOut: wantA + "4 MyFunc123()\n", wantExit: 2},
} {
cmd := exec.Command("go", "vet", "-vettool="+os.Args[0], "-findcall.name=MyFunc123")
cmd.Args = append(cmd.Args, strings.Fields(test.args)...)
@@ -77,16 +92,17 @@
)
out, err := cmd.CombinedOutput()
- exitcode := -1
+ exitcode := 0
if exitErr, ok := err.(*exec.ExitError); ok {
exitcode = exitErr.ExitCode()
}
- if exitcode != 2 {
- t.Errorf("%s: got exit code %d, want 2", test.args, exitcode)
+ if exitcode != test.wantExit {
+ t.Errorf("%s: got exit code %d, want %d", test.args, exitcode, test.wantExit)
}
+ got := strings.Replace(string(out), testdata, "$GOPATH", -1)
- if got := string(out); got != test.want {
- t.Errorf("%s: got <<%s>>, want <<%s>>", test.args, got, test.want)
+ if got != test.wantOut {
+ t.Errorf("%s: got <<%s>>, want <<%s>>", test.args, got, test.wantOut)
}
}
}
