[gopls-release-branch.0.15] gopls/internal/cache: don't create Views for vendored modules

We should not create Views for vendored modules, just as we don't create
Views for modules in the module cache. With this change, gopls behaves
similarly to [email protected] when navigating around the Kubernetes repo.

Also add some test coverage that vendored packages are not workspace
packages.

Fixes golang/go#65830

Change-Id: If9883dc9616774952bd49c395e1c0d37ad3c2a6a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/565458
Reviewed-by: Alan Donovan <[email protected]>
Auto-Submit: Robert Findley <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
(cherry picked from commit a821e61)
Reviewed-on: https://go-review.googlesource.com/c/tools/+/565695

Author: findleyr

gopls/internal/cache/session.go

@@ -470,7 +470,17 @@ func selectViewDefs(ctx context.Context, fs file.Source, folders []*Folder, open
 	folderForFile := func(uri protocol.DocumentURI) *Folder {
 		var longest *Folder
 		for _, folder := range folders {
-			if (longest == nil || len(folder.Dir) > len(longest.Dir)) && folder.Dir.Encloses(uri) {
+			// Check that this is a better match than longest, but not through a
+			// vendor directory. Count occurrences of "/vendor/" as a quick check
+			// that the vendor directory is between the folder and the file. Note the
+			// addition of a trailing "/" to handle the odd case where the folder is named
+			// vendor (which I hope is exceedingly rare in any case).
+			//
+			// Vendored packages are, by definition, part of an existing view.
+			if (longest == nil || len(folder.Dir) > len(longest.Dir)) &&
+				folder.Dir.Encloses(uri) &&
+				strings.Count(string(uri), "/vendor/") == strings.Count(string(folder.Dir)+"/", "/vendor/") {
+
 				longest = folder
 			}
 		}

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

@@ -495,9 +495,7 @@ const _ = b.K
 		}
 
 		// Run 'go mod vendor' outside the editor.
-		if err := env.Sandbox.RunGoCommand(env.Ctx, ".", "mod", []string{"vendor"}, nil, true); err != nil {
-			t.Fatalf("go mod vendor: %v", err)
-		}
+		env.RunGoCommand("mod", "vendor")
 
 		// Synchronize changes to watched files.
 		env.Await(env.DoneWithChangeWatchedFiles())

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

@@ -360,8 +360,6 @@ func _() {
 // implementations in vendored modules were not found. The actual fix
 // was the same as for #55995; see TestVendoringInvalidatesMetadata.
 func TestImplementationsInVendor(t *testing.T) {
-	t.Skip("golang/go#56169: file watching does not capture vendor dirs")
-
 	const proxy = `
 -- other.com/[email protected]/go.mod --
 module other.com/b
@@ -415,9 +413,7 @@ var _ b.B
 		checkVendor(env.Implementations(refLoc), false)
 
 		// Run 'go mod vendor' outside the editor.
-		if err := env.Sandbox.RunGoCommand(env.Ctx, ".", "mod", []string{"vendor"}, nil, true); err != nil {
-			t.Fatalf("go mod vendor: %v", err)
-		}
+		env.RunGoCommand("mod", "vendor")
 
 		// Synchronize changes to watched files.
 		env.Await(env.DoneWithChangeWatchedFiles())

gopls/internal/test/integration/workspace/vendor_test.go

@@ -0,0 +1,67 @@
+// Copyright 2024 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package workspace
+
+import (
+	"testing"
+
+	. "golang.org/x/tools/gopls/internal/test/integration"
+)
+
+func TestWorkspacePackagesExcludesVendor(t *testing.T) {
+	// This test verifies that packages in the vendor directory are not workspace
+	// packages. This would be an easy mistake for gopls to make, since mod
+	// vendoring excludes go.mod files, and therefore the nearest go.mod file for
+	// vendored packages is often the workspace mod file.
+	const proxy = `
+-- other.com/[email protected]/go.mod --
+module other.com/b
+
+go 1.18
+
+-- other.com/[email protected]/b.go --
+package b
+
+type B int
+
+func _() {
+	var V int // unused
+}
+`
+	const src = `
+-- go.mod --
+module example.com/a
+go 1.14
+require other.com/b v1.0.0
+
+-- go.sum --
+other.com/b v1.0.0 h1:ct1+0RPozzMvA2rSYnVvIfr/GDHcd7oVnw147okdi3g=
+other.com/b v1.0.0/go.mod h1:bfTSZo/4ZtAQJWBYScopwW6n9Ctfsl2mi8nXsqjDXR8=
+
+-- a.go --
+package a
+
+import "other.com/b"
+
+var _ b.B
+
+`
+	WithOptions(
+		ProxyFiles(proxy),
+		Modes(Default),
+	).Run(t, src, func(t *testing.T, env *Env) {
+		env.RunGoCommand("mod", "vendor")
+		// Uncomment for updated go.sum contents.
+		// env.DumpGoSum(".")
+		env.OpenFile("a.go")
+		env.AfterChange(
+			NoDiagnostics(), // as b is not a workspace package
+		)
+		env.GoToDefinition(env.RegexpSearch("a.go", `b\.(B)`))
+		env.AfterChange(
+			Diagnostics(env.AtRegexp("vendor/other.com/b/b.go", "V"), WithMessage("not used")),
+		)
+	})
+}

gopls/internal/test/integration/workspace/zero_config_test.go

@@ -5,6 +5,7 @@
 package workspace
 
 import (
+	"strings"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -268,3 +269,58 @@ package b
 		}
 	})
 }
+
+func TestVendorExcluded(t *testing.T) {
+	// Test that we don't create Views for vendored modules.
+	//
+	// We construct the vendor directory manually here, as `go mod vendor` will
+	// omit the go.mod file. This synthesizes the setup of Kubernetes, where the
+	// entire module is vendored through a symlinked directory.
+	const src = `
+-- go.mod --
+module example.com/a
+
+go 1.18
+
+require other.com/b v1.0.0
+
+-- a.go --
+package a
+import "other.com/b"
+var _ b.B
+
+-- vendor/modules.txt --
+# other.com/b v1.0.0
+## explicit; go 1.14
+other.com/b
+
+-- vendor/other.com/b/go.mod --
+module other.com/b
+go 1.14
+
+-- vendor/other.com/b/b.go --
+package b
+type B int
+
+func _() {
+	var V int // unused
+}
+`
+	WithOptions(
+		Modes(Default),
+	).Run(t, src, func(t *testing.T, env *Env) {
+		env.OpenFile("a.go")
+		env.AfterChange(NoDiagnostics())
+		loc := env.GoToDefinition(env.RegexpSearch("a.go", `b\.(B)`))
+		if !strings.Contains(string(loc.URI), "/vendor/") {
+			t.Fatalf("Definition(b.B) = %v, want vendored location", loc.URI)
+		}
+		env.AfterChange(
+			Diagnostics(env.AtRegexp("vendor/other.com/b/b.go", "V"), WithMessage("not used")),
+		)
+
+		if views := env.Views(); len(views) != 1 {
+			t.Errorf("After opening /vendor/, got %d views, want 1. Views:\n%v", len(views), views)
+		}
+	})
+}