cmd/go/internal/modload: use a structured error for 'ambiguous import'

This consolidates the construction of 'ambiguous import' errors to a
single location, ensuring consistency, and lays the groundwork for
automatic resolution in the future.

While we're at it, change "found" to "found package" to try to make
the cause of the error clearer.

Updates #32128
Updates #27899

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

diff --git a/src/cmd/go/internal/modload/import.go b/src/cmd/go/internal/modload/import.go
index f0777089d447e69c92485bff5e3c44e87c7990f9..68e0b6504b3bb438b126c4843642c7ae870394a3 100644 (file)
--- a/src/cmd/go/internal/modload/import.go
+++ b/src/cmd/go/internal/modload/import.go
@@ -5,7 +5,6 @@
 package modload
 
 import (
-       "bytes"
        "errors"
        "fmt"
        "go/build"
@@ -52,6 +51,41 @@ func (e *ImportMissingError) Unwrap() error {
        return e.QueryErr
 }
 
+// An AmbiguousImportError indicates an import of a package found in multiple
+// modules in the build list, or found in both the main module and its vendor
+// directory.
+type AmbiguousImportError struct {
+       ImportPath string
+       Dirs       []string
+       Modules    []module.Version // Either empty or 1:1 with Dirs.
+}
+
+func (e *AmbiguousImportError) Error() string {
+       locType := "modules"
+       if len(e.Modules) == 0 {
+               locType = "directories"
+       }
+
+       var buf strings.Builder
+       fmt.Fprintf(&buf, "ambiguous import: found package %s in multiple %s:", e.ImportPath, locType)
+
+       for i, dir := range e.Dirs {
+               buf.WriteString("\n\t")
+               if i < len(e.Modules) {
+                       m := e.Modules[i]
+                       buf.WriteString(m.Path)
+                       if m.Version != "" {
+                               fmt.Fprintf(&buf, " %s", m.Version)
+                       }
+                       fmt.Fprintf(&buf, " (%s)", dir)
+               } else {
+                       buf.WriteString(dir)
+               }
+       }
+
+       return buf.String()
+}
+
 // Import finds the module and directory in the build list
 // containing the package with the given import path.
 // The answer must be unique: Import returns an error
@@ -96,7 +130,7 @@ func Import(path string) (m module.Version, dir string, err error) {
                mainDir, mainOK := dirInModule(path, targetPrefix, ModRoot(), true)
                vendorDir, vendorOK := dirInModule(path, "", filepath.Join(ModRoot(), "vendor"), false)
                if mainOK && vendorOK {
-                       return module.Version{}, "", fmt.Errorf("ambiguous import: found %s in multiple directories:\n\t%s\n\t%s", path, mainDir, vendorDir)
+                       return module.Version{}, "", &AmbiguousImportError{ImportPath: path, Dirs: []string{mainDir, vendorDir}}
                }
                // Prefer to return main directory if there is one,
                // Note that we're not checking that the package exists.
@@ -136,16 +170,7 @@ func Import(path string) (m module.Version, dir string, err error) {
                return mods[0], dirs[0], nil
        }
        if len(mods) > 0 {
-               var buf bytes.Buffer
-               fmt.Fprintf(&buf, "ambiguous import: found %s in multiple modules:", path)
-               for i, m := range mods {
-                       fmt.Fprintf(&buf, "\n\t%s", m.Path)
-                       if m.Version != "" {
-                               fmt.Fprintf(&buf, " %s", m.Version)
-                       }
-                       fmt.Fprintf(&buf, " (%s)", dirs[i])
-               }
-               return module.Version{}, "", errors.New(buf.String())
+               return module.Version{}, "", &AmbiguousImportError{ImportPath: path, Dirs: dirs, Modules: mods}
        }
 
        // Look up module containing the package, for addition to the build list.

diff --git a/src/cmd/go/testdata/script/mod_ambiguous_import.txt b/src/cmd/go/testdata/script/mod_ambiguous_import.txt
new file mode 100644 (file)
index 0000000..9f9669c
--- /dev/null
+++ b/src/cmd/go/testdata/script/mod_ambiguous_import.txt
@@ -0,0 +1,49 @@
+env GO111MODULE=on
+
+cd $WORK
+
+# An import provided by two different modules should be flagged as an error.
+! go build ./importx
+stderr '^importx[/\\]importx.go:2:8: ambiguous import: found package example.com/a/x in multiple modules:\n\texample.com/a v0.1.0 \('$WORK'[/\\]a[/\\]x\)\n\texample.com/a/x v0.1.0 \('$WORK'[/\\]ax\)$'
+
+# However, it should not be an error if that import is unused.
+go build ./importy
+
+# An import provided by both the main module and the vendor directory
+# should be flagged as an error only when -mod=vendor is set.
+# TODO: This error message is a bit redundant.
+mkdir vendor/example.com/m/importy
+cp $WORK/importy/importy.go vendor/example.com/m/importy/importy.go
+go build example.com/m/importy
+! go build -mod=vendor example.com/m/importy
+stderr '^can.t load package: package example.com/m/importy: ambiguous import: found package example.com/m/importy in multiple directories:\n\t'$WORK'[/\\]importy\n\t'$WORK'[/\\]vendor[/\\]example.com[/\\]m[/\\]importy$'
+
+-- $WORK/go.mod --
+module example.com/m
+go 1.14
+require (
+       example.com/a v0.1.0
+       example.com/a/x v0.1.0
+)
+replace (
+       example.com/a v0.1.0 => ./a
+       example.com/a/x v0.1.0 => ./ax
+)
+-- $WORK/importx/importx.go --
+package importx
+import _ "example.com/a/x"
+-- $WORK/importy/importy.go --
+package importy
+import _ "example.com/a/y"
+-- $WORK/a/go.mod --
+module example.com/a
+go 1.14
+-- $WORK/a/x/x.go --
+package x
+-- $WORK/a/y/y.go --
+package y
+-- $WORK/ax/go.mod --
+module example.com/a/x
+go 1.14
+-- $WORK/ax/x.go --
+package x