Skip to content

Commit

Permalink
go/analysis/unitchecker: add test of go vet on std
Browse files Browse the repository at this point in the history
This change adds a new test that runs go vet on std, using
a unitchecker-based tool with (hopefully) the same set of
analyzers as the real cmd/vet in GOROOT. This should serve
as an early warning when a change to an analyzer causes
it to become stricter on std packages.

Change-Id: I87503f40ad2d54b928d876216a951ed88b216e58
Reviewed-on: https://go-review.googlesource.com/c/tools/+/493619
Run-TryBot: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
  • Loading branch information
adonovan committed May 8, 2023
1 parent 23e52a3 commit 8f7fb01
Show file tree
Hide file tree
Showing 5 changed files with 123 additions and 32 deletions.
14 changes: 14 additions & 0 deletions go/analysis/unitchecker/main.go
Expand Up @@ -27,16 +27,23 @@ import (
"golang.org/x/tools/go/analysis/passes/cgocall"
"golang.org/x/tools/go/analysis/passes/composite"
"golang.org/x/tools/go/analysis/passes/copylock"
"golang.org/x/tools/go/analysis/passes/directive"
"golang.org/x/tools/go/analysis/passes/errorsas"
"golang.org/x/tools/go/analysis/passes/framepointer"
"golang.org/x/tools/go/analysis/passes/httpresponse"
"golang.org/x/tools/go/analysis/passes/ifaceassert"
"golang.org/x/tools/go/analysis/passes/loopclosure"
"golang.org/x/tools/go/analysis/passes/lostcancel"
"golang.org/x/tools/go/analysis/passes/nilfunc"
"golang.org/x/tools/go/analysis/passes/printf"
"golang.org/x/tools/go/analysis/passes/shift"
"golang.org/x/tools/go/analysis/passes/sigchanyzer"
"golang.org/x/tools/go/analysis/passes/stdmethods"
"golang.org/x/tools/go/analysis/passes/stringintconv"
"golang.org/x/tools/go/analysis/passes/structtag"
"golang.org/x/tools/go/analysis/passes/testinggoroutine"
"golang.org/x/tools/go/analysis/passes/tests"
"golang.org/x/tools/go/analysis/passes/timeformat"
"golang.org/x/tools/go/analysis/passes/unmarshal"
"golang.org/x/tools/go/analysis/passes/unreachable"
"golang.org/x/tools/go/analysis/passes/unsafeptr"
Expand All @@ -53,16 +60,23 @@ func main() {
cgocall.Analyzer,
composite.Analyzer,
copylock.Analyzer,
directive.Analyzer,
errorsas.Analyzer,
framepointer.Analyzer,
httpresponse.Analyzer,
ifaceassert.Analyzer,
loopclosure.Analyzer,
lostcancel.Analyzer,
nilfunc.Analyzer,
printf.Analyzer,
shift.Analyzer,
sigchanyzer.Analyzer,
stdmethods.Analyzer,
stringintconv.Analyzer,
structtag.Analyzer,
tests.Analyzer,
testinggoroutine.Analyzer,
timeformat.Analyzer,
unmarshal.Analyzer,
unreachable.Analyzer,
unsafeptr.Analyzer,
Expand Down
7 changes: 1 addition & 6 deletions go/analysis/unitchecker/unitchecker.go
Expand Up @@ -183,11 +183,6 @@ func readConfig(filename string) (*Config, error) {
return cfg, nil
}

var importerForCompiler = func(_ *token.FileSet, compiler string, lookup importer.Lookup) types.Importer {
// broken legacy implementation (https://golang.org/issue/28995)
return importer.For(compiler, lookup)
}

func run(fset *token.FileSet, cfg *Config, analyzers []*analysis.Analyzer) ([]result, error) {
// Load, parse, typecheck.
var files []*ast.File
Expand All @@ -203,7 +198,7 @@ func run(fset *token.FileSet, cfg *Config, analyzers []*analysis.Analyzer) ([]re
}
files = append(files, f)
}
compilerImporter := importerForCompiler(fset, cfg.Compiler, func(path string) (io.ReadCloser, error) {
compilerImporter := importer.ForCompiler(fset, cfg.Compiler, func(path string) (io.ReadCloser, error) {
// path is a resolved package path, not an import path.
file, ok := cfg.PackageFile[path]
if !ok {
Expand Down
14 changes: 0 additions & 14 deletions go/analysis/unitchecker/unitchecker112.go

This file was deleted.

23 changes: 11 additions & 12 deletions go/analysis/unitchecker/unitchecker_test.go
Expand Up @@ -2,15 +2,8 @@
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

//go:build go1.12
// +build go1.12

package unitchecker_test

// This test depends on features such as
// go vet's support for vetx files (1.11) and
// the (*os.ProcessState).ExitCode method (1.12).

import (
"flag"
"os"
Expand All @@ -28,17 +21,23 @@ import (
)

func TestMain(m *testing.M) {
if os.Getenv("UNITCHECKER_CHILD") == "1" {
// child process
main()
// child process?
switch os.Getenv("ENTRYPOINT") {
case "vet":
vet()
panic("unreachable")
case "minivet":
minivet()
panic("unreachable")
}

// test process
flag.Parse()
os.Exit(m.Run())
}

func main() {
// minivet is a vet-like tool with a few analyzers, for testing.
func minivet() {
unitchecker.Main(
findcall.Analyzer,
printf.Analyzer,
Expand Down Expand Up @@ -162,7 +161,7 @@ func _() {
} {
cmd := exec.Command("go", "vet", "-vettool="+os.Args[0], "-findcall.name=MyFunc123")
cmd.Args = append(cmd.Args, strings.Fields(test.args)...)
cmd.Env = append(exported.Config.Env, "UNITCHECKER_CHILD=1")
cmd.Env = append(exported.Config.Env, "ENTRYPOINT=minivet")
cmd.Dir = exported.Config.Dir

out, err := cmd.CombinedOutput()
Expand Down
97 changes: 97 additions & 0 deletions go/analysis/unitchecker/vet_std_test.go
@@ -0,0 +1,97 @@
// Copyright 2023 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

package unitchecker_test

import (
"os"
"os/exec"
"runtime"
"strings"
"testing"

"golang.org/x/tools/go/analysis/passes/asmdecl"
"golang.org/x/tools/go/analysis/passes/assign"
"golang.org/x/tools/go/analysis/passes/atomic"
"golang.org/x/tools/go/analysis/passes/bools"
"golang.org/x/tools/go/analysis/passes/buildtag"
"golang.org/x/tools/go/analysis/passes/cgocall"
"golang.org/x/tools/go/analysis/passes/composite"
"golang.org/x/tools/go/analysis/passes/copylock"
"golang.org/x/tools/go/analysis/passes/directive"
"golang.org/x/tools/go/analysis/passes/errorsas"
"golang.org/x/tools/go/analysis/passes/framepointer"
"golang.org/x/tools/go/analysis/passes/httpresponse"
"golang.org/x/tools/go/analysis/passes/ifaceassert"
"golang.org/x/tools/go/analysis/passes/loopclosure"
"golang.org/x/tools/go/analysis/passes/lostcancel"
"golang.org/x/tools/go/analysis/passes/nilfunc"
"golang.org/x/tools/go/analysis/passes/printf"
"golang.org/x/tools/go/analysis/passes/shift"
"golang.org/x/tools/go/analysis/passes/sigchanyzer"
"golang.org/x/tools/go/analysis/passes/stdmethods"
"golang.org/x/tools/go/analysis/passes/stringintconv"
"golang.org/x/tools/go/analysis/passes/structtag"
"golang.org/x/tools/go/analysis/passes/testinggoroutine"
"golang.org/x/tools/go/analysis/passes/tests"
"golang.org/x/tools/go/analysis/passes/timeformat"
"golang.org/x/tools/go/analysis/passes/unmarshal"
"golang.org/x/tools/go/analysis/passes/unreachable"
"golang.org/x/tools/go/analysis/passes/unusedresult"
"golang.org/x/tools/go/analysis/unitchecker"
)

// vet is the entrypoint of this executable when ENTRYPOINT=vet.
// Keep consistent with the actual vet in GOROOT/src/cmd/vet/main.go.
func vet() {
unitchecker.Main(
asmdecl.Analyzer,
assign.Analyzer,
atomic.Analyzer,
bools.Analyzer,
buildtag.Analyzer,
cgocall.Analyzer,
composite.Analyzer,
copylock.Analyzer,
directive.Analyzer,
errorsas.Analyzer,
framepointer.Analyzer,
httpresponse.Analyzer,
ifaceassert.Analyzer,
loopclosure.Analyzer,
lostcancel.Analyzer,
nilfunc.Analyzer,
printf.Analyzer,
shift.Analyzer,
sigchanyzer.Analyzer,
stdmethods.Analyzer,
stringintconv.Analyzer,
structtag.Analyzer,
tests.Analyzer,
testinggoroutine.Analyzer,
timeformat.Analyzer,
unmarshal.Analyzer,
unreachable.Analyzer,
// unsafeptr.Analyzer, // currently reports findings in runtime
unusedresult.Analyzer,
)
}

// TestVetStdlib runs the same analyzers as the actual vet over the
// standard library, using go vet and unitchecker, to ensure that
// there are no findings.
func TestVetStdlib(t *testing.T) {
if testing.Short() {
t.Skip("skipping in -short mode")
}
if version := runtime.Version(); !strings.HasPrefix(version, "devel") {
t.Skipf("This test is only wanted on development branches where code can be easily fixed. Skipping because runtime.Version=%q.", version)
}

cmd := exec.Command("go", "vet", "-vettool="+os.Args[0], "std")
cmd.Env = append(os.Environ(), "ENTRYPOINT=vet")
if out, err := cmd.CombinedOutput(); err != nil {
t.Errorf("go vet std failed (%v):\n%s", err, out)
}
}

0 comments on commit 8f7fb01

Please sign in to comment.