From e2727e816f5aa0f0179e2ee7ec02d5546cd1c5ba Mon Sep 17 00:00:00 2001 From: Rebecca Stambler Date: Thu, 7 Nov 2019 17:52:35 -0500 Subject: [PATCH] internal/lsp: use the versions provided by the client This change propagates the versions sent by the client to the overlay so that they can be used when sending text edits for code actions and renames. Fixes golang/go#35243 Change-Id: I8d1eb86fe9f666f7aa287be5026b176b46712c97 Reviewed-on: https://go-review.googlesource.com/c/tools/+/205863 Run-TryBot: Rebecca Stambler Reviewed-by: Ian Cottrell --- internal/lsp/cache/session.go | 13 ++++++++----- internal/lsp/cache/snapshot.go | 4 ++-- internal/lsp/cache/view.go | 12 ++++++------ internal/lsp/lsp_test.go | 2 +- internal/lsp/source/source_test.go | 2 +- internal/lsp/source/view.go | 20 ++++++++++---------- internal/lsp/text_synchronization.go | 9 +++++---- 7 files changed, 33 insertions(+), 29 deletions(-) diff --git a/internal/lsp/cache/session.go b/internal/lsp/cache/session.go index a7b57a1931..1b8cd3522f 100644 --- a/internal/lsp/cache/session.go +++ b/internal/lsp/cache/session.go @@ -279,7 +279,7 @@ func (s *session) dropView(ctx context.Context, view *view) (int, error) { } // TODO: Propagate the language ID through to the view. -func (s *session) DidOpen(ctx context.Context, uri span.URI, kind source.FileKind, text []byte) error { +func (s *session) DidOpen(ctx context.Context, uri span.URI, kind source.FileKind, version float64, text []byte) error { ctx = telemetry.File.With(ctx, uri) // Files with _ prefixes are ignored. @@ -303,16 +303,17 @@ func (s *session) DidOpen(ctx context.Context, uri span.URI, kind source.FileKin // Read the file on disk and compare it to the text provided. // If it is the same as on disk, we can avoid sending it as an overlay to go/packages. - s.openOverlay(ctx, uri, kind, text) + s.openOverlay(ctx, uri, kind, version, text) return nil } -func (s *session) DidSave(uri span.URI) { +func (s *session) DidSave(uri span.URI, version float64) { s.overlayMu.Lock() defer s.overlayMu.Unlock() if overlay, ok := s.overlays[uri]; ok { overlay.sameContentOnDisk = true + overlay.version = version } } @@ -333,7 +334,7 @@ func (s *session) GetFile(uri span.URI, kind source.FileKind) source.FileHandle return s.cache.GetFile(uri, kind) } -func (s *session) SetOverlay(uri span.URI, kind source.FileKind, data []byte) bool { +func (s *session) SetOverlay(uri span.URI, kind source.FileKind, version float64, data []byte) bool { s.overlayMu.Lock() defer func() { s.overlayMu.Unlock() @@ -354,6 +355,7 @@ func (s *session) SetOverlay(uri span.URI, kind source.FileKind, data []byte) bo kind: kind, data: data, hash: hashContents(data), + version: version, unchanged: o == nil, } return firstChange @@ -368,7 +370,7 @@ func (s *session) clearOverlay(uri span.URI) { // openOverlay adds the file content to the overlay. // It also checks if the provided content is equivalent to the file's content on disk. -func (s *session) openOverlay(ctx context.Context, uri span.URI, kind source.FileKind, data []byte) { +func (s *session) openOverlay(ctx context.Context, uri span.URI, kind source.FileKind, version float64, data []byte) { s.overlayMu.Lock() defer func() { s.overlayMu.Unlock() @@ -381,6 +383,7 @@ func (s *session) openOverlay(ctx context.Context, uri span.URI, kind source.Fil data: data, hash: hashContents(data), unchanged: true, + version: version, } // If the file is on disk, check if its content is the same as the overlay. if _, hash, err := s.cache.GetFile(uri, kind).Read(ctx); err == nil { diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go index bece4f1d09..db332dc207 100644 --- a/internal/lsp/cache/snapshot.go +++ b/internal/lsp/cache/snapshot.go @@ -352,7 +352,7 @@ func (s *snapshot) clone(ctx context.Context, withoutURI *span.URI, withoutTypes // invalidateContent invalidates the content of a Go file, // including any position and type information that depends on it. -func (v *view) invalidateContent(ctx context.Context, f source.File, kind source.FileKind, changeType protocol.FileChangeType) bool { +func (v *view) invalidateContent(ctx context.Context, f source.File, changeType protocol.FileChangeType) bool { var ( withoutTypes = make(map[span.URI]struct{}) withoutMetadata = make(map[span.URI]struct{}) @@ -407,7 +407,7 @@ func (v *view) invalidateContent(ctx context.Context, f source.File, kind source } // Get the current FileHandle for the URI. - currentFH := v.session.GetFile(f.URI(), kind) + currentFH := v.session.GetFile(f.URI(), f.Kind()) // Check if the file's package name or imports have changed, // and if so, invalidate metadata. diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go index d872ddddb5..155a3743ee 100644 --- a/internal/lsp/cache/view.go +++ b/internal/lsp/cache/view.go @@ -273,8 +273,8 @@ func (v *view) storeModFileVersions() { func (v *view) fileVersion(filename string, kind source.FileKind) string { uri := span.FileURI(filename) - f := v.session.GetFile(uri, kind) - return f.Identity().Identifier + fh := v.session.GetFile(uri, kind) + return fh.Identity().String() } func (v *view) Shutdown(ctx context.Context) { @@ -324,7 +324,7 @@ func (v *view) getSnapshot() *snapshot { } // SetContent sets the overlay contents for a file. -func (v *view) SetContent(ctx context.Context, uri span.URI, content []byte) (bool, error) { +func (v *view) SetContent(ctx context.Context, uri span.URI, version float64, content []byte) (bool, error) { v.mu.Lock() defer v.mu.Unlock() @@ -338,7 +338,7 @@ func (v *view) SetContent(ctx context.Context, uri span.URI, content []byte) (bo } kind := source.DetectLanguage("", uri.Filename()) - return v.session.SetOverlay(uri, kind, content), nil + return v.session.SetOverlay(uri, kind, version, content), nil } // FindFile returns the file if the given URI is already a part of the view. @@ -374,11 +374,11 @@ func (v *view) getFile(ctx context.Context, uri span.URI, kind source.FileKind) f = &fileBase{ view: v, fname: uri.Filename(), - kind: source.Go, + kind: kind, } v.session.filesWatchMap.Watch(uri, func(changeType protocol.FileChangeType) bool { ctx := xcontext.Detach(ctx) - return v.invalidateContent(ctx, f, kind, changeType) + return v.invalidateContent(ctx, f, changeType) }) v.mapFile(uri, f) return f, nil diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go index f20b967de8..c4daa4bf21 100644 --- a/internal/lsp/lsp_test.go +++ b/internal/lsp/lsp_test.go @@ -58,7 +58,7 @@ func testLSP(t *testing.T, exporter packagestest.Exporter) { t.Fatal(err) } for filename, content := range data.Config.Overlay { - session.SetOverlay(span.FileURI(filename), source.DetectLanguage("", filename), content) + session.SetOverlay(span.FileURI(filename), source.DetectLanguage("", filename), -1, content) } r := &runner{ server: &Server{ diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go index 4403d06dea..3ed53a50d0 100644 --- a/internal/lsp/source/source_test.go +++ b/internal/lsp/source/source_test.go @@ -61,7 +61,7 @@ func testSource(t *testing.T, exporter packagestest.Exporter) { ctx: ctx, } for filename, content := range data.Config.Overlay { - session.SetOverlay(span.FileURI(filename), source.DetectLanguage("", filename), content) + session.SetOverlay(span.FileURI(filename), source.DetectLanguage("", filename), -1, content) } tests.Run(t, r, data) } diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go index 934743a328..aa1e2a8181 100644 --- a/internal/lsp/source/view.go +++ b/internal/lsp/source/view.go @@ -57,6 +57,12 @@ type FileSystem interface { GetFile(uri span.URI, kind FileKind) FileHandle } +// 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 @@ -172,10 +178,10 @@ type Session interface { FileSystem // DidOpen is invoked each time a file is opened in the editor. - DidOpen(ctx context.Context, uri span.URI, kind FileKind, text []byte) error + DidOpen(ctx context.Context, uri span.URI, kind FileKind, version float64, text []byte) error // DidSave is invoked each time an open file is saved in the editor. - DidSave(uri span.URI) + DidSave(uri span.URI, version float64) // DidClose is invoked each time an open file is closed in the editor. DidClose(uri span.URI) @@ -184,7 +190,7 @@ type Session interface { IsOpen(uri span.URI) bool // Called to set the effective contents of a file from this session. - SetOverlay(uri span.URI, kind FileKind, data []byte) (wasFirstChange bool) + SetOverlay(uri span.URI, kind FileKind, version float64, data []byte) (wasFirstChange bool) // DidChangeOutOfBand is called when a file under the root folder changes. // If the file was open in the editor, it returns true. @@ -222,7 +228,7 @@ type View interface { FindFile(ctx context.Context, uri span.URI) File // Called to set the effective contents of a file from this view. - SetContent(ctx context.Context, uri span.URI, content []byte) (wasFirstChange bool, err error) + SetContent(ctx context.Context, uri span.URI, version float64, content []byte) (wasFirstChange bool, err error) // BackgroundContext returns a context used for all background processing // on behalf of this view. @@ -293,12 +299,6 @@ type Snapshot interface { KnownPackages(ctx context.Context) []Package } -// File represents a source file of any type. -type File interface { - URI() span.URI - Kind() FileKind -} - type FileURI span.URI func (f FileURI) URI() span.URI { diff --git a/internal/lsp/text_synchronization.go b/internal/lsp/text_synchronization.go index 4019daca12..fbb67bfb25 100644 --- a/internal/lsp/text_synchronization.go +++ b/internal/lsp/text_synchronization.go @@ -21,12 +21,13 @@ import ( func (s *Server) didOpen(ctx context.Context, params *protocol.DidOpenTextDocumentParams) error { uri := span.NewURI(params.TextDocument.URI) text := []byte(params.TextDocument.Text) + version := params.TextDocument.Version // Confirm that the file's language ID is related to Go. fileKind := source.DetectLanguage(params.TextDocument.LanguageID, uri.Filename()) // Open the file. - s.session.DidOpen(ctx, uri, fileKind, text) + s.session.DidOpen(ctx, uri, fileKind, version, text) view := s.session.ViewOf(uri) @@ -64,7 +65,7 @@ func (s *Server) didChange(ctx context.Context, params *protocol.DidChangeTextDo } // Cache the new file content and send fresh diagnostics. view := s.session.ViewOf(uri) - wasFirstChange, err := view.SetContent(ctx, uri, []byte(text)) + wasFirstChange, err := view.SetContent(ctx, uri, params.TextDocument.Version, []byte(text)) if err != nil { return err } @@ -130,7 +131,7 @@ func (s *Server) applyChanges(ctx context.Context, uri span.URI, changes []proto } func (s *Server) didSave(ctx context.Context, params *protocol.DidSaveTextDocumentParams) error { - s.session.DidSave(span.NewURI(params.TextDocument.URI)) + s.session.DidSave(span.NewURI(params.TextDocument.URI), params.TextDocument.Version) return nil } @@ -139,7 +140,7 @@ func (s *Server) didClose(ctx context.Context, params *protocol.DidCloseTextDocu ctx = telemetry.URI.With(ctx, uri) s.session.DidClose(uri) view := s.session.ViewOf(uri) - if _, err := view.SetContent(ctx, uri, nil); err != nil { + if _, err := view.SetContent(ctx, uri, -1, nil); err != nil { return err } clear := []span.URI{uri} // by default, clear the closed URI