go/build: avoid duplicates in InvalidGoFiles

For #45827
For #39986
Updates #45999

Change-Id: I0c919b6a2e56e7003b90425487eafe0f0eadc609
Reviewed-on: https://go-review.googlesource.com/c/go/+/317299
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>

diff --git a/src/go/build/build.go b/src/go/build/build.go
index 8d1a107c6ea713836407ae143607cf2f8a858348..b85fa96de1fb55985490fe1748ee8b18a33f1029 100644 (file)
--- a/src/go/build/build.go
+++ b/src/go/build/build.go
@@ -810,6 +810,17 @@ Found:
        }
 
        var badGoError error
+       badFiles := make(map[string]bool)
+       badFile := func(name string, err error) {
+               if badGoError == nil {
+                       badGoError = err
+               }
+               if !badFiles[name] {
+                       p.InvalidGoFiles = append(p.InvalidGoFiles, name)
+                       badFiles[name] = true
+               }
+       }
+
        var Sfiles []string // files with ".S"(capital S)/.sx(capital s equivalent for case insensitive filesystems)
        var firstFile, firstCommentFile string
        embedPos := make(map[string][]token.Position)
@@ -834,16 +845,9 @@ Found:
                name := d.Name()
                ext := nameExt(name)
 
-               badFile := func(err error) {
-                       if badGoError == nil {
-                               badGoError = err
-                       }
-                       p.InvalidGoFiles = append(p.InvalidGoFiles, name)
-               }
-
                info, err := ctxt.matchFile(p.Dir, name, allTags, &p.BinaryOnly, fset)
                if err != nil {
-                       badFile(err)
+                       badFile(name, err)
                        continue
                }
                if info == nil {
@@ -874,7 +878,7 @@ Found:
                }
 
                if info.parseErr != nil {
-                       badFile(info.parseErr)
+                       badFile(name, info.parseErr)
                        continue
                }
                pf := info.parsed
@@ -896,12 +900,14 @@ Found:
                        p.Name = pkg
                        firstFile = name
                } else if pkg != p.Name {
-                       badFile(&MultiplePackageError{
+                       // TODO(#45999): The choice of p.Name is arbitrary based on file iteration
+                       // order. Instead of resolving p.Name arbitrarily, we should clear out the
+                       // existing name and mark the existing files as also invalid.
+                       badFile(name, &MultiplePackageError{
                                Dir:      p.Dir,
                                Packages: []string{p.Name, pkg},
                                Files:    []string{firstFile, name},
                        })
-                       p.InvalidGoFiles = append(p.InvalidGoFiles, name)
                }
                // Grab the first package comment as docs, provided it is not from a test file.
                if pf.Doc != nil && p.Doc == "" && !isTest && !isXTest {
@@ -913,12 +919,12 @@ Found:
                        if line != 0 {
                                com, err := strconv.Unquote(qcom)
                                if err != nil {
-                                       badFile(fmt.Errorf("%s:%d: cannot parse import comment", filename, line))
+                                       badFile(name, fmt.Errorf("%s:%d: cannot parse import comment", filename, line))
                                } else if p.ImportComment == "" {
                                        p.ImportComment = com
                                        firstCommentFile = name
                                } else if p.ImportComment != com {
-                                       badFile(fmt.Errorf("found import comments %q (%s) and %q (%s) in %s", p.ImportComment, firstCommentFile, com, name, p.Dir))
+                                       badFile(name, fmt.Errorf("found import comments %q (%s) and %q (%s) in %s", p.ImportComment, firstCommentFile, com, name, p.Dir))
                                }
                        }
                }
@@ -928,13 +934,13 @@ Found:
                for _, imp := range info.imports {
                        if imp.path == "C" {
                                if isTest {
-                                       badFile(fmt.Errorf("use of cgo in test %s not supported", filename))
+                                       badFile(name, fmt.Errorf("use of cgo in test %s not supported", filename))
                                        continue
                                }
                                isCgo = true
                                if imp.doc != nil {
                                        if err := ctxt.saveCgo(filename, p, imp.doc); err != nil {
-                                               badFile(err)
+                                               badFile(name, err)
                                        }
                                }
                        }

diff --git a/src/go/build/build_test.go b/src/go/build/build_test.go
index 7e785680831868422b9c78b38afec1a634bf0686..80f930f3c23bef47516580717c8cb729273d93d1 100644 (file)
--- a/src/go/build/build_test.go
+++ b/src/go/build/build_test.go
@@ -104,7 +104,8 @@ func TestEmptyFolderImport(t *testing.T) {
 }
 
 func TestMultiplePackageImport(t *testing.T) {
-       _, err := Import(".", "testdata/multi", 0)
+       pkg, err := Import(".", "testdata/multi", 0)
+
        mpe, ok := err.(*MultiplePackageError)
        if !ok {
                t.Fatal(`Import("testdata/multi") did not return MultiplePackageError.`)
@@ -115,7 +116,20 @@ func TestMultiplePackageImport(t *testing.T) {
                Files:    []string{"file.go", "file_appengine.go"},
        }
        if !reflect.DeepEqual(mpe, want) {
-               t.Errorf("got %#v; want %#v", mpe, want)
+               t.Errorf("err = %#v; want %#v", mpe, want)
+       }
+
+       // TODO(#45999): Since the name is ambiguous, pkg.Name should be left empty.
+       if wantName := "main"; pkg.Name != wantName {
+               t.Errorf("pkg.Name = %q; want %q", pkg.Name, wantName)
+       }
+
+       if wantGoFiles := []string{"file.go", "file_appengine.go"}; !reflect.DeepEqual(pkg.GoFiles, wantGoFiles) {
+               t.Errorf("pkg.GoFiles = %q; want %q", pkg.GoFiles, wantGoFiles)
+       }
+
+       if wantInvalidFiles := []string{"file_appengine.go"}; !reflect.DeepEqual(pkg.InvalidGoFiles, wantInvalidFiles) {
+               t.Errorf("pkg.InvalidGoFiles = %q; want %q", pkg.InvalidGoFiles, wantInvalidFiles)
        }
 }