From cb8d9cd24560c418be78de44753d8a45832bbaab Mon Sep 17 00:00:00 2001 From: Rob Findley Date: Wed, 6 May 2020 22:54:50 -0400 Subject: [PATCH] internal/lsp: support configurable codeLens Some code lenses may be undesirable for certain users or editors -- for example a code lens that runs tests, when VSCode already supports this functionality outside of the LSP. To handle such situations, support configuring code lenses via a new 'codelens' gopls option. Add support for code lens in regtests, and use this to test the new configuration. To achieve this, thread through a new 'EditorConfig' type that configures the fake editor's LSP session. It made sense to move the test Env overlay onto this config object as well. While looking at them, document some types in source.Options. Change-Id: I961077422a273829c5cbd83c3b87fae29f77eeda Reviewed-on: https://go-review.googlesource.com/c/tools/+/232680 Run-TryBot: Robert Findley TryBot-Result: Gobot Gobot Reviewed-by: Rebecca Stambler --- gopls/doc/settings.md | 14 +++- internal/lsp/command.go | 6 +- internal/lsp/fake/editor.go | 81 ++++++++++++++++++----- internal/lsp/fake/editor_test.go | 2 +- internal/lsp/fake/sandbox.go | 8 +-- internal/lsp/lsprpc/lsprpc_test.go | 4 +- internal/lsp/mod/code_lens.go | 8 ++- internal/lsp/regtest/codelens_test.go | 57 ++++++++++++++++ internal/lsp/regtest/diagnostics_test.go | 4 +- internal/lsp/regtest/env.go | 4 +- internal/lsp/regtest/runner.go | 23 ++++--- internal/lsp/regtest/shared_test.go | 2 +- internal/lsp/regtest/unix_test.go | 6 +- internal/lsp/regtest/wrappers.go | 11 ++++ internal/lsp/source/code_lens.go | 8 ++- internal/lsp/source/options.go | 83 +++++++++++++++++++----- internal/lsp/source/view.go | 4 ++ 17 files changed, 258 insertions(+), 67 deletions(-) create mode 100644 internal/lsp/regtest/codelens_test.go diff --git a/gopls/doc/settings.md b/gopls/doc/settings.md index 8c3d62a894..6355c5243b 100644 --- a/gopls/doc/settings.md +++ b/gopls/doc/settings.md @@ -83,9 +83,15 @@ Example Usage: ... ``` -### **staticcheck** *boolean* +### **codelens** *map[string]bool* -If true, it enables the use of the staticcheck.io analyzers. +Overrides the enabled/disabled state of various code lenses. Currently, we +support two code lenses: + +* `generate`: run `go generate` as specified by a `//go:generate` directive. +* `upgrade.dependency`: upgrade a dependency listed in a `go.mod` file. + +By default, both of these code lenses are enabled. ### **completionDocumentation** *boolean* @@ -129,3 +135,7 @@ At the location of the `<>` in this program, deep completion would suggest the r If true, this enables server side fuzzy matching of completion candidates. Default: `true`. + +### **staticcheck** *boolean* + +If true, it enables the use of the staticcheck.io analyzers. diff --git a/internal/lsp/command.go b/internal/lsp/command.go index 280f47e782..cd80d6df17 100644 --- a/internal/lsp/command.go +++ b/internal/lsp/command.go @@ -18,13 +18,13 @@ import ( func (s *Server) executeCommand(ctx context.Context, params *protocol.ExecuteCommandParams) (interface{}, error) { switch params.Command { - case "generate": + case source.CommandGenerate: dir, recursive, err := getGenerateRequest(params.Arguments) if err != nil { return nil, err } go s.runGenerate(xcontext.Detach(ctx), dir, recursive) - case "tidy": + case source.CommandTidy: if len(params.Arguments) == 0 || len(params.Arguments) > 1 { return nil, errors.Errorf("expected one file URI for call to `go mod tidy`, got %v", params.Arguments) } @@ -45,7 +45,7 @@ func (s *Server) executeCommand(ctx context.Context, params *protocol.ExecuteCom if _, err := gocmdRunner.Run(ctx, inv); err != nil { return nil, err } - case "upgrade.dependency": + case source.CommandUpgradeDependency: if len(params.Arguments) < 2 { return nil, errors.Errorf("expected one file URI and one dependency for call to `go get`, got %v", params.Arguments) } diff --git a/internal/lsp/fake/editor.go b/internal/lsp/fake/editor.go index 9116d278cf..dc9babb88f 100644 --- a/internal/lsp/fake/editor.go +++ b/internal/lsp/fake/editor.go @@ -20,8 +20,10 @@ import ( // Editor is a fake editor client. It keeps track of client state and can be // used for writing LSP tests. type Editor struct { - // server, client, and sandbox are concurrency safe and written only at - // construction, so do not require synchronization. + Config EditorConfig + + // server, client, and sandbox are concurrency safe and written only + // at construction time, so do not require synchronization. server protocol.Server client *Client sandbox *Sandbox @@ -49,11 +51,26 @@ func (b buffer) text() string { return strings.Join(b.content, "\n") } +// EditorConfig configures the editor's LSP session. This is similar to +// source.UserOptions, but we use a separate type here so that we expose only +// that configuration which we support. +// +// The zero value for EditorConfig should correspond to its defaults. +type EditorConfig struct { + Env []string + + // CodeLens is a map defining whether codelens are enabled, keyed by the + // codeLens command. CodeLens which are not present in this map are left in + // their default state. + CodeLens map[string]bool +} + // NewEditor Creates a new Editor. -func NewEditor(ws *Sandbox) *Editor { +func NewEditor(ws *Sandbox, config EditorConfig) *Editor { return &Editor{ buffers: make(map[string]buffer), sandbox: ws, + Config: config, } } @@ -105,15 +122,24 @@ func (e *Editor) Client() *Client { } func (e *Editor) configuration() map[string]interface{} { + config := map[string]interface{}{ + "verboseWorkDoneProgress": true, + } + + envvars := e.sandbox.GoEnv() + envvars = append(envvars, e.Config.Env...) env := map[string]interface{}{} - for _, value := range e.sandbox.GoEnv() { + for _, value := range envvars { kv := strings.SplitN(value, "=", 2) env[kv[0]] = kv[1] } - return map[string]interface{}{ - "env": env, - "verboseWorkDoneProgress": true, + config["env"] = env + + if e.Config.CodeLens != nil { + config["codelens"] = e.Config.CodeLens } + + return config } func (e *Editor) initialize(ctx context.Context) error { @@ -235,9 +261,7 @@ func (e *Editor) CloseBuffer(ctx context.Context, path string) error { if e.server != nil { if err := e.server.DidClose(ctx, &protocol.DidCloseTextDocumentParams{ - TextDocument: protocol.TextDocumentIdentifier{ - URI: e.sandbox.Workdir.URI(path), - }, + TextDocument: e.textDocumentIdentifier(path), }); err != nil { return fmt.Errorf("DidClose: %w", err) } @@ -245,6 +269,12 @@ func (e *Editor) CloseBuffer(ctx context.Context, path string) error { return nil } +func (e *Editor) textDocumentIdentifier(path string) protocol.TextDocumentIdentifier { + return protocol.TextDocumentIdentifier{ + URI: e.sandbox.Workdir.URI(path), + } +} + // SaveBuffer writes the content of the buffer specified by the given path to // the filesystem. func (e *Editor) SaveBuffer(ctx context.Context, path string) error { @@ -269,9 +299,7 @@ func (e *Editor) SaveBuffer(ctx context.Context, path string) error { } e.mu.Unlock() - docID := protocol.TextDocumentIdentifier{ - URI: e.sandbox.Workdir.URI(buf.path), - } + docID := e.textDocumentIdentifier(buf.path) if e.server != nil { if err := e.server.WillSave(ctx, &protocol.WillSaveTextDocumentParams{ TextDocument: docID, @@ -456,10 +484,8 @@ func (e *Editor) editBufferLocked(ctx context.Context, path string, edits []Edit } params := &protocol.DidChangeTextDocumentParams{ TextDocument: protocol.VersionedTextDocumentIdentifier{ - Version: float64(buf.version), - TextDocumentIdentifier: protocol.TextDocumentIdentifier{ - URI: e.sandbox.Workdir.URI(buf.path), - }, + Version: float64(buf.version), + TextDocumentIdentifier: e.textDocumentIdentifier(buf.path), }, ContentChanges: evts, } @@ -614,3 +640,24 @@ func (e *Editor) RunGenerate(ctx context.Context, dir string) error { // the caller. return nil } + +// CodeLens execute a codelens request on the server. +func (e *Editor) CodeLens(ctx context.Context, path string) ([]protocol.CodeLens, error) { + if e.server == nil { + return nil, nil + } + e.mu.Lock() + _, ok := e.buffers[path] + e.mu.Unlock() + if !ok { + return nil, fmt.Errorf("buffer %q is not open", path) + } + params := &protocol.CodeLensParams{ + TextDocument: e.textDocumentIdentifier(path), + } + lens, err := e.server.CodeLens(ctx, params) + if err != nil { + return nil, err + } + return lens, nil +} diff --git a/internal/lsp/fake/editor_test.go b/internal/lsp/fake/editor_test.go index e45984b179..b34be17365 100644 --- a/internal/lsp/fake/editor_test.go +++ b/internal/lsp/fake/editor_test.go @@ -54,7 +54,7 @@ func TestClientEditing(t *testing.T) { } defer ws.Close() ctx := context.Background() - editor := NewEditor(ws) + editor := NewEditor(ws, EditorConfig{}) if err := editor.OpenFile(ctx, "main.go"); err != nil { t.Fatal(err) } diff --git a/internal/lsp/fake/sandbox.go b/internal/lsp/fake/sandbox.go index d097600891..cee2993cef 100644 --- a/internal/lsp/fake/sandbox.go +++ b/internal/lsp/fake/sandbox.go @@ -22,7 +22,6 @@ type Sandbox struct { name string gopath string basedir string - env []string Proxy *Proxy Workdir *Workdir } @@ -31,10 +30,9 @@ type Sandbox struct { // working directory populated by the txtar-encoded content in srctxt, and a // file-based module proxy populated with the txtar-encoded content in // proxytxt. -func NewSandbox(name, srctxt, proxytxt string, inGopath bool, env ...string) (_ *Sandbox, err error) { +func NewSandbox(name, srctxt, proxytxt string, inGopath bool) (_ *Sandbox, err error) { sb := &Sandbox{ name: name, - env: env, } defer func() { // Clean up if we fail at any point in this constructor. @@ -103,12 +101,12 @@ func (sb *Sandbox) GOPATH() string { // GoEnv returns the default environment variables that can be used for // invoking Go commands in the sandbox. func (sb *Sandbox) GoEnv() []string { - return append([]string{ + return []string{ "GOPATH=" + sb.GOPATH(), "GOPROXY=" + sb.Proxy.GOPROXY(), "GO111MODULE=", "GOSUMDB=off", - }, sb.env...) + } } // RunGoCommand executes a go command in the sandbox. diff --git a/internal/lsp/lsprpc/lsprpc_test.go b/internal/lsp/lsprpc/lsprpc_test.go index 754013342e..5014ad82e9 100644 --- a/internal/lsp/lsprpc/lsprpc_test.go +++ b/internal/lsp/lsprpc/lsprpc_test.go @@ -218,13 +218,13 @@ func TestDebugInfoLifecycle(t *testing.T) { tsForwarder := servertest.NewPipeServer(clientCtx, forwarder) conn1 := tsForwarder.Connect(clientCtx) - ed1, err := fake.NewEditor(sb).Connect(clientCtx, conn1) + ed1, err := fake.NewEditor(sb, fake.EditorConfig{}).Connect(clientCtx, conn1) if err != nil { t.Fatal(err) } defer ed1.Shutdown(clientCtx) conn2 := tsBackend.Connect(baseCtx) - ed2, err := fake.NewEditor(sb).Connect(baseCtx, conn2) + ed2, err := fake.NewEditor(sb, fake.EditorConfig{}).Connect(baseCtx, conn2) if err != nil { t.Fatal(err) } diff --git a/internal/lsp/mod/code_lens.go b/internal/lsp/mod/code_lens.go index 3c4ada1393..39ca1932a7 100644 --- a/internal/lsp/mod/code_lens.go +++ b/internal/lsp/mod/code_lens.go @@ -13,7 +13,11 @@ import ( "golang.org/x/tools/internal/span" ) +// CodeLens computes code lens for a go.mod file. func CodeLens(ctx context.Context, snapshot source.Snapshot, uri span.URI) ([]protocol.CodeLens, error) { + if !snapshot.View().Options().EnabledCodeLens[source.CommandUpgradeDependency] { + return nil, nil + } realURI, _ := snapshot.View().ModFiles() if realURI == "" { return nil, nil @@ -50,7 +54,7 @@ func CodeLens(ctx context.Context, snapshot source.Snapshot, uri span.URI) ([]pr Range: rng, Command: protocol.Command{ Title: fmt.Sprintf("Upgrade dependency to %s", latest), - Command: "upgrade.dependency", + Command: source.CommandUpgradeDependency, Arguments: []interface{}{uri, dep}, }, }) @@ -67,7 +71,7 @@ func CodeLens(ctx context.Context, snapshot source.Snapshot, uri span.URI) ([]pr Range: rng, Command: protocol.Command{ Title: "Upgrade all dependencies", - Command: "upgrade.dependency", + Command: source.CommandUpgradeDependency, Arguments: []interface{}{uri, strings.Join(append([]string{"-u"}, allUpgrades...), " ")}, }, }) diff --git a/internal/lsp/regtest/codelens_test.go b/internal/lsp/regtest/codelens_test.go new file mode 100644 index 0000000000..9bc913fbc8 --- /dev/null +++ b/internal/lsp/regtest/codelens_test.go @@ -0,0 +1,57 @@ +// Copyright 2020 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package regtest + +import ( + "testing" + + "golang.org/x/tools/internal/lsp/fake" + "golang.org/x/tools/internal/lsp/source" +) + +func TestDisablingCodeLens(t *testing.T) { + const workspace = ` +-- go.mod -- +module codelens.test +-- lib.go -- +package lib + +type Number int + +const ( + Zero Number = iota + One + Two +) + +//go:generate stringer -type=Number +` + tests := []struct { + label string + enabled map[string]bool + wantCodeLens bool + }{ + { + label: "default", + wantCodeLens: true, + }, + { + label: "generate disabled", + enabled: map[string]bool{source.CommandGenerate: false}, + wantCodeLens: false, + }, + } + for _, test := range tests { + t.Run(test.label, func(t *testing.T) { + runner.Run(t, workspace, func(t *testing.T, env *Env) { + env.OpenFile("lib.go") + lens := env.CodeLens("lib.go") + if gotCodeLens := len(lens) > 0; gotCodeLens != test.wantCodeLens { + t.Errorf("got codeLens: %t, want %t", gotCodeLens, test.wantCodeLens) + } + }, WithEditorConfig(fake.EditorConfig{CodeLens: test.enabled})) + }) + } +} diff --git a/internal/lsp/regtest/diagnostics_test.go b/internal/lsp/regtest/diagnostics_test.go index f6d3dc4bd1..6a5f016e51 100644 --- a/internal/lsp/regtest/diagnostics_test.go +++ b/internal/lsp/regtest/diagnostics_test.go @@ -382,7 +382,7 @@ func _() { if err := env.Editor.OrganizeImports(env.Ctx, "main.go"); err == nil { t.Fatalf("organize imports should fail with an empty GOPATH") } - }, WithEnv("GOPATH=")) + }, WithEditorConfig(fake.EditorConfig{Env: []string{"GOPATH="}})) } // Tests golang/go#38669. @@ -404,7 +404,7 @@ var X = 0 env.OpenFile("main.go") env.OrganizeImports("main.go") env.Await(EmptyDiagnostics("main.go")) - }, WithEnv("GOFLAGS=-tags=foo")) + }, WithEditorConfig(fake.EditorConfig{Env: []string{"GOFLAGS=-tags=foo"}})) } // Tests golang/go#38467. diff --git a/internal/lsp/regtest/env.go b/internal/lsp/regtest/env.go index a9749a2b37..c8b1a23fef 100644 --- a/internal/lsp/regtest/env.go +++ b/internal/lsp/regtest/env.go @@ -102,10 +102,10 @@ type condition struct { // NewEnv creates a new test environment using the given scratch environment // and gopls server. -func NewEnv(ctx context.Context, t *testing.T, scratch *fake.Sandbox, ts servertest.Connector) *Env { +func NewEnv(ctx context.Context, t *testing.T, scratch *fake.Sandbox, ts servertest.Connector, editorConfig fake.EditorConfig) *Env { t.Helper() conn := ts.Connect(ctx) - editor, err := fake.NewEditor(scratch).Connect(ctx, conn) + editor, err := fake.NewEditor(scratch, editorConfig).Connect(ctx, conn) if err != nil { t.Fatal(err) } diff --git a/internal/lsp/regtest/runner.go b/internal/lsp/regtest/runner.go index 557104b56b..3d838fa599 100644 --- a/internal/lsp/regtest/runner.go +++ b/internal/lsp/regtest/runner.go @@ -66,12 +66,12 @@ type Runner struct { } type runConfig struct { - modes Mode - proxyTxt string - timeout time.Duration - env []string - skipCleanup bool - gopath bool + editorConfig fake.EditorConfig + modes Mode + proxyTxt string + timeout time.Duration + skipCleanup bool + gopath bool } func (r *Runner) defaultConfig() *runConfig { @@ -113,11 +113,10 @@ func WithModes(modes Mode) RunOption { }) } -// WithEnv overlays environment variables encoded by "=