diff --git a/internal/lsp/cache/session.go b/internal/lsp/cache/session.go index 0c815874e4..bec8fce39a 100644 --- a/internal/lsp/cache/session.go +++ b/internal/lsp/cache/session.go @@ -6,6 +6,7 @@ package cache import ( "context" + "fmt" "strconv" "strings" "sync" @@ -328,11 +329,6 @@ func (s *Session) DidModifyFiles(ctx context.Context, changes []source.FileModif if c.Action == source.InvalidateMetadata { forceReloadMetadata = true } - // Do nothing if the file is open in the editor and we receive - // an on-disk action. The editor is the source of truth. - if s.isOpen(c.URI) && c.OnDisk { - continue - } // Look through all of the session's views, invalidating the file for // all of the views to which it is known. for _, view := range s.views { @@ -379,13 +375,20 @@ func (s *Session) updateOverlays(ctx context.Context, changes []source.FileModif defer s.overlayMu.Unlock() for _, c := range changes { - // Don't update overlays for on-disk changes or metadata invalidations. - if c.OnDisk || c.Action == source.InvalidateMetadata { + // Don't update overlays for metadata invalidations. + if c.Action == source.InvalidateMetadata { continue } o, ok := s.overlays[c.URI] + // If the file is not opened in an overlay and the change is on disk, + // there's no need to update an overlay. If there is an overlay, we + // may need to update the overlay's saved value. + if !ok && c.OnDisk { + continue + } + // Determine the file kind on open, otherwise, assume it has been cached. var kind source.FileKind switch c.Action { @@ -408,35 +411,46 @@ func (s *Session) updateOverlays(ctx context.Context, changes []source.FileModif } // If the file is on disk, check if its content is the same as in the - // overlay. Saves don't necessarily come with the file's content. + // overlay. Saves and on-disk file changes don't come with the file's + // content. text := c.Text - if text == nil && c.Action == source.Save { + if text == nil && (c.Action == source.Save || c.OnDisk) { + if !ok { + return nil, fmt.Errorf("no known content for overlay for %s", c.Action) + } text = o.text } + // On-disk changes don't come with versions. + version := c.Version + if c.OnDisk { + version = o.version + } hash := hashContents(text) var sameContentOnDisk bool switch c.Action { - case source.Open: - fh, err := s.cache.getFile(ctx, c.URI) - if err != nil { - return nil, err - } - _, readErr := fh.Read() - sameContentOnDisk = (readErr == nil && fh.Identity().Identifier == hash) + case source.Delete: + // Do nothing. sameContentOnDisk should be false. case source.Save: // Make sure the version and content (if present) is the same. - if o.version != c.Version { + if o.version != version { return nil, errors.Errorf("updateOverlays: saving %s at version %v, currently at %v", c.URI, c.Version, o.version) } if c.Text != nil && o.hash != hash { return nil, errors.Errorf("updateOverlays: overlay %s changed on save", c.URI) } sameContentOnDisk = true + default: + fh, err := s.cache.getFile(ctx, c.URI) + if err != nil { + return nil, err + } + _, readErr := fh.Read() + sameContentOnDisk = (readErr == nil && fh.Identity().Identifier == hash) } o = &overlay{ session: s, uri: c.URI, - version: c.Version, + version: version, text: text, kind: kind, hash: hash, @@ -445,7 +459,8 @@ func (s *Session) updateOverlays(ctx context.Context, changes []source.FileModif s.overlays[c.URI] = o } - // Get the overlays for each change while the session's overlay map is locked. + // Get the overlays for each change while the session's overlay map is + // locked. overlays := make(map[span.URI]*overlay) for _, c := range changes { if o, ok := s.overlays[c.URI]; ok { diff --git a/internal/lsp/regtest/modfile_test.go b/internal/lsp/regtest/modfile_test.go index 3b5c2f829d..503a789c09 100644 --- a/internal/lsp/regtest/modfile_test.go +++ b/internal/lsp/regtest/modfile_test.go @@ -37,28 +37,58 @@ package main import "example.com/blah" func main() { - fmt.Println(blah.Name) -}` - withOptions(WithProxy(proxy)).run(t, untidyModule, func(t *testing.T, env *Env) { - // Open the file and make sure that the initial workspace load does not - // modify the go.mod file. - goModContent := env.ReadWorkspaceFile("go.mod") - env.OpenFile("main.go") - env.Await( - env.DiagnosticAtRegexp("main.go", "\"example.com/blah\""), - ) - if got := env.ReadWorkspaceFile("go.mod"); got != goModContent { - t.Fatalf("go.mod changed on disk:\n%s", tests.Diff(goModContent, got)) - } - // Save the buffer, which will format and organize imports. - // Confirm that the go.mod file still does not change. - env.SaveBuffer("main.go") - env.Await( - env.DiagnosticAtRegexp("main.go", "\"example.com/blah\""), - ) - if got := env.ReadWorkspaceFile("go.mod"); got != goModContent { - t.Fatalf("go.mod changed on disk:\n%s", tests.Diff(goModContent, got)) - } + println(blah.Name) +} +` + t.Run("basic", func(t *testing.T) { + withOptions(WithProxy(proxy)).run(t, untidyModule, func(t *testing.T, env *Env) { + // Open the file and make sure that the initial workspace load does not + // modify the go.mod file. + goModContent := env.ReadWorkspaceFile("go.mod") + env.OpenFile("main.go") + env.Await( + env.DiagnosticAtRegexp("main.go", "\"example.com/blah\""), + ) + if got := env.ReadWorkspaceFile("go.mod"); got != goModContent { + t.Fatalf("go.mod changed on disk:\n%s", tests.Diff(goModContent, got)) + } + // Save the buffer, which will format and organize imports. + // Confirm that the go.mod file still does not change. + env.SaveBuffer("main.go") + env.Await( + env.DiagnosticAtRegexp("main.go", "\"example.com/blah\""), + ) + if got := env.ReadWorkspaceFile("go.mod"); got != goModContent { + t.Fatalf("go.mod changed on disk:\n%s", tests.Diff(goModContent, got)) + } + }) + }) + + // Reproduce golang/go#40269 by deleting and recreating main.go. + t.Run("delete main.go", func(t *testing.T) { + t.Skipf("This test will be flaky until golang/go#40269 is resolved.") + + withOptions(WithProxy(proxy)).run(t, untidyModule, func(t *testing.T, env *Env) { + goModContent := env.ReadWorkspaceFile("go.mod") + mainContent := env.ReadWorkspaceFile("main.go") + env.OpenFile("main.go") + env.SaveBuffer("main.go") + + env.RemoveWorkspaceFile("main.go") + env.Await( + CompletedWork(lsp.DiagnosticWorkTitle(lsp.FromDidOpen), 1), + CompletedWork(lsp.DiagnosticWorkTitle(lsp.FromDidSave), 1), + CompletedWork(lsp.DiagnosticWorkTitle(lsp.FromDidChangeWatchedFiles), 2), + ) + + env.WriteWorkspaceFile("main.go", mainContent) + env.Await( + env.DiagnosticAtRegexp("main.go", "\"example.com/blah\""), + ) + if got := env.ReadWorkspaceFile("go.mod"); got != goModContent { + t.Fatalf("go.mod changed on disk:\n%s", tests.Diff(goModContent, got)) + } + }) }) } diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go index 195cb9012f..036da6cadc 100644 --- a/internal/lsp/source/view.go +++ b/internal/lsp/source/view.go @@ -274,6 +274,27 @@ const ( InvalidateMetadata ) +func (a FileAction) String() string { + switch a { + case Open: + return "Open" + case Change: + return "Change" + case Close: + return "Close" + case Save: + return "Save" + case Create: + return "Create" + case Delete: + return "Delete" + case InvalidateMetadata: + return "InvalidateMetadata" + default: + return "Unknown" + } +} + // Cache abstracts the core logic of dealing with the environment from the // higher level logic that processes the information to produce results. // The cache provides access to files and their contents, so the source