cmd/doc: search the tree in breadth-first order

This is a simple change to the command that should resolve problems like finding
vendored packages before their non-vendored siblings. By searching in breadth-first
order, we find the matching package lowest in the hierarchy, which is more likely
to be correct than the deeper one, such as a vendored package, that will be found
in a depth-first scan.

This may be sufficient to resolve the issue, and has the merit that it is very easy
to explain. I will leave the issue open for now in case my intuition is wrong.

Update #12423

Change-Id: Icf69e8beb1845277203fcb7d19ffb7cca9fa41f5
Reviewed-on: https://go-review.googlesource.com/17691
Reviewed-by: Russ Cox <rsc@golang.org>

diff --git a/src/cmd/doc/dirs.go b/src/cmd/doc/dirs.go
index e33cbc8ec77c896c216ef6fa30fa15877808f3f4..2982eeeb105a27679a78aa9774f5431c1a521d79 100644 (file)
--- a/src/cmd/doc/dirs.go
+++ b/src/cmd/doc/dirs.go
@@ -6,6 +6,7 @@ package main
 
 import (
        "go/build"
+       "log"
        "os"
        "path"
        "path/filepath"
@@ -54,65 +55,61 @@ func (d *Dirs) Next() (string, bool) {
 
 // walk walks the trees in GOROOT and GOPATH.
 func (d *Dirs) walk() {
-       d.walkRoot(build.Default.GOROOT)
+       d.bfsWalkRoot(build.Default.GOROOT)
        for _, root := range splitGopath() {
-               d.walkRoot(root)
+               d.bfsWalkRoot(root)
        }
        close(d.scan)
 }
 
-// walkRoot walks a single directory. Each Go source directory it finds is
-// delivered on d.scan.
-func (d *Dirs) walkRoot(root string) {
+// bfsWalkRoot walks a single directory hierarchy in breadth-first lexical order.
+// Each Go source directory it finds is delivered on d.scan.
+func (d *Dirs) bfsWalkRoot(root string) {
        root = path.Join(root, "src")
-       slashDot := string(filepath.Separator) + "."
-       // We put a slash on the pkg so can use simple string comparison below
-       // yet avoid inadvertent matches, like /foobar matching bar.
 
-       visit := func(pathName string, f os.FileInfo, err error) error {
-               if err != nil {
-                       return nil
-               }
-               // One package per directory. Ignore the files themselves.
-               if !f.IsDir() {
-                       return nil
-               }
-               // No .git or other dot nonsense please.
-               if strings.Contains(pathName, slashDot) {
-                       return filepath.SkipDir
-               }
-               // Does the directory contain any Go files? If so, it's a candidate.
-               if hasGoFiles(pathName) {
-                       d.scan <- pathName
-                       return nil
-               }
-               return nil
-       }
-
-       filepath.Walk(root, visit)
-}
+       // this is the queue of directories to examine in this pass.
+       this := []string{}
+       // next is the queue of directories to examine in the next pass.
+       next := []string{root}
 
-// hasGoFiles tests whether the directory contains at least one file with ".go"
-// extension
-func hasGoFiles(path string) bool {
-       dir, err := os.Open(path)
-       if err != nil {
-               // ignore unreadable directories
-               return false
-       }
-       defer dir.Close()
-
-       names, err := dir.Readdirnames(0)
-       if err != nil {
-               // ignore unreadable directories
-               return false
-       }
-
-       for _, name := range names {
-               if strings.HasSuffix(name, ".go") {
-                       return true
+       for len(next) > 0 {
+               this, next = next, this[0:0]
+               for _, dir := range this {
+                       fd, err := os.Open(dir)
+                       if err != nil {
+                               log.Printf("error opening %s: %v", dir, err)
+                               return // TODO? There may be entry before the error.
+                       }
+                       entries, err := fd.Readdir(0)
+                       fd.Close()
+                       if err != nil {
+                               log.Printf("error reading %s: %v", dir, err)
+                               return // TODO? There may be entry before the error.
+                       }
+                       hasGoFiles := false
+                       for _, entry := range entries {
+                               name := entry.Name()
+                               // For plain files, remember if this directory contains any .go
+                               // source files, but ignore them otherwise.
+                               if !entry.IsDir() {
+                                       if !hasGoFiles && strings.HasSuffix(name, ".go") {
+                                               hasGoFiles = true
+                                       }
+                                       continue
+                               }
+                               // Entry is a directory.
+                               // No .git or other dot nonsense please.
+                               if strings.HasPrefix(name, ".") {
+                                       continue
+                               }
+                               // Remember this (fully qualified) directory for the next pass.
+                               next = append(next, filepath.Join(dir, name))
+                       }
+                       if hasGoFiles {
+                               // It's a candidate.
+                               d.scan <- dir
+                       }
                }
-       }
 
-       return false
+       }
 }

diff --git a/src/cmd/go/alldocs.go b/src/cmd/go/alldocs.go
index 82f848d2e237eea042979b8a4d78d65a80845187..ecacf6d3dca440fcd583100d5bfc00158230566a 100644 (file)
--- a/src/cmd/go/alldocs.go
+++ b/src/cmd/go/alldocs.go
@@ -236,8 +236,10 @@ The first item in this list matched by the argument is the one whose documentati
 is printed. (See the examples below.) However, if the argument starts with a capital
 letter it is assumed to identify a symbol or method in the current directory.
 
-For packages, the order of scanning is determined lexically, but the GOROOT tree
-is always scanned before GOPATH.
+For packages, the order of scanning is determined lexically in breadth-first order.
+That is, the package presented is the one that matches the search and is nearest
+the root and lexically first at its level of the hierarchy.  The GOROOT tree is
+always scanned in its entirety before GOPATH.
 
 If there is no package specified or matched, the package in the current
 directory is selected, so "go doc Foo" shows the documentation for symbol Foo in

diff --git a/src/cmd/go/doc.go b/src/cmd/go/doc.go
index bed763679d2cca76804dfeb0f6af7f0451a93c2b..9b8b8dfc2491b898f7f51987414bdaf5ef93f609 100644 (file)
--- a/src/cmd/go/doc.go
+++ b/src/cmd/go/doc.go
@@ -39,8 +39,10 @@ The first item in this list matched by the argument is the one whose documentati
 is printed. (See the examples below.) However, if the argument starts with a capital
 letter it is assumed to identify a symbol or method in the current directory.
 
-For packages, the order of scanning is determined lexically, but the GOROOT tree
-is always scanned before GOPATH.
+For packages, the order of scanning is determined lexically in breadth-first order.
+That is, the package presented is the one that matches the search and is nearest
+the root and lexically first at its level of the hierarchy.  The GOROOT tree is
+always scanned in its entirety before GOPATH.
 
 If there is no package specified or matched, the package in the current
 directory is selected, so "go doc Foo" shows the documentation for symbol Foo in