Skip to content

Commit

Permalink
internal/lsp/lsprpc: add test for empty diagnostics in deleted files
Browse files Browse the repository at this point in the history 
Add a test for the bug reported in golang/go#37049: we are missing empty
diagnostics for deleted files. Doing this involved added a missing
RemoveFile method on the fake.Watcher type.

Skip the test for now, as it is failing.

Updates golang/go#37049
Updates golang/go#36879

Change-Id: Ib3b6907455cc44a2e6af00c2254aa444e9480749
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218278
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
  • Loading branch information
@findleyr
findleyr committed Feb 10, 2020
1 parent f958729 commit babff93
Show file tree
Hide file tree
Showing 2 changed files with 182 additions and 48 deletions.
41 changes: 32 additions & 9 deletions internal/lsp/fake/workspace.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,33 @@ func (w *Workspace) ReadFile(path string) (string, error) {
return string(b), nil
}

// RemoveFile removes a workspace-relative file path.
func (w *Workspace) RemoveFile(ctx context.Context, path string) error {
fp := w.filePath(path)
if err := os.Remove(fp); err != nil {
return fmt.Errorf("removing %q: %v", path, err)
}
evts := []FileEvent{{
Path: path,
ProtocolEvent: protocol.FileEvent{
URI: w.URI(path),
Type: protocol.Deleted,
},
}}
w.sendEvents(ctx, evts)
return nil
}

func (w *Workspace) sendEvents(ctx context.Context, evts []FileEvent) {
w.watcherMu.Lock()
watchers := make([]func(context.Context, []FileEvent), len(w.watchers))
copy(watchers, w.watchers)
w.watcherMu.Unlock()
for _, w := range watchers {
go w(ctx, evts)
}
}

// WriteFile writes text file content to a workspace-relative path.
func (w *Workspace) WriteFile(ctx context.Context, path, content string) error {
fp := w.filePath(path)
Expand All @@ -129,22 +156,18 @@ func (w *Workspace) WriteFile(ctx context.Context, path, content string) error {
} else {
changeType = protocol.Changed
}
werr := w.writeFileData(path, []byte(content))
w.watcherMu.Lock()
watchers := make([]func(context.Context, []FileEvent), len(w.watchers))
copy(watchers, w.watchers)
w.watcherMu.Unlock()
if err := w.writeFileData(path, []byte(content)); err != nil {
return err
}
evts := []FileEvent{{
Path: path,
ProtocolEvent: protocol.FileEvent{
URI: w.URI(path),
Type: changeType,
},
}}
for _, w := range watchers {
go w(ctx, evts)
}
return werr
w.sendEvents(ctx, evts)
return nil
}

func (w *Workspace) writeFileData(path string, data []byte) error {
Expand Down
189 changes: 150 additions & 39 deletions internal/lsp/lsprpc/diagnostics_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,14 @@ type testEnvironment struct {
ws *fake.Workspace
ts *servertest.Server

diagnostics <-chan *protocol.PublishDiagnosticsParams
dw diagnosticsWatcher
}

func setupEnv(t *testing.T) (context.Context, testEnvironment, func()) {
func setupEnv(t *testing.T, txt string) (context.Context, testEnvironment, func()) {
t.Helper()
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)

ws, err := fake.NewWorkspace("get-diagnostics", []byte(exampleProgram))
ws, err := fake.NewWorkspace("get-diagnostics", []byte(txt))
if err != nil {
t.Fatal(err)
}
Expand All @@ -55,24 +55,63 @@ func setupEnv(t *testing.T) (context.Context, testEnvironment, func()) {
if err != nil {
t.Fatal(err)
}
diags := make(chan *protocol.PublishDiagnosticsParams, 10)
editor.Client().OnDiagnostics(func(_ context.Context, params *protocol.PublishDiagnosticsParams) error {
diags <- params
return nil
})
dw := newDiagWatcher(ws)
editor.Client().OnDiagnostics(dw.onDiagnostics)
cleanup := func() {
cancel()
ts.Close()
ws.Close()
}
return ctx, testEnvironment{
editor: editor,
ws: ws,
ts: ts,
diagnostics: diags,
editor: editor,
ws: ws,
ts: ts,
dw: dw,
}, cleanup
}

type diagnosticsWatcher struct {
diagnostics chan *protocol.PublishDiagnosticsParams
ws *fake.Workspace
}

func newDiagWatcher(ws *fake.Workspace) diagnosticsWatcher {
return diagnosticsWatcher{
// Allow an arbitrarily large buffer, as we should never want onDiagnostics
// to block.
diagnostics: make(chan *protocol.PublishDiagnosticsParams, 1000),
ws: ws,
}
}

func (w diagnosticsWatcher) onDiagnostics(_ context.Context, p *protocol.PublishDiagnosticsParams) error {
w.diagnostics <- p
return nil
}

func (w diagnosticsWatcher) await(ctx context.Context, expected ...string) (map[string]*protocol.PublishDiagnosticsParams, error) {
expectedSet := make(map[string]bool)
for _, e := range expected {
expectedSet[e] = true
}
got := make(map[string]*protocol.PublishDiagnosticsParams)
for len(got) < len(expectedSet) {
select {
case <-ctx.Done():
return nil, ctx.Err()
case d := <-w.diagnostics:
pth, err := w.ws.URIToPath(d.URI)
if err != nil {
return nil, err
}
if expectedSet[pth] {
got[pth] = d
}
}
}
return got, nil
}

func checkDiagnosticLocation(params *protocol.PublishDiagnosticsParams, filename string, line, col int) error {
if got, want := params.URI, filename; got != want {
return fmt.Errorf("got diagnostics for URI %q, want %q", got, want)
Expand All @@ -89,7 +128,7 @@ func checkDiagnosticLocation(params *protocol.PublishDiagnosticsParams, filename

func TestDiagnosticErrorInEditedFile(t *testing.T) {
t.Parallel()
ctx, env, cleanup := setupEnv(t)
ctx, env, cleanup := setupEnv(t, exampleProgram)
defer cleanup()

// Deleting the 'n' at the end of Println should generate a single error
Expand All @@ -107,15 +146,18 @@ func TestDiagnosticErrorInEditedFile(t *testing.T) {
if err := env.editor.EditBuffer(ctx, "main.go", edits); err != nil {
t.Fatal(err)
}
params := awaitDiagnostics(ctx, t, env.diagnostics)
if err := checkDiagnosticLocation(params, env.ws.URI("main.go"), 5, 5); err != nil {
diags, err := env.dw.await(ctx, "main.go")
if err != nil {
t.Fatal(err)
}
if err := checkDiagnosticLocation(diags["main.go"], env.ws.URI("main.go"), 5, 5); err != nil {
t.Fatal(err)
}
}

func TestSimultaneousEdits(t *testing.T) {
t.Parallel()
ctx, env, cleanup := setupEnv(t)
ctx, env, cleanup := setupEnv(t, exampleProgram)
defer cleanup()

// Set up a second editor session connected to the same server, using the
Expand All @@ -125,12 +167,8 @@ func TestSimultaneousEdits(t *testing.T) {
if err != nil {
t.Fatal(err)
}
diags2 := make(chan *protocol.PublishDiagnosticsParams, 10)
editor2.Client().OnDiagnostics(func(_ context.Context, params *protocol.PublishDiagnosticsParams) error {
diags2 <- params
return nil
})

dw2 := newDiagWatcher(env.ws)
editor2.Client().OnDiagnostics(dw2.onDiagnostics)
// In editor #1, break fmt.Println as before.
edits1 := []fake.Edit{{
Start: fake.Pos{Line: 5, Column: 11},
Expand All @@ -156,23 +194,19 @@ func TestSimultaneousEdits(t *testing.T) {
if err := editor2.EditBuffer(ctx, "main.go", edits2); err != nil {
t.Fatal(err)
}
params1 := awaitDiagnostics(ctx, t, env.diagnostics)
params2 := awaitDiagnostics(ctx, t, diags2)
if err := checkDiagnosticLocation(params1, env.ws.URI("main.go"), 5, 5); err != nil {
diags1, err := env.dw.await(ctx, "main.go")
if err != nil {
t.Fatal(err)
}
if err := checkDiagnosticLocation(params2, env.ws.URI("main.go"), 7, 0); err != nil {
diags2, err := dw2.await(ctx, "main.go")
if err != nil {
t.Fatal(err)
}
}

func awaitDiagnostics(ctx context.Context, t *testing.T, diags <-chan *protocol.PublishDiagnosticsParams) *protocol.PublishDiagnosticsParams {
t.Helper()
select {
case <-ctx.Done():
panic(ctx.Err())
case d := <-diags:
return d
if err := checkDiagnosticLocation(diags1["main.go"], env.ws.URI("main.go"), 5, 5); err != nil {
t.Fatal(err)
}
if err := checkDiagnosticLocation(diags2["main.go"], env.ws.URI("main.go"), 7, 0); err != nil {
t.Fatal(err)
}
}

Expand All @@ -183,14 +217,91 @@ const Foo = "abc

func TestDiagnosticErrorInNewFile(t *testing.T) {
t.Parallel()
ctx, env, cleanup := setupEnv(t)
ctx, env, cleanup := setupEnv(t, exampleProgram)
defer cleanup()

if err := env.editor.CreateBuffer(ctx, "broken.go", brokenFile); err != nil {
t.Fatal(err)
}
params := awaitDiagnostics(ctx, t, env.diagnostics)
if got, want := params.URI, env.ws.URI("broken.go"); got != want {
t.Fatalf("got diagnostics for URI %q, want %q", got, want)
_, err := env.dw.await(ctx, "broken.go")
if err != nil {
t.Fatal(err)
}
}

// badPackage contains a duplicate definition of the 'a' const.
const badPackage = `
-- go.mod --
module mod
go 1.12
-- a.go --
package consts
const a = 1
-- b.go --
package consts
const a = 2
`

func TestDiagnosticClearingOnEdit(t *testing.T) {
t.Parallel()
ctx, env, cleanup := setupEnv(t, badPackage)
defer cleanup()

if err := env.editor.OpenFile(ctx, "b.go"); err != nil {
t.Fatal(err)
}
_, err := env.dw.await(ctx, "a.go", "b.go")
if err != nil {
t.Fatal(err)
}

// In editor #2 remove the closing brace.
edits := []fake.Edit{{
Start: fake.Pos{Line: 2, Column: 6},
End: fake.Pos{Line: 2, Column: 7},
Text: "b",
}}
if err := env.editor.EditBuffer(ctx, "b.go", edits); err != nil {
t.Fatal(err)
}
diags, err := env.dw.await(ctx, "a.go", "b.go")
if err != nil {
t.Fatal(err)
}
for pth, d := range diags {
if len(d.Diagnostics) != 0 {
t.Errorf("non-empty diagnostics for %q", pth)
}
}
}

func TestDiagnosticClearingOnDelete(t *testing.T) {
t.Skip("skipping due to golang.org/issues/37049")

t.Parallel()
ctx, env, cleanup := setupEnv(t, badPackage)
defer cleanup()

if err := env.editor.OpenFile(ctx, "a.go"); err != nil {
t.Fatal(err)
}
_, err := env.dw.await(ctx, "a.go", "b.go")
if err != nil {
t.Fatal(err)
}
env.ws.RemoveFile(ctx, "b.go")

// TODO(golang.org/issues/37049): here we only get diagnostics for a.go.
diags, err := env.dw.await(ctx, "a.go", "b.go")
if err != nil {
t.Fatal(err)
}
for pth, d := range diags {
if len(d.Diagnostics) != 0 {
t.Errorf("non-empty diagnostics for %q", pth)
}
}
}

0 comments on commit babff93

Please sign in to comment.