diff --git a/extensions/ql-vscode/src/local-queries/local-queries.ts b/extensions/ql-vscode/src/local-queries/local-queries.ts index 82f0b279c80..c88afbd6649 100644 --- a/extensions/ql-vscode/src/local-queries/local-queries.ts +++ b/extensions/ql-vscode/src/local-queries/local-queries.ts @@ -338,7 +338,7 @@ export class LocalQueries extends DisposableObject { this.cliServer, progress, credentials, - this.app.logger, + this.app, this.databaseManager, contextStoragePath, this.selectedQueryTreeViewItems, diff --git a/extensions/ql-vscode/src/local-queries/skeleton-query-wizard.ts b/extensions/ql-vscode/src/local-queries/skeleton-query-wizard.ts index ade942f910f..9669bacaca6 100644 --- a/extensions/ql-vscode/src/local-queries/skeleton-query-wizard.ts +++ b/extensions/ql-vscode/src/local-queries/skeleton-query-wizard.ts @@ -1,16 +1,20 @@ import { basename, dirname, join } from "path"; -import { Uri, window as Window, workspace } from "vscode"; +import { Uri, window, window as Window, workspace } from "vscode"; import { CodeQLCliServer } from "../codeql-cli/cli"; -import { BaseLogger } from "../common/logging"; +import { showAndLogExceptionWithTelemetry } from "../common/logging"; import { Credentials } from "../common/authentication"; -import { QueryLanguage } from "../common/query-language"; +import { + getLanguageDisplayName, + QueryLanguage, +} from "../common/query-language"; import { getFirstWorkspaceFolder } from "../common/vscode/workspace-folders"; -import { getErrorMessage } from "../common/helpers-pure"; +import { asError, getErrorMessage } from "../common/helpers-pure"; import { QlPackGenerator } from "./qlpack-generator"; import { DatabaseItem, DatabaseManager } from "../databases/local-databases"; import { ProgressCallback, UserCancellationException, + withProgress, } from "../common/vscode/progress"; import { askForGitHubRepo, @@ -23,6 +27,9 @@ import { } from "../config"; import { lstat, pathExists } from "fs-extra"; import { askForLanguage } from "../codeql-cli/query-language"; +import { showInformationMessageWithAction } from "../common/vscode/dialog"; +import { redactableError } from "../common/errors"; +import { App } from "../common/app"; import { QueryTreeViewItem } from "../queries-panel/query-tree-view-item"; type QueryLanguagesToDatabaseMap = Record; @@ -41,12 +48,13 @@ export const QUERY_LANGUAGE_TO_DATABASE_REPO: QueryLanguagesToDatabaseMap = { export class SkeletonQueryWizard { private fileName = "example.ql"; private qlPackStoragePath: string | undefined; + private downloadPromise: Promise | undefined; constructor( private readonly cliServer: CodeQLCliServer, private readonly progress: ProgressCallback, private readonly credentials: Credentials | undefined, - private readonly logger: BaseLogger, + private readonly app: App, private readonly databaseManager: DatabaseManager, private readonly databaseStoragePath: string | undefined, private readonly selectedItems: readonly QueryTreeViewItem[], @@ -57,6 +65,16 @@ export class SkeletonQueryWizard { return `codeql-custom-queries-${this.language}`; } + /** + * Wait for the download process to complete by waiting for the user to select + * either "Download database" or closing the dialog. This is used for testing. + */ + public async waitForDownload() { + if (this.downloadPromise) { + await this.downloadPromise; + } + } + public async execute() { if (!this.language) { // show quick pick to choose language @@ -85,7 +103,7 @@ export class SkeletonQueryWizard { try { await this.openExampleFile(); } catch (e: unknown) { - void this.logger.log( + void this.app.logger.log( `Could not open example query file: ${getErrorMessage(e)}`, ); } @@ -104,7 +122,9 @@ export class SkeletonQueryWizard { ); void workspace.openTextDocument(queryFileUri).then((doc) => { - void Window.showTextDocument(doc); + void Window.showTextDocument(doc, { + preview: false, + }); }); } @@ -208,7 +228,7 @@ export class SkeletonQueryWizard { await qlPackGenerator.generate(); } catch (e: unknown) { - void this.logger.log( + void this.app.logger.log( `Could not create skeleton QL pack: ${getErrorMessage(e)}`, ); } @@ -240,7 +260,7 @@ export class SkeletonQueryWizard { this.fileName = await this.determineNextFileName(this.folderName); await qlPackGenerator.createExampleQlFile(this.fileName); } catch (e: unknown) { - void this.logger.log( + void this.app.logger.log( `Could not create query example file: ${getErrorMessage(e)}`, ); } @@ -260,7 +280,47 @@ export class SkeletonQueryWizard { return `example${qlFiles.length + 1}.ql`; } - private async downloadDatabase() { + private async promptDownloadDatabase() { + if (this.qlPackStoragePath === undefined) { + throw new Error("QL Pack storage path is undefined"); + } + + if (this.language === undefined) { + throw new Error("Language is undefined"); + } + + const openFileLink = this.openFileMarkdownLink; + + const displayLanguage = getLanguageDisplayName(this.language); + const action = await showInformationMessageWithAction( + `New CodeQL query for ${displayLanguage} ${openFileLink} created, but no CodeQL databases for ${displayLanguage} were detected in your workspace. Would you like to download a CodeQL database for ${displayLanguage} to analyze with ${openFileLink}?`, + "Download database", + ); + + if (action) { + void withProgress(async (progress) => { + try { + await this.downloadDatabase(progress); + } catch (e: unknown) { + if (e instanceof UserCancellationException) { + return; + } + + void showAndLogExceptionWithTelemetry( + this.app.logger, + this.app.telemetry, + redactableError( + asError(e), + )`An error occurred while downloading the GitHub repository: ${getErrorMessage( + e, + )}`, + ); + } + }); + } + } + + private async downloadDatabase(progress: ProgressCallback) { if (this.qlPackStoragePath === undefined) { throw new Error("QL Pack storage path is undefined"); } @@ -273,10 +333,10 @@ export class SkeletonQueryWizard { throw new Error("Language is undefined"); } - this.progress({ + progress({ message: "Downloading database", - step: 3, - maxStep: 3, + step: 1, + maxStep: 2, }); const githubRepoNwo = QUERY_LANGUAGE_TO_DATABASE_REPO[this.language]; @@ -291,7 +351,7 @@ export class SkeletonQueryWizard { this.databaseManager, this.databaseStoragePath, this.credentials, - this.progress, + progress, this.cliServer, this.language, ); @@ -313,14 +373,42 @@ export class SkeletonQueryWizard { ); if (existingDatabaseItem) { - // select the found database - await this.databaseManager.setCurrentDatabaseItem(existingDatabaseItem); + const openFileLink = this.openFileMarkdownLink; + + if (this.databaseManager.currentDatabaseItem !== existingDatabaseItem) { + // select the found database + await this.databaseManager.setCurrentDatabaseItem(existingDatabaseItem); + + const displayLanguage = getLanguageDisplayName(this.language); + void window.showInformationMessage( + `New CodeQL query for ${displayLanguage} ${openFileLink} created. We have automatically selected your existing CodeQL ${displayLanguage} database ${existingDatabaseItem.name} for you to analyze with ${openFileLink}.`, + ); + } } else { // download new database and select it - await this.downloadDatabase(); + this.downloadPromise = this.promptDownloadDatabase().finally(() => { + this.downloadPromise = undefined; + }); } } + private get openFileMarkdownLink() { + if (this.qlPackStoragePath === undefined) { + throw new Error("QL Pack storage path is undefined"); + } + + const queryPath = join( + this.qlPackStoragePath, + this.folderName, + this.fileName, + ); + const queryPathUri = Uri.file(queryPath); + + const openFileArgs = [queryPathUri.toString(true)]; + const queryString = encodeURI(JSON.stringify(openFileArgs)); + return `[${this.fileName}](command:vscode.open?${queryString})`; + } + public static async findDatabaseItemByNwo( language: string, databaseNwo: string, diff --git a/extensions/ql-vscode/test/__mocks__/appMock.ts b/extensions/ql-vscode/test/__mocks__/appMock.ts index 797ea3d13ce..89f5cd534af 100644 --- a/extensions/ql-vscode/test/__mocks__/appMock.ts +++ b/extensions/ql-vscode/test/__mocks__/appMock.ts @@ -34,7 +34,7 @@ export function createMockApp({ environment?: EnvironmentContext; logger?: NotificationLogger; telemetry?: AppTelemetry; -}): App { +} = {}): App { return { mode: AppMode.Test, logger, diff --git a/extensions/ql-vscode/test/vscode-tests/cli-integration/local-queries/skeleton-query-wizard.test.ts b/extensions/ql-vscode/test/vscode-tests/cli-integration/local-queries/skeleton-query-wizard.test.ts index 9526e61cd86..84aa8fbc24f 100644 --- a/extensions/ql-vscode/test/vscode-tests/cli-integration/local-queries/skeleton-query-wizard.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/cli-integration/local-queries/skeleton-query-wizard.test.ts @@ -5,8 +5,13 @@ import { } from "../../../../src/local-queries/skeleton-query-wizard"; import { mockedObject, mockedQuickPickItem } from "../../utils/mocking.helpers"; import * as tmp from "tmp"; -import { TextDocument, window, workspace, WorkspaceFolder } from "vscode"; -import { extLogger } from "../../../../src/common/logging/vscode"; +import { + MessageItem, + TextDocument, + window, + workspace, + WorkspaceFolder, +} from "vscode"; import { QlPackGenerator } from "../../../../src/local-queries/qlpack-generator"; import { createFileSync, @@ -27,6 +32,8 @@ import { createMockDB } from "../../../factories/databases/databases"; import { asError } from "../../../../src/common/helpers-pure"; import { Setting } from "../../../../src/config"; import { QueryLanguage } from "../../../../src/common/query-language"; +import { App } from "../../../../src/common/app"; +import { createMockApp } from "../../../__mocks__/appMock"; import { createQueryTreeFileItem, createQueryTreeFolderItem, @@ -35,12 +42,16 @@ import { describe("SkeletonQueryWizard", () => { let mockCli: CodeQLCliServer; + let mockApp: App; let wizard: SkeletonQueryWizard; let mockDatabaseManager: DatabaseManager; let dir: tmp.DirResult; let storagePath: string; let quickPickSpy: jest.SpiedFunction; let showInputBoxSpy: jest.SpiedFunction; + let showInformationMessageSpy: jest.SpiedFunction< + typeof window.showInformationMessage + >; let generateSpy: jest.SpiedFunction< typeof QlPackGenerator.prototype.generate >; @@ -61,8 +72,6 @@ describe("SkeletonQueryWizard", () => { const chosenLanguage = "ruby"; const selectedItems: QueryTreeViewItem[] = []; - jest.spyOn(extLogger, "log").mockResolvedValue(undefined); - beforeEach(async () => { mockCli = mockedObject({ resolveLanguages: jest @@ -78,6 +87,7 @@ describe("SkeletonQueryWizard", () => { ]), getSupportedLanguages: jest.fn(), }); + mockApp = createMockApp(); mockDatabaseManager = mockedObject({ setCurrentDatabaseItem: jest.fn(), @@ -108,6 +118,9 @@ describe("SkeletonQueryWizard", () => { showInputBoxSpy = jest .spyOn(window, "showInputBox") .mockResolvedValue(storagePath); + showInformationMessageSpy = jest + .spyOn(window, "showInformationMessage") + .mockResolvedValue(undefined); generateSpy = jest .spyOn(QlPackGenerator.prototype, "generate") .mockResolvedValue(undefined); @@ -125,7 +138,7 @@ describe("SkeletonQueryWizard", () => { mockCli, jest.fn(), credentials, - extLogger, + mockApp, mockDatabaseManager, storagePath, selectedItems, @@ -153,7 +166,7 @@ describe("SkeletonQueryWizard", () => { mockCli, jest.fn(), credentials, - extLogger, + mockApp, mockDatabaseManager, storagePath, selectedItems, @@ -176,8 +189,31 @@ describe("SkeletonQueryWizard", () => { expect(generateSpy).toHaveBeenCalled(); }); - it("should download database for selected language", async () => { + it("should prompt for download database", async () => { + await wizard.execute(); + + expect(showInformationMessageSpy).toHaveBeenCalledWith( + expect.stringMatching(/a CodeQL database/i), + expect.objectContaining({ + title: "Download database", + }), + ); + expect(downloadGitHubDatabaseSpy).not.toHaveBeenCalled(); + }); + + it("should download database for selected language when selecting download in prompt", async () => { + showInformationMessageSpy.mockImplementation( + async (_message, options, item) => { + if (item === undefined) { + return options as MessageItem; + } + + return item; + }, + ); + await wizard.execute(); + await wizard.waitForDownload(); expect(downloadGitHubDatabaseSpy).toHaveBeenCalled(); }); @@ -263,52 +299,128 @@ describe("SkeletonQueryWizard", () => { name: databaseNwo, language: chosenLanguage, } as DatabaseItem; + }); + + describe("with database selected", () => { + beforeEach(async () => { + mockDatabaseManagerWithItems = mockedObject({ + currentDatabaseItem: databaseItem, + setCurrentDatabaseItem: jest.fn(), + databaseItems: [databaseItem] as DatabaseItem[], + }); + + wizard = new SkeletonQueryWizard( + mockCli, + jest.fn(), + credentials, + mockApp, + mockDatabaseManagerWithItems, + storagePath, + selectedItems, + ); + }); + + it("should not download a new database for language", async () => { + await wizard.execute(); - mockDatabaseManagerWithItems = mockedObject({ - setCurrentDatabaseItem: jest.fn(), - databaseItems: [databaseItem] as DatabaseItem[], + expect(downloadGitHubDatabaseSpy).not.toHaveBeenCalled(); }); - wizard = new SkeletonQueryWizard( - mockCli, - jest.fn(), - credentials, - extLogger, - mockDatabaseManagerWithItems, - storagePath, - selectedItems, - ); - }); + it("should not select the database", async () => { + await wizard.execute(); - it("should not download a new database for language", async () => { - await wizard.execute(); + expect( + mockDatabaseManagerWithItems.setCurrentDatabaseItem, + ).not.toHaveBeenCalled(); + }); - expect(downloadGitHubDatabaseSpy).not.toHaveBeenCalled(); - }); + it("should open the new query file", async () => { + await wizard.execute(); - it("should select an existing database", async () => { - await wizard.execute(); + expect(openTextDocumentSpy).toHaveBeenCalledWith( + expect.objectContaining({ + path: expect.stringMatching("example2.ql"), + }), + ); + }); + + it("should not show an information message", async () => { + await wizard.execute(); - expect( - mockDatabaseManagerWithItems.setCurrentDatabaseItem, - ).toHaveBeenCalledWith(databaseItem); + expect(showInformationMessageSpy).not.toHaveBeenCalled(); + }); }); - it("should open the new query file", async () => { - await wizard.execute(); + describe("with database not selected", () => { + beforeEach(async () => { + mockDatabaseManagerWithItems = mockedObject({ + currentDatabaseItem: undefined, + setCurrentDatabaseItem: jest.fn(), + databaseItems: [databaseItem] as DatabaseItem[], + }); - expect(openTextDocumentSpy).toHaveBeenCalledWith( - expect.objectContaining({ - path: expect.stringMatching("example2.ql"), - }), - ); + wizard = new SkeletonQueryWizard( + mockCli, + jest.fn(), + credentials, + mockApp, + mockDatabaseManagerWithItems, + storagePath, + selectedItems, + ); + }); + + it("should not download a new database for language", async () => { + await wizard.execute(); + + expect(downloadGitHubDatabaseSpy).not.toHaveBeenCalled(); + }); + + it("should select an existing database", async () => { + await wizard.execute(); + + expect( + mockDatabaseManagerWithItems.setCurrentDatabaseItem, + ).toHaveBeenCalledWith(databaseItem); + }); + + it("should open the new query file", async () => { + await wizard.execute(); + + expect(openTextDocumentSpy).toHaveBeenCalledWith( + expect.objectContaining({ + path: expect.stringMatching("example2.ql"), + }), + ); + }); + + it("should show an information message", async () => { + await wizard.execute(); + + expect(showInformationMessageSpy).toHaveBeenCalledWith( + expect.stringMatching(new RegExp(databaseNwo)), + ); + }); }); }); describe("if database is missing", () => { - describe("if the user choses to downloaded the suggested database from GitHub", () => { + describe("if the user chooses to downloaded the suggested database from GitHub", () => { + beforeEach(() => { + showInformationMessageSpy.mockImplementation( + async (_message, options, item) => { + if (item === undefined) { + return options as MessageItem; + } + + return item; + }, + ); + }); + it("should download a new database for language", async () => { await wizard.execute(); + await wizard.waitForDownload(); expect(askForGitHubRepoSpy).toHaveBeenCalled(); expect(downloadGitHubDatabaseSpy).toHaveBeenCalled(); @@ -317,6 +429,16 @@ describe("SkeletonQueryWizard", () => { describe("if the user choses to download a different database from GitHub than the one suggested", () => { beforeEach(() => { + showInformationMessageSpy.mockImplementation( + async (_message, options, item) => { + if (item === undefined) { + return options as MessageItem; + } + + return item; + }, + ); + const chosenGitHubRepo = "pickles-owner/pickles-repo"; askForGitHubRepoSpy = jest @@ -326,6 +448,7 @@ describe("SkeletonQueryWizard", () => { it("should download the newly chosen database", async () => { await wizard.execute(); + await wizard.waitForDownload(); expect(askForGitHubRepoSpy).toHaveBeenCalled(); expect(downloadGitHubDatabaseSpy).toHaveBeenCalled(); @@ -459,7 +582,7 @@ describe("SkeletonQueryWizard", () => { mockCli, jest.fn(), credentials, - extLogger, + mockApp, mockDatabaseManager, storagePath, selectedItems, @@ -489,7 +612,7 @@ describe("SkeletonQueryWizard", () => { mockCli, jest.fn(), credentials, - extLogger, + mockApp, mockDatabaseManager, storagePath, selectedItems, @@ -523,7 +646,7 @@ describe("SkeletonQueryWizard", () => { mockCli, jest.fn(), credentials, - extLogger, + mockApp, mockDatabaseManager, storagePath, selectedItems, @@ -565,7 +688,7 @@ describe("SkeletonQueryWizard", () => { mockCli, jest.fn(), credentials, - extLogger, + mockApp, mockDatabaseManager, storagePath, selectedItems,