diff --git a/go/analysis/internal/checker/checker.go b/go/analysis/internal/checker/checker.go index e0c10298e8..bf8bfdbc51 100644 --- a/go/analysis/internal/checker/checker.go +++ b/go/analysis/internal/checker/checker.go @@ -64,7 +64,8 @@ func RegisterFlags() { // Analysis flags must already have been set. // It provides most of the logic for the main functions of both the // singlechecker and the multi-analysis commands. -func Run(args []string, analyzers []*analysis.Analyzer) error { +// It returns the appropriate exit code. +func Run(args []string, analyzers []*analysis.Analyzer) (exitcode int) { if CPUProfile != "" { f, err := os.Create(CPUProfile) if err != nil { @@ -131,15 +132,14 @@ func Run(args []string, analyzers []*analysis.Analyzer) error { allSyntax := needFacts(analyzers) initial, err := load(args, allSyntax) if err != nil { - return err + log.Print(err) + return 1 // load errors } + // Print the results. roots := analyze(initial, analyzers) - // Print the results. - printDiagnostics(roots) - - return nil + return printDiagnostics(roots) } // load loads the initial packages. @@ -160,6 +160,9 @@ func load(patterns []string, allSyntax bool) ([]*packages.Package, error) { err = fmt.Errorf("error during loading") } } + if len(initial) == 0 { + err = fmt.Errorf("%s matched no packages", strings.Join(patterns, " ")) + } return initial, err } @@ -263,7 +266,12 @@ func analyze(pkgs []*packages.Package, analyzers []*analysis.Analyzer) []*action // printDiagnostics prints the diagnostics for the root packages in either // plain text or JSON format. JSON format also includes errors for any // dependencies. -func printDiagnostics(roots []*action) { +// +// It returns the exitcode: in plain mode, 0 for success, 1 for analysis +// errors, and 3 for diagnostics. We avoid 2 since the flag package uses +// it. JSON mode always succeeds at printing errors and diagnostics in a +// structured form to stdout. +func printDiagnostics(roots []*action) (exitcode int) { // Print the output. // // Print diagnostics only for root packages, @@ -341,6 +349,7 @@ func printDiagnostics(roots []*action) { print = func(act *action) { if act.err != nil { fmt.Fprintf(os.Stderr, "%s: %v\n", act.a.Name, act.err) + exitcode = 1 // analysis failed, at least partially return } if act.isroot { @@ -372,6 +381,10 @@ func printDiagnostics(roots []*action) { } } visitAll(roots) + + if exitcode == 0 && len(seen) > 0 { + exitcode = 3 // successfuly produced diagnostics + } } // Print timing info. @@ -399,6 +412,8 @@ func printDiagnostics(roots []*action) { } } } + + return exitcode } // needFacts reports whether any analysis required by the specified set diff --git a/go/analysis/multichecker/multichecker.go b/go/analysis/multichecker/multichecker.go index 3e9f699d38..d3aa7061a9 100644 --- a/go/analysis/multichecker/multichecker.go +++ b/go/analysis/multichecker/multichecker.go @@ -19,13 +19,6 @@ import ( "golang.org/x/tools/go/analysis/internal/checker" ) -// TODO(adonovan): document (and verify) the exit codes: -// "Vet's exit code is 2 for erroneous invocation of the tool, 1 if a -// problem was reported, and 0 otherwise. Note that the tool does not -// check every possible problem and depends on unreliable heuristics -// so it should be used as guidance only, not as a firm indicator of -// program correctness." - func Main(analyzers ...*analysis.Analyzer) { progname := filepath.Base(os.Args[0]) log.SetFlags(0) @@ -53,7 +46,5 @@ func Main(analyzers ...*analysis.Analyzer) { os.Exit(0) } - if err := checker.Run(args, analyzers); err != nil { - log.Fatal(err) - } + os.Exit(checker.Run(args, analyzers)) } diff --git a/go/analysis/multichecker/multichecker_test.go b/go/analysis/multichecker/multichecker_test.go new file mode 100644 index 0000000000..725007229b --- /dev/null +++ b/go/analysis/multichecker/multichecker_test.go @@ -0,0 +1,82 @@ +// +build go1.12 + +package multichecker_test + +import ( + "fmt" + "os" + "os/exec" + "runtime" + "testing" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/multichecker" + "golang.org/x/tools/go/analysis/passes/findcall" +) + +func main() { + fail := &analysis.Analyzer{ + Name: "fail", + Doc: "always fail on a package 'sort'", + Run: func(pass *analysis.Pass) (interface{}, error) { + if pass.Pkg.Path() == "sort" { + return nil, fmt.Errorf("failed") + } + return nil, nil + }, + } + multichecker.Main(findcall.Analyzer, fail) +} + +// TestExitCode ensures that analysis failures are reported correctly. +// This test fork/execs the main function above. +func TestExitCode(t *testing.T) { + if runtime.GOOS != "linux" { + t.Skipf("skipping fork/exec test on this platform") + } + + if os.Getenv("MULTICHECKER_CHILD") == "1" { + // child process + + // replace [progname -test.run=TestExitCode -- ...] + // by [progname ...] + os.Args = os.Args[2:] + os.Args[0] = "vet" + main() + panic("unreachable") + } + + for _, test := range []struct { + args []string + want int + }{ + {[]string{"nosuchdir/..."}, 1}, // matched no packages + {[]string{"nosuchpkg"}, 1}, // matched no packages + {[]string{"-unknownflag"}, 2}, // flag error + {[]string{"-findcall.name=panic", "io"}, 3}, // finds diagnostics + {[]string{"-findcall=0", "io"}, 0}, // no checkers + {[]string{"-findcall.name=nosuchfunc", "io"}, 0}, // no diagnostics + {[]string{"-findcall.name=panic", "sort", "io"}, 1}, // 'fail' failed on 'sort' + + // -json: exits zero even in face of diagnostics or package errors. + {[]string{"-findcall.name=panic", "-json", "io"}, 0}, + {[]string{"-findcall.name=panic", "-json", "io"}, 0}, + {[]string{"-findcall.name=panic", "-json", "sort", "io"}, 0}, + } { + args := []string{"-test.run=TestExitCode", "--"} + args = append(args, test.args...) + cmd := exec.Command(os.Args[0], args...) + cmd.Env = append(os.Environ(), "MULTICHECKER_CHILD=1") + out, err := cmd.CombinedOutput() + if len(out) > 0 { + t.Logf("%s: out=<<%s>>", test.args, out) + } + var exitcode int + if err, ok := err.(*exec.ExitError); ok { + exitcode = err.ExitCode() // requires go1.12 + } + if exitcode != test.want { + t.Errorf("%s: exited %d, want %d", test.args, exitcode, test.want) + } + } +} diff --git a/go/analysis/singlechecker/singlechecker.go b/go/analysis/singlechecker/singlechecker.go index da54022081..0ea62eab96 100644 --- a/go/analysis/singlechecker/singlechecker.go +++ b/go/analysis/singlechecker/singlechecker.go @@ -67,7 +67,5 @@ func Main(a *analysis.Analyzer) { os.Exit(1) } - if err := checker.Run(args, analyzers); err != nil { - log.Fatal(err) - } + os.Exit(checker.Run(args, analyzers)) }