extension/src/language: remove interface wrapping LanguageServerConfig

buildLanguageClientOption is confusing because it's wrapping over
LanguageServerConfig with additional channel information.

Explicitly annotate the type of serverOptions with additional
documentation.

Remove no effect flag from gopls "-mode=stdio".

For #3697

Change-Id: I7a04ce0ff2e76bb960bfda714c83c5b6823854cc
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/654435
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Madeline Kalil <mkalil@google.com>
Reviewed-by: Peter Weinberger <pjw@google.com>
kokoro-CI: kokoro <noreply+kokoro@google.com>

Author: h9jiang

extension/src/commands/startLanguageServer.ts

@@ -11,7 +11,6 @@ import { GoExtensionContext } from '../context';
 import { outputChannel, updateLanguageServerIconGoStatusBar } from '../goStatus';
 import {
 	buildLanguageClient,
-	buildLanguageClientOption,
 	buildLanguageServerConfig,
 	errorKind,
 	RestartReason,
@@ -85,7 +84,7 @@ export const startLanguageServer: CommandFactory = (ctx, goCtx) => {
 				return;
 			}
 
-			goCtx.languageClient = await buildLanguageClient(goCtx, buildLanguageClientOption(goCtx, cfg));
+			goCtx.languageClient = await buildLanguageClient(goCtx, cfg);
 			await goCtx.languageClient.start();
 			goCtx.serverInfo = toServerInfo(goCtx.languageClient.initializeResult);
 			goCtx.telemetryService = new TelemetryService(

extension/src/language/goLanguageServer.ts

@@ -36,7 +36,7 @@ import {
 	ResponseError,
 	RevealOutputChannelOn
 } from 'vscode-languageclient';
-import { LanguageClient, ServerOptions } from 'vscode-languageclient/node';
+import { Executable, LanguageClient, ServerOptions } from 'vscode-languageclient/node';
 import { getGoConfig, getGoplsConfig, extensionInfo } from '../config';
 import { toolExecutionEnvironment } from '../goEnv';
 import { GoDocumentFormattingEditProvider, usingCustomFormatTool } from './legacy/goFormat';
@@ -336,36 +336,6 @@ export function toServerInfo(res?: InitializeResult): ServerInfo | undefined {
 	return info;
 }
 
-export interface BuildLanguageClientOption extends LanguageServerConfig {
-	outputChannel?: vscode.OutputChannel;
-	traceOutputChannel?: vscode.OutputChannel;
-}
-
-// buildLanguageClientOption returns the default, extra configuration
-// used in building a new LanguageClient instance. Options specified
-// in LanguageServerConfig
-export function buildLanguageClientOption(
-	goCtx: GoExtensionContext,
-	cfg: LanguageServerConfig
-): BuildLanguageClientOption {
-	// Reuse the same output channel for each instance of the server.
-	if (cfg.enabled) {
-		if (!goCtx.serverOutputChannel) {
-			goCtx.serverOutputChannel = vscode.window.createOutputChannel(cfg.serverName + ' (server)');
-		}
-		if (!goCtx.serverTraceChannel) {
-			goCtx.serverTraceChannel = vscode.window.createOutputChannel(cfg.serverName);
-		}
-	}
-	return Object.assign(
-		{
-			outputChannel: goCtx.serverOutputChannel,
-			traceOutputChannel: goCtx.serverTraceChannel
-		},
-		cfg
-	);
-}
-
 export class GoLanguageClient extends LanguageClient implements vscode.Disposable {
 	constructor(
 		id: string,
@@ -409,8 +379,18 @@ type VulncheckEvent = {
 // The returned language client need to be started before use.
 export async function buildLanguageClient(
 	goCtx: GoExtensionContext,
-	cfg: BuildLanguageClientOption
+	cfg: LanguageServerConfig
 ): Promise<GoLanguageClient> {
+	// Reuse the same output channel for each instance of the server.
+	if (cfg.enabled) {
+		if (!goCtx.serverOutputChannel) {
+			goCtx.serverOutputChannel = vscode.window.createOutputChannel(cfg.serverName + ' (server)');
+		}
+		if (!goCtx.serverTraceChannel) {
+			goCtx.serverTraceChannel = vscode.window.createOutputChannel(cfg.serverName);
+		}
+	}
+
 	await getLocalGoplsVersion(cfg); // populate and cache cfg.version
 	const goplsWorkspaceConfig = await adjustGoplsWorkspaceConfiguration(cfg, getGoplsConfig(), 'gopls', undefined);
 
@@ -424,15 +404,20 @@ export async function buildLanguageClient(
 	const pendingVulncheckProgressToken = new Map<ProgressToken, any>();
 	const onDidChangeVulncheckResultEmitter = new vscode.EventEmitter<VulncheckEvent>();
 
+	// VSCode-Go prepares the information needed to start the language server.
+	// vscode-languageclient-node.LanguageClient will spin up the language
+	// server based on the provided information below.
+	const serverOption: Executable = {
+		command: cfg.path,
+		args: cfg.flags,
+		options: { env: cfg.env }
+	};
+
 	// cfg is captured by closures for later use during error report.
 	const c = new GoLanguageClient(
 		'go', // id
 		cfg.serverName, // name e.g. gopls
-		{
-			command: cfg.path,
-			args: ['-mode=stdio', ...cfg.flags],
-			options: { env: cfg.env }
-		} as ServerOptions,
+		serverOption as ServerOptions,
 		{
 			initializationOptions: goplsWorkspaceConfig,
 			documentSelector: GoDocumentSelector,
@@ -442,8 +427,8 @@ export async function buildLanguageClient(
 					(uri.scheme ? uri : uri.with({ scheme: 'file' })).toString(),
 				protocol2Code: (uri: string) => vscode.Uri.parse(uri)
 			},
-			outputChannel: cfg.outputChannel,
-			traceOutputChannel: cfg.traceOutputChannel,
+			outputChannel: goCtx.serverOutputChannel,
+			traceOutputChannel: goCtx.serverTraceChannel,
 			revealOutputChannelOn: RevealOutputChannelOn.Never,
 			initializationFailedHandler: (error: ResponseError<InitializeError>): boolean => {
 				initializationError = error;

extension/test/gopls/goplsTestEnv.utils.ts

@@ -12,7 +12,7 @@ import { LanguageClient } from 'vscode-languageclient/node';
 import { getGoConfig } from '../../src/config';
 import {
 	buildLanguageClient,
-	BuildLanguageClientOption,
+	LanguageServerConfig,
 	buildLanguageServerConfig,
 	toServerInfo
 } from '../../src/language/goLanguageServer';
@@ -108,14 +108,16 @@ export class Env {
 		if (!goConfig) {
 			goConfig = getGoConfig();
 		}
-		const cfg: BuildLanguageClientOption = await buildLanguageServerConfig(
+		const cfg: LanguageServerConfig = await buildLanguageServerConfig(
 			Object.create(goConfig, {
 				useLanguageServer: { value: true },
 				languageServerFlags: { value: ['-rpc.trace'] } // enable rpc tracing to monitor progress reports
 			})
 		);
-		cfg.outputChannel = this.fakeOutputChannel; // inject our fake output channel.
 		this.goCtx.latestConfig = cfg;
+		// Inject fake output channel.
+		this.goCtx.serverOutputChannel = this.fakeOutputChannel;
+		this.goCtx.serverTraceChannel = this.fakeOutputChannel;
 		this.languageClient = await buildLanguageClient(this.goCtx, cfg);
 		if (!this.languageClient) {
 			throw new Error('Language client not initialized.');