cmd/go: do not skip dirs with syntax errors in wildcard matching (lik…

…e ./...) Fixes golang#11407. Change-Id: If35a8e04a3abf8acf955250c909dde57131b6bb8 Reviewed-on: https://go-review.googlesource.com/17971 Reviewed-by: Ian Lance Taylor <[email protected]>
rohythakur · Dec 18, 2015 · ab74f59 · ab74f59
1 parent d446ba9
commit ab74f59
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 19 deletions.
diff --git a/src/cmd/go/go_test.go b/src/cmd/go/go_test.go
@@ -763,6 +763,18 @@ func TestGoInstallDetectsRemovedFiles(t *testing.T) {
 	tg.wantStale("mypkg", "./testgo list mypkg claims mypkg is NOT stale after removing y.go; should be stale")
 }
 
+func TestWildcardMatchesSyntaxErrorDirs(t *testing.T) {
+	tg := testgo(t)
+	defer tg.cleanup()
+	tg.tempFile("src/mypkg/x.go", `package mypkg`)
+	tg.tempFile("src/mypkg/y.go", `pkg mypackage`)
+	tg.setenv("GOPATH", tg.path("."))
+	tg.cd(tg.path("src/mypkg"))
+	tg.runFail("list", "./...")
+	tg.runFail("build", "./...")
+	tg.runFail("install", "./...")
+}
+
 func TestGoListWithTags(t *testing.T) {
 	tg := testgo(t)
 	defer tg.cleanup()

diff --git a/src/cmd/go/main.go b/src/cmd/go/main.go
@@ -674,7 +674,14 @@ func matchPackagesInFS(pattern string) []string {
 		if !match(name) {
 			return nil
 		}
-		if _, err = buildContext.ImportDir(path, 0); err != nil {
+
+		// We keep the directory if we can import it, or if we can't import it
+		// due to invalid Go source files. This means that directories containing
+		// parse errors will be built (and fail) instead of being silently skipped
+		// as not matching the pattern. Go 1.5 and earlier skipped, but that
+		// behavior means people miss serious mistakes.
+		// See golang.org/issue/11407.
+		if p, err := buildContext.ImportDir(path, 0); err != nil && (p == nil || len(p.InvalidGoFiles) == 0) {
 			if _, noGo := err.(*build.NoGoError); !noGo {
 				log.Print(err)
 			}

diff --git a/src/go/build/build.go b/src/go/build/build.go
@@ -379,6 +379,7 @@ type Package struct {
 	GoFiles        []string // .go source files (excluding CgoFiles, TestGoFiles, XTestGoFiles)
 	CgoFiles       []string // .go source files that import "C"
 	IgnoredGoFiles []string // .go source files ignored for this build
+	InvalidGoFiles []string // .go source files with detected problems (parse error, wrong package name, and so on)
 	CFiles         []string // .c source files
 	CXXFiles       []string // .cc, .cpp and .cxx source files
 	MFiles         []string // .m (Objective-C) source files
@@ -679,6 +680,7 @@ Found:
 		return p, err
 	}
 
+	var badGoError error
 	var Sfiles []string // files with ".S" (capital S)
 	var firstFile, firstCommentFile string
 	imported := make(map[string][]token.Position)
@@ -694,9 +696,17 @@ Found:
 		name := d.Name()
 		ext := nameExt(name)
 
+		badFile := func(err error) {
+			if badGoError == nil {
+				badGoError = err
+			}
+			p.InvalidGoFiles = append(p.InvalidGoFiles, name)
+		}
+
 		match, data, filename, err := ctxt.matchFile(p.Dir, name, true, allTags)
 		if err != nil {
-			return p, err
+			badFile(err)
+			continue
 		}
 		if !match {
 			if ext == ".go" {
@@ -741,7 +751,8 @@ Found:
 
 		pf, err := parser.ParseFile(fset, filename, data, parser.ImportsOnly|parser.ParseComments)
 		if err != nil {
-			return p, err
+			badFile(err)
+			continue
 		}
 
 		pkg := pf.Name.Name
@@ -761,11 +772,12 @@ Found:
 			p.Name = pkg
 			firstFile = name
 		} else if pkg != p.Name {
-			return p, &MultiplePackageError{
+			badFile(&MultiplePackageError{
 				Dir:      p.Dir,
 				Packages: []string{p.Name, pkg},
 				Files:    []string{firstFile, name},
-			}
+			})
+			p.InvalidGoFiles = append(p.InvalidGoFiles, name)
 		}
 		if pf.Doc != nil && p.Doc == "" {
 			p.Doc = doc.Synopsis(pf.Doc.Text())
@@ -776,13 +788,12 @@ Found:
 			if line != 0 {
 				com, err := strconv.Unquote(qcom)
 				if err != nil {
-					return p, fmt.Errorf("%s:%d: cannot parse import comment", filename, line)
-				}
-				if p.ImportComment == "" {
+					badFile(fmt.Errorf("%s:%d: cannot parse import comment", filename, line))
+				} else if p.ImportComment == "" {
 					p.ImportComment = com
 					firstCommentFile = name
 				} else if p.ImportComment != com {
-					return p, fmt.Errorf("found import comments %q (%s) and %q (%s) in %s", p.ImportComment, firstCommentFile, com, name, p.Dir)
+					badFile(fmt.Errorf("found import comments %q (%s) and %q (%s) in %s", p.ImportComment, firstCommentFile, com, name, p.Dir))
 				}
 			}
 		}
@@ -813,18 +824,19 @@ Found:
 				}
 				if path == "C" {
 					if isTest {
-						return p, fmt.Errorf("use of cgo in test %s not supported", filename)
-					}
-					cg := spec.Doc
-					if cg == nil && len(d.Specs) == 1 {
-						cg = d.Doc
-					}
-					if cg != nil {
-						if err := ctxt.saveCgo(filename, p, cg); err != nil {
-							return p, err
+						badFile(fmt.Errorf("use of cgo in test %s not supported", filename))
+					} else {
+						cg := spec.Doc
+						if cg == nil && len(d.Specs) == 1 {
+							cg = d.Doc
 						}
+						if cg != nil {
+							if err := ctxt.saveCgo(filename, p, cg); err != nil {
+								badFile(err)
+							}
+						}
+						isCgo = true
 					}
-					isCgo = true
 				}
 			}
 		}
@@ -843,6 +855,9 @@ Found:
 			p.GoFiles = append(p.GoFiles, name)
 		}
 	}
+	if badGoError != nil {
+		return p, badGoError
+	}
 	if len(p.GoFiles)+len(p.CgoFiles)+len(p.TestGoFiles)+len(p.XTestGoFiles) == 0 {
 		return p, &NoGoError{p.Dir}
 	}