Skip to content

Commit

Permalink
src: support custom formatTools with gopls enabled
Browse files Browse the repository at this point in the history
gopls doesn't support all custom formatters, so wire through support
for custom formatting tools in the VS Code Go extension. The new default
formatTool is empty, since it's not used when configured with gopls.
If the user has gopls disabled, the default is goimports, as before.

The only thing is that I can't figure out how to make settings so they
can be enums or any other string, so custom formatters may look like
unrecognized settings.

Fixes golang#1238

Change-Id: I1694dc80e9d119cf83cb9a3a19dbdaef0cfac41f
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/295809
Trust: Rebecca Stambler <[email protected]>
Run-TryBot: Rebecca Stambler <[email protected]>
TryBot-Result: kokoro <[email protected]>
Reviewed-by: Hyang-Ah Hana Kim <[email protected]>
  • Loading branch information
stamblerre committed Mar 11, 2021
1 parent a319988 commit 8f2bcc1
Show file tree
Hide file tree
Showing 9 changed files with 108 additions and 17 deletions.
12 changes: 10 additions & 2 deletions docs/settings.md
Original file line number Diff line number Diff line change
Expand Up @@ -233,9 +233,17 @@ Flags to pass to format tool (e.g. ["-s"]). Not applicable when using the langua
### `go.formatTool`

Not applicable when using the language server. Choosing 'goimports', 'goreturns', or 'gofumports' will add missing imports and remove unused imports.<br/>
Allowed Options: `gofmt`, `goimports`, `goreturns`, `goformat`, `gofumpt`, `gofumports`
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.
* `goimports`: Organizes imports and formats the file with gofmt.
* `goformat`: Configurable gofmt, see https://github.com/mbenkmann/goformat.
* `gofumpt`: Stricter version of gofmt, see https://github.com/mvdan/gofumpt.
* `gofumports`: Applies gofumpt formatting and organizes imports.

Default: `"goimports"`

Default: `""`
### `go.generateTestsFlags`

Additional command line flags to pass to `gotests` for generating tests.
Expand Down
12 changes: 10 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -1018,16 +1018,24 @@
},
"go.formatTool": {
"type": "string",
"default": "goimports",
"default": "",
"description": "Not applicable when using the language server. Choosing 'goimports', 'goreturns', or 'gofumports' will add missing imports and remove unused imports.",
"scope": "resource",
"enum": [
"default",
"gofmt",
"goimports",
"goreturns",
"goformat",
"gofumpt",
"gofumports"
],
"enumDescriptions": [
"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.",
"Organizes imports and formats the file with gofmt.",
"Configurable gofmt, see https://github.com/mbenkmann/goformat.",
"Stricter version of gofmt, see https://github.com/mvdan/gofumpt.",
"Applies gofumpt formatting and organizes imports."
]
},
"go.formatFlags": {
Expand Down
38 changes: 34 additions & 4 deletions src/goFormat.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,20 +27,23 @@ export class GoDocumentFormattingEditProvider implements vscode.DocumentFormatti

const filename = document.fileName;
const goConfig = getGoConfig(document.uri);
const formatTool = goConfig['formatTool'] || 'goreturns';
const formatFlags = goConfig['formatFlags'].slice() || [];

// We ignore the -w flag that updates file on disk because that would break undo feature
// Ignore -w because we don't want to write directly to disk.
if (formatFlags.indexOf('-w') > -1) {
formatFlags.splice(formatFlags.indexOf('-w'), 1);
}

// Fix for https://github.com/Microsoft/vscode-go/issues/613 and https://github.com/Microsoft/vscode-go/issues/630
const formatTool = getFormatTool(goConfig);

// Handle issues:
// https://github.com/Microsoft/vscode-go/issues/613
// https://github.com/Microsoft/vscode-go/issues/630
if (formatTool === 'goimports' || formatTool === 'goreturns' || formatTool === 'gofumports') {
formatFlags.push('-srcdir', filename);
}

// Since goformat supports the style flag, set tabsize if user has not passed any flags
// Since goformat supports the style flag, set tabsize if the user hasn't.
if (formatTool === 'goformat' && formatFlags.length === 0 && options.insertSpaces) {
formatFlags.push('-style=indent=' + options.tabSize);
}
Expand Down Expand Up @@ -111,3 +114,30 @@ 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;
}
}

export function getFormatTool(goConfig: { [key: string]: any }): string {
if (goConfig['formatTool'] === 'default') {
return 'goimports';
}
return goConfig['formatTool'];
}
27 changes: 23 additions & 4 deletions src/goLanguageServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import {
Message,
ProvideCodeLensesSignature,
ProvideCompletionItemsSignature,
ProvideDocumentFormattingEditsSignature,
ResponseError,
RevealOutputChannelOn
} from 'vscode-languageclient';
Expand All @@ -38,7 +39,7 @@ import { GoCodeActionProvider } from './goCodeAction';
import { GoDefinitionProvider } from './goDeclaration';
import { toolExecutionEnvironment } from './goEnv';
import { GoHoverProvider } from './goExtraInfo';
import { GoDocumentFormattingEditProvider } from './goFormat';
import { GoDocumentFormattingEditProvider, usingCustomFormatTool } from './goFormat';
import { GoImplementationProvider } from './goImplementations';
import { installTools, latestToolVersion, promptForMissingTool, promptForUpdatingTool } from './goInstallTools';
import { parseLiveFile } from './goLiveErrors';
Expand Down Expand Up @@ -83,6 +84,7 @@ export interface LanguageServerConfig {
env: any;
features: {
diagnostics: boolean;
formatter?: GoDocumentFormattingEditProvider;
};
checkForUpdates: string;
}
Expand All @@ -92,7 +94,7 @@ export interface LanguageServerConfig {
// new configurations.
export let languageClient: LanguageClient;
let languageServerDisposable: vscode.Disposable;
let latestConfig: LanguageServerConfig;
export let latestConfig: LanguageServerConfig;
export let serverOutputChannel: vscode.OutputChannel;
export let languageServerIsRunning = false;

Expand Down Expand Up @@ -557,6 +559,17 @@ export async function buildLanguageClient(cfg: BuildLanguageClientOption): Promi
}
}, []);
},
provideDocumentFormattingEdits: async (
document: vscode.TextDocument,
options: vscode.FormattingOptions,
token: vscode.CancellationToken,
next: ProvideDocumentFormattingEditsSignature
) => {
if (cfg.features.formatter) {
return cfg.features.formatter.provideDocumentFormattingEdits(document, options, token);
}
return next(document, options, token);
},
handleDiagnostics: (
uri: vscode.Uri,
diagnostics: vscode.Diagnostic[],
Expand Down Expand Up @@ -889,7 +902,8 @@ export async function watchLanguageServerConfiguration(e: vscode.ConfigurationCh
e.affectsConfiguration('go.languageServerFlags') ||
e.affectsConfiguration('go.languageServerExperimentalFeatures') ||
e.affectsConfiguration('go.alternateTools') ||
e.affectsConfiguration('go.toolsEnvVars')
e.affectsConfiguration('go.toolsEnvVars') ||
e.affectsConfiguration('go.formatTool')
// TODO: Should we check http.proxy too? That affects toolExecutionEnvironment too.
) {
restartLanguageServer();
Expand All @@ -901,6 +915,10 @@ export async function watchLanguageServerConfiguration(e: vscode.ConfigurationCh
}

export function buildLanguageServerConfig(goConfig: vscode.WorkspaceConfiguration): LanguageServerConfig {
let formatter: GoDocumentFormattingEditProvider;
if (usingCustomFormatTool(goConfig)) {
formatter = new GoDocumentFormattingEditProvider();
}
const cfg: LanguageServerConfig = {
serverName: '',
path: '',
Expand All @@ -911,7 +929,8 @@ export function buildLanguageServerConfig(goConfig: vscode.WorkspaceConfiguratio
features: {
// TODO: We should have configs that match these names.
// Ultimately, we should have a centralized language server config rather than separate fields.
diagnostics: goConfig['languageServerExperimentalFeatures']['diagnostics']
diagnostics: goConfig['languageServerExperimentalFeatures']['diagnostics'],
formatter: formatter
},
env: toolExecutionEnvironment(),
checkForUpdates: getCheckForToolsUpdatesConfig(goConfig)
Expand Down
3 changes: 2 additions & 1 deletion src/goMain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ import { clearCacheForTools, fileExists, getCurrentGoRoot, setCurrentGoRoot } fr
import { WelcomePanel } from './welcome';
import semver = require('semver');
import vscode = require('vscode');
import { getFormatTool } from './goFormat';

export let buildDiagnosticCollection: vscode.DiagnosticCollection;
export let lintDiagnosticCollection: vscode.DiagnosticCollection;
Expand Down Expand Up @@ -451,7 +452,7 @@ If you would like additional configuration for diagnostics from gopls, please se
}

if (e.affectsConfiguration('go.formatTool')) {
checkToolExists(updatedGoConfig['formatTool']);
checkToolExists(getFormatTool(updatedGoConfig));
}
if (e.affectsConfiguration('go.lintTool')) {
checkToolExists(updatedGoConfig['lintTool']);
Expand Down
3 changes: 2 additions & 1 deletion src/goModules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import path = require('path');
import vscode = require('vscode');
import { getGoConfig } from './config';
import { toolExecutionEnvironment } from './goEnv';
import { getFormatTool } from './goFormat';
import { installTools } from './goInstallTools';
import { getTool } from './goTools';
import { getFromGlobalState, updateGlobalState } from './stateUtils';
Expand Down Expand Up @@ -74,7 +75,7 @@ export async function getModFolderPath(fileuri: vscode.Uri, isDir?: boolean): Pr
);
}

if (goConfig['useLanguageServer'] === false && goConfig['formatTool'] === 'goreturns') {
if (goConfig['useLanguageServer'] === false && getFormatTool(goConfig) === 'goreturns') {
const promptFormatToolMsg =
'The goreturns tool does not support Go modules. Please update the "formatTool" setting to "goimports".';
promptToUpdateToolForModules('switchFormatToolToGoimports', promptFormatToolMsg, goConfig);
Expand Down
8 changes: 6 additions & 2 deletions src/goTools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import moment = require('moment');
import path = require('path');
import semver = require('semver');
import util = require('util');
import { getFormatTool, usingCustomFormatTool } from './goFormat';
import { goLiveErrorsEnabled } from './goLiveErrors';
import { getBinPath, GoVersion } from './util';

Expand Down Expand Up @@ -165,8 +166,11 @@ export function getConfiguredTools(goVersion: GoVersion, goConfig: { [key: strin
break;
}

// Add the format tool that was chosen by the user.
maybeAddTool(goConfig['formatTool']);
// Only add format tools if the language server is disabled and the
// format tool is known to us.
if (goConfig['useLanguageServer'] === false && !usingCustomFormatTool(goConfig)) {
maybeAddTool(getFormatTool(goConfig));
}

// Add the linter that was chosen by the user.
maybeAddTool(goConfig['lintTool']);
Expand Down
15 changes: 14 additions & 1 deletion test/gopls/extension.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,8 @@ class Env {
// Start the language server with the fakeOutputChannel.
const goConfig = Object.create(getGoConfig(), {
useLanguageServer: { value: true },
languageServerFlags: { value: ['-rpc.trace'] } // enable rpc tracing to monitor progress reports
languageServerFlags: { value: ['-rpc.trace'] }, // enable rpc tracing to monitor progress reports
formatTool: { value: 'nonexistent' } // to test custom formatters
});
const cfg: BuildLanguageClientOption = buildLanguageServerConfig(goConfig);
cfg.outputChannel = this.fakeOutputChannel; // inject our fake output channel.
Expand Down Expand Up @@ -245,4 +246,16 @@ suite('Go Extension Tests With Gopls', function () {
}
}
});

test('Nonexistent formatter', async () => {
const { uri } = await env.openDoc(testdataDir, 'gogetdocTestData', 'format.go');
const result = (await vscode.commands.executeCommand(
'vscode.executeFormatDocumentProvider',
uri,
{} // empty options
)) as vscode.TextEdit[];
if (result) {
assert.fail(`expected no result, got one: ${result}`);
}
});
});
7 changes: 7 additions & 0 deletions test/testdata/gogetdocTestData/format.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package abc

func hello() {

var x int
_ = x
}

0 comments on commit 8f2bcc1

Please sign in to comment.