From d6c2f1e90ed2eb25ca5b00fef9a4d13b01a4a1c5 Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Wed, 18 Sep 2019 16:35:00 -0400 Subject: [PATCH] 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 TryBot-Result: Gobot Gobot Reviewed-by: Jay Conrod --- src/cmd/go/internal/modload/import.go | 49 ++++++++++++++----- .../testdata/script/mod_ambiguous_import.txt | 49 +++++++++++++++++++ 2 files changed, 86 insertions(+), 12 deletions(-) create mode 100644 src/cmd/go/testdata/script/mod_ambiguous_import.txt diff --git a/src/cmd/go/internal/modload/import.go b/src/cmd/go/internal/modload/import.go index f0777089d447e..68e0b6504b3bb 100644 --- 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 index 0000000000000..9f9669c762ad9 --- /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