From 264dba4625bc1b3259c6e051f0cd4dbbeb5c49db Mon Sep 17 00:00:00 2001 From: Andrew Eisenberg Date: Thu, 24 Sep 2020 15:59:06 -0700 Subject: [PATCH 1/4] Add the commandRunner The commandRunner wraps all vscode command registrations. It provides uniform error handling and an optional progress monitor. In general, progress monitors should only be created by the commandRunner and passed through to the locations that use it. --- extensions/ql-vscode/src/astViewer.ts | 12 +- .../src/contextual/locationFinder.ts | 80 +++-- .../src/contextual/templateProvider.ts | 57 +++- extensions/ql-vscode/src/databaseFetcher.ts | 154 ++++----- extensions/ql-vscode/src/databases-ui.ts | 299 ++++++++++-------- extensions/ql-vscode/src/extension.ts | 235 +++++++++----- extensions/ql-vscode/src/helpers.ts | 90 +++++- extensions/ql-vscode/src/interface.ts | 7 +- extensions/ql-vscode/src/query-history.ts | 70 ++-- extensions/ql-vscode/src/quick-query.ts | 15 +- extensions/ql-vscode/src/run-queries.ts | 82 ++--- extensions/ql-vscode/src/upgrades.ts | 11 +- .../ql-vscode/src/vscode-utils/ui-service.ts | 7 +- 13 files changed, 658 insertions(+), 461 deletions(-) diff --git a/extensions/ql-vscode/src/astViewer.ts b/extensions/ql-vscode/src/astViewer.ts index f1e4993f910..bab2987c86c 100644 --- a/extensions/ql-vscode/src/astViewer.ts +++ b/extensions/ql-vscode/src/astViewer.ts @@ -3,7 +3,6 @@ import { ExtensionContext, TreeDataProvider, EventEmitter, - commands, Event, ProviderResult, TreeItemCollapsibleState, @@ -19,6 +18,7 @@ import { DatabaseItem } from './databases'; import { UrlValue, BqrsId } from './bqrs-cli-types'; import { showLocation } from './interface-utils'; import { isStringLoc, isWholeFileLoc, isLineColumnLoc } from './bqrs-utils'; +import { commandRunner } from './helpers'; export interface AstItem { @@ -45,10 +45,10 @@ class AstViewerDataProvider implements TreeDataProvider { this._onDidChangeTreeData.event; constructor() { - commands.registerCommand('codeQLAstViewer.gotoCode', - async (item: AstItem) => { - await showLocation(item.fileLocation); - }); + commandRunner('codeQLAstViewer.gotoCode', + async (item: AstItem) => { + await showLocation(item.fileLocation); + }); } refresh(): void { @@ -109,7 +109,7 @@ export class AstViewer { showCollapseAll: true }); - commands.registerCommand('codeQLAstViewer.clear', () => { + commandRunner('codeQLAstViewer.clear', async () => { this.clear(); }); diff --git a/extensions/ql-vscode/src/contextual/locationFinder.ts b/extensions/ql-vscode/src/contextual/locationFinder.ts index 65ef6079d66..a6001818028 100644 --- a/extensions/ql-vscode/src/contextual/locationFinder.ts +++ b/extensions/ql-vscode/src/contextual/locationFinder.ts @@ -1,13 +1,14 @@ import * as vscode from 'vscode'; import { decodeSourceArchiveUri, zipArchiveScheme } from '../archive-filesystem-provider'; -import { ColumnKindCode, EntityValue, getResultSetSchema } from '../bqrs-cli-types'; +import { ColumnKindCode, EntityValue, getResultSetSchema, ResultSetSchema } from '../bqrs-cli-types'; import { CodeQLCliServer } from '../cli'; import { DatabaseManager, DatabaseItem } from '../databases'; import fileRangeFromURI from './fileRangeFromURI'; import * as messages from '../messages'; import { QueryServerClient } from '../queryserver-client'; import { QueryWithResults, compileAndRunQueryAgainstDatabase } from '../run-queries'; +import { ProgressCallback } from '../helpers'; import { KeyType } from './keyType'; import { qlpackOfDatabase, resolveQueries } from './queryResolver'; @@ -28,6 +29,8 @@ export interface FullLocationLink extends vscode.LocationLink { * @param dbm The database manager * @param uriString The selected source file and location * @param keyType The contextual query type to run + * @param progress A progress callback + * @param token A CancellationToken * @param filter A function that will filter extraneous results */ export async function getLocationsForUriString( @@ -36,37 +39,42 @@ export async function getLocationsForUriString( dbm: DatabaseManager, uriString: string, keyType: KeyType, + progress: ProgressCallback, + token: vscode.CancellationToken, filter: (src: string, dest: string) => boolean ): Promise { const uri = decodeSourceArchiveUri(vscode.Uri.parse(uriString)); const sourceArchiveUri = vscode.Uri.file(uri.sourceArchiveZipPath).with({ scheme: zipArchiveScheme }); const db = dbm.findDatabaseItemBySourceArchive(sourceArchiveUri); - if (db) { - const qlpack = await qlpackOfDatabase(cli, db); - if (qlpack === undefined) { - throw new Error('Can\'t infer qlpack from database source archive'); - } - const links: FullLocationLink[] = []; - for (const query of await resolveQueries(cli, qlpack, keyType)) { - const templates: messages.TemplateDefinitions = { - [TEMPLATE_NAME]: { - values: { - tuples: [[{ - stringValue: uri.pathWithinSourceArchive - }]] - } - } - }; - const results = await compileAndRunQueryAgainstDatabase(cli, qs, db, false, vscode.Uri.file(query), templates); - if (results.result.resultType == messages.QueryResultType.SUCCESS) { - links.push(...await getLinksFromResults(results, cli, db, filter)); - } - } - return links; - } else { + if (!db) { return []; } + + const qlpack = await qlpackOfDatabase(cli, db); + if (qlpack === undefined) { + throw new Error('Can\'t infer qlpack from database source archive'); + } + const templates = createTemplates(uri.pathWithinSourceArchive); + + const links: FullLocationLink[] = []; + for (const query of await resolveQueries(cli, qlpack, keyType)) { + const results = await compileAndRunQueryAgainstDatabase( + cli, + qs, + db, + false, + vscode.Uri.file(query), + progress, + token, + templates + ); + + if (results.result.resultType == messages.QueryResultType.SUCCESS) { + links.push(...await getLinksFromResults(results, cli, db, filter)); + } + } + return links; } async function getLinksFromResults( @@ -79,10 +87,7 @@ async function getLinksFromResults( const bqrsPath = results.query.resultsPaths.resultsPath; const info = await cli.bqrsInfo(bqrsPath); const selectInfo = getResultSetSchema(SELECT_QUERY_NAME, info); - if (selectInfo && selectInfo.columns.length == 3 - && selectInfo.columns[0].kind == ColumnKindCode.ENTITY - && selectInfo.columns[1].kind == ColumnKindCode.ENTITY - && selectInfo.columns[2].kind == ColumnKindCode.STRING) { + if (isValidSelect(selectInfo)) { // TODO: Page this const allTuples = await cli.bqrsDecode(bqrsPath, SELECT_QUERY_NAME); for (const tuple of allTuples.tuples) { @@ -101,3 +106,22 @@ async function getLinksFromResults( } return localLinks; } + +function createTemplates(path: string): messages.TemplateDefinitions { + return { + [TEMPLATE_NAME]: { + values: { + tuples: [[{ + stringValue: path + }]] + } + } + }; +} + +function isValidSelect(selectInfo: ResultSetSchema | undefined) { + return selectInfo && selectInfo.columns.length == 3 + && selectInfo.columns[0].kind == ColumnKindCode.ENTITY + && selectInfo.columns[1].kind == ColumnKindCode.ENTITY + && selectInfo.columns[2].kind == ColumnKindCode.STRING; +} diff --git a/extensions/ql-vscode/src/contextual/templateProvider.ts b/extensions/ql-vscode/src/contextual/templateProvider.ts index 13fbcefd481..327a1e60569 100644 --- a/extensions/ql-vscode/src/contextual/templateProvider.ts +++ b/extensions/ql-vscode/src/contextual/templateProvider.ts @@ -3,7 +3,7 @@ import * as vscode from 'vscode'; import { decodeSourceArchiveUri, zipArchiveScheme } from '../archive-filesystem-provider'; import { CodeQLCliServer } from '../cli'; import { DatabaseManager } from '../databases'; -import { CachedOperation } from '../helpers'; +import { CachedOperation, ProgressCallback, withProgress } from '../helpers'; import * as messages from '../messages'; import { QueryServerClient } from '../queryserver-client'; import { compileAndRunQueryAgainstDatabase, QueryWithResults } from '../run-queries'; @@ -44,14 +44,22 @@ export class TemplateQueryDefinitionProvider implements vscode.DefinitionProvide } private async getDefinitions(uriString: string): Promise { - return getLocationsForUriString( - this.cli, - this.qs, - this.dbm, - uriString, - KeyType.DefinitionQuery, - (src, _dest) => src === uriString - ); + return await withProgress({ + location: vscode.ProgressLocation.Notification, + cancellable: true, + title: 'Finding definitions' + }, async (progress, token) => { + return getLocationsForUriString( + this.cli, + this.qs, + this.dbm, + uriString, + KeyType.DefinitionQuery, + progress, + token, + (src, _dest) => src === uriString + ); + }); } } @@ -83,14 +91,22 @@ export class TemplateQueryReferenceProvider implements vscode.ReferenceProvider } private async getReferences(uriString: string): Promise { - return getLocationsForUriString( - this.cli, - this.qs, - this.dbm, - uriString, - KeyType.ReferenceQuery, - (_src, dest) => dest === uriString - ); + return await withProgress({ + location: vscode.ProgressLocation.Notification, + cancellable: true, + title: 'Finding references' + }, async (progress, token) => { + return getLocationsForUriString( + this.cli, + this.qs, + this.dbm, + uriString, + KeyType.DefinitionQuery, + progress, + token, + (src, _dest) => src === uriString + ); + }); } } @@ -101,6 +117,10 @@ export class TemplatePrintAstProvider { private cli: CodeQLCliServer, private qs: QueryServerClient, private dbm: DatabaseManager, + + // Note: progress and token are only used if a cached value is not available + private progress: ProgressCallback, + private token: vscode.CancellationToken ) { this.cache = new CachedOperation(this.getAst.bind(this)); } @@ -157,12 +177,15 @@ export class TemplatePrintAstProvider { } } }; + return await compileAndRunQueryAgainstDatabase( this.cli, this.qs, db, false, vscode.Uri.file(query), + this.progress, + this.token, templates ); } diff --git a/extensions/ql-vscode/src/databaseFetcher.ts b/extensions/ql-vscode/src/databaseFetcher.ts index 3fab83a5447..e47f579d514 100644 --- a/extensions/ql-vscode/src/databaseFetcher.ts +++ b/extensions/ql-vscode/src/databaseFetcher.ts @@ -3,8 +3,7 @@ import * as unzipper from 'unzipper'; import { zip } from 'zip-a-folder'; import { Uri, - ProgressOptions, - ProgressLocation, + CancellationToken, commands, window, } from 'vscode'; @@ -14,8 +13,6 @@ import * as path from 'path'; import { DatabaseManager, DatabaseItem } from './databases'; import { ProgressCallback, - showAndLogErrorMessage, - withProgress, showAndLogInformationMessage, } from './helpers'; import { logger } from './logging'; @@ -28,42 +25,32 @@ import { logger } from './logging'; */ export async function promptImportInternetDatabase( databasesManager: DatabaseManager, - storagePath: string + storagePath: string, + progress: ProgressCallback, + _: CancellationToken, ): Promise { - let item: DatabaseItem | undefined = undefined; - - try { - const databaseUrl = await window.showInputBox({ - prompt: 'Enter URL of zipfile of database to download', - }); - if (databaseUrl) { - validateHttpsUrl(databaseUrl); - - const progressOptions: ProgressOptions = { - location: ProgressLocation.Notification, - title: 'Adding database from URL', - cancellable: false, - }; - await withProgress( - progressOptions, - async (progress) => - (item = await databaseArchiveFetcher( - databaseUrl, - databasesManager, - storagePath, - progress - )) - ); - if (item) { - commands.executeCommand('codeQLDatabases.focus'); - showAndLogInformationMessage('Database downloaded and imported successfully.'); - } - } - } catch (e) { - showAndLogErrorMessage(e.message); + const databaseUrl = await window.showInputBox({ + prompt: 'Enter URL of zipfile of database to download', + }); + if (!databaseUrl) { + return; } + validateHttpsUrl(databaseUrl); + + const item = await databaseArchiveFetcher( + databaseUrl, + databasesManager, + storagePath, + progress + ); + + if (item) { + commands.executeCommand('codeQLDatabases.focus'); + showAndLogInformationMessage('Database downloaded and imported successfully.'); + } return item; + } /** @@ -76,49 +63,37 @@ export async function promptImportInternetDatabase( */ export async function promptImportLgtmDatabase( databasesManager: DatabaseManager, - storagePath: string + storagePath: string, + progress: ProgressCallback, + _: CancellationToken ): Promise { - let item: DatabaseItem | undefined = undefined; + const lgtmUrl = await window.showInputBox({ + prompt: + 'Enter the project URL on LGTM (e.g., https://lgtm.com/projects/g/github/codeql)', + }); + if (!lgtmUrl) { + return; + } - try { - const lgtmUrl = await window.showInputBox({ - prompt: - 'Enter the project URL on LGTM (e.g., https://lgtm.com/projects/g/github/codeql)', - }); - if (!lgtmUrl) { - return; - } - if (looksLikeLgtmUrl(lgtmUrl)) { - const databaseUrl = await convertToDatabaseUrl(lgtmUrl); - if (databaseUrl) { - const progressOptions: ProgressOptions = { - location: ProgressLocation.Notification, - title: 'Adding database from LGTM', - cancellable: false, - }; - await withProgress( - progressOptions, - async (progress) => - (item = await databaseArchiveFetcher( - databaseUrl, - databasesManager, - storagePath, - progress - )) - ); - if (item) { - commands.executeCommand('codeQLDatabases.focus'); - showAndLogInformationMessage('Database downloaded and imported successfully.'); - } + if (looksLikeLgtmUrl(lgtmUrl)) { + const databaseUrl = await convertToDatabaseUrl(lgtmUrl); + if (databaseUrl) { + const item = await databaseArchiveFetcher( + databaseUrl, + databasesManager, + storagePath, + progress + ); + if (item) { + commands.executeCommand('codeQLDatabases.focus'); + showAndLogInformationMessage('Database downloaded and imported successfully.'); } - } else { - throw new Error(`Invalid LGTM URL: ${lgtmUrl}`); + return item; } - } catch (e) { - showAndLogErrorMessage(e.message); + } else { + throw new Error(`Invalid LGTM URL: ${lgtmUrl}`); } - - return item; + return; } /** @@ -131,37 +106,30 @@ export async function promptImportLgtmDatabase( export async function importArchiveDatabase( databaseUrl: string, databasesManager: DatabaseManager, - storagePath: string + storagePath: string, + progress: ProgressCallback, + _: CancellationToken, ): Promise { - let item: DatabaseItem | undefined = undefined; try { - const progressOptions: ProgressOptions = { - location: ProgressLocation.Notification, - title: 'Importing database from archive', - cancellable: false, - }; - await withProgress( - progressOptions, - async (progress) => - (item = await databaseArchiveFetcher( - databaseUrl, - databasesManager, - storagePath, - progress - )) + const item = await databaseArchiveFetcher( + databaseUrl, + databasesManager, + storagePath, + progress ); if (item) { commands.executeCommand('codeQLDatabases.focus'); showAndLogInformationMessage('Database unzipped and imported successfully.'); } + return item; } catch (e) { if (e.message.includes('unexpected end of file')) { - showAndLogErrorMessage('Database is corrupt or too large. Try unzipping outside of VS Code and importing the unzipped folder instead.'); + throw new Error('Database is corrupt or too large. Try unzipping outside of VS Code and importing the unzipped folder instead.'); } else { - showAndLogErrorMessage(e.message); + // delegate + throw e; } } - return item; } /** diff --git a/extensions/ql-vscode/src/databases-ui.ts b/extensions/ql-vscode/src/databases-ui.ts index 26a8af8f1bd..e0dd8b808f4 100644 --- a/extensions/ql-vscode/src/databases-ui.ts +++ b/extensions/ql-vscode/src/databases-ui.ts @@ -1,7 +1,6 @@ import * as path from 'path'; import { DisposableObject } from './vscode-utils/disposable-object'; import { - commands, Event, EventEmitter, ExtensionContext, @@ -11,6 +10,7 @@ import { Uri, window, env, + ProgressLocation } from 'vscode'; import * as fs from 'fs-extra'; @@ -21,9 +21,14 @@ import { DatabaseManager, getUpgradesDirectories, } from './databases'; -import { getOnDiskWorkspaceFolders, showAndLogErrorMessage } from './helpers'; +import { + commandRunner, + getOnDiskWorkspaceFolders, + ProgressCallback, + showAndLogErrorMessage +} from './helpers'; import { logger } from './logging'; -import { clearCacheInDatabase, UserCancellationException } from './run-queries'; +import { clearCacheInDatabase } from './run-queries'; import * as qsClient from './queryserver-client'; import { upgradeDatabase } from './upgrades'; import { @@ -31,6 +36,7 @@ import { promptImportInternetDatabase, promptImportLgtmDatabase, } from './databaseFetcher'; +import { CancellationToken } from 'vscode-jsonrpc'; type ThemableIconPath = { light: string; dark: string } | string; @@ -227,83 +233,104 @@ export class DatabaseUI extends DisposableObject { logger.log('Registering database panel commands.'); ctx.subscriptions.push( - commands.registerCommand( + commandRunner( 'codeQL.setCurrentDatabase', - this.handleSetCurrentDatabase + this.handleSetCurrentDatabase, + { + location: ProgressLocation.Notification, + title: 'Importing database from archive', + cancellable: false, + } ) ); ctx.subscriptions.push( - commands.registerCommand( + commandRunner( 'codeQL.upgradeCurrentDatabase', this.handleUpgradeCurrentDatabase ) ); ctx.subscriptions.push( - commands.registerCommand('codeQL.clearCache', this.handleClearCache) + commandRunner( + 'codeQL.clearCache', + this.handleClearCache, + { + location: ProgressLocation.Notification, + title: 'Clearing Cache', + cancellable: false, + }) ); ctx.subscriptions.push( - commands.registerCommand( + commandRunner( 'codeQLDatabases.chooseDatabaseFolder', this.handleChooseDatabaseFolder ) ); ctx.subscriptions.push( - commands.registerCommand( + commandRunner( 'codeQLDatabases.chooseDatabaseArchive', this.handleChooseDatabaseArchive ) ); ctx.subscriptions.push( - commands.registerCommand( + commandRunner( 'codeQLDatabases.chooseDatabaseInternet', - this.handleChooseDatabaseInternet + this.handleChooseDatabaseInternet, + { + location: ProgressLocation.Notification, + title: 'Adding database from URL', + cancellable: false, + } ) ); ctx.subscriptions.push( - commands.registerCommand( + commandRunner( 'codeQLDatabases.chooseDatabaseLgtm', - this.handleChooseDatabaseLgtm - ) + this.handleChooseDatabaseLgtm, + { + location: ProgressLocation.Notification, + title: 'Adding database from LGTM', + cancellable: false, + }) ); ctx.subscriptions.push( - commands.registerCommand( + commandRunner( 'codeQLDatabases.setCurrentDatabase', this.handleMakeCurrentDatabase ) ); ctx.subscriptions.push( - commands.registerCommand( + commandRunner( 'codeQLDatabases.sortByName', this.handleSortByName ) ); ctx.subscriptions.push( - commands.registerCommand( + commandRunner( 'codeQLDatabases.sortByDateAdded', this.handleSortByDateAdded ) ); ctx.subscriptions.push( - commands.registerCommand( + commandRunner( 'codeQLDatabases.removeDatabase', this.handleRemoveDatabase ) ); ctx.subscriptions.push( - commands.registerCommand( + commandRunner( 'codeQLDatabases.upgradeDatabase', this.handleUpgradeDatabase ) ); ctx.subscriptions.push( - commands.registerCommand( + commandRunner( 'codeQLDatabases.renameDatabase', this.handleRenameDatabase ) ); ctx.subscriptions.push( - commands.registerCommand( + commandRunner( 'codeQLDatabases.openDatabaseFolder', this.handleOpenFolder ) @@ -316,37 +343,53 @@ export class DatabaseUI extends DisposableObject { await this.databaseManager.setCurrentDatabaseItem(databaseItem); }; - handleChooseDatabaseFolder = async (): Promise => { + handleChooseDatabaseFolder = async ( + progress: ProgressCallback, + token: CancellationToken + ): Promise => { try { - return await this.chooseAndSetDatabase(true); + return await this.chooseAndSetDatabase(true, progress, token); } catch (e) { showAndLogErrorMessage(e.message); return undefined; } }; - handleChooseDatabaseArchive = async (): Promise => { + handleChooseDatabaseArchive = async ( + progress: ProgressCallback, + token: CancellationToken + ): Promise => { try { - return await this.chooseAndSetDatabase(false); + return await this.chooseAndSetDatabase(false, progress, token); } catch (e) { showAndLogErrorMessage(e.message); return undefined; } }; - handleChooseDatabaseInternet = async (): Promise< + handleChooseDatabaseInternet = async ( + progress: ProgressCallback, + token: CancellationToken + ): Promise< DatabaseItem | undefined > => { return await promptImportInternetDatabase( this.databaseManager, - this.storagePath + this.storagePath, + progress, + token ); }; - handleChooseDatabaseLgtm = async (): Promise => { + handleChooseDatabaseLgtm = async ( + progress: ProgressCallback, + token: CancellationToken + ): Promise => { return await promptImportLgtmDatabase( this.databaseManager, - this.storagePath + this.storagePath, + progress, + token ); }; @@ -377,116 +420,115 @@ export class DatabaseUI extends DisposableObject { databaseItem: DatabaseItem | undefined, multiSelect: DatabaseItem[] | undefined ): Promise => { - try { - if (multiSelect?.length) { - await Promise.all( - multiSelect.map((dbItem) => this.handleUpgradeDatabase(dbItem, [])) - ); - } - if (this.queryServer === undefined) { - logger.log( - 'Received request to upgrade database, but there is no running query server.' - ); - return; - } - if (databaseItem === undefined) { - logger.log( - 'Received request to upgrade database, but no database was provided.' - ); - return; - } - if (databaseItem.contents === undefined) { - logger.log( - 'Received request to upgrade database, but database contents could not be found.' - ); - return; - } - if (databaseItem.contents.dbSchemeUri === undefined) { - logger.log( - 'Received request to upgrade database, but database has no schema.' - ); - return; - } - - // Search for upgrade scripts in any workspace folders available - const searchPath: string[] = getOnDiskWorkspaceFolders(); - - const upgradeInfo = await this.cliserver.resolveUpgrades( - databaseItem.contents.dbSchemeUri.fsPath, - searchPath + if (multiSelect?.length) { + await Promise.all( + multiSelect.map((dbItem) => this.handleUpgradeDatabase(dbItem, [])) + ); + } + if (this.queryServer === undefined) { + logger.log( + 'Received request to upgrade database, but there is no running query server.' + ); + return; + } + if (databaseItem === undefined) { + logger.log( + 'Received request to upgrade database, but no database was provided.' + ); + return; + } + if (databaseItem.contents === undefined) { + logger.log( + 'Received request to upgrade database, but database contents could not be found.' ); + return; + } + if (databaseItem.contents.dbSchemeUri === undefined) { + logger.log( + 'Received request to upgrade database, but database has no schema.' + ); + return; + } - const { scripts, finalDbscheme } = upgradeInfo; + // Search for upgrade scripts in any workspace folders available + const searchPath: string[] = getOnDiskWorkspaceFolders(); - if (finalDbscheme === undefined) { - logger.log('Could not determine target dbscheme to upgrade to.'); - return; - } - const targetDbSchemeUri = Uri.file(finalDbscheme); + const upgradeInfo = await this.cliserver.resolveUpgrades( + databaseItem.contents.dbSchemeUri.fsPath, + searchPath + ); - await upgradeDatabase( - this.queryServer, - databaseItem, - targetDbSchemeUri, - getUpgradesDirectories(scripts) - ); - } catch (e) { - if (e instanceof UserCancellationException) { - logger.log(e.message); - } else throw e; + const { scripts, finalDbscheme } = upgradeInfo; + + if (finalDbscheme === undefined) { + logger.log('Could not determine target dbscheme to upgrade to.'); + return; } + const targetDbSchemeUri = Uri.file(finalDbscheme); + + await upgradeDatabase( + this.queryServer, + databaseItem, + targetDbSchemeUri, + getUpgradesDirectories(scripts) + ); }; - private handleClearCache = async (): Promise => { + private handleClearCache = async ( + progress: ProgressCallback, + token: CancellationToken, + ): Promise => { if ( this.queryServer !== undefined && this.databaseManager.currentDatabaseItem !== undefined ) { await clearCacheInDatabase( this.queryServer, - this.databaseManager.currentDatabaseItem + this.databaseManager.currentDatabaseItem, + progress, + token ); } }; private handleSetCurrentDatabase = async ( - uri: Uri - ): Promise => { + uri: Uri, + progress: ProgressCallback, + token: CancellationToken, + ): Promise => { try { // Assume user has selected an archive if the file has a .zip extension if (uri.path.endsWith('.zip')) { - return await importArchiveDatabase( + await importArchiveDatabase( uri.toString(true), this.databaseManager, - this.storagePath + this.storagePath, + progress, + token ); + } else { + await this.setCurrentDatabase(uri); } - - return await this.setCurrentDatabase(uri); } catch (e) { - showAndLogErrorMessage( + // rethrow and let this be handled by default error handling. + throw new Error( `Could not set database to ${path.basename(uri.fsPath)}. Reason: ${ e.message }` ); - return undefined; } }; - private handleRemoveDatabase = ( + private handleRemoveDatabase = async ( databaseItem: DatabaseItem, multiSelect: DatabaseItem[] | undefined - ): void => { - try { - if (multiSelect?.length) { - multiSelect.forEach((dbItem) => - this.databaseManager.removeDatabaseItem(dbItem) - ); - } else { - this.databaseManager.removeDatabaseItem(databaseItem); - } - } catch (e) { - showAndLogErrorMessage(e.message); + ): Promise => { + if (multiSelect?.length) { + multiSelect.forEach((dbItem) => + this.databaseManager.removeDatabaseItem(dbItem) + ); + } else { + this.databaseManager.removeDatabaseItem(databaseItem); } }; @@ -494,19 +536,15 @@ export class DatabaseUI extends DisposableObject { databaseItem: DatabaseItem, multiSelect: DatabaseItem[] | undefined ): Promise => { - try { - this.assertSingleDatabase(multiSelect); + this.assertSingleDatabase(multiSelect); - const newName = await window.showInputBox({ - prompt: 'Choose new database name', - value: databaseItem.name, - }); + const newName = await window.showInputBox({ + prompt: 'Choose new database name', + value: databaseItem.name, + }); - if (newName) { - this.databaseManager.renameDatabaseItem(databaseItem, newName); - } - } catch (e) { - showAndLogErrorMessage(e.message); + if (newName) { + this.databaseManager.renameDatabaseItem(databaseItem, newName); } }; @@ -514,16 +552,12 @@ export class DatabaseUI extends DisposableObject { databaseItem: DatabaseItem, multiSelect: DatabaseItem[] | undefined ): Promise => { - try { - if (multiSelect?.length) { - await Promise.all( - multiSelect.map((dbItem) => env.openExternal(dbItem.databaseUri)) - ); - } else { - await env.openExternal(databaseItem.databaseUri); - } - } catch (e) { - showAndLogErrorMessage(e.message); + if (multiSelect?.length) { + await Promise.all( + multiSelect.map((dbItem) => env.openExternal(dbItem.databaseUri)) + ); + } else { + await env.openExternal(databaseItem.databaseUri); } }; @@ -532,9 +566,12 @@ export class DatabaseUI extends DisposableObject { * current database, ask the user for one, and return that, or * undefined if they cancel. */ - public async getDatabaseItem(): Promise { + public async getDatabaseItem( + progress: ProgressCallback, + token: CancellationToken + ): Promise { if (this.databaseManager.currentDatabaseItem === undefined) { - await this.chooseAndSetDatabase(false); + await this.chooseAndSetDatabase(false, progress, token); } return this.databaseManager.currentDatabaseItem; @@ -557,7 +594,9 @@ export class DatabaseUI extends DisposableObject { * operation was canceled. */ private async chooseAndSetDatabase( - byFolder: boolean + byFolder: boolean, + progress: ProgressCallback, + token: CancellationToken, ): Promise { const uri = await chooseDatabaseDir(byFolder); @@ -575,7 +614,9 @@ export class DatabaseUI extends DisposableObject { return await importArchiveDatabase( uri.toString(true), this.databaseManager, - this.storagePath + this.storagePath, + progress, + token ); } } diff --git a/extensions/ql-vscode/src/extension.ts b/extensions/ql-vscode/src/extension.ts index 9202a27f2b2..a7fabb3b605 100644 --- a/extensions/ql-vscode/src/extension.ts +++ b/extensions/ql-vscode/src/extension.ts @@ -1,4 +1,5 @@ import { + CancellationToken, commands, Disposable, ExtensionContext, @@ -18,7 +19,12 @@ import { testExplorerExtensionId, TestHub } from 'vscode-test-adapter-api'; import { AstViewer } from './astViewer'; import * as archiveFilesystemProvider from './archive-filesystem-provider'; import { CodeQLCliServer } from './cli'; -import { DistributionConfigListener, MAX_QUERIES, QueryHistoryConfigListener, QueryServerConfigListener } from './config'; +import { + DistributionConfigListener, + MAX_QUERIES, + QueryHistoryConfigListener, + QueryServerConfigListener +} from './config'; import * as languageSupport from './languageSupport'; import { DatabaseManager } from './databases'; import { DatabaseUI } from './databases-ui'; @@ -47,7 +53,7 @@ import { QueryHistoryManager } from './query-history'; import { CompletedQuery } from './query-results'; import * as qsClient from './queryserver-client'; import { displayQuickQuery } from './quick-query'; -import { compileAndRunQueryAgainstDatabase, tmpDirDisposal, UserCancellationException } from './run-queries'; +import { compileAndRunQueryAgainstDatabase, tmpDirDisposal } from './run-queries'; import { QLTestAdapterFactory } from './test-adapter'; import { TestUIService } from './test-ui'; import { CompareInterfaceManager } from './compare/compare-interface'; @@ -86,7 +92,7 @@ let isInstallingOrUpdatingDistribution = false; * * @param excludedCommands List of commands for which we should not register error stubs. */ -function registerErrorStubs(excludedCommands: string[], stubGenerator: (command: string) => () => void): void { +function registerErrorStubs(excludedCommands: string[], stubGenerator: (command: string) => () => Promise): void { // Remove existing stubs errorStubs.forEach(stub => stub.dispose()); @@ -101,7 +107,7 @@ function registerErrorStubs(excludedCommands: string[], stubGenerator: (command: stubbedCommands.forEach(command => { if (excludedCommands.indexOf(command) === -1) { - errorStubs.push(commands.registerCommand(command, stubGenerator(command))); + errorStubs.push(helpers.commandRunner(command, stubGenerator(command))); } }); } @@ -119,9 +125,9 @@ export async function activate(ctx: ExtensionContext): Promise { const shouldUpdateOnNextActivationKey = 'shouldUpdateOnNextActivation'; - registerErrorStubs([checkForUpdatesCommand], command => () => { + registerErrorStubs([checkForUpdatesCommand], command => (async () => { helpers.showAndLogErrorMessage(`Can't execute ${command}: waiting to finish loading CodeQL CLI.`); - }); + })); interface DistributionUpdateConfig { isUserInitiated: boolean; @@ -163,6 +169,8 @@ export async function activate(ctx: ExtensionContext): Promise { title: progressTitle, cancellable: false, }; + + // Avoid using commandRunner here because this function might be called upon extension activation await helpers.withProgress(progressOptions, progress => distributionManager.installExtensionManagedDistributionRelease(result.updatedRelease, progress)); @@ -276,7 +284,7 @@ export async function activate(ctx: ExtensionContext): Promise { shouldDisplayMessageWhenNoUpdates: false, allowAutoUpdating: true }))); - ctx.subscriptions.push(commands.registerCommand(checkForUpdatesCommand, () => installOrUpdateThenTryActivate({ + ctx.subscriptions.push(helpers.commandRunner(checkForUpdatesCommand, () => installOrUpdateThenTryActivate({ isUserInitiated: true, shouldDisplayMessageWhenNoUpdates: true, allowAutoUpdating: true @@ -389,40 +397,30 @@ async function activateWithInstalledDistribution( async function compileAndRunQuery( quickEval: boolean, - selectedQuery: Uri | undefined + selectedQuery: Uri | undefined, + progress: helpers.ProgressCallback, + token: CancellationToken, ): Promise { if (qs !== undefined) { - try { - const dbItem = await databaseUI.getDatabaseItem(); - if (dbItem === undefined) { - throw new Error('Can\'t run query without a selected database'); - } - const info = await compileAndRunQueryAgainstDatabase( - cliServer, - qs, - dbItem, - quickEval, - selectedQuery - ); - const item = qhm.addQuery(info); - await showResultsForCompletedQuery(item, WebviewReveal.NotForced); - // The call to showResults potentially creates SARIF file; - // Update the tree item context value to allow viewing that - // SARIF file from context menu. - await qhm.updateTreeItemContextValue(item); - } catch (e) { - if (e instanceof UserCancellationException) { - if (e.silent) { - logger.log(e.message); - } else { - helpers.showAndLogWarningMessage(e.message); - } - } else if (e instanceof Error) { - helpers.showAndLogErrorMessage(e.message); - } else { - throw e; - } + const dbItem = await databaseUI.getDatabaseItem(progress, token); + if (dbItem === undefined) { + throw new Error('Can\'t run query without a selected database'); } + const info = await compileAndRunQueryAgainstDatabase( + cliServer, + qs, + dbItem, + quickEval, + selectedQuery, + progress, + token + ); + const item = qhm.addQuery(info); + await showResultsForCompletedQuery(item, WebviewReveal.NotForced); + // The call to showResults potentially creates SARIF file; + // Update the tree item context value to allow viewing that + // SARIF file from context menu. + await qhm.updateTreeItemContextValue(item); } } @@ -461,53 +459,99 @@ async function activateWithInstalledDistribution( logger.log('Registering top-level command palette commands.'); ctx.subscriptions.push( - commands.registerCommand( + helpers.commandRunner( 'codeQL.runQuery', - async (uri: Uri | undefined) => await compileAndRunQuery(false, uri) + async ( + progress: helpers.ProgressCallback, + token: CancellationToken, + uri: Uri | undefined + ) => await compileAndRunQuery(false, uri, progress, token), + { + location: ProgressLocation.Notification, + title: 'Running query', + cancellable: true + } ) ); ctx.subscriptions.push( - commands.registerCommand( + helpers.commandRunner( 'codeQL.runQueries', - async (_: Uri | undefined, multi: Uri[]) => { + async ( + progress: helpers.ProgressCallback, + token: CancellationToken, + _: Uri | undefined, + multi: Uri[] + ) => { const maxQueryCount = MAX_QUERIES.getValue() as number; - try { - const [files, dirFound] = await gatherQlFiles(multi.map(uri => uri.fsPath)); - if (files.length > maxQueryCount) { - throw new Error(`You tried to run ${files.length} queries, but the maximum is ${maxQueryCount}. Try selecting fewer queries or changing the 'codeQL.runningQueries.maxQueries' setting.`); - } - // warn user and display selected files when a directory is selected because some ql - // files may be hidden from the user. - if (dirFound) { - const fileString = files.map(file => path.basename(file)).join(', '); - const res = await helpers.showBinaryChoiceDialog( - `You are about to run ${files.length} queries: ${fileString} Do you want to continue?` - ); - if (!res) { - return; - } + const [files, dirFound] = await gatherQlFiles(multi.map(uri => uri.fsPath)); + if (files.length > maxQueryCount) { + throw new Error(`You tried to run ${files.length} queries, but the maximum is ${maxQueryCount}. Try selecting fewer queries or changing the 'codeQL.runningQueries.maxQueries' setting.`); + } + // warn user and display selected files when a directory is selected because some ql + // files may be hidden from the user. + if (dirFound) { + const fileString = files.map(file => path.basename(file)).join(', '); + const res = await helpers.showBinaryChoiceDialog( + `You are about to run ${files.length} queries: ${fileString} Do you want to continue?` + ); + if (!res) { + return; } - const queryUris = files.map(path => Uri.parse(`file:${path}`, true)); - await Promise.all(queryUris.map(uri => compileAndRunQuery(false, uri))); - } catch (e) { - helpers.showAndLogErrorMessage(e.message); } - } - ) + const queryUris = files.map(path => Uri.parse(`file:${path}`, true)); + + // Use a wrapped progress so that messages appear with the queries remaining in it. + let queriesRemaining = queryUris.length; + function wrappedProgress(update: helpers.ProgressUpdate) { + const message = queriesRemaining > 1 + ? `${queriesRemaining} remaining. ${update.message}` + : update.message; + progress({ + ...update, + message + }); + } + + wrappedProgress({ + maxStep: queryUris.length, + step: queryUris.length - queriesRemaining, + message: '' + }); + await Promise.all(queryUris.map(async uri => + compileAndRunQuery(false, uri, wrappedProgress, token) + .then(() => queriesRemaining--) + )); + }, + { + location: ProgressLocation.Notification, + title: 'Running queries', + cancellable: true + }) ); ctx.subscriptions.push( - commands.registerCommand( + helpers.commandRunner( 'codeQL.quickEval', - async (uri: Uri | undefined) => await compileAndRunQuery(true, uri) - ) + async ( + progress: helpers.ProgressCallback, + token: CancellationToken, + uri: Uri | undefined + ) => await compileAndRunQuery(true, uri, progress, token), + { + location: ProgressLocation.Notification, + title: 'Running query', + cancellable: true + }) ); ctx.subscriptions.push( - commands.registerCommand('codeQL.quickQuery', async () => - displayQuickQuery(ctx, cliServer, databaseUI) + helpers.commandRunner('codeQL.quickQuery', async ( + progress: helpers.ProgressCallback, + token: CancellationToken + ) => + displayQuickQuery(ctx, cliServer, databaseUI, progress, token) ) ); ctx.subscriptions.push( - commands.registerCommand('codeQL.restartQueryServer', async () => { + helpers.commandRunner('codeQL.restartQueryServer', async () => { await qs.restartQueryServer(); helpers.showAndLogInformationMessage('CodeQL Query Server restarted.', { outputLogger: queryServerLogger, @@ -515,24 +559,45 @@ async function activateWithInstalledDistribution( }) ); ctx.subscriptions.push( - commands.registerCommand('codeQL.chooseDatabaseFolder', () => - databaseUI.handleChooseDatabaseFolder() + helpers.commandRunner('codeQL.chooseDatabaseFolder', ( + progress: helpers.ProgressCallback, + token: CancellationToken + ) => + databaseUI.handleChooseDatabaseFolder(progress, token) ) ); ctx.subscriptions.push( - commands.registerCommand('codeQL.chooseDatabaseArchive', () => - databaseUI.handleChooseDatabaseArchive() + helpers.commandRunner('codeQL.chooseDatabaseArchive', ( + progress: helpers.ProgressCallback, + token: CancellationToken + ) => + databaseUI.handleChooseDatabaseArchive(progress, token) ) ); ctx.subscriptions.push( - commands.registerCommand('codeQL.chooseDatabaseLgtm', () => - databaseUI.handleChooseDatabaseLgtm() - ) + helpers.commandRunner('codeQL.chooseDatabaseLgtm', ( + progress: helpers.ProgressCallback, + token: CancellationToken + ) => + databaseUI.handleChooseDatabaseLgtm(progress, token), + { + location: ProgressLocation.Notification, + title: 'Adding database from LGTM', + cancellable: false, + }) ); ctx.subscriptions.push( - commands.registerCommand('codeQL.chooseDatabaseInternet', () => - databaseUI.handleChooseDatabaseInternet() - ) + helpers.commandRunner('codeQL.chooseDatabaseInternet', ( + progress: helpers.ProgressCallback, + token: CancellationToken + ) => + databaseUI.handleChooseDatabaseInternet(progress, token), + + { + location: ProgressLocation.Notification, + title: 'Adding database from URL', + cancellable: false, + }) ); logger.log('Starting language server.'); @@ -544,18 +609,26 @@ async function activateWithInstalledDistribution( { scheme: archiveFilesystemProvider.zipArchiveScheme }, new TemplateQueryDefinitionProvider(cliServer, qs, dbm) ); + languages.registerReferenceProvider( { scheme: archiveFilesystemProvider.zipArchiveScheme }, new TemplateQueryReferenceProvider(cliServer, qs, dbm) ); const astViewer = new AstViewer(ctx); - ctx.subscriptions.push(commands.registerCommand('codeQL.viewAst', async () => { - const ast = await new TemplatePrintAstProvider(cliServer, qs, dbm) + ctx.subscriptions.push(helpers.commandRunner('codeQL.viewAst', async ( + progress: helpers.ProgressCallback, + token: CancellationToken + ) => { + const ast = await new TemplatePrintAstProvider(cliServer, qs, dbm, progress, token) .provideAst(window.activeTextEditor?.document); if (ast) { astViewer.updateRoots(await ast.getRoots(), ast.db, ast.fileName); } + }, { + location: ProgressLocation.Notification, + cancellable: true, + title: 'Calculate AST' })); logger.log('Successfully finished extension initialization.'); diff --git a/extensions/ql-vscode/src/helpers.ts b/extensions/ql-vscode/src/helpers.ts index c92ddbd662a..8758cf8ddfd 100644 --- a/extensions/ql-vscode/src/helpers.ts +++ b/extensions/ql-vscode/src/helpers.ts @@ -7,11 +7,23 @@ import { ExtensionContext, ProgressOptions, window as Window, - workspace + workspace, + commands, + Disposable } from 'vscode'; import { CodeQLCliServer } from './cli'; import { logger } from './logging'; +export class UserCancellationException extends Error { + /** + * @param message The error message + * @param silent If silent is true, then this exception will avoid showing a warning message to the user. + */ + constructor(message?: string, public readonly silent = false) { + super(message); + } +} + export interface ProgressUpdate { /** * The current step @@ -29,18 +41,34 @@ export interface ProgressUpdate { export type ProgressCallback = (p: ProgressUpdate) => void; +export type ProgressTask = ( + progress: ProgressCallback, + token: CancellationToken, + ...args: any[] +) => Thenable; + +type NoProgressTask = ((...args: any[]) => Promise); + /** * This mediates between the kind of progress callbacks we want to * write (where we *set* current progress position and give * `maxSteps`) and the kind vscode progress api expects us to write - * (which increment progress by a certain amount out of 100%) + * (which increment progress by a certain amount out of 100%). + * + * Where possible, the `commandRunner` function below should be used + * instead of this function. The commandRunner is meant for wrapping + * top-level commands and provides error handling and other support + * automatically. + * + * Only use this function if you need a progress monitor and the + * control flow does not always come from a command (eg- during + * extension activation, or from an internal language server + * request). */ export function withProgress( options: ProgressOptions, - task: ( - progress: (p: ProgressUpdate) => void, - token: CancellationToken - ) => Thenable + task: ProgressTask, + ...args: any[] ): Thenable { let progressAchieved = 0; return Window.withProgress(options, @@ -50,10 +78,58 @@ export function withProgress( const increment = 100 * (step - progressAchieved) / maxStep; progressAchieved = step; progress.report({ message, increment }); - }, token); + }, token, ...args); }); } +/** + * A generic wrapper for commands. This wrapper adds error handling and progress monitoring + * for any command. + * + * There are two ways to invoke the command task: with or without a progress monitor + * If progressOptions are passed in, then the command task will run with a progress monitor + * Otherwise, no progress monitor will be used. + * + * If a task is run with a progress monitor, the first two arguments to the task are always + * the progress callback, and the cancellation token. And this is followed by any extra command arguments + * (eg- selection, multiselection, ...). + * + * If there is no progress monitor, then only extra command arguments are passed in. + * + * @param commandId The ID of the command to register. + * @param task The task to run. If passing taskOptions, then this task must be a `ProgressTask`. + * @param progressOptions Optional argument. If present, then the task is run with a progress monitor + * and cancellation token, otherwise it is run with no arguments. + */ +export function commandRunner( + commandId: string, + task: ProgressTask | NoProgressTask, + progressOptions?: ProgressOptions +): Disposable { + return commands.registerCommand(commandId, async (...args: any[]) => { + try { + if (progressOptions) { + await withProgress(progressOptions, task as ProgressTask, ...args); + } else { + await (task as ((...args: any[]) => Promise))(...args); + } + } catch (e) { + if (e instanceof UserCancellationException) { + // User has cancelled this action manually + if (e.silent) { + logger.log(e.message); + } else { + showAndLogWarningMessage(e.message); + } + } else if (e instanceof Error) { + showAndLogErrorMessage(e.message); + } else { + throw e; + } + } + }); +} + /** * Show an error message and log it to the console * diff --git a/extensions/ql-vscode/src/interface.ts b/extensions/ql-vscode/src/interface.ts index 6948241904d..d3a8004cbd7 100644 --- a/extensions/ql-vscode/src/interface.ts +++ b/extensions/ql-vscode/src/interface.ts @@ -32,6 +32,7 @@ import { RawResultsSortState, } from './interface-types'; import { Logger } from './logging'; +import { commandRunner } from './helpers'; import * as messages from './messages'; import { CompletedQuery, interpretResults } from './query-results'; import { QueryInfo, tmpDir } from './run-queries'; @@ -121,13 +122,13 @@ export class InterfaceManager extends DisposableObject { ); logger.log('Registering path-step navigation commands.'); this.push( - vscode.commands.registerCommand( + commandRunner( 'codeQLQueryResults.nextPathStep', this.navigatePathStep.bind(this, 1) ) ); this.push( - vscode.commands.registerCommand( + commandRunner( 'codeQLQueryResults.previousPathStep', this.navigatePathStep.bind(this, -1) ) @@ -145,7 +146,7 @@ export class InterfaceManager extends DisposableObject { ); } - navigatePathStep(direction: number): void { + async navigatePathStep(direction: number): Promise { this.postMessage({ t: 'navigatePath', direction }); } diff --git a/extensions/ql-vscode/src/query-history.ts b/extensions/ql-vscode/src/query-history.ts index 3dffdadf267..9921d091991 100644 --- a/extensions/ql-vscode/src/query-history.ts +++ b/extensions/ql-vscode/src/query-history.ts @@ -209,51 +209,51 @@ export class QueryHistoryManager { }); logger.log('Registering query history panel commands.'); ctx.subscriptions.push( - vscode.commands.registerCommand( + helpers.commandRunner( 'codeQLQueryHistory.openQuery', this.handleOpenQuery.bind(this) ) ); ctx.subscriptions.push( - vscode.commands.registerCommand( + helpers.commandRunner( 'codeQLQueryHistory.removeHistoryItem', this.handleRemoveHistoryItem.bind(this) ) ); ctx.subscriptions.push( - vscode.commands.registerCommand( + helpers.commandRunner( 'codeQLQueryHistory.setLabel', this.handleSetLabel.bind(this) ) ); ctx.subscriptions.push( - vscode.commands.registerCommand( + helpers.commandRunner( 'codeQLQueryHistory.compareWith', this.handleCompareWith.bind(this) ) ); ctx.subscriptions.push( - vscode.commands.registerCommand( + helpers.commandRunner( 'codeQLQueryHistory.showQueryLog', this.handleShowQueryLog.bind(this) ) ); ctx.subscriptions.push( - vscode.commands.registerCommand( + helpers.commandRunner( 'codeQLQueryHistory.showQueryText', this.handleShowQueryText.bind(this) ) ); ctx.subscriptions.push( - vscode.commands.registerCommand( + helpers.commandRunner( 'codeQLQueryHistory.viewSarif', this.handleViewSarif.bind(this) ) ); ctx.subscriptions.push( - vscode.commands.registerCommand( + helpers.commandRunner( 'codeQLQueryHistory.itemClicked', - async (item) => { + async (item: CompletedQuery) => { return this.handleItemClicked(item, [item]); } ) @@ -424,22 +424,18 @@ export class QueryHistoryManager { return; } - try { - const queryName = singleItem.queryName.endsWith('.ql') - ? singleItem.queryName - : singleItem.queryName + '.ql'; - const params = new URLSearchParams({ - isQuickEval: String(!!singleItem.query.quickEvalPosition), - queryText: encodeURIComponent(await this.getQueryText(singleItem)), - }); - const uri = vscode.Uri.parse( - `codeql:${singleItem.query.queryID}-${queryName}?${params.toString()}` - ); - const doc = await vscode.workspace.openTextDocument(uri); - await vscode.window.showTextDocument(doc, { preview: false }); - } catch (e) { - helpers.showAndLogErrorMessage(e.message); - } + const queryName = singleItem.queryName.endsWith('.ql') + ? singleItem.queryName + : singleItem.queryName + '.ql'; + const params = new URLSearchParams({ + isQuickEval: String(!!singleItem.query.quickEvalPosition), + queryText: encodeURIComponent(await this.getQueryText(singleItem)), + }); + const uri = vscode.Uri.parse( + `codeql:${singleItem.query.queryID}-${queryName}?${params.toString()}` + ); + const doc = await vscode.workspace.openTextDocument(uri); + await vscode.window.showTextDocument(doc, { preview: false }); } async handleViewSarif( @@ -450,20 +446,16 @@ export class QueryHistoryManager { return; } - try { - const hasInterpretedResults = await singleItem.query.canHaveInterpretedResults(); - if (hasInterpretedResults) { - await this.tryOpenExternalFile( - singleItem.query.resultsPaths.interpretedResultsPath - ); - } else { - const label = singleItem.getLabel(); - helpers.showAndLogInformationMessage( - `Query ${label} has no interpreted results.` - ); - } - } catch (e) { - helpers.showAndLogErrorMessage(e.message); + const hasInterpretedResults = await singleItem.query.canHaveInterpretedResults(); + if (hasInterpretedResults) { + await this.tryOpenExternalFile( + singleItem.query.resultsPaths.interpretedResultsPath + ); + } else { + const label = singleItem.getLabel(); + helpers.showAndLogInformationMessage( + `Query ${label} has no interpreted results.` + ); } } diff --git a/extensions/ql-vscode/src/quick-query.ts b/extensions/ql-vscode/src/quick-query.ts index 802b4f8293d..ac65e391a93 100644 --- a/extensions/ql-vscode/src/quick-query.ts +++ b/extensions/ql-vscode/src/quick-query.ts @@ -1,13 +1,12 @@ import * as fs from 'fs-extra'; import * as yaml from 'js-yaml'; import * as path from 'path'; -import { ExtensionContext, window as Window, workspace, Uri } from 'vscode'; +import { CancellationToken, ExtensionContext, window as Window, workspace, Uri } from 'vscode'; import { ErrorCodes, ResponseError } from 'vscode-languageclient'; import { CodeQLCliServer } from './cli'; import { DatabaseUI } from './databases-ui'; import * as helpers from './helpers'; import { logger } from './logging'; -import { UserCancellationException } from './run-queries'; const QUICK_QUERIES_DIR_NAME = 'quick-queries'; const QUICK_QUERY_QUERY_NAME = 'quick-query.ql'; @@ -48,7 +47,13 @@ function getQuickQueriesDir(ctx: ExtensionContext): string { /** * Show a buffer the user can enter a simple query into. */ -export async function displayQuickQuery(ctx: ExtensionContext, cliServer: CodeQLCliServer, databaseUI: DatabaseUI) { +export async function displayQuickQuery( + ctx: ExtensionContext, + cliServer: CodeQLCliServer, + databaseUI: DatabaseUI, + progress: helpers.ProgressCallback, + token: CancellationToken +) { function updateQuickQueryDir(queriesDir: string, index: number, len: number) { workspace.updateWorkspaceFolders( @@ -94,7 +99,7 @@ export async function displayQuickQuery(ctx: ExtensionContext, cliServer: CodeQL updateQuickQueryDir(queriesDir, index, 1); // We're going to infer which qlpack to use from the current database - const dbItem = await databaseUI.getDatabaseItem(); + const dbItem = await databaseUI.getDatabaseItem(progress, token); if (dbItem === undefined) { throw new Error('Can\'t start quick query without a selected database'); } @@ -116,7 +121,7 @@ export async function displayQuickQuery(ctx: ExtensionContext, cliServer: CodeQL // TODO: clean up error handling for top-level commands like this catch (e) { - if (e instanceof UserCancellationException) { + if (e instanceof helpers.UserCancellationException) { logger.log(e.message); } else if (e instanceof ResponseError && e.code == ErrorCodes.RequestCancelled) { diff --git a/extensions/ql-vscode/src/run-queries.ts b/extensions/ql-vscode/src/run-queries.ts index a5215a4fa01..040d278a1f1 100644 --- a/extensions/ql-vscode/src/run-queries.ts +++ b/extensions/ql-vscode/src/run-queries.ts @@ -2,7 +2,14 @@ import * as crypto from 'crypto'; import * as fs from 'fs-extra'; import * as path from 'path'; import * as tmp from 'tmp'; -import * as vscode from 'vscode'; +import { + CancellationToken, + ConfigurationTarget, + TextDocument, + TextEditor, + Uri, + window +} from 'vscode'; import { ErrorCodes, ResponseError } from 'vscode-languageclient'; import * as cli from './cli'; @@ -34,16 +41,6 @@ export const tmpDirDisposal = { } }; -export class UserCancellationException extends Error { - /** - * @param message The error message - * @param silent If silent is true, then this exception will avoid showing a warning message to the user. - */ - constructor(message?: string, public readonly silent = false) { - super(message); - } -} - /** * A collection of evaluation-time information about a query, * including the query itself, and where we have decided to put @@ -55,7 +52,7 @@ export class QueryInfo { readonly compiledQueryPath: string; readonly resultsPaths: ResultsPaths; - readonly dataset: vscode.Uri; // guarantee the existence of a well-defined dataset dir at this point + readonly dataset: Uri; // guarantee the existence of a well-defined dataset dir at this point readonly queryID: number; constructor( @@ -80,6 +77,8 @@ export class QueryInfo { async run( qs: qsClient.QueryServerClient, + progress: helpers.ProgressCallback, + token: CancellationToken, ): Promise { let result: messages.EvaluationResult | null = null; @@ -87,7 +86,7 @@ export class QueryInfo { const queryToRun: messages.QueryToRun = { resultsPath: this.resultsPaths.resultsPath, - qlo: vscode.Uri.file(this.compiledQueryPath).toString(), + qlo: Uri.file(this.compiledQueryPath).toString(), allowUnknownTemplates: true, templateValues: this.templates, id: callbackId, @@ -105,13 +104,7 @@ export class QueryInfo { useSequenceHint: false }; try { - await helpers.withProgress({ - location: vscode.ProgressLocation.Notification, - title: 'Running Query', - cancellable: true, - }, (progress, token) => { - return qs.sendRequest(messages.runQueries, params, token, progress); - }); + await qs.sendRequest(messages.runQueries, params, token, progress); } finally { qs.unRegisterCallback(callbackId); } @@ -126,6 +119,8 @@ export class QueryInfo { async compile( qs: qsClient.QueryServerClient, + progress: helpers.ProgressCallback, + token: CancellationToken, ): Promise { let compiled: messages.CheckQueryResult | undefined; try { @@ -150,13 +145,7 @@ export class QueryInfo { target, }; - compiled = await helpers.withProgress({ - location: vscode.ProgressLocation.Notification, - title: 'Compiling Query', - cancellable: true, - }, (progress, token) => { - return qs.sendRequest(messages.compileQuery, params, token, progress); - }); + compiled = await qs.sendRequest(messages.compileQuery, params, token, progress); } finally { qs.logger.log(' - - - COMPILATION DONE - - - '); } @@ -192,7 +181,10 @@ export interface QueryWithResults { } export async function clearCacheInDatabase( - qs: qsClient.QueryServerClient, dbItem: DatabaseItem + qs: qsClient.QueryServerClient, + dbItem: DatabaseItem, + progress: helpers.ProgressCallback, + token: CancellationToken, ): Promise { if (dbItem.contents === undefined) { throw new Error('Can\'t clear the cache in an invalid database.'); @@ -208,13 +200,7 @@ export async function clearCacheInDatabase( db, }; - return helpers.withProgress({ - location: vscode.ProgressLocation.Notification, - title: 'Clearing Cache', - cancellable: false, - }, (progress, token) => - qs.sendRequest(messages.clearCache, params, token, progress) - ); + return qs.sendRequest(messages.clearCache, params, token, progress); } /** @@ -250,7 +236,7 @@ async function convertToQlPath(filePath: string): Promise { /** Gets the selected position within the given editor. */ -async function getSelectedPosition(editor: vscode.TextEditor): Promise { +async function getSelectedPosition(editor: TextEditor): Promise { const pos = editor.selection.start; const posEnd = editor.selection.end; // Convert from 0-based to 1-based line and column numbers. @@ -306,7 +292,7 @@ async function checkDbschemeCompatibility( await upgradeDatabase( qs, query.dbItem, - vscode.Uri.file(finalDbscheme), + Uri.file(finalDbscheme), getUpgradesDirectories(scripts) ); } @@ -321,7 +307,7 @@ async function checkDbschemeCompatibility( * @returns true if we should save changes and false if we should continue without saving changes. * @throws UserCancellationException if we should abort whatever operation triggered this prompt */ -async function promptUserToSaveChanges(document: vscode.TextDocument): Promise { +async function promptUserToSaveChanges(document: TextDocument): Promise { if (document.isDirty) { if (config.AUTOSAVE_SETTING.getValue()) { return true; @@ -332,14 +318,14 @@ async function promptUserToSaveChanges(document: vscode.TextDocument): Promise { - const editor = vscode.window.activeTextEditor; +export async function determineSelectedQuery(selectedResourceUri: Uri | undefined, quickEval: boolean): Promise { + const editor = window.activeTextEditor; // Choose which QL file to use. - let queryUri: vscode.Uri; + let queryUri: Uri; if (selectedResourceUri === undefined) { // No resource was passed to the command handler, so obtain it from the active editor. // This usually happens when the command is called from the Command Palette. @@ -437,7 +423,9 @@ export async function compileAndRunQueryAgainstDatabase( qs: qsClient.QueryServerClient, db: DatabaseItem, quickEval: boolean, - selectedQueryUri: vscode.Uri | undefined, + selectedQueryUri: Uri | undefined, + progress: helpers.ProgressCallback, + token: CancellationToken, templates?: messages.TemplateDefinitions, ): Promise { if (!db.contents || !db.contents.dbSchemeUri) { @@ -497,7 +485,7 @@ export async function compileAndRunQueryAgainstDatabase( let errors; try { - errors = await query.compile(qs); + errors = await query.compile(qs, progress, token); } catch (e) { if (e instanceof ResponseError && e.code == ErrorCodes.RequestCancelled) { return createSyntheticResult(query, db, historyItemOptions, 'Query cancelled', messages.QueryResultType.CANCELLATION); @@ -507,7 +495,7 @@ export async function compileAndRunQueryAgainstDatabase( } if (errors.length == 0) { - const result = await query.run(qs); + const result = await query.run(qs, progress, token); if (result.resultType !== messages.QueryResultType.SUCCESS) { const message = result.message || 'Failed to run query'; logger.log(message); diff --git a/extensions/ql-vscode/src/upgrades.ts b/extensions/ql-vscode/src/upgrades.ts index b512c216195..efec7d6acc8 100644 --- a/extensions/ql-vscode/src/upgrades.ts +++ b/extensions/ql-vscode/src/upgrades.ts @@ -4,7 +4,7 @@ import * as helpers from './helpers'; import { logger } from './logging'; import * as messages from './messages'; import * as qsClient from './queryserver-client'; -import { upgradesTmpDir, UserCancellationException } from './run-queries'; +import { upgradesTmpDir } from './run-queries'; /** * Maximum number of lines to include from database upgrade message, @@ -101,7 +101,7 @@ async function checkAndConfirmDatabaseUpgrade( return params; } else { - throw new UserCancellationException('User cancelled the database upgrade.'); + throw new helpers.UserCancellationException('User cancelled the database upgrade.'); } } @@ -112,7 +112,9 @@ async function checkAndConfirmDatabaseUpgrade( * Reports errors during compilation and evaluation of upgrades to the user. */ export async function upgradeDatabase( - qs: qsClient.QueryServerClient, db: DatabaseItem, targetDbScheme: vscode.Uri, upgradesDirectories: vscode.Uri[] + qs: qsClient.QueryServerClient, + db: DatabaseItem, targetDbScheme: vscode.Uri, + upgradesDirectories: vscode.Uri[] ): Promise { const upgradeParams = await checkAndConfirmDatabaseUpgrade(qs, db, targetDbScheme, upgradesDirectories); @@ -155,6 +157,7 @@ export async function upgradeDatabase( async function checkDatabaseUpgrade( qs: qsClient.QueryServerClient, upgradeParams: messages.UpgradeParams ): Promise { + // Avoid using commandRunner here because this function might be called upon extension activation return helpers.withProgress({ location: vscode.ProgressLocation.Notification, title: 'Checking for database upgrades', @@ -170,6 +173,7 @@ async function compileDatabaseUpgrade( upgradeTempDir: upgradesTmpDir.name }; + // Avoid using commandRunner here because this function might be called upon extension activation return helpers.withProgress({ location: vscode.ProgressLocation.Notification, title: 'Compiling database upgrades', @@ -195,6 +199,7 @@ async function runDatabaseUpgrade( toRun: upgrades }; + // Avoid using commandRunner here because this function might be called upon extension activation return helpers.withProgress({ location: vscode.ProgressLocation.Notification, title: 'Running database upgrades', diff --git a/extensions/ql-vscode/src/vscode-utils/ui-service.ts b/extensions/ql-vscode/src/vscode-utils/ui-service.ts index 574e517084b..bacace8c25c 100644 --- a/extensions/ql-vscode/src/vscode-utils/ui-service.ts +++ b/extensions/ql-vscode/src/vscode-utils/ui-service.ts @@ -1,5 +1,6 @@ -import { commands, TreeDataProvider, window } from 'vscode'; +import { TreeDataProvider, window } from 'vscode'; import { DisposableObject } from './disposable-object'; +import { commandRunner } from '../helpers'; /** * A VS Code service that interacts with the UI, including handling commands. @@ -16,7 +17,7 @@ export class UIService extends DisposableObject { * @remarks The command handler is automatically unregistered when the service is disposed. */ protected registerCommand(command: string, callback: (...args: any[]) => any): void { - this.push(commands.registerCommand(command, callback, this)); + this.push(commandRunner(command, callback.bind(this))); } protected registerTreeDataProvider(viewId: string, treeDataProvider: TreeDataProvider): @@ -24,4 +25,4 @@ export class UIService extends DisposableObject { this.push(window.registerTreeDataProvider(viewId, treeDataProvider)); } -} \ No newline at end of file +} From 2873e74b5b8321b6d15b365a5f6db9d39720739d Mon Sep 17 00:00:00 2001 From: Andrew Eisenberg Date: Wed, 30 Sep 2020 14:39:43 -0700 Subject: [PATCH 2/4] Ensure database upgrade request happens only once When a user runs multiple queries on a non-upgraded database, ensure that only one dialog appears for upgrade. This commit also migrates the upgrades.ts file to using the passed-in cancellation token and progress monitor. This ensures that cancelling a database upgrade command will also cancel out of any wrapper operations. Fixes #534 --- extensions/ql-vscode/CHANGELOG.md | 3 + extensions/ql-vscode/src/databases-ui.ts | 66 ++++++++++++++----- extensions/ql-vscode/src/extension.ts | 13 +++- extensions/ql-vscode/src/run-queries.ts | 10 ++- extensions/ql-vscode/src/upgrades.ts | 83 ++++++++++++++---------- 5 files changed, 119 insertions(+), 56 deletions(-) diff --git a/extensions/ql-vscode/CHANGELOG.md b/extensions/ql-vscode/CHANGELOG.md index a08ef61433d..8f605273c13 100644 --- a/extensions/ql-vscode/CHANGELOG.md +++ b/extensions/ql-vscode/CHANGELOG.md @@ -5,6 +5,9 @@ - Add friendly welcome message when the databases view is empty. - Add open query, open results, and remove query commands in the query history view title bar. - Max number of simultaneous queries launchable by runQueries command is now configurable by changing the `codeQL.runningQueries.maxQueries` setting. +- Allow simultaneously run queries to be canceled in a single-click. +- Prevent multiple upgrade dialogs from appearing when running simultaneous queries on upgradeable databases. +- Max number of simultaneous queries launchable by runQueries command is now configurable by changing the codeQL.runningQueries.maxQueries setting. - Fix sorting of results. Some pages of results would have the wrong sort order and columns. - Remember previous sort order when reloading query results. - Fix proper escaping of backslashes in SARIF message strings. diff --git a/extensions/ql-vscode/src/databases-ui.ts b/extensions/ql-vscode/src/databases-ui.ts index e0dd8b808f4..6e641d843dd 100644 --- a/extensions/ql-vscode/src/databases-ui.ts +++ b/extensions/ql-vscode/src/databases-ui.ts @@ -246,7 +246,12 @@ export class DatabaseUI extends DisposableObject { ctx.subscriptions.push( commandRunner( 'codeQL.upgradeCurrentDatabase', - this.handleUpgradeCurrentDatabase + this.handleUpgradeCurrentDatabase, + { + location: ProgressLocation.Notification, + title: 'Upgrading current database', + cancellable: true, + } ) ); ctx.subscriptions.push( @@ -263,13 +268,23 @@ export class DatabaseUI extends DisposableObject { ctx.subscriptions.push( commandRunner( 'codeQLDatabases.chooseDatabaseFolder', - this.handleChooseDatabaseFolder + this.handleChooseDatabaseFolder, + { + location: ProgressLocation.Notification, + title: 'Adding database from folder', + cancellable: false, + } ) ); ctx.subscriptions.push( commandRunner( 'codeQLDatabases.chooseDatabaseArchive', - this.handleChooseDatabaseArchive + this.handleChooseDatabaseArchive, + { + location: ProgressLocation.Notification, + title: 'Adding database from archive', + cancellable: false, + } ) ); ctx.subscriptions.push( @@ -320,7 +335,12 @@ export class DatabaseUI extends DisposableObject { ctx.subscriptions.push( commandRunner( 'codeQLDatabases.upgradeDatabase', - this.handleUpgradeDatabase + this.handleUpgradeDatabase, + { + location: ProgressLocation.Notification, + title: 'Upgrading database', + cancellable: true, + } ) ); ctx.subscriptions.push( @@ -393,6 +413,13 @@ export class DatabaseUI extends DisposableObject { ); }; + async tryUpgradeCurrentDatabase( + progress: ProgressCallback, + token: CancellationToken + ) { + await this.handleUpgradeCurrentDatabase(progress, token); + } + private handleSortByName = async () => { if (this.treeDataProvider.sortOrder === SortOrder.NameAsc) { this.treeDataProvider.sortOrder = SortOrder.NameDesc; @@ -409,45 +436,47 @@ export class DatabaseUI extends DisposableObject { } }; - private handleUpgradeCurrentDatabase = async (): Promise => { + private handleUpgradeCurrentDatabase = async ( + progress: ProgressCallback, + token: CancellationToken, + ): Promise => { await this.handleUpgradeDatabase( + progress, token, this.databaseManager.currentDatabaseItem, [] ); }; private handleUpgradeDatabase = async ( + progress: ProgressCallback, + token: CancellationToken, databaseItem: DatabaseItem | undefined, - multiSelect: DatabaseItem[] | undefined + multiSelect: DatabaseItem[] | undefined, ): Promise => { if (multiSelect?.length) { await Promise.all( - multiSelect.map((dbItem) => this.handleUpgradeDatabase(dbItem, [])) + multiSelect.map((dbItem) => this.handleUpgradeDatabase(progress, token, dbItem, [])) ); } if (this.queryServer === undefined) { - logger.log( + throw new Error( 'Received request to upgrade database, but there is no running query server.' ); - return; } if (databaseItem === undefined) { - logger.log( + throw new Error( 'Received request to upgrade database, but no database was provided.' ); - return; } if (databaseItem.contents === undefined) { - logger.log( + throw new Error( 'Received request to upgrade database, but database contents could not be found.' ); - return; } if (databaseItem.contents.dbSchemeUri === undefined) { - logger.log( + throw new Error( 'Received request to upgrade database, but database has no schema.' ); - return; } // Search for upgrade scripts in any workspace folders available @@ -461,8 +490,7 @@ export class DatabaseUI extends DisposableObject { const { scripts, finalDbscheme } = upgradeInfo; if (finalDbscheme === undefined) { - logger.log('Could not determine target dbscheme to upgrade to.'); - return; + throw new Error('Could not determine target dbscheme to upgrade to.'); } const targetDbSchemeUri = Uri.file(finalDbscheme); @@ -470,7 +498,9 @@ export class DatabaseUI extends DisposableObject { this.queryServer, databaseItem, targetDbSchemeUri, - getUpgradesDirectories(scripts) + getUpgradesDirectories(scripts), + progress, + token ); }; diff --git a/extensions/ql-vscode/src/extension.ts b/extensions/ql-vscode/src/extension.ts index a7fabb3b605..f03db59c6b7 100644 --- a/extensions/ql-vscode/src/extension.ts +++ b/extensions/ql-vscode/src/extension.ts @@ -170,7 +170,7 @@ export async function activate(ctx: ExtensionContext): Promise { cancellable: false, }; - // Avoid using commandRunner here because this function might be called upon extension activation + // Avoid using commandRunner here because this function is called upon extension activation await helpers.withProgress(progressOptions, progress => distributionManager.installExtensionManagedDistributionRelease(result.updatedRelease, progress)); @@ -512,11 +512,21 @@ async function activateWithInstalledDistribution( }); } + if (queryUris.length > 1) { + // Try to upgrade the current database before running any queries + // so that the user isn't confronted with multiple upgrade + // requests for each query to run. + // Only do it if running multiple queries since this check is + // performed on each query run anyway. + await databaseUI.tryUpgradeCurrentDatabase(progress, token); + } + wrappedProgress({ maxStep: queryUris.length, step: queryUris.length - queriesRemaining, message: '' }); + await Promise.all(queryUris.map(async uri => compileAndRunQuery(false, uri, wrappedProgress, token) .then(() => queriesRemaining--) @@ -550,6 +560,7 @@ async function activateWithInstalledDistribution( displayQuickQuery(ctx, cliServer, databaseUI, progress, token) ) ); + ctx.subscriptions.push( helpers.commandRunner('codeQL.restartQueryServer', async () => { await qs.restartQueryServer(); diff --git a/extensions/ql-vscode/src/run-queries.ts b/extensions/ql-vscode/src/run-queries.ts index 040d278a1f1..aef48045638 100644 --- a/extensions/ql-vscode/src/run-queries.ts +++ b/extensions/ql-vscode/src/run-queries.ts @@ -258,7 +258,9 @@ async function getSelectedPosition(editor: TextEditor): Promise { const searchPath = helpers.getOnDiskWorkspaceFolders(); @@ -293,7 +295,9 @@ async function checkDbschemeCompatibility( qs, query.dbItem, Uri.file(finalDbscheme), - getUpgradesDirectories(scripts) + getUpgradesDirectories(scripts), + progress, + token ); } } @@ -481,7 +485,7 @@ export async function compileAndRunQueryAgainstDatabase( } const query = new QueryInfo(qlProgram, db, packConfig.dbscheme, quickEvalPosition, metadata, templates); - await checkDbschemeCompatibility(cliServer, qs, query); + await checkDbschemeCompatibility(cliServer, qs, query, progress, token); let errors; try { diff --git a/extensions/ql-vscode/src/upgrades.ts b/extensions/ql-vscode/src/upgrades.ts index efec7d6acc8..324dc024452 100644 --- a/extensions/ql-vscode/src/upgrades.ts +++ b/extensions/ql-vscode/src/upgrades.ts @@ -20,11 +20,15 @@ const MAX_UPGRADE_MESSAGE_LINES = 10; * @returns the `UpgradeParams` needed to start the upgrade, if the upgrade is possible and was confirmed by the user, or `undefined` otherwise. */ async function checkAndConfirmDatabaseUpgrade( - qs: qsClient.QueryServerClient, db: DatabaseItem, targetDbScheme: vscode.Uri, upgradesDirectories: vscode.Uri[] + qs: qsClient.QueryServerClient, + db: DatabaseItem, + targetDbScheme: vscode.Uri, + upgradesDirectories: vscode.Uri[], + progress: helpers.ProgressCallback, + token: vscode.CancellationToken, ): Promise { if (db.contents === undefined || db.contents.dbSchemeUri === undefined) { - helpers.showAndLogErrorMessage('Database is invalid, and cannot be upgraded.'); - return; + throw new Error('Database is invalid, and cannot be upgraded.'); } const params: messages.UpgradeParams = { fromDbscheme: db.contents.dbSchemeUri.fsPath, @@ -35,11 +39,10 @@ async function checkAndConfirmDatabaseUpgrade( let checkUpgradeResult: messages.CheckUpgradeResult; try { qs.logger.log('Checking database upgrade...'); - checkUpgradeResult = await checkDatabaseUpgrade(qs, params); + checkUpgradeResult = await checkDatabaseUpgrade(qs, params, progress, token); } catch (e) { - helpers.showAndLogErrorMessage(`Database cannot be upgraded: ${e}`); - return; + throw new Error(`Database cannot be upgraded: ${e}`); } finally { qs.logger.log('Done checking database upgrade.'); @@ -48,12 +51,15 @@ async function checkAndConfirmDatabaseUpgrade( const checkedUpgrades = checkUpgradeResult.checkedUpgrades; if (checkedUpgrades === undefined) { const error = checkUpgradeResult.upgradeError || '[no error message available]'; - await helpers.showAndLogErrorMessage(`Database cannot be upgraded: ${error}`); - return; + throw new Error(`Database cannot be upgraded: ${error}`); } if (checkedUpgrades.scripts.length === 0) { - await helpers.showAndLogInformationMessage('Database is already up to date; nothing to do.'); + progress({ + step: 3, + maxStep: 3, + message: 'Database is already up to date; nothing to do.' + }); return; } @@ -114,9 +120,11 @@ async function checkAndConfirmDatabaseUpgrade( export async function upgradeDatabase( qs: qsClient.QueryServerClient, db: DatabaseItem, targetDbScheme: vscode.Uri, - upgradesDirectories: vscode.Uri[] + upgradesDirectories: vscode.Uri[], + progress: helpers.ProgressCallback, + token: vscode.CancellationToken, ): Promise { - const upgradeParams = await checkAndConfirmDatabaseUpgrade(qs, db, targetDbScheme, upgradesDirectories); + const upgradeParams = await checkAndConfirmDatabaseUpgrade(qs, db, targetDbScheme, upgradesDirectories, progress, token); if (upgradeParams === undefined) { return; @@ -124,7 +132,7 @@ export async function upgradeDatabase( let compileUpgradeResult: messages.CompileUpgradeResult; try { - compileUpgradeResult = await compileDatabaseUpgrade(qs, upgradeParams); + compileUpgradeResult = await compileDatabaseUpgrade(qs, upgradeParams, progress, token); } catch (e) { helpers.showAndLogErrorMessage(`Compilation of database upgrades failed: ${e}`); @@ -143,7 +151,7 @@ export async function upgradeDatabase( try { qs.logger.log('Running the following database upgrade:'); qs.logger.log(compileUpgradeResult.compiledUpgrades.scripts.map(s => s.description.description).join('\n')); - return await runDatabaseUpgrade(qs, db, compileUpgradeResult.compiledUpgrades); + return await runDatabaseUpgrade(qs, db, compileUpgradeResult.compiledUpgrades, progress, token); } catch (e) { helpers.showAndLogErrorMessage(`Database upgrade failed: ${e}`); @@ -155,34 +163,46 @@ export async function upgradeDatabase( } async function checkDatabaseUpgrade( - qs: qsClient.QueryServerClient, upgradeParams: messages.UpgradeParams + qs: qsClient.QueryServerClient, + upgradeParams: messages.UpgradeParams, + progress: helpers.ProgressCallback, + token: vscode.CancellationToken, ): Promise { - // Avoid using commandRunner here because this function might be called upon extension activation - return helpers.withProgress({ - location: vscode.ProgressLocation.Notification, - title: 'Checking for database upgrades', - cancellable: true, - }, (progress, token) => qs.sendRequest(messages.checkUpgrade, upgradeParams, token, progress)); + progress({ + step: 1, + maxStep: 3, + message: 'Checking for database upgrades' + }); + + return qs.sendRequest(messages.checkUpgrade, upgradeParams, token, progress); } async function compileDatabaseUpgrade( - qs: qsClient.QueryServerClient, upgradeParams: messages.UpgradeParams + qs: qsClient.QueryServerClient, + upgradeParams: messages.UpgradeParams, + progress: helpers.ProgressCallback, + token: vscode.CancellationToken, ): Promise { const params: messages.CompileUpgradeParams = { upgrade: upgradeParams, upgradeTempDir: upgradesTmpDir.name }; - // Avoid using commandRunner here because this function might be called upon extension activation - return helpers.withProgress({ - location: vscode.ProgressLocation.Notification, - title: 'Compiling database upgrades', - cancellable: true, - }, (progress, token) => qs.sendRequest(messages.compileUpgrade, params, token, progress)); + progress({ + step: 2, + maxStep: 3, + message: 'Compiling database upgrades' + }); + + return qs.sendRequest(messages.compileUpgrade, params, token, progress); } async function runDatabaseUpgrade( - qs: qsClient.QueryServerClient, db: DatabaseItem, upgrades: messages.CompiledUpgrades + qs: qsClient.QueryServerClient, + db: DatabaseItem, + upgrades: messages.CompiledUpgrades, + progress: helpers.ProgressCallback, + token: vscode.CancellationToken, ): Promise { if (db.contents === undefined || db.contents.datasetUri === undefined) { @@ -199,10 +219,5 @@ async function runDatabaseUpgrade( toRun: upgrades }; - // Avoid using commandRunner here because this function might be called upon extension activation - return helpers.withProgress({ - location: vscode.ProgressLocation.Notification, - title: 'Running database upgrades', - cancellable: true, - }, (progress, token) => qs.sendRequest(messages.runUpgrade, params, token, progress)); + return qs.sendRequest(messages.runUpgrade, params, token, progress); } From 7e7623bb67eb34844b78df10648abbf3c6e1f031 Mon Sep 17 00:00:00 2001 From: Andrew Eisenberg Date: Tue, 29 Sep 2020 10:06:34 -0700 Subject: [PATCH 3/4] Update changelog Co-authored-by: Shati Patel <42641846+shati-patel@users.noreply.github.com> --- extensions/ql-vscode/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/extensions/ql-vscode/CHANGELOG.md b/extensions/ql-vscode/CHANGELOG.md index 8f605273c13..261200834b6 100644 --- a/extensions/ql-vscode/CHANGELOG.md +++ b/extensions/ql-vscode/CHANGELOG.md @@ -7,6 +7,7 @@ - Max number of simultaneous queries launchable by runQueries command is now configurable by changing the `codeQL.runningQueries.maxQueries` setting. - Allow simultaneously run queries to be canceled in a single-click. - Prevent multiple upgrade dialogs from appearing when running simultaneous queries on upgradeable databases. +- Allow simultaneously run queries to be canceled in a single-click. - Max number of simultaneous queries launchable by runQueries command is now configurable by changing the codeQL.runningQueries.maxQueries setting. - Fix sorting of results. Some pages of results would have the wrong sort order and columns. - Remember previous sort order when reloading query results. From ad06780291f2a116c98192edfb491bdcce811a78 Mon Sep 17 00:00:00 2001 From: Andrew Eisenberg Date: Thu, 8 Oct 2020 10:43:40 -0700 Subject: [PATCH 4/4] Refactor the commandRunner Split commandRunner into two functions: commandRunner and commandRunnerWithProgress. Also, take advantage of default arguments for ProgressOptions. And updates changelog. --- extensions/ql-vscode/CHANGELOG.md | 4 +- extensions/ql-vscode/src/astViewer.ts | 1 - .../src/contextual/templateProvider.ts | 4 +- extensions/ql-vscode/src/databases-ui.ts | 36 +++----- extensions/ql-vscode/src/extension.ts | 31 +++---- extensions/ql-vscode/src/helpers.ts | 92 ++++++++++++++----- extensions/ql-vscode/src/interface-utils.ts | 54 +++++------ 7 files changed, 122 insertions(+), 100 deletions(-) diff --git a/extensions/ql-vscode/CHANGELOG.md b/extensions/ql-vscode/CHANGELOG.md index 261200834b6..d0ce2822121 100644 --- a/extensions/ql-vscode/CHANGELOG.md +++ b/extensions/ql-vscode/CHANGELOG.md @@ -4,11 +4,9 @@ - Add friendly welcome message when the databases view is empty. - Add open query, open results, and remove query commands in the query history view title bar. -- Max number of simultaneous queries launchable by runQueries command is now configurable by changing the `codeQL.runningQueries.maxQueries` setting. +- The maximum number of simultaneous queries launchable by the `CodeQL: Run Queries in Selected Files` command is now configurable by changing the `codeQL.runningQueries.maxQueries` setting. - Allow simultaneously run queries to be canceled in a single-click. - Prevent multiple upgrade dialogs from appearing when running simultaneous queries on upgradeable databases. -- Allow simultaneously run queries to be canceled in a single-click. -- Max number of simultaneous queries launchable by runQueries command is now configurable by changing the codeQL.runningQueries.maxQueries setting. - Fix sorting of results. Some pages of results would have the wrong sort order and columns. - Remember previous sort order when reloading query results. - Fix proper escaping of backslashes in SARIF message strings. diff --git a/extensions/ql-vscode/src/astViewer.ts b/extensions/ql-vscode/src/astViewer.ts index bab2987c86c..f7bb43b9bfc 100644 --- a/extensions/ql-vscode/src/astViewer.ts +++ b/extensions/ql-vscode/src/astViewer.ts @@ -20,7 +20,6 @@ import { showLocation } from './interface-utils'; import { isStringLoc, isWholeFileLoc, isLineColumnLoc } from './bqrs-utils'; import { commandRunner } from './helpers'; - export interface AstItem { id: BqrsId; label?: string; diff --git a/extensions/ql-vscode/src/contextual/templateProvider.ts b/extensions/ql-vscode/src/contextual/templateProvider.ts index 327a1e60569..b2d74499396 100644 --- a/extensions/ql-vscode/src/contextual/templateProvider.ts +++ b/extensions/ql-vscode/src/contextual/templateProvider.ts @@ -44,7 +44,7 @@ export class TemplateQueryDefinitionProvider implements vscode.DefinitionProvide } private async getDefinitions(uriString: string): Promise { - return await withProgress({ + return withProgress({ location: vscode.ProgressLocation.Notification, cancellable: true, title: 'Finding definitions' @@ -91,7 +91,7 @@ export class TemplateQueryReferenceProvider implements vscode.ReferenceProvider } private async getReferences(uriString: string): Promise { - return await withProgress({ + return withProgress({ location: vscode.ProgressLocation.Notification, cancellable: true, title: 'Finding references' diff --git a/extensions/ql-vscode/src/databases-ui.ts b/extensions/ql-vscode/src/databases-ui.ts index 6e641d843dd..f8896e760f8 100644 --- a/extensions/ql-vscode/src/databases-ui.ts +++ b/extensions/ql-vscode/src/databases-ui.ts @@ -9,8 +9,7 @@ import { TreeItem, Uri, window, - env, - ProgressLocation + env } from 'vscode'; import * as fs from 'fs-extra'; @@ -23,6 +22,7 @@ import { } from './databases'; import { commandRunner, + commandRunnerWithProgress, getOnDiskWorkspaceFolders, ProgressCallback, showAndLogErrorMessage @@ -233,79 +233,66 @@ export class DatabaseUI extends DisposableObject { logger.log('Registering database panel commands.'); ctx.subscriptions.push( - commandRunner( + commandRunnerWithProgress( 'codeQL.setCurrentDatabase', this.handleSetCurrentDatabase, { - location: ProgressLocation.Notification, title: 'Importing database from archive', - cancellable: false, } ) ); ctx.subscriptions.push( - commandRunner( + commandRunnerWithProgress( 'codeQL.upgradeCurrentDatabase', this.handleUpgradeCurrentDatabase, { - location: ProgressLocation.Notification, title: 'Upgrading current database', cancellable: true, } ) ); ctx.subscriptions.push( - commandRunner( + commandRunnerWithProgress( 'codeQL.clearCache', this.handleClearCache, { - location: ProgressLocation.Notification, title: 'Clearing Cache', - cancellable: false, }) ); ctx.subscriptions.push( - commandRunner( + commandRunnerWithProgress( 'codeQLDatabases.chooseDatabaseFolder', this.handleChooseDatabaseFolder, { - location: ProgressLocation.Notification, title: 'Adding database from folder', - cancellable: false, } ) ); ctx.subscriptions.push( - commandRunner( + commandRunnerWithProgress( 'codeQLDatabases.chooseDatabaseArchive', this.handleChooseDatabaseArchive, { - location: ProgressLocation.Notification, title: 'Adding database from archive', - cancellable: false, } ) ); ctx.subscriptions.push( - commandRunner( + commandRunnerWithProgress( 'codeQLDatabases.chooseDatabaseInternet', this.handleChooseDatabaseInternet, { - location: ProgressLocation.Notification, title: 'Adding database from URL', - cancellable: false, } ) ); ctx.subscriptions.push( - commandRunner( + commandRunnerWithProgress( 'codeQLDatabases.chooseDatabaseLgtm', this.handleChooseDatabaseLgtm, { - location: ProgressLocation.Notification, title: 'Adding database from LGTM', - cancellable: false, }) ); ctx.subscriptions.push( @@ -333,11 +320,10 @@ export class DatabaseUI extends DisposableObject { ) ); ctx.subscriptions.push( - commandRunner( + commandRunnerWithProgress( 'codeQLDatabases.upgradeDatabase', this.handleUpgradeDatabase, { - location: ProgressLocation.Notification, title: 'Upgrading database', cancellable: true, } @@ -522,9 +508,9 @@ export class DatabaseUI extends DisposableObject { }; private handleSetCurrentDatabase = async ( - uri: Uri, progress: ProgressCallback, token: CancellationToken, + uri: Uri, ): Promise => { try { // Assume user has selected an archive if the file has a .zip extension diff --git a/extensions/ql-vscode/src/extension.ts b/extensions/ql-vscode/src/extension.ts index f03db59c6b7..c0f3d153c52 100644 --- a/extensions/ql-vscode/src/extension.ts +++ b/extensions/ql-vscode/src/extension.ts @@ -165,12 +165,10 @@ export async function activate(ctx: ExtensionContext): Promise { } } else { const progressOptions: ProgressOptions = { - location: ProgressLocation.Notification, title: progressTitle, - cancellable: false, + location: ProgressLocation.Notification, }; - // Avoid using commandRunner here because this function is called upon extension activation await helpers.withProgress(progressOptions, progress => distributionManager.installExtensionManagedDistributionRelease(result.updatedRelease, progress)); @@ -459,7 +457,7 @@ async function activateWithInstalledDistribution( logger.log('Registering top-level command palette commands.'); ctx.subscriptions.push( - helpers.commandRunner( + helpers.commandRunnerWithProgress( 'codeQL.runQuery', async ( progress: helpers.ProgressCallback, @@ -467,14 +465,13 @@ async function activateWithInstalledDistribution( uri: Uri | undefined ) => await compileAndRunQuery(false, uri, progress, token), { - location: ProgressLocation.Notification, title: 'Running query', cancellable: true } ) ); ctx.subscriptions.push( - helpers.commandRunner( + helpers.commandRunnerWithProgress( 'codeQL.runQueries', async ( progress: helpers.ProgressCallback, @@ -533,13 +530,12 @@ async function activateWithInstalledDistribution( )); }, { - location: ProgressLocation.Notification, title: 'Running queries', cancellable: true }) ); ctx.subscriptions.push( - helpers.commandRunner( + helpers.commandRunnerWithProgress( 'codeQL.quickEval', async ( progress: helpers.ProgressCallback, @@ -547,17 +543,19 @@ async function activateWithInstalledDistribution( uri: Uri | undefined ) => await compileAndRunQuery(true, uri, progress, token), { - location: ProgressLocation.Notification, title: 'Running query', cancellable: true }) ); ctx.subscriptions.push( - helpers.commandRunner('codeQL.quickQuery', async ( + helpers.commandRunnerWithProgress('codeQL.quickQuery', async ( progress: helpers.ProgressCallback, token: CancellationToken ) => - displayQuickQuery(ctx, cliServer, databaseUI, progress, token) + displayQuickQuery(ctx, cliServer, databaseUI, progress, token), + { + title: 'Run Quick Query' + } ) ); @@ -586,28 +584,24 @@ async function activateWithInstalledDistribution( ) ); ctx.subscriptions.push( - helpers.commandRunner('codeQL.chooseDatabaseLgtm', ( + helpers.commandRunnerWithProgress('codeQL.chooseDatabaseLgtm', ( progress: helpers.ProgressCallback, token: CancellationToken ) => databaseUI.handleChooseDatabaseLgtm(progress, token), { - location: ProgressLocation.Notification, title: 'Adding database from LGTM', - cancellable: false, }) ); ctx.subscriptions.push( - helpers.commandRunner('codeQL.chooseDatabaseInternet', ( + helpers.commandRunnerWithProgress('codeQL.chooseDatabaseInternet', ( progress: helpers.ProgressCallback, token: CancellationToken ) => databaseUI.handleChooseDatabaseInternet(progress, token), { - location: ProgressLocation.Notification, title: 'Adding database from URL', - cancellable: false, }) ); @@ -627,7 +621,7 @@ async function activateWithInstalledDistribution( ); const astViewer = new AstViewer(ctx); - ctx.subscriptions.push(helpers.commandRunner('codeQL.viewAst', async ( + ctx.subscriptions.push(helpers.commandRunnerWithProgress('codeQL.viewAst', async ( progress: helpers.ProgressCallback, token: CancellationToken ) => { @@ -637,7 +631,6 @@ async function activateWithInstalledDistribution( astViewer.updateRoots(await ast.getRoots(), ast.db, ast.fileName); } }, { - location: ProgressLocation.Notification, cancellable: true, title: 'Calculate AST' })); diff --git a/extensions/ql-vscode/src/helpers.ts b/extensions/ql-vscode/src/helpers.ts index 8758cf8ddfd..e8ac27ba1b9 100644 --- a/extensions/ql-vscode/src/helpers.ts +++ b/extensions/ql-vscode/src/helpers.ts @@ -9,7 +9,8 @@ import { window as Window, workspace, commands, - Disposable + Disposable, + ProgressLocation } from 'vscode'; import { CodeQLCliServer } from './cli'; import { logger } from './logging'; @@ -41,12 +42,35 @@ export interface ProgressUpdate { export type ProgressCallback = (p: ProgressUpdate) => void; +/** + * A task that handles command invocations from `commandRunner` + * and includes a progress monitor. + * + * + * Arguments passed to the command handler are passed along, + * untouched to this `ProgressTask` instance. + * + * @param progress a progress handler function. Call this + * function with a `ProgressUpdate` instance in order to + * denote some progress being achieved on this task. + * @param token a cencellation token + * @param args arguments passed to this task passed on from + * `commands.registerCommand`. + */ export type ProgressTask = ( progress: ProgressCallback, token: CancellationToken, ...args: any[] ) => Thenable; +/** + * A task that handles command invocations from `commandRunner`. + * Arguments passed to the command handler are passed along, + * untouched to this `NoProgressTask` instance. + * + * @param args arguments passed to this task passed on from + * `commands.registerCommand`. + */ type NoProgressTask = ((...args: any[]) => Promise); /** @@ -83,36 +107,58 @@ export function withProgress( } /** - * A generic wrapper for commands. This wrapper adds error handling and progress monitoring - * for any command. + * A generic wrapper for command registration. This wrapper adds uniform error handling for commands. * - * There are two ways to invoke the command task: with or without a progress monitor - * If progressOptions are passed in, then the command task will run with a progress monitor - * Otherwise, no progress monitor will be used. - * - * If a task is run with a progress monitor, the first two arguments to the task are always - * the progress callback, and the cancellation token. And this is followed by any extra command arguments - * (eg- selection, multiselection, ...). - * - * If there is no progress monitor, then only extra command arguments are passed in. + * In this variant of the command runner, no progress monitor is used. * * @param commandId The ID of the command to register. - * @param task The task to run. If passing taskOptions, then this task must be a `ProgressTask`. - * @param progressOptions Optional argument. If present, then the task is run with a progress monitor - * and cancellation token, otherwise it is run with no arguments. + * @param task The task to run. It is passed directly to `commands.registerCommand`. Any + * arguments to the command handler are passed on to the task. */ -export function commandRunner( +export function commandRunner( commandId: string, - task: ProgressTask | NoProgressTask, - progressOptions?: ProgressOptions + task: NoProgressTask, ): Disposable { return commands.registerCommand(commandId, async (...args: any[]) => { try { - if (progressOptions) { - await withProgress(progressOptions, task as ProgressTask, ...args); + await task(...args); + } catch (e) { + if (e instanceof UserCancellationException) { + // User has cancelled this action manually + if (e.silent) { + logger.log(e.message); + } else { + showAndLogWarningMessage(e.message); + } } else { - await (task as ((...args: any[]) => Promise))(...args); + showAndLogErrorMessage(e.message || e); } + } + }); +} + +/** + * A generic wrapper for command registration. This wrapper adds uniform error handling, + * progress monitoring, and cancellation for commands. + * + * @param commandId The ID of the command to register. + * @param task The task to run. It is passed directly to `commands.registerCommand`. Any + * arguments to the command handler are passed on to the task after the progress callback + * and cancellation token. + * @param progressOptions Progress options to be sent to the progress monitor. + */ +export function commandRunnerWithProgress( + commandId: string, + task: ProgressTask, + progressOptions: Partial +): Disposable { + return commands.registerCommand(commandId, async (...args: any[]) => { + const progressOptionsWithDefaults = { + location: ProgressLocation.Notification, + ...progressOptions + }; + try { + await withProgress(progressOptionsWithDefaults, task, ...args); } catch (e) { if (e instanceof UserCancellationException) { // User has cancelled this action manually @@ -121,10 +167,8 @@ export function commandRunner( } else { showAndLogWarningMessage(e.message); } - } else if (e instanceof Error) { - showAndLogErrorMessage(e.message); } else { - throw e; + showAndLogErrorMessage(e.message || e); } } }); diff --git a/extensions/ql-vscode/src/interface-utils.ts b/extensions/ql-vscode/src/interface-utils.ts index ef02aff9bc8..b0cff72221d 100644 --- a/extensions/ql-vscode/src/interface-utils.ts +++ b/extensions/ql-vscode/src/interface-utils.ts @@ -151,33 +151,35 @@ export async function showResolvableLocation( } export async function showLocation(location?: Location) { - if (location) { - const doc = await workspace.openTextDocument(location.uri); - const editorsWithDoc = Window.visibleTextEditors.filter( - (e) => e.document === doc - ); - const editor = - editorsWithDoc.length > 0 - ? editorsWithDoc[0] - : await Window.showTextDocument(doc, ViewColumn.One); - const range = location.range; - // When highlighting the range, vscode's occurrence-match and bracket-match highlighting will - // trigger based on where we place the cursor/selection, and will compete for the user's attention. - // For reference: - // - Occurences are highlighted when the cursor is next to or inside a word or a whole word is selected. - // - Brackets are highlighted when the cursor is next to a bracket and there is an empty selection. - // - Multi-line selections explicitly highlight line-break characters, but multi-line decorators do not. - // - // For single-line ranges, select the whole range, mainly to disable bracket highlighting. - // For multi-line ranges, place the cursor at the beginning to avoid visual artifacts from selected line-breaks. - // Multi-line ranges are usually large enough to overshadow the noise from bracket highlighting. - const selectionEnd = - range.start.line === range.end.line ? range.end : range.start; - editor.selection = new Selection(range.start, selectionEnd); - editor.revealRange(range, TextEditorRevealType.InCenter); - editor.setDecorations(shownLocationDecoration, [range]); - editor.setDecorations(shownLocationLineDecoration, [range]); + if (!location) { + return; } + + const doc = await workspace.openTextDocument(location.uri); + const editorsWithDoc = Window.visibleTextEditors.filter( + (e) => e.document === doc + ); + const editor = + editorsWithDoc.length > 0 + ? editorsWithDoc[0] + : await Window.showTextDocument(doc, ViewColumn.One); + const range = location.range; + // When highlighting the range, vscode's occurrence-match and bracket-match highlighting will + // trigger based on where we place the cursor/selection, and will compete for the user's attention. + // For reference: + // - Occurences are highlighted when the cursor is next to or inside a word or a whole word is selected. + // - Brackets are highlighted when the cursor is next to a bracket and there is an empty selection. + // - Multi-line selections explicitly highlight line-break characters, but multi-line decorators do not. + // + // For single-line ranges, select the whole range, mainly to disable bracket highlighting. + // For multi-line ranges, place the cursor at the beginning to avoid visual artifacts from selected line-breaks. + // Multi-line ranges are usually large enough to overshadow the noise from bracket highlighting. + const selectionEnd = + range.start.line === range.end.line ? range.end : range.start; + editor.selection = new Selection(range.start, selectionEnd); + editor.revealRange(range, TextEditorRevealType.InCenter); + editor.setDecorations(shownLocationDecoration, [range]); + editor.setDecorations(shownLocationLineDecoration, [range]); } const findMatchBackground = new ThemeColor('editor.findMatchBackground');