Fix Code Action Resolution

This fixes several parts of the code action handler workflow.

- Remove `source` as it is a base type and not a specific action we can use.
- Remove `source.fixAll` because the action implies fixing errors, but terraform fmt only formats code style.
- Remove `source.formatAll` to allow fine grained selection of actions.
- Rename `source.formatAll.terraform-ls` to `source.formatAll.terraform` to follow existing Code Action conventions.
- Correctly set the code action value in the returned `lsp.CodeAction` response to the one chosen by the user.
- Exit early so as to not format a document without an explicit request. When additional code actions are added, we can modify this to return a merged list of actions to apply.

docs/code-actions.md

@@ -0,0 +1,63 @@
+# 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
+
+### User Code Actions
+
+There are currently no Code Actions that can be invoked by the user yet.
+
+### Automatic Code Actions
+
+Automatic Code Actions happen when certain events are trigged, for example saving a document.
+
+#### `source.formatAll.terraform`
+
+The server will format a given document according to Terraform formatting conventions.
+
+> *Important:* Disable `editor.formatOnSave` if using `source.formatAll.terraform`
+
+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 based on the list provided. If you have both settings enabled, then your document will be formatted twice.
+
+## 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",
+}
+```
+
+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
+  },
+},
+```

docs/language-clients.md

@@ -59,11 +59,13 @@ 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.
 
+See [code-actions](./code-actions.md) for more detailed information.
+
 ### 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).
+This action is available as `source.formatAll.terraform` for clients which configure actions globally (such as Sublime Text LSP) and for clients which allow languageID or server specific configuration (such as VS Code).
 
 ## Code Lens
 

internal/langserver/handlers/code_action.go

@@ -22,13 +22,30 @@ 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
+	h.logger.Printf("Code action handler initiated")
+
+	// 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 action %q received", 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)
 	}
 
+	for ca := range wantedCodeActions {
+		h.logger.Printf("Code action %q supported", ca)
+	}
+
 	fh := ilsp.FileHandlerFromDocumentURI(params.TextDocument.URI)
 
 	fs, err := lsctx.DocumentStorage(ctx)
@@ -46,13 +63,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 +78,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,

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)
+		})
+	}
+}

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": {},