diff --git a/internal/lsp/cache/session.go b/internal/lsp/cache/session.go index 7beb610e2a..2f1abf071f 100644 --- a/internal/lsp/cache/session.go +++ b/internal/lsp/cache/session.go @@ -31,7 +31,7 @@ type session struct { viewMu sync.Mutex views []*view - viewMap map[span.URI]source.View + viewMap map[span.URI]*view overlayMu sync.Mutex overlays map[span.URI]*overlay @@ -85,7 +85,7 @@ func (s *session) NewView(ctx context.Context, name string, folder span.URI, opt } s.views = append(s.views, v) // we always need to drop the view map - s.viewMap = make(map[span.URI]source.View) + s.viewMap = make(map[span.URI]*view) return v, snapshot, nil } @@ -167,6 +167,10 @@ func (s *session) View(name string) source.View { // ViewOf returns a view corresponding to the given URI. // If the file is not already associated with a view, pick one using some heuristics. func (s *session) ViewOf(uri span.URI) (source.View, error) { + return s.viewOf(uri) +} + +func (s *session) viewOf(uri span.URI) (*view, error) { s.viewMu.Lock() defer s.viewMu.Unlock() @@ -208,12 +212,12 @@ func (s *session) Views() []source.View { // bestView finds the best view to associate a given URI with. // viewMu must be held when calling this method. -func (s *session) bestView(uri span.URI) (source.View, error) { +func (s *session) bestView(uri span.URI) (*view, error) { if len(s.views) == 0 { return nil, errors.Errorf("no views in the session") } // we need to find the best view for this file - var longest source.View + var longest *view for _, view := range s.views { if longest != nil && len(longest.Folder()) > len(view.Folder()) { continue @@ -265,22 +269,51 @@ func (s *session) updateView(ctx context.Context, view *view, options source.Opt return v, snapshot, nil } -func (s *session) dropView(ctx context.Context, view *view) (int, error) { +func (s *session) dropView(ctx context.Context, v *view) (int, error) { // we always need to drop the view map - s.viewMap = make(map[span.URI]source.View) - for i, v := range s.views { - if view == v { + s.viewMap = make(map[span.URI]*view) + for i := range s.views { + if v == s.views[i] { // we found the view, drop it and return the index it was found at s.views[i] = nil v.shutdown(ctx) return i, nil } } - return -1, errors.Errorf("view %s for %v not found", view.Name(), view.Folder()) + return -1, errors.Errorf("view %s for %v not found", v.Name(), v.Folder()) +} + +func (s *session) DidModifyFile(ctx context.Context, c source.FileModification) error { + ctx = telemetry.URI.With(ctx, c.URI) + for _, view := range s.viewsOf(c.URI) { + switch c.Action { + case source.Open: + kind := source.DetectLanguage(c.LanguageID, c.URI.Filename()) + if kind == source.UnknownKind { + return errors.Errorf("DidModifyFile: unknown file kind for %s", c.URI) + } + return s.didOpen(ctx, c.URI, kind, c.Version, c.Text) + case source.Save: + s.didSave(c.URI, c.Version) + case source.Close: + s.didClose(c.URI) + } + + // Set the content for the file, only for didChange and didClose events. + switch c.Action { + case source.Change, source.Close: + f, err := view.GetFile(ctx, c.URI) + if err != nil { + return err + } + view.setContent(ctx, f, c.Version, c.Text) + } + } + return nil } // TODO: Propagate the language ID through to the view. -func (s *session) DidOpen(ctx context.Context, uri span.URI, kind source.FileKind, version float64, 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. @@ -311,7 +344,7 @@ func (s *session) DidOpen(ctx context.Context, uri span.URI, kind source.FileKin return nil } -func (s *session) DidSave(uri span.URI, version float64) { +func (s *session) didSave(uri span.URI, version float64) { s.overlayMu.Lock() defer s.overlayMu.Unlock() @@ -321,7 +354,7 @@ func (s *session) DidSave(uri span.URI, version float64) { } } -func (s *session) DidClose(uri span.URI) { +func (s *session) didClose(uri span.URI) { s.openFiles.Delete(uri) } @@ -338,22 +371,22 @@ 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, version float64, data []byte) { +func (s *session) setOverlay(f source.File, version float64, data []byte) { s.overlayMu.Lock() defer func() { s.overlayMu.Unlock() - s.filesWatchMap.Notify(uri, source.Change) + s.filesWatchMap.Notify(f.URI(), source.Change) }() if data == nil { - delete(s.overlays, uri) + delete(s.overlays, f.URI()) return } - s.overlays[uri] = &overlay{ + s.overlays[f.URI()] = &overlay{ session: s, - uri: uri, - kind: kind, + uri: f.URI(), + kind: f.Kind(), data: data, hash: hashContents(data), version: version, diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go index 6cd68b6d72..0d87755dd4 100644 --- a/internal/lsp/cache/view.go +++ b/internal/lsp/cache/view.go @@ -323,12 +323,12 @@ func (v *view) getSnapshot() *snapshot { return v.snapshot } -// SetContent sets the overlay contents for a file. -func (v *view) SetContent(ctx context.Context, uri span.URI, version float64, content []byte) { +// setContent sets the overlay contents for a file. +func (v *view) setContent(ctx context.Context, f source.File, version float64, content []byte) { v.mu.Lock() defer v.mu.Unlock() - if v.Ignore(uri) { + if v.Ignore(f.URI()) { return } @@ -337,8 +337,7 @@ func (v *view) SetContent(ctx context.Context, uri span.URI, version float64, co v.cancel() v.backgroundCtx, v.cancel = context.WithCancel(v.baseCtx) - kind := source.DetectLanguage("", uri.Filename()) - v.session.SetOverlay(uri, kind, version, content) + v.session.setOverlay(f, version, content) } // FindFile returns the file if the given URI is already a part of the view. diff --git a/internal/lsp/diagnostics.go b/internal/lsp/diagnostics.go index 40fff9d205..7a709da1a5 100644 --- a/internal/lsp/diagnostics.go +++ b/internal/lsp/diagnostics.go @@ -11,11 +11,20 @@ import ( "golang.org/x/tools/internal/lsp/protocol" "golang.org/x/tools/internal/lsp/source" "golang.org/x/tools/internal/lsp/telemetry" - "golang.org/x/tools/internal/span" "golang.org/x/tools/internal/telemetry/log" "golang.org/x/tools/internal/telemetry/trace" ) +func (s *Server) diagnose(snapshot source.Snapshot, f source.File) error { + switch f.Kind() { + case source.Go: + go s.diagnoseFile(snapshot, f) + case source.Mod: + go s.diagnoseSnapshot(snapshot) + } + return nil +} + func (s *Server) diagnoseSnapshot(snapshot source.Snapshot) { ctx := snapshot.View().BackgroundContext() ctx, done := trace.StartSpan(ctx, "lsp:background-worker") @@ -50,18 +59,13 @@ func (s *Server) diagnoseSnapshot(snapshot source.Snapshot) { } } -func (s *Server) diagnoseFile(snapshot source.Snapshot, uri span.URI) { +func (s *Server) diagnoseFile(snapshot source.Snapshot, f source.File) { ctx := snapshot.View().BackgroundContext() ctx, done := trace.StartSpan(ctx, "lsp:background-worker") defer done() - ctx = telemetry.File.With(ctx, uri) + ctx = telemetry.File.With(ctx, f.URI()) - f, err := snapshot.View().GetFile(ctx, uri) - if err != nil { - log.Error(ctx, "diagnoseFile: no file", err) - return - } reports, warningMsg, err := source.Diagnostics(ctx, snapshot, f, true, snapshot.View().Options().DisabledAnalyses) // Check the warning message first. if warningMsg != "" { diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go index 2a1033e797..3ff4642d80 100644 --- a/internal/lsp/lsp_test.go +++ b/internal/lsp/lsp_test.go @@ -57,7 +57,19 @@ 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), -1, content) + kind := source.DetectLanguage("", filename) + if kind != source.Go { + continue + } + if err := session.DidModifyFile(ctx, source.FileModification{ + URI: span.FileURI(filename), + Action: source.Open, + Version: -1, + Text: content, + LanguageID: "go", + }); err != nil { + t.Fatal(err) + } } r := &runner{ server: &Server{ diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go index f30e2914bc..9c7a5cdefd 100644 --- a/internal/lsp/source/source_test.go +++ b/internal/lsp/source/source_test.go @@ -61,7 +61,19 @@ 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), -1, content) + kind := source.DetectLanguage("", filename) + if kind != source.Go { + continue + } + if err := session.DidModifyFile(ctx, source.FileModification{ + URI: span.FileURI(filename), + Action: source.Open, + Version: -1, + Text: content, + LanguageID: "go", + }); err != nil { + t.Fatal(err) + } } tests.Run(t, r, data) } diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go index 6e7a140599..f4cb971d1d 100644 --- a/internal/lsp/source/view.go +++ b/internal/lsp/source/view.go @@ -99,9 +99,6 @@ type View interface { // already part of the view. 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, version float64, content []byte) - // BackgroundContext returns a context used for all background processing // on behalf of this view. BackgroundContext() context.Context @@ -167,20 +164,11 @@ type Session interface { // content from the underlying cache if no overlay is present. FileSystem - // DidOpen is invoked each time a file is opened in the editor. - 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, version float64) - - // DidClose is invoked each time an open file is closed in the editor. - DidClose(uri span.URI) - // IsOpen returns whether the editor currently has a file open. IsOpen(uri span.URI) bool - // Called to set the effective contents of a file from this session. - SetOverlay(uri span.URI, kind FileKind, version float64, data []byte) + // DidModifyFile reports a file modification to the session. + DidModifyFile(ctx context.Context, c FileModification) error // DidChangeOutOfBand is called when a file under the root folder changes. // If the file was open in the editor, it returns true. diff --git a/internal/lsp/text_synchronization.go b/internal/lsp/text_synchronization.go index 6496cf3829..91125354d4 100644 --- a/internal/lsp/text_synchronization.go +++ b/internal/lsp/text_synchronization.go @@ -12,7 +12,6 @@ import ( "golang.org/x/tools/internal/jsonrpc2" "golang.org/x/tools/internal/lsp/protocol" "golang.org/x/tools/internal/lsp/source" - "golang.org/x/tools/internal/lsp/telemetry" "golang.org/x/tools/internal/span" errors "golang.org/x/xerrors" ) @@ -20,13 +19,25 @@ import ( func (s *Server) didOpen(ctx context.Context, params *protocol.DidOpenTextDocumentParams) error { // Confirm that the file's language ID is related to Go. uri := span.NewURI(params.TextDocument.URI) - return s.didModifyFile(ctx, source.FileModification{ + if err := s.session.DidModifyFile(ctx, source.FileModification{ URI: uri, Action: source.Open, Version: params.TextDocument.Version, Text: []byte(params.TextDocument.Text), LanguageID: params.TextDocument.LanguageID, - }) + }); err != nil { + return err + } + view, err := s.session.ViewOf(uri) + if err != nil { + return err + } + f, err := view.GetFile(ctx, uri) + if err != nil { + return err + } + // Always run diagnostics when a file is opened. + return s.diagnose(view.Snapshot(), f) } func (s *Server) didSave(ctx context.Context, params *protocol.DidSaveTextDocumentParams) error { @@ -38,11 +49,11 @@ func (s *Server) didSave(ctx context.Context, params *protocol.DidSaveTextDocume if params.Text != nil { c.Text = []byte(*params.Text) } - return s.didModifyFile(ctx, c) + return s.session.DidModifyFile(ctx, c) } func (s *Server) didClose(ctx context.Context, params *protocol.DidCloseTextDocumentParams) error { - return s.didModifyFile(ctx, source.FileModification{ + return s.session.DidModifyFile(ctx, source.FileModification{ URI: span.NewURI(params.TextDocument.URI), Action: source.Close, Version: -1, @@ -56,67 +67,32 @@ func (s *Server) didChange(ctx context.Context, params *protocol.DidChangeTextDo if err != nil { return err } - return s.didModifyFile(ctx, source.FileModification{ + if err := s.session.DidModifyFile(ctx, source.FileModification{ URI: uri, Action: source.Change, Version: params.TextDocument.Version, Text: text, - }) -} - -// didModifyFile propagates the information about the file modification -// to the cache layer and runs diagnostics. -// -// TODO(rstambler): This function should be mostly unnecessary once we unify the methods -// for making changes to a file in internal/lsp/cache. -func (s *Server) didModifyFile(ctx context.Context, c source.FileModification) error { - ctx = telemetry.URI.With(ctx, c.URI) - - view, err := s.session.ViewOf(c.URI) + }); err != nil { + return err + } + view, err := s.session.ViewOf(uri) if err != nil { return err } - switch c.Action { - case source.Open: - kind := source.DetectLanguage(c.LanguageID, c.URI.Filename()) - if kind == source.UnknownKind { - return errors.Errorf("didModifyFile: unknown file kind for %s", c.URI) - } - if err := s.session.DidOpen(ctx, c.URI, kind, c.Version, c.Text); err != nil { - return err - } - case source.Change: - view.SetContent(ctx, c.URI, c.Version, c.Text) - - // Ideally, we should be able to specify that a generated file should be opened as read-only. - // Tell the user that they should not be editing a generated file. - if s.wasFirstChange(c.URI) && source.IsGenerated(ctx, view, c.URI) { - s.client.ShowMessage(ctx, &protocol.ShowMessageParams{ - Message: fmt.Sprintf("Do not edit this file! %s is a generated file.", c.URI.Filename()), - Type: protocol.Warning, - }) - } - case source.Save: - s.session.DidSave(c.URI, c.Version) - case source.Close: - s.session.DidClose(c.URI) - view.SetContent(ctx, c.URI, c.Version, c.Text) + // Ideally, we should be able to specify that a generated file should be opened as read-only. + // Tell the user that they should not be editing a generated file. + if s.wasFirstChange(uri) && source.IsGenerated(ctx, view, uri) { + s.client.ShowMessage(ctx, &protocol.ShowMessageParams{ + Message: fmt.Sprintf("Do not edit this file! %s is a generated file.", uri.Filename()), + Type: protocol.Warning, + }) } - - // We should run diagnostics after opening or changing a file. - switch c.Action { - case source.Open, source.Change: - f, err := view.GetFile(ctx, c.URI) - if err != nil { - return err - } - if f.Kind() == source.Mod { - go s.diagnoseSnapshot(view.Snapshot()) - } else { - go s.diagnoseFile(view.Snapshot(), c.URI) - } + f, err := view.GetFile(ctx, uri) + if err != nil { + return err } - return nil + // Always update diagnostics after a file change. + return s.diagnose(view.Snapshot(), f) } func (s *Server) wasFirstChange(uri span.URI) bool { diff --git a/internal/lsp/watched_files.go b/internal/lsp/watched_files.go index 30c243ec5c..dd0bc73d46 100644 --- a/internal/lsp/watched_files.go +++ b/internal/lsp/watched_files.go @@ -34,7 +34,11 @@ 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. - go s.diagnoseFile(view.Snapshot(), uri) + f, err := view.GetFile(ctx, uri) + if err != nil { + return err + } + return s.diagnose(view.Snapshot(), f) } case source.Delete: f := view.FindFile(ctx, uri) @@ -82,7 +86,7 @@ func (s *Server) didChangeWatchedFiles(ctx context.Context, params *protocol.Did } // Refresh diagnostics for the package the file belonged to. - go s.diagnoseFile(view.Snapshot(), otherFile.URI()) + go s.diagnoseFile(view.Snapshot(), otherFile) } } }