Skip to content

Commit 13648cd

Browse files
adonovanadonovan
committed
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 <[email protected]> TryBot-Result: Gopher Robot <[email protected]> gopls-CI: kokoro <[email protected]> Reviewed-by: Robert Findley <[email protected]>
1 parent 8f32411 commit 13648cd

31 files changed

+144
-123
lines changed

‎gopls/internal/lsp/cache/analysis.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -286,7 +286,7 @@ func actionImpl(ctx context.Context, snapshot *snapshot, deps []*actionHandle, a
286286
var rawDiagnostics []analysis.Diagnostic
287287
pass := &analysis.Pass{
288288
Analyzer: analyzer,
289-
Fset: snapshot.FileSet(),
289+
Fset: pkg.FileSet(),
290290
Files: syntax,
291291
Pkg: pkg.GetTypes(),
292292
TypesInfo: pkg.GetTypesInfo(),

‎gopls/internal/lsp/cache/check.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -450,6 +450,7 @@ func doTypeCheck(ctx context.Context, snapshot *snapshot, goFiles, compiledGoFil
450450
pkg := &pkg{
451451
m: m,
452452
mode: mode,
453+
fset: snapshot.FileSet(), // must match parse call below (snapshot.ParseGo for now)
453454
deps: make(map[PackageID]*pkg),
454455
types: types.NewPackage(string(m.PkgPath), string(m.Name)),
455456
typesInfo: &types.Info{
@@ -565,7 +566,7 @@ func doTypeCheck(ctx context.Context, snapshot *snapshot, goFiles, compiledGoFil
565566
// We passed typecheckCgo to go/packages when we Loaded.
566567
typesinternal.SetUsesCgo(cfg)
567568

568-
check := types.NewChecker(cfg, snapshot.FileSet(), pkg.types, pkg.typesInfo)
569+
check := types.NewChecker(cfg, pkg.fset, pkg.types, pkg.typesInfo)
569570

570571
var files []*ast.File
571572
for _, cgf := range pkg.compiledGoFiles {
@@ -593,7 +594,7 @@ func parseCompiledGoFiles(ctx context.Context, compiledGoFiles []source.FileHand
593594
if mode == source.ParseFull {
594595
pgf, err = snapshot.ParseGo(ctx, fh, mode)
595596
} else {
596-
pgf, err = parseGoImpl(ctx, snapshot.FileSet(), fh, mode) // ~20us/KB
597+
pgf, err = parseGoImpl(ctx, pkg.fset, fh, mode) // ~20us/KB
597598
}
598599
if err != nil {
599600
return err
@@ -661,6 +662,7 @@ func (s *snapshot) depsErrors(ctx context.Context, pkg *pkg) ([]*source.Diagnost
661662
}
662663
allImports := map[string][]fileImport{}
663664
for _, cgf := range pkg.compiledGoFiles {
665+
// TODO(adonovan): modify Imports() to accept a single token.File (cgf.Tok).
664666
for _, group := range astutil.Imports(s.FileSet(), cgf.File) {
665667
for _, imp := range group {
666668
if imp.Path == nil {

‎gopls/internal/lsp/cache/errors.go

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ var importErrorRe = regexp.MustCompile(`could not import ([^\s]+)`)
104104
var unsupportedFeatureRe = regexp.MustCompile(`.*require.* go(\d+\.\d+) or later`)
105105

106106
func typeErrorDiagnostics(snapshot *snapshot, pkg *pkg, e extendedError) ([]*source.Diagnostic, error) {
107-
code, spn, err := typeErrorData(snapshot.FileSet(), pkg, e.primary)
107+
code, spn, err := typeErrorData(pkg, e.primary)
108108
if err != nil {
109109
return nil, err
110110
}
@@ -129,7 +129,7 @@ func typeErrorDiagnostics(snapshot *snapshot, pkg *pkg, e extendedError) ([]*sou
129129
}
130130

131131
for _, secondary := range e.secondaries {
132-
_, secondarySpan, err := typeErrorData(snapshot.FileSet(), pkg, secondary)
132+
_, secondarySpan, err := typeErrorData(pkg, secondary)
133133
if err != nil {
134134
return nil, err
135135
}
@@ -201,7 +201,7 @@ func analysisDiagnosticDiagnostics(snapshot *snapshot, pkg *pkg, a *analysis.Ana
201201
break
202202
}
203203
}
204-
tokFile := snapshot.FileSet().File(e.Pos)
204+
tokFile := pkg.fset.File(e.Pos)
205205
if tokFile == nil {
206206
return nil, bug.Errorf("no file for position of %q diagnostic", e.Category)
207207
}
@@ -221,7 +221,7 @@ func analysisDiagnosticDiagnostics(snapshot *snapshot, pkg *pkg, a *analysis.Ana
221221
if len(srcAnalyzer.ActionKind) == 0 {
222222
kinds = append(kinds, protocol.QuickFix)
223223
}
224-
fixes, err := suggestedAnalysisFixes(snapshot, pkg, e, kinds)
224+
fixes, err := suggestedAnalysisFixes(pkg, e, kinds)
225225
if err != nil {
226226
return nil, err
227227
}
@@ -238,7 +238,7 @@ func analysisDiagnosticDiagnostics(snapshot *snapshot, pkg *pkg, a *analysis.Ana
238238
fixes = append(fixes, source.SuggestedFixFromCommand(cmd, kind))
239239
}
240240
}
241-
related, err := relatedInformation(pkg, snapshot.FileSet(), e)
241+
related, err := relatedInformation(pkg, e)
242242
if err != nil {
243243
return nil, err
244244
}
@@ -289,12 +289,12 @@ func typesCodeHref(snapshot *snapshot, code typesinternal.ErrorCode) string {
289289
return source.BuildLink(target, "golang.org/x/tools/internal/typesinternal", code.String())
290290
}
291291

292-
func suggestedAnalysisFixes(snapshot *snapshot, pkg *pkg, diag *analysis.Diagnostic, kinds []protocol.CodeActionKind) ([]source.SuggestedFix, error) {
292+
func suggestedAnalysisFixes(pkg *pkg, diag *analysis.Diagnostic, kinds []protocol.CodeActionKind) ([]source.SuggestedFix, error) {
293293
var fixes []source.SuggestedFix
294294
for _, fix := range diag.SuggestedFixes {
295295
edits := make(map[span.URI][]protocol.TextEdit)
296296
for _, e := range fix.TextEdits {
297-
tokFile := snapshot.FileSet().File(e.Pos)
297+
tokFile := pkg.fset.File(e.Pos)
298298
if tokFile == nil {
299299
return nil, bug.Errorf("no file for edit position")
300300
}
@@ -327,10 +327,10 @@ func suggestedAnalysisFixes(snapshot *snapshot, pkg *pkg, diag *analysis.Diagnos
327327
return fixes, nil
328328
}
329329

330-
func relatedInformation(pkg *pkg, fset *token.FileSet, diag *analysis.Diagnostic) ([]source.RelatedInformation, error) {
330+
func relatedInformation(pkg *pkg, diag *analysis.Diagnostic) ([]source.RelatedInformation, error) {
331331
var out []source.RelatedInformation
332332
for _, related := range diag.Related {
333-
tokFile := fset.File(related.Pos)
333+
tokFile := pkg.fset.File(related.Pos)
334334
if tokFile == nil {
335335
return nil, bug.Errorf("no file for %q diagnostic position", diag.Category)
336336
}
@@ -355,7 +355,7 @@ func relatedInformation(pkg *pkg, fset *token.FileSet, diag *analysis.Diagnostic
355355
return out, nil
356356
}
357357

358-
func typeErrorData(fset *token.FileSet, pkg *pkg, terr types.Error) (typesinternal.ErrorCode, span.Span, error) {
358+
func typeErrorData(pkg *pkg, terr types.Error) (typesinternal.ErrorCode, span.Span, error) {
359359
ecode, start, end, ok := typesinternal.ReadGo116ErrorData(terr)
360360
if !ok {
361361
start, end = terr.Pos, terr.Pos
@@ -368,6 +368,7 @@ func typeErrorData(fset *token.FileSet, pkg *pkg, terr types.Error) (typesintern
368368
}
369369
// go/types errors retain their FileSet.
370370
// Sanity-check that we're using the right one.
371+
fset := pkg.FileSet()
371372
if fset != terr.Fset {
372373
return 0, span.Span{}, bug.Errorf("wrong FileSet for type error")
373374
}

‎gopls/internal/lsp/cache/load.go

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,6 @@ func (s *snapshot) load(ctx context.Context, allowNetwork bool, scopes ...loadSc
5050
for _, scope := range scopes {
5151
switch scope := scope.(type) {
5252
case packageLoadScope:
53-
// TODO(adonovan): is this cast sound?? A
54-
// packageLoadScope is really a PackagePath I think.
55-
if source.IsCommandLineArguments(PackageID(scope)) {
56-
panic("attempted to load command-line-arguments")
57-
}
5853
// The only time we pass package paths is when we're doing a
5954
// partial workspace load. In those cases, the paths came back from
6055
// go list and should already be GOPATH-vendorized when appropriate.

‎gopls/internal/lsp/cache/parse.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@ type parseKey struct {
3636
// ParseGo parses the file whose contents are provided by fh, using a cache.
3737
// The resulting tree may have be fixed up.
3838
//
39+
// Token position information will be added to the snapshot's FileSet.
40+
//
3941
// The parser mode must not be ParseExported: that mode is used during
4042
// type checking to destructively trim the tree to reduce work,
4143
// which is not safe for values from a shared cache.

‎gopls/internal/lsp/cache/pkg.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"fmt"
99
"go/ast"
1010
"go/scanner"
11+
"go/token"
1112
"go/types"
1213
"sort"
1314

@@ -21,6 +22,7 @@ import (
2122
type pkg struct {
2223
m *Metadata
2324
mode source.ParseMode
25+
fset *token.FileSet // for now, same as the snapshot's FileSet
2426
goFiles []*source.ParsedGoFile
2527
compiledGoFiles []*source.ParsedGoFile
2628
diagnostics []*source.Diagnostic
@@ -45,7 +47,7 @@ type loadScope interface {
4547

4648
type (
4749
fileLoadScope span.URI // load packages containing a file (including command-line-arguments)
48-
packageLoadScope string // load a specific package
50+
packageLoadScope string // load a specific package (the value is its PackageID)
4951
moduleLoadScope string // load packages in a specific module
5052
viewLoadScope span.URI // load the workspace
5153
)
@@ -90,6 +92,10 @@ func (p *pkg) GetSyntax() []*ast.File {
9092
return syntax
9193
}
9294

95+
func (p *pkg) FileSet() *token.FileSet {
96+
return p.fset
97+
}
98+
9399
func (p *pkg) GetTypes() *types.Package {
94100
return p.types
95101
}

‎gopls/internal/lsp/command.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -757,7 +757,7 @@ func (c *commandHandler) ListImports(ctx context.Context, args command.URIArg) (
757757
if err != nil {
758758
return err
759759
}
760-
for _, group := range astutil.Imports(deps.snapshot.FileSet(), pgf.File) {
760+
for _, group := range astutil.Imports(pkg.FileSet(), pgf.File) {
761761
for _, imp := range group {
762762
if imp.Path == nil {
763763
continue

‎gopls/internal/lsp/completion.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,14 @@ import (
99
"fmt"
1010
"strings"
1111

12-
"golang.org/x/tools/internal/event"
13-
"golang.org/x/tools/internal/event/tag"
1412
"golang.org/x/tools/gopls/internal/lsp/lsppos"
1513
"golang.org/x/tools/gopls/internal/lsp/protocol"
1614
"golang.org/x/tools/gopls/internal/lsp/source"
1715
"golang.org/x/tools/gopls/internal/lsp/source/completion"
1816
"golang.org/x/tools/gopls/internal/lsp/template"
1917
"golang.org/x/tools/gopls/internal/lsp/work"
18+
"golang.org/x/tools/internal/event"
19+
"golang.org/x/tools/internal/event/tag"
2020
)
2121

2222
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
6464
if err != nil {
6565
return nil, err
6666
}
67-
tf := snapshot.FileSet().File(surrounding.Start())
68-
mapper := lsppos.NewTokenMapper(src, tf)
69-
rng, err := mapper.Range(surrounding.Start(), surrounding.End())
67+
srng := surrounding.Range()
68+
tf := snapshot.FileSet().File(srng.Start) // not same as srng.TokFile due to //line
69+
rng, err := lsppos.NewTokenMapper(src, tf).Range(srng.Start, srng.End)
7070
if err != nil {
7171
return nil, err
7272
}

‎gopls/internal/lsp/semantic.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ func (s *Server) computeSemanticTokens(ctx context.Context, td protocol.TextDocu
112112
rng: rng,
113113
ti: pkg.GetTypesInfo(),
114114
pkg: pkg,
115-
fset: snapshot.FileSet(),
115+
fset: pkg.FileSet(),
116116
tokTypes: s.session.Options().SemanticTypes,
117117
tokMods: s.session.Options().SemanticMods,
118118
noStrings: vv.Options().NoSemanticString,

‎gopls/internal/lsp/source/call_hierarchy.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ func OutgoingCalls(ctx context.Context, snapshot Snapshot, fh FileHandle, pos pr
205205
// TODO(adonovan): avoid Fileset.File call by somehow getting at
206206
// declMappedRange.spanRange.TokFile, or making Identifier retain the
207207
// token.File of the identifier and its declaration, since it looks up both anyway.
208-
tokFile := snapshot.FileSet().File(node.Pos())
208+
tokFile := identifier.pkg.FileSet().File(node.Pos())
209209
if tokFile == nil {
210210
return nil, fmt.Errorf("no file for position")
211211
}

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

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -289,20 +289,16 @@ type completionContext struct {
289289
// A Selection represents the cursor position and surrounding identifier.
290290
type Selection struct {
291291
content string
292-
cursor token.Pos
292+
cursor token.Pos // relative to rng.TokFile
293293
rng span.Range
294294
}
295295

296296
func (p Selection) Content() string {
297297
return p.content
298298
}
299299

300-
func (p Selection) Start() token.Pos {
301-
return p.rng.Start
302-
}
303-
304-
func (p Selection) End() token.Pos {
305-
return p.rng.End
300+
func (p Selection) Range() span.Range {
301+
return p.rng
306302
}
307303

308304
func (p Selection) Prefix() string {
@@ -693,7 +689,7 @@ func (c *completer) containingIdent(src []byte) *ast.Ident {
693689

694690
// scanToken scans pgh's contents for the token containing pos.
695691
func (c *completer) scanToken(contents []byte) (token.Pos, token.Token, string) {
696-
tok := c.snapshot.FileSet().File(c.pos)
692+
tok := c.pkg.FileSet().File(c.pos)
697693

698694
var s scanner.Scanner
699695
s.Init(tok, contents, nil, 0)
@@ -879,7 +875,7 @@ func (c *completer) populateCommentCompletions(ctx context.Context, comment *ast
879875
}
880876

881877
// Using the comment position find the line after
882-
file := c.snapshot.FileSet().File(comment.End())
878+
file := c.pkg.FileSet().File(comment.End())
883879
if file == nil {
884880
return
885881
}
@@ -1246,7 +1242,7 @@ func (c *completer) methodsAndFields(typ types.Type, addressable bool, imp *impo
12461242

12471243
if isStarTestingDotF(typ) && addressable {
12481244
// is that a sufficient test? (or is more care needed?)
1249-
if c.fuzz(typ, mset, imp, cb, c.snapshot.FileSet()) {
1245+
if c.fuzz(typ, mset, imp, cb, c.pkg.FileSet()) {
12501246
return
12511247
}
12521248
}
@@ -1331,7 +1327,7 @@ func (c *completer) lexical(ctx context.Context) error {
13311327
node = c.path[i-1]
13321328
}
13331329
if node != nil {
1334-
if resolved := resolveInvalid(c.snapshot.FileSet(), obj, node, c.pkg.GetTypesInfo()); resolved != nil {
1330+
if resolved := resolveInvalid(c.pkg.FileSet(), obj, node, c.pkg.GetTypesInfo()); resolved != nil {
13351331
obj = resolved
13361332
}
13371333
}
@@ -2033,7 +2029,7 @@ Nodes:
20332029
//
20342030
// TODO: remove this after https://go.dev/issue/52503
20352031
info := &types.Info{Types: make(map[ast.Expr]types.TypeAndValue)}
2036-
types.CheckExpr(c.snapshot.FileSet(), c.pkg.GetTypes(), node.Fun.Pos(), node.Fun, info)
2032+
types.CheckExpr(c.pkg.FileSet(), c.pkg.GetTypes(), node.Fun.Pos(), node.Fun, info)
20372033
sig, _ = info.Types[node.Fun].Type.(*types.Signature)
20382034
}
20392035

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ func (c *completer) item(ctx context.Context, cand candidate) (CompletionItem, e
8080
if _, ok := obj.Type().(*types.Struct); ok {
8181
detail = "struct{...}" // for anonymous structs
8282
} else if obj.IsField() {
83-
detail = source.FormatVarType(c.snapshot.FileSet(), c.pkg, obj, c.qf)
83+
detail = source.FormatVarType(c.pkg, obj, c.qf)
8484
}
8585
if obj.IsField() {
8686
kind = protocol.FieldCompletion
@@ -227,7 +227,7 @@ Suffixes:
227227
if !c.opts.documentation {
228228
return item, nil
229229
}
230-
pos := c.snapshot.FileSet().Position(obj.Pos())
230+
pos := c.pkg.FileSet().Position(obj.Pos())
231231

232232
// We ignore errors here, because some types, like "unsafe" or "error",
233233
// may not have valid positions that we can use to get documentation.
@@ -237,7 +237,7 @@ Suffixes:
237237
uri := span.URIFromPath(pos.Filename)
238238

239239
// Find the source file of the candidate.
240-
pkg, err := source.FindPackageFromPos(c.snapshot.FileSet(), c.pkg, obj.Pos())
240+
pkg, err := source.FindPackageFromPos(c.pkg, obj.Pos())
241241
if err != nil {
242242
return item, nil
243243
}

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ func (c *completer) functionLiteral(sig *types.Signature, matchScore float64) {
202202
// If the param has no name in the signature, guess a name based
203203
// on the type. Use an empty qualifier to ignore the package.
204204
// For example, we want to name "http.Request" "r", not "hr".
205-
name = source.FormatVarType(c.snapshot.FileSet(), c.pkg, p, func(p *types.Package) string {
205+
name = source.FormatVarType(c.pkg, p, func(p *types.Package) string {
206206
return ""
207207
})
208208
name = abbreviateTypeName(name)
@@ -264,7 +264,7 @@ func (c *completer) functionLiteral(sig *types.Signature, matchScore float64) {
264264
// of "i int, j int".
265265
if i == sig.Params().Len()-1 || !types.Identical(p.Type(), sig.Params().At(i+1).Type()) {
266266
snip.WriteText(" ")
267-
typeStr := source.FormatVarType(c.snapshot.FileSet(), c.pkg, p, c.qf)
267+
typeStr := source.FormatVarType(c.pkg, p, c.qf)
268268
if sig.Variadic() && i == sig.Params().Len()-1 {
269269
typeStr = strings.Replace(typeStr, "[]", "...", 1)
270270
}
@@ -314,7 +314,7 @@ func (c *completer) functionLiteral(sig *types.Signature, matchScore float64) {
314314
snip.WriteText(name + " ")
315315
}
316316

317-
text := source.FormatVarType(c.snapshot.FileSet(), c.pkg, r, c.qf)
317+
text := source.FormatVarType(c.pkg, r, c.qf)
318318
if tp, _ := r.Type().(*typeparams.TypeParam); tp != nil && !c.typeParamInScope(tp) {
319319
snip.WritePlaceholder(func(snip *snippet.Builder) {
320320
snip.WriteText(text)

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import (
3131
func packageClauseCompletions(ctx context.Context, snapshot source.Snapshot, fh source.FileHandle, position protocol.Position) ([]CompletionItem, *Selection, error) {
3232
// We know that the AST for this file will be empty due to the missing
3333
// package declaration, but parse it anyway to get a mapper.
34+
fset := snapshot.FileSet()
3435
pgf, err := snapshot.ParseGo(ctx, fh, source.ParseFull)
3536
if err != nil {
3637
return nil, nil, err
@@ -41,7 +42,7 @@ func packageClauseCompletions(ctx context.Context, snapshot source.Snapshot, fh
4142
return nil, nil, err
4243
}
4344

44-
surrounding, err := packageCompletionSurrounding(snapshot.FileSet(), pgf, pos)
45+
surrounding, err := packageCompletionSurrounding(fset, pgf, pos)
4546
if err != nil {
4647
return nil, nil, fmt.Errorf("invalid position for package completion: %w", err)
4748
}

0 commit comments

Comments
 (0)