From 13648cdeaf9ceb853cfba57e63c7ae5cfe4e7b7d Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Mon, 7 Nov 2022 18:08:35 -0500 Subject: [PATCH] gopls/internal/lsp/cache: use Package.FileSet where possible This change adds a FileSet field to Package, and uses it in preference to Snapshot.FileSet wherever that is appropriate: all but a handful of places. For now, they must continue to refer to the same instance, but once we do away with the Snapshot's cache of parsed files, there will be no need for a global FileSet and each Package will be able to create its own. (Some care will be required to make sure it is always clear which package owns each each token.Pos/ast.Node/types.Object when there are several in play.) Updates golang/go#56291 Change-Id: I017eb794ffb737550da6ae88462d23a8f5c1e1e7 Reviewed-on: https://go-review.googlesource.com/c/tools/+/448377 Run-TryBot: Alan Donovan TryBot-Result: Gopher Robot gopls-CI: kokoro Reviewed-by: Robert Findley --- gopls/internal/lsp/cache/analysis.go | 2 +- gopls/internal/lsp/cache/check.go | 6 +++-- gopls/internal/lsp/cache/errors.go | 21 ++++++++------- gopls/internal/lsp/cache/load.go | 5 ---- gopls/internal/lsp/cache/parse.go | 2 ++ gopls/internal/lsp/cache/pkg.go | 8 +++++- gopls/internal/lsp/command.go | 2 +- gopls/internal/lsp/completion.go | 10 +++---- gopls/internal/lsp/semantic.go | 2 +- gopls/internal/lsp/source/call_hierarchy.go | 2 +- .../lsp/source/completion/completion.go | 20 ++++++-------- .../internal/lsp/source/completion/format.go | 6 ++--- .../internal/lsp/source/completion/literal.go | 6 ++--- .../internal/lsp/source/completion/package.go | 3 ++- .../lsp/source/completion/postfix_snippets.go | 7 ++--- .../internal/lsp/source/completion/snippet.go | 2 +- .../lsp/source/completion/statements.go | 4 +-- gopls/internal/lsp/source/fix.go | 14 +++++----- gopls/internal/lsp/source/format.go | 3 +-- gopls/internal/lsp/source/highlight.go | 2 +- gopls/internal/lsp/source/hover.go | 8 +++--- gopls/internal/lsp/source/identifier.go | 25 +++++++++-------- gopls/internal/lsp/source/implementation.go | 22 +++++++++++---- gopls/internal/lsp/source/references.go | 4 +-- gopls/internal/lsp/source/rename.go | 10 +++---- gopls/internal/lsp/source/rename_check.go | 6 ++--- gopls/internal/lsp/source/signature_help.go | 2 +- gopls/internal/lsp/source/stub.go | 27 ++++++++++--------- gopls/internal/lsp/source/types_format.go | 19 ++++++------- gopls/internal/lsp/source/util.go | 11 ++++---- gopls/internal/lsp/source/view.go | 6 +++++ 31 files changed, 144 insertions(+), 123 deletions(-) diff --git a/gopls/internal/lsp/cache/analysis.go b/gopls/internal/lsp/cache/analysis.go index cbb6f04e76..43f1afbe89 100644 --- a/gopls/internal/lsp/cache/analysis.go +++ b/gopls/internal/lsp/cache/analysis.go @@ -286,7 +286,7 @@ func actionImpl(ctx context.Context, snapshot *snapshot, deps []*actionHandle, a var rawDiagnostics []analysis.Diagnostic pass := &analysis.Pass{ Analyzer: analyzer, - Fset: snapshot.FileSet(), + Fset: pkg.FileSet(), Files: syntax, Pkg: pkg.GetTypes(), TypesInfo: pkg.GetTypesInfo(), diff --git a/gopls/internal/lsp/cache/check.go b/gopls/internal/lsp/cache/check.go index 580ed0c87e..4b1ea3debe 100644 --- a/gopls/internal/lsp/cache/check.go +++ b/gopls/internal/lsp/cache/check.go @@ -450,6 +450,7 @@ func doTypeCheck(ctx context.Context, snapshot *snapshot, goFiles, compiledGoFil pkg := &pkg{ m: m, mode: mode, + fset: snapshot.FileSet(), // must match parse call below (snapshot.ParseGo for now) deps: make(map[PackageID]*pkg), types: types.NewPackage(string(m.PkgPath), string(m.Name)), typesInfo: &types.Info{ @@ -565,7 +566,7 @@ func doTypeCheck(ctx context.Context, snapshot *snapshot, goFiles, compiledGoFil // We passed typecheckCgo to go/packages when we Loaded. typesinternal.SetUsesCgo(cfg) - check := types.NewChecker(cfg, snapshot.FileSet(), pkg.types, pkg.typesInfo) + check := types.NewChecker(cfg, pkg.fset, pkg.types, pkg.typesInfo) var files []*ast.File for _, cgf := range pkg.compiledGoFiles { @@ -593,7 +594,7 @@ func parseCompiledGoFiles(ctx context.Context, compiledGoFiles []source.FileHand if mode == source.ParseFull { pgf, err = snapshot.ParseGo(ctx, fh, mode) } else { - pgf, err = parseGoImpl(ctx, snapshot.FileSet(), fh, mode) // ~20us/KB + pgf, err = parseGoImpl(ctx, pkg.fset, fh, mode) // ~20us/KB } if err != nil { return err @@ -661,6 +662,7 @@ func (s *snapshot) depsErrors(ctx context.Context, pkg *pkg) ([]*source.Diagnost } allImports := map[string][]fileImport{} for _, cgf := range pkg.compiledGoFiles { + // TODO(adonovan): modify Imports() to accept a single token.File (cgf.Tok). for _, group := range astutil.Imports(s.FileSet(), cgf.File) { for _, imp := range group { if imp.Path == nil { diff --git a/gopls/internal/lsp/cache/errors.go b/gopls/internal/lsp/cache/errors.go index bca68d5049..1c9c21097e 100644 --- a/gopls/internal/lsp/cache/errors.go +++ b/gopls/internal/lsp/cache/errors.go @@ -104,7 +104,7 @@ var importErrorRe = regexp.MustCompile(`could not import ([^\s]+)`) var unsupportedFeatureRe = regexp.MustCompile(`.*require.* go(\d+\.\d+) or later`) func typeErrorDiagnostics(snapshot *snapshot, pkg *pkg, e extendedError) ([]*source.Diagnostic, error) { - code, spn, err := typeErrorData(snapshot.FileSet(), pkg, e.primary) + code, spn, err := typeErrorData(pkg, e.primary) if err != nil { return nil, err } @@ -129,7 +129,7 @@ func typeErrorDiagnostics(snapshot *snapshot, pkg *pkg, e extendedError) ([]*sou } for _, secondary := range e.secondaries { - _, secondarySpan, err := typeErrorData(snapshot.FileSet(), pkg, secondary) + _, secondarySpan, err := typeErrorData(pkg, secondary) if err != nil { return nil, err } @@ -201,7 +201,7 @@ func analysisDiagnosticDiagnostics(snapshot *snapshot, pkg *pkg, a *analysis.Ana break } } - tokFile := snapshot.FileSet().File(e.Pos) + tokFile := pkg.fset.File(e.Pos) if tokFile == nil { return nil, bug.Errorf("no file for position of %q diagnostic", e.Category) } @@ -221,7 +221,7 @@ func analysisDiagnosticDiagnostics(snapshot *snapshot, pkg *pkg, a *analysis.Ana if len(srcAnalyzer.ActionKind) == 0 { kinds = append(kinds, protocol.QuickFix) } - fixes, err := suggestedAnalysisFixes(snapshot, pkg, e, kinds) + fixes, err := suggestedAnalysisFixes(pkg, e, kinds) if err != nil { return nil, err } @@ -238,7 +238,7 @@ func analysisDiagnosticDiagnostics(snapshot *snapshot, pkg *pkg, a *analysis.Ana fixes = append(fixes, source.SuggestedFixFromCommand(cmd, kind)) } } - related, err := relatedInformation(pkg, snapshot.FileSet(), e) + related, err := relatedInformation(pkg, e) if err != nil { return nil, err } @@ -289,12 +289,12 @@ func typesCodeHref(snapshot *snapshot, code typesinternal.ErrorCode) string { return source.BuildLink(target, "golang.org/x/tools/internal/typesinternal", code.String()) } -func suggestedAnalysisFixes(snapshot *snapshot, pkg *pkg, diag *analysis.Diagnostic, kinds []protocol.CodeActionKind) ([]source.SuggestedFix, error) { +func suggestedAnalysisFixes(pkg *pkg, diag *analysis.Diagnostic, kinds []protocol.CodeActionKind) ([]source.SuggestedFix, error) { var fixes []source.SuggestedFix for _, fix := range diag.SuggestedFixes { edits := make(map[span.URI][]protocol.TextEdit) for _, e := range fix.TextEdits { - tokFile := snapshot.FileSet().File(e.Pos) + tokFile := pkg.fset.File(e.Pos) if tokFile == nil { return nil, bug.Errorf("no file for edit position") } @@ -327,10 +327,10 @@ func suggestedAnalysisFixes(snapshot *snapshot, pkg *pkg, diag *analysis.Diagnos return fixes, nil } -func relatedInformation(pkg *pkg, fset *token.FileSet, diag *analysis.Diagnostic) ([]source.RelatedInformation, error) { +func relatedInformation(pkg *pkg, diag *analysis.Diagnostic) ([]source.RelatedInformation, error) { var out []source.RelatedInformation for _, related := range diag.Related { - tokFile := fset.File(related.Pos) + tokFile := pkg.fset.File(related.Pos) if tokFile == nil { return nil, bug.Errorf("no file for %q diagnostic position", diag.Category) } @@ -355,7 +355,7 @@ func relatedInformation(pkg *pkg, fset *token.FileSet, diag *analysis.Diagnostic return out, nil } -func typeErrorData(fset *token.FileSet, pkg *pkg, terr types.Error) (typesinternal.ErrorCode, span.Span, error) { +func typeErrorData(pkg *pkg, terr types.Error) (typesinternal.ErrorCode, span.Span, error) { ecode, start, end, ok := typesinternal.ReadGo116ErrorData(terr) if !ok { start, end = terr.Pos, terr.Pos @@ -368,6 +368,7 @@ func typeErrorData(fset *token.FileSet, pkg *pkg, terr types.Error) (typesintern } // go/types errors retain their FileSet. // Sanity-check that we're using the right one. + fset := pkg.FileSet() if fset != terr.Fset { return 0, span.Span{}, bug.Errorf("wrong FileSet for type error") } diff --git a/gopls/internal/lsp/cache/load.go b/gopls/internal/lsp/cache/load.go index 5250a67f14..e779cbac4a 100644 --- a/gopls/internal/lsp/cache/load.go +++ b/gopls/internal/lsp/cache/load.go @@ -50,11 +50,6 @@ func (s *snapshot) load(ctx context.Context, allowNetwork bool, scopes ...loadSc for _, scope := range scopes { switch scope := scope.(type) { case packageLoadScope: - // TODO(adonovan): is this cast sound?? A - // packageLoadScope is really a PackagePath I think. - if source.IsCommandLineArguments(PackageID(scope)) { - panic("attempted to load command-line-arguments") - } // The only time we pass package paths is when we're doing a // partial workspace load. In those cases, the paths came back from // go list and should already be GOPATH-vendorized when appropriate. diff --git a/gopls/internal/lsp/cache/parse.go b/gopls/internal/lsp/cache/parse.go index 5bac871315..4f905e96f7 100644 --- a/gopls/internal/lsp/cache/parse.go +++ b/gopls/internal/lsp/cache/parse.go @@ -36,6 +36,8 @@ type parseKey struct { // ParseGo parses the file whose contents are provided by fh, using a cache. // The resulting tree may have be fixed up. // +// Token position information will be added to the snapshot's FileSet. +// // The parser mode must not be ParseExported: that mode is used during // type checking to destructively trim the tree to reduce work, // which is not safe for values from a shared cache. diff --git a/gopls/internal/lsp/cache/pkg.go b/gopls/internal/lsp/cache/pkg.go index ef8177871b..6e1233a37b 100644 --- a/gopls/internal/lsp/cache/pkg.go +++ b/gopls/internal/lsp/cache/pkg.go @@ -8,6 +8,7 @@ import ( "fmt" "go/ast" "go/scanner" + "go/token" "go/types" "sort" @@ -21,6 +22,7 @@ import ( type pkg struct { m *Metadata mode source.ParseMode + fset *token.FileSet // for now, same as the snapshot's FileSet goFiles []*source.ParsedGoFile compiledGoFiles []*source.ParsedGoFile diagnostics []*source.Diagnostic @@ -45,7 +47,7 @@ type loadScope interface { type ( fileLoadScope span.URI // load packages containing a file (including command-line-arguments) - packageLoadScope string // load a specific package + packageLoadScope string // load a specific package (the value is its PackageID) moduleLoadScope string // load packages in a specific module viewLoadScope span.URI // load the workspace ) @@ -90,6 +92,10 @@ func (p *pkg) GetSyntax() []*ast.File { return syntax } +func (p *pkg) FileSet() *token.FileSet { + return p.fset +} + func (p *pkg) GetTypes() *types.Package { return p.types } diff --git a/gopls/internal/lsp/command.go b/gopls/internal/lsp/command.go index 3b0d3b849d..a3d93e265c 100644 --- a/gopls/internal/lsp/command.go +++ b/gopls/internal/lsp/command.go @@ -757,7 +757,7 @@ func (c *commandHandler) ListImports(ctx context.Context, args command.URIArg) ( if err != nil { return err } - for _, group := range astutil.Imports(deps.snapshot.FileSet(), pgf.File) { + for _, group := range astutil.Imports(pkg.FileSet(), pgf.File) { for _, imp := range group { if imp.Path == nil { continue diff --git a/gopls/internal/lsp/completion.go b/gopls/internal/lsp/completion.go index 465526668a..c967c1faa2 100644 --- a/gopls/internal/lsp/completion.go +++ b/gopls/internal/lsp/completion.go @@ -9,14 +9,14 @@ import ( "fmt" "strings" - "golang.org/x/tools/internal/event" - "golang.org/x/tools/internal/event/tag" "golang.org/x/tools/gopls/internal/lsp/lsppos" "golang.org/x/tools/gopls/internal/lsp/protocol" "golang.org/x/tools/gopls/internal/lsp/source" "golang.org/x/tools/gopls/internal/lsp/source/completion" "golang.org/x/tools/gopls/internal/lsp/template" "golang.org/x/tools/gopls/internal/lsp/work" + "golang.org/x/tools/internal/event" + "golang.org/x/tools/internal/event/tag" ) func (s *Server) completion(ctx context.Context, params *protocol.CompletionParams) (*protocol.CompletionList, error) { @@ -64,9 +64,9 @@ func (s *Server) completion(ctx context.Context, params *protocol.CompletionPara if err != nil { return nil, err } - tf := snapshot.FileSet().File(surrounding.Start()) - mapper := lsppos.NewTokenMapper(src, tf) - rng, err := mapper.Range(surrounding.Start(), surrounding.End()) + srng := surrounding.Range() + tf := snapshot.FileSet().File(srng.Start) // not same as srng.TokFile due to //line + rng, err := lsppos.NewTokenMapper(src, tf).Range(srng.Start, srng.End) if err != nil { return nil, err } diff --git a/gopls/internal/lsp/semantic.go b/gopls/internal/lsp/semantic.go index 75f88b3efa..4cdca20556 100644 --- a/gopls/internal/lsp/semantic.go +++ b/gopls/internal/lsp/semantic.go @@ -112,7 +112,7 @@ func (s *Server) computeSemanticTokens(ctx context.Context, td protocol.TextDocu rng: rng, ti: pkg.GetTypesInfo(), pkg: pkg, - fset: snapshot.FileSet(), + fset: pkg.FileSet(), tokTypes: s.session.Options().SemanticTypes, tokMods: s.session.Options().SemanticMods, noStrings: vv.Options().NoSemanticString, diff --git a/gopls/internal/lsp/source/call_hierarchy.go b/gopls/internal/lsp/source/call_hierarchy.go index 1097d62907..5c5c584fd9 100644 --- a/gopls/internal/lsp/source/call_hierarchy.go +++ b/gopls/internal/lsp/source/call_hierarchy.go @@ -205,7 +205,7 @@ func OutgoingCalls(ctx context.Context, snapshot Snapshot, fh FileHandle, pos pr // TODO(adonovan): avoid Fileset.File call by somehow getting at // declMappedRange.spanRange.TokFile, or making Identifier retain the // token.File of the identifier and its declaration, since it looks up both anyway. - tokFile := snapshot.FileSet().File(node.Pos()) + tokFile := identifier.pkg.FileSet().File(node.Pos()) if tokFile == nil { return nil, fmt.Errorf("no file for position") } diff --git a/gopls/internal/lsp/source/completion/completion.go b/gopls/internal/lsp/source/completion/completion.go index 1f8dd929b2..1260e0e7c3 100644 --- a/gopls/internal/lsp/source/completion/completion.go +++ b/gopls/internal/lsp/source/completion/completion.go @@ -289,7 +289,7 @@ type completionContext struct { // A Selection represents the cursor position and surrounding identifier. type Selection struct { content string - cursor token.Pos + cursor token.Pos // relative to rng.TokFile rng span.Range } @@ -297,12 +297,8 @@ func (p Selection) Content() string { return p.content } -func (p Selection) Start() token.Pos { - return p.rng.Start -} - -func (p Selection) End() token.Pos { - return p.rng.End +func (p Selection) Range() span.Range { + return p.rng } func (p Selection) Prefix() string { @@ -693,7 +689,7 @@ func (c *completer) containingIdent(src []byte) *ast.Ident { // scanToken scans pgh's contents for the token containing pos. func (c *completer) scanToken(contents []byte) (token.Pos, token.Token, string) { - tok := c.snapshot.FileSet().File(c.pos) + tok := c.pkg.FileSet().File(c.pos) var s scanner.Scanner s.Init(tok, contents, nil, 0) @@ -879,7 +875,7 @@ func (c *completer) populateCommentCompletions(ctx context.Context, comment *ast } // Using the comment position find the line after - file := c.snapshot.FileSet().File(comment.End()) + file := c.pkg.FileSet().File(comment.End()) if file == nil { return } @@ -1246,7 +1242,7 @@ func (c *completer) methodsAndFields(typ types.Type, addressable bool, imp *impo if isStarTestingDotF(typ) && addressable { // is that a sufficient test? (or is more care needed?) - if c.fuzz(typ, mset, imp, cb, c.snapshot.FileSet()) { + if c.fuzz(typ, mset, imp, cb, c.pkg.FileSet()) { return } } @@ -1331,7 +1327,7 @@ func (c *completer) lexical(ctx context.Context) error { node = c.path[i-1] } if node != nil { - if resolved := resolveInvalid(c.snapshot.FileSet(), obj, node, c.pkg.GetTypesInfo()); resolved != nil { + if resolved := resolveInvalid(c.pkg.FileSet(), obj, node, c.pkg.GetTypesInfo()); resolved != nil { obj = resolved } } @@ -2033,7 +2029,7 @@ Nodes: // // TODO: remove this after https://go.dev/issue/52503 info := &types.Info{Types: make(map[ast.Expr]types.TypeAndValue)} - types.CheckExpr(c.snapshot.FileSet(), c.pkg.GetTypes(), node.Fun.Pos(), node.Fun, info) + types.CheckExpr(c.pkg.FileSet(), c.pkg.GetTypes(), node.Fun.Pos(), node.Fun, info) sig, _ = info.Types[node.Fun].Type.(*types.Signature) } diff --git a/gopls/internal/lsp/source/completion/format.go b/gopls/internal/lsp/source/completion/format.go index c2693dc12d..2ff449a454 100644 --- a/gopls/internal/lsp/source/completion/format.go +++ b/gopls/internal/lsp/source/completion/format.go @@ -80,7 +80,7 @@ func (c *completer) item(ctx context.Context, cand candidate) (CompletionItem, e if _, ok := obj.Type().(*types.Struct); ok { detail = "struct{...}" // for anonymous structs } else if obj.IsField() { - detail = source.FormatVarType(c.snapshot.FileSet(), c.pkg, obj, c.qf) + detail = source.FormatVarType(c.pkg, obj, c.qf) } if obj.IsField() { kind = protocol.FieldCompletion @@ -227,7 +227,7 @@ Suffixes: if !c.opts.documentation { return item, nil } - pos := c.snapshot.FileSet().Position(obj.Pos()) + pos := c.pkg.FileSet().Position(obj.Pos()) // We ignore errors here, because some types, like "unsafe" or "error", // may not have valid positions that we can use to get documentation. @@ -237,7 +237,7 @@ Suffixes: uri := span.URIFromPath(pos.Filename) // Find the source file of the candidate. - pkg, err := source.FindPackageFromPos(c.snapshot.FileSet(), c.pkg, obj.Pos()) + pkg, err := source.FindPackageFromPos(c.pkg, obj.Pos()) if err != nil { return item, nil } diff --git a/gopls/internal/lsp/source/completion/literal.go b/gopls/internal/lsp/source/completion/literal.go index 0a9fc83e66..2975c8fe59 100644 --- a/gopls/internal/lsp/source/completion/literal.go +++ b/gopls/internal/lsp/source/completion/literal.go @@ -202,7 +202,7 @@ func (c *completer) functionLiteral(sig *types.Signature, matchScore float64) { // If the param has no name in the signature, guess a name based // on the type. Use an empty qualifier to ignore the package. // For example, we want to name "http.Request" "r", not "hr". - name = source.FormatVarType(c.snapshot.FileSet(), c.pkg, p, func(p *types.Package) string { + name = source.FormatVarType(c.pkg, p, func(p *types.Package) string { return "" }) name = abbreviateTypeName(name) @@ -264,7 +264,7 @@ func (c *completer) functionLiteral(sig *types.Signature, matchScore float64) { // of "i int, j int". if i == sig.Params().Len()-1 || !types.Identical(p.Type(), sig.Params().At(i+1).Type()) { snip.WriteText(" ") - typeStr := source.FormatVarType(c.snapshot.FileSet(), c.pkg, p, c.qf) + typeStr := source.FormatVarType(c.pkg, p, c.qf) if sig.Variadic() && i == sig.Params().Len()-1 { typeStr = strings.Replace(typeStr, "[]", "...", 1) } @@ -314,7 +314,7 @@ func (c *completer) functionLiteral(sig *types.Signature, matchScore float64) { snip.WriteText(name + " ") } - text := source.FormatVarType(c.snapshot.FileSet(), c.pkg, r, c.qf) + text := source.FormatVarType(c.pkg, r, c.qf) if tp, _ := r.Type().(*typeparams.TypeParam); tp != nil && !c.typeParamInScope(tp) { snip.WritePlaceholder(func(snip *snippet.Builder) { snip.WriteText(text) diff --git a/gopls/internal/lsp/source/completion/package.go b/gopls/internal/lsp/source/completion/package.go index 1b7c731fd8..19c52491ce 100644 --- a/gopls/internal/lsp/source/completion/package.go +++ b/gopls/internal/lsp/source/completion/package.go @@ -31,6 +31,7 @@ import ( func packageClauseCompletions(ctx context.Context, snapshot source.Snapshot, fh source.FileHandle, position protocol.Position) ([]CompletionItem, *Selection, error) { // We know that the AST for this file will be empty due to the missing // package declaration, but parse it anyway to get a mapper. + fset := snapshot.FileSet() pgf, err := snapshot.ParseGo(ctx, fh, source.ParseFull) if err != nil { return nil, nil, err @@ -41,7 +42,7 @@ func packageClauseCompletions(ctx context.Context, snapshot source.Snapshot, fh return nil, nil, err } - surrounding, err := packageCompletionSurrounding(snapshot.FileSet(), pgf, pos) + surrounding, err := packageCompletionSurrounding(fset, pgf, pos) if err != nil { return nil, nil, fmt.Errorf("invalid position for package completion: %w", err) } diff --git a/gopls/internal/lsp/source/completion/postfix_snippets.go b/gopls/internal/lsp/source/completion/postfix_snippets.go index 3c805a77e1..07d07235df 100644 --- a/gopls/internal/lsp/source/completion/postfix_snippets.go +++ b/gopls/internal/lsp/source/completion/postfix_snippets.go @@ -322,7 +322,7 @@ func (c *completer) addPostfixSnippetCandidates(ctx context.Context, sel *ast.Se return } - tokFile := c.snapshot.FileSet().File(c.pos) + tokFile := c.pkg.FileSet().File(c.pos) // Only replace sel with a statement if sel is already a statement. var stmtOK bool @@ -379,7 +379,7 @@ func (c *completer) addPostfixSnippetCandidates(ctx context.Context, sel *ast.Se } tmplArgs := postfixTmplArgs{ - X: source.FormatNode(c.snapshot.FileSet(), sel.X), + X: source.FormatNode(c.pkg.FileSet(), sel.X), StmtOK: stmtOK, Obj: exprObj(c.pkg.GetTypesInfo(), sel.X), Type: selType, @@ -442,7 +442,8 @@ func (c *completer) importIfNeeded(pkgPath string, scope *types.Scope) (string, // Check if file already imports pkgPath. for _, s := range c.file.Imports { - // TODO(adonovan): what if pkgPath has a vendor/ prefix? + // TODO(adonovan): what if pkgPath has a vendor/ suffix? + // This may be the cause of go.dev/issue/56291. if source.UnquoteImportPath(s) == source.ImportPath(pkgPath) { if s.Name == nil { return defaultName, nil, nil diff --git a/gopls/internal/lsp/source/completion/snippet.go b/gopls/internal/lsp/source/completion/snippet.go index 1a9ebb1d4b..751f61b7a2 100644 --- a/gopls/internal/lsp/source/completion/snippet.go +++ b/gopls/internal/lsp/source/completion/snippet.go @@ -39,7 +39,7 @@ func (c *completer) structFieldSnippet(cand candidate, detail string, snip *snip } }) - fset := c.snapshot.FileSet() + fset := c.pkg.FileSet() // If the cursor position is on a different line from the literal's opening brace, // we are in a multiline literal. diff --git a/gopls/internal/lsp/source/completion/statements.go b/gopls/internal/lsp/source/completion/statements.go index 1f80193f19..dcce76ff76 100644 --- a/gopls/internal/lsp/source/completion/statements.go +++ b/gopls/internal/lsp/source/completion/statements.go @@ -52,7 +52,7 @@ func (c *completer) addAssignAppend() { // needsLHS is true if we need to prepend the LHS slice name and // "=" to our candidate. needsLHS = false - fset = c.snapshot.FileSet() + fset = c.pkg.FileSet() ) switch n := c.path[1].(type) { @@ -213,7 +213,7 @@ func (c *completer) addErrCheck() { var ( // errVar is e.g. "err" in "foo, err := bar()". - errVar = source.FormatNode(c.snapshot.FileSet(), lastAssignee) + errVar = source.FormatNode(c.pkg.FileSet(), lastAssignee) // Whether we need to include the "if" keyword in our candidate. needsIf = true diff --git a/gopls/internal/lsp/source/fix.go b/gopls/internal/lsp/source/fix.go index a4c6676cf0..64498e22b8 100644 --- a/gopls/internal/lsp/source/fix.go +++ b/gopls/internal/lsp/source/fix.go @@ -26,7 +26,7 @@ type ( // suggested fixes with their diagnostics, so we have to compute them // separately. Such analyzers should provide a function with a signature of // SuggestedFixFunc. - SuggestedFixFunc func(ctx context.Context, snapshot Snapshot, fh VersionedFileHandle, pRng protocol.Range) (*analysis.SuggestedFix, error) + SuggestedFixFunc func(ctx context.Context, snapshot Snapshot, fh VersionedFileHandle, pRng protocol.Range) (*token.FileSet, *analysis.SuggestedFix, error) singleFileFixFunc func(fset *token.FileSet, rng span.Range, src []byte, file *ast.File, pkg *types.Package, info *types.Info) (*analysis.SuggestedFix, error) ) @@ -51,12 +51,13 @@ 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 VersionedFileHandle, pRng protocol.Range) (*analysis.SuggestedFix, error) { + return func(ctx context.Context, snapshot Snapshot, fh VersionedFileHandle, pRng protocol.Range) (*token.FileSet, *analysis.SuggestedFix, error) { fset, rng, src, file, pkg, info, err := getAllSuggestedFixInputs(ctx, snapshot, fh, pRng) if err != nil { - return nil, err + return nil, nil, err } - return sf(fset, rng, src, file, pkg, info) + fix, err := sf(fset, rng, src, file, pkg, info) + return fset, fix, err } } @@ -75,14 +76,13 @@ func ApplyFix(ctx context.Context, fix string, snapshot Snapshot, fh VersionedFi if !ok { return nil, fmt.Errorf("no suggested fix function for %s", fix) } - suggestion, err := handler(ctx, snapshot, fh, pRng) + fset, suggestion, err := handler(ctx, snapshot, fh, pRng) if err != nil { return nil, err } if suggestion == nil { return nil, nil } - fset := snapshot.FileSet() editsPerFile := map[span.URI]*protocol.TextDocumentEdit{} for _, edit := range suggestion.TextEdits { tokFile := fset.File(edit.Pos) @@ -140,5 +140,5 @@ func getAllSuggestedFixInputs(ctx context.Context, snapshot Snapshot, fh FileHan if err != nil { return nil, span.Range{}, nil, nil, nil, nil, err } - return snapshot.FileSet(), rng, pgf.Src, pgf.File, pkg.GetTypes(), pkg.GetTypesInfo(), nil + return pkg.FileSet(), rng, pgf.Src, pgf.File, pkg.GetTypes(), pkg.GetTypesInfo(), nil } diff --git a/gopls/internal/lsp/source/format.go b/gopls/internal/lsp/source/format.go index b713893fba..a83c9e471e 100644 --- a/gopls/internal/lsp/source/format.go +++ b/gopls/internal/lsp/source/format.go @@ -34,6 +34,7 @@ func Format(ctx context.Context, snapshot Snapshot, fh FileHandle) ([]protocol.T return nil, fmt.Errorf("can't format %q: file is generated", fh.URI().Filename()) } + fset := snapshot.FileSet() pgf, err := snapshot.ParseGo(ctx, fh, ParseFull) if err != nil { return nil, err @@ -49,8 +50,6 @@ func Format(ctx context.Context, snapshot Snapshot, fh FileHandle) ([]protocol.T return computeTextEdits(ctx, snapshot, pgf, string(formatted)) } - fset := snapshot.FileSet() - // format.Node changes slightly from one release to another, so the version // of Go used to build the LSP server will determine how it formats code. // This should be acceptable for all users, who likely be prompted to rebuild diff --git a/gopls/internal/lsp/source/highlight.go b/gopls/internal/lsp/source/highlight.go index 5ec71d075a..166dc3cd46 100644 --- a/gopls/internal/lsp/source/highlight.go +++ b/gopls/internal/lsp/source/highlight.go @@ -59,7 +59,7 @@ func Highlight(ctx context.Context, snapshot Snapshot, fh FileHandle, position p } var ranges []protocol.Range for rng := range result { - mRng, err := posToMappedRange(snapshot.FileSet(), pkg, rng.start, rng.end) + mRng, err := posToMappedRange(pkg, rng.start, rng.end) if err != nil { return nil, err } diff --git a/gopls/internal/lsp/source/hover.go b/gopls/internal/lsp/source/hover.go index 0bd5a3055e..26d5e23ad3 100644 --- a/gopls/internal/lsp/source/hover.go +++ b/gopls/internal/lsp/source/hover.go @@ -237,7 +237,7 @@ func findRune(ctx context.Context, snapshot Snapshot, fh FileHandle, position pr return 0, MappedRange{}, ErrNoRuneFound } - mappedRange, err := posToMappedRange(snapshot.FileSet(), pkg, start, end) + mappedRange, err := posToMappedRange(pkg, start, end) if err != nil { return 0, MappedRange{}, err } @@ -258,7 +258,7 @@ func HoverIdentifier(ctx context.Context, i *IdentifierInfo) (*HoverJSON, error) Synopsis: doc.Synopsis(hoverCtx.Comment.Text()), } - fset := i.Snapshot.FileSet() + fset := i.pkg.FileSet() // Determine the symbol's signature. switch x := hoverCtx.signatureSource.(type) { case string: @@ -564,7 +564,7 @@ func FindHoverContext(ctx context.Context, s Snapshot, pkg Package, obj types.Ob } // obj may not have been produced by type checking the AST containing // node, so we need to be careful about using token.Pos. - tok := s.FileSet().File(obj.Pos()) + tok := pkg.FileSet().File(obj.Pos()) offset, err := safetoken.Offset(tok, obj.Pos()) if err != nil { return nil, err @@ -572,7 +572,7 @@ func FindHoverContext(ctx context.Context, s Snapshot, pkg Package, obj types.Ob // fullTok and fullPos are the *token.File and object position in for the // full AST. - fullTok := s.FileSet().File(node.Pos()) + fullTok := pkg.FileSet().File(node.Pos()) fullPos, err := safetoken.Pos(fullTok, offset) if err != nil { return nil, err diff --git a/gopls/internal/lsp/source/identifier.go b/gopls/internal/lsp/source/identifier.go index 526c108470..53ad88bc33 100644 --- a/gopls/internal/lsp/source/identifier.go +++ b/gopls/internal/lsp/source/identifier.go @@ -26,7 +26,7 @@ import ( // IdentifierInfo holds information about an identifier in Go source. type IdentifierInfo struct { Name string - Snapshot Snapshot + Snapshot Snapshot // only needed for .View(); TODO(adonovan): reduce. MappedRange Type struct { @@ -122,7 +122,7 @@ func findIdentifier(ctx context.Context, snapshot Snapshot, pkg Package, pgf *Pa // Special case for package declarations, since they have no // corresponding types.Object. if ident == file.Name { - rng, err := posToMappedRange(snapshot.FileSet(), pkg, file.Name.Pos(), file.Name.End()) + rng, err := posToMappedRange(pkg, file.Name.Pos(), file.Name.End()) if err != nil { return nil, err } @@ -136,7 +136,7 @@ func findIdentifier(ctx context.Context, snapshot Snapshot, pkg Package, pgf *Pa if declAST == nil { declAST = file } - declRng, err := posToMappedRange(snapshot.FileSet(), pkg, declAST.Name.Pos(), declAST.Name.End()) + declRng, err := posToMappedRange(pkg, declAST.Name.Pos(), declAST.Name.End()) if err != nil { return nil, err } @@ -164,7 +164,7 @@ func findIdentifier(ctx context.Context, snapshot Snapshot, pkg Package, pgf *Pa result.Name = result.ident.Name var err error - if result.MappedRange, err = posToMappedRange(snapshot.FileSet(), pkg, result.ident.Pos(), result.ident.End()); err != nil { + if result.MappedRange, err = posToMappedRange(pkg, result.ident.Pos(), result.ident.End()); err != nil { return nil, err } @@ -263,13 +263,13 @@ func findIdentifier(ctx context.Context, snapshot Snapshot, pkg Package, pgf *Pa } } - rng, err := objToMappedRange(snapshot.FileSet(), pkg, result.Declaration.obj) + rng, err := objToMappedRange(pkg, result.Declaration.obj) if err != nil { return nil, err } result.Declaration.MappedRange = append(result.Declaration.MappedRange, rng) - declPkg, err := FindPackageFromPos(snapshot.FileSet(), pkg, result.Declaration.obj.Pos()) + declPkg, err := FindPackageFromPos(pkg, result.Declaration.obj.Pos()) if err != nil { return nil, err } @@ -277,7 +277,7 @@ func findIdentifier(ctx context.Context, snapshot Snapshot, pkg Package, pgf *Pa // Ensure that we have the full declaration, in case the declaration was // parsed in ParseExported and therefore could be missing information. - if result.Declaration.fullDecl, err = fullNode(snapshot, result.Declaration.obj, declPkg); err != nil { + if result.Declaration.fullDecl, err = fullNode(pkg.FileSet(), result.Declaration.obj, declPkg); err != nil { return nil, err } typ := pkg.GetTypesInfo().TypeOf(result.ident) @@ -293,7 +293,7 @@ func findIdentifier(ctx context.Context, snapshot Snapshot, pkg Package, pgf *Pa if hasErrorType(result.Type.Object) { return result, nil } - if result.Type.MappedRange, err = objToMappedRange(snapshot.FileSet(), pkg, result.Type.Object); err != nil { + if result.Type.MappedRange, err = objToMappedRange(pkg, result.Type.Object); err != nil { return nil, err } } @@ -315,9 +315,9 @@ func findGenDecl(f *ast.File, spec ast.Spec) *ast.GenDecl { // fullNode tries to extract the full spec corresponding to obj's declaration. // If the package was not parsed in full, the declaration file will be // re-parsed to ensure it has complete syntax. -func fullNode(snapshot Snapshot, obj types.Object, pkg Package) (ast.Decl, error) { +func fullNode(fset *token.FileSet, obj types.Object, pkg Package) (ast.Decl, error) { // declaration in a different package... make sure we have full AST information. - tok := snapshot.FileSet().File(obj.Pos()) + tok := fset.File(obj.Pos()) uri := span.URIFromPath(tok.Name()) pgf, err := pkg.File(uri) if err != nil { @@ -326,7 +326,6 @@ func fullNode(snapshot Snapshot, obj types.Object, pkg Package) (ast.Decl, error file := pgf.File pos := obj.Pos() if pgf.Mode != ParseFull { - fset := snapshot.FileSet() file2, _ := parser.ParseFile(fset, tok.Name(), pgf.Src, parser.AllErrors|parser.ParseComments) if file2 != nil { offset, err := safetoken.Offset(tok, obj.Pos()) @@ -465,13 +464,13 @@ func importSpec(snapshot Snapshot, pkg Package, file *ast.File, pos token.Pos) ( Name: importPath, // should this perhaps be imported.PkgPath()? pkg: pkg, } - if result.MappedRange, err = posToMappedRange(snapshot.FileSet(), pkg, imp.Path.Pos(), imp.Path.End()); err != nil { + if result.MappedRange, err = posToMappedRange(pkg, imp.Path.Pos(), imp.Path.End()); err != nil { return nil, err } // Consider the "declaration" of an import spec to be the imported package. // Return all of the files in the package as the definition of the import spec. for _, dst := range imported.GetSyntax() { - rng, err := posToMappedRange(snapshot.FileSet(), pkg, dst.Pos(), dst.End()) + rng, err := posToMappedRange(pkg, dst.Pos(), dst.End()) if err != nil { return nil, err } diff --git a/gopls/internal/lsp/source/implementation.go b/gopls/internal/lsp/source/implementation.go index 87f435108d..ca62f4e664 100644 --- a/gopls/internal/lsp/source/implementation.go +++ b/gopls/internal/lsp/source/implementation.go @@ -32,7 +32,7 @@ func Implementation(ctx context.Context, snapshot Snapshot, f FileHandle, pp pro if impl.pkg == nil || len(impl.pkg.CompiledGoFiles()) == 0 { continue } - rng, err := objToMappedRange(snapshot.FileSet(), impl.pkg, impl.obj) + rng, err := objToMappedRange(impl.pkg, impl.obj) if err != nil { return nil, err } @@ -145,16 +145,28 @@ func implementations(ctx context.Context, s Snapshot, f FileHandle, pp protocol. candObj = sel.Obj() } - pos := s.FileSet().Position(candObj.Pos()) - if candObj == queryMethod || seen[pos] { + if candObj == queryMethod { continue } - seen[pos] = true + pkg := pkgs[candObj.Pkg()] // may be nil (e.g. error) + + // TODO(adonovan): the logic below assumes there is only one + // predeclared (pkg=nil) object of interest, the error type. + // That could change in a future version of Go. + + var posn token.Position + if pkg != nil { + posn = pkg.FileSet().Position(candObj.Pos()) + } + if seen[posn] { + continue + } + seen[posn] = true impls = append(impls, qualifiedObject{ obj: candObj, - pkg: pkgs[candObj.Pkg()], // may be nil (e.g. error) + pkg: pkg, }) } } diff --git a/gopls/internal/lsp/source/references.go b/gopls/internal/lsp/source/references.go index 4db716e6dd..d0310560c1 100644 --- a/gopls/internal/lsp/source/references.go +++ b/gopls/internal/lsp/source/references.go @@ -144,7 +144,7 @@ func references(ctx context.Context, snapshot Snapshot, qos []qualifiedObject, i } // Inv: qos[0].pkg != nil, since Pos is valid. // Inv: qos[*].pkg != nil, since all qos are logically the same declaration. - filename := snapshot.FileSet().Position(pos).Filename + filename := qos[0].pkg.FileSet().File(pos).Name() pgf, err := qos[0].pkg.File(span.URIFromPath(filename)) if err != nil { return nil, err @@ -208,7 +208,7 @@ func references(ctx context.Context, snapshot Snapshot, qos []qualifiedObject, i continue } seen[key] = true - rng, err := posToMappedRange(snapshot.FileSet(), pkg, ident.Pos(), ident.End()) + rng, err := posToMappedRange(pkg, ident.Pos(), ident.End()) if err != nil { return nil, err } diff --git a/gopls/internal/lsp/source/rename.go b/gopls/internal/lsp/source/rename.go index 3a8d7cfb6b..9586d8418c 100644 --- a/gopls/internal/lsp/source/rename.go +++ b/gopls/internal/lsp/source/rename.go @@ -28,7 +28,6 @@ import ( type renamer struct { ctx context.Context - fset *token.FileSet refs []*ReferenceInfo objsToUpdate map[types.Object]bool hadConflicts bool @@ -136,7 +135,7 @@ func PrepareRename(ctx context.Context, snapshot Snapshot, f FileHandle, pp prot } func computePrepareRenameResp(snapshot Snapshot, pkg Package, node ast.Node, text string) (*PrepareItem, error) { - mr, err := posToMappedRange(snapshot.FileSet(), pkg, node.Pos(), node.End()) + mr, err := posToMappedRange(pkg, node.Pos(), node.End()) if err != nil { return nil, err } @@ -484,7 +483,6 @@ func renameObj(ctx context.Context, s Snapshot, newName string, qos []qualifiedO } r := renamer{ ctx: ctx, - fset: s.FileSet(), refs: refs, objsToUpdate: make(map[types.Object]bool), from: obj.Name(), @@ -604,7 +602,7 @@ func (r *renamer) update() (map[span.URI][]diff.Edit, error) { // TODO(adonovan): why are we looping over lines? // Just run the loop body once over the entire multiline comment. lines := strings.Split(comment.Text, "\n") - tokFile := r.fset.File(comment.Pos()) + tokFile := ref.pkg.FileSet().File(comment.Pos()) commentLine := tokFile.Line(comment.Pos()) uri := span.URIFromPath(tokFile.Name()) for i, line := range lines { @@ -632,7 +630,7 @@ func (r *renamer) update() (map[span.URI][]diff.Edit, error) { // docComment returns the doc for an identifier. func (r *renamer) docComment(pkg Package, id *ast.Ident) *ast.CommentGroup { - _, tokFile, nodes, _ := pathEnclosingInterval(r.fset, pkg, id.Pos(), id.End()) + _, tokFile, nodes, _ := pathEnclosingInterval(pkg, id.Pos(), id.End()) for _, node := range nodes { switch decl := node.(type) { case *ast.FuncDecl: @@ -685,7 +683,7 @@ func (r *renamer) docComment(pkg Package, id *ast.Ident) *ast.CommentGroup { func (r *renamer) updatePkgName(pkgName *types.PkgName) (*diff.Edit, error) { // Modify ImportSpec syntax to add or remove the Name as needed. pkg := r.packages[pkgName.Pkg()] - _, tokFile, path, _ := pathEnclosingInterval(r.fset, pkg, pkgName.Pos(), pkgName.Pos()) + _, tokFile, path, _ := pathEnclosingInterval(pkg, pkgName.Pos(), pkgName.Pos()) if len(path) < 2 { return nil, fmt.Errorf("no path enclosing interval for %s", pkgName.Name()) } diff --git a/gopls/internal/lsp/source/rename_check.go b/gopls/internal/lsp/source/rename_check.go index b81b6f6856..ce625dedd6 100644 --- a/gopls/internal/lsp/source/rename_check.go +++ b/gopls/internal/lsp/source/rename_check.go @@ -371,7 +371,7 @@ func (r *renamer) checkStructField(from *types.Var) { if !ok { return } - pkg, _, path, _ := pathEnclosingInterval(r.fset, fromPkg, from.Pos(), from.Pos()) + pkg, _, path, _ := pathEnclosingInterval(fromPkg, from.Pos(), from.Pos()) if pkg == nil || path == nil { return } @@ -826,7 +826,7 @@ func someUse(info *types.Info, obj types.Object) *ast.Ident { // exact is defined as for astutil.PathEnclosingInterval. // // The zero value is returned if not found. -func pathEnclosingInterval(fset *token.FileSet, pkg Package, start, end token.Pos) (resPkg Package, tokFile *token.File, path []ast.Node, exact bool) { +func pathEnclosingInterval(pkg Package, start, end token.Pos) (resPkg Package, tokFile *token.File, path []ast.Node, exact bool) { pkgs := []Package{pkg} for _, f := range pkg.GetSyntax() { for _, imp := range f.Imports { @@ -852,7 +852,7 @@ func pathEnclosingInterval(fset *token.FileSet, pkg Package, start, end token.Po // (Use parser.AllErrors to prevent that.) continue } - tokFile := fset.File(f.Pos()) + tokFile := p.FileSet().File(f.Pos()) if !tokenFileContainsPos(tokFile, start) { continue } diff --git a/gopls/internal/lsp/source/signature_help.go b/gopls/internal/lsp/source/signature_help.go index 68ac1beeb2..8c62a77961 100644 --- a/gopls/internal/lsp/source/signature_help.go +++ b/gopls/internal/lsp/source/signature_help.go @@ -94,7 +94,7 @@ FindCall: comment *ast.CommentGroup ) if obj != nil { - declPkg, err := FindPackageFromPos(snapshot.FileSet(), pkg, obj.Pos()) + declPkg, err := FindPackageFromPos(pkg, obj.Pos()) if err != nil { return nil, 0, err } diff --git a/gopls/internal/lsp/source/stub.go b/gopls/internal/lsp/source/stub.go index 8e3d8567d0..3dcdb0ec6b 100644 --- a/gopls/internal/lsp/source/stub.go +++ b/gopls/internal/lsp/source/stub.go @@ -24,22 +24,22 @@ import ( "golang.org/x/tools/internal/typeparams" ) -func stubSuggestedFixFunc(ctx context.Context, snapshot Snapshot, fh VersionedFileHandle, rng protocol.Range) (*analysis.SuggestedFix, error) { +func stubSuggestedFixFunc(ctx context.Context, snapshot Snapshot, fh VersionedFileHandle, rng protocol.Range) (*token.FileSet, *analysis.SuggestedFix, error) { pkg, pgf, err := GetParsedFile(ctx, snapshot, fh, NarrowestPackage) if err != nil { - return nil, fmt.Errorf("GetParsedFile: %w", err) + return nil, nil, fmt.Errorf("GetParsedFile: %w", err) } nodes, pos, err := getStubNodes(pgf, rng) if err != nil { - return nil, fmt.Errorf("getNodes: %w", err) + return nil, nil, fmt.Errorf("getNodes: %w", err) } si := stubmethods.GetStubInfo(pkg.GetTypesInfo(), nodes, pos) if si == nil { - return nil, fmt.Errorf("nil interface request") + return nil, nil, fmt.Errorf("nil interface request") } parsedConcreteFile, concreteFH, err := getStubFile(ctx, si.Concrete.Obj(), snapshot) if err != nil { - return nil, fmt.Errorf("getFile(concrete): %w", err) + return nil, nil, fmt.Errorf("getFile(concrete): %w", err) } var ( methodsSrc []byte @@ -51,16 +51,16 @@ func stubSuggestedFixFunc(ctx context.Context, snapshot Snapshot, fh VersionedFi methodsSrc, stubImports, err = stubMethods(ctx, parsedConcreteFile.File, si, snapshot) } if err != nil { - return nil, fmt.Errorf("stubMethods: %w", err) + return nil, nil, fmt.Errorf("stubMethods: %w", err) } nodes, _ = astutil.PathEnclosingInterval(parsedConcreteFile.File, si.Concrete.Obj().Pos(), si.Concrete.Obj().Pos()) concreteSrc, err := concreteFH.Read() if err != nil { - return nil, fmt.Errorf("error reading concrete file source: %w", err) + return nil, nil, fmt.Errorf("error reading concrete file source: %w", err) } insertPos, err := safetoken.Offset(parsedConcreteFile.Tok, nodes[1].End()) if err != nil || insertPos >= len(concreteSrc) { - return nil, fmt.Errorf("insertion position is past the end of the file") + return nil, nil, fmt.Errorf("insertion position is past the end of the file") } var buf bytes.Buffer buf.Write(concreteSrc[:insertPos]) @@ -70,7 +70,7 @@ func stubSuggestedFixFunc(ctx context.Context, snapshot Snapshot, fh VersionedFi fset := token.NewFileSet() newF, err := parser.ParseFile(fset, parsedConcreteFile.File.Name.Name, buf.Bytes(), parser.ParseComments) if err != nil { - return nil, fmt.Errorf("could not reparse file: %w", err) + return nil, nil, fmt.Errorf("could not reparse file: %w", err) } for _, imp := range stubImports { astutil.AddNamedImport(fset, newF, imp.Name, imp.Path) @@ -78,7 +78,7 @@ func stubSuggestedFixFunc(ctx context.Context, snapshot Snapshot, fh VersionedFi var source bytes.Buffer err = format.Node(&source, fset, newF) if err != nil { - return nil, fmt.Errorf("format.Node: %w", err) + return nil, nil, fmt.Errorf("format.Node: %w", err) } diffs := snapshot.View().Options().ComputeEdits(string(parsedConcreteFile.Src), source.String()) tf := parsedConcreteFile.Mapper.TokFile @@ -90,9 +90,9 @@ func stubSuggestedFixFunc(ctx context.Context, snapshot Snapshot, fh VersionedFi NewText: []byte(edit.New), }) } - return &analysis.SuggestedFix{ - TextEdits: edits, - }, nil + return snapshot.FileSet(), // from getStubFile + &analysis.SuggestedFix{TextEdits: edits}, + nil } // stubMethods returns the Go code of all methods @@ -304,6 +304,7 @@ func missingMethods(ctx context.Context, snapshot Snapshot, concMS *types.Method return missing, nil } +// Token position information for obj.Pos and the ParsedGoFile result is in Snapshot.FileSet. func getStubFile(ctx context.Context, obj types.Object, snapshot Snapshot) (*ParsedGoFile, VersionedFileHandle, error) { objPos := snapshot.FileSet().Position(obj.Pos()) objFile := span.URIFromPath(objPos.Filename) diff --git a/gopls/internal/lsp/source/types_format.go b/gopls/internal/lsp/source/types_format.go index f1c03cea65..e3a884d854 100644 --- a/gopls/internal/lsp/source/types_format.go +++ b/gopls/internal/lsp/source/types_format.go @@ -86,6 +86,7 @@ func (s *signature) Params() []string { // NewBuiltinSignature returns signature for the builtin object with a given // name, if a builtin object with the name exists. func NewBuiltinSignature(ctx context.Context, s Snapshot, name string) (*signature, error) { + fset := s.FileSet() builtin, err := s.BuiltinFile(ctx) if err != nil { return nil, err @@ -109,8 +110,8 @@ func NewBuiltinSignature(ctx context.Context, s Snapshot, name string) (*signatu variadic = true } } - params, _ := formatFieldList(ctx, s, decl.Type.Params, variadic) - results, needResultParens := formatFieldList(ctx, s, decl.Type.Results, false) + params, _ := formatFieldList(ctx, fset, decl.Type.Params, variadic) + results, needResultParens := formatFieldList(ctx, fset, decl.Type.Results, false) d := decl.Doc.Text() switch s.View().Options().HoverKind { case SynopsisDocumentation: @@ -134,7 +135,7 @@ var replacer = strings.NewReplacer( `IntegerType`, `int`, ) -func formatFieldList(ctx context.Context, snapshot Snapshot, list *ast.FieldList, variadic bool) ([]string, bool) { +func formatFieldList(ctx context.Context, fset *token.FileSet, list *ast.FieldList, variadic bool) ([]string, bool) { if list == nil { return nil, false } @@ -147,7 +148,7 @@ func formatFieldList(ctx context.Context, snapshot Snapshot, list *ast.FieldList p := list.List[i] cfg := printer.Config{Mode: printer.UseSpaces | printer.TabIndent, Tabwidth: 4} b := &bytes.Buffer{} - if err := cfg.Fprint(b, snapshot.FileSet(), p.Type); err != nil { + if err := cfg.Fprint(b, fset, p.Type); err != nil { event.Error(ctx, "unable to print type", nil, tag.Type.Of(p.Type)) continue } @@ -205,7 +206,7 @@ func NewSignature(ctx context.Context, s Snapshot, pkg Package, sig *types.Signa params := make([]string, 0, sig.Params().Len()) for i := 0; i < sig.Params().Len(); i++ { el := sig.Params().At(i) - typ := FormatVarType(s.FileSet(), pkg, el, qf) + typ := FormatVarType(pkg, el, qf) p := typ if el.Name() != "" { p = el.Name() + " " + typ @@ -220,7 +221,7 @@ func NewSignature(ctx context.Context, s Snapshot, pkg Package, sig *types.Signa needResultParens = true } el := sig.Results().At(i) - typ := FormatVarType(s.FileSet(), pkg, el, qf) + typ := FormatVarType(pkg, el, qf) if el.Name() == "" { results = append(results, typ) } else { @@ -253,8 +254,8 @@ func NewSignature(ctx context.Context, s Snapshot, pkg Package, sig *types.Signa // FormatVarType formats a *types.Var, accounting for type aliases. // To do this, it looks in the AST of the file in which the object is declared. // On any errors, it always falls back to types.TypeString. -func FormatVarType(fset *token.FileSet, srcpkg Package, obj *types.Var, qf types.Qualifier) string { - pkg, err := FindPackageFromPos(fset, srcpkg, obj.Pos()) +func FormatVarType(srcpkg Package, obj *types.Var, qf types.Qualifier) string { + pkg, err := FindPackageFromPos(srcpkg, obj.Pos()) if err != nil { return types.TypeString(obj.Type(), qf) } @@ -283,7 +284,7 @@ func FormatVarType(fset *token.FileSet, srcpkg Package, obj *types.Var, qf types // If the request came from a different package than the one in which the // types are defined, we may need to modify the qualifiers. qualified = qualifyExpr(qualified, srcpkg, pkg, clonedInfo, qf) - fmted := FormatNode(fset, qualified) + fmted := FormatNode(srcpkg.FileSet(), qualified) return fmted } diff --git a/gopls/internal/lsp/source/util.go b/gopls/internal/lsp/source/util.go index 3ad17c0a84..d7ab5c579b 100644 --- a/gopls/internal/lsp/source/util.go +++ b/gopls/internal/lsp/source/util.go @@ -125,7 +125,7 @@ func IsGenerated(ctx context.Context, snapshot Snapshot, uri span.URI) bool { return false } -func objToMappedRange(fset *token.FileSet, pkg Package, obj types.Object) (MappedRange, error) { +func objToMappedRange(pkg Package, obj types.Object) (MappedRange, error) { nameLen := len(obj.Name()) if pkgName, ok := obj.(*types.PkgName); ok { // An imported Go package has a package-local, unqualified name. @@ -142,12 +142,12 @@ func objToMappedRange(fset *token.FileSet, pkg Package, obj types.Object) (Mappe nameLen = len(pkgName.Imported().Path()) + len(`""`) } } - return posToMappedRange(fset, pkg, obj.Pos(), obj.Pos()+token.Pos(nameLen)) + return posToMappedRange(pkg, obj.Pos(), obj.Pos()+token.Pos(nameLen)) } // posToMappedRange returns the MappedRange for the given [start, end) span, // which must be among the transitive dependencies of pkg. -func posToMappedRange(fset *token.FileSet, pkg Package, pos, end token.Pos) (MappedRange, error) { +func posToMappedRange(pkg Package, pos, end token.Pos) (MappedRange, error) { if !pos.IsValid() { return MappedRange{}, fmt.Errorf("invalid start position") } @@ -155,6 +155,7 @@ func posToMappedRange(fset *token.FileSet, pkg Package, pos, end token.Pos) (Map return MappedRange{}, fmt.Errorf("invalid end position") } + fset := pkg.FileSet() tokFile := fset.File(pos) // Subtle: it is not safe to simplify this to tokFile.Name // because, due to //line directives, a Position within a @@ -183,11 +184,11 @@ func posToMappedRange(fset *token.FileSet, pkg Package, pos, end token.Pos) (Map // TODO(rfindley): is this the best factoring of this API? This function is // really a trivial wrapper around findFileInDeps, which may be a more useful // function to expose. -func FindPackageFromPos(fset *token.FileSet, pkg Package, pos token.Pos) (Package, error) { +func FindPackageFromPos(pkg Package, pos token.Pos) (Package, error) { if !pos.IsValid() { return nil, fmt.Errorf("invalid position") } - fileName := fset.File(pos).Name() + fileName := pkg.FileSet().File(pos).Name() uri := span.URIFromPath(fileName) _, pkg, err := findFileInDeps(pkg, uri) return pkg, err diff --git a/gopls/internal/lsp/source/view.go b/gopls/internal/lsp/source/view.go index be2b7f6a42..45708b3aa4 100644 --- a/gopls/internal/lsp/source/view.go +++ b/gopls/internal/lsp/source/view.go @@ -40,6 +40,11 @@ type Snapshot interface { BackgroundContext() context.Context // Fileset returns the Fileset used to parse all the Go files in this snapshot. + // + // If the files are known to belong to a specific Package, use + // Package.FileSet instead. (We plan to eliminate the + // Snapshot's cache of parsed files, and thus the need for a + // snapshot-wide FileSet.) FileSet() *token.FileSet // ValidBuildConfiguration returns true if there is some error in the @@ -610,6 +615,7 @@ type Package interface { PkgPath() PackagePath CompiledGoFiles() []*ParsedGoFile File(uri span.URI) (*ParsedGoFile, error) + FileSet() *token.FileSet GetSyntax() []*ast.File GetTypes() *types.Package GetTypesInfo() *types.Info