extension/src: getFormatTool return "customFormatter" instead of path

Previously, getFormatTool returns the path specified in
go.alternateTools, but the tool existence check is expecting a tool
name not a tool path.

Instead of returning a path, getFormatTool can return the
"customFormatter" string. The getBinPath function will resolve the path.

Validate config also throw error when formatTool is set to custom but
formatter's path is not provided in the alternateTools.

[bug fix] tool existence check should be triggered under all condition
regardless of whether user is using language server.

Fix #3906

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

Author: h9jiang

extension/src/config.ts

@@ -103,6 +103,19 @@ This will result in duplicate diagnostics.`;
 
 	// Format tool check.
 	const formatTool = getFormatTool(goConfig);
+	if (formatTool === 'customFormatter') {
+		const alternateTools = goConfig.get<any>('alternateTools');
+		if (alternateTools === undefined || alternateTools['customFormatter'] === undefined) {
+			const message =
+				"Error: formatter is configured to custom (go.formatTool=custom) but custom formatter path is not provided in go.alternateTools['customFormatter']";
+
+			const selected = await vscode.window.showWarningMessage(message, 'Open Settings');
+			if (selected === 'Open Settings') {
+				vscode.commands.executeCommand('workbench.action.openSettingsJson');
+			}
+		}
+	}
+
 	if (formatTool !== '' && goplsConfig['formatting.gofumpt'] === true) {
 		const message = `Warning: formatter is configured to run from both client side (go.formatTool=${formatTool}) and server side (gopls.formatting.gofumpt=true).
 This may result in double formatting.`;

extension/src/goMain.ts

@@ -312,13 +312,7 @@ export function addConfigChangeListener(ctx: vscode.ExtensionContext) {
 			}
 
 			if (e.affectsConfiguration('go.formatTool')) {
-				const tool = getFormatTool(goConfig);
-				// When language server gopls is active (by default), existence
-				// checks are skipped only for tools that gopls replaces. Other
-				// tools are still checked.
-				if (goConfig['useLanguageServer'] === false || !new Set(['gofmt', 'goimports', 'goformat']).has(tool)) {
-					checkToolExists(tool);
-				}
+				checkToolExists(getFormatTool(goConfig));
 			}
 			if (e.affectsConfiguration('go.docsTool')) {
 				checkToolExists(goConfig['docsTool']);

extension/src/language/legacy/goFormat.ts

@@ -12,7 +12,7 @@ import vscode = require('vscode');
 import { getGoConfig } from '../../config';
 import { toolExecutionEnvironment } from '../../goEnv';
 import { promptForMissingTool, promptForUpdatingTool } from '../../goInstallTools';
-import { getBinPath, resolvePath } from '../../util';
+import { getBinPath } from '../../util';
 import { killProcessTree } from '../../utils/processUtils';
 
 /**
@@ -130,8 +130,8 @@ export class GoDocumentFormattingEditProvider implements vscode.DocumentFormatti
  * 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 set to "custom", it returns "customFormatter". User
+ * should specify "customFormatter" in setting "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.
@@ -143,7 +143,7 @@ export function getFormatTool(goConfig: vscode.WorkspaceConfiguration): string {
 		return ''; // not specified, yield to gopls by return empty string.
 	}
 	if (formatTool === 'custom') {
-		return resolvePath(goConfig['alternateTools']['customFormatter']);
+		return 'customFormatter';
 	}
 	return formatTool;
 }

extension/test/gopls/formatting.test.ts

@@ -155,6 +155,13 @@ fmt.Println("hello")
 			const config = require('../../src/config');
 			sandbox.stub(config, 'getGoConfig').returns(goConfig);
 
+			// Disable path resolver's cache.
+			const util = require('../../src/util');
+			sandbox.stub(util, 'getBinPath').callsFake((...args: any[]) => {
+				const [tool] = args;
+				return util.getBinPathWithExplanation(tool, false).binPath;
+			});
+
 			// Create formatter script under tool dir and make it executable.
 			if (tc.formatter && tc.formatterScript) {
 				fs.writeFileSync(path.join(toolPath, tc.formatter), tc.formatterScript);

extension/test/integration/configuration.test.ts

@@ -107,6 +107,25 @@ suite('Configuration validation tests', () => {
 			expectWarning: false
 		},
 		// Format tool setting tests.
+		{
+			name: 'Error: formatTool is custom but path not provided',
+			goConfig: new Map<string, any>([
+				['useLanguageServer', true],
+				['formatTool', 'custom']
+			]),
+			expectWarning: true,
+			warningMessage:
+				"Error: formatter is configured to custom (go.formatTool=custom) but custom formatter path is not provided in go.alternateTools['customFormatter']"
+		},
+		{
+			name: 'Happy path: formatTool is custom and path is provided',
+			goConfig: new Map<string, any>([
+				['useLanguageServer', true],
+				['formatTool', 'custom'],
+				['alternateTools', { customFormatter: 'foo' }]
+			]),
+			expectWarning: false
+		},
 		{
 			name: 'Conflict: formatTool is gofumpt and gopls gofumpt setting is true',
 			suggested: true,