gopls/internal/lsp/source: reduce usage of TypecheckWorkspace

Fix calls to TypeCheck using TypecheckWorkspace where we really want
TypecheckFull. Also, use NarrowestPackage where it suffices.

In approximate order of appearance:
- Code actions, semantic tokens, code lens, and document highlights are
  all scoped to a file; the narrowest package for that file should
  suffice.
- When completing at a position, we need the full package to find
  enclosing context. Furthermore, that file is open, and so will be
  fully type-checked by other operations.
- Ditto for suggested fixes, inlay hints, and signature help.

The current behavior leads to incorrect or missing functionality when
outside the workspace. I did not add comprehensive tests demonstrating
this in all cases, but added one for signature help.

For golang/go#57987

Change-Id: I8270d0f0a0787e36bd4103378176d150426d37f2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/463375
Reviewed-by: Alan Donovan <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
gopls-CI: kokoro <[email protected]>
Run-TryBot: Robert Findley <[email protected]>

Author: findleyr

gopls/internal/lsp/code_action.go

@@ -158,7 +158,7 @@ func (s *Server) codeAction(ctx context.Context, params *protocol.CodeActionPara
 
 		// Type-check the package and also run analysis,
 		// then combine their diagnostics.
-		pkg, _, err := source.PackageForFile(ctx, snapshot, fh.URI(), source.TypecheckFull, source.WidestPackage)
+		pkg, _, err := source.PackageForFile(ctx, snapshot, fh.URI(), source.TypecheckFull, source.NarrowestPackage)
 		if err != nil {
 			return nil, err
 		}

gopls/internal/lsp/regtest/wrappers.go

@@ -131,6 +131,9 @@ func (e *Env) RegexpRange(name, re string) protocol.Range {
 // RegexpSearch returns the starting position of the first match for re in the
 // buffer specified by name, calling t.Fatal on any error. It first searches
 // for the position in open buffers, then in workspace files.
+//
+// TODO(rfindley): RegexpSearch should return a protocol.Location (but that is
+// a large change).
 func (e *Env) RegexpSearch(name, re string) protocol.Position {
 	e.T.Helper()
 	pos, err := e.Editor.RegexpSearch(name, re)

gopls/internal/lsp/semantic.go

@@ -93,7 +93,7 @@ func (s *Server) computeSemanticTokens(ctx context.Context, td protocol.TextDocu
 	if kind != source.Go {
 		return nil, nil
 	}
-	pkg, pgf, err := source.PackageForFile(ctx, snapshot, fh.URI(), source.TypecheckFull, source.WidestPackage)
+	pkg, pgf, err := source.PackageForFile(ctx, snapshot, fh.URI(), source.TypecheckFull, source.NarrowestPackage)
 	if err != nil {
 		return nil, err
 	}

gopls/internal/lsp/source/code_lens.go

@@ -100,7 +100,7 @@ func TestsAndBenchmarks(ctx context.Context, snapshot Snapshot, fh FileHandle) (
 	if !strings.HasSuffix(fh.URI().Filename(), "_test.go") {
 		return out, nil
 	}
-	pkg, pgf, err := PackageForFile(ctx, snapshot, fh.URI(), TypecheckWorkspace, WidestPackage)
+	pkg, pgf, err := PackageForFile(ctx, snapshot, fh.URI(), TypecheckFull, NarrowestPackage)
 	if err != nil {
 		return out, err
 	}

gopls/internal/lsp/source/completion/completion.go

@@ -430,7 +430,7 @@ func Completion(ctx context.Context, snapshot source.Snapshot, fh source.FileHan
 
 	startTime := time.Now()
 
-	pkg, pgf, err := source.PackageForFile(ctx, snapshot, fh.URI(), source.TypecheckWorkspace, source.NarrowestPackage)
+	pkg, pgf, err := source.PackageForFile(ctx, snapshot, fh.URI(), source.TypecheckFull, source.NarrowestPackage)
 	if err != nil || pgf.File.Package == token.NoPos {
 		// If we can't parse this file or find position for the package
 		// keyword, it may be missing a package declaration. Try offering

gopls/internal/lsp/source/fix.go

@@ -53,12 +53,16 @@ var suggestedFixes = map[string]SuggestedFixFunc{
 // singleFile calls analyzers that expect inputs for a single file
 func singleFile(sf singleFileFixFunc) SuggestedFixFunc {
 	return func(ctx context.Context, snapshot Snapshot, fh FileHandle, pRng protocol.Range) (*token.FileSet, *analysis.SuggestedFix, error) {
-		fset, rng, src, file, pkg, info, err := getAllSuggestedFixInputs(ctx, snapshot, fh, pRng)
+		pkg, pgf, err := PackageForFile(ctx, snapshot, fh.URI(), TypecheckFull, NarrowestPackage)
 		if err != nil {
 			return nil, nil, err
 		}
-		fix, err := sf(fset, rng, src, file, pkg, info)
-		return fset, fix, err
+		rng, err := pgf.RangeToTokenRange(pRng)
+		if err != nil {
+			return nil, nil, err
+		}
+		fix, err := sf(pkg.FileSet(), rng, pgf.Src, pgf.File, pkg.GetTypes(), pkg.GetTypesInfo())
+		return pkg.FileSet(), fix, err
 	}
 }
 
@@ -130,17 +134,3 @@ func ApplyFix(ctx context.Context, fix string, snapshot Snapshot, fh FileHandle,
 	}
 	return edits, nil
 }
-
-// getAllSuggestedFixInputs is a helper function to collect all possible needed
-// inputs for an AppliesFunc or SuggestedFixFunc.
-func getAllSuggestedFixInputs(ctx context.Context, snapshot Snapshot, fh FileHandle, pRng protocol.Range) (*token.FileSet, safetoken.Range, []byte, *ast.File, *types.Package, *types.Info, error) {
-	pkg, pgf, err := PackageForFile(ctx, snapshot, fh.URI(), TypecheckWorkspace, NarrowestPackage)
-	if err != nil {
-		return nil, safetoken.Range{}, nil, nil, nil, nil, fmt.Errorf("getting file for Identifier: %w", err)
-	}
-	rng, err := pgf.RangeToTokenRange(pRng)
-	if err != nil {
-		return nil, safetoken.Range{}, nil, nil, nil, nil, err
-	}
-	return pkg.FileSet(), rng, pgf.Src, pgf.File, pkg.GetTypes(), pkg.GetTypesInfo(), nil
-}

gopls/internal/lsp/source/highlight.go

@@ -23,7 +23,7 @@ func Highlight(ctx context.Context, snapshot Snapshot, fh FileHandle, position p
 
 	// We always want fully parsed files for highlight, regardless
 	// of whether the file belongs to a workspace package.
-	pkg, pgf, err := PackageForFile(ctx, snapshot, fh.URI(), TypecheckFull, WidestPackage)
+	pkg, pgf, err := PackageForFile(ctx, snapshot, fh.URI(), TypecheckFull, NarrowestPackage)
 	if err != nil {
 		return nil, fmt.Errorf("getting package for Highlight: %w", err)
 	}

gopls/internal/lsp/source/inlay_hint.go

@@ -82,7 +82,7 @@ func InlayHint(ctx context.Context, snapshot Snapshot, fh FileHandle, pRng proto
 	ctx, done := event.Start(ctx, "source.InlayHint")
 	defer done()
 
-	pkg, pgf, err := PackageForFile(ctx, snapshot, fh.URI(), TypecheckWorkspace, NarrowestPackage)
+	pkg, pgf, err := PackageForFile(ctx, snapshot, fh.URI(), TypecheckFull, NarrowestPackage)
 	if err != nil {
 		return nil, fmt.Errorf("getting file for InlayHint: %w", err)
 	}

gopls/internal/lsp/source/signature_help.go

@@ -20,7 +20,9 @@ func SignatureHelp(ctx context.Context, snapshot Snapshot, fh FileHandle, positi
 	ctx, done := event.Start(ctx, "source.SignatureHelp")
 	defer done()
 
-	pkg, pgf, err := PackageForFile(ctx, snapshot, fh.URI(), TypecheckWorkspace, NarrowestPackage)
+	// We need full type-checking here, as we must type-check function bodies in
+	// order to provide signature help at the requested position.
+	pkg, pgf, err := PackageForFile(ctx, snapshot, fh.URI(), TypecheckFull, NarrowestPackage)
 	if err != nil {
 		return nil, 0, fmt.Errorf("getting file for SignatureHelp: %w", err)
 	}

gopls/internal/lsp/source/stub.go

@@ -26,7 +26,7 @@ import (
 )
 
 func stubSuggestedFixFunc(ctx context.Context, snapshot Snapshot, fh FileHandle, rng protocol.Range) (*token.FileSet, *analysis.SuggestedFix, error) {
-	pkg, pgf, err := PackageForFile(ctx, snapshot, fh.URI(), TypecheckWorkspace, NarrowestPackage)
+	pkg, pgf, err := PackageForFile(ctx, snapshot, fh.URI(), TypecheckFull, NarrowestPackage)
 	if err != nil {
 		return nil, nil, fmt.Errorf("GetTypedFile: %w", err)
 	}

gopls/internal/regtest/misc/signature_help_test.go

@@ -0,0 +1,69 @@
+// Copyright 2023 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 misc
+
+import (
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"golang.org/x/tools/gopls/internal/lsp/protocol"
+	. "golang.org/x/tools/gopls/internal/lsp/regtest"
+)
+
+func TestSignatureHelpInNonWorkspacePackage(t *testing.T) {
+	const files = `
+-- a/go.mod --
+module a.com
+
+go 1.18
+-- a/a/a.go --
+package a
+
+func DoSomething(int) {}
+
+func _() {
+	DoSomething()
+}
+-- b/go.mod --
+module b.com
+go 1.18
+
+require a.com v1.0.0
+
+replace a.com => ../a
+-- b/b/b.go --
+package b
+
+import "a.com/a"
+
+func _() {
+	a.DoSomething()
+}
+`
+
+	WithOptions(
+		WorkspaceFolders("a"),
+	).Run(t, files, func(t *testing.T, env *Env) {
+		env.OpenFile("a/a/a.go")
+		env.OpenFile("b/b/b.go")
+		signatureHelp := func(filename string) *protocol.SignatureHelp {
+			pos := env.RegexpSearch(filename, `DoSomething\(()\)`)
+			var params protocol.SignatureHelpParams
+			params.Position = pos
+			params.TextDocument.URI = env.Sandbox.Workdir.URI(filename)
+			help, err := env.Editor.Server.SignatureHelp(env.Ctx, &params)
+			if err != nil {
+				t.Fatal(err)
+			}
+			return help
+		}
+		ahelp := signatureHelp("a/a/a.go")
+		bhelp := signatureHelp("b/b/b.go")
+
+		if diff := cmp.Diff(ahelp, bhelp); diff != "" {
+			t.Fatal(diff)
+		}
+	})
+}