diff --git a/docs/code-actions.md b/docs/code-actions.md new file mode 100644 index 000000000..7ddfd5d16 --- /dev/null +++ b/docs/code-actions.md @@ -0,0 +1,54 @@ +# Code Actions + +The Terraform Language Server implements a set of Code Actions which perform different actions on the current document. These commands are typically code fixes to either refactor code, fix problems or to beautify/refactor code. + +## Available Actions + +### `source.formatAll.terraform` + +The server will format a given document according to Terraform formatting conventions. + + +## Usage + +### VS Code + +To enable the format code action globally, set `source.formatAll.terraform` to *true* for the `editor.codeActionsOnSave` setting and set `editor.formatOnSave` to *false*. + +```json +"editor.formatOnSave": false, +"editor.codeActionsOnSave": { + "source.formatAll.terraform": true +}, +"[terraform]": { + "editor.defaultFormatter": "hashicorp.terraform", +} +``` + +> *Important:* Disable `editor.formatOnSave` if you are using `source.formatAll.terraform` in `editor.codeActionsOnSave`. The `source.formatAll.terraform` code action is is meant to be used instead of `editor.formatOnSave`, as it provides a [guarantee of order of execution](https://github.com/microsoft/vscode-docs/blob/71643d75d942e2c32cfd781c2b5322521775fb4a/release-notes/v1_44.md#explicit-ordering-for-editorcodeactionsonsave) based on the list provided. If you have both settings enabled, then your document will be formatted twice. + +If you would like `editor.formatOnSave` to be *true* for other extensions but *false* for the Terraform extension, you can configure your settings as follows: + +```json +"editor.formatOnSave": true, +"editor.codeActionsOnSave": { + "source.formatAll.terraform": true +}, +"[terraform]": { + "editor.defaultFormatter": "hashicorp.terraform", + "editor.formatOnSave": false, +}, +``` + +Alternatively, you can include all terraform related Code Actions inside the language specific setting if you prefer: + +```json +"editor.formatOnSave": true, +"[terraform]": { + "editor.defaultFormatter": "hashicorp.terraform", + "editor.formatOnSave": false, + "editor.codeActionsOnSave": { + "source.formatAll.terraform": true + }, +}, +``` diff --git a/docs/language-clients.md b/docs/language-clients.md index f2c6f7fdc..d693de6d1 100644 --- a/docs/language-clients.md +++ b/docs/language-clients.md @@ -59,11 +59,47 @@ in the `initialize` request per LSP. 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 +See [code-actions](./code-actions.md) for a list of supported code actions. -The server will format a given document according to Terraform formatting conventions. +A Code Action is an action that changes content in the active editor. Each Code Action is grouped into kinds that have a `command` and/or a series of `edits`. They are triggered either by the user or through events. -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). +> Documentation for code actions outside of VS Code is unfortunately very limited beyond description of the LSP methods. VS Code internally makes certain assumptions. We follow these assumptions (as documented below) and we recommend other clients to follow these assumptions for best experience, unless/until LSP documentation recommends otherwise. + +### Triggers + +In VS Code, code action can be _invoked manually_ or _automatically_ based on the respective [CodeActionTriggerKind](https://code.visualstudio.com/api/references/vscode-api#CodeActionTriggerKind). + +**Manually invoked** actions come from the contextual in-lineđź’ˇ icon inside the editor, and are chosen by the user. The user can choose which action is invoked and *then* invoke it. However, in order for the client to display the contextual actions, the client requests LS to "pre-calculate" any actions relevant to the cursor position. Then, when the user selects the action in the UI, the client applies the `edits` or executes the `command` as provided by the server. + +**Automatically triggered** actions come from events such as "on save", as configured via the `editor.codeActionsOnSave` setting. These usually do not give much choice to the user, they are either on or off, as they cannot accept user input. For example, formatting a document or removing simple style errors don't prompt for user action before or during execution. + +### Kinds + +Each `Code Action` has a [`CodeActionKind`](https://code.visualstudio.com/api/references/vscode-api#CodeActionKind). `Code Action Kinds` are a hierarchical list of identifiers separated by `.`. For example in `refactor.extract.function`: `refactor` is the trunk, `extract` is the branch, and `function` is the leaf. The branches and leaves are the point of intended customization, you add new branches and/or leaves for each type of function you perform. In this example, a new code action that operated on variables would be called `refactor.extract.variable`. + +#### Custom Kinds + +Adding new roots or branches of the hierarchy is not emphasized or really encouraged. In most cases they are meant to be concepts that can be applied generically, not specifically to a certain language. For example, extracting a value to a variable is something common to most languages. You do not need a `refactor.extract.golang.variable` to extract a variable in Go, it still makes sense to use `refactor.extract.variable` whether in Go or in PowerShell. + +Keeping to existing `kinds` also helps in registration of supported code actions. If you register support for `source.fixAll` instead of `source.fixAll.languageId`, then a user that has `source.fixAll` in their `codeActionsOnSave` does not need to add specific lines for your extension, all supported extensions are called on save. Another reason is VS Code won't list your custom supported code actions inside `editor.codeActionsOnSave`, and there isn't a way for the extension to get them there. The user will have to consult your documentation to find out what actions are supported and add them without intellisense. + +A reason to add custom _kinds_ is if the action is sufficiently different from an existing base action. For example, formatting of the current file on save. The interpretation of `source.fixAll` is to _apply any/all actions that address an existing diagnostic and have a clear fix that does not require user input_. Formatting therefore doesn't fit the interpretation of `source.fixAll`. + +A custom kind `source.formatAll.terraform` may format code. A user can request both `source.fixAll` and `source.formatAll.terraform` via their editor/client settings and the server would run `source.formatAll.terraform` only. Other servers may run `source.fixAll` but not `source.formatAll.terraform`, assuming they do not support that custom code action kind. + +Unlike generic kinds, custom ones are only discoverable in server-specific documentation and only relevant to the server. + +### Execution of Code Actions + +A request can have zero or more code actions to perform and the [LS is responsible for processing](https://github.com/microsoft/language-server-protocol/issues/970) all requested actions. The client will send a list of any code actions to execute (which may also be empty). + +An empty list means execute anything supported and return the list of edits to the client. This often comes from _manually invoked_ actions by the user. This is the easiest situation for the LS to choose what to do. The LS has a list of supported actions it can execute, so it executes and returns the edits. However, such actions will not include any that either require user input or make a change that could introduce an error (creating files, changing code structure, etc). + +A list of actions means to execute anything in that list, which is actually interpreted as _execute the hierarchy of a code action from these actions in the list_. For example, if a client requests a `refactor` action the LS should return all of the possible `refactor` actions it can execute (`refactor.extract`, `refactor.inline`, `refactor.rewrite`, etc.), and the user can choose which to execute. A client that sends `refactor.extract.method` would receive just that single code action from the LS. So a request with one action could return ten results, or just one. + +Clients are expected to filter actions to only send unique ones. For example, the user may have configured both `source.fixAll` and `source.fixAll.eslint` which are equivalent from the perspective of a single server (eslint). The client should favour the most specific (in this case `source.fixAll.eslint`) if possible, such that the server doesn't have to perform any de-duplication and the same action doesn't run multiple times. + +Clients may also impose a timeout for returning response for any of these requests. If the LS takes too long to process and return an action, the client may either give up and not do anything, or (preferably) display a progress indicator. This timeout may be configurable by the user, but ideally the default one is sufficient. ## Code Lens diff --git a/internal/langserver/handlers/code_action.go b/internal/langserver/handlers/code_action.go index e17512f62..c517459a8 100644 --- a/internal/langserver/handlers/code_action.go +++ b/internal/langserver/handlers/code_action.go @@ -23,12 +23,26 @@ func (h *logHandler) TextDocumentCodeAction(ctx context.Context, params lsp.Code func (h *logHandler) textDocumentCodeAction(ctx context.Context, params lsp.CodeActionParams) ([]lsp.CodeAction, error) { var ca []lsp.CodeAction + // For action definitions, refer to https://code.visualstudio.com/api/references/vscode-api#CodeActionKind + // We only support format type code actions at the moment, and do not want to format without the client asking for + // them, so exit early here if nothing is requested. + if len(params.Context.Only) == 0 { + h.logger.Printf("No code action requested, exiting") + return ca, nil + } + + for _, o := range params.Context.Only { + h.logger.Printf("Code actions requested: %q", o) + } + 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) } + h.logger.Printf("Code actions supported: %v", wantedCodeActions) + fh := ilsp.FileHandlerFromDocumentURI(params.TextDocument.URI) fs, err := lsctx.DocumentStorage(ctx) @@ -46,13 +60,13 @@ func (h *logHandler) textDocumentCodeAction(ctx context.Context, params lsp.Code for action := range wantedCodeActions { switch action { - case lsp.Source, lsp.SourceFixAll, ilsp.SourceFormatAll, ilsp.SourceFormatAllTerraformLs: + case ilsp.SourceFormatAllTerraform: tfExec, err := module.TerraformExecutorForModule(ctx, fh.Dir()) if err != nil { return ca, errors.EnrichTfExecError(err) } - h.logger.Printf("formatting document via %q", tfExec.GetExecPath()) + h.logger.Printf("Formatting document via %q", tfExec.GetExecPath()) edits, err := formatDocument(ctx, tfExec, original, file) if err != nil { @@ -61,7 +75,7 @@ func (h *logHandler) textDocumentCodeAction(ctx context.Context, params lsp.Code ca = append(ca, lsp.CodeAction{ Title: "Format Document", - Kind: lsp.SourceFixAll, + Kind: action, Edit: lsp.WorkspaceEdit{ Changes: map[string][]lsp.TextEdit{ string(fh.URI()): edits, diff --git a/internal/langserver/handlers/code_action_test.go b/internal/langserver/handlers/code_action_test.go index f8edba727..f81d54c02 100644 --- a/internal/langserver/handlers/code_action_test.go +++ b/internal/langserver/handlers/code_action_test.go @@ -101,14 +101,14 @@ func TestLangServer_codeAction_basic(t *testing.T) { "start": { "line": 0, "character": 0 }, "end": { "line": 1, "character": 0 } }, - "context": { "diagnostics": [], "only": ["source.fixAll"] } + "context": { "diagnostics": [], "only": ["source.formatAll.terraform"] } }`, tmpDir.URI())}, fmt.Sprintf(`{ "jsonrpc": "2.0", "id": 3, "result": [ { "title": "Format Document", - "kind": "source.fixAll", + "kind": "source.formatAll.terraform", "edit":{ "changes":{ "%s/main.tf": [ @@ -145,3 +145,194 @@ func TestLangServer_codeAction_basic(t *testing.T) { ] }`, tmpDir.URI())) } + +func TestLangServer_codeAction_no_code_action_requested(t *testing.T) { + tmpDir := TempDir(t) + + tests := []struct { + name string + request *langserver.CallRequest + want string + }{ + { + name: "no code action requested", + request: &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": [""] } + }`, tmpDir.URI())}, + want: `{ + "jsonrpc": "2.0", + "id": 3, + "result": null + }`, + }, + { + name: "source.formatAll.terraform code action requested", + request: &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.formatAll.terraform"] } + }`, tmpDir.URI())}, + want: fmt.Sprintf(`{ + "jsonrpc": "2.0", + "id": 3, + "result": [ + { + "title":"Format Document", + "kind":"source.formatAll.terraform", + "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()), + }, + { + name: "source.fixAll and source.formatAll.terraform code action requested", + request: &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", "source.formatAll.terraform"] } + }`, tmpDir.URI()), + }, + want: fmt.Sprintf(`{ + "jsonrpc": "2.0", + "id": 3, + "result": [ + { + "title": "Format Document", + "kind": "source.formatAll.terraform", + "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()), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.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": 123456 + }`, 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, tt.request, tt.want) + }) + } +} diff --git a/internal/langserver/handlers/handlers_test.go b/internal/langserver/handlers/handlers_test.go index aa64ed355..2c7f603fa 100644 --- a/internal/langserver/handlers/handlers_test.go +++ b/internal/langserver/handlers/handlers_test.go @@ -43,7 +43,7 @@ func initializeResponse(t *testing.T, commandPrefix string) string { "referencesProvider": true, "documentSymbolProvider": true, "codeActionProvider": { - "codeActionKinds": ["source", "source.fixAll", "source.formatAll", "source.formatAll.terraform-ls"] + "codeActionKinds": ["source.formatAll.terraform"] }, "codeLensProvider": {}, "documentLinkProvider": {}, diff --git a/internal/lsp/code_actions.go b/internal/lsp/code_actions.go index e2dcc6c23..93d781b6b 100644 --- a/internal/lsp/code_actions.go +++ b/internal/lsp/code_actions.go @@ -7,18 +7,31 @@ import ( ) const ( - SourceFormatAll = "source.formatAll" - SourceFormatAllTerraformLs = "source.formatAll.terraform-ls" + // SourceFormatAllTerraform is a Terraform specific format code action. + SourceFormatAllTerraform = "source.formatAll.terraform" ) type CodeActions map[lsp.CodeActionKind]bool var ( + // `source.*`: Source code actions apply to the entire file. They must be explicitly + // requested and will not show in the normal lightbulb menu. Source actions + // can be run on save using editor.codeActionsOnSave and are also shown in + // the source context menu. + // For action definitions, refer to: https://code.visualstudio.com/api/references/vscode-api#CodeActionKind + + // `source.fixAll`: Fix all actions automatically fix errors that have a clear fix that do + // not require user input. They should not suppress errors or perform unsafe + // fixes such as generating new types or classes. + // ** We don't support this as terraform fmt only adjusts style** + // lsp.SourceFixAll: true, + + // `source.formatAll`: Generic format code action. + // We do not register this for terraform to allow fine grained selection of actions. + // A user should be able to set `source.formatAll` to true, and source.formatAll.terraform to false to allow all + // files to be formatted, but not terraform files (or vice versa). SupportedCodeActions = CodeActions{ - lsp.Source: true, - lsp.SourceFixAll: true, - SourceFormatAll: true, - SourceFormatAllTerraformLs: true, + SourceFormatAllTerraform: true, } ) @@ -35,13 +48,8 @@ func (c CodeActions) AsSlice() []lsp.CodeActionKind { } 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