From a21c3ee1614a9c3c93588a516a389c07e5943921 Mon Sep 17 00:00:00 2001 From: James Pogran Date: Wed, 18 Aug 2021 15:05:56 -0400 Subject: [PATCH] (GH-327) Format on save code action This enables in the language server to perform one or more code actions on a range of text or a full document. This adds a code action to format a file based on the `editor.codeActionsOnSave` setting or when a request has `source.formatAll` or `source.formatAll.terraform-ls`. This can either be a global setting or one specific to the terraform language. --- docs/language-clients.md | 10 ++ internal/langserver/handlers/code_action.go | 75 +++++++++ .../langserver/handlers/code_action_test.go | 147 ++++++++++++++++++ internal/langserver/handlers/formatting.go | 13 ++ internal/langserver/handlers/handlers_test.go | 3 + internal/langserver/handlers/initialize.go | 4 + internal/langserver/handlers/service.go | 13 ++ internal/lsp/code_actions.go | 52 +++++++ 8 files changed, 317 insertions(+) create mode 100644 internal/langserver/handlers/code_action.go create mode 100644 internal/langserver/handlers/code_action_test.go create mode 100644 internal/lsp/code_actions.go diff --git a/docs/language-clients.md b/docs/language-clients.md index 098f628b..f2c6f7fd 100644 --- a/docs/language-clients.md +++ b/docs/language-clients.md @@ -55,6 +55,16 @@ Client is expected to always launch a single instance of the server and check fo It is assumed that paths to these folders will be provided as part of `workspaceFolders` in the `initialize` request per LSP. +## Code Actions + +The server implements a set of opt-in code actions which perform different actions for the user. The code action request is sent from the client to the server to compute commands for a given text document and range. These commands are typically code fixes to either fix problems or to beautify/refactor code. + +### Format Document + +The server will format a given document according to Terraform formatting conventions. + +This action is available as `source.formatAll.terraform-ls` for clients which configure actions globally (such as Sublime Text LSP) and as `source.formatAll` for clients which allow languageID or server specific configuration (such as VS Code). + ## Code Lens ### Reference Counts (opt-in) diff --git a/internal/langserver/handlers/code_action.go b/internal/langserver/handlers/code_action.go new file mode 100644 index 00000000..e17512f6 --- /dev/null +++ b/internal/langserver/handlers/code_action.go @@ -0,0 +1,75 @@ +package handlers + +import ( + "context" + "fmt" + + lsctx "github.com/hashicorp/terraform-ls/internal/context" + "github.com/hashicorp/terraform-ls/internal/langserver/errors" + ilsp "github.com/hashicorp/terraform-ls/internal/lsp" + lsp "github.com/hashicorp/terraform-ls/internal/protocol" + "github.com/hashicorp/terraform-ls/internal/terraform/module" +) + +func (h *logHandler) TextDocumentCodeAction(ctx context.Context, params lsp.CodeActionParams) []lsp.CodeAction { + ca, err := h.textDocumentCodeAction(ctx, params) + if err != nil { + h.logger.Printf("code action failed: %s", err) + } + + return ca +} + +func (h *logHandler) textDocumentCodeAction(ctx context.Context, params lsp.CodeActionParams) ([]lsp.CodeAction, error) { + var ca []lsp.CodeAction + + wantedCodeActions := ilsp.SupportedCodeActions.Only(params.Context.Only) + if len(wantedCodeActions) == 0 { + return nil, fmt.Errorf("could not find a supported code action to execute for %s, wanted %v", + params.TextDocument.URI, params.Context.Only) + } + + fh := ilsp.FileHandlerFromDocumentURI(params.TextDocument.URI) + + fs, err := lsctx.DocumentStorage(ctx) + if err != nil { + return ca, err + } + file, err := fs.GetDocument(fh) + if err != nil { + return ca, err + } + original, err := file.Text() + if err != nil { + return ca, err + } + + for action := range wantedCodeActions { + switch action { + case lsp.Source, lsp.SourceFixAll, ilsp.SourceFormatAll, ilsp.SourceFormatAllTerraformLs: + tfExec, err := module.TerraformExecutorForModule(ctx, fh.Dir()) + if err != nil { + return ca, errors.EnrichTfExecError(err) + } + + h.logger.Printf("formatting document via %q", tfExec.GetExecPath()) + + edits, err := formatDocument(ctx, tfExec, original, file) + if err != nil { + return ca, err + } + + ca = append(ca, lsp.CodeAction{ + Title: "Format Document", + Kind: lsp.SourceFixAll, + Edit: lsp.WorkspaceEdit{ + Changes: map[string][]lsp.TextEdit{ + string(fh.URI()): edits, + }, + }, + }) + } + } + + return ca, nil +} diff --git a/internal/langserver/handlers/code_action_test.go b/internal/langserver/handlers/code_action_test.go new file mode 100644 index 00000000..f8edba72 --- /dev/null +++ b/internal/langserver/handlers/code_action_test.go @@ -0,0 +1,147 @@ +package handlers + +import ( + "fmt" + "testing" + + "github.com/hashicorp/go-version" + "github.com/hashicorp/terraform-ls/internal/langserver" + "github.com/hashicorp/terraform-ls/internal/langserver/session" + "github.com/hashicorp/terraform-ls/internal/terraform/exec" + "github.com/stretchr/testify/mock" +) + +func TestLangServer_codeActionWithoutInitialization(t *testing.T) { + ls := langserver.NewLangServerMock(t, NewMockSession(nil)) + stop := ls.Start(t) + defer stop() + + ls.CallAndExpectError(t, &langserver.CallRequest{ + Method: "textDocument/codeAction", + ReqParams: fmt.Sprintf(`{ + "textDocument": { + "version": 0, + "languageId": "terraform", + "text": "provider \"github\" {}", + "uri": "%s/main.tf" + } + }`, TempDir(t).URI())}, session.SessionNotInitialized.Err()) +} + +func TestLangServer_codeAction_basic(t *testing.T) { + tmpDir := TempDir(t) + + ls := langserver.NewLangServerMock(t, NewMockSession(&MockSessionInput{ + TerraformCalls: &exec.TerraformMockCalls{ + PerWorkDir: map[string][]*mock.Call{ + tmpDir.Dir(): { + { + Method: "Version", + Repeatability: 1, + Arguments: []interface{}{ + mock.AnythingOfType(""), + }, + ReturnArguments: []interface{}{ + version.Must(version.NewVersion("0.12.0")), + nil, + nil, + }, + }, + { + Method: "GetExecPath", + Repeatability: 1, + ReturnArguments: []interface{}{ + "", + }, + }, + { + Method: "Format", + Repeatability: 1, + Arguments: []interface{}{ + mock.AnythingOfType(""), + []byte("provider \"test\" {\n\n }\n"), + }, + ReturnArguments: []interface{}{ + []byte("provider \"test\" {\n\n}\n"), + nil, + }, + }}, + }, + }, + })) + stop := ls.Start(t) + defer stop() + + ls.Call(t, &langserver.CallRequest{ + Method: "initialize", + ReqParams: fmt.Sprintf(`{ + "capabilities": {}, + "rootUri": %q, + "processId": 12345 + }`, tmpDir.URI())}) + ls.Notify(t, &langserver.CallRequest{ + Method: "initialized", + ReqParams: "{}", + }) + ls.Call(t, &langserver.CallRequest{ + Method: "textDocument/didOpen", + ReqParams: fmt.Sprintf(`{ + "textDocument": { + "version": 0, + "languageId": "terraform", + "text": "provider \"test\" {\n\n }\n", + "uri": "%s/main.tf" + } + }`, tmpDir.URI())}) + ls.CallAndExpectResponse(t, &langserver.CallRequest{ + Method: "textDocument/codeAction", + ReqParams: fmt.Sprintf(`{ + "textDocument": { "uri": "%s/main.tf" }, + "range": { + "start": { "line": 0, "character": 0 }, + "end": { "line": 1, "character": 0 } + }, + "context": { "diagnostics": [], "only": ["source.fixAll"] } + }`, tmpDir.URI())}, fmt.Sprintf(`{ + "jsonrpc": "2.0", + "id": 3, + "result": [ + { + "title": "Format Document", + "kind": "source.fixAll", + "edit":{ + "changes":{ + "%s/main.tf": [ + { + "range": { + "start": { + "line": 0, + "character": 0 + }, + "end": { + "line": 1, + "character": 0 + } + }, + "newText": "provider \"test\" {\n" + }, + { + "range": { + "start": { + "line": 2, + "character": 0 + }, + "end": { + "line": 3, + "character": 0 + } + }, + "newText": "}\n" + } + ] + } + } + } + ] + }`, tmpDir.URI())) +} diff --git a/internal/langserver/handlers/formatting.go b/internal/langserver/handlers/formatting.go index 7f88f926..2c6a1d7b 100644 --- a/internal/langserver/handlers/formatting.go +++ b/internal/langserver/handlers/formatting.go @@ -4,10 +4,12 @@ import ( "context" lsctx "github.com/hashicorp/terraform-ls/internal/context" + "github.com/hashicorp/terraform-ls/internal/filesystem" "github.com/hashicorp/terraform-ls/internal/hcl" "github.com/hashicorp/terraform-ls/internal/langserver/errors" ilsp "github.com/hashicorp/terraform-ls/internal/lsp" lsp "github.com/hashicorp/terraform-ls/internal/protocol" + "github.com/hashicorp/terraform-ls/internal/terraform/exec" "github.com/hashicorp/terraform-ls/internal/terraform/module" ) @@ -38,6 +40,17 @@ func (h *logHandler) TextDocumentFormatting(ctx context.Context, params lsp.Docu h.logger.Printf("formatting document via %q", tfExec.GetExecPath()) + edits, err = formatDocument(ctx, tfExec, original, file) + if err != nil { + return edits, err + } + + return edits, nil +} + +func formatDocument(ctx context.Context, tfExec exec.TerraformExecutor, original []byte, file filesystem.Document) ([]lsp.TextEdit, error) { + var edits []lsp.TextEdit + formatted, err := tfExec.Format(ctx, original) if err != nil { return edits, err diff --git a/internal/langserver/handlers/handlers_test.go b/internal/langserver/handlers/handlers_test.go index 4754f82e..aa64ed35 100644 --- a/internal/langserver/handlers/handlers_test.go +++ b/internal/langserver/handlers/handlers_test.go @@ -42,6 +42,9 @@ func initializeResponse(t *testing.T, commandPrefix string) string { "definitionProvider": true, "referencesProvider": true, "documentSymbolProvider": true, + "codeActionProvider": { + "codeActionKinds": ["source", "source.fixAll", "source.formatAll", "source.formatAll.terraform-ls"] + }, "codeLensProvider": {}, "documentLinkProvider": {}, "workspaceSymbolProvider": true, diff --git a/internal/langserver/handlers/initialize.go b/internal/langserver/handlers/initialize.go index 8fd7d8f8..8de7fe12 100644 --- a/internal/langserver/handlers/initialize.go +++ b/internal/langserver/handlers/initialize.go @@ -25,6 +25,10 @@ func (svc *service) Initialize(ctx context.Context, params lsp.InitializeParams) ResolveProvider: false, TriggerCharacters: []string{".", "["}, }, + CodeActionProvider: lsp.CodeActionOptions{ + CodeActionKinds: ilsp.SupportedCodeActions.AsSlice(), + ResolveProvider: false, + }, DeclarationProvider: lsp.DeclarationOptions{}, DefinitionProvider: true, CodeLensProvider: lsp.CodeLensOptions{}, diff --git a/internal/langserver/handlers/service.go b/internal/langserver/handlers/service.go index 1f69d06c..0848d935 100644 --- a/internal/langserver/handlers/service.go +++ b/internal/langserver/handlers/service.go @@ -231,6 +231,19 @@ func (svc *service) Assigner() (jrpc2.Assigner, error) { return handle(ctx, req, lh.TextDocumentHover) }, + "textDocument/codeAction": func(ctx context.Context, req *jrpc2.Request) (interface{}, error) { + err := session.CheckInitializationIsConfirmed() + if err != nil { + return nil, err + } + + ctx = lsctx.WithClientCapabilities(ctx, cc) + ctx = lsctx.WithDocumentStorage(ctx, svc.fs) + ctx = exec.WithExecutorOpts(ctx, svc.tfExecOpts) + ctx = exec.WithExecutorFactory(ctx, svc.tfExecFactory) + + return handle(ctx, req, lh.TextDocumentCodeAction) + }, "textDocument/codeLens": func(ctx context.Context, req *jrpc2.Request) (interface{}, error) { err := session.CheckInitializationIsConfirmed() if err != nil { diff --git a/internal/lsp/code_actions.go b/internal/lsp/code_actions.go new file mode 100644 index 00000000..e2dcc6c2 --- /dev/null +++ b/internal/lsp/code_actions.go @@ -0,0 +1,52 @@ +package lsp + +import ( + "sort" + + lsp "github.com/hashicorp/terraform-ls/internal/protocol" +) + +const ( + SourceFormatAll = "source.formatAll" + SourceFormatAllTerraformLs = "source.formatAll.terraform-ls" +) + +type CodeActions map[lsp.CodeActionKind]bool + +var ( + SupportedCodeActions = CodeActions{ + lsp.Source: true, + lsp.SourceFixAll: true, + SourceFormatAll: true, + SourceFormatAllTerraformLs: true, + } +) + +func (c CodeActions) AsSlice() []lsp.CodeActionKind { + s := make([]lsp.CodeActionKind, 0) + for v := range c { + s = append(s, v) + } + + sort.SliceStable(s, func(i, j int) bool { + return string(s[i]) < string(s[j]) + }) + return s +} + +func (ca CodeActions) Only(only []lsp.CodeActionKind) CodeActions { + // if only is empty, assume that the client wants all code actions + // else build mapping of requested and determine if supported + if len(only) == 0 { + return ca + } + + wanted := make(CodeActions, 0) + for _, kind := range only { + if v, ok := ca[kind]; ok { + wanted[kind] = v + } + } + + return wanted +}