diff --git a/gopls/internal/regtest/workspace_test.go b/gopls/internal/regtest/workspace_test.go index 9b76436a52..5a4c029a2c 100644 --- a/gopls/internal/regtest/workspace_test.go +++ b/gopls/internal/regtest/workspace_test.go @@ -10,6 +10,8 @@ import ( "testing" "golang.org/x/tools/internal/lsp" + "golang.org/x/tools/internal/lsp/fake" + "golang.org/x/tools/internal/testenv" ) const workspaceProxy = ` @@ -457,3 +459,26 @@ replace b.com => %s/modb } }) } + +func TestNonWorkspaceFileCreation(t *testing.T) { + testenv.NeedsGo1Point(t, 13) + + const files = ` +-- go.mod -- +module mod.com + +-- x.go -- +package x +` + + const code = ` +package foo +import "fmt" +var _ = fmt.Printf +` + run(t, files, func(t *testing.T, env *Env) { + env.CreateBuffer("/tmp/foo.go", "") + env.EditBuffer("/tmp/foo.go", fake.NewEdit(0, 0, 0, 0, code)) + env.GoToDefinition("/tmp/foo.go", env.RegexpSearch("/tmp/foo.go", `Printf`)) + }) +} diff --git a/internal/lsp/cache/session.go b/internal/lsp/cache/session.go index e5978737ce..e74a0d192b 100644 --- a/internal/lsp/cache/session.go +++ b/internal/lsp/cache/session.go @@ -430,16 +430,16 @@ func (s *Session) dropView(ctx context.Context, v *View) (int, error) { } func (s *Session) ModifyFiles(ctx context.Context, changes []source.FileModification) error { - _, releases, _, err := s.DidModifyFiles(ctx, changes) + _, _, releases, _, err := s.DidModifyFiles(ctx, changes) for _, release := range releases { release() } return err } -func (s *Session) DidModifyFiles(ctx context.Context, changes []source.FileModification) ([]source.Snapshot, []func(), []span.URI, error) { +func (s *Session) DidModifyFiles(ctx context.Context, changes []source.FileModification) (map[span.URI]source.View, map[source.View]source.Snapshot, []func(), []span.URI, error) { views := make(map[*View]map[span.URI]source.VersionedFileHandle) - + bestViews := map[span.URI]source.View{} // Keep track of deleted files so that we can clear their diagnostics. // A file might be re-created after deletion, so only mark files that // have truly been deleted. @@ -447,7 +447,7 @@ func (s *Session) DidModifyFiles(ctx context.Context, changes []source.FileModif overlays, err := s.updateOverlays(ctx, changes) if err != nil { - return nil, nil, nil, err + return nil, nil, nil, nil, err } var forceReloadMetadata bool for _, c := range changes { @@ -455,17 +455,32 @@ func (s *Session) DidModifyFiles(ctx context.Context, changes []source.FileModif forceReloadMetadata = true } - // Look through all of the session's views, invalidating the file for - // all of the views to which it is known. + // Build the list of affected views. + bestView, err := s.viewOf(c.URI) + if err != nil { + return nil, nil, nil, nil, err + } + bestViews[c.URI] = bestView + + var changedViews []*View for _, view := range s.views { // Don't propagate changes that are outside of the view's scope // or knowledge. if !view.relevantChange(c) { continue } + changedViews = append(changedViews, view) + } + // If no view matched the change, assign it to the best view. + if len(changedViews) == 0 { + changedViews = append(changedViews, bestView) + } + + // Apply the changes to all affected views. + for _, view := range changedViews { // Make sure that the file is added to the view. if _, err := view.getFile(c.URI); err != nil { - return nil, nil, nil, err + return nil, nil, nil, nil, err } if _, ok := views[view]; !ok { views[view] = make(map[span.URI]source.VersionedFileHandle) @@ -480,7 +495,7 @@ func (s *Session) DidModifyFiles(ctx context.Context, changes []source.FileModif } else { fsFile, err := s.cache.getFile(ctx, c.URI) if err != nil { - return nil, nil, nil, err + return nil, nil, nil, nil, err } fh = &closedFile{fsFile} views[view][c.URI] = fh @@ -490,18 +505,19 @@ func (s *Session) DidModifyFiles(ctx context.Context, changes []source.FileModif } } } - var snapshots []source.Snapshot + + snapshots := map[source.View]source.Snapshot{} var releases []func() for view, uris := range views { snapshot, release := view.invalidateContent(ctx, uris, forceReloadMetadata) - snapshots = append(snapshots, snapshot) + snapshots[view] = snapshot releases = append(releases, release) } var deletionsSlice []span.URI for uri := range deletions { deletionsSlice = append(deletionsSlice, uri) } - return snapshots, releases, deletionsSlice, nil + return bestViews, snapshots, releases, deletionsSlice, nil } func (s *Session) updateOverlays(ctx context.Context, changes []source.FileModification) (map[span.URI]*overlay, error) { diff --git a/internal/lsp/general.go b/internal/lsp/general.go index 1c0ebf2598..ff76d58dac 100644 --- a/internal/lsp/general.go +++ b/internal/lsp/general.go @@ -269,7 +269,7 @@ func (s *Server) addFolders(ctx context.Context, folders []protocol.WorkspaceFol // with the previously registered set of directories. If the set of directories // has changed, we unregister and re-register for file watching notifications. // updatedSnapshots is the set of snapshots that have been updated. -func (s *Server) updateWatchedDirectories(ctx context.Context, updatedSnapshots []source.Snapshot) error { +func (s *Server) updateWatchedDirectories(ctx context.Context, updatedSnapshots map[source.View]source.Snapshot) error { dirsToWatch := map[span.URI]struct{}{} seenViews := map[source.View]struct{}{} diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go index 90a5f4d09f..bbe95c4d94 100644 --- a/internal/lsp/source/view.go +++ b/internal/lsp/source/view.go @@ -269,7 +269,7 @@ type Session interface { // DidModifyFile reports a file modification to the session. It returns the // resulting snapshots, a guaranteed one per view. - DidModifyFiles(ctx context.Context, changes []FileModification) ([]Snapshot, []func(), []span.URI, error) + DidModifyFiles(ctx context.Context, changes []FileModification) (map[span.URI]View, map[View]Snapshot, []func(), []span.URI, error) // Overlays returns a slice of file overlays for the session. Overlays() []Overlay diff --git a/internal/lsp/text_synchronization.go b/internal/lsp/text_synchronization.go index 0a781147ee..805393c57e 100644 --- a/internal/lsp/text_synchronization.go +++ b/internal/lsp/text_synchronization.go @@ -186,7 +186,7 @@ func (s *Server) didModifyFiles(ctx context.Context, modifications []source.File }() }() } - snapshots, releases, deletions, err := s.session.DidModifyFiles(ctx, modifications) + views, snapshots, releases, deletions, err := s.session.DidModifyFiles(ctx, modifications) if err != nil { return err } @@ -201,43 +201,15 @@ func (s *Server) didModifyFiles(ctx context.Context, modifications []source.File return err } } - snapshotByURI := make(map[span.URI]source.Snapshot) - for _, c := range modifications { - snapshotByURI[c.URI] = nil - } - // Avoid diagnosing the same snapshot twice. - snapshotSet := make(map[source.Snapshot][]span.URI) - for uri := range snapshotByURI { - view, err := s.session.ViewOf(uri) - if err != nil { - return err - } - var snapshot source.Snapshot - for _, s := range snapshots { - if s.View() == view { - if snapshot != nil { - return errors.New("duplicate snapshots for the same view") - } - snapshot = s - } - } - // If the file isn't in any known views (for example, if it's in a dependency), - // we may not have a snapshot to map it to. As a result, we won't try to - // diagnose it. TODO(rstambler): Figure out how to handle this better. - if snapshot == nil { - continue - } - snapshotSet[snapshot] = append(snapshotSet[snapshot], uri) - snapshotByURI[uri] = snapshot - } + // Check if the user is trying to modify a generated file. for _, mod := range modifications { if mod.OnDisk || mod.Action != source.Change { continue } - snapshot, ok := snapshotByURI[mod.URI] - if !ok { - continue + snapshot := snapshots[views[mod.URI]] + if snapshot == nil { + panic("no snapshot assigned for file " + mod.URI) } // 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. @@ -251,12 +223,17 @@ func (s *Server) didModifyFiles(ctx context.Context, modifications []source.File } } - for snapshot, uris := range snapshotSet { + // Group files by best view and diagnose them. + viewURIs := map[source.View][]span.URI{} + for uri, view := range views { + viewURIs[view] = append(viewURIs[view], uri) + } + for view, uris := range viewURIs { diagnosticWG.Add(1) go func(snapshot source.Snapshot, uris []span.URI) { defer diagnosticWG.Done() s.diagnoseSnapshot(snapshot, uris) - }(snapshot, uris) + }(snapshots[view], uris) } go func() { @@ -265,6 +242,7 @@ func (s *Server) didModifyFiles(ctx context.Context, modifications []source.File release() } }() + // After any file modifications, we need to update our watched files, // in case something changed. Compute the new set of directories to watch, // and if it differs from the current set, send updated registrations.