Skip to content

Commit 2208e16

Browse files
stamblerrestamblerre
committed
internal/lsp: eliminate source.File type and move GetFile to snapshot
This change eliminates the extra step of calling GetFile on the view and getting the FileHandle from the snapshot. It also eliminiates the redundant source.File type. Follow up changes will clean up the file kind handling, since it still exists on the fileBase type. Change-Id: I635ab8632821b36e062be5151eaab425a5698f60 Reviewed-on: https://go-review.googlesource.com/c/tools/+/211778 Run-TryBot: Rebecca Stambler <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Heschi Kreinick <[email protected]>
1 parent 56b0b28 commit 2208e16

35 files changed

+239
-274
lines changed

‎internal/lsp/cache/check.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -222,11 +222,10 @@ func (ph *packageHandle) cached() (*pkg, error) {
222222
func (s *snapshot) parseGoHandles(ctx context.Context, files []span.URI, mode source.ParseMode) ([]source.ParseGoHandle, error) {
223223
phs := make([]source.ParseGoHandle, 0, len(files))
224224
for _, uri := range files {
225-
f, err := s.view.GetFile(ctx, uri)
225+
fh, err := s.GetFile(ctx, uri)
226226
if err != nil {
227227
return nil, err
228228
}
229-
fh := s.Handle(ctx, f)
230229
phs = append(phs, s.view.session.cache.ParseGoHandle(fh, mode))
231230
}
232231
return phs, nil

‎internal/lsp/cache/file.go

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,6 @@ import (
1313
"golang.org/x/tools/internal/span"
1414
)
1515

16-
// viewFile extends source.File with helper methods for the view package.
17-
type viewFile interface {
18-
source.File
19-
20-
filename() string
21-
addURI(uri span.URI) int
22-
}
23-
2416
// fileBase holds the common functionality for all files.
2517
// It is intended to be embedded in the file implementations
2618
type fileBase struct {
@@ -43,10 +35,6 @@ func (f *fileBase) URI() span.URI {
4335
return f.uris[0]
4436
}
4537

46-
func (f *fileBase) Kind() source.FileKind {
47-
return f.kind
48-
}
49-
5038
func (f *fileBase) filename() string {
5139
return f.fname
5240
}

‎internal/lsp/cache/session.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -94,8 +94,8 @@ func (s *session) createView(ctx context.Context, name string, folder span.URI,
9494
name: name,
9595
modfiles: modfiles,
9696
folder: folder,
97-
filesByURI: make(map[span.URI]viewFile),
98-
filesByBase: make(map[string][]viewFile),
97+
filesByURI: make(map[span.URI]*fileBase),
98+
filesByBase: make(map[string][]*fileBase),
9999
snapshot: &snapshot{
100100
packages: make(map[packageKey]*packageHandle),
101101
ids: make(map[span.URI][]packageID),
@@ -301,11 +301,11 @@ func (s *session) DidModifyFile(ctx context.Context, c source.FileModification)
301301
return nil, errors.Errorf("ignored file %v", c.URI)
302302
}
303303
// Set the content for the file, only for didChange and didClose events.
304-
f, err := view.GetFile(ctx, c.URI)
304+
f, err := view.getFileLocked(ctx, c.URI)
305305
if err != nil {
306306
return nil, err
307307
}
308-
snapshots = append(snapshots, view.invalidateContent(ctx, f, c.Action))
308+
snapshots = append(snapshots, view.invalidateContent(ctx, c.URI, f.kind, c.Action))
309309
}
310310
return snapshots, nil
311311
}
@@ -328,10 +328,10 @@ func (s *session) DidChangeOutOfBand(ctx context.Context, uri span.URI, action s
328328
if err != nil {
329329
return false
330330
}
331-
f, err := view.GetFile(ctx, uri)
331+
f, err := view.getFileLocked(ctx, uri)
332332
if err != nil {
333333
return false
334334
}
335-
view.invalidateContent(ctx, f, action)
335+
view.invalidateContent(ctx, f.URI(), f.kind, action)
336336
return true
337337
}

‎internal/lsp/cache/snapshot.go

Lines changed: 34 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -447,51 +447,71 @@ func (s *snapshot) getFileURIs() []span.URI {
447447
return uris
448448
}
449449

450-
func (s *snapshot) getFile(uri span.URI) source.FileHandle {
451-
s.mu.Lock()
452-
defer s.mu.Unlock()
450+
// FindFile returns the file if the given URI is already a part of the view.
451+
func (s *snapshot) FindFile(ctx context.Context, uri span.URI) source.FileHandle {
452+
s.view.mu.Lock()
453+
defer s.view.mu.Unlock()
453454

454-
return s.files[uri]
455+
f, err := s.view.findFile(uri)
456+
if f == nil || err != nil {
457+
return nil
458+
}
459+
return s.getFileHandle(ctx, f)
460+
}
461+
462+
// GetFile returns a File for the given URI. It will always succeed because it
463+
// adds the file to the managed set if needed.
464+
func (s *snapshot) GetFile(ctx context.Context, uri span.URI) (source.FileHandle, error) {
465+
s.view.mu.Lock()
466+
defer s.view.mu.Unlock()
467+
468+
// TODO(rstambler): Should there be a version that provides a kind explicitly?
469+
kind := source.DetectLanguage("", uri.Filename())
470+
f, err := s.view.getFile(ctx, uri, kind)
471+
if err != nil {
472+
return nil, err
473+
}
474+
return s.getFileHandle(ctx, f), nil
455475
}
456476

457-
func (s *snapshot) Handle(ctx context.Context, f source.File) source.FileHandle {
477+
func (s *snapshot) getFileHandle(ctx context.Context, f *fileBase) source.FileHandle {
458478
s.mu.Lock()
459479
defer s.mu.Unlock()
460480

461481
if _, ok := s.files[f.URI()]; !ok {
462-
s.files[f.URI()] = s.view.session.GetFile(f.URI(), f.Kind())
482+
s.files[f.URI()] = s.view.session.GetFile(f.URI(), f.kind)
463483
}
464484
return s.files[f.URI()]
465485
}
466486

467-
func (s *snapshot) clone(ctx context.Context, withoutFile source.File) *snapshot {
487+
func (s *snapshot) clone(ctx context.Context, withoutURI span.URI, withoutFileKind source.FileKind) *snapshot {
468488
s.mu.Lock()
469489
defer s.mu.Unlock()
470490

471491
directIDs := map[packageID]struct{}{}
472492
// Collect all of the package IDs that correspond to the given file.
473493
// TODO: if the file has moved into a new package, we should invalidate that too.
474-
for _, id := range s.ids[withoutFile.URI()] {
494+
for _, id := range s.ids[withoutURI] {
475495
directIDs[id] = struct{}{}
476496
}
477497

478498
// If we are invalidating a go.mod file then we should invalidate all of the packages in the module
479-
if withoutFile.Kind() == source.Mod {
499+
if withoutFileKind == source.Mod {
480500
for id := range s.workspacePackages {
481501
directIDs[id] = struct{}{}
482502
}
483503
}
484504

485505
// Get the original FileHandle for the URI, if it exists.
486-
originalFH := s.files[withoutFile.URI()]
506+
originalFH := s.files[withoutURI]
487507

488508
// If this is a file we don't yet know about,
489509
// then we do not yet know what packages it should belong to.
490510
// Make a rough estimate of what metadata to invalidate by finding the package IDs
491511
// of all of the files in the same directory as this one.
492512
// TODO(rstambler): Speed this up by mapping directories to filenames.
493513
if originalFH == nil {
494-
if dirStat, err := os.Stat(dir(withoutFile.URI().Filename())); err == nil {
514+
if dirStat, err := os.Stat(dir(withoutURI.Filename())); err == nil {
495515
for uri := range s.files {
496516
if fdirStat, err := os.Stat(dir(uri.Filename())); err == nil {
497517
if os.SameFile(dirStat, fdirStat) {
@@ -546,11 +566,11 @@ func (s *snapshot) clone(ctx context.Context, withoutFile source.File) *snapshot
546566
result.files[k] = v
547567
}
548568
// Handle the invalidated file; it may have new contents or not exist.
549-
currentFH := s.view.session.GetFile(withoutFile.URI(), withoutFile.Kind())
569+
currentFH := s.view.session.GetFile(withoutURI, withoutFileKind)
550570
if _, _, err := currentFH.Read(ctx); os.IsNotExist(err) {
551-
delete(result.files, withoutFile.URI())
571+
delete(result.files, withoutURI)
552572
} else {
553-
result.files[withoutFile.URI()] = currentFH
573+
result.files[withoutURI] = currentFH
554574
}
555575

556576
// Collect the IDs for the packages associated with the excluded URIs.

‎internal/lsp/cache/view.go

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -75,8 +75,8 @@ type view struct {
7575

7676
// keep track of files by uri and by basename, a single file may be mapped
7777
// to multiple uris, and the same basename may map to multiple files
78-
filesByURI map[span.URI]viewFile
79-
filesByBase map[string][]viewFile
78+
filesByURI map[span.URI]*fileBase
79+
filesByBase map[string][]*fileBase
8080

8181
snapshotMu sync.Mutex
8282
snapshot *snapshot
@@ -348,7 +348,7 @@ func (v *view) getSnapshot() *snapshot {
348348
// invalidateContent invalidates the content of a Go file,
349349
// including any position and type information that depends on it.
350350
// It returns true if we were already tracking the given file, false otherwise.
351-
func (v *view) invalidateContent(ctx context.Context, f source.File, action source.FileAction) source.Snapshot {
351+
func (v *view) invalidateContent(ctx context.Context, uri span.URI, kind source.FileKind, action source.FileAction) source.Snapshot {
352352
// Cancel all still-running previous requests, since they would be
353353
// operating on stale data.
354354
switch action {
@@ -360,7 +360,7 @@ func (v *view) invalidateContent(ctx context.Context, f source.File, action sour
360360
v.snapshotMu.Lock()
361361
defer v.snapshotMu.Unlock()
362362

363-
v.snapshot = v.snapshot.clone(ctx, f)
363+
v.snapshot = v.snapshot.clone(ctx, uri, kind)
364364
return v.snapshot
365365
}
366366

@@ -373,19 +373,20 @@ func (v *view) cancelBackground() {
373373
}
374374

375375
// FindFile returns the file if the given URI is already a part of the view.
376-
func (v *view) FindFile(ctx context.Context, uri span.URI) source.File {
376+
func (v *view) findFileLocked(ctx context.Context, uri span.URI) *fileBase {
377377
v.mu.Lock()
378378
defer v.mu.Unlock()
379+
379380
f, err := v.findFile(uri)
380381
if err != nil {
381382
return nil
382383
}
383384
return f
384385
}
385386

386-
// GetFile returns a File for the given URI. It will always succeed because it
387+
// getFileLocked returns a File for the given URI. It will always succeed because it
387388
// adds the file to the managed set if needed.
388-
func (v *view) GetFile(ctx context.Context, uri span.URI) (source.File, error) {
389+
func (v *view) getFileLocked(ctx context.Context, uri span.URI) (*fileBase, error) {
389390
v.mu.Lock()
390391
defer v.mu.Unlock()
391392

@@ -395,7 +396,7 @@ func (v *view) GetFile(ctx context.Context, uri span.URI) (source.File, error) {
395396
}
396397

397398
// getFile is the unlocked internal implementation of GetFile.
398-
func (v *view) getFile(ctx context.Context, uri span.URI, kind source.FileKind) (viewFile, error) {
399+
func (v *view) getFile(ctx context.Context, uri span.URI, kind source.FileKind) (*fileBase, error) {
399400
f, err := v.findFile(uri)
400401
if err != nil {
401402
return nil, err
@@ -415,7 +416,7 @@ func (v *view) getFile(ctx context.Context, uri span.URI, kind source.FileKind)
415416
//
416417
// An error is only returned for an irreparable failure, for example, if the
417418
// filename in question does not exist.
418-
func (v *view) findFile(uri span.URI) (viewFile, error) {
419+
func (v *view) findFile(uri span.URI) (*fileBase, error) {
419420
if f := v.filesByURI[uri]; f != nil {
420421
// a perfect match
421422
return f, nil
@@ -451,7 +452,7 @@ func (f *fileBase) addURI(uri span.URI) int {
451452
return len(f.uris)
452453
}
453454

454-
func (v *view) mapFile(uri span.URI, f viewFile) {
455+
func (v *view) mapFile(uri span.URI, f *fileBase) {
455456
v.filesByURI[uri] = f
456457
if f.addURI(uri) == 1 {
457458
basename := basename(f.filename())

‎internal/lsp/code_action.go

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,11 @@ func (s *Server) codeAction(ctx context.Context, params *protocol.CodeActionPara
2525
if err != nil {
2626
return nil, err
2727
}
28-
f, err := view.GetFile(ctx, uri)
28+
snapshot := view.Snapshot()
29+
fh, err := snapshot.GetFile(ctx, uri)
2930
if err != nil {
3031
return nil, err
3132
}
32-
snapshot := view.Snapshot()
33-
fh := snapshot.Handle(ctx, f)
3433

3534
// Determine the supported actions for this file kind.
3635
supportedCodeActions, ok := view.Options().SupportedCodeActions[fh.Identity().Kind]
@@ -63,21 +62,19 @@ func (s *Server) codeAction(ctx context.Context, params *protocol.CodeActionPara
6362
Title: "Tidy",
6463
Kind: protocol.SourceOrganizeImports,
6564
Command: &protocol.Command{
66-
Title: "Tidy",
67-
Command: "tidy",
68-
Arguments: []interface{}{
69-
f.URI(),
70-
},
65+
Title: "Tidy",
66+
Command: "tidy",
67+
Arguments: []interface{}{fh.Identity().URI},
7168
},
7269
})
7370
case source.Go:
74-
edits, editsPerFix, err := source.AllImportsFixes(ctx, snapshot, f)
71+
edits, editsPerFix, err := source.AllImportsFixes(ctx, snapshot, fh)
7572
if err != nil {
7673
return nil, err
7774
}
7875
if diagnostics := params.Context.Diagnostics; wanted[protocol.QuickFix] && len(diagnostics) > 0 {
7976
// First, add the quick fixes reported by go/analysis.
80-
qf, err := quickFixes(ctx, snapshot, f, diagnostics)
77+
qf, err := quickFixes(ctx, snapshot, fh, diagnostics)
8178
if err != nil {
8279
log.Error(ctx, "quick fixes failed", err, telemetry.File.Of(uri))
8380
}
@@ -203,10 +200,9 @@ func importDiagnostics(fix *imports.ImportFix, diagnostics []protocol.Diagnostic
203200
return results
204201
}
205202

206-
func quickFixes(ctx context.Context, snapshot source.Snapshot, f source.File, diagnostics []protocol.Diagnostic) ([]protocol.CodeAction, error) {
203+
func quickFixes(ctx context.Context, snapshot source.Snapshot, fh source.FileHandle, diagnostics []protocol.Diagnostic) ([]protocol.CodeAction, error) {
207204
var codeActions []protocol.CodeAction
208205

209-
fh := snapshot.Handle(ctx, f)
210206
phs, err := snapshot.PackageHandles(ctx, fh)
211207
if err != nil {
212208
return nil, err
@@ -232,12 +228,11 @@ func quickFixes(ctx context.Context, snapshot source.Snapshot, f source.File, di
232228
Edit: protocol.WorkspaceEdit{},
233229
}
234230
for uri, edits := range fix.Edits {
235-
f, err := snapshot.View().GetFile(ctx, uri)
231+
fh, err := snapshot.GetFile(ctx, uri)
236232
if err != nil {
237233
log.Error(ctx, "no file", err, telemetry.URI.Of(uri))
238234
continue
239235
}
240-
fh := snapshot.Handle(ctx, f)
241236
action.Edit.DocumentChanges = append(action.Edit.DocumentChanges, documentChanges(fh, edits)...)
242237
}
243238
codeActions = append(codeActions, action)

‎internal/lsp/command.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,10 @@ func (s *Server) executeCommand(ctx context.Context, params *protocol.ExecuteCom
2121
if err != nil {
2222
return nil, err
2323
}
24-
f, err := view.GetFile(ctx, uri)
24+
fh, err := view.Snapshot().GetFile(ctx, uri)
2525
if err != nil {
2626
return nil, err
2727
}
28-
fh := view.Snapshot().Handle(ctx, f)
2928
if fh.Identity().Kind != source.Mod {
3029
return nil, errors.Errorf("%s is not a mod file", uri)
3130
}

‎internal/lsp/completion.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,17 +24,16 @@ func (s *Server) completion(ctx context.Context, params *protocol.CompletionPara
2424
}
2525
snapshot := view.Snapshot()
2626
options := view.Options()
27-
f, err := view.GetFile(ctx, uri)
27+
fh, err := snapshot.GetFile(ctx, uri)
2828
if err != nil {
2929
return nil, err
3030
}
31-
3231
var candidates []source.CompletionItem
3332
var surrounding *source.Selection
34-
switch f.Kind() {
33+
switch fh.Identity().Kind {
3534
case source.Go:
3635
options.Completion.FullDocumentation = options.HoverKind == source.FullDocumentation
37-
candidates, surrounding, err = source.Completion(ctx, snapshot, f, params.Position, options.Completion)
36+
candidates, surrounding, err = source.Completion(ctx, snapshot, fh, params.Position, options.Completion)
3837
case source.Mod:
3938
candidates, surrounding = nil, nil
4039
}

‎internal/lsp/definition.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,14 @@ func (s *Server) definition(ctx context.Context, params *protocol.DefinitionPara
1919
return nil, err
2020
}
2121
snapshot := view.Snapshot()
22-
f, err := view.GetFile(ctx, uri)
22+
fh, err := snapshot.GetFile(ctx, uri)
2323
if err != nil {
2424
return nil, err
2525
}
26-
if f.Kind() != source.Go {
26+
if fh.Identity().Kind != source.Go {
2727
return nil, nil
2828
}
29-
ident, err := source.Identifier(ctx, snapshot, f, params.Position, source.WidestCheckPackageHandle)
29+
ident, err := source.Identifier(ctx, snapshot, fh, params.Position, source.WidestCheckPackageHandle)
3030
if err != nil {
3131
return nil, err
3232
}
@@ -49,14 +49,14 @@ func (s *Server) typeDefinition(ctx context.Context, params *protocol.TypeDefini
4949
return nil, err
5050
}
5151
snapshot := view.Snapshot()
52-
f, err := view.GetFile(ctx, uri)
52+
fh, err := snapshot.GetFile(ctx, uri)
5353
if err != nil {
5454
return nil, err
5555
}
56-
if f.Kind() != source.Go {
56+
if fh.Identity().Kind != source.Go {
5757
return nil, nil
5858
}
59-
ident, err := source.Identifier(ctx, snapshot, f, params.Position, source.WidestCheckPackageHandle)
59+
ident, err := source.Identifier(ctx, snapshot, fh, params.Position, source.WidestCheckPackageHandle)
6060
if err != nil {
6161
return nil, err
6262
}

0 commit comments

Comments
 (0)