Show completion for newly created package. (#1191)

* Refactor getNonVendorPackages to getPackages and respect the 1.9 version behavior * Inline the function test arguments building * Test all packages for Go 1.9 use "./...", the lower still remain using the same way * Fix autocomplete un-imported packages: show newly added package * Use spawn to avoid maxBuffer exceeded * Support large output by using spawn * Completions shows result instantly, no longer need to pre-call to goListAll() * Fix lint warning * Expect proper gopkgs binary * Change the gopkgs package * Use spawn instead of execFile to support large output * Use gopkgs from http://github.com/uudashr/gopkgs * Update the go tools in .travis.yml * Fix the gopkgs missing tools detection * Cache the gopkgs tool invocation result * Refresh gopkgs cache * Drop the cache for gopkgs since it already fast enough * Adapt with the changes of gopkgs by using -format * trigger travis build: use array desctructuring assignment * Install libsecret-1-0 * Fix the travis script * Fix the travis script, install libsecret-1-0 * Use sudo apt-get install * apt-get should buy some time * Add go 1.9.x to the .travis.yml * Prompt for missing tools when "gopkgs" not found * Update the comment of goListAll * Revert back the function name "goPackages" to "goNonVendorPackages" * Rename "goListAll" to "getAllPackages" * Use existing getImportablePackages * Missing tool prompt will be handled by function caller * Remove unused import * Handle missing tool when browsing package * Use cache if still used for less than 5s * Ignore the stderr to avoid stuck * Fix lint warning * Prompt to update gopkgs if using old pkgs * Cleanup * Fix tests * Sort pkgs before testing
microsoft · Sep 12, 2017 · e5aa763 · e5aa763
1 parent d07eb35
commit e5aa763
Show file tree

Hide file tree

Showing 9 changed files with 118 additions and 155 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -5,6 +5,7 @@ go:
   - 1.6.x
   - 1.7.x
   - 1.8.x
+  - 1.9.x
   - tip
 
 sudo: false
@@ -17,7 +18,7 @@ before_install:
   - if [ $TRAVIS_OS_NAME == "linux" ]; then
       export CXX="g++-4.9" CC="gcc-4.9" DISPLAY=:99.0;
       sh -e /etc/init.d/xvfb start;
-      sleep 3;
+      sudo apt-get update && sudo apt-get install -y libsecret-1-0;
     fi
 
 install:
@@ -41,7 +42,7 @@ install:
   - go get -u -v github.com/ramya-rao-a/go-outline
   - go get -u -v sourcegraph.com/sqs/goreturns
   - go get -u -v golang.org/x/tools/cmd/gorename
-  - go get -u -v github.com/tpng/gopkgs
+  - go get -u -v github.com/uudashr/gopkgs/cmd/gopkgs
   - go get -u -v github.com/acroca/go-symbols
   - go get -u -v github.com/alecthomas/gometalinter
   - go get -u -v github.com/cweill/gotests/...

diff --git a/src/goBrowsePackage.ts b/src/goBrowsePackage.ts
@@ -9,7 +9,7 @@ import vscode = require('vscode');
 import cp = require('child_process');
 import { getGoRuntimePath } from './goPath';
 import path = require('path');
-import { goListAll, isGoListComplete } from './goPackages';
+import { getAllPackages } from './goPackages';
 
 export function browsePackages() {
 	let selectedText = '';
@@ -70,30 +70,22 @@ function showPackageFiles(pkg: string, showAllPkgsIfPkgNotFound: boolean)  {
 }
 
 function showPackageList() {
-	if (!isGoListComplete()) {
-		return showTryAgainLater();
-	}
-
-	goListAll().then(pkgMap => {
+	getAllPackages().then(pkgMap => {
 		const pkgs: string[] = Array.from(pkgMap.keys());
-		if (!pkgs || pkgs.length === 0) {
+		if (pkgs.length === 0) {
 			return vscode.window.showErrorMessage('Could not find packages. Ensure `go list all` runs successfully.');
 		}
 
 		vscode
 			.window
-			.showQuickPick(pkgs, { placeHolder: 'Select a package to browse' })
+			.showQuickPick(pkgs.sort(), { placeHolder: 'Select a package to browse' })
 			.then(pkgFromDropdown => {
 				if (!pkgFromDropdown) return;
 				showPackageFiles(pkgFromDropdown, false);
 			});
 	});
 }
 
-function showTryAgainLater() {
-	vscode.window.showInformationMessage('Finding packages... Try after sometime.');
-}
-
 function getImportPath(text: string): string {
 	// Catch cases like `import alias "importpath"` and `import "importpath"`
 	let singleLineImportMatches = text.match(/^\s*import\s+([a-z,A-Z,_,\.]\w*\s+)?\"([^\"]+)\"/);

diff --git a/src/goImport.ts b/src/goImport.ts
@@ -11,57 +11,19 @@ import { parseFilePrelude, isVendorSupported, getBinPath, getCurrentGoPath, getT
 import { documentSymbols } from './goOutline';
 import { promptForMissingTool } from './goInstallTools';
 import path = require('path');
-import { getRelativePackagePath } from './goPackages';
-import { getCurrentGoWorkspaceFromGOPATH } from './goPath';
+import { getImportablePackages } from './goPackages';
 
 const missingToolMsg = 'Missing tool: ';
 
 export function listPackages(excludeImportedPkgs: boolean = false): Thenable<string[]> {
 	let importsPromise = excludeImportedPkgs && vscode.window.activeTextEditor ? getImports(vscode.window.activeTextEditor.document) : Promise.resolve([]);
-	let vendorSupportPromise = isVendorSupported();
-	let goPkgsPromise = new Promise<string[]>((resolve, reject) => {
-		cp.execFile(getBinPath('gopkgs'), [], {env: getToolsEnvVars()}, (err, stdout, stderr) => {
-			if (err && (<any>err).code === 'ENOENT') {
-				return reject(missingToolMsg + 'gopkgs');
-			}
-			let lines = stdout.toString().split('\n');
-			if (lines[lines.length - 1] === '') {
-				// Drop the empty entry from the final '\n'
-				lines.pop();
-			}
-			return resolve(lines);
-		});
-	});
-
-	return vendorSupportPromise.then((vendorSupport: boolean) => {
-		return Promise.all<string[]>([goPkgsPromise, importsPromise]).then(values => {
-			let pkgs = values[0];
-			let importedPkgs = values[1];
-
-			if (!vendorSupport) {
-				if (importedPkgs.length > 0) {
-					pkgs = pkgs.filter(element => {
-						return importedPkgs.indexOf(element) === -1;
-					});
-				}
-				return pkgs.sort();
-			}
+	let pkgsPromise = getImportablePackages(vscode.window.activeTextEditor.document.fileName);
 
-			let currentFileDirPath = path.dirname(vscode.window.activeTextEditor.document.fileName);
-			let currentWorkspace = getCurrentGoWorkspaceFromGOPATH(getCurrentGoPath(), currentFileDirPath);
-			let pkgSet = new Set<string>();
-			pkgs.forEach(pkg => {
-				if (!pkg || importedPkgs.indexOf(pkg) > -1) {
-					return;
-				}
-				let relativePkgPath = getRelativePackagePath(currentFileDirPath, currentWorkspace, pkg);
-				if (relativePkgPath) {
-					pkgSet.add(relativePkgPath);
-				}
-			});
-
-			return Array.from(pkgSet).sort();
+	return Promise.all([pkgsPromise, importsPromise]).then(([pkgMap, importedPkgs]) => {
+		importedPkgs.forEach(pkg => {
+			pkgMap.delete(pkg);
 		});
+		return Array.from(pkgMap.keys()).sort();
 	});
 }
 
@@ -133,4 +95,4 @@ export function addImport(arg: string) {
 			});
 		}
 	});
-}
+}
diff --git a/src/goInstallTools.ts b/src/goInstallTools.ts
@@ -23,7 +23,7 @@ function getTools(goVersion: SemVersion): { [key: string]: string } {
 	let goConfig = vscode.workspace.getConfiguration('go');
 	let tools: { [key: string]: string } = {
 		'gocode': 'github.com/nsf/gocode',
-		'gopkgs': 'github.com/tpng/gopkgs',
+		'gopkgs': 'github.com/uudashr/gopkgs/cmd/gopkgs',
 		'go-outline': 'github.com/ramya-rao-a/go-outline',
 		'go-symbols': 'github.com/acroca/go-symbols',
 		'guru': 'golang.org/x/tools/cmd/guru',

diff --git a/src/goMain.ts b/src/goMain.ts
@@ -36,7 +36,6 @@ import { addTags, removeTags } from './goModifytags';
 import { parseLiveFile } from './goLiveErrors';
 import { GoCodeLensProvider } from './goCodelens';
 import { implCursor } from './goImpl';
-import { goListAll } from './goPackages';
 import { browsePackages } from './goBrowsePackage';
 
 export let errorDiagnosticCollection: vscode.DiagnosticCollection;
@@ -66,7 +65,6 @@ export function activate(ctx: vscode.ExtensionContext): void {
 				}
 			}
 		});
-		goListAll();
 		offerToInstallTools();
 		let langServerAvailable = checkLanguageServer();
 		if (langServerAvailable) {
@@ -421,4 +419,4 @@ function didLangServerConfigChange(useLangServer: boolean, langServerFlags: stri
 		}
 	}
 	return false;
-}
+}
diff --git a/src/goPackages.ts b/src/goPackages.ts
@@ -2,54 +2,60 @@ import vscode = require('vscode');
 import cp = require('child_process');
 import path = require('path');
 import { getGoRuntimePath, getCurrentGoWorkspaceFromGOPATH } from './goPath';
-import { isVendorSupported, getCurrentGoPath, getToolsEnvVars } from './util';
+import { isVendorSupported, getCurrentGoPath, getToolsEnvVars, getGoVersion, getBinPath, SemVersion } from './util';
+import { promptForMissingTool, promptForUpdatingTool } from './goInstallTools';
 
-let allPkgs = new Map<string, string>();
-let goListAllCompleted: boolean = false;
-let goListAllPromise: Promise<Map<string, string>>;
+let allPkgsCache: Map<string, string>;
+let allPkgsLastHit: number;
 
-export function isGoListComplete(): boolean {
-	return goListAllCompleted;
-}
-
-/**
- * Runs go list all
- * @returns Map<string, string> mapping between package import path and package name
- */
-export function goListAll(): Promise<Map<string, string>> {
-	let goRuntimePath = getGoRuntimePath();
-
-	if (!goRuntimePath) {
-		vscode.window.showInformationMessage('Cannot find "go" binary. Update PATH or GOROOT appropriately');
-		return Promise.resolve(null);
-	}
-
-	if (goListAllPromise) {
-		return goListAllPromise;
-	}
+function getAllPackagesNoCache(): Promise<Map<string, string>> {
+	return new Promise<Map<string, string>>((resolve, reject) => {
+		const cmd = cp.spawn(getBinPath('gopkgs'), ['-format', '{{.Name}};{{.ImportPath}}'], { env: getToolsEnvVars() });
+		const chunks = [];
+		const errchunks = [];
+		let err: any;
+		cmd.stdout.on('data', d => chunks.push(d));
+		cmd.stderr.on('data', d => errchunks.push(d));
+		cmd.on('error', e => err = e);
+		cmd.on('close', () => {
+			let pkgs = new Map<string, string>();
+			if (err && err.code === 'ENOENT') {
+				return promptForMissingTool('gopkgs');
+			}
 
-	goListAllPromise = new Promise<Map<string, string>>((resolve, reject) => {
-		// Use `{env: {}}` to make the execution faster. Include GOPATH to account if custom work space exists.
-		const env: any = getToolsEnvVars();
+			if (err || errchunks.length > 0) return resolve(pkgs);
 
-		const cmd = cp.spawn(goRuntimePath, ['list', '-f', '{{.Name}};{{.ImportPath}}', 'all'], { env: env, stdio: ['pipe', 'pipe', 'ignore'] });
-		const chunks = [];
-		cmd.stdout.on('data', (d) => {
-			chunks.push(d);
-		});
+			const output = chunks.join('');
+			if (output.indexOf(';') === -1) {
+				// User might be using the old gopkgs tool, prompt to update
+				return promptForUpdatingTool('gopkgs');
+			}
 
-		cmd.on('close', (status) => {
-			chunks.join('').split('\n').forEach(pkgDetail => {
+			output.split('\n').forEach((pkgDetail) => {
 				if (!pkgDetail || !pkgDetail.trim() || pkgDetail.indexOf(';') === -1) return;
 				let [pkgName, pkgPath] = pkgDetail.trim().split(';');
-				allPkgs.set(pkgPath, pkgName);
+				pkgs.set(pkgPath, pkgName);
 			});
-			goListAllCompleted = true;
-			return resolve(allPkgs);
+			return resolve(pkgs);
 		});
 	});
+}
+
+/**
+ * Runs gopkgs
+ * @returns Map<string, string> mapping between package import path and package name
+ */
+export function getAllPackages(): Promise<Map<string, string>> {
+	let useCache = allPkgsCache && allPkgsLastHit && (new Date().getTime() - allPkgsLastHit) < 5000;
+	if (useCache) {
+		allPkgsLastHit = new Date().getTime();
+		return Promise.resolve(allPkgsCache);
+	}
 
-	return goListAllPromise;
+	return getAllPackagesNoCache().then((pkgs) => {
+		allPkgsLastHit = new Date().getTime();
+		return allPkgsCache = pkgs;
+	});
 }
 
 /**
@@ -59,13 +65,14 @@ export function goListAll(): Promise<Map<string, string>> {
  */
 export function getImportablePackages(filePath: string): Promise<Map<string, string>> {
 
-	return Promise.all([isVendorSupported(), goListAll()]).then(values => {
+	return Promise.all([isVendorSupported(), getAllPackages()]).then(values => {
 		let isVendorSupported = values[0];
+		let pkgs = values[1];
 		let currentFileDirPath = path.dirname(filePath);
 		let currentWorkspace = getCurrentGoWorkspaceFromGOPATH(getCurrentGoPath(), currentFileDirPath);
 		let pkgMap = new Map<string, string>();
 
-		allPkgs.forEach((pkgName, pkgPath) => {
+		pkgs.forEach((pkgName, pkgPath) => {
 			if (pkgName === 'main') {
 				return;
 			}
@@ -87,7 +94,7 @@ export function getImportablePackages(filePath: string): Promise<Map<string, str
  * If given pkgPath is not vendor pkg, then the same pkgPath is returned
  * Else, the import path for the vendor pkg relative to given filePath is returned.
  */
-export function getRelativePackagePath(currentFileDirPath: string, currentWorkspace: string, pkgPath: string): string {
+function getRelativePackagePath(currentFileDirPath: string, currentWorkspace: string, pkgPath: string): string {
 	let magicVendorString = '/vendor/';
 	let vendorIndex = pkgPath.indexOf(magicVendorString);
 	if (vendorIndex === -1) {
@@ -113,7 +120,7 @@ export function getRelativePackagePath(currentFileDirPath: string, currentWorksp
 }
 
 /**
- * Returns import paths for all non vendor packages under given folder
+ * Returns import paths for all packages under given folder (vendor will be excluded)
  */
 export function getNonVendorPackages(folderPath: string): Promise<string[]> {
 	let goRuntimePath = getGoRuntimePath();
@@ -123,15 +130,21 @@ export function getNonVendorPackages(folderPath: string): Promise<string[]> {
 		return Promise.resolve(null);
 	}
 	return new Promise<string[]>((resolve, reject) => {
-		const childProcess = cp.spawn(goRuntimePath, ['list', './...'], { cwd: folderPath, env: getToolsEnvVars() });
-		const chunks = [];
+		let childProcess = cp.spawn(goRuntimePath, ['list', './...'], { cwd: folderPath, env: getToolsEnvVars() });
+		let chunks = [];
 		childProcess.stdout.on('data', (stdout) => {
 			chunks.push(stdout);
 		});
 
 		childProcess.on('close', (status) => {
-			const pkgs = chunks.join('').toString().split('\n').filter(pkgPath => pkgPath && !pkgPath.includes('/vendor/'));
-			return resolve(pkgs);
+			let pkgs = chunks.join('').toString().split('\n');
+			getGoVersion().then((ver: SemVersion) => {
+				if (ver && (ver.major > 1 || (ver.major === 1 && ver.minor >= 9))) {
+					resolve(pkgs);
+				} else {
+					resolve(pkgs.filter(pkgPath => pkgPath && !pkgPath.includes('/vendor/')));
+				}
+			});
 		});
 	});
 }

diff --git a/src/goSuggest.ts b/src/goSuggest.ts
@@ -37,7 +37,6 @@ interface GoCodeSuggestion {
 
 export class GoCompletionItemProvider implements vscode.CompletionItemProvider {
 
-	private gocodeConfigurationComplete = false;
 	private pkgsList = new Map<string, string>();
 
 	public provideCompletionItems(document: vscode.TextDocument, position: vscode.Position, token: vscode.CancellationToken): Thenable<vscode.CompletionItem[]> {
@@ -215,16 +214,12 @@ export class GoCompletionItemProvider implements vscode.CompletionItemProvider {
 	}
 	// TODO: Shouldn't lib-path also be set?
 	private ensureGoCodeConfigured(): Thenable<void> {
-		getImportablePackages(vscode.window.activeTextEditor.document.fileName).then(pkgMap => {
+		let setPkgsList = getImportablePackages(vscode.window.activeTextEditor.document.fileName).then(pkgMap => {
 			this.pkgsList = pkgMap;
+			return;
 		});
 
-		return new Promise<void>((resolve, reject) => {
-			// TODO: Since the gocode daemon is shared amongst clients, shouldn't settings be
-			// adjusted per-invocation to avoid conflicts from other gocode-using programs?
-			if (this.gocodeConfigurationComplete) {
-				return resolve();
-			}
+		let setGocodeProps = new Promise<void>((resolve, reject) => {
 			let gocode = getBinPath('gocode');
 			let autobuild = vscode.workspace.getConfiguration('go')['gocodeAutoBuild'];
 			let env = getToolsEnvVars();
@@ -235,6 +230,9 @@ export class GoCompletionItemProvider implements vscode.CompletionItemProvider {
 			});
 		});
 
+		return Promise.all([setPkgsList, setGocodeProps]).then(() => {
+			return;
+		});
 	}
 
 	// Return importable packages that match given word as Completion Items