From 57d48a7b046b29e8334833e928a547dfb7b1929e Mon Sep 17 00:00:00 2001 From: Elena Tanasoiu Date: Mon, 17 Apr 2023 14:45:29 +0000 Subject: [PATCH 1/5] Prompt non-codespace users for storage path Offer non-codespace users the option to configure their storage folder for skeleton packs. Suggested here: https://github.com/github/vscode-codeql/pull/2310#issuecomment-1507428217 At the moment we're choosing to create our skeleton packs in the first folder in the workspace. This is fine for the codespace template because we can control the folder structure in that repo. For users outside of this we'd like to offer them the option to choose where to save their skeleton packs. --- extensions/ql-vscode/src/config.ts | 16 +++ .../ql-vscode/src/skeleton-query-wizard.ts | 41 +++++- .../skeleton-query-wizard.test.ts | 117 ++++++++++++++++++ 3 files changed, 172 insertions(+), 2 deletions(-) diff --git a/extensions/ql-vscode/src/config.ts b/extensions/ql-vscode/src/config.ts index bd6268a88..adac56c6d 100644 --- a/extensions/ql-vscode/src/config.ts +++ b/extensions/ql-vscode/src/config.ts @@ -619,3 +619,19 @@ export const ALLOW_HTTP_SETTING = new Setting( export function allowHttp(): boolean { return ALLOW_HTTP_SETTING.getValue() || false; } + +/** + * The name of the folder where we want to create skeleton wizard QL packs. + **/ +const SKELETON_WIZARD_FOLDER = new Setting( + "folder", + new Setting("skeletonWizard", ROOT_SETTING), +); + +export function getSkeletonWizardFolder(): string | undefined { + return SKELETON_WIZARD_FOLDER.getValue() || undefined; +} + +export async function setSkeletonWizardFolder(folder: string | undefined) { + await SKELETON_WIZARD_FOLDER.updateValue(folder, ConfigurationTarget.Global); +} diff --git a/extensions/ql-vscode/src/skeleton-query-wizard.ts b/extensions/ql-vscode/src/skeleton-query-wizard.ts index 10f3dbf77..11542f1ff 100644 --- a/extensions/ql-vscode/src/skeleton-query-wizard.ts +++ b/extensions/ql-vscode/src/skeleton-query-wizard.ts @@ -14,7 +14,12 @@ import { QlPackGenerator } from "./qlpack-generator"; import { DatabaseItem, DatabaseManager } from "./local-databases"; import { ProgressCallback, UserCancellationException } from "./progress"; import { askForGitHubRepo, downloadGitHubDatabase } from "./databaseFetcher"; -import { existsSync } from "fs"; +import { + getSkeletonWizardFolder, + isCodespacesTemplate, + setSkeletonWizardFolder, +} from "./config"; +import { existsSync } from "fs-extra"; type QueryLanguagesToDatabaseMap = Record; @@ -55,7 +60,7 @@ export class SkeletonQueryWizard { return; } - this.qlPackStoragePath = getFirstWorkspaceFolder(); + this.qlPackStoragePath = await this.determineStoragePath(); const skeletonPackAlreadyExists = existsSync(join(this.qlPackStoragePath, this.folderName)) || @@ -98,6 +103,38 @@ export class SkeletonQueryWizard { }); } + public async determineStoragePath() { + const firstStorageFolder = getFirstWorkspaceFolder(); + + if (isCodespacesTemplate()) { + return firstStorageFolder; + } + + let storageFolder = getSkeletonWizardFolder(); + + if (storageFolder === undefined || !existsSync(storageFolder)) { + storageFolder = await Window.showInputBox({ + title: + "Please choose a folder in which to create your new query pack. You can change this in the extension settings.", + value: firstStorageFolder, + ignoreFocusOut: true, + }); + } + + if (storageFolder === undefined) { + throw new UserCancellationException("No storage folder entered."); + } + + if (!existsSync(storageFolder)) { + throw new UserCancellationException( + "Invalid folder. Must be a folder that already exists.", + ); + } + + await setSkeletonWizardFolder(storageFolder); + return storageFolder; + } + private async chooseLanguage() { this.progress({ message: "Choose language", diff --git a/extensions/ql-vscode/test/vscode-tests/cli-integration/skeleton-query-wizard.test.ts b/extensions/ql-vscode/test/vscode-tests/cli-integration/skeleton-query-wizard.test.ts index e101cbf17..e82b86c2d 100644 --- a/extensions/ql-vscode/test/vscode-tests/cli-integration/skeleton-query-wizard.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/cli-integration/skeleton-query-wizard.test.ts @@ -21,6 +21,7 @@ import { import * as databaseFetcher from "../../../src/databaseFetcher"; import { createMockDB } from "../../factories/databases/databases"; import { asError } from "../../../src/pure/helpers-pure"; +import { Setting } from "../../../src/config"; describe("SkeletonQueryWizard", () => { let mockCli: CodeQLCliServer; @@ -29,6 +30,7 @@ describe("SkeletonQueryWizard", () => { let dir: tmp.DirResult; let storagePath: string; let quickPickSpy: jest.SpiedFunction; + let showInputBoxSpy: jest.SpiedFunction; let generateSpy: jest.SpiedFunction< typeof QlPackGenerator.prototype.generate >; @@ -93,6 +95,9 @@ describe("SkeletonQueryWizard", () => { quickPickSpy = jest .spyOn(window, "showQuickPick") .mockResolvedValueOnce(mockedQuickPickItem(chosenLanguage)); + showInputBoxSpy = jest + .spyOn(window, "showInputBox") + .mockResolvedValue(storagePath); generateSpy = jest .spyOn(QlPackGenerator.prototype, "generate") .mockResolvedValue(undefined); @@ -433,4 +438,116 @@ describe("SkeletonQueryWizard", () => { }); }); }); + + describe("determineStoragePath", () => { + it("should prompt the user to provide a storage path", async () => { + const chosenPath = await wizard.determineStoragePath(); + + expect(showInputBoxSpy).toHaveBeenCalledWith( + expect.objectContaining({ value: storagePath }), + ); + expect(chosenPath).toEqual(storagePath); + }); + + it("should write the chosen folder to settings", async () => { + const updateValueSpy = jest.spyOn(Setting.prototype, "updateValue"); + + await wizard.determineStoragePath(); + + expect(updateValueSpy).toHaveBeenCalledWith(storagePath, 1); + }); + + describe("when the user is using the codespace template", () => { + let originalValue: any; + let storedPath: string; + + beforeEach(async () => { + storedPath = join(dir.name, "pickles-folder"); + ensureDirSync(storedPath); + + originalValue = workspace + .getConfiguration("codeQL.skeletonWizard") + .get("folder"); + + // Set isCodespacesTemplate to true to indicate we are in the codespace template + await workspace + .getConfiguration("codeQL") + .update("codespacesTemplate", true); + }); + + afterEach(async () => { + await workspace + .getConfiguration("codeQL") + .update("codespacesTemplate", originalValue); + }); + + it("should not prompt the user", async () => { + const chosenPath = await wizard.determineStoragePath(); + + expect(showInputBoxSpy).not.toHaveBeenCalled(); + expect(chosenPath).toEqual(storagePath); + }); + }); + + describe("when there is already a saved storage path in settings", () => { + describe("when the saved storage path exists", () => { + let originalValue: any; + let storedPath: string; + + beforeEach(async () => { + storedPath = join(dir.name, "pickles-folder"); + ensureDirSync(storedPath); + + originalValue = workspace + .getConfiguration("codeQL.skeletonWizard") + .get("folder"); + await workspace + .getConfiguration("codeQL.skeletonWizard") + .update("folder", storedPath); + }); + + afterEach(async () => { + await workspace + .getConfiguration("codeQL.skeletonWizard") + .update("folder", originalValue); + }); + + it("should return it and not prompt the user", async () => { + const chosenPath = await wizard.determineStoragePath(); + + expect(showInputBoxSpy).not.toHaveBeenCalled(); + expect(chosenPath).toEqual(storedPath); + }); + }); + + describe("when the saved storage path does not exist", () => { + let originalValue: any; + let storedPath: string; + + beforeEach(async () => { + storedPath = join(dir.name, "this-folder-does-not-exist"); + + originalValue = workspace + .getConfiguration("codeQL.skeletonWizard") + .get("folder"); + await workspace + .getConfiguration("codeQL.skeletonWizard") + .update("folder", storedPath); + }); + + afterEach(async () => { + await workspace + .getConfiguration("codeQL.skeletonWizard") + .update("folder", originalValue); + }); + + it("should prompt the user for to provide a new folder name", async () => { + const chosenPath = await wizard.determineStoragePath(); + + expect(showInputBoxSpy).toHaveBeenCalled(); + expect(chosenPath).toEqual(storagePath); + }); + }); + }); + }); }); From e0bf1ca40fa5b660040946e6beda86b7ee6f35fb Mon Sep 17 00:00:00 2001 From: Elena Tanasoiu Date: Tue, 18 Apr 2023 15:27:58 +0000 Subject: [PATCH 2/5] Rename `skeletonWizard` setting to `createQuery` --- extensions/ql-vscode/src/config.ts | 2 +- .../cli-integration/skeleton-query-wizard.test.ts | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/extensions/ql-vscode/src/config.ts b/extensions/ql-vscode/src/config.ts index adac56c6d..1239ec905 100644 --- a/extensions/ql-vscode/src/config.ts +++ b/extensions/ql-vscode/src/config.ts @@ -625,7 +625,7 @@ export function allowHttp(): boolean { **/ const SKELETON_WIZARD_FOLDER = new Setting( "folder", - new Setting("skeletonWizard", ROOT_SETTING), + new Setting("createQuery", ROOT_SETTING), ); export function getSkeletonWizardFolder(): string | undefined { diff --git a/extensions/ql-vscode/test/vscode-tests/cli-integration/skeleton-query-wizard.test.ts b/extensions/ql-vscode/test/vscode-tests/cli-integration/skeleton-query-wizard.test.ts index e82b86c2d..87fbbc1e5 100644 --- a/extensions/ql-vscode/test/vscode-tests/cli-integration/skeleton-query-wizard.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/cli-integration/skeleton-query-wizard.test.ts @@ -466,7 +466,7 @@ describe("SkeletonQueryWizard", () => { ensureDirSync(storedPath); originalValue = workspace - .getConfiguration("codeQL.skeletonWizard") + .getConfiguration("codeQL.createQuery") .get("folder"); // Set isCodespacesTemplate to true to indicate we are in the codespace template @@ -499,16 +499,16 @@ describe("SkeletonQueryWizard", () => { ensureDirSync(storedPath); originalValue = workspace - .getConfiguration("codeQL.skeletonWizard") + .getConfiguration("codeQL.createQuery") .get("folder"); await workspace - .getConfiguration("codeQL.skeletonWizard") + .getConfiguration("codeQL.createQuery") .update("folder", storedPath); }); afterEach(async () => { await workspace - .getConfiguration("codeQL.skeletonWizard") + .getConfiguration("codeQL.createQuery") .update("folder", originalValue); }); @@ -528,16 +528,16 @@ describe("SkeletonQueryWizard", () => { storedPath = join(dir.name, "this-folder-does-not-exist"); originalValue = workspace - .getConfiguration("codeQL.skeletonWizard") + .getConfiguration("codeQL.createQuery") .get("folder"); await workspace - .getConfiguration("codeQL.skeletonWizard") + .getConfiguration("codeQL.createQuery") .update("folder", storedPath); }); afterEach(async () => { await workspace - .getConfiguration("codeQL.skeletonWizard") + .getConfiguration("codeQL.createQuery") .update("folder", originalValue); }); From db35cb294da4fe4a6fab8952ecbccb51c362dc4d Mon Sep 17 00:00:00 2001 From: Elena Tanasoiu Date: Tue, 18 Apr 2023 15:30:21 +0000 Subject: [PATCH 3/5] Add new setting to package.json --- extensions/ql-vscode/package.json | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/extensions/ql-vscode/package.json b/extensions/ql-vscode/package.json index 9905eecbf..d15882443 100644 --- a/extensions/ql-vscode/package.json +++ b/extensions/ql-vscode/package.json @@ -340,6 +340,12 @@ "type": "boolean", "default": false, "description": "Allow database to be downloaded via HTTP. Warning: enabling this option will allow downloading from insecure servers." + }, + "codeQL.createQuery.Folder": { + "type": "string", + "default": "", + "patternErrorMessage": "Please enter a valid folder", + "markdownDescription": "The name of the folder where we want to create queries and query packs via the \"CodeQL: Create Query\" command. The folder should exist in the workspace.`)." } } }, From 5e76d0b1adc3b3dcb3efada289d8e43d2ea1f204 Mon Sep 17 00:00:00 2001 From: Elena Tanasoiu Date: Tue, 18 Apr 2023 17:27:36 +0100 Subject: [PATCH 4/5] Fix typos Co-authored-by: Shati Patel <42641846+shati-patel@users.noreply.github.com> --- extensions/ql-vscode/package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/extensions/ql-vscode/package.json b/extensions/ql-vscode/package.json index d15882443..c5f69ccff 100644 --- a/extensions/ql-vscode/package.json +++ b/extensions/ql-vscode/package.json @@ -341,11 +341,11 @@ "default": false, "description": "Allow database to be downloaded via HTTP. Warning: enabling this option will allow downloading from insecure servers." }, - "codeQL.createQuery.Folder": { + "codeQL.createQuery.folder": { "type": "string", "default": "", "patternErrorMessage": "Please enter a valid folder", - "markdownDescription": "The name of the folder where we want to create queries and query packs via the \"CodeQL: Create Query\" command. The folder should exist in the workspace.`)." + "markdownDescription": "The name of the folder where we want to create queries and query packs via the \"CodeQL: Create Query\" command. The folder should exist in the workspace." } } }, From 154fab845a9515f2ab1a7da237dbf7df577d57fc Mon Sep 17 00:00:00 2001 From: Elena Tanasoiu Date: Wed, 19 Apr 2023 07:56:06 +0000 Subject: [PATCH 5/5] Don't mention workspace --- extensions/ql-vscode/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions/ql-vscode/package.json b/extensions/ql-vscode/package.json index c5f69ccff..ad787b172 100644 --- a/extensions/ql-vscode/package.json +++ b/extensions/ql-vscode/package.json @@ -345,7 +345,7 @@ "type": "string", "default": "", "patternErrorMessage": "Please enter a valid folder", - "markdownDescription": "The name of the folder where we want to create queries and query packs via the \"CodeQL: Create Query\" command. The folder should exist in the workspace." + "markdownDescription": "The name of the folder where we want to create queries and query packs via the \"CodeQL: Create Query\" command. The folder should exist." } } },