extension/src: run formatter specified from setting "go.formatTool"

This change is similar to CL 709035. The "formatTool" will be the
source of truth indicating which tool will be run as formatter.
vscode-go will now execute the exact tool based on the setting without
any special yielding logic.

This change is slightly different from CL 709035. The default value
for "formatTool" is "default". To avoid any disruption, the "default"
will be interpreted as no formatter provided and gopls will handle
the formatting.

In addition, an E2E test is added to test the formatter behavior:
- Create unformatted go file, and prepare mock formatters.
- Execute vscode "Format file" command.
- Read the formatted go file from vscode buffer.
- Compare the content from buffer with the desired state.

For #3861

Change-Id: Ib39b1845fb3b9a058c71dd1ab36f4ad5b9fe22b2
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/713200
Auto-Submit: Hongxiang Jiang <[email protected]>
Reviewed-by: Madeline Kalil <[email protected]>
Reviewed-by: Alan Donovan <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>

Author: h9jiang

docs/settings.md

@@ -238,15 +238,15 @@ Default:
 Flags to pass to format tool (e.g. ["-s"]). Not applicable when using the language server.
 ### `go.formatTool`
 
-When the language server is enabled and one of `default`/`gofmt`/`goimports`/`gofumpt` is chosen, the language server will handle formatting. If `custom` tool is selected, the extension will use the `customFormatter` tool in the `#go.alternateTools#` section.<br/>
+Specifies the tool for formatting Go code. The default is `default`, which uses the language server `gopls` as formatting provider. To configure gopls's formatting, see the 'gopls.formatting' settings. When a specific tool (e.g., `gofmt`, `goimports`) is selected, the extension will run it instead.<br/>
 Allowed Options:
 
-* `default`: If the language server is enabled, format via the language server, which already supports gofmt, goimports, goreturns, and gofumpt. Otherwise, goimports.
-* `gofmt`: Formats the file according to the standard Go style. (not applicable when the language server is enabled)
-* `goimports`: Organizes imports and formats the file with gofmt. (not applicable when the language server is enabled)
-* `goformat`: Configurable gofmt, see https://github.com/mbenkmann/goformat. (Deprecated due to the lack of generics support)
-* `gofumpt`: Stricter version of gofmt, see https://github.com/mvdan/gofumpt. . Use `#gopls.format.gofumpt#` instead)
-* `custom`: Formats using the custom tool specified as `customFormatter` in the `#go.alternateTools#` setting. The tool should take the input as STDIN and output the formatted code as STDOUT.
+* `default`: Formatting is performed by the language server, gopls. (recommended)
+* `gofmt`: Formats using `gofmt`, the standard Go formatter. See https://pkg.go.dev/cmd/gofmt.
+* `goimports`: Formats using `goimports`, which organizes imports and applies `gofmt`. See https://pkg.go.dev/golang.org/x/tools/cmd/goimports.
+* `goformat`: Formats using `goformat`, a configurable version of `gofmt`. See https://github.com/mbenkmann/goformat. (Deprecated due to lack of generics support)
+* `gofumpt`: Formats using `gofumpt`, a stricter version of `gofmt`. See https://github.com/mvdan/gofumpt. Note: `gopls` can also be configured to use `gofumpt` via the `#gopls.formatting.gofumpt#` setting.
+* `custom`: Formats using a custom tool. The tool's path must be specified in the `#go.alternateTools#` setting under the `customFormatter` key.
 
 
 Default: `"default"`

extension/package.json

@@ -1236,7 +1236,7 @@
         "go.formatTool": {
           "type": "string",
           "default": "default",
-          "markdownDescription": "When the language server is enabled and one of `default`/`gofmt`/`goimports`/`gofumpt` is chosen, the language server will handle formatting. If `custom` tool is selected, the extension will use the `customFormatter` tool in the `#go.alternateTools#` section.",
+          "markdownDescription": "Specifies the tool for formatting Go code. The default is `default`, which uses the language server `gopls` as formatting provider. To configure gopls's formatting, see the 'gopls.formatting' settings. When a specific tool (e.g., `gofmt`, `goimports`) is selected, the extension will run it instead.",
           "scope": "resource",
           "enum": [
             "default",
@@ -1247,12 +1247,12 @@
             "custom"
           ],
           "markdownEnumDescriptions": [
-            "If the language server is enabled, format via the language server, which already supports gofmt, goimports, goreturns, and gofumpt. Otherwise, goimports.",
-            "Formats the file according to the standard Go style. (not applicable when the language server is enabled)",
-            "Organizes imports and formats the file with gofmt. (not applicable when the language server is enabled)",
-            "Configurable gofmt, see https://github.com/mbenkmann/goformat. (Deprecated due to the lack of generics support)",
-            "Stricter version of gofmt, see https://github.com/mvdan/gofumpt. . Use `#gopls.format.gofumpt#` instead)",
-            "Formats using the custom tool specified as `customFormatter` in the `#go.alternateTools#` setting. The tool should take the input as STDIN and output the formatted code as STDOUT."
+            "Formatting is performed by the language server, gopls. (recommended)",
+            "Formats using `gofmt`, the standard Go formatter. See https://pkg.go.dev/cmd/gofmt.",
+            "Formats using `goimports`, which organizes imports and applies `gofmt`. See https://pkg.go.dev/golang.org/x/tools/cmd/goimports.",
+            "Formats using `goformat`, a configurable version of `gofmt`. See https://github.com/mbenkmann/goformat. (Deprecated due to lack of generics support)",
+            "Formats using `gofumpt`, a stricter version of `gofmt`. See https://github.com/mvdan/gofumpt. Note: `gopls` can also be configured to use `gofumpt` via the `#gopls.formatting.gofumpt#` setting.",
+            "Formats using a custom tool. The tool's path must be specified in the `#go.alternateTools#` setting under the `customFormatter` key."
           ]
         },
         "go.formatFlags": {

extension/src/config.ts

@@ -86,6 +86,13 @@ This will result in duplicate diagnostics.`;
 			vscode.commands.executeCommand('workbench.action.openSettingsJson');
 		}
 	}
+
+	// Format tool check.
+	// TODO(hxjiang): send warning to user if "go.formatTool" is "gofumpt" and
+	// "gopls.formatting.gofumpt" is true.
+	// TODO(hxjiang): send one time notification suggest user to switch "gofmt",
+	// "gofumpt" and "goimports" formatter to gopls. Configure using setting
+	// "gopls.formatting".
 }
 
 /**

extension/src/goTools.ts

@@ -6,9 +6,10 @@
 
 'use strict';
 
-import moment = require('moment');
-import semver = require('semver');
-import { getFormatTool, usingCustomFormatTool } from './language/legacy/goFormat';
+import * as moment from 'moment';
+import * as semver from 'semver';
+import * as vscode from 'vscode';
+import { getFormatTool } from './language/legacy/goFormat';
 import { allToolsInformation } from './goToolsInformation';
 import { GoVersion } from './util';
 
@@ -110,7 +111,10 @@ export function hasModSuffix(tool: Tool): boolean {
 	return tool.name.endsWith('-gomod');
 }
 
-export function getConfiguredTools(goConfig: { [key: string]: any }, goplsConfig: { [key: string]: any }): Tool[] {
+export function getConfiguredTools(
+	goConfig: vscode.WorkspaceConfiguration,
+	goplsConfig: vscode.WorkspaceConfiguration
+): Tool[] {
 	// If language server is enabled, don't suggest tools that are replaced by gopls.
 	// TODO(github.com/golang/vscode-go/issues/388): decide what to do when
 	// the go version is no longer supported by gopls while the legacy tools are
@@ -144,11 +148,7 @@ export function getConfiguredTools(goConfig: { [key: string]: any }, goplsConfig
 		maybeAddTool('dlv');
 	}
 
-	// Only add format tools if the language server is disabled or the
-	// format tool is known to us.
-	if (!useLanguageServer || usingCustomFormatTool(goConfig)) {
-		maybeAddTool(getFormatTool(goConfig));
-	}
+	maybeAddTool(getFormatTool(goConfig));
 
 	maybeAddTool(goConfig['lintTool']);
 

extension/src/language/goLanguageServer.ts

@@ -40,7 +40,7 @@ import {
 import { Executable, LanguageClient, ServerOptions } from 'vscode-languageclient/node';
 import { getGoConfig, getGoplsConfig, extensionInfo } from '../config';
 import { toolExecutionEnvironment } from '../goEnv';
-import { GoDocumentFormattingEditProvider, usingCustomFormatTool } from './legacy/goFormat';
+import { GoDocumentFormattingEditProvider, getFormatTool } from './legacy/goFormat';
 import { installTools, latestModuleVersion, promptForMissingTool, promptForUpdatingTool } from '../goInstallTools';
 import { getTool, Tool } from '../goTools';
 import { updateGlobalState, updateWorkspaceState } from '../stateUtils';
@@ -79,6 +79,9 @@ export interface LanguageServerConfig {
 	flags: string[];
 	env: any;
 	features: {
+		// A custom formatter can be configured to run instead of gopls.
+		// This is enabled when the user has configured a specific format
+		// tool in the "go.formatTool" setting.
 		formatter?: GoDocumentFormattingEditProvider;
 	};
 	checkForUpdates: string;
@@ -650,9 +653,11 @@ export async function buildLanguageClient(
 					token: vscode.CancellationToken,
 					next: ProvideDocumentFormattingEditsSignature
 				) => {
+					// If a custom formatter is configured, use it.
 					if (cfg.features.formatter) {
 						return cfg.features.formatter.provideDocumentFormattingEdits(document, options, token);
 					}
+					// Otherwise, fall back to gopls.
 					return next(document, options, token);
 				},
 				handleDiagnostics: (
@@ -1016,9 +1021,10 @@ export async function buildLanguageServerConfig(
 	goConfig: vscode.WorkspaceConfiguration
 ): Promise<LanguageServerConfig> {
 	let formatter: GoDocumentFormattingEditProvider | undefined;
-	if (usingCustomFormatTool(goConfig)) {
+	if (getFormatTool(goConfig) !== '') {
 		formatter = new GoDocumentFormattingEditProvider();
 	}
+
 	const cfg: LanguageServerConfig = {
 		serverName: '', // remain empty if gopls binary can't be found.
 		path: '',

extension/src/language/legacy/goFormat.ts

@@ -15,6 +15,14 @@ import { promptForMissingTool, promptForUpdatingTool } from '../../goInstallTool
 import { getBinPath, resolvePath } from '../../util';
 import { killProcessTree } from '../../utils/processUtils';
 
+/**
+ * GoDocumentFormattingEditProvider is a feature that provides formatting
+ * functionality. It is only used when the user has configured a formatter in
+ * the "go.formatTool" setting.
+ *
+ * By default, the Go extension uses the language server (gopls) to provide
+ * formatting, so this class is not instantiated.
+ */
 export class GoDocumentFormattingEditProvider implements vscode.DocumentFormattingEditProvider {
 	public provideDocumentFormattingEdits(
 		document: vscode.TextDocument,
@@ -27,7 +35,7 @@ export class GoDocumentFormattingEditProvider implements vscode.DocumentFormatti
 
 		const filename = document.fileName;
 		const goConfig = getGoConfig(document.uri);
-		const formatFlags = goConfig['formatFlags'].slice() || [];
+		const formatFlags = goConfig.get<string[]>('formatFlags') ?? [];
 
 		// Ignore -w because we don't want to write directly to disk.
 		if (formatFlags.indexOf('-w') > -1) {
@@ -118,33 +126,24 @@ export class GoDocumentFormattingEditProvider implements vscode.DocumentFormatti
 	}
 }
 
-export function usingCustomFormatTool(goConfig: { [key: string]: any }): boolean {
-	const formatTool = getFormatTool(goConfig);
-	switch (formatTool) {
-		case 'goreturns':
-			return false;
-		case 'goimports':
-			return false;
-		case 'gofmt':
-			return false;
-		case 'gofumpt':
-			// TODO(rstambler): Prompt to configure setting in gopls.
-			return false;
-		case 'gofumports':
-			// TODO(rstambler): Prompt to configure setting in gopls.
-			return false;
-		default:
-			return true;
-	}
-}
+/**
+ * getFormatTool returns the formatter tool configured through the "go.formatTool"
+ * setting.
+ *
+ * If "go.formatTool" is set to "custom", it returns the "customFormatter"
+ * specified in "go.alternateTools".
+ *
+ * If "go.formatTool" is not set, it returns an empty string, indicating that
+ * no specific format tool is selected and gopls should be used.
+ */
+export function getFormatTool(goConfig: vscode.WorkspaceConfiguration): string {
+	const formatTool = goConfig.get<string>('formatTool');
 
-export function getFormatTool(goConfig: { [key: string]: any }): string {
-	const formatTool = goConfig['formatTool'];
-	if (formatTool === 'default') {
-		return 'goimports';
+	if (formatTool === undefined || formatTool === 'default') {
+		return ''; // not specified, yield to gopls by return empty string.
 	}
 	if (formatTool === 'custom') {
-		return resolvePath(goConfig['alternateTools']['customFormatter'] || 'goimports');
+		return resolvePath(goConfig['alternateTools']['customFormatter']);
 	}
 	return formatTool;
 }

extension/test/gopls/extension.test.ts

@@ -7,7 +7,6 @@
 import assert from 'assert';
 import * as path from 'path';
 import * as vscode from 'vscode';
-import { getGoConfig } from '../../src/config';
 import sinon = require('sinon');
 import { getGoVersion, GoVersion } from '../../src/util';
 import { GOPLS_MAYBE_PROMPT_FOR_TELEMETRY, recordTelemetryStartTime, TelemetryService } from '../../src/goTelemetry';
@@ -159,48 +158,6 @@ suite('Go Extension Tests With Gopls', function () {
 		}
 	});
 
-	async function testCustomFormatter(goConfig: vscode.WorkspaceConfiguration, customFormatter: string) {
-		const config = require('../../src/config');
-		sandbox.stub(config, 'getGoConfig').returns(goConfig);
-		const workspaceDir = path.resolve(testdataDir, 'gogetdocTestData');
-		await env.startGopls(path.join(workspaceDir, 'test.go'), goConfig, workspaceDir);
-		const { doc } = await env.openDoc(testdataDir, 'gogetdocTestData', 'format.go');
-		await vscode.window.showTextDocument(doc);
-
-		const formatFeature = env.languageClient?.getFeature('textDocument/formatting');
-		const formatter = formatFeature?.getProvider(doc);
-		const tokensrc = new vscode.CancellationTokenSource();
-		try {
-			const result = await formatter?.provideDocumentFormattingEdits(
-				doc,
-				{} as vscode.FormattingOptions,
-				tokensrc.token
-			);
-			assert.fail(`formatter unexpectedly succeeded and returned a result: ${JSON.stringify(result)}`);
-		} catch (e) {
-			assert(`${e}`.includes(`errors when formatting with ${customFormatter}`), `${e}`);
-		}
-	}
-
-	test('Nonexistent formatter', async () => {
-		const customFormatter = 'nonexistent';
-		const goConfig = Object.create(getGoConfig(), {
-			formatTool: { value: customFormatter } // this should make the formatter fail.
-		}) as vscode.WorkspaceConfiguration;
-
-		await testCustomFormatter(goConfig, customFormatter);
-	});
-
-	test('Custom formatter', async () => {
-		const customFormatter = 'coolCustomFormatter';
-		const goConfig = Object.create(getGoConfig(), {
-			formatTool: { value: 'custom' }, // this should make the formatter fail.
-			alternateTools: { value: { customFormatter: customFormatter } } // this should make the formatter fail.
-		}) as vscode.WorkspaceConfiguration;
-
-		await testCustomFormatter(goConfig, customFormatter);
-	});
-
 	test('Prompt For telemetry', async () => {
 		const workspaceDir = path.resolve(testdataDir, 'gogetdocTestData');
 		await env.startGopls(path.join(workspaceDir, 'test.go'), undefined, workspaceDir);