cmd/go: attribute direct imports from indirect dependencies to the importing package

For #36460
Updates #40775

Change-Id: I833ee8ee733151aafcff508bf91d0e6e37c50032
Reviewed-on: https://go-review.googlesource.com/c/go/+/303869
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>

src/cmd/go/internal/load/pkg.go

@@ -29,6 +29,7 @@ import (
 	"cmd/go/internal/base"
 	"cmd/go/internal/cfg"
 	"cmd/go/internal/fsys"
+	"cmd/go/internal/imports"
 	"cmd/go/internal/modinfo"
 	"cmd/go/internal/modload"
 	"cmd/go/internal/par"
@@ -478,6 +479,7 @@ var (
 	_ ImportPathError = (*importError)(nil)
 	_ ImportPathError = (*modload.ImportMissingError)(nil)
 	_ ImportPathError = (*modload.ImportMissingSumError)(nil)
+	_ ImportPathError = (*modload.DirectImportFromImplicitDependencyError)(nil)
 )
 
 type importError struct {
@@ -675,6 +677,8 @@ func loadImport(ctx context.Context, pre *preload, path, srcDir string, parent *
 			stk.Push(path)
 			defer stk.Pop()
 		}
+		// TODO(bcmills): Why are we constructing Error inline here instead of
+		// calling setLoadPackageDataError?
 		return &Package{
 			PackagePublic: PackagePublic{
 				ImportPath: path,
@@ -840,6 +844,27 @@ func loadPackageData(ctx context.Context, path, parentPath, parentDir, parentRoo
 					data.p.Root = info.Dir
 				}
 			}
+			if r.err != nil {
+				if data.err != nil {
+					// ImportDir gave us one error, and the module loader gave us another.
+					// We arbitrarily choose to keep the error from ImportDir because
+					// that's what our tests already expect, and it seems to provide a bit
+					// more detail in most cases.
+				} else if errors.Is(r.err, imports.ErrNoGo) {
+					// ImportDir said there were files in the package, but the module
+					// loader said there weren't. Which one is right?
+					// Without this special-case hack, the TestScript/test_vet case fails
+					// on the vetfail/p1 package (added in CL 83955).
+					// Apparently, imports.ShouldBuild biases toward rejecting files
+					// with invalid build constraints, whereas ImportDir biases toward
+					// accepting them.
+					//
+					// TODO(#41410: Figure out how this actually ought to work and fix
+					// this mess.
+				} else {
+					data.err = r.err
+				}
+			}
 		} else if r.err != nil {
 			data.p = new(build.Package)
 			data.err = r.err

src/cmd/go/internal/modload/import.go

@@ -128,6 +128,23 @@ func (e *AmbiguousImportError) Error() string {
 	return buf.String()
 }
 
+// A DirectImportFromImplicitDependencyError indicates a package directly
+// imported by a package or test in the main module that is satisfied by a
+// dependency that is not explicit in the main module's go.mod file.
+type DirectImportFromImplicitDependencyError struct {
+	ImporterPath string
+	ImportedPath string
+	Module       module.Version
+}
+
+func (e *DirectImportFromImplicitDependencyError) Error() string {
+	return fmt.Sprintf("package %s imports %s from implicitly required module; to add missing requirements, run:\n\tgo get %s@%s", e.ImporterPath, e.ImportedPath, e.Module.Path, e.Module.Version)
+}
+
+func (e *DirectImportFromImplicitDependencyError) ImportPath() string {
+	return e.ImporterPath
+}
+
 // ImportMissingSumError is reported in readonly mode when we need to check
 // if a module contains a package, but we don't have a sum for its .zip file.
 // We might need sums for multiple modules to verify the package is unique.

src/cmd/go/internal/modload/load.go

@@ -940,32 +940,51 @@ func loadFromRoots(ctx context.Context, params loaderParams) *loader {
 	}
 	base.ExitIfErrors() // TODO(bcmills): Is this actually needed?
 
+	rs := ld.requirements
+
 	// Compute directly referenced dependency modules.
 	direct := make(map[string]bool)
 	for _, pkg := range ld.pkgs {
-		if pkg.mod == Target {
+		if pkg.mod != Target {
-			for _, dep := range pkg.imports {
+			continue
-				if dep.mod.Path != "" && dep.mod.Path != Target.Path && index != nil {
+		}
-					_, explicit := index.require[dep.mod]
+		for _, dep := range pkg.imports {
-					if allowWriteGoMod && cfg.BuildMod == "readonly" && !explicit {
+			if dep.mod.Path == "" || dep.mod.Path == Target.Path {
-						// TODO(#40775): attach error to package instead of using
+				continue
-						// base.Errorf. Ideally, 'go list' should not fail because of this,
+			}
-						// but today, LoadPackages calls WriteGoMod unconditionally, which
+
-						// would fail with a less clear message.
+			if pkg.err == nil && cfg.BuildMod != "mod" {
-						base.Errorf("go: %[1]s: package %[2]s imported from implicitly required module; to add missing requirements, run:\n\tgo get %[2]s@%[3]s", pkg.path, dep.path, dep.mod.Version)
+				if v, ok := rs.rootSelected(dep.mod.Path); !ok || v != dep.mod.Version {
+					// dep.mod is not an explicit dependency, but needs to be.
+					// Because we are not in "mod" mod, we will not be able to update it.
+					// Instead, mark the importing package with an error.
+					//
+					// TODO(#41688): The resulting error message fails to include the file
+					// position of the erroneous import (because that information is not
+					// tracked by the module loader). Figure out how to plumb the import
+					// position through.
+					pkg.err = &DirectImportFromImplicitDependencyError{
+						ImporterPath: pkg.path,
+						ImportedPath: dep.path,
+						Module:       dep.mod,
 					}
-					direct[dep.mod.Path] = true
+					// cfg.BuildMod does not allow us to change dep.mod to be a direct
+					// dependency, so don't mark it as such.
+					continue
 				}
 			}
+
+			// dep is a package directly imported by a package or test in the main
+			// module and loaded from some other module (not the standard library).
+			// Mark its module as a direct dependency.
+			direct[dep.mod.Path] = true
 		}
 	}
-	base.ExitIfErrors()
 
 	// If we didn't scan all of the imports from the main module, or didn't use
 	// imports.AnyTags, then we didn't necessarily load every package that
 	// contributes “direct” imports — so we can't safely mark existing
 	// direct dependencies in ld.requirements as indirect-only. Propagate them as direct.
-	rs := ld.requirements
 	if !ld.loadedDirect() {
 		for mPath := range rs.direct {
 			direct[mPath] = true

src/cmd/go/testdata/script/mod_get_promote_implicit.txt

@@ -6,7 +6,7 @@ cp go.mod.orig go.mod
 go list -m indirect-with-pkg
 stdout '^indirect-with-pkg v1.0.0 => ./indirect-with-pkg$'
 ! go list ./use-indirect
-stderr '^go: m/use-indirect: package indirect-with-pkg imported from implicitly required module; to add missing requirements, run:\n\tgo get indirect-with-pkg@v1.0.0$'
+stderr '^package m/use-indirect imports indirect-with-pkg from implicitly required module; to add missing requirements, run:\n\tgo get indirect-with-pkg@v1.0.0$'
 
 # We can promote the implicit requirement by getting the importing package.
 # NOTE: the hint recommends getting the imported package (tested below) since