Commit 85a4f447 authored by Konstantin Shaposhnikov's avatar Konstantin Shaposhnikov Committed by Ian Lance Taylor

cmd/vet: make checking example names in _test packages more robust

Prior to this change package "foo" had to be installed in order to check
example names in "foo_test" package.

However by the time "foo_test" package is checked a parsed "foo" package
has been already constructed. Use it to check example names.

Also change TestDivergentPackagesExamples test to pass directory of the
package to the vet tool as it is the most common way to invoke it. This
requires changes to errchk to add support for grabbing source files from
a directory.

Fixes #16189

Change-Id: Ief103d07b024822282b86c24250835cc591793e8
Reviewed-on: https://go-review.googlesource.com/24488Reviewed-by: 's avatarIan Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
parent 733aefd0
...@@ -182,6 +182,9 @@ type File struct { ...@@ -182,6 +182,9 @@ type File struct {
file *ast.File file *ast.File
b bytes.Buffer // for use by methods b bytes.Buffer // for use by methods
// Parsed package "foo" when checking package "foo_test"
basePkg *Package
// The objects that are receivers of a "String() string" method. // The objects that are receivers of a "String() string" method.
// This is used by the recursiveStringer method in print.go. // This is used by the recursiveStringer method in print.go.
stringers map[*ast.Object]bool stringers map[*ast.Object]bool
...@@ -238,7 +241,7 @@ func main() { ...@@ -238,7 +241,7 @@ func main() {
} }
os.Exit(exitCode) os.Exit(exitCode)
} }
if !doPackage(".", flag.Args()) { if doPackage(".", flag.Args(), nil) == nil {
warnf("no files checked") warnf("no files checked")
} }
os.Exit(exitCode) os.Exit(exitCode)
...@@ -278,12 +281,12 @@ func doPackageDir(directory string) { ...@@ -278,12 +281,12 @@ func doPackageDir(directory string) {
names = append(names, pkg.TestGoFiles...) // These are also in the "foo" package. names = append(names, pkg.TestGoFiles...) // These are also in the "foo" package.
names = append(names, pkg.SFiles...) names = append(names, pkg.SFiles...)
prefixDirectory(directory, names) prefixDirectory(directory, names)
doPackage(directory, names) basePkg := doPackage(directory, names, nil)
// Is there also a "foo_test" package? If so, do that one as well. // Is there also a "foo_test" package? If so, do that one as well.
if len(pkg.XTestGoFiles) > 0 { if len(pkg.XTestGoFiles) > 0 {
names = pkg.XTestGoFiles names = pkg.XTestGoFiles
prefixDirectory(directory, names) prefixDirectory(directory, names)
doPackage(directory, names) doPackage(directory, names, basePkg)
} }
} }
...@@ -299,8 +302,8 @@ type Package struct { ...@@ -299,8 +302,8 @@ type Package struct {
} }
// doPackage analyzes the single package constructed from the named files. // doPackage analyzes the single package constructed from the named files.
// It returns whether any files were checked. // It returns the parsed Package or nil if none of the files have been checked.
func doPackage(directory string, names []string) bool { func doPackage(directory string, names []string, basePkg *Package) *Package {
var files []*File var files []*File
var astFiles []*ast.File var astFiles []*ast.File
fs := token.NewFileSet() fs := token.NewFileSet()
...@@ -309,7 +312,7 @@ func doPackage(directory string, names []string) bool { ...@@ -309,7 +312,7 @@ func doPackage(directory string, names []string) bool {
if err != nil { if err != nil {
// Warn but continue to next package. // Warn but continue to next package.
warnf("%s: %s", name, err) warnf("%s: %s", name, err)
return false return nil
} }
checkBuildTag(name, data) checkBuildTag(name, data)
var parsedFile *ast.File var parsedFile *ast.File
...@@ -317,14 +320,14 @@ func doPackage(directory string, names []string) bool { ...@@ -317,14 +320,14 @@ func doPackage(directory string, names []string) bool {
parsedFile, err = parser.ParseFile(fs, name, data, 0) parsedFile, err = parser.ParseFile(fs, name, data, 0)
if err != nil { if err != nil {
warnf("%s: %s", name, err) warnf("%s: %s", name, err)
return false return nil
} }
astFiles = append(astFiles, parsedFile) astFiles = append(astFiles, parsedFile)
} }
files = append(files, &File{fset: fs, content: data, name: name, file: parsedFile}) files = append(files, &File{fset: fs, content: data, name: name, file: parsedFile})
} }
if len(astFiles) == 0 { if len(astFiles) == 0 {
return false return nil
} }
pkg := new(Package) pkg := new(Package)
pkg.path = astFiles[0].Name.Name pkg.path = astFiles[0].Name.Name
...@@ -346,13 +349,14 @@ func doPackage(directory string, names []string) bool { ...@@ -346,13 +349,14 @@ func doPackage(directory string, names []string) bool {
} }
for _, file := range files { for _, file := range files {
file.pkg = pkg file.pkg = pkg
file.basePkg = basePkg
file.checkers = chk file.checkers = chk
if file.file != nil { if file.file != nil {
file.walkFile(file.name, file.file) file.walkFile(file.name, file.file)
} }
} }
asmCheck(pkg) asmCheck(pkg)
return true return pkg
} }
func visit(path string, f os.FileInfo, err error) error { func visit(path string, f os.FileInfo, err error) error {
......
...@@ -4,11 +4,11 @@ package buf_test ...@@ -4,11 +4,11 @@ package buf_test
func Example() {} // OK because is package-level. func Example() {} // OK because is package-level.
func Example_suffix() // OK because refers to suffix annotation. func Example_suffix() {} // OK because refers to suffix annotation.
func Example_BadSuffix() // ERROR "Example_BadSuffix has malformed example suffix: BadSuffix" func Example_BadSuffix() {} // ERROR "Example_BadSuffix has malformed example suffix: BadSuffix"
func ExampleBuf() // OK because refers to known top-level type. func ExampleBuf() {} // OK because refers to known top-level type.
func ExampleBuf_Append() {} // OK because refers to known method. func ExampleBuf_Append() {} // OK because refers to known method.
...@@ -28,8 +28,8 @@ func ExampleBuf_Len(i int) {} // ERROR "ExampleBuf_Len should be niladic" ...@@ -28,8 +28,8 @@ func ExampleBuf_Len(i int) {} // ERROR "ExampleBuf_Len should be niladic"
// "Puffer" is German for "Buffer". // "Puffer" is German for "Buffer".
func ExamplePuffer() // ERROR "ExamplePuffer refers to unknown identifier: Puffer" func ExamplePuffer() {} // ERROR "ExamplePuffer refers to unknown identifier: Puffer"
func ExamplePuffer_Append() // ERROR "ExamplePuffer_Append refers to unknown identifier: Puffer" func ExamplePuffer_Append() {} // ERROR "ExamplePuffer_Append refers to unknown identifier: Puffer"
func ExamplePuffer_suffix() // ERROR "ExamplePuffer_suffix refers to unknown identifier: Puffer" func ExamplePuffer_suffix() {} // ERROR "ExamplePuffer_suffix refers to unknown identifier: Puffer"
...@@ -59,23 +59,28 @@ func lookup(name string, scopes []*types.Scope) types.Object { ...@@ -59,23 +59,28 @@ func lookup(name string, scopes []*types.Scope) types.Object {
return nil return nil
} }
func extendedScope(pkg *Package) []*types.Scope { func extendedScope(f *File) []*types.Scope {
scopes := []*types.Scope{pkg.typesPkg.Scope()} scopes := []*types.Scope{f.pkg.typesPkg.Scope()}
if f.basePkg != nil {
pkgName := pkg.typesPkg.Name() scopes = append(scopes, f.basePkg.typesPkg.Scope())
} else {
// If basePkg is not specified (e.g. when checking a single file) try to
// find it among imports.
pkgName := f.pkg.typesPkg.Name()
if strings.HasSuffix(pkgName, "_test") { if strings.HasSuffix(pkgName, "_test") {
basePkg := strings.TrimSuffix(pkgName, "_test") basePkgName := strings.TrimSuffix(pkgName, "_test")
for _, p := range pkg.typesPkg.Imports() { for _, p := range f.pkg.typesPkg.Imports() {
if p.Name() == basePkg { if p.Name() == basePkgName {
scopes = append(scopes, p.Scope()) scopes = append(scopes, p.Scope())
break break
} }
} }
} }
}
return scopes return scopes
} }
func checkExample(fn *ast.FuncDecl, pkg *Package, report reporter) { func checkExample(fn *ast.FuncDecl, f *File, report reporter) {
fnName := fn.Name.Name fnName := fn.Name.Name
if params := fn.Type.Params; len(params.List) != 0 { if params := fn.Type.Params; len(params.List) != 0 {
report("%s should be niladic", fnName) report("%s should be niladic", fnName)
...@@ -100,7 +105,7 @@ func checkExample(fn *ast.FuncDecl, pkg *Package, report reporter) { ...@@ -100,7 +105,7 @@ func checkExample(fn *ast.FuncDecl, pkg *Package, report reporter) {
exName = strings.TrimPrefix(fnName, "Example") exName = strings.TrimPrefix(fnName, "Example")
elems = strings.SplitN(exName, "_", 3) elems = strings.SplitN(exName, "_", 3)
ident = elems[0] ident = elems[0]
obj = lookup(ident, extendedScope(pkg)) obj = lookup(ident, extendedScope(f))
) )
if ident != "" && obj == nil { if ident != "" && obj == nil {
// Check ExampleFoo and ExampleBadFoo. // Check ExampleFoo and ExampleBadFoo.
...@@ -173,7 +178,7 @@ func checkTestFunctions(f *File, node ast.Node) { ...@@ -173,7 +178,7 @@ func checkTestFunctions(f *File, node ast.Node) {
switch { switch {
case strings.HasPrefix(fn.Name.Name, "Example"): case strings.HasPrefix(fn.Name.Name, "Example"):
checkExample(fn, f.pkg, report) checkExample(fn, f, report)
case strings.HasPrefix(fn.Name.Name, "Test"): case strings.HasPrefix(fn.Name.Name, "Test"):
checkTest(fn, "Test", report) checkTest(fn, "Test", report)
case strings.HasPrefix(fn.Name.Name, "Benchmark"): case strings.HasPrefix(fn.Name.Name, "Benchmark"):
......
...@@ -102,7 +102,7 @@ func TestVet(t *testing.T) { ...@@ -102,7 +102,7 @@ func TestVet(t *testing.T) {
func TestDivergentPackagesExamples(t *testing.T) { func TestDivergentPackagesExamples(t *testing.T) {
Build(t) Build(t)
// errchk ./testvet // errchk ./testvet
Vet(t, []string{"testdata/divergent/buf.go", "testdata/divergent/buf_test.go"}) Vet(t, []string{"testdata/divergent"})
} }
func TestIncompleteExamples(t *testing.T) { func TestIncompleteExamples(t *testing.T) {
......
...@@ -37,6 +37,17 @@ foreach(reverse 0 .. @ARGV-1) { ...@@ -37,6 +37,17 @@ foreach(reverse 0 .. @ARGV-1) {
} }
} }
# If no files have been specified try to grab SOURCEFILES from the last
# argument that is an existing directory if any
unless(@file) {
foreach(reverse 0 .. @ARGV-1) {
if(-d $ARGV[$_]) {
@file = glob($ARGV[$_] . "/*.go");
last;
}
}
}
foreach $file (@file) { foreach $file (@file) {
open(SRC, $file) || die "BUG: errchk: open $file: $!"; open(SRC, $file) || die "BUG: errchk: open $file: $!";
$src{$file} = [<SRC>]; $src{$file} = [<SRC>];
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment