internal/lsp: always return file handles for nonexistent files
findFile has a case that returns early if the file does not exist.
Handle this error in getFile to avoid inconsistently returning errors
when getting file handles for files that don't exist.
Unskip the test, since it is no longer flaky.
Fixes golang/go#44227
Change-Id: I07a4f860cfc9f852728c31706bd924e419bd54e2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/291391
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
diff --git a/gopls/internal/regtest/workspace/workspace_test.go b/gopls/internal/regtest/workspace/workspace_test.go
index b48b4b1..8712784 100644
--- a/gopls/internal/regtest/workspace/workspace_test.go
+++ b/gopls/internal/regtest/workspace/workspace_test.go
@@ -786,9 +786,6 @@
// Confirm that a fix for a tidy module will correct all modules in the
// workspace.
func TestMultiModule_OneBrokenModule(t *testing.T) {
- if testing.Short() {
- t.Skip("flaky test - golang.org/issue/44227")
- }
testenv.NeedsGo1Point(t, 15)
const mod = `
diff --git a/internal/lsp/cache/session.go b/internal/lsp/cache/session.go
index 273573a..928249e 100644
--- a/internal/lsp/cache/session.go
+++ b/internal/lsp/cache/session.go
@@ -454,9 +454,7 @@
// 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, err
- }
+ _ = view.getFile(c.URI)
if _, ok := views[view]; !ok {
views[view] = make(map[span.URI]*fileChange)
}
diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go
index 7ecb1b8..0f44cab 100644
--- a/internal/lsp/cache/snapshot.go
+++ b/internal/lsp/cache/snapshot.go
@@ -956,10 +956,7 @@
}
func (s *snapshot) FindFile(uri span.URI) source.VersionedFileHandle {
- f, err := s.view.getFile(uri)
- if err != nil {
- return nil
- }
+ f := s.view.getFile(uri)
s.mu.Lock()
defer s.mu.Unlock()
@@ -973,10 +970,7 @@
// GetVersionedFile succeeds even if the file does not exist. A non-nil error return
// indicates some type of internal error, for example if ctx is cancelled.
func (s *snapshot) GetVersionedFile(ctx context.Context, uri span.URI) (source.VersionedFileHandle, error) {
- f, err := s.view.getFile(uri)
- if err != nil {
- return nil, err
- }
+ f := s.view.getFile(uri)
s.mu.Lock()
defer s.mu.Unlock()
diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go
index f8baf45..e71a84c 100644
--- a/internal/lsp/cache/view.go
+++ b/internal/lsp/cache/view.go
@@ -384,24 +384,21 @@
return f != nil && err == nil
}
-// getFile 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(uri span.URI) (*fileBase, error) {
+// getFile returns a file for the given URI.
+func (v *View) getFile(uri span.URI) *fileBase {
v.mu.Lock()
defer v.mu.Unlock()
- f, err := v.findFile(uri)
- if err != nil {
- return nil, err
- } else if f != nil {
- return f, nil
+ f, _ := v.findFile(uri)
+ if f != nil {
+ return f
}
f = &fileBase{
view: v,
fname: uri.Filename(),
}
v.mapFile(uri, f)
- return f, nil
+ return f
}
// findFile checks the cache for any file matching the given uri.
