src/goOutline: use language server to get document symbols

This change replaces the use of go-outline with gopls when
available. This is done by fixing up the results from gopls
to resemble those returned by go-outline to avoid having to
handle these document symbols differently in various places.

Also sets the testing gopls version to pre release.

Updates #1020

Change-Id: Ib3ce4fe04c3ffe9428f6701ffaa24dd8444781c0
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/384574
Trust: Suzy Mueller <[email protected]>
Run-TryBot: Suzy Mueller <[email protected]>
Trust: Hyang-Ah Hana Kim <[email protected]>
Reviewed-by: Hyang-Ah Hana Kim <[email protected]>
TryBot-Result: kokoro <[email protected]>

Author: suzmue

docs/settings.md

@@ -730,7 +730,7 @@ Example Usage:
 | `nilfunc` | check for useless comparisons between functions and nil <br/> A useless comparison is one like f == nil as opposed to f() == nil. <br/> Default: `true` |
 | `nilness` | check for redundant or impossible nil comparisons <br/> The nilness checker inspects the control-flow graph of each function in a package and reports nil pointer dereferences, degenerate nil pointers, and panics with nil values. A degenerate comparison is of the form x==nil or x!=nil where x is statically known to be nil or non-nil. These are often a mistake, especially in control flow related to errors. Panics with nil values are checked because they are not detectable by <br/> <pre>if r := recover(); r != nil {</pre><br/> This check reports conditions such as: <br/> <pre>if f == nil { // impossible condition (f is a function)<br/>}</pre><br/> and: <br/> <pre>p := &v<br/>...<br/>if p != nil { // tautological condition<br/>}</pre><br/> and: <br/> <pre>if p == nil {<br/>	print(*p) // nil dereference<br/>}</pre><br/> and: <br/> <pre>if p == nil {<br/>	panic(p)<br/>}</pre><br/> <br/> Default: `false` |
 | `nonewvars` | suggested fixes for "no new vars on left side of :=" <br/> This checker provides suggested fixes for type errors of the type "no new vars on left side of :=". For example: <pre>z := 1<br/>z := 2</pre>will turn into <pre>z := 1<br/>z = 2</pre><br/> <br/> Default: `true` |
-| `noresultvalues` | suggested fixes for "no result values expected" <br/> This checker provides suggested fixes for type errors of the type "no result values expected". For example: <pre>func z() { return nil }</pre>will turn into <pre>func z() { return }</pre><br/> <br/> Default: `true` |
+| `noresultvalues` | suggested fixes for unexpected return values <br/> This checker provides suggested fixes for type errors of the type "no result values expected" or "too many return values". For example: <pre>func z() { return nil }</pre>will turn into <pre>func z() { return }</pre><br/> <br/> Default: `true` |
 | `printf` | check consistency of Printf format strings and arguments <br/> The check applies to known functions (for example, those in package fmt) as well as any detected wrappers of known functions. <br/> A function that wants to avail itself of printf checking but is not found by this analyzer's heuristics (for example, due to use of dynamic calls) can insert a bogus call: <br/> <pre>if false {<br/>	_ = fmt.Sprintf(format, args...) // enable printf checking<br/>}</pre><br/> The -funcs flag specifies a comma-separated list of names of additional known formatting functions or methods. If the name contains a period, it must denote a specific function using one of the following forms: <br/> <pre>dir/pkg.Function<br/>dir/pkg.Type.Method<br/>(*dir/pkg.Type).Method</pre><br/> Otherwise the name is interpreted as a case-insensitive unqualified identifier such as "errorf". Either way, if a listed name ends in f, the function is assumed to be Printf-like, taking a format string before the argument list. Otherwise it is assumed to be Print-like, taking a list of arguments with no format string. <br/> <br/> Default: `true` |
 | `shadow` | check for possible unintended shadowing of variables <br/> This analyzer check for shadowed variables. A shadowed variable is a variable declared in an inner scope with the same name and type as a variable in an outer scope, and where the outer variable is mentioned after the inner one is declared. <br/> (This definition can be refined; the module generates too many false positives and is not yet enabled by default.) <br/> For example: <br/> <pre>func BadRead(f *os.File, buf []byte) error {<br/>	var err error<br/>	for {<br/>		n, err := f.Read(buf) // shadows the function variable 'err'<br/>		if err != nil {<br/>			break // causes return of wrong value<br/>		}<br/>		foo(buf)<br/>	}<br/>	return err<br/>}</pre><br/> <br/> Default: `false` |
 | `shift` | check for shifts that equal or exceed the width of the integer <br/> Default: `true` |
@@ -840,7 +840,7 @@ Default: `"Both"`
 <br/>
 Allowed Options: `CaseInsensitive`, `CaseSensitive`, `FastFuzzy`, `Fuzzy`
 
-Default: `"Fuzzy"`
+Default: `"FastFuzzy"`
 ### `ui.navigation.symbolStyle`
 
 (Advanced) symbolStyle controls how symbols are qualified in symbol responses.

package.json

@@ -2272,7 +2272,7 @@
                 },
                 "noresultvalues": {
                   "type": "boolean",
-                  "markdownDescription": "suggested fixes for \"no result values expected\"\n\nThis checker provides suggested fixes for type errors of the\ntype \"no result values expected\". For example:\n\tfunc z() { return nil }\nwill turn into\n\tfunc z() { return }\n",
+                  "markdownDescription": "suggested fixes for unexpected return values\n\nThis checker provides suggested fixes for type errors of the\ntype \"no result values expected\" or \"too many return values\".\nFor example:\n\tfunc z() { return nil }\nwill turn into\n\tfunc z() { return }\n",
                   "default": true
                 },
                 "printf": {
@@ -2485,7 +2485,7 @@
                 "",
                 ""
               ],
-              "default": "Fuzzy",
+              "default": "FastFuzzy",
               "scope": "resource"
             },
             "ui.navigation.symbolStyle": {

src/goOutline.ts

@@ -7,10 +7,13 @@
 'use strict';
 
 import cp = require('child_process');
+import semver = require('semver');
 import vscode = require('vscode');
+import { ExecuteCommandParams, ExecuteCommandRequest } from 'vscode-languageserver-protocol';
 import { getGoConfig } from './config';
 import { toolExecutionEnvironment } from './goEnv';
 import { promptForMissingTool, promptForUpdatingTool } from './goInstallTools';
+import { getLocalGoplsVersion, languageClient, latestConfig } from './goLanguageServer';
 import { getBinPath, getFileArchive, makeMemoizedByteOffsetConverter } from './util';
 import { killProcess } from './utils/processUtils';
 
@@ -192,14 +195,63 @@ function convertToCodeSymbols(
 export class GoDocumentSymbolProvider implements vscode.DocumentSymbolProvider {
 	constructor(private includeImports?: boolean) {}
 
-	public provideDocumentSymbols(
+	public async provideDocumentSymbols(
 		document: vscode.TextDocument,
 		token: vscode.CancellationToken
-	): Thenable<vscode.DocumentSymbol[]> {
+	): Promise<vscode.DocumentSymbol[]> {
 		if (typeof this.includeImports !== 'boolean') {
 			const gotoSymbolConfig = getGoConfig(document.uri)['gotoSymbol'];
 			this.includeImports = gotoSymbolConfig ? gotoSymbolConfig['includeImports'] : false;
 		}
+
+		// TODO(suzmue): Check the commands available instead of the version.
+		const goplsVersion = await getLocalGoplsVersion(latestConfig);
+		const sv = semver.parse(goplsVersion, true);
+		if (languageClient && (goplsVersion === '(devel)' || semver.gt(sv, 'v0.7.6'))) {
+			const symbols: vscode.DocumentSymbol[] = await vscode.commands.executeCommand(
+				'vscode.executeDocumentSymbolProvider',
+				document.uri
+			);
+			if (!symbols || symbols.length === 0) {
+				return [];
+			}
+
+			// Stitch the results together to make the results look like
+			// go-outline.
+			// TODO(suzmue): use regexp to find the package declaration.
+			const packageSymbol = new vscode.DocumentSymbol(
+				'unknown',
+				'package',
+				vscode.SymbolKind.Package,
+				new vscode.Range(new vscode.Position(0, 0), new vscode.Position(0, 0)),
+				new vscode.Range(new vscode.Position(0, 0), new vscode.Position(0, 0))
+			);
+			packageSymbol.children = symbols;
+			if (this.includeImports) {
+				try {
+					const imports = await listImports(document);
+					imports?.forEach((value) => {
+						packageSymbol.children.unshift(
+							new vscode.DocumentSymbol(
+								value.Path,
+								'import',
+								vscode.SymbolKind.Namespace,
+								new vscode.Range(new vscode.Position(0, 0), new vscode.Position(0, 0)),
+								new vscode.Range(new vscode.Position(0, 0), new vscode.Position(0, 0))
+							)
+						);
+					});
+				} catch (e) {
+					// Fall back to use go-outline.
+					return this.runGoOutline(document, token);
+				}
+			}
+			return [packageSymbol];
+		}
+		return this.runGoOutline(document, token);
+	}
+
+	private runGoOutline(document: vscode.TextDocument, token: vscode.CancellationToken) {
 		const options: GoOutlineOptions = {
 			fileName: document.fileName,
 			document,
@@ -208,3 +260,17 @@ export class GoDocumentSymbolProvider implements vscode.DocumentSymbolProvider {
 		return documentSymbols(options, token);
 	}
 }
+
+async function listImports(document: vscode.TextDocument): Promise<{ Path: string; Name: string }[]> {
+	const uri = languageClient.code2ProtocolConverter.asTextDocumentIdentifier(document).uri;
+	const params: ExecuteCommandParams = {
+		command: 'gopls.list_imports',
+		arguments: [
+			{
+				URI: uri
+			}
+		]
+	};
+	const resp = await languageClient.sendRequest(ExecuteCommandRequest.type, params);
+	return resp.Imports;
+}

src/goTest/resolve.ts

@@ -24,6 +24,7 @@ import { getCurrentGoPath } from '../util';
 import { getGoConfig } from '../config';
 import { dispose, disposeIfEmpty, FileSystem, GoTest, GoTestKind, isInTest, Workspace } from './utils';
 import { walk, WalkStop } from './walk';
+import { importsTestify } from '../testUtils';
 
 export type ProvideSymbols = (doc: TextDocument, token: CancellationToken) => Thenable<DocumentSymbol[]>;
 
@@ -202,11 +203,7 @@ export class GoTestResolver {
 		const seen = new Set<string>();
 		const item = await this.getFile(doc.uri);
 		const symbols = await this.provideDocumentSymbols(doc, null);
-		const testify = symbols.some((s) =>
-			s.children.some(
-				(sym) => sym.kind === SymbolKind.Namespace && sym.name === '"github.com/stretchr/testify/suite"'
-			)
-		);
+		const testify = importsTestify(symbols);
 		for (const symbol of symbols) {
 			await this.processSymbol(doc, item, seen, testify, symbol);
 		}
@@ -410,7 +407,7 @@ export class GoTestResolver {
 		}
 
 		// Recursively process symbols that are nested
-		if (symbol.kind !== SymbolKind.Function) {
+		if (symbol.kind !== SymbolKind.Function && symbol.kind !== SymbolKind.Method) {
 			for (const sym of symbol.children) await this.processSymbol(doc, file, seen, importsTestify, sym);
 			return;
 		}

src/testUtils.ts

@@ -154,12 +154,10 @@ export async function getTestFunctions(
 		return;
 	}
 	const children = symbol.children;
-	const testify = children.some(
-		(sym) => sym.kind === vscode.SymbolKind.Namespace && sym.name === '"github.com/stretchr/testify/suite"'
-	);
+	const testify = importsTestify(symbols);
 	return children.filter(
 		(sym) =>
-			sym.kind === vscode.SymbolKind.Function &&
+			(sym.kind === vscode.SymbolKind.Function || sym.kind === vscode.SymbolKind.Method) &&
 			(testFuncRegex.test(sym.name) || fuzzFuncRegx.test(sym.name) || (testify && testMethodRegex.test(sym.name)))
 	);
 }
@@ -632,3 +630,15 @@ function removeRunFlag(flags: string[]): void {
 		flags.splice(index, 2);
 	}
 }
+
+export function importsTestify(syms: vscode.DocumentSymbol[]): boolean {
+	if (!syms || syms.length === 0 || !syms[0]) {
+		return false;
+	}
+	const children = syms[0].children;
+	return children.some(
+		(sym) =>
+			sym.kind === vscode.SymbolKind.Namespace &&
+			(sym.name === '"github.com/stretchr/testify/suite"' || sym.name === 'github.com/stretchr/testify/suite')
+	);
+}

tools/installtools/main.go

@@ -17,7 +17,7 @@ var tools = []struct {
 	dest    string
 }{
 	// TODO: auto-generate based on allTools.ts.in.
-	{"golang.org/x/tools/gopls", "", ""},
+	{"golang.org/x/tools/gopls", "v0.8.0-pre.1", ""}, // TODO: make this not hardcoded
 	{"github.com/acroca/go-symbols", "", ""},
 	{"github.com/cweill/gotests/gotests", "", ""},
 	{"github.com/davidrjenni/reftools/cmd/fillstruct", "", ""},