gopls/internal/golang/completion: don't offer internal std packages

This CL ensures completions should not offer symbols from unimported
packages that cannot be imported because they are "internal".

Fixes golang/go#44890
Fixes golang/go#74611

Change-Id: Iae6cdd67518274b8c838c215ec12ef74e3a66c0d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/701635
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Alan Donovan <[email protected]>
Commit-Queue: Alan Donovan <[email protected]>
Reviewed-by: Robert Findley <[email protected]>
Auto-Submit: Alan Donovan <[email protected]>

Author: xieyuschen

gopls/internal/golang/completion/deep_completion.go

@@ -11,6 +11,7 @@ import (
 	"time"
 
 	"golang.org/x/tools/gopls/internal/util/typesutil"
+	"golang.org/x/tools/internal/analysisinternal"
 )
 
 // MaxDeepCompletions limits deep completion results because in most cases
@@ -161,6 +162,10 @@ func (c *completer) deepSearch(ctx context.Context, minDepth int, deadline *time
 				continue
 			}
 
+			if cand.imp != nil && !analysisinternal.CanImport(string(c.pkg.Metadata().PkgPath), cand.imp.importPath) {
+				continue // inaccessible internal package
+			}
+
 			// If we want a type name, don't offer non-type name candidates.
 			// However, do offer package names since they can contain type names,
 			// and do offer any candidate without a type since we aren't sure if it

gopls/internal/test/integration/completion/completion_test.go

@@ -300,6 +300,49 @@ func compareCompletionLabels(want []string, gotItems []protocol.CompletionItem)
 	return ""
 }
 
+func TestIssue74611(t *testing.T) {
+	// Completions should not offer symbols from unimported packages
+	// that cannot be imported because they are "internal".
+	//
+	// Ideally, we should test std case as well but because mocking
+	// a fake stdlib is hard, we just test the 3rd user case.
+	// std test is done interactively.
+	const files = `-- go.mod --
+module mod.com/cmd
+
+go 1.21
+
+-- a.go --
+package pkg
+import "fmt"
+
+func main() {
+	fmt.Println("call Println to use fmt")
+	_ = maps
+} // (completion requested at start of line)
+`
+
+	WithOptions().Run(t, files, func(t *testing.T, env *Env) {
+		filename := "a.go"
+		// Trigger unimported completions for the maps package.
+		env.OpenFile(filename)
+		env.Await(env.DoneWithOpen())
+		loc := env.RegexpSearch(filename, "\n}")
+		completions := env.Completion(loc)
+		if len(completions.Items) == 0 {
+			t.Fatalf("no completion items")
+		}
+		runtimeMaps := `"internal/runtime/maps"`
+		found := slices.ContainsFunc(completions.Items, func(item protocol.CompletionItem) bool {
+			return item.Detail == runtimeMaps
+		})
+
+		if found {
+			t.Fatalf("unwanted completion: %s", runtimeMaps)
+		}
+	})
+}
+
 func TestUnimportedCompletion(t *testing.T) {
 	const mod = `
 -- go.mod --

internal/analysisinternal/analysis_test.go

@@ -22,6 +22,8 @@ func TestCanImport(t *testing.T) {
 	}{
 		{"fmt", "internal", true},
 		{"fmt", "internal/foo", true},
+		{"fmt", "fmt/internal/foo", true},
+		{"fmt", "cmd/internal/archive", false},
 		{"a.com/b", "internal", false},
 		{"a.com/b", "xinternal", true},
 		{"a.com/b", "internal/foo", false},