]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/vet: make checking example names in _test packages more robust
authorKonstantin Shaposhnikov <k.shaposhnikov@gmail.com>
Mon, 27 Jun 2016 09:13:15 +0000 (17:13 +0800)
committerIan Lance Taylor <iant@golang.org>
Tue, 28 Jun 2016 22:09:00 +0000 (22:09 +0000)
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/24488
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

src/cmd/vet/main.go
src/cmd/vet/testdata/divergent/buf_test.go
src/cmd/vet/tests.go
src/cmd/vet/vet_test.go
test/errchk

index 8212a14f0398ba472c331ea6f1265a9d92db5647..4f3cca8f6dd765a8fcbf2ee86a6282e137c9d1df 100644 (file)
@@ -182,6 +182,9 @@ type File struct {
        file    *ast.File
        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.
        // This is used by the recursiveStringer method in print.go.
        stringers map[*ast.Object]bool
@@ -238,7 +241,7 @@ func main() {
                }
                os.Exit(exitCode)
        }
-       if !doPackage(".", flag.Args()) {
+       if doPackage(".", flag.Args(), nil) == nil {
                warnf("no files checked")
        }
        os.Exit(exitCode)
@@ -278,12 +281,12 @@ func doPackageDir(directory string) {
        names = append(names, pkg.TestGoFiles...) // These are also in the "foo" package.
        names = append(names, pkg.SFiles...)
        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.
        if len(pkg.XTestGoFiles) > 0 {
                names = pkg.XTestGoFiles
                prefixDirectory(directory, names)
-               doPackage(directory, names)
+               doPackage(directory, names, basePkg)
        }
 }
 
@@ -299,8 +302,8 @@ type Package struct {
 }
 
 // doPackage analyzes the single package constructed from the named files.
-// It returns whether any files were checked.
-func doPackage(directory string, names []string) bool {
+// It returns the parsed Package or nil if none of the files have been checked.
+func doPackage(directory string, names []string, basePkg *Package) *Package {
        var files []*File
        var astFiles []*ast.File
        fs := token.NewFileSet()
@@ -309,7 +312,7 @@ func doPackage(directory string, names []string) bool {
                if err != nil {
                        // Warn but continue to next package.
                        warnf("%s: %s", name, err)
-                       return false
+                       return nil
                }
                checkBuildTag(name, data)
                var parsedFile *ast.File
@@ -317,14 +320,14 @@ func doPackage(directory string, names []string) bool {
                        parsedFile, err = parser.ParseFile(fs, name, data, 0)
                        if err != nil {
                                warnf("%s: %s", name, err)
-                               return false
+                               return nil
                        }
                        astFiles = append(astFiles, parsedFile)
                }
                files = append(files, &File{fset: fs, content: data, name: name, file: parsedFile})
        }
        if len(astFiles) == 0 {
-               return false
+               return nil
        }
        pkg := new(Package)
        pkg.path = astFiles[0].Name.Name
@@ -346,13 +349,14 @@ func doPackage(directory string, names []string) bool {
        }
        for _, file := range files {
                file.pkg = pkg
+               file.basePkg = basePkg
                file.checkers = chk
                if file.file != nil {
                        file.walkFile(file.name, file.file)
                }
        }
        asmCheck(pkg)
-       return true
+       return pkg
 }
 
 func visit(path string, f os.FileInfo, err error) error {
index 6b9cba3f01d2e073f5afd2f18036924c9e94a9ba..b75d55eaf4f2dd4e7314a98cba17e55a3b02cd57 100644 (file)
@@ -4,11 +4,11 @@ package buf_test
 
 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.
 
@@ -28,8 +28,8 @@ func ExampleBuf_Len(i int) {} // ERROR "ExampleBuf_Len should be niladic"
 
 // "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"
index 076835b9808d8be269c11bcb0d0ee9474f050548..8c051f133620b53e9a282239ee07753aa6737c59 100644 (file)
@@ -59,23 +59,28 @@ func lookup(name string, scopes []*types.Scope) types.Object {
        return nil
 }
 
-func extendedScope(pkg *Package) []*types.Scope {
-       scopes := []*types.Scope{pkg.typesPkg.Scope()}
-
-       pkgName := pkg.typesPkg.Name()
-       if strings.HasSuffix(pkgName, "_test") {
-               basePkg := strings.TrimSuffix(pkgName, "_test")
-               for _, p := range pkg.typesPkg.Imports() {
-                       if p.Name() == basePkg {
-                               scopes = append(scopes, p.Scope())
-                               break
+func extendedScope(f *File) []*types.Scope {
+       scopes := []*types.Scope{f.pkg.typesPkg.Scope()}
+       if f.basePkg != nil {
+               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") {
+                       basePkgName := strings.TrimSuffix(pkgName, "_test")
+                       for _, p := range f.pkg.typesPkg.Imports() {
+                               if p.Name() == basePkgName {
+                                       scopes = append(scopes, p.Scope())
+                                       break
+                               }
                        }
                }
        }
        return scopes
 }
 
-func checkExample(fn *ast.FuncDecl, pkg *Package, report reporter) {
+func checkExample(fn *ast.FuncDecl, f *File, report reporter) {
        fnName := fn.Name.Name
        if params := fn.Type.Params; len(params.List) != 0 {
                report("%s should be niladic", fnName)
@@ -100,7 +105,7 @@ func checkExample(fn *ast.FuncDecl, pkg *Package, report reporter) {
                exName = strings.TrimPrefix(fnName, "Example")
                elems  = strings.SplitN(exName, "_", 3)
                ident  = elems[0]
-               obj    = lookup(ident, extendedScope(pkg))
+               obj    = lookup(ident, extendedScope(f))
        )
        if ident != "" && obj == nil {
                // Check ExampleFoo and ExampleBadFoo.
@@ -173,7 +178,7 @@ func checkTestFunctions(f *File, node ast.Node) {
 
        switch {
        case strings.HasPrefix(fn.Name.Name, "Example"):
-               checkExample(fn, f.pkg, report)
+               checkExample(fn, f, report)
        case strings.HasPrefix(fn.Name.Name, "Test"):
                checkTest(fn, "Test", report)
        case strings.HasPrefix(fn.Name.Name, "Benchmark"):
index 2dd8ae4053350ee33dd97b3a8687881dcff01657..31d4b9001d3de94966a92f45255550f722ec52dc 100644 (file)
@@ -102,7 +102,7 @@ func TestVet(t *testing.T) {
 func TestDivergentPackagesExamples(t *testing.T) {
        Build(t)
        // errchk ./testvet
-       Vet(t, []string{"testdata/divergent/buf.go", "testdata/divergent/buf_test.go"})
+       Vet(t, []string{"testdata/divergent"})
 }
 
 func TestIncompleteExamples(t *testing.T) {
index b07bbc739d487274207e09d553fe5930ec59ace6..bc8ef19cb0bd831e8d1b6cde02cb4993b3097c5a 100755 (executable)
@@ -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) {
        open(SRC, $file) || die "BUG: errchk: open $file: $!";
        $src{$file} = [<SRC>];