internal/lsp: clear diagnostics for deleted files
Diagnostics should be cleared for files which are (1) deleted on disk
and not open in the editor, and (2) closed and only open in the editor.
Enable the corresponding regression test, and fix a few issues raised by
staticcheck.
Fixes golang/go#37049
Change-Id: Iff736a7f6c3eaacda4237c2e4cf7926e9949dece
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220079
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
diff --git a/internal/lsp/cmd/serve.go b/internal/lsp/cmd/serve.go
index 54a443e..10349aa 100644
--- a/internal/lsp/cmd/serve.go
+++ b/internal/lsp/cmd/serve.go
@@ -53,7 +53,7 @@
return tool.CommandLineErrorf("server does not take arguments, got %v", args)
}
- err, closeLog := s.app.debug.SetLogFile(s.Logfile)
+ closeLog, err := s.app.debug.SetLogFile(s.Logfile)
if err != nil {
return err
}
diff --git a/internal/lsp/debug/serve.go b/internal/lsp/debug/serve.go
index c65c8f3..cea54c6 100644
--- a/internal/lsp/debug/serve.go
+++ b/internal/lsp/debug/serve.go
@@ -258,7 +258,7 @@
export.AddExporters(i.ocagent, i.prometheus, i.rpcs, i.traces)
}
-func (i *Instance) SetLogFile(logfile string) (error, func()) {
+func (i *Instance) SetLogFile(logfile string) (func(), error) {
// TODO: probably a better solution for deferring closure to the caller would
// be for the debug instance to itself be closed, but this fixes the
// immediate bug of logs not being captured.
@@ -269,7 +269,7 @@
}
f, err := os.Create(logfile)
if err != nil {
- return fmt.Errorf("Unable to create log file: %v", err), nil
+ return nil, fmt.Errorf("unable to create log file: %v", err)
}
closeLog = func() {
defer f.Close()
@@ -278,7 +278,7 @@
i.LogWriter = f
}
i.Logfile = logfile
- return nil, closeLog
+ return closeLog, nil
}
// Serve starts and runs a debug server in the background.
diff --git a/internal/lsp/regtest/diagnostics_test.go b/internal/lsp/regtest/diagnostics_test.go
index d7044f3..cbf4a0e 100644
--- a/internal/lsp/regtest/diagnostics_test.go
+++ b/internal/lsp/regtest/diagnostics_test.go
@@ -80,15 +80,25 @@
}
func TestDiagnosticClearingOnDelete(t *testing.T) {
- t.Skip("skipping due to golang.org/issues/37049")
-
t.Parallel()
runner.Run(t, badPackage, func(ctx context.Context, t *testing.T, env *Env) {
env.OpenFile("a.go")
env.Await(DiagnosticAt("a.go", 2, 6), DiagnosticAt("b.go", 2, 6))
env.RemoveFileFromWorkspace("b.go")
- // TODO(golang.org/issues/37049): here we only get diagnostics for a.go.
env.Await(EmptyDiagnostics("a.go"), EmptyDiagnostics("b.go"))
})
}
+
+func TestDiagnosticClearingOnClose(t *testing.T) {
+ t.Parallel()
+ runner.Run(t, badPackage, func(ctx context.Context, t *testing.T, env *Env) {
+ env.E.CreateBuffer(env.ctx, "c.go", `package consts
+
+const a = 3`)
+ env.Await(DiagnosticAt("a.go", 2, 6), DiagnosticAt("b.go", 2, 6), DiagnosticAt("c.go", 2, 6))
+ env.E.CloseBuffer(env.ctx, "c.go")
+
+ env.Await(DiagnosticAt("a.go", 2, 6), DiagnosticAt("b.go", 2, 6), EmptyDiagnostics("c.go"))
+ })
+}
diff --git a/internal/lsp/text_synchronization.go b/internal/lsp/text_synchronization.go
index cb05419..1ebe8be 100644
--- a/internal/lsp/text_synchronization.go
+++ b/internal/lsp/text_synchronization.go
@@ -73,19 +73,47 @@
func (s *Server) didChangeWatchedFiles(ctx context.Context, params *protocol.DidChangeWatchedFilesParams) error {
var modifications []source.FileModification
+ deletions := make(map[span.URI]struct{})
for _, change := range params.Changes {
uri := change.URI.SpanURI()
if !uri.IsFile() {
continue
}
+ action := changeTypeToFileAction(change.Type)
modifications = append(modifications, source.FileModification{
URI: uri,
- Action: changeTypeToFileAction(change.Type),
+ Action: action,
OnDisk: true,
})
+ // 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.
+ switch action {
+ case source.Delete:
+ deletions[uri] = struct{}{}
+ case source.Close:
+ default:
+ delete(deletions, uri)
+ }
}
- _, err := s.didModifyFiles(ctx, modifications)
- return err
+ snapshots, err := s.didModifyFiles(ctx, modifications)
+ if err != nil {
+ return err
+ }
+ // Clear the diagnostics for any deleted files.
+ for uri := range deletions {
+ if snapshot := snapshots[uri]; snapshot == nil || snapshot.IsOpen(uri) {
+ continue
+ }
+ if err := s.client.PublishDiagnostics(ctx, &protocol.PublishDiagnosticsParams{
+ URI: protocol.URIFromSpanURI(uri),
+ Diagnostics: []protocol.Diagnostic{},
+ Version: 0,
+ }); err != nil {
+ return err
+ }
+ }
+ return nil
}
func (s *Server) didSave(ctx context.Context, params *protocol.DidSaveTextDocumentParams) error {
@@ -93,7 +121,6 @@
if !uri.IsFile() {
return nil
}
-
c := source.FileModification{
URI: uri,
Action: source.Save,
@@ -111,8 +138,7 @@
if !uri.IsFile() {
return nil
}
-
- _, err := s.didModifyFiles(ctx, []source.FileModification{
+ snapshots, err := s.didModifyFiles(ctx, []source.FileModification{
{
URI: uri,
Action: source.Close,
@@ -120,7 +146,26 @@
Text: nil,
},
})
- return err
+ if err != nil {
+ return err
+ }
+ snapshot := snapshots[uri]
+ if snapshot == nil {
+ return errors.Errorf("no snapshot for %s", uri)
+ }
+ fh, err := snapshot.GetFile(uri)
+ if err != nil {
+ return err
+ }
+ // If a file has been closed and is not on disk, clear its diagnostics.
+ if _, _, err := fh.Read(ctx); err != nil {
+ return s.client.PublishDiagnostics(ctx, &protocol.PublishDiagnosticsParams{
+ URI: protocol.URIFromSpanURI(uri),
+ Diagnostics: []protocol.Diagnostic{},
+ Version: 0,
+ })
+ }
+ return nil
}
func (s *Server) didModifyFiles(ctx context.Context, modifications []source.FileModification) (map[span.URI]source.Snapshot, error) {
