internal/imports: use a clean GOMODCACHE for the scan root directory

The directories processed by gopathwalk are clean, yet in the scan
callback are assumed to have the root as a prefix. For the module cache,
this root was previous not guaranteed to be clean, if it came from a
GOMODCACHE environment variable. As a result, the computed relative path
may be inaccurate, and may even panic if the unclean root is much longer
than its clean form.

Reproduce the crash of golang/go#67156 in a test, update
scanDirForPackage to be more robust, and fix the uncleanliness of the
module cache root.

Also fix some handling of GOMODCACHE cleanup in imports tests.

Fixes golang/go#67156

Change-Id: Ia899256fed9629b7e753a52feb02b4235bfc8388
Reviewed-on: https://go-review.googlesource.com/c/tools/+/603635
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Alan Donovan <[email protected]>

Author: findleyr

gopls/internal/test/integration/misc/imports_test.go

@@ -6,6 +6,7 @@ package misc
 
 import (
 	"os"
+	"os/exec"
 	"path/filepath"
 	"strings"
 	"testing"
@@ -195,8 +196,7 @@ func main() {
 	})
 }
 
-func TestGOMODCACHE(t *testing.T) {
-	const proxy = `
+const exampleProxy = `
 -- [email protected]/go.mod --
 module example.com
 
@@ -210,31 +210,29 @@ package y
 
 const Y = 2
 `
+
+func TestGOMODCACHE(t *testing.T) {
 	const files = `
 -- go.mod --
 module mod.com
 
 go 1.12
 
 require example.com v1.2.3
--- go.sum --
-example.com v1.2.3 h1:6vTQqzX+pnwngZF1+5gcO3ZEWmix1jJ/h+pWS8wUxK0=
-example.com v1.2.3/go.mod h1:Y2Rc5rVWjWur0h3pd9aEvK5Pof8YKDANh9gHA2Maujo=
 -- main.go --
 package main
 
 import "example.com/x"
 
 var _, _ = x.X, y.Y
 `
-	modcache, err := os.MkdirTemp("", "TestGOMODCACHE-modcache")
-	if err != nil {
-		t.Fatal(err)
-	}
-	defer os.RemoveAll(modcache)
+	modcache := t.TempDir()
+	defer cleanModCache(t, modcache) // see doc comment of cleanModCache
+
 	WithOptions(
 		EnvVars{"GOMODCACHE": modcache},
-		ProxyFiles(proxy),
+		ProxyFiles(exampleProxy),
+		WriteGoSum("."),
 	).Run(t, files, func(t *testing.T, env *Env) {
 		env.OpenFile("main.go")
 		env.AfterChange(Diagnostics(env.AtRegexp("main.go", `y.Y`)))
@@ -248,6 +246,58 @@ var _, _ = x.X, y.Y
 	})
 }
 
+func TestRelativeReplace(t *testing.T) {
+	const files = `
+-- go.mod --
+module mod.com/a
+
+go 1.20
+
+require (
+	example.com   v1.2.3
+)
+
+replace example.com/b => ../b
+-- main.go --
+package main
+
+import "example.com/x"
+
+var _, _ = x.X, y.Y
+`
+	modcache := t.TempDir()
+	base := filepath.Base(modcache)
+	defer cleanModCache(t, modcache) // see doc comment of cleanModCache
+
+	// Construct a very unclean module cache whose length exceeds the length of
+	// the clean directory path, to reproduce the crash in golang/go#67156
+	const sep = string(filepath.Separator)
+	modcache += strings.Repeat(sep+".."+sep+base, 10)
+
+	WithOptions(
+		EnvVars{"GOMODCACHE": modcache},
+		ProxyFiles(exampleProxy),
+		WriteGoSum("."),
+	).Run(t, files, func(t *testing.T, env *Env) {
+		env.OpenFile("main.go")
+		env.AfterChange(Diagnostics(env.AtRegexp("main.go", `y.Y`)))
+		env.SaveBuffer("main.go")
+		env.AfterChange(NoDiagnostics(ForFile("main.go")))
+	})
+}
+
+// TODO(rfindley): this is only necessary as the module cache cleaning of the
+// sandbox does not respect GOMODCACHE set via EnvVars. We should fix this, but
+// that is probably part of a larger refactoring of the sandbox that I'm not
+// inclined to undertake.
+func cleanModCache(t *testing.T, modcache string) {
+	cmd := exec.Command("go", "clean", "-modcache")
+	cmd.Env = append(os.Environ(), "GOMODCACHE="+modcache)
+	if err := cmd.Run(); err != nil {
+		t.Errorf("cleaning modcache: %v", err)
+	}
+}
+
 // Tests golang/go#40685.
 func TestAcceptImportsQuickFixTestVariant(t *testing.T) {
 	const pkg = `

internal/imports/mod.go

@@ -245,7 +245,10 @@ func newModuleResolver(e *ProcessEnv, moduleCacheCache *DirInfoCache) (*ModuleRe
 //  2. Use this to separate module cache scanning from other scanning.
 func gomodcacheForEnv(goenv map[string]string) string {
 	if gmc := goenv["GOMODCACHE"]; gmc != "" {
-		return gmc
+		// golang/go#67156: ensure that the module cache is clean, since it is
+		// assumed as a prefix to directories scanned by gopathwalk, which are
+		// themselves clean.
+		return filepath.Clean(gmc)
 	}
 	gopaths := filepath.SplitList(goenv["GOPATH"])
 	if len(gopaths) == 0 {
@@ -740,8 +743,8 @@ func (r *ModuleResolver) loadExports(ctx context.Context, pkg *pkg, includeTest
 
 func (r *ModuleResolver) scanDirForPackage(root gopathwalk.Root, dir string) directoryPackageInfo {
 	subdir := ""
-	if dir != root.Path {
-		subdir = dir[len(root.Path)+len("/"):]
+	if prefix := root.Path + string(filepath.Separator); strings.HasPrefix(dir, prefix) {
+		subdir = dir[len(prefix):]
 	}
 	importPath := filepath.ToSlash(subdir)
 	if strings.HasPrefix(importPath, "vendor/") {