From e750c417fbbc76ecc8514bcc384e9b3337b49a7d Mon Sep 17 00:00:00 2001 From: Rebecca Stambler Date: Thu, 27 Jun 2019 21:26:42 -0400 Subject: [PATCH] internal/lsp: handle the context.only parameter for code actions This change refactors code actions to handle the Context.Only parameter, which indicates which code actions a language server should execute. Change-Id: Iddfccbbeba3a53fde2aa8df844434f2ab9d01666 Reviewed-on: https://go-review.googlesource.com/c/tools/+/184158 Run-TryBot: Rebecca Stambler Reviewed-by: Ian Cottrell --- internal/lsp/code_action.go | 117 ++++++++++++++++++++++++++---------- internal/lsp/general.go | 11 +++- internal/lsp/lsp_test.go | 4 ++ internal/lsp/server.go | 3 + 4 files changed, 101 insertions(+), 34 deletions(-) diff --git a/internal/lsp/code_action.go b/internal/lsp/code_action.go index ea4607a4..e5195e7b 100644 --- a/internal/lsp/code_action.go +++ b/internal/lsp/code_action.go @@ -6,6 +6,7 @@ package lsp import ( "context" + "fmt" "strings" "golang.org/x/tools/internal/lsp/protocol" @@ -14,7 +15,24 @@ import ( ) func (s *Server) codeAction(ctx context.Context, params *protocol.CodeActionParams) ([]protocol.CodeAction, error) { + + // The Only field of the context specifies which code actions the client wants. + // If Only is empty, assume that the client wants all of the possible code actions. + var wanted map[protocol.CodeActionKind]bool + if len(params.Context.Only) == 0 { + wanted = s.supportedCodeActions + } else { + wanted = make(map[protocol.CodeActionKind]bool) + for _, only := range params.Context.Only { + wanted[only] = s.supportedCodeActions[only] + } + } + uri := span.NewURI(params.TextDocument.URI) + if len(wanted) == 0 { + return nil, fmt.Errorf("no supported code action to execute for %s, wanted %v", uri, params.Context.Only) + } + view := s.session.ViewOf(uri) gof, m, err := getGoFile(ctx, view, uri) if err != nil { @@ -24,25 +42,27 @@ func (s *Server) codeAction(ctx context.Context, params *protocol.CodeActionPara if err != nil { return nil, err } + var codeActions []protocol.CodeAction - // TODO(rstambler): Handle params.Context.Only when VSCode-Go uses a - // version of vscode-languageclient that fixes - // https://github.com/Microsoft/vscode-languageserver-node/issues/442. + edits, err := organizeImports(ctx, view, spn) if err != nil { return nil, err } - if len(edits) > 0 { - codeActions = append(codeActions, protocol.CodeAction{ - Title: "Organize Imports", - Kind: protocol.SourceOrganizeImports, - Edit: &protocol.WorkspaceEdit{ - Changes: &map[string][]protocol.TextEdit{ - string(spn.URI()): edits, - }, - }, - }) - // If we also have diagnostics, we can associate them with quick fixes. + + // If the user wants to see quickfixes. + if wanted[protocol.QuickFix] { + // First, add the quick fixes reported by go/analysis. + // TODO: Enable this when this actually works. For now, it's needless work. + if s.wantSuggestedFixes { + qf, err := quickFixes(ctx, view, gof) + if err != nil { + view.Session().Logger().Errorf(ctx, "quick fixes failed for %s: %v", uri, err) + } + codeActions = append(codeActions, qf...) + } + + // If we also have diagnostics for missing imports, we can associate them with quick fixes. if findImportErrors(params.Context.Diagnostics) { // TODO(rstambler): Separate this into a set of codeActions per diagnostic, // where each action is the addition or removal of one import. @@ -57,26 +77,21 @@ func (s *Server) codeAction(ctx context.Context, params *protocol.CodeActionPara }, }) } - diags := gof.GetPackage(ctx).GetDiagnostics() - for _, diag := range diags { - pdiag, err := toProtocolDiagnostic(ctx, view, diag) - if err != nil { - return nil, err - } - for _, ca := range diag.SuggestedFixes { - codeActions = append(codeActions, protocol.CodeAction{ - Title: ca.Title, - Kind: protocol.QuickFix, // TODO(matloob): Be more accurate about these? - Edit: &protocol.WorkspaceEdit{ - Changes: &map[string][]protocol.TextEdit{ - string(spn.URI()): edits, - }, - }, - Diagnostics: []protocol.Diagnostic{pdiag}, - }) - } - } } + + // Add the results of import organization as source.OrganizeImports. + if wanted[protocol.SourceOrganizeImports] { + codeActions = append(codeActions, protocol.CodeAction{ + Title: "Organize Imports", + Kind: protocol.SourceOrganizeImports, + Edit: &protocol.WorkspaceEdit{ + Changes: &map[string][]protocol.TextEdit{ + string(spn.URI()): edits, + }, + }, + }) + } + return codeActions, nil } @@ -112,3 +127,39 @@ func findImportErrors(diagnostics []protocol.Diagnostic) bool { } return false } + +func quickFixes(ctx context.Context, view source.View, gof source.GoFile) ([]protocol.CodeAction, error) { + var codeActions []protocol.CodeAction + + // TODO: This is technically racy because the diagnostics provided by the code action + // may not be the same as the ones that gopls is aware of. + // We need to figure out some way to solve this problem. + diags := gof.GetPackage(ctx).GetDiagnostics() + for _, diag := range diags { + pdiag, err := toProtocolDiagnostic(ctx, view, diag) + if err != nil { + return nil, err + } + for _, ca := range diag.SuggestedFixes { + _, m, err := getGoFile(ctx, view, diag.URI()) + if err != nil { + return nil, err + } + edits, err := ToProtocolEdits(m, ca.Edits) + if err != nil { + return nil, err + } + codeActions = append(codeActions, protocol.CodeAction{ + Title: ca.Title, + Kind: protocol.QuickFix, // TODO(matloob): Be more accurate about these? + Edit: &protocol.WorkspaceEdit{ + Changes: &map[string][]protocol.TextEdit{ + string(diag.URI()): edits, + }, + }, + Diagnostics: []protocol.Diagnostic{pdiag}, + }) + } + } + return codeActions, nil +} diff --git a/internal/lsp/general.go b/internal/lsp/general.go index c9767609..2ba32b49 100644 --- a/internal/lsp/general.go +++ b/internal/lsp/general.go @@ -34,6 +34,11 @@ func (s *Server) initialize(ctx context.Context, params *protocol.InitializePara } } + s.supportedCodeActions = map[protocol.CodeActionKind]bool{ + protocol.SourceOrganizeImports: true, + protocol.QuickFix: true, + } + s.setClientCapabilities(params.Capabilities) folders := params.WorkspaceFolders @@ -188,10 +193,14 @@ func (s *Server) processConfig(view source.View, config interface{}) error { if usePlaceholders, ok := c["usePlaceholders"].(bool); ok { s.usePlaceholders = usePlaceholders } - // Check if user has disabled documentation on hover. + // Check if the user has disabled documentation on hover. if noDocsOnHover, ok := c["noDocsOnHover"].(bool); ok { s.noDocsOnHover = noDocsOnHover } + // Check if the user wants to see suggested fixes from go/analysis. + if wantSuggestedFixes, ok := c["wantSuggestedFixes"].(bool); ok { + s.wantSuggestedFixes = wantSuggestedFixes + } // Check if the user has explicitly disabled any analyses. if disabledAnalyses, ok := c["experimentalDisabledAnalyses"].([]interface{}); ok { s.disabledAnalyses = make(map[string]struct{}) diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go index 8f3eda4a..39603a7e 100644 --- a/internal/lsp/lsp_test.go +++ b/internal/lsp/lsp_test.go @@ -51,6 +51,10 @@ func testLSP(t *testing.T, exporter packagestest.Exporter) { server: &Server{ session: session, undelivered: make(map[span.URI][]source.Diagnostic), + supportedCodeActions: map[protocol.CodeActionKind]bool{ + protocol.SourceOrganizeImports: true, + protocol.QuickFix: true, + }, }, data: data, } diff --git a/internal/lsp/server.go b/internal/lsp/server.go index 97f8b5b7..44c145b6 100644 --- a/internal/lsp/server.go +++ b/internal/lsp/server.go @@ -78,6 +78,9 @@ type Server struct { dynamicConfigurationSupported bool preferredContentFormat protocol.MarkupKind disabledAnalyses map[string]struct{} + wantSuggestedFixes bool + + supportedCodeActions map[protocol.CodeActionKind]bool textDocumentSyncKind protocol.TextDocumentSyncKind