[release] src/goLanguageServer: partial revert of cl/280601

This partially reverts commit 4c91c38.

In https://go-review.googlesource.com/c/vscode-go/+/280601,
we tried to remove the languageServerExperimentalFeatures setting
because gopls's diagnostics feature is no longer in its experimental
state and that was the only flag left in this setting.

However, we learned some users depend on this flag because the
extension turns off buildOnSave and vetOnSave features when gopls's
diagnostics is used and they need to run custom vet analyzers.
This is not ideal and the extension shouldn't prevent users from
running their custom analyzers. That needs more investigation and
experiment.

For now, we rollback the change, but place the deprecation notice.

Update #50
Fixes #1110

Change-Id: I376692b152d3011aaa8da7a1b5121ba33e2188b6
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/285253
Trust: Hyang-Ah Hana Kim <[email protected]>
Run-TryBot: Hyang-Ah Hana Kim <[email protected]>
TryBot-Result: kokoro <[email protected]>
Reviewed-by: Rebecca Stambler <[email protected]>
(cherry picked from commit fbd2fc4)
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/285256

Author: hyangah

docs/settings.md

@@ -76,7 +76,7 @@ Flags to `go build`/`go test` used during build-on-save or running tests. (e.g.
 
 ### `go.buildOnSave`
 
-Compiles code on file save using 'go build -i' or 'go test -c -i'. Options are 'workspace', 'package', or 'off'.
+Compiles code on file save using 'go build' or 'go test -c'. Options are 'workspace', 'package', or 'off'.  Not applicable when using the language server's diagnostics is used. See 'go.languageServerExperimentalFeatures.diagnostics' setting.
 
 Allowed Values:`[package workspace off]`
 
@@ -324,6 +324,21 @@ If true, then `-i` flag will be passed to `go build` everytime the code is compi
 
 Default: `false`
 
+### `go.languageServerExperimentalFeatures`
+
+Temporary flag to enable/disable diagnostics from the language server. This setting will be deprecated soon. Please see and response to [Issue 50](https://github.com/golang/vscode-go/issues/50).
+
+| Properties | Description |
+| --- | --- |
+| `diagnostics` | If true, the language server will provide build, vet errors and the extension will ignore the `buildOnSave`, `vetOnSave` settings. |
+| | |
+
+
+Default:{<br/>
+&nbsp;&nbsp;`"diagnostics": true`,<br/>
+    }
+
+
 ### `go.languageServerFlags`
 
 Flags like -rpc.trace and -logfile to be used while running the language server.
@@ -515,7 +530,7 @@ Flags to pass to `go tool vet` (e.g. ["-all", "-shadow"])
 
 ### `go.vetOnSave`
 
-Vets code on file save using 'go tool vet'. Not applicable when using the language server.
+Vets code on file save using 'go tool vet'. Not applicable when using the language server's diagnostics is used. See 'go.languageServerExperimentalFeatures.diagnostics' setting.
 
 Allowed Values:`[package workspace off]`
 

package.json

@@ -865,7 +865,7 @@
             "off"
           ],
           "default": "package",
-          "description": "Compiles code on file save using 'go build -i' or 'go test -c -i'. Options are 'workspace', 'package', or 'off'.",
+          "description": "Compiles code on file save using 'go build' or 'go test -c'. Options are 'workspace', 'package', or 'off'.  Not applicable when using the language server's diagnostics is used. See 'go.languageServerExperimentalFeatures.diagnostics' setting.",
           "scope": "resource"
         },
         "go.buildFlags": {
@@ -939,7 +939,7 @@
             "off"
           ],
           "default": "package",
-          "description": "Vets code on file save using 'go tool vet'. Not applicable when using the language server.",
+          "description": "Vets code on file save using 'go tool vet'. Not applicable when using the language server's diagnostics is used. See 'go.languageServerExperimentalFeatures.diagnostics' setting.",
           "scope": "resource"
         },
         "go.vetFlags": {
@@ -1253,6 +1253,21 @@
           "default": [],
           "description": "Flags like -rpc.trace and -logfile to be used while running the language server."
         },
+        "go.languageServerExperimentalFeatures": {
+          "type": "object",
+          "properties": {
+            "diagnostics": {
+              "type": "boolean",
+              "default": true,
+              "description": "If true, the language server will provide build, vet errors and the extension will ignore the `buildOnSave`, `vetOnSave` settings."
+            }
+          },
+          "additionalProperties": false,
+          "default": {
+            "diagnostics": true
+          },
+          "markdownDescription": "Temporary flag to enable/disable diagnostics from the language server. This setting will be deprecated soon. Please see and response to [Issue 50](https://github.com/golang/vscode-go/issues/50)."
+        },
         "go.trace.server": {
           "type": "string",
           "enum": [

src/goCheck.ts

@@ -60,7 +60,7 @@ export function check(fileUri: vscode.Uri, goConfig: vscode.WorkspaceConfigurati
 	// If a user has enabled diagnostics via a language server,
 	// then we disable running build or vet to avoid duplicate errors and warnings.
 	const lspConfig = buildLanguageServerConfig(goConfig);
-	const disableBuildAndVet = lspConfig.enabled;
+	const disableBuildAndVet = lspConfig.enabled && lspConfig.features.diagnostics;
 
 	let testPromise: Thenable<boolean>;
 	const testConfig: TestConfig = {

src/goLanguageServer.ts

@@ -27,6 +27,7 @@ import {
 	Message,
 	ProvideCodeLensesSignature,
 	ProvideCompletionItemsSignature,
+	ProvideDocumentLinksSignature,
 	ResponseError,
 	RevealOutputChannelOn
 } from 'vscode-languageclient';
@@ -74,6 +75,9 @@ export interface LanguageServerConfig {
 	enabled: boolean;
 	flags: string[];
 	env: any;
+	features: {
+		diagnostics: boolean;
+	};
 	checkForUpdates: string;
 }
 
@@ -105,6 +109,7 @@ let lastUserAction: Date = new Date();
 // startLanguageServerWithFallback starts the language server, if enabled,
 // or falls back to the default language providers.
 export async function startLanguageServerWithFallback(ctx: vscode.ExtensionContext, activation: boolean) {
+
 	for (const folder of vscode.workspace.workspaceFolders || []) {
 		if (folder.uri.scheme === 'vsls') {
 			outputChannel.appendLine(`Language service on the guest side is disabled. ` +
@@ -400,10 +405,14 @@ export async function buildLanguageClient(cfg: BuildLanguageClientOption): Promi
 					diagnostics: vscode.Diagnostic[],
 					next: HandleDiagnosticsSignature
 				) => {
+					if (!cfg.features.diagnostics) {
+						return null;
+					}
 					// Deduplicate diagnostics with those found by the other tools.
 					removeDuplicateDiagnostics(vetDiagnosticCollection, uri, diagnostics);
 					removeDuplicateDiagnostics(buildDiagnosticCollection, uri, diagnostics);
 					removeDuplicateDiagnostics(lintDiagnosticCollection, uri, diagnostics);
+
 					return next(uri, diagnostics);
 				},
 				provideCompletionItem: async (
@@ -716,6 +725,11 @@ export function buildLanguageServerConfig(goConfig: vscode.WorkspaceConfiguratio
 		modtime: null,
 		enabled: goConfig['useLanguageServer'] === true,
 		flags: goConfig['languageServerFlags'] || [],
+		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'],
+		},
 		env: toolExecutionEnvironment(),
 		checkForUpdates: getCheckForToolsUpdatesConfig(goConfig),
 	};

src/goMain.ts

@@ -126,19 +126,27 @@ export async function activate(ctx: vscode.ExtensionContext) {
 	// Present a warning about the deprecation of the go.documentLink setting.
 	const experimentalFeatures = getGoConfig()['languageServerExperimentalFeatures'];
 	if (experimentalFeatures) {
-		// TODO(rstambler): Eventually notify about deprecation of all of the settings.
+		// TODO(golang/vscode-go#50): Eventually notify about deprecation of
+		// all of the settings. See golang/vscode-go#1109 too.
+		// The `diagnostics` setting is still used as a workaround for running custom vet.
 		if (experimentalFeatures['documentLink'] === false) {
 			vscode.window.showErrorMessage(`The 'go.languageServerExperimentalFeature.documentLink' setting is now deprecated.
-	Please use 'gopls.importShortcut' instead.
-	See https://github.com/golang/tools/blob/master/gopls/doc/settings.md#importshortcut-enum for more details.`);
+Please use 'gopls.importShortcut' instead.
+See https://github.com/golang/tools/blob/master/gopls/doc/settings.md#importshortcut-enum for more details.`);
 		}
-		if (experimentalFeatures['diagnostics'] === false) {
-			vscode.window.showErrorMessage(`The 'go.languageServerExperimentalFeature.diagnostics' setting is now deprecated.
+		const promptKey = 'promptedLanguageServerExperimentalFeatureDeprecation';
+		const prompted = getFromGlobalState(promptKey, false);
+		if (!prompted && experimentalFeatures['diagnostics'] === false) {
+			const msg = `The 'go.languageServerExperimentalFeature.diagnostics' setting will be deprecated soon.
 If you would like additional configuration for diagnostics from gopls, please see and response to
-https://github.com/golang/vscode-go/issues/50.`);
+https://github.com/golang/vscode-go/issues/50.`;
+			const selected = await vscode.window.showInformationMessage(msg, `Don't show again`);
+			switch (selected) {
+			case `Don't show again`:
+				updateGlobalState(promptKey, true);
+			}
 		}
 	}
-
 	updateGoVarsFromConfig().then(async () => {
 		suggestUpdates(ctx);
 		offerToInstallLatestGoVersion();

test/gopls/update.test.ts

@@ -175,6 +175,9 @@ suite('gopls update tests', () => {
 				version: '',
 				checkForUpdates: 'proxy',
 				env: {},
+				features: {
+					diagnostics: true,
+				},
 				flags: [],
 				modtime: new Date(),
 				serverName: 'gopls',