cmd/go/internal/vet: print line numbers appropriately on list errors

Fixes #36173

For reasons that are unclear to me, this commit:
f1d5ce0
introduces a TestPackagesFor function that strips line numbers from error
messages. This commit introduces a new version of that function for 'go vet'
that always keeps the line numbers.

Author: nicks

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

@@ -1300,8 +1300,15 @@ func disallowInternal(srcDir string, importer *Package, importerPath string, p *
 
 	// Internal is present, and srcDir is outside parent's tree. Not allowed.
 	perr := *p
+
+	// The stack in the error should point to the importing package,
+	// so pop off the last stack frame.
+	importStack := stk.Copy()
+	if len(importStack) > 0 {
+		importStack = importStack[:len(importStack)-1]
+	}
 	perr.Error = &PackageError{
-		ImportStack: stk.Copy(),
+		ImportStack: importStack,
 		Err:         ImportErrorf(p.ImportPath, "use of internal package "+p.ImportPath+" not allowed"),
 	}
 	perr.Incomplete = true

src/cmd/go/internal/load/test.go

@@ -55,7 +55,19 @@ func TestPackagesFor(p *Package, cover *TestCover) (pmain, ptest, pxtest *Packag
 		}
 		if len(p1.DepsErrors) > 0 {
 			perr := p1.DepsErrors[0]
-			perr.Pos = "" // show full import stack
+			isDirect := false
+
+			// The package with the error has the highest index in the stack.
+			if len(perr.ImportStack) >= 1 {
+				importer := strings.TrimSuffix(perr.ImportStack[len(perr.ImportStack)-1], " (test)")
+				isDirect = importer == p1.ImportPath
+			}
+
+			if !isDirect {
+				// Remove the position information, so that we'll
+				// print the full import stack to the error.
+				perr.Pos = ""
+			}
 			err = perr
 			break
 		}

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

@@ -1,6 +1,9 @@
 ! go test testdep/p1
 stderr 'package testdep/p1 \(test\)\n\timports testdep/p2\n\timports testdep/p3: build constraints exclude all Go files ' # check for full import stack
 
+! go vet testdep/p1
+stderr 'package testdep/p1 \(test\)\n\timports testdep/p2\n\timports testdep/p3: build constraints exclude all Go files ' # check for full import stack
+
 -- testdep/p1/p1.go --
 package p1
 -- testdep/p1/p1_test.go --

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

@@ -0,0 +1,71 @@
+env GO111MODULE=off
+
+# Issue 36173. Verify that "go vet" prints line numbers on load errors.
+
+! go vet a/a.go
+stderr 'a.go:5:3: use of internal package'
+
+! go vet a/a_test.go
+stderr 'a_test.go:4:3: use of internal package'
+
+! go vet a
+stderr 'a.go:5:3: use of internal package'
+
+go vet b/b.go
+! stderr 'use of internal package'
+
+! go vet b/b_test.go
+stderr 'b_test.go:4:3: use of internal package'
+
+! go vet depends-on-a/depends-on-a.go
+stderr 'a.go:5:3: use of internal package'
+
+! go vet depends-on-a/depends-on-a_test.go
+stderr 'package command-line-arguments \(test\)\n\timports a: use of internal package a/x/internal/y not allowed'
+
+! go vet depends-on-a
+stderr 'a.go:5:3: use of internal package'
+
+-- a/a.go --
+// A package with bad imports in both src and test
+package a
+
+import (
+  _ "a/x/internal/y"
+)
+
+-- a/a_test.go --
+package a
+
+import (
+  _ "a/x/internal/y"
+)
+
+-- b/b.go --
+// A package with a bad import in test only
+package b
+
+-- b/b_test.go --
+package b
+
+import (
+  _ "a/x/internal/y"
+)
+
+-- depends-on-a/depends-on-a.go --
+// A package that depends on a package with a bad import
+package depends
+
+import (
+  _ "a"
+)
+
+-- depends-on-a/depends-on-a_test.go --
+package depends
+
+import (
+  _ "a"
+)
+
+-- a/x/internal/y/y.go --
+package y