From 2208e1677e7b76008b07abe18e667c2f04296433 Mon Sep 17 00:00:00 2001 From: Rebecca Stambler Date: Tue, 17 Dec 2019 18:57:54 -0500 Subject: [PATCH] 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 TryBot-Result: Gobot Gobot Reviewed-by: Heschi Kreinick --- internal/lsp/cache/check.go | 3 +- internal/lsp/cache/file.go | 12 --- internal/lsp/cache/session.go | 12 +-- internal/lsp/cache/snapshot.go | 48 ++++++--- internal/lsp/cache/view.go | 21 ++-- internal/lsp/code_action.go | 23 ++-- internal/lsp/command.go | 3 +- internal/lsp/completion.go | 7 +- internal/lsp/definition.go | 12 +-- internal/lsp/diagnostics.go | 23 ++-- internal/lsp/folding_range.go | 7 +- internal/lsp/format.go | 7 +- internal/lsp/highlight.go | 7 +- internal/lsp/hover.go | 6 +- internal/lsp/implementation.go | 8 +- internal/lsp/link.go | 3 +- internal/lsp/lsp_test.go | 16 +-- internal/lsp/references.go | 8 +- internal/lsp/rename.go | 15 ++- internal/lsp/signature_help.go | 6 +- internal/lsp/source/completion.go | 6 +- internal/lsp/source/diagnostics.go | 11 +- internal/lsp/source/folding_range.go | 3 +- internal/lsp/source/format.go | 18 ++-- internal/lsp/source/highlight.go | 4 +- internal/lsp/source/identifier.go | 4 +- internal/lsp/source/rename.go | 3 +- internal/lsp/source/signature_help.go | 4 +- internal/lsp/source/source_test.go | 150 ++++++++++++-------------- internal/lsp/source/symbols.go | 4 +- internal/lsp/source/util.go | 9 +- internal/lsp/source/view.go | 23 ++-- internal/lsp/symbols.go | 7 +- internal/lsp/text_synchronization.go | 10 +- internal/lsp/watched_files.go | 18 ++-- 35 files changed, 243 insertions(+), 278 deletions(-) diff --git a/internal/lsp/cache/check.go b/internal/lsp/cache/check.go index 97b86da8bf..30e4af2a22 100644 --- a/internal/lsp/cache/check.go +++ b/internal/lsp/cache/check.go @@ -222,11 +222,10 @@ func (ph *packageHandle) cached() (*pkg, error) { func (s *snapshot) parseGoHandles(ctx context.Context, files []span.URI, mode source.ParseMode) ([]source.ParseGoHandle, error) { phs := make([]source.ParseGoHandle, 0, len(files)) for _, uri := range files { - f, err := s.view.GetFile(ctx, uri) + fh, err := s.GetFile(ctx, uri) if err != nil { return nil, err } - fh := s.Handle(ctx, f) phs = append(phs, s.view.session.cache.ParseGoHandle(fh, mode)) } return phs, nil diff --git a/internal/lsp/cache/file.go b/internal/lsp/cache/file.go index 122dcdebd2..534892ba7e 100644 --- a/internal/lsp/cache/file.go +++ b/internal/lsp/cache/file.go @@ -13,14 +13,6 @@ import ( "golang.org/x/tools/internal/span" ) -// viewFile extends source.File with helper methods for the view package. -type viewFile interface { - source.File - - filename() string - addURI(uri span.URI) int -} - // fileBase holds the common functionality for all files. // It is intended to be embedded in the file implementations type fileBase struct { @@ -43,10 +35,6 @@ func (f *fileBase) URI() span.URI { return f.uris[0] } -func (f *fileBase) Kind() source.FileKind { - return f.kind -} - func (f *fileBase) filename() string { return f.fname } diff --git a/internal/lsp/cache/session.go b/internal/lsp/cache/session.go index 6d5a41d3f0..2ff2ce8183 100644 --- a/internal/lsp/cache/session.go +++ b/internal/lsp/cache/session.go @@ -94,8 +94,8 @@ func (s *session) createView(ctx context.Context, name string, folder span.URI, name: name, modfiles: modfiles, folder: folder, - filesByURI: make(map[span.URI]viewFile), - filesByBase: make(map[string][]viewFile), + filesByURI: make(map[span.URI]*fileBase), + filesByBase: make(map[string][]*fileBase), snapshot: &snapshot{ packages: make(map[packageKey]*packageHandle), ids: make(map[span.URI][]packageID), @@ -301,11 +301,11 @@ func (s *session) DidModifyFile(ctx context.Context, c source.FileModification) return nil, errors.Errorf("ignored file %v", c.URI) } // Set the content for the file, only for didChange and didClose events. - f, err := view.GetFile(ctx, c.URI) + f, err := view.getFileLocked(ctx, c.URI) if err != nil { return nil, err } - snapshots = append(snapshots, view.invalidateContent(ctx, f, c.Action)) + snapshots = append(snapshots, view.invalidateContent(ctx, c.URI, f.kind, c.Action)) } return snapshots, nil } @@ -328,10 +328,10 @@ func (s *session) DidChangeOutOfBand(ctx context.Context, uri span.URI, action s if err != nil { return false } - f, err := view.GetFile(ctx, uri) + f, err := view.getFileLocked(ctx, uri) if err != nil { return false } - view.invalidateContent(ctx, f, action) + view.invalidateContent(ctx, f.URI(), f.kind, action) return true } diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go index 4314e49dd8..3b5fe63c10 100644 --- a/internal/lsp/cache/snapshot.go +++ b/internal/lsp/cache/snapshot.go @@ -447,43 +447,63 @@ func (s *snapshot) getFileURIs() []span.URI { return uris } -func (s *snapshot) getFile(uri span.URI) source.FileHandle { - s.mu.Lock() - defer s.mu.Unlock() +// FindFile returns the file if the given URI is already a part of the view. +func (s *snapshot) FindFile(ctx context.Context, uri span.URI) source.FileHandle { + s.view.mu.Lock() + defer s.view.mu.Unlock() - return s.files[uri] + f, err := s.view.findFile(uri) + if f == nil || err != nil { + return nil + } + return s.getFileHandle(ctx, f) } -func (s *snapshot) Handle(ctx context.Context, f source.File) source.FileHandle { +// GetFile returns a File for the given URI. It will always succeed because it +// adds the file to the managed set if needed. +func (s *snapshot) GetFile(ctx context.Context, uri span.URI) (source.FileHandle, error) { + s.view.mu.Lock() + defer s.view.mu.Unlock() + + // TODO(rstambler): Should there be a version that provides a kind explicitly? + kind := source.DetectLanguage("", uri.Filename()) + f, err := s.view.getFile(ctx, uri, kind) + if err != nil { + return nil, err + } + return s.getFileHandle(ctx, f), nil +} + +func (s *snapshot) getFileHandle(ctx context.Context, f *fileBase) source.FileHandle { s.mu.Lock() defer s.mu.Unlock() if _, ok := s.files[f.URI()]; !ok { - s.files[f.URI()] = s.view.session.GetFile(f.URI(), f.Kind()) + s.files[f.URI()] = s.view.session.GetFile(f.URI(), f.kind) } return s.files[f.URI()] } -func (s *snapshot) clone(ctx context.Context, withoutFile source.File) *snapshot { +func (s *snapshot) clone(ctx context.Context, withoutURI span.URI, withoutFileKind source.FileKind) *snapshot { s.mu.Lock() defer s.mu.Unlock() directIDs := map[packageID]struct{}{} // Collect all of the package IDs that correspond to the given file. // TODO: if the file has moved into a new package, we should invalidate that too. - for _, id := range s.ids[withoutFile.URI()] { + for _, id := range s.ids[withoutURI] { directIDs[id] = struct{}{} } // If we are invalidating a go.mod file then we should invalidate all of the packages in the module - if withoutFile.Kind() == source.Mod { + if withoutFileKind == source.Mod { for id := range s.workspacePackages { directIDs[id] = struct{}{} } } // Get the original FileHandle for the URI, if it exists. - originalFH := s.files[withoutFile.URI()] + originalFH := s.files[withoutURI] // If this is a file we don't yet know about, // then we do not yet know what packages it should belong to. @@ -491,7 +511,7 @@ func (s *snapshot) clone(ctx context.Context, withoutFile source.File) *snapshot // of all of the files in the same directory as this one. // TODO(rstambler): Speed this up by mapping directories to filenames. if originalFH == nil { - if dirStat, err := os.Stat(dir(withoutFile.URI().Filename())); err == nil { + if dirStat, err := os.Stat(dir(withoutURI.Filename())); err == nil { for uri := range s.files { if fdirStat, err := os.Stat(dir(uri.Filename())); err == nil { if os.SameFile(dirStat, fdirStat) { @@ -546,11 +566,11 @@ func (s *snapshot) clone(ctx context.Context, withoutFile source.File) *snapshot result.files[k] = v } // Handle the invalidated file; it may have new contents or not exist. - currentFH := s.view.session.GetFile(withoutFile.URI(), withoutFile.Kind()) + currentFH := s.view.session.GetFile(withoutURI, withoutFileKind) if _, _, err := currentFH.Read(ctx); os.IsNotExist(err) { - delete(result.files, withoutFile.URI()) + delete(result.files, withoutURI) } else { - result.files[withoutFile.URI()] = currentFH + result.files[withoutURI] = currentFH } // Collect the IDs for the packages associated with the excluded URIs. diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go index c6cea0b59e..8825e63007 100644 --- a/internal/lsp/cache/view.go +++ b/internal/lsp/cache/view.go @@ -75,8 +75,8 @@ type view struct { // keep track of files by uri and by basename, a single file may be mapped // to multiple uris, and the same basename may map to multiple files - filesByURI map[span.URI]viewFile - filesByBase map[string][]viewFile + filesByURI map[span.URI]*fileBase + filesByBase map[string][]*fileBase snapshotMu sync.Mutex snapshot *snapshot @@ -348,7 +348,7 @@ func (v *view) getSnapshot() *snapshot { // invalidateContent invalidates the content of a Go file, // including any position and type information that depends on it. // It returns true if we were already tracking the given file, false otherwise. -func (v *view) invalidateContent(ctx context.Context, f source.File, action source.FileAction) source.Snapshot { +func (v *view) invalidateContent(ctx context.Context, uri span.URI, kind source.FileKind, action source.FileAction) source.Snapshot { // Cancel all still-running previous requests, since they would be // operating on stale data. switch action { @@ -360,7 +360,7 @@ func (v *view) invalidateContent(ctx context.Context, f source.File, action sour v.snapshotMu.Lock() defer v.snapshotMu.Unlock() - v.snapshot = v.snapshot.clone(ctx, f) + v.snapshot = v.snapshot.clone(ctx, uri, kind) return v.snapshot } @@ -373,9 +373,10 @@ func (v *view) cancelBackground() { } // FindFile returns the file if the given URI is already a part of the view. -func (v *view) FindFile(ctx context.Context, uri span.URI) source.File { +func (v *view) findFileLocked(ctx context.Context, uri span.URI) *fileBase { v.mu.Lock() defer v.mu.Unlock() + f, err := v.findFile(uri) if err != nil { return nil @@ -383,9 +384,9 @@ func (v *view) FindFile(ctx context.Context, uri span.URI) source.File { return f } -// GetFile returns a File for the given URI. It will always succeed because it +// getFileLocked returns a File for the given URI. It will always succeed because it // adds the file to the managed set if needed. -func (v *view) GetFile(ctx context.Context, uri span.URI) (source.File, error) { +func (v *view) getFileLocked(ctx context.Context, uri span.URI) (*fileBase, error) { v.mu.Lock() defer v.mu.Unlock() @@ -395,7 +396,7 @@ func (v *view) GetFile(ctx context.Context, uri span.URI) (source.File, error) { } // getFile is the unlocked internal implementation of GetFile. -func (v *view) getFile(ctx context.Context, uri span.URI, kind source.FileKind) (viewFile, error) { +func (v *view) getFile(ctx context.Context, uri span.URI, kind source.FileKind) (*fileBase, error) { f, err := v.findFile(uri) if err != nil { return nil, err @@ -415,7 +416,7 @@ func (v *view) getFile(ctx context.Context, uri span.URI, kind source.FileKind) // // An error is only returned for an irreparable failure, for example, if the // filename in question does not exist. -func (v *view) findFile(uri span.URI) (viewFile, error) { +func (v *view) findFile(uri span.URI) (*fileBase, error) { if f := v.filesByURI[uri]; f != nil { // a perfect match return f, nil @@ -451,7 +452,7 @@ func (f *fileBase) addURI(uri span.URI) int { return len(f.uris) } -func (v *view) mapFile(uri span.URI, f viewFile) { +func (v *view) mapFile(uri span.URI, f *fileBase) { v.filesByURI[uri] = f if f.addURI(uri) == 1 { basename := basename(f.filename()) diff --git a/internal/lsp/code_action.go b/internal/lsp/code_action.go index f78b90343d..a7b8ea44a5 100644 --- a/internal/lsp/code_action.go +++ b/internal/lsp/code_action.go @@ -25,12 +25,11 @@ func (s *Server) codeAction(ctx context.Context, params *protocol.CodeActionPara if err != nil { return nil, err } - f, err := view.GetFile(ctx, uri) + snapshot := view.Snapshot() + fh, err := snapshot.GetFile(ctx, uri) if err != nil { return nil, err } - snapshot := view.Snapshot() - fh := snapshot.Handle(ctx, f) // Determine the supported actions for this file kind. supportedCodeActions, ok := view.Options().SupportedCodeActions[fh.Identity().Kind] @@ -63,21 +62,19 @@ func (s *Server) codeAction(ctx context.Context, params *protocol.CodeActionPara Title: "Tidy", Kind: protocol.SourceOrganizeImports, Command: &protocol.Command{ - Title: "Tidy", - Command: "tidy", - Arguments: []interface{}{ - f.URI(), - }, + Title: "Tidy", + Command: "tidy", + Arguments: []interface{}{fh.Identity().URI}, }, }) case source.Go: - edits, editsPerFix, err := source.AllImportsFixes(ctx, snapshot, f) + edits, editsPerFix, err := source.AllImportsFixes(ctx, snapshot, fh) if err != nil { return nil, err } if diagnostics := params.Context.Diagnostics; wanted[protocol.QuickFix] && len(diagnostics) > 0 { // First, add the quick fixes reported by go/analysis. - qf, err := quickFixes(ctx, snapshot, f, diagnostics) + qf, err := quickFixes(ctx, snapshot, fh, diagnostics) if err != nil { log.Error(ctx, "quick fixes failed", err, telemetry.File.Of(uri)) } @@ -203,10 +200,9 @@ func importDiagnostics(fix *imports.ImportFix, diagnostics []protocol.Diagnostic return results } -func quickFixes(ctx context.Context, snapshot source.Snapshot, f source.File, diagnostics []protocol.Diagnostic) ([]protocol.CodeAction, error) { +func quickFixes(ctx context.Context, snapshot source.Snapshot, fh source.FileHandle, diagnostics []protocol.Diagnostic) ([]protocol.CodeAction, error) { var codeActions []protocol.CodeAction - fh := snapshot.Handle(ctx, f) phs, err := snapshot.PackageHandles(ctx, fh) if err != nil { return nil, err @@ -232,12 +228,11 @@ func quickFixes(ctx context.Context, snapshot source.Snapshot, f source.File, di Edit: protocol.WorkspaceEdit{}, } for uri, edits := range fix.Edits { - f, err := snapshot.View().GetFile(ctx, uri) + fh, err := snapshot.GetFile(ctx, uri) if err != nil { log.Error(ctx, "no file", err, telemetry.URI.Of(uri)) continue } - fh := snapshot.Handle(ctx, f) action.Edit.DocumentChanges = append(action.Edit.DocumentChanges, documentChanges(fh, edits)...) } codeActions = append(codeActions, action) diff --git a/internal/lsp/command.go b/internal/lsp/command.go index 5075e9091c..6742ab5e9b 100644 --- a/internal/lsp/command.go +++ b/internal/lsp/command.go @@ -21,11 +21,10 @@ func (s *Server) executeCommand(ctx context.Context, params *protocol.ExecuteCom if err != nil { return nil, err } - f, err := view.GetFile(ctx, uri) + fh, err := view.Snapshot().GetFile(ctx, uri) if err != nil { return nil, err } - fh := view.Snapshot().Handle(ctx, f) if fh.Identity().Kind != source.Mod { return nil, errors.Errorf("%s is not a mod file", uri) } diff --git a/internal/lsp/completion.go b/internal/lsp/completion.go index a10981f8de..00240cf6ff 100644 --- a/internal/lsp/completion.go +++ b/internal/lsp/completion.go @@ -24,17 +24,16 @@ func (s *Server) completion(ctx context.Context, params *protocol.CompletionPara } snapshot := view.Snapshot() options := view.Options() - f, err := view.GetFile(ctx, uri) + fh, err := snapshot.GetFile(ctx, uri) if err != nil { return nil, err } - var candidates []source.CompletionItem var surrounding *source.Selection - switch f.Kind() { + switch fh.Identity().Kind { case source.Go: options.Completion.FullDocumentation = options.HoverKind == source.FullDocumentation - candidates, surrounding, err = source.Completion(ctx, snapshot, f, params.Position, options.Completion) + candidates, surrounding, err = source.Completion(ctx, snapshot, fh, params.Position, options.Completion) case source.Mod: candidates, surrounding = nil, nil } diff --git a/internal/lsp/definition.go b/internal/lsp/definition.go index df75d22c65..41617d3fba 100644 --- a/internal/lsp/definition.go +++ b/internal/lsp/definition.go @@ -19,14 +19,14 @@ func (s *Server) definition(ctx context.Context, params *protocol.DefinitionPara return nil, err } snapshot := view.Snapshot() - f, err := view.GetFile(ctx, uri) + fh, err := snapshot.GetFile(ctx, uri) if err != nil { return nil, err } - if f.Kind() != source.Go { + if fh.Identity().Kind != source.Go { return nil, nil } - ident, err := source.Identifier(ctx, snapshot, f, params.Position, source.WidestCheckPackageHandle) + ident, err := source.Identifier(ctx, snapshot, fh, params.Position, source.WidestCheckPackageHandle) if err != nil { return nil, err } @@ -49,14 +49,14 @@ func (s *Server) typeDefinition(ctx context.Context, params *protocol.TypeDefini return nil, err } snapshot := view.Snapshot() - f, err := view.GetFile(ctx, uri) + fh, err := snapshot.GetFile(ctx, uri) if err != nil { return nil, err } - if f.Kind() != source.Go { + if fh.Identity().Kind != source.Go { return nil, nil } - ident, err := source.Identifier(ctx, snapshot, f, params.Position, source.WidestCheckPackageHandle) + ident, err := source.Identifier(ctx, snapshot, fh, params.Position, source.WidestCheckPackageHandle) if err != nil { return nil, err } diff --git a/internal/lsp/diagnostics.go b/internal/lsp/diagnostics.go index 22f056e4a4..283db56224 100644 --- a/internal/lsp/diagnostics.go +++ b/internal/lsp/diagnostics.go @@ -15,10 +15,10 @@ import ( "golang.org/x/tools/internal/telemetry/trace" ) -func (s *Server) diagnose(snapshot source.Snapshot, f source.File) error { - switch f.Kind() { +func (s *Server) diagnose(snapshot source.Snapshot, fh source.FileHandle) error { + switch fh.Identity().Kind { case source.Go: - go s.diagnoseFile(snapshot, f) + go s.diagnoseFile(snapshot, fh) case source.Mod: go s.diagnoseSnapshot(snapshot) } @@ -41,32 +41,31 @@ func (s *Server) diagnoseSnapshot(snapshot source.Snapshot) { } // Find a file on which to call diagnostics. uri := ph.CompiledGoFiles()[0].File().Identity().URI - f, err := snapshot.View().GetFile(ctx, uri) + fh, err := snapshot.GetFile(ctx, uri) if err != nil { - log.Error(ctx, "no file", err, telemetry.URI.Of(uri)) continue } // Run diagnostics on the workspace package. - go func(snapshot source.Snapshot, f source.File) { - reports, _, err := source.Diagnostics(ctx, snapshot, f, false, snapshot.View().Options().DisabledAnalyses) + go func(snapshot source.Snapshot, fh source.FileHandle) { + reports, _, err := source.Diagnostics(ctx, snapshot, fh, false, snapshot.View().Options().DisabledAnalyses) if err != nil { - log.Error(ctx, "no diagnostics", err, telemetry.URI.Of(f.URI())) + log.Error(ctx, "no diagnostics", err, telemetry.URI.Of(fh.Identity().URI)) return } // Don't publish empty diagnostics. s.publishReports(ctx, reports, false) - }(snapshot, f) + }(snapshot, fh) } } -func (s *Server) diagnoseFile(snapshot source.Snapshot, f source.File) { +func (s *Server) diagnoseFile(snapshot source.Snapshot, fh source.FileHandle) { ctx := snapshot.View().BackgroundContext() ctx, done := trace.StartSpan(ctx, "lsp:background-worker") defer done() - ctx = telemetry.File.With(ctx, f.URI()) + ctx = telemetry.File.With(ctx, fh.Identity().URI) - reports, warningMsg, err := source.Diagnostics(ctx, snapshot, f, true, snapshot.View().Options().DisabledAnalyses) + reports, warningMsg, err := source.Diagnostics(ctx, snapshot, fh, true, snapshot.View().Options().DisabledAnalyses) // Check the warning message first. if warningMsg != "" { s.client.ShowMessage(ctx, &protocol.ShowMessageParams{ diff --git a/internal/lsp/folding_range.go b/internal/lsp/folding_range.go index f3eede0797..7a4bb9cb4b 100644 --- a/internal/lsp/folding_range.go +++ b/internal/lsp/folding_range.go @@ -15,15 +15,14 @@ func (s *Server) foldingRange(ctx context.Context, params *protocol.FoldingRange return nil, err } snapshot := view.Snapshot() - f, err := view.GetFile(ctx, uri) + fh, err := snapshot.GetFile(ctx, uri) if err != nil { return nil, err } - var ranges []*source.FoldingRangeInfo - switch f.Kind() { + switch fh.Identity().Kind { case source.Go: - ranges, err = source.FoldingRange(ctx, snapshot, f, view.Options().LineFoldingOnly) + ranges, err = source.FoldingRange(ctx, snapshot, fh, view.Options().LineFoldingOnly) case source.Mod: ranges = nil } diff --git a/internal/lsp/format.go b/internal/lsp/format.go index 51c7bcb952..248f92105e 100644 --- a/internal/lsp/format.go +++ b/internal/lsp/format.go @@ -19,15 +19,14 @@ func (s *Server) formatting(ctx context.Context, params *protocol.DocumentFormat return nil, err } snapshot := view.Snapshot() - f, err := view.GetFile(ctx, uri) + fh, err := snapshot.GetFile(ctx, uri) if err != nil { return nil, err } - var edits []protocol.TextEdit - switch f.Kind() { + switch fh.Identity().Kind { case source.Go: - edits, err = source.Format(ctx, snapshot, f) + edits, err = source.Format(ctx, snapshot, fh) case source.Mod: return nil, nil } diff --git a/internal/lsp/highlight.go b/internal/lsp/highlight.go index 59874f51bb..2125534b02 100644 --- a/internal/lsp/highlight.go +++ b/internal/lsp/highlight.go @@ -21,15 +21,14 @@ func (s *Server) documentHighlight(ctx context.Context, params *protocol.Documen return nil, err } snapshot := view.Snapshot() - f, err := view.GetFile(ctx, uri) + fh, err := snapshot.GetFile(ctx, uri) if err != nil { return nil, err } - var rngs []protocol.Range - switch f.Kind() { + switch fh.Identity().Kind { case source.Go: - rngs, err = source.Highlight(ctx, snapshot, f, params.Position) + rngs, err = source.Highlight(ctx, snapshot, fh, params.Position) case source.Mod: return nil, nil } diff --git a/internal/lsp/hover.go b/internal/lsp/hover.go index d8341d050b..2a60599e6e 100644 --- a/internal/lsp/hover.go +++ b/internal/lsp/hover.go @@ -19,14 +19,14 @@ func (s *Server) hover(ctx context.Context, params *protocol.HoverParams) (*prot return nil, err } snapshot := view.Snapshot() - f, err := view.GetFile(ctx, uri) + fh, err := snapshot.GetFile(ctx, uri) if err != nil { return nil, err } - if f.Kind() != source.Go { + if fh.Identity().Kind != source.Go { return nil, nil } - ident, err := source.Identifier(ctx, snapshot, f, params.Position, source.WidestCheckPackageHandle) + ident, err := source.Identifier(ctx, snapshot, fh, params.Position, source.WidestCheckPackageHandle) if err != nil { return nil, nil } diff --git a/internal/lsp/implementation.go b/internal/lsp/implementation.go index 344deab602..c4df76a268 100644 --- a/internal/lsp/implementation.go +++ b/internal/lsp/implementation.go @@ -21,14 +21,14 @@ func (s *Server) implementation(ctx context.Context, params *protocol.Implementa return nil, err } snapshot := view.Snapshot() - f, err := view.GetFile(ctx, uri) + fh, err := snapshot.GetFile(ctx, uri) if err != nil { return nil, err } - if f.Kind() != source.Go { + if fh.Identity().Kind != source.Go { return nil, nil } - phs, err := snapshot.PackageHandles(ctx, snapshot.Handle(ctx, f)) + phs, err := snapshot.PackageHandles(ctx, fh) if err != nil { return nil, err } @@ -39,7 +39,7 @@ func (s *Server) implementation(ctx context.Context, params *protocol.Implementa for _, ph := range phs { ctx := telemetry.Package.With(ctx, ph.ID()) - ident, err := source.Identifier(ctx, snapshot, f, params.Position, source.SpecificPackageHandle(ph.ID())) + ident, err := source.Identifier(ctx, snapshot, fh, params.Position, source.SpecificPackageHandle(ph.ID())) if err != nil { if err == source.ErrNoIdentFound { return nil, err diff --git a/internal/lsp/link.go b/internal/lsp/link.go index 777b91ee22..66ded42c19 100644 --- a/internal/lsp/link.go +++ b/internal/lsp/link.go @@ -27,11 +27,10 @@ func (s *Server) documentLink(ctx context.Context, params *protocol.DocumentLink if err != nil { return nil, err } - f, err := view.GetFile(ctx, uri) + fh, err := view.Snapshot().GetFile(ctx, uri) if err != nil { return nil, err } - fh := view.Snapshot().Handle(ctx, f) if fh.Identity().Kind == source.Mod { return nil, nil } diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go index ce636ead54..ff9de385c7 100644 --- a/internal/lsp/lsp_test.go +++ b/internal/lsp/lsp_test.go @@ -86,12 +86,12 @@ func testLSP(t *testing.T, exporter packagestest.Exporter) { // TODO: Actually test the LSP diagnostics function in this test. func (r *runner) Diagnostics(t *testing.T, uri span.URI, want []source.Diagnostic) { v := r.server.session.View(viewName) - f, err := v.GetFile(r.ctx, uri) + fh, err := v.Snapshot().GetFile(r.ctx, uri) if err != nil { - t.Fatalf("no file for %s: %v", f, err) + t.Fatal(err) } - identity := v.Snapshot().Handle(r.ctx, f).Identity() - results, _, err := source.Diagnostics(r.ctx, v.Snapshot(), f, true, nil) + identity := fh.Identity() + results, _, err := source.Diagnostics(r.ctx, v.Snapshot(), fh, true, nil) if err != nil { t.Fatal(err) } @@ -339,13 +339,13 @@ func (r *runner) SuggestedFix(t *testing.T, spn span.Span) { if err != nil { t.Fatal(err) } - f, err := view.GetFile(r.ctx, uri) + snapshot := view.Snapshot() + fh, err := snapshot.GetFile(r.ctx, spn.URI()) if err != nil { t.Fatal(err) } - snapshot := view.Snapshot() - fileID := snapshot.Handle(r.ctx, f).Identity() - diagnostics, _, err := source.Diagnostics(r.ctx, snapshot, f, true, nil) + fileID := fh.Identity() + diagnostics, _, err := source.Diagnostics(r.ctx, snapshot, fh, true, nil) if err != nil { t.Fatal(err) } diff --git a/internal/lsp/references.go b/internal/lsp/references.go index 0029264a07..a61a7d4d48 100644 --- a/internal/lsp/references.go +++ b/internal/lsp/references.go @@ -21,15 +21,15 @@ func (s *Server) references(ctx context.Context, params *protocol.ReferenceParam return nil, err } snapshot := view.Snapshot() - f, err := view.GetFile(ctx, uri) + fh, err := snapshot.GetFile(ctx, uri) if err != nil { return nil, err } // Find all references to the identifier at the position. - if f.Kind() != source.Go { + if fh.Identity().Kind != source.Go { return nil, nil } - phs, err := snapshot.PackageHandles(ctx, snapshot.Handle(ctx, f)) + phs, err := snapshot.PackageHandles(ctx, fh) if err != nil { return nil, nil } @@ -41,7 +41,7 @@ func (s *Server) references(ctx context.Context, params *protocol.ReferenceParam lastIdent *source.IdentifierInfo ) for _, ph := range phs { - ident, err := source.Identifier(ctx, snapshot, f, params.Position, source.SpecificPackageHandle(ph.ID())) + ident, err := source.Identifier(ctx, snapshot, fh, params.Position, source.SpecificPackageHandle(ph.ID())) if err != nil { if err == source.ErrNoIdentFound { return nil, err diff --git a/internal/lsp/rename.go b/internal/lsp/rename.go index 96f3f4f25a..6c1c223583 100644 --- a/internal/lsp/rename.go +++ b/internal/lsp/rename.go @@ -19,14 +19,14 @@ func (s *Server) rename(ctx context.Context, params *protocol.RenameParams) (*pr return nil, err } snapshot := view.Snapshot() - f, err := view.GetFile(ctx, uri) + fh, err := snapshot.GetFile(ctx, uri) if err != nil { return nil, err } - if f.Kind() != source.Go { + if fh.Identity().Kind != source.Go { return nil, nil } - ident, err := source.Identifier(ctx, snapshot, f, params.Position, source.WidestCheckPackageHandle) + ident, err := source.Identifier(ctx, snapshot, fh, params.Position, source.WidestCheckPackageHandle) if err != nil { return nil, nil } @@ -36,11 +36,10 @@ func (s *Server) rename(ctx context.Context, params *protocol.RenameParams) (*pr } var docChanges []protocol.TextDocumentEdit for uri, e := range edits { - f, err := view.GetFile(ctx, uri) + fh, err := snapshot.GetFile(ctx, uri) if err != nil { return nil, err } - fh := ident.Snapshot.Handle(ctx, f) docChanges = append(docChanges, documentChanges(fh, e)...) } return &protocol.WorkspaceEdit{ @@ -55,14 +54,14 @@ func (s *Server) prepareRename(ctx context.Context, params *protocol.PrepareRena return nil, err } snapshot := view.Snapshot() - f, err := view.GetFile(ctx, uri) + fh, err := snapshot.GetFile(ctx, uri) if err != nil { return nil, err } - if f.Kind() != source.Go { + if fh.Identity().Kind != source.Go { return nil, nil } - ident, err := source.Identifier(ctx, snapshot, f, params.Position, source.WidestCheckPackageHandle) + ident, err := source.Identifier(ctx, snapshot, fh, params.Position, source.WidestCheckPackageHandle) if err != nil { return nil, nil // ignore errors } diff --git a/internal/lsp/signature_help.go b/internal/lsp/signature_help.go index 3068c3abc1..096abe95a3 100644 --- a/internal/lsp/signature_help.go +++ b/internal/lsp/signature_help.go @@ -21,14 +21,14 @@ func (s *Server) signatureHelp(ctx context.Context, params *protocol.SignatureHe return nil, err } snapshot := view.Snapshot() - f, err := view.GetFile(ctx, uri) + fh, err := snapshot.GetFile(ctx, uri) if err != nil { return nil, err } - if f.Kind() != source.Go { + if fh.Identity().Kind != source.Go { return nil, nil } - info, err := source.SignatureHelp(ctx, snapshot, f, params.Position) + info, err := source.SignatureHelp(ctx, snapshot, fh, params.Position) if err != nil { log.Print(ctx, "no signature help", tag.Of("At", params.Position), tag.Of("Failure", err)) return nil, nil diff --git a/internal/lsp/source/completion.go b/internal/lsp/source/completion.go index 9d948d8b7c..73e8ec30c9 100644 --- a/internal/lsp/source/completion.go +++ b/internal/lsp/source/completion.go @@ -393,13 +393,13 @@ func (e ErrIsDefinition) Error() string { // The selection is computed based on the preceding identifier and can be used by // the client to score the quality of the completion. For instance, some clients // may tolerate imperfect matches as valid completion results, since users may make typos. -func Completion(ctx context.Context, snapshot Snapshot, f File, pos protocol.Position, opts CompletionOptions) ([]CompletionItem, *Selection, error) { +func Completion(ctx context.Context, snapshot Snapshot, fh FileHandle, pos protocol.Position, opts CompletionOptions) ([]CompletionItem, *Selection, error) { ctx, done := trace.StartSpan(ctx, "source.Completion") defer done() startTime := time.Now() - pkg, pgh, err := getParsedFile(ctx, snapshot, f, NarrowestCheckPackageHandle) + pkg, pgh, err := getParsedFile(ctx, snapshot, fh, NarrowestCheckPackageHandle) if err != nil { return nil, nil, fmt.Errorf("getting file for Completion: %v", err) } @@ -432,7 +432,7 @@ func Completion(ctx context.Context, snapshot Snapshot, f File, pos protocol.Pos snapshot: snapshot, qf: qualifier(file, pkg.GetTypes(), pkg.GetTypesInfo()), ctx: ctx, - filename: f.URI().Filename(), + filename: fh.Identity().URI.Filename(), file: file, path: path, pos: rng.Start, diff --git a/internal/lsp/source/diagnostics.go b/internal/lsp/source/diagnostics.go index 9d3c8d41d2..b49194316a 100644 --- a/internal/lsp/source/diagnostics.go +++ b/internal/lsp/source/diagnostics.go @@ -39,11 +39,10 @@ type RelatedInformation struct { Message string } -func Diagnostics(ctx context.Context, snapshot Snapshot, f File, withAnalysis bool, disabledAnalyses map[string]struct{}) (map[FileIdentity][]Diagnostic, string, error) { - ctx, done := trace.StartSpan(ctx, "source.Diagnostics", telemetry.File.Of(f.URI())) +func Diagnostics(ctx context.Context, snapshot Snapshot, fh FileHandle, withAnalysis bool, disabledAnalyses map[string]struct{}) (map[FileIdentity][]Diagnostic, string, error) { + ctx, done := trace.StartSpan(ctx, "source.Diagnostics", telemetry.File.Of(fh.Identity().URI)) defer done() - fh := snapshot.Handle(ctx, f) phs, err := snapshot.PackageHandles(ctx, fh) if err != nil { return nil, "", err @@ -56,8 +55,8 @@ func Diagnostics(ctx context.Context, snapshot Snapshot, f File, withAnalysis bo // not correctly configured. Report errors, if possible. var warningMsg string if len(ph.MissingDependencies()) > 0 { - if warningMsg, err = checkCommonErrors(ctx, snapshot.View(), f.URI()); err != nil { - log.Error(ctx, "error checking common errors", err, telemetry.File.Of(f.URI)) + if warningMsg, err = checkCommonErrors(ctx, snapshot.View(), fh.Identity().URI); err != nil { + log.Error(ctx, "error checking common errors", err, telemetry.File.Of(fh.Identity().URI)) } } pkg, err := ph.Check(ctx) @@ -89,7 +88,7 @@ func Diagnostics(ctx context.Context, snapshot Snapshot, f File, withAnalysis bo if err == context.Canceled { return nil, "", err } - log.Error(ctx, "failed to run analyses", err, telemetry.File.Of(f.URI())) + log.Error(ctx, "failed to run analyses", err, telemetry.File.Of(fh.Identity().URI)) } } // Updates to the diagnostics for this package may need to be propagated. diff --git a/internal/lsp/source/folding_range.go b/internal/lsp/source/folding_range.go index c01139c351..e7b06e34cf 100644 --- a/internal/lsp/source/folding_range.go +++ b/internal/lsp/source/folding_range.go @@ -15,10 +15,9 @@ type FoldingRangeInfo struct { } // FoldingRange gets all of the folding range for f. -func FoldingRange(ctx context.Context, snapshot Snapshot, f File, lineFoldingOnly bool) (ranges []*FoldingRangeInfo, err error) { +func FoldingRange(ctx context.Context, snapshot Snapshot, fh FileHandle, lineFoldingOnly bool) (ranges []*FoldingRangeInfo, err error) { // TODO(suzmue): consider limiting the number of folding ranges returned, and // implement a way to prioritize folding ranges in that case. - fh := snapshot.Handle(ctx, f) pgh := snapshot.View().Session().Cache().ParseGoHandle(fh, ParseFull) file, m, _, err := pgh.Parse(ctx) if err != nil { diff --git a/internal/lsp/source/format.go b/internal/lsp/source/format.go index 3a846bf417..f2a0a2d471 100644 --- a/internal/lsp/source/format.go +++ b/internal/lsp/source/format.go @@ -24,11 +24,11 @@ import ( ) // Format formats a file with a given range. -func Format(ctx context.Context, snapshot Snapshot, f File) ([]protocol.TextEdit, error) { +func Format(ctx context.Context, snapshot Snapshot, fh FileHandle) ([]protocol.TextEdit, error) { ctx, done := trace.StartSpan(ctx, "source.Format") defer done() - pkg, pgh, err := getParsedFile(ctx, snapshot, f, NarrowestCheckPackageHandle) + pkg, pgh, err := getParsedFile(ctx, snapshot, fh, NarrowestCheckPackageHandle) if err != nil { return nil, fmt.Errorf("getting file for Format: %v", err) } @@ -42,12 +42,12 @@ func Format(ctx context.Context, snapshot Snapshot, f File) ([]protocol.TextEdit if err != nil { return nil, err } - if hasListErrors(pkg) || hasParseErrors(pkg, f.URI()) { + if hasListErrors(pkg) || hasParseErrors(pkg, fh.Identity().URI) { // Even if this package has list or parse errors, this file may not // have any parse errors and can still be formatted. Using format.Node // on an ast with errors may result in code being added or removed. // Attempt to format the source of this file instead. - formatted, err := formatSource(ctx, snapshot, f) + formatted, err := formatSource(ctx, snapshot, fh) if err != nil { return nil, err } @@ -67,11 +67,11 @@ func Format(ctx context.Context, snapshot Snapshot, f File) ([]protocol.TextEdit return computeTextEdits(ctx, snapshot.View(), pgh.File(), m, buf.String()) } -func formatSource(ctx context.Context, s Snapshot, f File) ([]byte, error) { +func formatSource(ctx context.Context, s Snapshot, fh FileHandle) ([]byte, error) { ctx, done := trace.StartSpan(ctx, "source.formatSource") defer done() - data, _, err := s.Handle(ctx, f).Read(ctx) + data, _, err := fh.Read(ctx) if err != nil { return nil, err } @@ -87,16 +87,16 @@ type ImportFix struct { // In addition to returning the result of applying all edits, // it returns a list of fixes that could be applied to the file, with the // corresponding TextEdits that would be needed to apply that fix. -func AllImportsFixes(ctx context.Context, snapshot Snapshot, f File) (allFixEdits []protocol.TextEdit, editsPerFix []*ImportFix, err error) { +func AllImportsFixes(ctx context.Context, snapshot Snapshot, fh FileHandle) (allFixEdits []protocol.TextEdit, editsPerFix []*ImportFix, err error) { ctx, done := trace.StartSpan(ctx, "source.AllImportsFixes") defer done() - pkg, pgh, err := getParsedFile(ctx, snapshot, f, NarrowestCheckPackageHandle) + pkg, pgh, err := getParsedFile(ctx, snapshot, fh, NarrowestCheckPackageHandle) if err != nil { return nil, nil, errors.Errorf("getting file for AllImportsFixes: %v", err) } if hasListErrors(pkg) { - return nil, nil, errors.Errorf("%s has list errors, not running goimports", f.URI()) + return nil, nil, errors.Errorf("%s has list errors, not running goimports", fh.Identity().URI) } options := &imports.Options{ // Defaults. diff --git a/internal/lsp/source/highlight.go b/internal/lsp/source/highlight.go index affe7d11e4..ba07546826 100644 --- a/internal/lsp/source/highlight.go +++ b/internal/lsp/source/highlight.go @@ -17,11 +17,11 @@ import ( errors "golang.org/x/xerrors" ) -func Highlight(ctx context.Context, snapshot Snapshot, f File, pos protocol.Position) ([]protocol.Range, error) { +func Highlight(ctx context.Context, snapshot Snapshot, fh FileHandle, pos protocol.Position) ([]protocol.Range, error) { ctx, done := trace.StartSpan(ctx, "source.Highlight") defer done() - pkg, pgh, err := getParsedFile(ctx, snapshot, f, WidestCheckPackageHandle) + pkg, pgh, err := getParsedFile(ctx, snapshot, fh, WidestCheckPackageHandle) if err != nil { return nil, fmt.Errorf("getting file for Highlight: %v", err) } diff --git a/internal/lsp/source/identifier.go b/internal/lsp/source/identifier.go index 68b701dea7..977a62bede 100644 --- a/internal/lsp/source/identifier.go +++ b/internal/lsp/source/identifier.go @@ -59,11 +59,11 @@ func (i *IdentifierInfo) DeclarationReferenceInfo() *ReferenceInfo { // Identifier returns identifier information for a position // in a file, accounting for a potentially incomplete selector. -func Identifier(ctx context.Context, snapshot Snapshot, f File, pos protocol.Position, selectPackage PackagePolicy) (*IdentifierInfo, error) { +func Identifier(ctx context.Context, snapshot Snapshot, fh FileHandle, pos protocol.Position, selectPackage PackagePolicy) (*IdentifierInfo, error) { ctx, done := trace.StartSpan(ctx, "source.Identifier") defer done() - pkg, pgh, err := getParsedFile(ctx, snapshot, f, selectPackage) + pkg, pgh, err := getParsedFile(ctx, snapshot, fh, selectPackage) if err != nil { return nil, fmt.Errorf("getting file for Identifier: %v", err) } diff --git a/internal/lsp/source/rename.go b/internal/lsp/source/rename.go index 7368dcfe20..64a25894d4 100644 --- a/internal/lsp/source/rename.go +++ b/internal/lsp/source/rename.go @@ -155,11 +155,10 @@ func (i *IdentifierInfo) Rename(ctx context.Context, newName string) (map[span.U for uri, edits := range changes { // These edits should really be associated with FileHandles for maximal correctness. // For now, this is good enough. - f, err := i.Snapshot.View().GetFile(ctx, uri) + fh, err := i.Snapshot.GetFile(ctx, uri) if err != nil { return nil, err } - fh := i.Snapshot.Handle(ctx, f) data, _, err := fh.Read(ctx) if err != nil { return nil, err diff --git a/internal/lsp/source/signature_help.go b/internal/lsp/source/signature_help.go index c8199562c7..b3882b0c44 100644 --- a/internal/lsp/source/signature_help.go +++ b/internal/lsp/source/signature_help.go @@ -28,11 +28,11 @@ type ParameterInformation struct { Label string } -func SignatureHelp(ctx context.Context, snapshot Snapshot, f File, pos protocol.Position) (*SignatureInformation, error) { +func SignatureHelp(ctx context.Context, snapshot Snapshot, fh FileHandle, pos protocol.Position) (*SignatureInformation, error) { ctx, done := trace.StartSpan(ctx, "source.SignatureHelp") defer done() - pkg, pgh, err := getParsedFile(ctx, snapshot, f, NarrowestCheckPackageHandle) + pkg, pgh, err := getParsedFile(ctx, snapshot, fh, NarrowestCheckPackageHandle) if err != nil { return nil, fmt.Errorf("getting file for SignatureHelp: %v", err) } diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go index 1697756995..94159a55af 100644 --- a/internal/lsp/source/source_test.go +++ b/internal/lsp/source/source_test.go @@ -79,13 +79,13 @@ func testSource(t *testing.T, exporter packagestest.Exporter) { } func (r *runner) Diagnostics(t *testing.T, uri span.URI, want []source.Diagnostic) { - f, err := r.view.GetFile(r.ctx, uri) + snapshot := r.view.Snapshot() + fh, err := snapshot.GetFile(r.ctx, uri) if err != nil { t.Fatal(err) } - snapshot := r.view.Snapshot() - fileID := snapshot.Handle(r.ctx, f).Identity() - results, _, err := source.Diagnostics(r.ctx, snapshot, f, true, nil) + fileID := fh.Identity() + results, _, err := source.Diagnostics(r.ctx, snapshot, fh, true, nil) if err != nil { t.Fatal(err) } @@ -252,11 +252,11 @@ func (r *runner) RankCompletion(t *testing.T, src span.Span, test tests.Completi } func (r *runner) callCompletion(t *testing.T, src span.Span, options source.CompletionOptions) (string, []protocol.CompletionItem) { - f, err := r.view.GetFile(r.ctx, src.URI()) + fh, err := r.view.Snapshot().GetFile(r.ctx, src.URI()) if err != nil { t.Fatal(err) } - list, surrounding, err := source.Completion(r.ctx, r.view.Snapshot(), f, protocol.Position{ + list, surrounding, err := source.Completion(r.ctx, r.view.Snapshot(), fh, protocol.Position{ Line: float64(src.Start().Line() - 1), Character: float64(src.Start().Column() - 1), }, options) @@ -293,11 +293,10 @@ func (r *runner) callCompletion(t *testing.T, src span.Span, options source.Comp func (r *runner) FoldingRanges(t *testing.T, spn span.Span) { uri := spn.URI() - f, err := r.view.GetFile(r.ctx, uri) + fh, err := r.view.Snapshot().GetFile(r.ctx, spn.URI()) if err != nil { - t.Fatalf("failed for %v: %v", spn, err) + t.Fatal(err) } - fh := r.view.Snapshot().Handle(r.ctx, f) data, _, err := fh.Read(r.ctx) if err != nil { t.Error(err) @@ -305,7 +304,7 @@ func (r *runner) FoldingRanges(t *testing.T, spn span.Span) { } // Test all folding ranges. - ranges, err := source.FoldingRange(r.ctx, r.view.Snapshot(), f, false) + ranges, err := source.FoldingRange(r.ctx, r.view.Snapshot(), fh, false) if err != nil { t.Error(err) return @@ -313,7 +312,7 @@ func (r *runner) FoldingRanges(t *testing.T, spn span.Span) { r.foldingRanges(t, "foldingRange", uri, string(data), ranges) // Test folding ranges with lineFoldingOnly - ranges, err = source.FoldingRange(r.ctx, r.view.Snapshot(), f, true) + ranges, err = source.FoldingRange(r.ctx, r.view.Snapshot(), fh, true) if err != nil { t.Error(err) return @@ -429,31 +428,27 @@ func foldRanges(contents string, ranges []*source.FoldingRangeInfo) (string, err } func (r *runner) Format(t *testing.T, spn span.Span) { - ctx := r.ctx - uri := spn.URI() - filename := uri.Filename() - gofmted := string(r.data.Golden("gofmt", filename, func() ([]byte, error) { - cmd := exec.Command("gofmt", filename) + gofmted := string(r.data.Golden("gofmt", spn.URI().Filename(), func() ([]byte, error) { + cmd := exec.Command("gofmt", spn.URI().Filename()) out, _ := cmd.Output() // ignore error, sometimes we have intentionally ungofmt-able files return out, nil })) - f, err := r.view.GetFile(ctx, uri) + fh, err := r.view.Snapshot().GetFile(r.ctx, spn.URI()) if err != nil { - t.Fatalf("failed for %v: %v", spn, err) + t.Fatal(err) } - edits, err := source.Format(ctx, r.view.Snapshot(), f) + edits, err := source.Format(r.ctx, r.view.Snapshot(), fh) if err != nil { if gofmted != "" { t.Error(err) } return } - fh := r.view.Snapshot().Handle(r.ctx, f) - data, _, err := fh.Read(ctx) + data, _, err := fh.Read(r.ctx) if err != nil { t.Fatal(err) } - m, err := r.data.Mapper(f.URI()) + m, err := r.data.Mapper(spn.URI()) if err != nil { t.Fatal(err) } @@ -463,24 +458,20 @@ func (r *runner) Format(t *testing.T, spn span.Span) { } got := diff.ApplyEdits(string(data), diffEdits) if gofmted != got { - t.Errorf("format failed for %s, expected:\n%v\ngot:\n%v", filename, gofmted, got) + t.Errorf("format failed for %s, expected:\n%v\ngot:\n%v", spn.URI().Filename(), gofmted, got) } } func (r *runner) Import(t *testing.T, spn span.Span) { - ctx := r.ctx - uri := spn.URI() - filename := uri.Filename() - f, err := r.view.GetFile(ctx, uri) + fh, err := r.view.Snapshot().GetFile(r.ctx, spn.URI()) if err != nil { - t.Fatalf("failed for %v: %v", spn, err) + t.Fatal(err) } - fh := r.view.Snapshot().Handle(r.ctx, f) - edits, _, err := source.AllImportsFixes(ctx, r.view.Snapshot(), f) + edits, _, err := source.AllImportsFixes(r.ctx, r.view.Snapshot(), fh) if err != nil { t.Error(err) } - data, _, err := fh.Read(ctx) + data, _, err := fh.Read(r.ctx) if err != nil { t.Fatal(err) } @@ -493,32 +484,30 @@ func (r *runner) Import(t *testing.T, spn span.Span) { t.Error(err) } got := diff.ApplyEdits(string(data), diffEdits) - want := string(r.data.Golden("goimports", filename, func() ([]byte, error) { + want := string(r.data.Golden("goimports", spn.URI().Filename(), func() ([]byte, error) { return []byte(got), nil })) if want != got { - t.Errorf("import failed for %s, expected:\n%v\ngot:\n%v", filename, want, got) + t.Errorf("import failed for %s, expected:\n%v\ngot:\n%v", spn.URI().Filename(), want, got) } } -func (r *runner) SuggestedFix(t *testing.T, spn span.Span) { -} +func (r *runner) SuggestedFix(t *testing.T, spn span.Span) {} func (r *runner) Definition(t *testing.T, spn span.Span, d tests.Definition) { - ctx := r.ctx - f, err := r.view.GetFile(ctx, d.Src.URI()) - if err != nil { - t.Fatalf("failed for %v: %v", d.Src, err) - } _, srcRng, err := spanToRange(r.data, d.Src) if err != nil { t.Fatal(err) } - ident, err := source.Identifier(ctx, r.view.Snapshot(), f, srcRng.Start, source.WidestCheckPackageHandle) + fh, err := r.view.Snapshot().GetFile(r.ctx, spn.URI()) + if err != nil { + t.Fatal(err) + } + ident, err := source.Identifier(r.ctx, r.view.Snapshot(), fh, srcRng.Start, source.WidestCheckPackageHandle) if err != nil { t.Fatalf("failed for %v: %v", d.Src, err) } - h, err := ident.Hover(ctx) + h, err := ident.Hover(r.ctx) if err != nil { t.Fatalf("failed for %v: %v", d.Src, err) } @@ -562,11 +551,6 @@ func (r *runner) Definition(t *testing.T, spn span.Span, d tests.Definition) { } func (r *runner) Implementation(t *testing.T, spn span.Span, impls []span.Span) { - ctx := r.ctx - f, err := r.view.GetFile(ctx, spn.URI()) - if err != nil { - t.Fatalf("failed for %v: %v", spn, err) - } sm, err := r.data.Mapper(spn.URI()) if err != nil { t.Fatal(err) @@ -575,7 +559,11 @@ func (r *runner) Implementation(t *testing.T, spn span.Span, impls []span.Span) if err != nil { t.Fatalf("failed for %v: %v", spn, err) } - ident, err := source.Identifier(ctx, r.view.Snapshot(), f, loc.Range.Start, source.WidestCheckPackageHandle) + fh, err := r.view.Snapshot().GetFile(r.ctx, spn.URI()) + if err != nil { + t.Fatal(err) + } + ident, err := source.Identifier(r.ctx, r.view.Snapshot(), fh, loc.Range.Start, source.WidestCheckPackageHandle) if err != nil { t.Fatalf("failed for %v: %v", spn, err) } @@ -620,11 +608,11 @@ func (r *runner) Highlight(t *testing.T, src span.Span, locations []span.Span) { if err != nil { t.Fatal(err) } - f, err := r.view.GetFile(ctx, src.URI()) + fh, err := r.view.Snapshot().GetFile(r.ctx, src.URI()) if err != nil { - t.Fatalf("failed for %v: %v", src, err) + t.Fatal(err) } - highlights, err := source.Highlight(ctx, r.view.Snapshot(), f, srcRng.Start) + highlights, err := source.Highlight(ctx, r.view.Snapshot(), fh, srcRng.Start) if err != nil { t.Errorf("highlight failed for %s: %v", src.URI(), err) } @@ -654,15 +642,15 @@ func (r *runner) Highlight(t *testing.T, src span.Span, locations []span.Span) { func (r *runner) References(t *testing.T, src span.Span, itemList []span.Span) { ctx := r.ctx - f, err := r.view.GetFile(ctx, src.URI()) - if err != nil { - t.Fatalf("failed for %v: %v", src, err) - } _, srcRng, err := spanToRange(r.data, src) if err != nil { t.Fatal(err) } - ident, err := source.Identifier(ctx, r.view.Snapshot(), f, srcRng.Start, source.WidestCheckPackageHandle) + fh, err := r.view.Snapshot().GetFile(r.ctx, src.URI()) + if err != nil { + t.Fatal(err) + } + ident, err := source.Identifier(ctx, r.view.Snapshot(), fh, srcRng.Start, source.WidestCheckPackageHandle) if err != nil { t.Fatalf("failed for %v: %v", src, err) } @@ -698,15 +686,15 @@ func (r *runner) Rename(t *testing.T, spn span.Span, newText string) { ctx := r.ctx tag := fmt.Sprintf("%s-rename", newText) - f, err := r.view.GetFile(ctx, spn.URI()) - if err != nil { - t.Fatalf("failed for %v: %v", spn, err) - } _, srcRng, err := spanToRange(r.data, spn) if err != nil { t.Fatal(err) } - ident, err := source.Identifier(r.ctx, r.view.Snapshot(), f, srcRng.Start, source.WidestCheckPackageHandle) + fh, err := r.view.Snapshot().GetFile(r.ctx, spn.URI()) + if err != nil { + t.Fatal(err) + } + ident, err := source.Identifier(r.ctx, r.view.Snapshot(), fh, srcRng.Start, source.WidestCheckPackageHandle) if err != nil { t.Error(err) return @@ -723,12 +711,11 @@ func (r *runner) Rename(t *testing.T, spn span.Span, newText string) { } var res []string - for editSpn, edits := range changes { - f, err := r.view.GetFile(ctx, editSpn) + for editURI, edits := range changes { + fh, err := r.view.Snapshot().GetFile(r.ctx, editURI) if err != nil { - t.Fatalf("failed for %v: %v", spn, err) + t.Fatal(err) } - fh := r.view.Snapshot().Handle(r.ctx, f) data, _, err := fh.Read(ctx) if err != nil { t.Fatal(err) @@ -737,13 +724,13 @@ func (r *runner) Rename(t *testing.T, spn span.Span, newText string) { if err != nil { t.Fatal(err) } - filename := filepath.Base(editSpn.Filename()) diffEdits, err := source.FromProtocolEdits(m, edits) if err != nil { t.Fatal(err) } contents := applyEdits(string(data), diffEdits) if len(changes) > 1 { + filename := filepath.Base(editURI.Filename()) contents = fmt.Sprintf("%s:\n%s", filename, contents) } res = append(res, contents) @@ -785,24 +772,23 @@ func applyEdits(contents string, edits []diff.TextEdit) string { } func (r *runner) PrepareRename(t *testing.T, src span.Span, want *source.PrepareItem) { - ctx := context.Background() - f, err := r.view.GetFile(ctx, src.URI()) - if err != nil { - t.Fatalf("failed for %v: %v", src, err) - } _, srcRng, err := spanToRange(r.data, src) if err != nil { t.Fatal(err) } // Find the identifier at the position. - ident, err := source.Identifier(ctx, r.view.Snapshot(), f, srcRng.Start, source.WidestCheckPackageHandle) + fh, err := r.view.Snapshot().GetFile(r.ctx, src.URI()) + if err != nil { + t.Fatal(err) + } + ident, err := source.Identifier(r.ctx, r.view.Snapshot(), fh, srcRng.Start, source.WidestCheckPackageHandle) if err != nil { if want.Text != "" { // expected an ident. t.Errorf("prepare rename failed for %v: got error: %v", src, err) } return } - item, err := ident.PrepareRename(ctx) + item, err := ident.PrepareRename(r.ctx) if err != nil { if want.Text != "" { // expected an ident. t.Errorf("prepare rename failed for %v: got error: %v", src, err) @@ -833,12 +819,11 @@ func (r *runner) PrepareRename(t *testing.T, src span.Span, want *source.Prepare } func (r *runner) Symbols(t *testing.T, uri span.URI, expectedSymbols []protocol.DocumentSymbol) { - ctx := r.ctx - f, err := r.view.GetFile(ctx, uri) + fh, err := r.view.Snapshot().GetFile(r.ctx, uri) if err != nil { - t.Fatalf("failed for %v: %v", uri, err) + t.Fatal(err) } - symbols, err := source.DocumentSymbols(ctx, r.view.Snapshot(), f) + symbols, err := source.DocumentSymbols(r.ctx, r.view.Snapshot(), fh) if err != nil { t.Errorf("symbols failed for %s: %v", uri, err) } @@ -895,16 +880,15 @@ func summarizeSymbols(t *testing.T, i int, want, got []protocol.DocumentSymbol, } func (r *runner) SignatureHelp(t *testing.T, spn span.Span, expectedSignature *source.SignatureInformation) { - ctx := r.ctx - f, err := r.view.GetFile(ctx, spn.URI()) - if err != nil { - t.Fatalf("failed for %v: %v", spn, err) - } _, rng, err := spanToRange(r.data, spn) if err != nil { t.Fatal(err) } - gotSignature, err := source.SignatureHelp(ctx, r.view.Snapshot(), f, rng.Start) + fh, err := r.view.Snapshot().GetFile(r.ctx, spn.URI()) + if err != nil { + t.Fatal(err) + } + gotSignature, err := source.SignatureHelp(r.ctx, r.view.Snapshot(), fh, rng.Start) if err != nil { // Only fail if we got an error we did not expect. if expectedSignature != nil { diff --git a/internal/lsp/source/symbols.go b/internal/lsp/source/symbols.go index a2fc1bedb0..eabd09ca53 100644 --- a/internal/lsp/source/symbols.go +++ b/internal/lsp/source/symbols.go @@ -14,11 +14,11 @@ import ( "golang.org/x/tools/internal/telemetry/trace" ) -func DocumentSymbols(ctx context.Context, snapshot Snapshot, f File) ([]protocol.DocumentSymbol, error) { +func DocumentSymbols(ctx context.Context, snapshot Snapshot, fh FileHandle) ([]protocol.DocumentSymbol, error) { ctx, done := trace.StartSpan(ctx, "source.DocumentSymbols") defer done() - pkg, pgh, err := getParsedFile(ctx, snapshot, f, NarrowestCheckPackageHandle) + pkg, pgh, err := getParsedFile(ctx, snapshot, fh, NarrowestCheckPackageHandle) if err != nil { return nil, fmt.Errorf("getting file for DocumentSymbols: %v", err) } diff --git a/internal/lsp/source/util.go b/internal/lsp/source/util.go index a84c70c981..5588165827 100644 --- a/internal/lsp/source/util.go +++ b/internal/lsp/source/util.go @@ -67,8 +67,7 @@ func (s mappedRange) URI() span.URI { // getParsedFile is a convenience function that extracts the Package and ParseGoHandle for a File in a Snapshot. // selectPackage is typically Narrowest/WidestCheckPackageHandle below. -func getParsedFile(ctx context.Context, snapshot Snapshot, f File, selectPackage PackagePolicy) (Package, ParseGoHandle, error) { - fh := snapshot.Handle(ctx, f) +func getParsedFile(ctx context.Context, snapshot Snapshot, fh FileHandle, selectPackage PackagePolicy) (Package, ParseGoHandle, error) { phs, err := snapshot.PackageHandles(ctx, fh) if err != nil { return nil, nil, err @@ -81,7 +80,7 @@ func getParsedFile(ctx context.Context, snapshot Snapshot, f File, selectPackage if err != nil { return nil, nil, err } - pgh, err := pkg.File(f.URI()) + pgh, err := pkg.File(fh.Identity().URI) return pkg, pgh, err } @@ -143,11 +142,11 @@ func SpecificPackageHandle(desiredID string) PackagePolicy { } func IsGenerated(ctx context.Context, view View, uri span.URI) bool { - f, err := view.GetFile(ctx, uri) + fh, err := view.Snapshot().GetFile(ctx, uri) if err != nil { return false } - ph := view.Session().Cache().ParseGoHandle(view.Snapshot().Handle(ctx, f), ParseHeader) + ph := view.Session().Cache().ParseGoHandle(fh, ParseHeader) parsed, _, _, err := ph.Parse(ctx) if err != nil { return false diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go index c64cae3f06..19a27a3808 100644 --- a/internal/lsp/source/view.go +++ b/internal/lsp/source/view.go @@ -24,8 +24,13 @@ type Snapshot interface { // View returns the View associated with this snapshot. View() View - // Handle returns the FileHandle for the given file. - Handle(ctx context.Context, f File) FileHandle + // GetFile returns the file object for a given URI, initializing it + // if it is not already part of the view. + GetFile(ctx context.Context, uri span.URI) (FileHandle, error) + + // FindFile returns the file object for a given URI if it is + // already part of the view. + FindFile(ctx context.Context, uri span.URI) FileHandle // Analyze runs the analyses for the given package at this snapshot. Analyze(ctx context.Context, id string, analyzers []*analysis.Analyzer) ([]*Error, error) @@ -92,14 +97,6 @@ type View interface { // BuiltinPackage returns the type information for the special "builtin" package. BuiltinPackage() BuiltinPackage - // GetFile returns the file object for a given URI, initializing it - // if it is not already part of the view. - GetFile(ctx context.Context, uri span.URI) (File, error) - - // FindFile returns the file object for a given URI if it is - // already part of the view. - FindFile(ctx context.Context, uri span.URI) File - // BackgroundContext returns a context used for all background processing // on behalf of this view. BackgroundContext() context.Context @@ -319,12 +316,6 @@ func (fileID FileIdentity) String() string { return fmt.Sprintf("%s%s%s", fileID.URI, fileID.Identifier, fileID.Kind) } -// File represents a source file of any type. -type File interface { - URI() span.URI - Kind() FileKind -} - // FileKind describes the kind of the file in question. // It can be one of Go, mod, or sum. type FileKind int diff --git a/internal/lsp/symbols.go b/internal/lsp/symbols.go index 0855ace928..4d6b20ab95 100644 --- a/internal/lsp/symbols.go +++ b/internal/lsp/symbols.go @@ -25,15 +25,14 @@ func (s *Server) documentSymbol(ctx context.Context, params *protocol.DocumentSy return nil, err } snapshot := view.Snapshot() - f, err := view.GetFile(ctx, uri) + fh, err := snapshot.GetFile(ctx, uri) if err != nil { return nil, err } - var symbols []protocol.DocumentSymbol - switch f.Kind() { + switch fh.Identity().Kind { case source.Go: - symbols, err = source.DocumentSymbols(ctx, snapshot, f) + symbols, err = source.DocumentSymbols(ctx, snapshot, fh) case source.Mod: return []protocol.DocumentSymbol{}, nil } diff --git a/internal/lsp/text_synchronization.go b/internal/lsp/text_synchronization.go index 09a0f9ca46..490b0e22e2 100644 --- a/internal/lsp/text_synchronization.go +++ b/internal/lsp/text_synchronization.go @@ -29,16 +29,16 @@ func (s *Server) didOpen(ctx context.Context, params *protocol.DidOpenTextDocume if err != nil { return err } - snapshot, view, err := snapshotOf(s.session, uri, snapshots) + snapshot, _, err := snapshotOf(s.session, uri, snapshots) if err != nil { return err } - f, err := view.GetFile(ctx, uri) + fh, err := snapshot.GetFile(ctx, uri) if err != nil { return err } // Always run diagnostics when a file is opened. - return s.diagnose(snapshot, f) + return s.diagnose(snapshot, fh) } func (s *Server) didChange(ctx context.Context, params *protocol.DidChangeTextDocumentParams) error { @@ -68,12 +68,12 @@ func (s *Server) didChange(ctx context.Context, params *protocol.DidChangeTextDo Type: protocol.Warning, }) } - f, err := view.GetFile(ctx, uri) + fh, err := snapshot.GetFile(ctx, uri) if err != nil { return err } // Always update diagnostics after a file change. - return s.diagnose(snapshot, f) + return s.diagnose(snapshot, fh) } func (s *Server) didSave(ctx context.Context, params *protocol.DidSaveTextDocumentParams) error { diff --git a/internal/lsp/watched_files.go b/internal/lsp/watched_files.go index dd0bc73d46..fb97d878db 100644 --- a/internal/lsp/watched_files.go +++ b/internal/lsp/watched_files.go @@ -34,20 +34,20 @@ func (s *Server) didChangeWatchedFiles(ctx context.Context, params *protocol.Did if s.session.DidChangeOutOfBand(ctx, uri, action) { // If we had been tracking the given file, // recompute diagnostics to reflect updated file contents. - f, err := view.GetFile(ctx, uri) + snapshot := view.Snapshot() + fh, err := snapshot.GetFile(ctx, uri) if err != nil { return err } - return s.diagnose(view.Snapshot(), f) + return s.diagnose(snapshot, fh) } case source.Delete: - f := view.FindFile(ctx, uri) + snapshot := view.Snapshot() + fh := snapshot.FindFile(ctx, uri) // If we have never seen this file before, there is nothing to do. - if f == nil { + if fh == nil { continue } - snapshot := view.Snapshot() - fh := snapshot.Handle(ctx, f) phs, err := snapshot.PackageHandles(ctx, fh) if err != nil { log.Error(ctx, "didChangeWatchedFiles: CheckPackageHandles", err, telemetry.File) @@ -60,12 +60,12 @@ func (s *Server) didChangeWatchedFiles(ctx context.Context, params *protocol.Did } // Find a different file in the same package we can use to trigger diagnostics. // TODO(rstambler): Allow diagnostics to be called per-package to avoid this. - var otherFile source.File + var otherFile source.FileHandle for _, pgh := range ph.CompiledGoFiles() { - if pgh.File().Identity().URI == f.URI() { + if pgh.File().Identity().URI == fh.Identity().URI { continue } - if f := view.FindFile(ctx, pgh.File().Identity().URI); f != nil && s.session.IsOpen(f.URI()) { + if f := snapshot.FindFile(ctx, pgh.File().Identity().URI); f != nil && s.session.IsOpen(fh.Identity().URI) { otherFile = f break }