From 90093fb9f5be153ead19757ac2479405522f53ad Mon Sep 17 00:00:00 2001 From: Robert Date: Fri, 15 Sep 2023 15:01:32 +0100 Subject: [PATCH 01/46] Rearrange settings so the telemetry settings are clearer --- extensions/ql-vscode/src/config.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/extensions/ql-vscode/src/config.ts b/extensions/ql-vscode/src/config.ts index 245079635..bb9b145df 100644 --- a/extensions/ql-vscode/src/config.ts +++ b/extensions/ql-vscode/src/config.ts @@ -72,15 +72,8 @@ export const VSCODE_SAVE_BEFORE_START_SETTING = new Setting( const ROOT_SETTING = new Setting("codeQL"); -// Global configuration +// Telemetry configuration const TELEMETRY_SETTING = new Setting("telemetry", ROOT_SETTING); -const AST_VIEWER_SETTING = new Setting("astViewer", ROOT_SETTING); -const CONTEXTUAL_QUERIES_SETTINGS = new Setting( - "contextualQueries", - ROOT_SETTING, -); -const GLOBAL_TELEMETRY_SETTING = new Setting("telemetry"); -const LOG_INSIGHTS_SETTING = new Setting("logInsights", ROOT_SETTING); export const LOG_TELEMETRY = new Setting("logTelemetry", TELEMETRY_SETTING); export const ENABLE_TELEMETRY = new Setting( @@ -88,6 +81,7 @@ export const ENABLE_TELEMETRY = new Setting( TELEMETRY_SETTING, ); +const GLOBAL_TELEMETRY_SETTING = new Setting("telemetry"); export const GLOBAL_ENABLE_TELEMETRY = new Setting( "enableTelemetry", GLOBAL_TELEMETRY_SETTING, @@ -475,6 +469,7 @@ export function allowCanaryQueryServer() { return value === undefined ? true : !!value; } +const LOG_INSIGHTS_SETTING = new Setting("logInsights", ROOT_SETTING); export const JOIN_ORDER_WARNING_THRESHOLD = new Setting( "joinOrderWarningThreshold", LOG_INSIGHTS_SETTING, @@ -484,6 +479,7 @@ export function joinOrderWarningThreshold(): number { return JOIN_ORDER_WARNING_THRESHOLD.getValue(); } +const AST_VIEWER_SETTING = new Setting("astViewer", ROOT_SETTING); /** * Hidden setting: Avoids caching in the AST viewer if the user is also a canary user. */ @@ -492,6 +488,10 @@ export const NO_CACHE_AST_VIEWER = new Setting( AST_VIEWER_SETTING, ); +const CONTEXTUAL_QUERIES_SETTINGS = new Setting( + "contextualQueries", + ROOT_SETTING, +); /** * Hidden setting: Avoids caching in jump to def and find refs contextual queries if the user is also a canary user. */ From 9c76ba35f11e12f5c80b1aa2778da4301dcbfd54 Mon Sep 17 00:00:00 2001 From: Robert Date: Fri, 15 Sep 2023 15:13:46 +0100 Subject: [PATCH 02/46] Check both telemetry.telemetryLevel and telemetry.enableTelemetry --- .../ql-vscode/src/common/vscode/telemetry.ts | 24 +++++++++++++++++-- extensions/ql-vscode/src/config.ts | 4 ++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/extensions/ql-vscode/src/common/vscode/telemetry.ts b/extensions/ql-vscode/src/common/vscode/telemetry.ts index eb4a0f407..59d4685e2 100644 --- a/extensions/ql-vscode/src/common/vscode/telemetry.ts +++ b/extensions/ql-vscode/src/common/vscode/telemetry.ts @@ -13,6 +13,7 @@ import { LOG_TELEMETRY, isIntegrationTestMode, isCanary, + GLOBAL_TELEMETRY_LEVEL, } from "../../config"; import * as appInsights from "applicationinsights"; import { extLogger } from "../logging/vscode"; @@ -93,7 +94,8 @@ export class ExtensionTelemetryListener ): Promise { if ( e.affectsConfiguration("codeQL.telemetry.enableTelemetry") || - e.affectsConfiguration("telemetry.enableTelemetry") + e.affectsConfiguration("telemetry.enableTelemetry") || + e.affectsConfiguration("telemetry.telemetryLevel") ) { await this.initialize(); } @@ -224,7 +226,7 @@ export class ExtensionTelemetryListener // if global telemetry is disabled, avoid showing the dialog or making any changes let result = undefined; if ( - GLOBAL_ENABLE_TELEMETRY.getValue() && + isGlobalTelemetryEnabled() && // Avoid showing the dialog if we are in integration test mode. !isIntegrationTestMode() ) { @@ -297,3 +299,21 @@ export async function initializeTelemetry( ctx.subscriptions.push(telemetryListener); return telemetryListener; } + +function isGlobalTelemetryEnabled(): boolean { + // If "enableTelemetry" is set to false, no telemetry will be sent regardless of the value of "telemetryLevel" + const enableTelemetry: boolean | undefined = + GLOBAL_ENABLE_TELEMETRY.getValue(); + if (enableTelemetry === false) { + return false; + } + + // If a value for "telemetry.telemetryLevel" is provided, then use that + const telemetryLevel: string | undefined = GLOBAL_TELEMETRY_LEVEL.getValue(); + if (telemetryLevel !== undefined) { + return telemetryLevel === "error" || telemetryLevel === "on"; + } + + // Otherwise fall back to the deprecated "telemetry.enableTelemetry" setting + return !!enableTelemetry; +} diff --git a/extensions/ql-vscode/src/config.ts b/extensions/ql-vscode/src/config.ts index bb9b145df..3ebdb8fa3 100644 --- a/extensions/ql-vscode/src/config.ts +++ b/extensions/ql-vscode/src/config.ts @@ -86,6 +86,10 @@ export const GLOBAL_ENABLE_TELEMETRY = new Setting( "enableTelemetry", GLOBAL_TELEMETRY_SETTING, ); +export const GLOBAL_TELEMETRY_LEVEL = new Setting( + "telemetryLevel", + GLOBAL_TELEMETRY_SETTING, +); // Distribution configuration const DISTRIBUTION_SETTING = new Setting("cli", ROOT_SETTING); From c7bd343f5498d3f026d10d0657efaa13ccc74bf8 Mon Sep 17 00:00:00 2001 From: Robert Date: Fri, 15 Sep 2023 16:03:01 +0100 Subject: [PATCH 03/46] Use sendTelemetryErrorEvent when sending errors --- extensions/ql-vscode/src/common/vscode/telemetry.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions/ql-vscode/src/common/vscode/telemetry.ts b/extensions/ql-vscode/src/common/vscode/telemetry.ts index 59d4685e2..8fadb2d21 100644 --- a/extensions/ql-vscode/src/common/vscode/telemetry.ts +++ b/extensions/ql-vscode/src/common/vscode/telemetry.ts @@ -214,7 +214,7 @@ export class ExtensionTelemetryListener properties.stack = error.stack; } - this.reporter.sendTelemetryEvent("error", properties, {}); + this.reporter.sendTelemetryErrorEvent("error", properties, {}); } /** From c7a9337ac0a306dc1d344a7a34d0dc1db32d5d2d Mon Sep 17 00:00:00 2001 From: Robert Date: Fri, 15 Sep 2023 15:28:04 +0100 Subject: [PATCH 04/46] Use .qualifiedName instead of writing out manually --- extensions/ql-vscode/src/common/vscode/telemetry.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/extensions/ql-vscode/src/common/vscode/telemetry.ts b/extensions/ql-vscode/src/common/vscode/telemetry.ts index 8fadb2d21..c972b050c 100644 --- a/extensions/ql-vscode/src/common/vscode/telemetry.ts +++ b/extensions/ql-vscode/src/common/vscode/telemetry.ts @@ -93,9 +93,9 @@ export class ExtensionTelemetryListener e: ConfigurationChangeEvent, ): Promise { if ( - e.affectsConfiguration("codeQL.telemetry.enableTelemetry") || - e.affectsConfiguration("telemetry.enableTelemetry") || - e.affectsConfiguration("telemetry.telemetryLevel") + e.affectsConfiguration(ENABLE_TELEMETRY.qualifiedName) || + e.affectsConfiguration(GLOBAL_ENABLE_TELEMETRY.qualifiedName) || + e.affectsConfiguration(GLOBAL_TELEMETRY_LEVEL.qualifiedName) ) { await this.initialize(); } @@ -104,7 +104,7 @@ export class ExtensionTelemetryListener // Re-request if codeQL.canary is being set to `true` and telemetry // is not currently enabled. if ( - e.affectsConfiguration("codeQL.canary") && + e.affectsConfiguration(CANARY_FEATURES.qualifiedName) && CANARY_FEATURES.getValue() && !ENABLE_TELEMETRY.getValue() ) { From f3e72a0ab8cdc676b8f930a461f336679ea18788 Mon Sep 17 00:00:00 2001 From: Robert Date: Fri, 15 Sep 2023 15:30:09 +0100 Subject: [PATCH 05/46] Delete unused relevantSettings field --- extensions/ql-vscode/src/common/vscode/telemetry.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/extensions/ql-vscode/src/common/vscode/telemetry.ts b/extensions/ql-vscode/src/common/vscode/telemetry.ts index c972b050c..052b7d6aa 100644 --- a/extensions/ql-vscode/src/common/vscode/telemetry.ts +++ b/extensions/ql-vscode/src/common/vscode/telemetry.ts @@ -60,8 +60,6 @@ export class ExtensionTelemetryListener extends ConfigListener implements AppTelemetry { - static relevantSettings = [ENABLE_TELEMETRY, CANARY_FEATURES]; - private reporter?: TelemetryReporter; private cliVersionStr = NOT_SET_CLI_VERSION; From a5062766357c65a5e81db92dddae1352608cb9ff Mon Sep 17 00:00:00 2001 From: Robert Date: Fri, 15 Sep 2023 16:00:32 +0100 Subject: [PATCH 06/46] Update custom telemetry setting description --- 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 11e191dd0..a31af4c9b 100644 --- a/extensions/ql-vscode/package.json +++ b/extensions/ql-vscode/package.json @@ -446,7 +446,7 @@ "type": "boolean", "default": false, "scope": "application", - "markdownDescription": "Specifies whether to send CodeQL usage telemetry. This setting AND the global `#telemetry.enableTelemetry#` setting must be checked for telemetry to be sent to GitHub. For more information, see the [telemetry documentation](https://codeql.github.com/docs/codeql-for-visual-studio-code/about-telemetry-in-codeql-for-visual-studio-code)" + "markdownDescription": "Specifies whether to send CodeQL usage telemetry. This setting AND the one of the global telemetry settings (`#telemetry.enableTelemetry#` or `#telemetry.telemetryLevel#`) must be enabled for telemetry to be sent to GitHub. For more information, see the [telemetry documentation](https://codeql.github.com/docs/codeql-for-visual-studio-code/about-telemetry-in-codeql-for-visual-studio-code)" }, "codeQL.telemetry.logTelemetry": { "type": "boolean", From af5547fb77418f1dec7cc2f2b6d1efb2b93a32d8 Mon Sep 17 00:00:00 2001 From: Robert Date: Fri, 15 Sep 2023 16:25:03 +0100 Subject: [PATCH 07/46] Use correct values of telemtetry level --- extensions/ql-vscode/src/common/vscode/telemetry.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions/ql-vscode/src/common/vscode/telemetry.ts b/extensions/ql-vscode/src/common/vscode/telemetry.ts index 052b7d6aa..7bbc97996 100644 --- a/extensions/ql-vscode/src/common/vscode/telemetry.ts +++ b/extensions/ql-vscode/src/common/vscode/telemetry.ts @@ -309,7 +309,7 @@ function isGlobalTelemetryEnabled(): boolean { // If a value for "telemetry.telemetryLevel" is provided, then use that const telemetryLevel: string | undefined = GLOBAL_TELEMETRY_LEVEL.getValue(); if (telemetryLevel !== undefined) { - return telemetryLevel === "error" || telemetryLevel === "on"; + return telemetryLevel !== "off"; } // Otherwise fall back to the deprecated "telemetry.enableTelemetry" setting From 5fc9a73e2096cb7902ff1338ad98a8dae1def0bf Mon Sep 17 00:00:00 2001 From: Robert Date: Mon, 18 Sep 2023 10:58:43 +0100 Subject: [PATCH 08/46] Add telemtry tag to settings --- extensions/ql-vscode/package.json | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/extensions/ql-vscode/package.json b/extensions/ql-vscode/package.json index a31af4c9b..4a702745c 100644 --- a/extensions/ql-vscode/package.json +++ b/extensions/ql-vscode/package.json @@ -446,13 +446,20 @@ "type": "boolean", "default": false, "scope": "application", - "markdownDescription": "Specifies whether to send CodeQL usage telemetry. This setting AND the one of the global telemetry settings (`#telemetry.enableTelemetry#` or `#telemetry.telemetryLevel#`) must be enabled for telemetry to be sent to GitHub. For more information, see the [telemetry documentation](https://codeql.github.com/docs/codeql-for-visual-studio-code/about-telemetry-in-codeql-for-visual-studio-code)" + "markdownDescription": "Specifies whether to send CodeQL usage telemetry. This setting AND the one of the global telemetry settings (`#telemetry.enableTelemetry#` or `#telemetry.telemetryLevel#`) must be enabled for telemetry to be sent to GitHub. For more information, see the [telemetry documentation](https://codeql.github.com/docs/codeql-for-visual-studio-code/about-telemetry-in-codeql-for-visual-studio-code)", + "tags": [ + "telemetry", + "usesOnlineServices" + ] }, "codeQL.telemetry.logTelemetry": { "type": "boolean", "default": false, "scope": "application", - "description": "Specifies whether or not to write telemetry events to the extension log." + "description": "Specifies whether or not to write telemetry events to the extension log.", + "tags": [ + "telemetry" + ] } } } From 9ec65905483c1418a5ceb9bc06b41357dd8d4519 Mon Sep 17 00:00:00 2001 From: Robert Date: Mon, 18 Sep 2023 11:09:22 +0100 Subject: [PATCH 09/46] Use env.isTelemetryEnabled instead of checking configuration values ourselves --- .../ql-vscode/src/common/vscode/telemetry.ts | 21 ++----------------- 1 file changed, 2 insertions(+), 19 deletions(-) diff --git a/extensions/ql-vscode/src/common/vscode/telemetry.ts b/extensions/ql-vscode/src/common/vscode/telemetry.ts index 7bbc97996..e69850448 100644 --- a/extensions/ql-vscode/src/common/vscode/telemetry.ts +++ b/extensions/ql-vscode/src/common/vscode/telemetry.ts @@ -3,6 +3,7 @@ import { Extension, ExtensionContext, ConfigurationChangeEvent, + env, } from "vscode"; import TelemetryReporter from "vscode-extension-telemetry"; import { @@ -224,7 +225,7 @@ export class ExtensionTelemetryListener // if global telemetry is disabled, avoid showing the dialog or making any changes let result = undefined; if ( - isGlobalTelemetryEnabled() && + env.isTelemetryEnabled && // Avoid showing the dialog if we are in integration test mode. !isIntegrationTestMode() ) { @@ -297,21 +298,3 @@ export async function initializeTelemetry( ctx.subscriptions.push(telemetryListener); return telemetryListener; } - -function isGlobalTelemetryEnabled(): boolean { - // If "enableTelemetry" is set to false, no telemetry will be sent regardless of the value of "telemetryLevel" - const enableTelemetry: boolean | undefined = - GLOBAL_ENABLE_TELEMETRY.getValue(); - if (enableTelemetry === false) { - return false; - } - - // If a value for "telemetry.telemetryLevel" is provided, then use that - const telemetryLevel: string | undefined = GLOBAL_TELEMETRY_LEVEL.getValue(); - if (telemetryLevel !== undefined) { - return telemetryLevel !== "off"; - } - - // Otherwise fall back to the deprecated "telemetry.enableTelemetry" setting - return !!enableTelemetry; -} From d4033615c81945e94e10b62c14deecf9da9499b4 Mon Sep 17 00:00:00 2001 From: Robert Date: Mon, 18 Sep 2023 11:13:19 +0100 Subject: [PATCH 10/46] Use onDidChangeTelemetryEnabled instead of listening for config value changes --- extensions/ql-vscode/src/common/vscode/telemetry.ts | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/extensions/ql-vscode/src/common/vscode/telemetry.ts b/extensions/ql-vscode/src/common/vscode/telemetry.ts index e69850448..a2470b4cd 100644 --- a/extensions/ql-vscode/src/common/vscode/telemetry.ts +++ b/extensions/ql-vscode/src/common/vscode/telemetry.ts @@ -10,11 +10,9 @@ import { ConfigListener, CANARY_FEATURES, ENABLE_TELEMETRY, - GLOBAL_ENABLE_TELEMETRY, LOG_TELEMETRY, isIntegrationTestMode, isCanary, - GLOBAL_TELEMETRY_LEVEL, } from "../../config"; import * as appInsights from "applicationinsights"; import { extLogger } from "../logging/vscode"; @@ -72,6 +70,10 @@ export class ExtensionTelemetryListener private readonly ctx: ExtensionContext, ) { super(); + + env.onDidChangeTelemetryEnabled(async () => { + await this.initialize(); + }); } /** @@ -91,11 +93,7 @@ export class ExtensionTelemetryListener async handleDidChangeConfiguration( e: ConfigurationChangeEvent, ): Promise { - if ( - e.affectsConfiguration(ENABLE_TELEMETRY.qualifiedName) || - e.affectsConfiguration(GLOBAL_ENABLE_TELEMETRY.qualifiedName) || - e.affectsConfiguration(GLOBAL_TELEMETRY_LEVEL.qualifiedName) - ) { + if (e.affectsConfiguration(ENABLE_TELEMETRY.qualifiedName)) { await this.initialize(); } From 177c770f4d4ddc368cdb4ffd8fd5d859e879e968 Mon Sep 17 00:00:00 2001 From: Robert Date: Mon, 18 Sep 2023 11:15:34 +0100 Subject: [PATCH 11/46] Delete config values that we on longer read directly --- extensions/ql-vscode/src/config.ts | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/extensions/ql-vscode/src/config.ts b/extensions/ql-vscode/src/config.ts index 3ebdb8fa3..cf8c32d9c 100644 --- a/extensions/ql-vscode/src/config.ts +++ b/extensions/ql-vscode/src/config.ts @@ -81,16 +81,6 @@ export const ENABLE_TELEMETRY = new Setting( TELEMETRY_SETTING, ); -const GLOBAL_TELEMETRY_SETTING = new Setting("telemetry"); -export const GLOBAL_ENABLE_TELEMETRY = new Setting( - "enableTelemetry", - GLOBAL_TELEMETRY_SETTING, -); -export const GLOBAL_TELEMETRY_LEVEL = new Setting( - "telemetryLevel", - GLOBAL_TELEMETRY_SETTING, -); - // Distribution configuration const DISTRIBUTION_SETTING = new Setting("cli", ROOT_SETTING); export const CUSTOM_CODEQL_PATH_SETTING = new Setting( From aad1fee78725191775a98287a51e288fff5b1f8d Mon Sep 17 00:00:00 2001 From: Robert Date: Tue, 19 Sep 2023 17:18:38 +0100 Subject: [PATCH 12/46] Stop mocking sendTelemetryException because we don't use that anywhere in the program --- .../vscode-tests/no-workspace/telemetry.test.ts | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/telemetry.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/telemetry.test.ts index d8d855494..31e0fb5ea 100644 --- a/extensions/ql-vscode/test/vscode-tests/no-workspace/telemetry.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/telemetry.test.ts @@ -30,9 +30,6 @@ describe("telemetry reporting", () => { let sendTelemetryEventSpy: jest.SpiedFunction< typeof TelemetryReporter.prototype.sendTelemetryEvent >; - let sendTelemetryExceptionSpy: jest.SpiedFunction< - typeof TelemetryReporter.prototype.sendTelemetryException - >; let disposeSpy: jest.SpiedFunction< typeof TelemetryReporter.prototype.dispose >; @@ -56,9 +53,6 @@ describe("telemetry reporting", () => { sendTelemetryEventSpy = jest .spyOn(TelemetryReporter.prototype, "sendTelemetryEvent") .mockReturnValue(undefined); - sendTelemetryExceptionSpy = jest - .spyOn(TelemetryReporter.prototype, "sendTelemetryException") - .mockReturnValue(undefined); disposeSpy = jest .spyOn(TelemetryReporter.prototype, "dispose") .mockResolvedValue(undefined); @@ -198,8 +192,6 @@ describe("telemetry reporting", () => { }, { executionTime: 1234 }, ); - - expect(sendTelemetryExceptionSpy).not.toBeCalled(); }); it("should send a command usage event with an error", async () => { @@ -221,8 +213,6 @@ describe("telemetry reporting", () => { }, { executionTime: 1234 }, ); - - expect(sendTelemetryExceptionSpy).not.toBeCalled(); }); it("should send a command usage event with a cli version", async () => { @@ -246,8 +236,6 @@ describe("telemetry reporting", () => { { executionTime: 1234 }, ); - expect(sendTelemetryExceptionSpy).not.toBeCalled(); - // Verify that if the cli version is not set, then the telemetry falls back to "not-set" sendTelemetryEventSpy.mockClear(); telemetryListener.cliVersion = undefined; @@ -278,7 +266,6 @@ describe("telemetry reporting", () => { telemetryListener.sendCommandUsage("command-id", 1234, new Error()); expect(sendTelemetryEventSpy).not.toBeCalled(); - expect(sendTelemetryExceptionSpy).not.toBeCalled(); }); it("should send an event when telemetry is re-enabled", async () => { From ca96cdf879f9a14935e0cf2aa61abbbc3bd044cf Mon Sep 17 00:00:00 2001 From: Robert Date: Tue, 19 Sep 2023 17:20:30 +0100 Subject: [PATCH 13/46] Mock sendTelemetryErrorEvent and check appropriately --- .../no-workspace/telemetry.test.ts | 23 ++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/telemetry.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/telemetry.test.ts index 31e0fb5ea..544ea2700 100644 --- a/extensions/ql-vscode/test/vscode-tests/no-workspace/telemetry.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/telemetry.test.ts @@ -30,6 +30,9 @@ describe("telemetry reporting", () => { let sendTelemetryEventSpy: jest.SpiedFunction< typeof TelemetryReporter.prototype.sendTelemetryEvent >; + let sendTelemetryErrorEventSpy: jest.SpiedFunction< + typeof TelemetryReporter.prototype.sendTelemetryErrorEvent + >; let disposeSpy: jest.SpiedFunction< typeof TelemetryReporter.prototype.dispose >; @@ -53,6 +56,9 @@ describe("telemetry reporting", () => { sendTelemetryEventSpy = jest .spyOn(TelemetryReporter.prototype, "sendTelemetryEvent") .mockReturnValue(undefined); + sendTelemetryErrorEventSpy = jest + .spyOn(TelemetryReporter.prototype, "sendTelemetryErrorEvent") + .mockReturnValue(undefined); disposeSpy = jest .spyOn(TelemetryReporter.prototype, "dispose") .mockResolvedValue(undefined); @@ -192,6 +198,7 @@ describe("telemetry reporting", () => { }, { executionTime: 1234 }, ); + expect(sendTelemetryErrorEventSpy).not.toBeCalled(); }); it("should send a command usage event with an error", async () => { @@ -213,6 +220,7 @@ describe("telemetry reporting", () => { }, { executionTime: 1234 }, ); + expect(sendTelemetryErrorEventSpy).not.toBeCalled(); }); it("should send a command usage event with a cli version", async () => { @@ -235,6 +243,7 @@ describe("telemetry reporting", () => { }, { executionTime: 1234 }, ); + expect(sendTelemetryErrorEventSpy).not.toBeCalled(); // Verify that if the cli version is not set, then the telemetry falls back to "not-set" sendTelemetryEventSpy.mockClear(); @@ -256,6 +265,7 @@ describe("telemetry reporting", () => { }, { executionTime: 5678 }, ); + expect(sendTelemetryErrorEventSpy).not.toBeCalled(); }); it("should avoid sending an event when telemetry is disabled", async () => { @@ -266,6 +276,7 @@ describe("telemetry reporting", () => { telemetryListener.sendCommandUsage("command-id", 1234, new Error()); expect(sendTelemetryEventSpy).not.toBeCalled(); + expect(sendTelemetryErrorEventSpy).not.toBeCalled(); }); it("should send an event when telemetry is re-enabled", async () => { @@ -285,6 +296,7 @@ describe("telemetry reporting", () => { }, { executionTime: 1234 }, ); + expect(sendTelemetryErrorEventSpy).not.toBeCalled(); }); it("should filter undesired properties from telemetry payload", async () => { @@ -442,6 +454,7 @@ describe("telemetry reporting", () => { }, {}, ); + expect(sendTelemetryErrorEventSpy).not.toBeCalled(); }); it("should send a ui-interaction telementry event with a cli version", async () => { @@ -459,6 +472,7 @@ describe("telemetry reporting", () => { }, {}, ); + expect(sendTelemetryErrorEventSpy).not.toBeCalled(); }); it("should send an error telementry event", async () => { @@ -466,7 +480,8 @@ describe("telemetry reporting", () => { telemetryListener.sendError(redactableError`test`); - expect(sendTelemetryEventSpy).toHaveBeenCalledWith( + expect(sendTelemetryEventSpy).not.toBeCalled(); + expect(sendTelemetryErrorEventSpy).toHaveBeenCalledWith( "error", { message: "test", @@ -484,7 +499,8 @@ describe("telemetry reporting", () => { telemetryListener.sendError(redactableError`test`); - expect(sendTelemetryEventSpy).toHaveBeenCalledWith( + expect(sendTelemetryEventSpy).not.toBeCalled(); + expect(sendTelemetryErrorEventSpy).toHaveBeenCalledWith( "error", { message: "test", @@ -503,7 +519,8 @@ describe("telemetry reporting", () => { redactableError`test message with secret information: ${42} and more ${"secret"} parts`, ); - expect(sendTelemetryEventSpy).toHaveBeenCalledWith( + expect(sendTelemetryEventSpy).not.toBeCalled(); + expect(sendTelemetryErrorEventSpy).toHaveBeenCalledWith( "error", { message: From 4227ff63388b847129599f5b58d82f12af63e4bd Mon Sep 17 00:00:00 2001 From: Robert Date: Tue, 19 Sep 2023 17:24:53 +0100 Subject: [PATCH 14/46] Mock env.itTelemetryEnable --- .../vscode-tests/no-workspace/telemetry.test.ts | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/telemetry.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/telemetry.test.ts index 544ea2700..7e1b03eae 100644 --- a/extensions/ql-vscode/test/vscode-tests/no-workspace/telemetry.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/telemetry.test.ts @@ -4,6 +4,7 @@ import { workspace, ConfigurationTarget, window, + env, } from "vscode"; import { ExtensionTelemetryListener, @@ -37,6 +38,11 @@ describe("telemetry reporting", () => { typeof TelemetryReporter.prototype.dispose >; + let isTelemetryEnabledSpy: jest.SpyInstance< + typeof env.isTelemetryEnabled, + [] + >; + let showInformationMessageSpy: jest.SpiedFunction< typeof window.showInformationMessage >; @@ -78,6 +84,9 @@ describe("telemetry reporting", () => { .get("codeQL.canary")).toString(); // each test will default to telemetry being enabled + isTelemetryEnabledSpy = jest + .spyOn(env, "isTelemetryEnabled", "get") + .mockReturnValue(true); await enableTelemetry("telemetry", true); await enableTelemetry("codeQL.telemetry", true); @@ -116,6 +125,7 @@ describe("telemetry reporting", () => { }); it("should initialize telemetry when global option disabled", async () => { + isTelemetryEnabledSpy.mockReturnValue(false); await enableTelemetry("telemetry", false); await telemetryListener.initialize(); expect(telemetryListener._reporter).toBeDefined(); @@ -133,6 +143,7 @@ describe("telemetry reporting", () => { it("should not initialize telemetry when both options disabled", async () => { await enableTelemetry("codeQL.telemetry", false); + isTelemetryEnabledSpy.mockReturnValue(false); await enableTelemetry("telemetry", false); await telemetryListener.initialize(); expect(telemetryListener._reporter).toBeUndefined(); @@ -179,6 +190,7 @@ describe("telemetry reporting", () => { const reporter: any = telemetryListener._reporter; expect(reporter.userOptIn).toBe(true); // enabled + isTelemetryEnabledSpy.mockReturnValue(false); await enableTelemetry("telemetry", false); expect(reporter.userOptIn).toBe(false); // disabled }); @@ -344,6 +356,8 @@ describe("telemetry reporting", () => { resolveArg(3 /* "yes" item */), ); await ctx.globalState.update("telemetry-request-viewed", false); + expect(env.isTelemetryEnabled).toBe(true); + await enableTelemetry("codeQL.telemetry", false); await telemetryListener.initialize(); @@ -414,6 +428,7 @@ describe("telemetry reporting", () => { await ctx.globalState.update("telemetry-request-viewed", false); await telemetryListener.initialize(); + isTelemetryEnabledSpy.mockReturnValue(false); // popup should not be shown even though we have initialized telemetry expect(showInformationMessageSpy).not.toBeCalled(); From e8f68c1b5f6265320180d09aacc8a1898c1f24c7 Mon Sep 17 00:00:00 2001 From: Andrew Eisenberg Date: Thu, 28 Sep 2023 16:23:20 -0700 Subject: [PATCH 15/46] Adds `MultiCancellationToken` This is a cancellation token that cancels when any of its constituent cancellation tokens are cancelled. This token is used to fix a bug in Find Definitions. Previously, when clicking `CTRL` (or `CMD` on macs) inside a source file in an archive and hovering over a token, this will automatically invoke the definitions finder (in preparation for navigating to the definition). The only way to cancel is to move down to the message popup and click cancel there. However, this is a bug. What _should_ happen is that if a user moves their mouse away from the token, the operation should cancel. The underlying problem is that the extension was only listening to the cancellation token from inside `getLocationsForUriString` the cancellation token used by the Language Server protocol to cancel operations in flight was being ignored. This fix will ensure we are listening to _both_ cancellation tokens and cancel the query if either are cancelled. --- .../src/common/multi-cancellation-token.ts | 28 +++++++++++++++ .../contextual/template-provider.ts | 35 ++++++++++++------- 2 files changed, 51 insertions(+), 12 deletions(-) create mode 100644 extensions/ql-vscode/src/common/multi-cancellation-token.ts diff --git a/extensions/ql-vscode/src/common/multi-cancellation-token.ts b/extensions/ql-vscode/src/common/multi-cancellation-token.ts new file mode 100644 index 000000000..3d9726f5c --- /dev/null +++ b/extensions/ql-vscode/src/common/multi-cancellation-token.ts @@ -0,0 +1,28 @@ +import { CancellationToken, Disposable } from "vscode"; + +/** + * A cancellation token that cancels when any of its constituent + * cancellation tokens are cancelled. + */ +export class MultiCancellationToken implements CancellationToken { + private readonly tokens: CancellationToken[]; + + constructor(...tokens: CancellationToken[]) { + this.tokens = tokens; + } + + get isCancellationRequested(): boolean { + return this.tokens.some((t) => t.isCancellationRequested); + } + + onCancellationRequested(listener: (e: T) => any): Disposable { + this.tokens.forEach((t) => t.onCancellationRequested(listener)); + return { + dispose: () => { + this.tokens.forEach((t) => + t.onCancellationRequested(listener).dispose(), + ); + }, + }; + } +} diff --git a/extensions/ql-vscode/src/language-support/contextual/template-provider.ts b/extensions/ql-vscode/src/language-support/contextual/template-provider.ts index e72f5ed30..2a905e2c0 100644 --- a/extensions/ql-vscode/src/language-support/contextual/template-provider.ts +++ b/extensions/ql-vscode/src/language-support/contextual/template-provider.ts @@ -36,6 +36,7 @@ import { import { CoreCompletedQuery, QueryRunner } from "../../query-server"; import { AstBuilder } from "../ast-viewer/ast-builder"; import { qlpackOfDatabase } from "../../local-queries"; +import { MultiCancellationToken } from "../../common/multi-cancellation-token"; /** * Runs templated CodeQL queries to find definitions in @@ -43,6 +44,7 @@ import { qlpackOfDatabase } from "../../local-queries"; * generalize this to other custom queries, e.g. showing dataflow to * or from a selected identifier. */ + export class TemplateQueryDefinitionProvider implements DefinitionProvider { private cache: CachedOperation; @@ -60,11 +62,11 @@ export class TemplateQueryDefinitionProvider implements DefinitionProvider { async provideDefinition( document: TextDocument, position: Position, - _token: CancellationToken, + token: CancellationToken, ): Promise { const fileLinks = this.shouldUseCache() - ? await this.cache.get(document.uri.toString()) - : await this.getDefinitions(document.uri.toString()); + ? await this.cache.get(document.uri.toString(), token) + : await this.getDefinitions(document.uri.toString(), token); const locLinks: LocationLink[] = []; for (const link of fileLinks) { @@ -79,9 +81,13 @@ export class TemplateQueryDefinitionProvider implements DefinitionProvider { return !(isCanary() && NO_CACHE_CONTEXTUAL_QUERIES.getValue()); } - private async getDefinitions(uriString: string): Promise { + private async getDefinitions( + uriString: string, + token: CancellationToken, + ): Promise { return withProgress( - async (progress, token) => { + async (progress, tokenInner) => { + const multiToken = new MultiCancellationToken(token, tokenInner); return getLocationsForUriString( this.cli, this.qs, @@ -90,7 +96,7 @@ export class TemplateQueryDefinitionProvider implements DefinitionProvider { KeyType.DefinitionQuery, this.queryStorageDir, progress, - token, + multiToken, (src, _dest) => src === uriString, ); }, @@ -126,11 +132,11 @@ export class TemplateQueryReferenceProvider implements ReferenceProvider { document: TextDocument, position: Position, _context: ReferenceContext, - _token: CancellationToken, + token: CancellationToken, ): Promise { const fileLinks = this.shouldUseCache() - ? await this.cache.get(document.uri.toString()) - : await this.getReferences(document.uri.toString()); + ? await this.cache.get(document.uri.toString(), token) + : await this.getReferences(document.uri.toString(), token); const locLinks: Location[] = []; for (const link of fileLinks) { @@ -148,9 +154,14 @@ export class TemplateQueryReferenceProvider implements ReferenceProvider { return !(isCanary() && NO_CACHE_CONTEXTUAL_QUERIES.getValue()); } - private async getReferences(uriString: string): Promise { + private async getReferences( + uriString: string, + token: CancellationToken, + ): Promise { return withProgress( - async (progress, token) => { + async (progress, tokenInner) => { + const multiToken = new MultiCancellationToken(token, tokenInner); + return getLocationsForUriString( this.cli, this.qs, @@ -159,7 +170,7 @@ export class TemplateQueryReferenceProvider implements ReferenceProvider { KeyType.DefinitionQuery, this.queryStorageDir, progress, - token, + multiToken, (src, _dest) => src === uriString, ); }, From 3ba1712be094539cc4cf40a00da404046eeaac4f Mon Sep 17 00:00:00 2001 From: Andrew Eisenberg Date: Thu, 28 Sep 2023 16:27:29 -0700 Subject: [PATCH 16/46] Update Changelog --- extensions/ql-vscode/CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/extensions/ql-vscode/CHANGELOG.md b/extensions/ql-vscode/CHANGELOG.md index 8309f0cda..efefd2f42 100644 --- a/extensions/ql-vscode/CHANGELOG.md +++ b/extensions/ql-vscode/CHANGELOG.md @@ -2,6 +2,8 @@ ## [UNRELEASED] +- Fix a bug where the query to Find Definitions in database source files would not be cancelled appropriately. [#2885](https://github.com/github/vscode-codeql/pull/2885) + ## 1.9.1 - 29 September 2023 - Add warning when using a VS Code version older than 1.82.0. [#2854](https://github.com/github/vscode-codeql/pull/2854) From 04dfc4e647a79ae11d7b2addbf71a5fdbfa3dd58 Mon Sep 17 00:00:00 2001 From: Andrew Eisenberg Date: Fri, 29 Sep 2023 08:31:20 -0700 Subject: [PATCH 17/46] Move location of multi-cancellation-token This avoids a code-scanning warning. --- .../src/common/{ => vscode}/multi-cancellation-token.ts | 0 .../src/language-support/contextual/template-provider.ts | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename extensions/ql-vscode/src/common/{ => vscode}/multi-cancellation-token.ts (100%) diff --git a/extensions/ql-vscode/src/common/multi-cancellation-token.ts b/extensions/ql-vscode/src/common/vscode/multi-cancellation-token.ts similarity index 100% rename from extensions/ql-vscode/src/common/multi-cancellation-token.ts rename to extensions/ql-vscode/src/common/vscode/multi-cancellation-token.ts diff --git a/extensions/ql-vscode/src/language-support/contextual/template-provider.ts b/extensions/ql-vscode/src/language-support/contextual/template-provider.ts index 2a905e2c0..21777ab43 100644 --- a/extensions/ql-vscode/src/language-support/contextual/template-provider.ts +++ b/extensions/ql-vscode/src/language-support/contextual/template-provider.ts @@ -36,7 +36,7 @@ import { import { CoreCompletedQuery, QueryRunner } from "../../query-server"; import { AstBuilder } from "../ast-viewer/ast-builder"; import { qlpackOfDatabase } from "../../local-queries"; -import { MultiCancellationToken } from "../../common/multi-cancellation-token"; +import { MultiCancellationToken } from "../../common/vscode/multi-cancellation-token"; /** * Runs templated CodeQL queries to find definitions in From 0fa3cf5d8e1c5914f4c4fd343a2050d1a287e9a3 Mon Sep 17 00:00:00 2001 From: Andrew Eisenberg Date: Mon, 2 Oct 2023 10:21:17 -0700 Subject: [PATCH 18/46] Use `EventEmitter` in `MultiCancellationToken` --- .../common/vscode/multi-cancellation-token.ts | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/extensions/ql-vscode/src/common/vscode/multi-cancellation-token.ts b/extensions/ql-vscode/src/common/vscode/multi-cancellation-token.ts index 3d9726f5c..b6d38a790 100644 --- a/extensions/ql-vscode/src/common/vscode/multi-cancellation-token.ts +++ b/extensions/ql-vscode/src/common/vscode/multi-cancellation-token.ts @@ -1,4 +1,4 @@ -import { CancellationToken, Disposable } from "vscode"; +import { CancellationToken, Event, EventEmitter } from "vscode"; /** * A cancellation token that cancels when any of its constituent @@ -6,23 +6,20 @@ import { CancellationToken, Disposable } from "vscode"; */ export class MultiCancellationToken implements CancellationToken { private readonly tokens: CancellationToken[]; + private readonly onCancellationRequestedEvent = new EventEmitter(); constructor(...tokens: CancellationToken[]) { this.tokens = tokens; + tokens.forEach((t) => + t.onCancellationRequested(() => this.onCancellationRequestedEvent.fire()), + ); } get isCancellationRequested(): boolean { return this.tokens.some((t) => t.isCancellationRequested); } - onCancellationRequested(listener: (e: T) => any): Disposable { - this.tokens.forEach((t) => t.onCancellationRequested(listener)); - return { - dispose: () => { - this.tokens.forEach((t) => - t.onCancellationRequested(listener).dispose(), - ); - }, - }; + get onCancellationRequested(): Event { + return this.onCancellationRequestedEvent.event; } } From 3699f0b3b3a7aedf5a198d7e337526ece88d7c9e Mon Sep 17 00:00:00 2001 From: Andrew Eisenberg Date: Tue, 3 Oct 2023 21:42:00 +0000 Subject: [PATCH 19/46] Add `BasicDisposableObject` Use it for `MultiCancellationToken`. And ensure that adding a cancellation requested listener to the `MultiCancellationToken` will forward any cancellation requests to all constituent tokens. --- .../ql-vscode/src/common/disposable-object.ts | 9 +++++++++ .../src/common/vscode/multi-cancellation-token.ts | 14 +++++++------- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/extensions/ql-vscode/src/common/disposable-object.ts b/extensions/ql-vscode/src/common/disposable-object.ts index 018ee5f91..35396e6f4 100644 --- a/extensions/ql-vscode/src/common/disposable-object.ts +++ b/extensions/ql-vscode/src/common/disposable-object.ts @@ -82,3 +82,12 @@ export abstract class DisposableObject implements Disposable { } } } + +export class BasicDisposableObject extends DisposableObject { + constructor(...dispoables: Disposable[]) { + super(); + for (const d of dispoables) { + this.push(d); + } + } +} diff --git a/extensions/ql-vscode/src/common/vscode/multi-cancellation-token.ts b/extensions/ql-vscode/src/common/vscode/multi-cancellation-token.ts index b6d38a790..fc6841066 100644 --- a/extensions/ql-vscode/src/common/vscode/multi-cancellation-token.ts +++ b/extensions/ql-vscode/src/common/vscode/multi-cancellation-token.ts @@ -1,4 +1,5 @@ -import { CancellationToken, Event, EventEmitter } from "vscode"; +import { CancellationToken, Disposable } from "vscode"; +import { BasicDisposableObject } from "../disposable-object"; /** * A cancellation token that cancels when any of its constituent @@ -6,20 +7,19 @@ import { CancellationToken, Event, EventEmitter } from "vscode"; */ export class MultiCancellationToken implements CancellationToken { private readonly tokens: CancellationToken[]; - private readonly onCancellationRequestedEvent = new EventEmitter(); constructor(...tokens: CancellationToken[]) { this.tokens = tokens; - tokens.forEach((t) => - t.onCancellationRequested(() => this.onCancellationRequestedEvent.fire()), - ); } get isCancellationRequested(): boolean { return this.tokens.some((t) => t.isCancellationRequested); } - get onCancellationRequested(): Event { - return this.onCancellationRequestedEvent.event; + onCancellationRequested(listener: (e: T) => any): Disposable { + const disposables = this.tokens.map((t) => + t.onCancellationRequested(listener), + ); + return new BasicDisposableObject(...disposables); } } From c5175e040ec2e54dba0d9937b24892951a6511c3 Mon Sep 17 00:00:00 2001 From: Koen Vlaswinkel Date: Wed, 4 Oct 2023 13:47:30 +0200 Subject: [PATCH 20/46] Convert `modeled-method-fs.ts` to handle multiple models per method This will change the input/output types for modeled methods in the `modeled-method-fs.ts` file to take in multiple models per method. This removes the need for conversion functions between this file and `yaml.ts` files. Instead, the conversion functions are done when calling any functions defined in `modeled-method-fs.ts` files. --- .../src/model-editor/model-editor-view.ts | 11 ++++++-- .../src/model-editor/modeled-method-fs.ts | 28 +++++++++---------- .../model-editor/modeled-methods-legacy.ts | 10 ------- 3 files changed, 22 insertions(+), 27 deletions(-) diff --git a/extensions/ql-vscode/src/model-editor/model-editor-view.ts b/extensions/ql-vscode/src/model-editor/model-editor-view.ts index 7724d5651..87cdb88e3 100644 --- a/extensions/ql-vscode/src/model-editor/model-editor-view.ts +++ b/extensions/ql-vscode/src/model-editor/model-editor-view.ts @@ -43,6 +43,10 @@ import { AutoModeler } from "./auto-modeler"; import { telemetryListener } from "../common/vscode/telemetry"; import { ModelingStore } from "./modeling-store"; import { ModelEditorViewTracker } from "./model-editor-view-tracker"; +import { + convertFromLegacyModeledMethods, + convertToLegacyModeledMethods, +} from "./modeled-methods-legacy"; export class ModelEditorView extends AbstractWebview< ToModelEditorMessage, @@ -235,7 +239,7 @@ export class ModelEditorView extends AbstractWebview< this.extensionPack, this.databaseItem.language, msg.methods, - msg.modeledMethods, + convertFromLegacyModeledMethods(msg.modeledMethods), this.mode, this.cliServer, this.app.logger, @@ -381,7 +385,10 @@ export class ModelEditorView extends AbstractWebview< this.cliServer, this.app.logger, ); - this.modelingStore.setModeledMethods(this.databaseItem, modeledMethods); + this.modelingStore.setModeledMethods( + this.databaseItem, + convertToLegacyModeledMethods(modeledMethods), + ); } catch (e: unknown) { void showAndLogErrorMessage( this.app.logger, diff --git a/extensions/ql-vscode/src/model-editor/modeled-method-fs.ts b/extensions/ql-vscode/src/model-editor/modeled-method-fs.ts index 8f3ce34aa..10a92ef63 100644 --- a/extensions/ql-vscode/src/model-editor/modeled-method-fs.ts +++ b/extensions/ql-vscode/src/model-editor/modeled-method-fs.ts @@ -10,17 +10,12 @@ import { getOnDiskWorkspaceFolders } from "../common/vscode/workspace-folders"; import { load as loadYaml } from "js-yaml"; import { CodeQLCliServer } from "../codeql-cli/cli"; import { pathsEqual } from "../common/files"; -import { - convertFromLegacyModeledMethods, - convertFromLegacyModeledMethodsFiles, - convertToLegacyModeledMethods, -} from "./modeled-methods-legacy"; export async function saveModeledMethods( extensionPack: ExtensionPack, language: string, methods: Method[], - modeledMethods: Record, + modeledMethods: Record, mode: Mode, cliServer: CodeQLCliServer, logger: NotificationLogger, @@ -34,8 +29,8 @@ export async function saveModeledMethods( const yamls = createDataExtensionYamls( language, methods, - convertFromLegacyModeledMethods(modeledMethods), - convertFromLegacyModeledMethodsFiles(existingModeledMethods), + modeledMethods, + existingModeledMethods, mode, ); @@ -50,12 +45,12 @@ async function loadModeledMethodFiles( extensionPack: ExtensionPack, cliServer: CodeQLCliServer, logger: NotificationLogger, -): Promise>> { +): Promise>> { const modelFiles = await listModelFiles(extensionPack.path, cliServer); const modeledMethodsByFile: Record< string, - Record + Record > = {}; for (const modelFile of modelFiles) { @@ -73,8 +68,7 @@ async function loadModeledMethodFiles( ); continue; } - modeledMethodsByFile[modelFile] = - convertToLegacyModeledMethods(modeledMethods); + modeledMethodsByFile[modelFile] = modeledMethods; } return modeledMethodsByFile; @@ -84,8 +78,8 @@ export async function loadModeledMethods( extensionPack: ExtensionPack, cliServer: CodeQLCliServer, logger: NotificationLogger, -): Promise> { - const existingModeledMethods: Record = {}; +): Promise> { + const existingModeledMethods: Record = {}; const modeledMethodsByFile = await loadModeledMethodFiles( extensionPack, @@ -94,7 +88,11 @@ export async function loadModeledMethods( ); for (const modeledMethods of Object.values(modeledMethodsByFile)) { for (const [key, value] of Object.entries(modeledMethods)) { - existingModeledMethods[key] = value; + if (!(key in existingModeledMethods)) { + existingModeledMethods[key] = value; + } + + existingModeledMethods[key].push(...value); } } diff --git a/extensions/ql-vscode/src/model-editor/modeled-methods-legacy.ts b/extensions/ql-vscode/src/model-editor/modeled-methods-legacy.ts index a482af0e3..2e6f3af1c 100644 --- a/extensions/ql-vscode/src/model-editor/modeled-methods-legacy.ts +++ b/extensions/ql-vscode/src/model-editor/modeled-methods-legacy.ts @@ -21,13 +21,3 @@ export function convertToLegacyModeledMethods( }), ); } - -export function convertFromLegacyModeledMethodsFiles( - modeledMethods: Record>, -): Record> { - return Object.fromEntries( - Object.entries(modeledMethods).map(([filename, modeledMethods]) => { - return [filename, convertFromLegacyModeledMethods(modeledMethods)]; - }), - ); -} From 76ec9e2e50905bad771eae80991b8dd20ffdf54b Mon Sep 17 00:00:00 2001 From: Andrew Eisenberg Date: Wed, 4 Oct 2023 20:09:47 +0000 Subject: [PATCH 21/46] Make `DisposableObject` a concrete class --- .../ql-vscode/src/common/disposable-object.ts | 17 +++++++---------- .../common/vscode/multi-cancellation-token.ts | 7 +++---- 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/extensions/ql-vscode/src/common/disposable-object.ts b/extensions/ql-vscode/src/common/disposable-object.ts index 35396e6f4..96af15ac0 100644 --- a/extensions/ql-vscode/src/common/disposable-object.ts +++ b/extensions/ql-vscode/src/common/disposable-object.ts @@ -9,10 +9,16 @@ export type DisposeHandler = (disposable: Disposable) => void; /** * Base class to make it easier to implement a `Disposable` that owns other disposable object. */ -export abstract class DisposableObject implements Disposable { +export class DisposableObject implements Disposable { private disposables: Disposable[] = []; private tracked?: Set = undefined; + constructor(...dispoables: Disposable[]) { + for (const d of dispoables) { + this.push(d); + } + } + /** * Adds `obj` to a list of objects to dispose when `this` is disposed. Objects added by `push` are * disposed in reverse order of being added. @@ -82,12 +88,3 @@ export abstract class DisposableObject implements Disposable { } } } - -export class BasicDisposableObject extends DisposableObject { - constructor(...dispoables: Disposable[]) { - super(); - for (const d of dispoables) { - this.push(d); - } - } -} diff --git a/extensions/ql-vscode/src/common/vscode/multi-cancellation-token.ts b/extensions/ql-vscode/src/common/vscode/multi-cancellation-token.ts index fc6841066..08be10de9 100644 --- a/extensions/ql-vscode/src/common/vscode/multi-cancellation-token.ts +++ b/extensions/ql-vscode/src/common/vscode/multi-cancellation-token.ts @@ -1,5 +1,5 @@ import { CancellationToken, Disposable } from "vscode"; -import { BasicDisposableObject } from "../disposable-object"; +import { DisposableObject } from "../disposable-object"; /** * A cancellation token that cancels when any of its constituent @@ -17,9 +17,8 @@ export class MultiCancellationToken implements CancellationToken { } onCancellationRequested(listener: (e: T) => any): Disposable { - const disposables = this.tokens.map((t) => - t.onCancellationRequested(listener), + return new DisposableObject( + ...this.tokens.map((t) => t.onCancellationRequested(listener)), ); - return new BasicDisposableObject(...disposables); } } From 1806108166929a9435faf5d8c69431d599656ad9 Mon Sep 17 00:00:00 2001 From: Charis Kyriakou Date: Thu, 5 Oct 2023 09:04:17 +0100 Subject: [PATCH 22/46] Remove unnecessary TODOs (#2900) --- extensions/ql-vscode/src/packages/commands/CommandManager.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/extensions/ql-vscode/src/packages/commands/CommandManager.ts b/extensions/ql-vscode/src/packages/commands/CommandManager.ts index 6583840a7..850c4718a 100644 --- a/extensions/ql-vscode/src/packages/commands/CommandManager.ts +++ b/extensions/ql-vscode/src/packages/commands/CommandManager.ts @@ -23,8 +23,6 @@ export class CommandManager< CommandName extends keyof Commands & string = keyof Commands & string, > implements Disposable { - // TODO: should this be a map? - // TODO: handle multiple command names private commands: Disposable[] = []; constructor( From 2b0bd840e616d941459b0451905588856f9c0240 Mon Sep 17 00:00:00 2001 From: Robert Date: Thu, 5 Oct 2023 15:25:42 +0100 Subject: [PATCH 23/46] Place setup before initialising listener --- .../ql-vscode/test/vscode-tests/no-workspace/telemetry.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/telemetry.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/telemetry.test.ts index 7e1b03eae..f07cc4e61 100644 --- a/extensions/ql-vscode/test/vscode-tests/no-workspace/telemetry.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/telemetry.test.ts @@ -424,11 +424,11 @@ describe("telemetry reporting", () => { // If the user ever turns global telemetry back on, then we can // show the dialog. + isTelemetryEnabledSpy.mockReturnValue(false); await enableTelemetry("telemetry", false); await ctx.globalState.update("telemetry-request-viewed", false); await telemetryListener.initialize(); - isTelemetryEnabledSpy.mockReturnValue(false); // popup should not be shown even though we have initialized telemetry expect(showInformationMessageSpy).not.toBeCalled(); From 0fabcda49b3acef564f5cd919b8763d17118ebb0 Mon Sep 17 00:00:00 2001 From: Robert Date: Thu, 5 Oct 2023 16:41:56 +0100 Subject: [PATCH 24/46] Convert RevealMethodMessage to only include method signature --- extensions/ql-vscode/src/common/interface-types.ts | 2 +- extensions/ql-vscode/src/model-editor/model-editor-view.ts | 2 +- extensions/ql-vscode/src/view/model-editor/ModelEditor.tsx | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/extensions/ql-vscode/src/common/interface-types.ts b/extensions/ql-vscode/src/common/interface-types.ts index b5444eb0c..4a552a14f 100644 --- a/extensions/ql-vscode/src/common/interface-types.ts +++ b/extensions/ql-vscode/src/common/interface-types.ts @@ -579,7 +579,7 @@ interface SetModeledMethodMessage { interface RevealMethodMessage { t: "revealMethod"; - method: Method; + methodSignature: string; } export type ToModelEditorMessage = diff --git a/extensions/ql-vscode/src/model-editor/model-editor-view.ts b/extensions/ql-vscode/src/model-editor/model-editor-view.ts index fdb1f8ffc..72d2cafa8 100644 --- a/extensions/ql-vscode/src/model-editor/model-editor-view.ts +++ b/extensions/ql-vscode/src/model-editor/model-editor-view.ts @@ -355,7 +355,7 @@ export class ModelEditorView extends AbstractWebview< await this.postMessage({ t: "revealMethod", - method, + methodSignature: method.signature, }); } diff --git a/extensions/ql-vscode/src/view/model-editor/ModelEditor.tsx b/extensions/ql-vscode/src/view/model-editor/ModelEditor.tsx index 1f130ee60..60dacefa9 100644 --- a/extensions/ql-vscode/src/view/model-editor/ModelEditor.tsx +++ b/extensions/ql-vscode/src/view/model-editor/ModelEditor.tsx @@ -142,7 +142,7 @@ export function ModelEditor({ ); break; case "revealMethod": - setRevealedMethodSignature(msg.method.signature); + setRevealedMethodSignature(msg.methodSignature); break; default: From 9d40d9a70369522dac7a7ef5695020e658e0acfa Mon Sep 17 00:00:00 2001 From: Robert Date: Wed, 4 Oct 2023 11:48:50 +0100 Subject: [PATCH 25/46] Remove method and modeledMethods from SaveModeledMethods message --- .../ql-vscode/src/common/interface-types.ts | 3 +- .../src/model-editor/model-editor-view.ts | 87 +++++++++++-------- .../src/model-editor/modeling-store.ts | 36 ++++++++ .../src/view/model-editor/LibraryRow.tsx | 9 +- .../src/view/model-editor/ModelEditor.tsx | 20 ++--- .../view/model-editor/ModeledMethodsList.tsx | 5 +- 6 files changed, 97 insertions(+), 63 deletions(-) diff --git a/extensions/ql-vscode/src/common/interface-types.ts b/extensions/ql-vscode/src/common/interface-types.ts index b5444eb0c..0c68edab6 100644 --- a/extensions/ql-vscode/src/common/interface-types.ts +++ b/extensions/ql-vscode/src/common/interface-types.ts @@ -543,8 +543,7 @@ interface RefreshMethods { interface SaveModeledMethods { t: "saveModeledMethods"; - methods: Method[]; - modeledMethods: Record; + methodSignatures?: string[]; } interface GenerateMethodMessage { diff --git a/extensions/ql-vscode/src/model-editor/model-editor-view.ts b/extensions/ql-vscode/src/model-editor/model-editor-view.ts index fdb1f8ffc..9d200c931 100644 --- a/extensions/ql-vscode/src/model-editor/model-editor-view.ts +++ b/extensions/ql-vscode/src/model-editor/model-editor-view.ts @@ -228,47 +228,58 @@ export class ModelEditorView extends AbstractWebview< break; case "saveModeledMethods": - await withProgress( - async (progress) => { - progress({ - step: 1, - maxStep: 500 + externalApiQueriesProgressMaxStep, - message: "Writing model files", - }); - await saveModeledMethods( - this.extensionPack, - this.databaseItem.language, - msg.methods, - msg.modeledMethods, - this.mode, - this.cliServer, - this.app.logger, - ); + { + const methods = this.modelingStore.getMethods( + this.databaseItem, + msg.methodSignatures, + ); + const modeledMethods = this.modelingStore.getModeledMethods( + this.databaseItem, + msg.methodSignatures, + ); - await Promise.all([ - this.setViewState(), - this.loadMethods((update) => - progress({ - ...update, - step: update.step + 500, - maxStep: 500 + externalApiQueriesProgressMaxStep, - }), - ), - ]); - }, - { - cancellable: false, - }, - ); + await withProgress( + async (progress) => { + progress({ + step: 1, + maxStep: 500 + externalApiQueriesProgressMaxStep, + message: "Writing model files", + }); + await saveModeledMethods( + this.extensionPack, + this.databaseItem.language, + methods, + modeledMethods, + this.mode, + this.cliServer, + this.app.logger, + ); - this.modelingStore.removeModifiedMethods( - this.databaseItem, - Object.keys(msg.modeledMethods), - ); + await Promise.all([ + this.setViewState(), + this.loadMethods((update) => + progress({ + ...update, + step: update.step + 500, + maxStep: 500 + externalApiQueriesProgressMaxStep, + }), + ), + ]); + }, + { + cancellable: false, + }, + ); - void telemetryListener?.sendUIInteraction( - "model-editor-save-modeled-methods", - ); + this.modelingStore.removeModifiedMethods( + this.databaseItem, + Object.keys(modeledMethods), + ); + + void telemetryListener?.sendUIInteraction( + "model-editor-save-modeled-methods", + ); + } break; case "generateMethod": diff --git a/extensions/ql-vscode/src/model-editor/modeling-store.ts b/extensions/ql-vscode/src/model-editor/modeling-store.ts index 2ed4ede2c..b089bcafd 100644 --- a/extensions/ql-vscode/src/model-editor/modeling-store.ts +++ b/extensions/ql-vscode/src/model-editor/modeling-store.ts @@ -154,6 +154,23 @@ export class ModelingStore extends DisposableObject { return this.state.get(this.activeDb); } + /** + * Returns the methods for the given database item and method signatures. + * If no method signatures are provided, returns all methods. + */ + public getMethods( + dbItem: DatabaseItem, + methodSignatures?: string[], + ): Method[] { + const methods = this.getState(dbItem).methods; + if (!methodSignatures) { + return methods; + } + return methods.filter((method) => + methodSignatures.includes(method.signature), + ); + } + public setMethods(dbItem: DatabaseItem, methods: Method[]) { const dbState = this.getState(dbItem); const dbUri = dbItem.databaseUri.toString(); @@ -182,6 +199,25 @@ export class ModelingStore extends DisposableObject { }); } + /** + * Returns the modeled methods for the given database item and method signatures. + * If no method signatures are provided, returns all modeled methods. + */ + public getModeledMethods( + dbItem: DatabaseItem, + methodSignatures?: string[], + ): Record { + const modeledMethods = this.getState(dbItem).modeledMethods; + if (!methodSignatures) { + return modeledMethods; + } + return Object.fromEntries( + Object.entries(modeledMethods).filter(([key]) => + methodSignatures.includes(key), + ), + ); + } + public addModeledMethods( dbItem: DatabaseItem, methods: Record, diff --git a/extensions/ql-vscode/src/view/model-editor/LibraryRow.tsx b/extensions/ql-vscode/src/view/model-editor/LibraryRow.tsx index 2c7871a69..b5b9d275c 100644 --- a/extensions/ql-vscode/src/view/model-editor/LibraryRow.tsx +++ b/extensions/ql-vscode/src/view/model-editor/LibraryRow.tsx @@ -78,10 +78,7 @@ export type LibraryRowProps = { hideModeledMethods: boolean; revealedMethodSignature: string | null; onChange: (modeledMethod: ModeledMethod) => void; - onSaveModelClick: ( - methods: Method[], - modeledMethods: Record, - ) => void; + onSaveModelClick: (methodSignatures: string[]) => void; onGenerateFromLlmClick: ( dependencyName: string, methods: Method[], @@ -165,11 +162,11 @@ export const LibraryRow = ({ const handleSave = useCallback( async (e: React.MouseEvent) => { - onSaveModelClick(methods, modeledMethods); + onSaveModelClick(methods.map((m) => m.signature)); e.stopPropagation(); e.preventDefault(); }, - [methods, modeledMethods, onSaveModelClick], + [methods, onSaveModelClick], ); const hasUnsavedChanges = useMemo(() => { diff --git a/extensions/ql-vscode/src/view/model-editor/ModelEditor.tsx b/extensions/ql-vscode/src/view/model-editor/ModelEditor.tsx index 1f130ee60..8c56510c2 100644 --- a/extensions/ql-vscode/src/view/model-editor/ModelEditor.tsx +++ b/extensions/ql-vscode/src/view/model-editor/ModelEditor.tsx @@ -196,21 +196,15 @@ export function ModelEditor({ const onSaveAllClick = useCallback(() => { vscode.postMessage({ t: "saveModeledMethods", - methods, - modeledMethods, }); - }, [methods, modeledMethods]); + }, []); - const onSaveModelClick = useCallback( - (methods: Method[], modeledMethods: Record) => { - vscode.postMessage({ - t: "saveModeledMethods", - methods, - modeledMethods, - }); - }, - [], - ); + const onSaveModelClick = useCallback((methodSignatures: string[]) => { + vscode.postMessage({ + t: "saveModeledMethods", + methodSignatures, + }); + }, []); const onGenerateFromSourceClick = useCallback(() => { vscode.postMessage({ diff --git a/extensions/ql-vscode/src/view/model-editor/ModeledMethodsList.tsx b/extensions/ql-vscode/src/view/model-editor/ModeledMethodsList.tsx index dfb812ff4..d59657ecc 100644 --- a/extensions/ql-vscode/src/view/model-editor/ModeledMethodsList.tsx +++ b/extensions/ql-vscode/src/view/model-editor/ModeledMethodsList.tsx @@ -20,10 +20,7 @@ export type ModeledMethodsListProps = { viewState: ModelEditorViewState; hideModeledMethods: boolean; onChange: (modeledMethod: ModeledMethod) => void; - onSaveModelClick: ( - methods: Method[], - modeledMethods: Record, - ) => void; + onSaveModelClick: (methodSignatures: string[]) => void; onGenerateFromLlmClick: ( packageName: string, methods: Method[], From db3d24236c944c12d039a101339b32c645eb2b02 Mon Sep 17 00:00:00 2001 From: Charis Kyriakou Date: Fri, 6 Oct 2023 09:34:54 +0100 Subject: [PATCH 26/46] Add empty states for modeling panel (#2914) --- extensions/ql-vscode/src/common/commands.ts | 1 + .../ql-vscode/src/common/interface-types.ts | 7 +- .../vscode/abstract-webview-view-provider.ts | 2 +- .../method-modeling-view-provider.ts | 6 + .../src/model-editor/model-editor-module.ts | 224 +++++++++--------- .../common/ResponsiveContainer.stories.tsx | 18 ++ .../NoMethodSelected.stories.tsx | 16 ++ .../NotInModelingMode.stories.tsx | 16 ++ .../src/view/common/ResponsiveContainer.tsx | 15 ++ .../view/method-modeling/NoMethodSelected.tsx | 8 + .../method-modeling/NotInModelingMode.tsx | 25 ++ 11 files changed, 227 insertions(+), 111 deletions(-) create mode 100644 extensions/ql-vscode/src/stories/common/ResponsiveContainer.stories.tsx create mode 100644 extensions/ql-vscode/src/stories/method-modeling/NoMethodSelected.stories.tsx create mode 100644 extensions/ql-vscode/src/stories/method-modeling/NotInModelingMode.stories.tsx create mode 100644 extensions/ql-vscode/src/view/common/ResponsiveContainer.tsx create mode 100644 extensions/ql-vscode/src/view/method-modeling/NoMethodSelected.tsx create mode 100644 extensions/ql-vscode/src/view/method-modeling/NotInModelingMode.tsx diff --git a/extensions/ql-vscode/src/common/commands.ts b/extensions/ql-vscode/src/common/commands.ts index d5e9a6a30..daa94b737 100644 --- a/extensions/ql-vscode/src/common/commands.ts +++ b/extensions/ql-vscode/src/common/commands.ts @@ -323,6 +323,7 @@ export type PackagingCommands = { export type ModelEditorCommands = { "codeQL.openModelEditor": () => Promise; + "codeQL.openModelEditorFromModelingPanel": () => Promise; "codeQLModelEditor.jumpToUsageLocation": ( method: Method, usage: Usage, diff --git a/extensions/ql-vscode/src/common/interface-types.ts b/extensions/ql-vscode/src/common/interface-types.ts index b5444eb0c..0c3f412ec 100644 --- a/extensions/ql-vscode/src/common/interface-types.ts +++ b/extensions/ql-vscode/src/common/interface-types.ts @@ -610,10 +610,15 @@ interface RevealInEditorMessage { method: Method; } +interface StartModelingMessage { + t: "startModeling"; +} + export type FromMethodModelingMessage = | CommonFromViewMessages | SetModeledMethodMessage - | RevealInEditorMessage; + | RevealInEditorMessage + | StartModelingMessage; interface SetMethodMessage { t: "setMethod"; diff --git a/extensions/ql-vscode/src/common/vscode/abstract-webview-view-provider.ts b/extensions/ql-vscode/src/common/vscode/abstract-webview-view-provider.ts index 05bcab308..83b07d974 100644 --- a/extensions/ql-vscode/src/common/vscode/abstract-webview-view-provider.ts +++ b/extensions/ql-vscode/src/common/vscode/abstract-webview-view-provider.ts @@ -13,7 +13,7 @@ export abstract class AbstractWebviewViewProvider< private disposables: Disposable[] = []; constructor( - private readonly app: App, + protected readonly app: App, private readonly webviewKind: WebviewKind, ) {} diff --git a/extensions/ql-vscode/src/model-editor/method-modeling/method-modeling-view-provider.ts b/extensions/ql-vscode/src/model-editor/method-modeling/method-modeling-view-provider.ts index 8b04153d6..7ce237679 100644 --- a/extensions/ql-vscode/src/model-editor/method-modeling/method-modeling-view-provider.ts +++ b/extensions/ql-vscode/src/model-editor/method-modeling/method-modeling-view-provider.ts @@ -92,6 +92,12 @@ export class MethodModelingViewProvider extends AbstractWebviewViewProvider< await this.revealInModelEditor(msg.method); break; + + case "startModeling": + await this.app.commands.execute( + "codeQL.openModelEditorFromModelingPanel", + ); + break; default: assertNever(msg); } diff --git a/extensions/ql-vscode/src/model-editor/model-editor-module.ts b/extensions/ql-vscode/src/model-editor/model-editor-module.ts index bb4ee6bb8..c5645de2c 100644 --- a/extensions/ql-vscode/src/model-editor/model-editor-module.ts +++ b/extensions/ql-vscode/src/model-editor/model-editor-module.ts @@ -73,115 +73,9 @@ export class ModelEditorModule extends DisposableObject { public getCommands(): ModelEditorCommands { return { - "codeQL.openModelEditor": async () => { - const db = this.databaseManager.currentDatabaseItem; - if (!db) { - void showAndLogErrorMessage(this.app.logger, "No database selected"); - return; - } - - const language = db.language; - if ( - !SUPPORTED_LANGUAGES.includes(language) || - !isQueryLanguage(language) - ) { - void showAndLogErrorMessage( - this.app.logger, - `The CodeQL Model Editor is not supported for ${language} databases.`, - ); - return; - } - - return withProgress( - async (progress) => { - const maxStep = 4; - - if (!(await this.cliServer.cliConstraints.supportsQlpacksKind())) { - void showAndLogErrorMessage( - this.app.logger, - `This feature requires CodeQL CLI version ${CliVersionConstraint.CLI_VERSION_WITH_QLPACKS_KIND.format()} or later.`, - ); - return; - } - - if ( - !(await this.cliServer.cliConstraints.supportsResolveExtensions()) - ) { - void showAndLogErrorMessage( - this.app.logger, - `This feature requires CodeQL CLI version ${CliVersionConstraint.CLI_VERSION_WITH_RESOLVE_EXTENSIONS.format()} or later.`, - ); - return; - } - - const modelFile = await pickExtensionPack( - this.cliServer, - db, - this.app.logger, - progress, - maxStep, - ); - if (!modelFile) { - return; - } - - progress({ - message: "Installing dependencies...", - step: 3, - maxStep, - }); - - // Create new temporary directory for query files and pack dependencies - const { path: queryDir, cleanup: cleanupQueryDir } = await dir({ - unsafeCleanup: true, - }); - - const success = await setUpPack(this.cliServer, queryDir, language); - if (!success) { - await cleanupQueryDir(); - return; - } - - progress({ - message: "Opening editor...", - step: 4, - maxStep, - }); - - const view = new ModelEditorView( - this.app, - this.modelingStore, - this.editorViewTracker, - this.databaseManager, - this.cliServer, - this.queryRunner, - this.queryStorageDir, - queryDir, - db, - modelFile, - Mode.Application, - ); - - this.modelingStore.onDbClosed(async (dbUri) => { - if (dbUri === db.databaseUri.toString()) { - await cleanupQueryDir(); - } - }); - - this.push(view); - this.push({ - dispose(): void { - void cleanupQueryDir(); - }, - }); - - await view.openView(); - }, - { - title: "Opening CodeQL Model Editor", - }, - ); - }, + "codeQL.openModelEditor": this.openModelEditor.bind(this), + "codeQL.openModelEditorFromModelingPanel": + this.openModelEditor.bind(this), "codeQLModelEditor.jumpToUsageLocation": async ( method: Method, usage: Usage, @@ -213,4 +107,116 @@ export class ModelEditorModule extends DisposableObject { await this.methodModelingPanel.setMethod(method); await showResolvableLocation(usage.url, databaseItem, this.app.logger); } + + private async openModelEditor(): Promise { + { + const db = this.databaseManager.currentDatabaseItem; + if (!db) { + void showAndLogErrorMessage(this.app.logger, "No database selected"); + return; + } + + const language = db.language; + if ( + !SUPPORTED_LANGUAGES.includes(language) || + !isQueryLanguage(language) + ) { + void showAndLogErrorMessage( + this.app.logger, + `The CodeQL Model Editor is not supported for ${language} databases.`, + ); + return; + } + + return withProgress( + async (progress) => { + const maxStep = 4; + + if (!(await this.cliServer.cliConstraints.supportsQlpacksKind())) { + void showAndLogErrorMessage( + this.app.logger, + `This feature requires CodeQL CLI version ${CliVersionConstraint.CLI_VERSION_WITH_QLPACKS_KIND.format()} or later.`, + ); + return; + } + + if ( + !(await this.cliServer.cliConstraints.supportsResolveExtensions()) + ) { + void showAndLogErrorMessage( + this.app.logger, + `This feature requires CodeQL CLI version ${CliVersionConstraint.CLI_VERSION_WITH_RESOLVE_EXTENSIONS.format()} or later.`, + ); + return; + } + + const modelFile = await pickExtensionPack( + this.cliServer, + db, + this.app.logger, + progress, + maxStep, + ); + if (!modelFile) { + return; + } + + progress({ + message: "Installing dependencies...", + step: 3, + maxStep, + }); + + // Create new temporary directory for query files and pack dependencies + const { path: queryDir, cleanup: cleanupQueryDir } = await dir({ + unsafeCleanup: true, + }); + + const success = await setUpPack(this.cliServer, queryDir, language); + if (!success) { + await cleanupQueryDir(); + return; + } + + progress({ + message: "Opening editor...", + step: 4, + maxStep, + }); + + const view = new ModelEditorView( + this.app, + this.modelingStore, + this.editorViewTracker, + this.databaseManager, + this.cliServer, + this.queryRunner, + this.queryStorageDir, + queryDir, + db, + modelFile, + Mode.Application, + ); + + this.modelingStore.onDbClosed(async (dbUri) => { + if (dbUri === db.databaseUri.toString()) { + await cleanupQueryDir(); + } + }); + + this.push(view); + this.push({ + dispose(): void { + void cleanupQueryDir(); + }, + }); + + await view.openView(); + }, + { + title: "Opening CodeQL Model Editor", + }, + ); + } + } } diff --git a/extensions/ql-vscode/src/stories/common/ResponsiveContainer.stories.tsx b/extensions/ql-vscode/src/stories/common/ResponsiveContainer.stories.tsx new file mode 100644 index 000000000..5639d147b --- /dev/null +++ b/extensions/ql-vscode/src/stories/common/ResponsiveContainer.stories.tsx @@ -0,0 +1,18 @@ +import * as React from "react"; + +import { Meta, StoryFn } from "@storybook/react"; + +import { ResponsiveContainer as ResponsiveContainerComponent } from "../../view/common/ResponsiveContainer"; + +export default { + title: "Responsive Container", + component: ResponsiveContainerComponent, +} as Meta; + +const Template: StoryFn = (args) => ( + + Hello + +); + +export const ResponsiveContainer = Template.bind({}); diff --git a/extensions/ql-vscode/src/stories/method-modeling/NoMethodSelected.stories.tsx b/extensions/ql-vscode/src/stories/method-modeling/NoMethodSelected.stories.tsx new file mode 100644 index 000000000..488170203 --- /dev/null +++ b/extensions/ql-vscode/src/stories/method-modeling/NoMethodSelected.stories.tsx @@ -0,0 +1,16 @@ +import * as React from "react"; + +import { Meta, StoryFn } from "@storybook/react"; + +import { NoMethodSelected as NoMethodSelectedComponent } from "../../view/method-modeling/NoMethodSelected"; + +export default { + title: "Method Modeling/No Method Selected", + component: NoMethodSelectedComponent, +} as Meta; + +const Template: StoryFn = () => ( + +); + +export const NoMethodSelected = Template.bind({}); diff --git a/extensions/ql-vscode/src/stories/method-modeling/NotInModelingMode.stories.tsx b/extensions/ql-vscode/src/stories/method-modeling/NotInModelingMode.stories.tsx new file mode 100644 index 000000000..654bbdb12 --- /dev/null +++ b/extensions/ql-vscode/src/stories/method-modeling/NotInModelingMode.stories.tsx @@ -0,0 +1,16 @@ +import * as React from "react"; + +import { Meta, StoryFn } from "@storybook/react"; + +import { NotInModelingMode as NotInModelingModeComponent } from "../../view/method-modeling/NotInModelingMode"; + +export default { + title: "Method Modeling/Not In Modeling Mode", + component: NotInModelingModeComponent, +} as Meta; + +const Template: StoryFn = () => ( + +); + +export const NotInModelingMode = Template.bind({}); diff --git a/extensions/ql-vscode/src/view/common/ResponsiveContainer.tsx b/extensions/ql-vscode/src/view/common/ResponsiveContainer.tsx new file mode 100644 index 000000000..5c73f582e --- /dev/null +++ b/extensions/ql-vscode/src/view/common/ResponsiveContainer.tsx @@ -0,0 +1,15 @@ +import { styled } from "styled-components"; + +export const ResponsiveContainer = styled.div` + display: flex; + flex-direction: column; + align-items: flex-start; + justify-content: flex-start; + height: 100vh; + + @media (min-height: 300px) { + align-items: center; + justify-content: center; + text-align: center; + } +`; diff --git a/extensions/ql-vscode/src/view/method-modeling/NoMethodSelected.tsx b/extensions/ql-vscode/src/view/method-modeling/NoMethodSelected.tsx new file mode 100644 index 000000000..bd4866d59 --- /dev/null +++ b/extensions/ql-vscode/src/view/method-modeling/NoMethodSelected.tsx @@ -0,0 +1,8 @@ +import * as React from "react"; +import { ResponsiveContainer } from "../common/ResponsiveContainer"; + +export const NoMethodSelected = () => { + return ( + Select an API or method to model + ); +}; diff --git a/extensions/ql-vscode/src/view/method-modeling/NotInModelingMode.tsx b/extensions/ql-vscode/src/view/method-modeling/NotInModelingMode.tsx new file mode 100644 index 000000000..fa120c8a1 --- /dev/null +++ b/extensions/ql-vscode/src/view/method-modeling/NotInModelingMode.tsx @@ -0,0 +1,25 @@ +import * as React from "react"; +import { useCallback } from "react"; +import { vscode } from "../vscode-api"; +import { styled } from "styled-components"; +import TextButton from "../common/TextButton"; +import { ResponsiveContainer } from "../common/ResponsiveContainer"; + +const Button = styled(TextButton)` + margin-top: 0.2rem; +`; + +export const NotInModelingMode = () => { + const handleClick = useCallback(() => { + vscode.postMessage({ + t: "startModeling", + }); + }, []); + + return ( + + Not in modeling mode + + + ); +}; From 08a4457e39c2db6a81bedfef22cddc4ce18e3e77 Mon Sep 17 00:00:00 2001 From: Henry Mercer Date: Fri, 6 Oct 2023 10:38:14 +0100 Subject: [PATCH 27/46] Packaging: Remove ML-powered queries pack from known list ML-powered queries are [now deprecated](https://github.blog/changelog/2023-09-29-codeql-code-scanning-deprecates-ml-powered-alerts/). --- extensions/ql-vscode/src/common/query-language.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/extensions/ql-vscode/src/common/query-language.ts b/extensions/ql-vscode/src/common/query-language.ts index 02d728054..2cf0155b5 100644 --- a/extensions/ql-vscode/src/common/query-language.ts +++ b/extensions/ql-vscode/src/common/query-language.ts @@ -40,10 +40,7 @@ export const PACKS_BY_QUERY_LANGUAGE = { ], [QueryLanguage.Go]: ["codeql/go-queries"], [QueryLanguage.Java]: ["codeql/java-queries"], - [QueryLanguage.Javascript]: [ - "codeql/javascript-queries", - "codeql/javascript-experimental-atm-queries", - ], + [QueryLanguage.Javascript]: ["codeql/javascript-queries"], [QueryLanguage.Python]: ["codeql/python-queries"], [QueryLanguage.Ruby]: ["codeql/ruby-queries"], }; From 9c275130a5a36281da1486d3daaafbd83de0e9f3 Mon Sep 17 00:00:00 2001 From: Koen Vlaswinkel Date: Fri, 6 Oct 2023 12:09:00 +0200 Subject: [PATCH 28/46] Fix duplicate modeled methods Co-authored-by: Robert --- extensions/ql-vscode/src/model-editor/modeled-method-fs.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions/ql-vscode/src/model-editor/modeled-method-fs.ts b/extensions/ql-vscode/src/model-editor/modeled-method-fs.ts index 10a92ef63..5e8ad9347 100644 --- a/extensions/ql-vscode/src/model-editor/modeled-method-fs.ts +++ b/extensions/ql-vscode/src/model-editor/modeled-method-fs.ts @@ -89,7 +89,7 @@ export async function loadModeledMethods( for (const modeledMethods of Object.values(modeledMethodsByFile)) { for (const [key, value] of Object.entries(modeledMethods)) { if (!(key in existingModeledMethods)) { - existingModeledMethods[key] = value; + existingModeledMethods[key] = []; } existingModeledMethods[key].push(...value); From 20c63921f742620225c087ebc948feb514b28bcc Mon Sep 17 00:00:00 2001 From: Charis Kyriakou Date: Fri, 6 Oct 2023 11:27:16 +0100 Subject: [PATCH 29/46] Wire up modeling panel empty states (#2915) --- extensions/ql-vscode/package.json | 2 +- .../ql-vscode/src/common/interface-types.ts | 8 +++- .../method-modeling-view-provider.ts | 38 +++++++++++++++---- .../src/model-editor/model-editor-view.ts | 27 ------------- .../src/model-editor/modeling-store.ts | 15 ++++++++ .../method-modeling/MethodModelingView.tsx | 13 ++++++- 6 files changed, 65 insertions(+), 38 deletions(-) diff --git a/extensions/ql-vscode/package.json b/extensions/ql-vscode/package.json index ea7e9efad..447f5b34c 100644 --- a/extensions/ql-vscode/package.json +++ b/extensions/ql-vscode/package.json @@ -1996,7 +1996,7 @@ "id": "codeQLMethodModeling", "type": "webview", "name": "CodeQL Method Modeling", - "when": "config.codeQL.canary && config.codeQL.model.methodModelingView && codeql.modelEditorOpen && !codeql.modelEditorActive" + "when": "config.codeQL.canary && config.codeQL.model.methodModelingView" } ], "codeql-methods-usage": [ diff --git a/extensions/ql-vscode/src/common/interface-types.ts b/extensions/ql-vscode/src/common/interface-types.ts index 0c3f412ec..f1af10fab 100644 --- a/extensions/ql-vscode/src/common/interface-types.ts +++ b/extensions/ql-vscode/src/common/interface-types.ts @@ -577,6 +577,11 @@ interface SetModeledMethodMessage { method: ModeledMethod; } +interface SetInModelingModeMessage { + t: "setInModelingMode"; + inModelingMode: boolean; +} + interface RevealMethodMessage { t: "revealMethod"; method: Method; @@ -641,4 +646,5 @@ export type ToMethodModelingMessage = | SetMethodMessage | SetModeledMethodMessage | SetMethodModifiedMessage - | SetSelectedMethodMessage; + | SetSelectedMethodMessage + | SetInModelingModeMessage; diff --git a/extensions/ql-vscode/src/model-editor/method-modeling/method-modeling-view-provider.ts b/extensions/ql-vscode/src/model-editor/method-modeling/method-modeling-view-provider.ts index 7ce237679..41706c2bb 100644 --- a/extensions/ql-vscode/src/model-editor/method-modeling/method-modeling-view-provider.ts +++ b/extensions/ql-vscode/src/model-editor/method-modeling/method-modeling-view-provider.ts @@ -46,14 +46,16 @@ export class MethodModelingViewProvider extends AbstractWebviewViewProvider< } private setInitialState(): void { - const selectedMethod = this.modelingStore.getSelectedMethodDetails(); - if (selectedMethod) { - void this.postMessage({ - t: "setSelectedMethod", - method: selectedMethod.method, - modeledMethod: selectedMethod.modeledMethod, - isModified: selectedMethod.isModified, - }); + if (this.modelingStore.hasStateForActiveDb()) { + const selectedMethod = this.modelingStore.getSelectedMethodDetails(); + if (selectedMethod) { + void this.postMessage({ + t: "setSelectedMethod", + method: selectedMethod.method, + modeledMethod: selectedMethod.modeledMethod, + isModified: selectedMethod.isModified, + }); + } } } @@ -165,5 +167,25 @@ export class MethodModelingViewProvider extends AbstractWebviewViewProvider< } }), ); + + this.push( + this.modelingStore.onDbOpened(async () => { + await this.postMessage({ + t: "setInModelingMode", + inModelingMode: true, + }); + }), + ); + + this.push( + this.modelingStore.onDbClosed(async () => { + if (!this.modelingStore.anyDbsBeingModeled()) { + await this.postMessage({ + t: "setInModelingMode", + inModelingMode: false, + }); + } + }), + ); } } diff --git a/extensions/ql-vscode/src/model-editor/model-editor-view.ts b/extensions/ql-vscode/src/model-editor/model-editor-view.ts index fdb1f8ffc..a2a0e42ba 100644 --- a/extensions/ql-vscode/src/model-editor/model-editor-view.ts +++ b/extensions/ql-vscode/src/model-editor/model-editor-view.ts @@ -100,9 +100,6 @@ export class ModelEditorView extends AbstractWebview< panel.onDidChangeViewState(async () => { if (panel.active) { this.modelingStore.setActiveDb(this.databaseItem); - await this.markModelEditorAsActive(); - } else { - await this.updateModelEditorActiveContext(); } }); @@ -126,36 +123,12 @@ export class ModelEditorView extends AbstractWebview< ); } - private async markModelEditorAsActive(): Promise { - void this.app.commands.execute( - "setContext", - "codeql.modelEditorActive", - true, - ); - } - - private async updateModelEditorActiveContext(): Promise { - await this.app.commands.execute( - "setContext", - "codeql.modelEditorActive", - this.isAModelEditorActive(), - ); - } - private isAModelEditorOpen(): boolean { return window.tabGroups.all.some((tabGroup) => tabGroup.tabs.some((tab) => this.isTabModelEditorView(tab)), ); } - private isAModelEditorActive(): boolean { - return window.tabGroups.all.some((tabGroup) => - tabGroup.tabs.some( - (tab) => this.isTabModelEditorView(tab) && tab.isActive, - ), - ); - } - private isTabModelEditorView(tab: Tab): boolean { if (!(tab.input instanceof TabInputWebview)) { return false; diff --git a/extensions/ql-vscode/src/model-editor/modeling-store.ts b/extensions/ql-vscode/src/model-editor/modeling-store.ts index 2ed4ede2c..c6e27986d 100644 --- a/extensions/ql-vscode/src/model-editor/modeling-store.ts +++ b/extensions/ql-vscode/src/model-editor/modeling-store.ts @@ -49,6 +49,7 @@ interface SelectedMethodChangedEvent { export class ModelingStore extends DisposableObject { public readonly onActiveDbChanged: AppEvent; + public readonly onDbOpened: AppEvent; public readonly onDbClosed: AppEvent; public readonly onMethodsChanged: AppEvent; public readonly onHideModeledMethodsChanged: AppEvent; @@ -60,6 +61,7 @@ export class ModelingStore extends DisposableObject { private activeDb: string | undefined; private readonly onActiveDbChangedEventEmitter: AppEventEmitter; + private readonly onDbOpenedEventEmitter: AppEventEmitter; private readonly onDbClosedEventEmitter: AppEventEmitter; private readonly onMethodsChangedEventEmitter: AppEventEmitter; private readonly onHideModeledMethodsChangedEventEmitter: AppEventEmitter; @@ -79,6 +81,9 @@ export class ModelingStore extends DisposableObject { ); this.onActiveDbChanged = this.onActiveDbChangedEventEmitter.event; + this.onDbOpenedEventEmitter = this.push(app.createEventEmitter()); + this.onDbOpened = this.onDbOpenedEventEmitter.event; + this.onDbClosedEventEmitter = this.push(app.createEventEmitter()); this.onDbClosed = this.onDbClosedEventEmitter.event; @@ -123,6 +128,8 @@ export class ModelingStore extends DisposableObject { selectedMethod: undefined, selectedUsage: undefined, }); + + this.onDbOpenedEventEmitter.fire(dbUri); } public setActiveDb(databaseItem: DatabaseItem) { @@ -154,6 +161,14 @@ export class ModelingStore extends DisposableObject { return this.state.get(this.activeDb); } + public hasStateForActiveDb(): boolean { + return !!this.getStateForActiveDb(); + } + + public anyDbsBeingModeled(): boolean { + return this.state.size > 0; + } + public setMethods(dbItem: DatabaseItem, methods: Method[]) { const dbState = this.getState(dbItem); const dbUri = dbItem.databaseUri.toString(); diff --git a/extensions/ql-vscode/src/view/method-modeling/MethodModelingView.tsx b/extensions/ql-vscode/src/view/method-modeling/MethodModelingView.tsx index 390d2fde9..0974f3a50 100644 --- a/extensions/ql-vscode/src/view/method-modeling/MethodModelingView.tsx +++ b/extensions/ql-vscode/src/view/method-modeling/MethodModelingView.tsx @@ -7,8 +7,12 @@ import { ToMethodModelingMessage } from "../../common/interface-types"; import { assertNever } from "../../common/helpers-pure"; import { ModeledMethod } from "../../model-editor/modeled-method"; import { vscode } from "../vscode-api"; +import { NotInModelingMode } from "./NotInModelingMode"; +import { NoMethodSelected } from "./NoMethodSelected"; export function MethodModelingView(): JSX.Element { + const [inModelingMode, setInModelingMode] = useState(false); + const [method, setMethod] = useState(undefined); const [modeledMethod, setModeledMethod] = React.useState< @@ -27,6 +31,9 @@ export function MethodModelingView(): JSX.Element { if (evt.origin === window.origin) { const msg: ToMethodModelingMessage = evt.data; switch (msg.t) { + case "setInModelingMode": + setInModelingMode(msg.inModelingMode); + break; case "setMethod": setMethod(msg.method); break; @@ -57,8 +64,12 @@ export function MethodModelingView(): JSX.Element { }; }, []); + if (!inModelingMode) { + return ; + } + if (!method) { - return <>Select method to model; + return ; } const onChange = (modeledMethod: ModeledMethod) => { From 1d691c2b7fb983099b7479f41332584a1ca1ad34 Mon Sep 17 00:00:00 2001 From: Charis Kyriakou Date: Fri, 6 Oct 2023 11:57:52 +0100 Subject: [PATCH 30/46] UI polish for modeling panel (#2917) * Don't push content down with unsaved tag * Adjust spacing between components --- .../view/method-modeling/MethodModeling.tsx | 26 +++++++++++++++---- .../method-modeling/MethodModelingInputs.tsx | 7 +++-- .../method-modeling/ReviewInEditorButton.tsx | 2 +- 3 files changed, 27 insertions(+), 8 deletions(-) diff --git a/extensions/ql-vscode/src/view/method-modeling/MethodModeling.tsx b/extensions/ql-vscode/src/view/method-modeling/MethodModeling.tsx index 0e1d16d43..f64d4b25e 100644 --- a/extensions/ql-vscode/src/view/method-modeling/MethodModeling.tsx +++ b/extensions/ql-vscode/src/view/method-modeling/MethodModeling.tsx @@ -10,17 +10,18 @@ import { VSCodeTag } from "@vscode/webview-ui-toolkit/react"; import { ReviewInEditorButton } from "./ReviewInEditorButton"; const Container = styled.div` - padding: 0.3rem; + padding-top: 0.5rem; margin-bottom: 1rem; width: 100%; `; const Title = styled.div` - padding-bottom: 0.3rem; - font-size: 0.7rem; + padding-bottom: 0.5rem; + font-size: 0.9rem; text-transform: uppercase; display: flex; justify-content: space-between; + align-items: center; `; const DependencyContainer = styled.div` @@ -34,8 +35,23 @@ const DependencyContainer = styled.div` padding: 0.5rem; word-wrap: break-word; word-break: break-all; + margin-bottom: 0.8rem; `; +const StyledMethodModelingInputs = styled(MethodModelingInputs)` + padding-bottom: 0.5rem; +`; + +const StyledVSCodeTag = styled(VSCodeTag)<{ visible: boolean }>` + visibility: ${(props) => (props.visible ? "visible" : "hidden")}; +`; + +const UnsavedTag = ({ modelingStatus }: { modelingStatus: ModelingStatus }) => ( + + Unsaved + +); + export type MethodModelingProps = { modelingStatus: ModelingStatus; method: Method; @@ -54,13 +70,13 @@ export const MethodModeling = ({ {method.packageName} {method.libraryVersion && <>@{method.libraryVersion}</>} - {modelingStatus === "unsaved" ? <VSCodeTag>Unsaved</VSCodeTag> : null} + <UnsavedTag modelingStatus={modelingStatus} /> - Date: Fri, 6 Oct 2023 14:04:06 +0200 Subject: [PATCH 31/46] Add view state to method modeling panel This adds a view state to the method modeling panel similar to the model editor. This will be used to send the state of the show multiple models feature flag to the webview so this can be used to selectively show/hide components in the method modeling panel. --- .../ql-vscode/src/common/interface-types.ts | 11 +++++++++- .../method-modeling-view-provider.ts | 20 ++++++++++++++----- .../src/model-editor/shared/view-state.ts | 4 ++++ .../view/method-modeling/MethodModeling.tsx | 1 + .../method-modeling/MethodModelingView.tsx | 14 ++++++++++++- 5 files changed, 43 insertions(+), 7 deletions(-) diff --git a/extensions/ql-vscode/src/common/interface-types.ts b/extensions/ql-vscode/src/common/interface-types.ts index f1af10fab..650d48498 100644 --- a/extensions/ql-vscode/src/common/interface-types.ts +++ b/extensions/ql-vscode/src/common/interface-types.ts @@ -19,7 +19,10 @@ import { ErrorLike } from "../common/errors"; import { DataFlowPaths } from "../variant-analysis/shared/data-flow-paths"; import { Method, Usage } from "../model-editor/method"; import { ModeledMethod } from "../model-editor/modeled-method"; -import { ModelEditorViewState } from "../model-editor/shared/view-state"; +import { + MethodModelingPanelViewState, + ModelEditorViewState, +} from "../model-editor/shared/view-state"; import { Mode } from "../model-editor/shared/mode"; import { QueryLanguage } from "./query-language"; @@ -625,6 +628,11 @@ export type FromMethodModelingMessage = | RevealInEditorMessage | StartModelingMessage; +interface SetMethodModelingPanelViewStateMessage { + t: "setMethodModelingPanelViewState"; + viewState: MethodModelingPanelViewState; +} + interface SetMethodMessage { t: "setMethod"; method: Method; @@ -643,6 +651,7 @@ interface SetSelectedMethodMessage { } export type ToMethodModelingMessage = + | SetMethodModelingPanelViewStateMessage | SetMethodMessage | SetModeledMethodMessage | SetMethodModifiedMessage diff --git a/extensions/ql-vscode/src/model-editor/method-modeling/method-modeling-view-provider.ts b/extensions/ql-vscode/src/model-editor/method-modeling/method-modeling-view-provider.ts index 41706c2bb..e80c5528a 100644 --- a/extensions/ql-vscode/src/model-editor/method-modeling/method-modeling-view-provider.ts +++ b/extensions/ql-vscode/src/model-editor/method-modeling/method-modeling-view-provider.ts @@ -12,6 +12,7 @@ import { DbModelingState, ModelingStore } from "../modeling-store"; import { AbstractWebviewViewProvider } from "../../common/vscode/abstract-webview-view-provider"; import { assertNever } from "../../common/helpers-pure"; import { ModelEditorViewTracker } from "../model-editor-view-tracker"; +import { showMultipleModels } from "../../config"; export class MethodModelingViewProvider extends AbstractWebviewViewProvider< ToMethodModelingMessage, @@ -29,11 +30,20 @@ export class MethodModelingViewProvider extends AbstractWebviewViewProvider< super(app, "method-modeling"); } - protected override onWebViewLoaded(): void { - this.setInitialState(); + protected override async onWebViewLoaded(): Promise { + await Promise.all([this.setViewState(), this.setInitialState()]); this.registerToModelingStoreEvents(); } + private async setViewState(): Promise { + await this.postMessage({ + t: "setMethodModelingPanelViewState", + viewState: { + showMultipleModels: showMultipleModels(), + }, + }); + } + public async setMethod(method: Method): Promise { this.method = method; @@ -45,11 +55,11 @@ export class MethodModelingViewProvider extends AbstractWebviewViewProvider< } } - private setInitialState(): void { + private async setInitialState(): Promise { if (this.modelingStore.hasStateForActiveDb()) { const selectedMethod = this.modelingStore.getSelectedMethodDetails(); if (selectedMethod) { - void this.postMessage({ + await this.postMessage({ t: "setSelectedMethod", method: selectedMethod.method, modeledMethod: selectedMethod.modeledMethod, @@ -64,7 +74,7 @@ export class MethodModelingViewProvider extends AbstractWebviewViewProvider< ): Promise { switch (msg.t) { case "viewLoaded": - this.onWebViewLoaded(); + await this.onWebViewLoaded(); break; case "telemetry": diff --git a/extensions/ql-vscode/src/model-editor/shared/view-state.ts b/extensions/ql-vscode/src/model-editor/shared/view-state.ts index e16036b5c..f7250916b 100644 --- a/extensions/ql-vscode/src/model-editor/shared/view-state.ts +++ b/extensions/ql-vscode/src/model-editor/shared/view-state.ts @@ -8,3 +8,7 @@ export interface ModelEditorViewState { showMultipleModels: boolean; mode: Mode; } + +export interface MethodModelingPanelViewState { + showMultipleModels: boolean; +} diff --git a/extensions/ql-vscode/src/view/method-modeling/MethodModeling.tsx b/extensions/ql-vscode/src/view/method-modeling/MethodModeling.tsx index 0e1d16d43..3743c2e76 100644 --- a/extensions/ql-vscode/src/view/method-modeling/MethodModeling.tsx +++ b/extensions/ql-vscode/src/view/method-modeling/MethodModeling.tsx @@ -40,6 +40,7 @@ export type MethodModelingProps = { modelingStatus: ModelingStatus; method: Method; modeledMethod: ModeledMethod | undefined; + showMultipleModels?: boolean; onChange: (modeledMethod: ModeledMethod) => void; }; diff --git a/extensions/ql-vscode/src/view/method-modeling/MethodModelingView.tsx b/extensions/ql-vscode/src/view/method-modeling/MethodModelingView.tsx index 0974f3a50..2c32c3ee5 100644 --- a/extensions/ql-vscode/src/view/method-modeling/MethodModelingView.tsx +++ b/extensions/ql-vscode/src/view/method-modeling/MethodModelingView.tsx @@ -9,8 +9,16 @@ import { ModeledMethod } from "../../model-editor/modeled-method"; import { vscode } from "../vscode-api"; import { NotInModelingMode } from "./NotInModelingMode"; import { NoMethodSelected } from "./NoMethodSelected"; +import { MethodModelingPanelViewState } from "../../model-editor/shared/view-state"; -export function MethodModelingView(): JSX.Element { +type Props = { + initialViewState?: MethodModelingPanelViewState; +}; + +export function MethodModelingView({ initialViewState }: Props): JSX.Element { + const [viewState, setViewState] = useState< + MethodModelingPanelViewState | undefined + >(initialViewState); const [inModelingMode, setInModelingMode] = useState(false); const [method, setMethod] = useState(undefined); @@ -31,6 +39,9 @@ export function MethodModelingView(): JSX.Element { if (evt.origin === window.origin) { const msg: ToMethodModelingMessage = evt.data; switch (msg.t) { + case "setMethodModelingPanelViewState": + setViewState(msg.viewState); + break; case "setInModelingMode": setInModelingMode(msg.inModelingMode); break; @@ -84,6 +95,7 @@ export function MethodModelingView(): JSX.Element { modelingStatus={modelingStatus} method={method} modeledMethod={modeledMethod} + showMultipleModels={viewState?.showMultipleModels} onChange={onChange} /> ); From 08944a292c79d5c4ffdb6c6218999e190a84dd7a Mon Sep 17 00:00:00 2001 From: Koen Vlaswinkel Date: Fri, 6 Oct 2023 14:15:23 +0200 Subject: [PATCH 32/46] Update method modeling panel after changes to config This updates the method modeling panel's view state when the `codeQL.model.showMultipleModels` setting changes. This will ensure that the setting updates without needing to restart VS Code since this view is much harder to restart than the model editor. --- extensions/ql-vscode/src/config.ts | 29 +++++++++++++++++++ .../method-modeling/method-modeling-panel.ts | 7 +++++ .../method-modeling-view-provider.ts | 14 +++++++-- 3 files changed, 48 insertions(+), 2 deletions(-) diff --git a/extensions/ql-vscode/src/config.ts b/extensions/ql-vscode/src/config.ts index 94ddebdf3..19accd73c 100644 --- a/extensions/ql-vscode/src/config.ts +++ b/extensions/ql-vscode/src/config.ts @@ -711,6 +711,35 @@ const LLM_GENERATION = new Setting("llmGeneration", MODEL_SETTING); const EXTENSIONS_DIRECTORY = new Setting("extensionsDirectory", MODEL_SETTING); const SHOW_MULTIPLE_MODELS = new Setting("showMultipleModels", MODEL_SETTING); +export interface ModelConfig { + flowGeneration: boolean; + llmGeneration: boolean; + extensionsDirectory: string | undefined; + showMultipleModels: boolean; +} + +export class ModelConfigListener extends ConfigListener implements ModelConfig { + protected handleDidChangeConfiguration(e: ConfigurationChangeEvent): void { + this.handleDidChangeConfigurationForRelevantSettings([MODEL_SETTING], e); + } + + public get flowGeneration(): boolean { + return !!FLOW_GENERATION.getValue(); + } + + public get llmGeneration(): boolean { + return !!LLM_GENERATION.getValue(); + } + + public get extensionsDirectory(): string | undefined { + return EXTENSIONS_DIRECTORY.getValue(); + } + + public get showMultipleModels(): boolean { + return !!SHOW_MULTIPLE_MODELS.getValue(); + } +} + export function showFlowGeneration(): boolean { return !!FLOW_GENERATION.getValue(); } diff --git a/extensions/ql-vscode/src/model-editor/method-modeling/method-modeling-panel.ts b/extensions/ql-vscode/src/model-editor/method-modeling/method-modeling-panel.ts index f120bcd19..e4e54694b 100644 --- a/extensions/ql-vscode/src/model-editor/method-modeling/method-modeling-panel.ts +++ b/extensions/ql-vscode/src/model-editor/method-modeling/method-modeling-panel.ts @@ -5,6 +5,7 @@ import { MethodModelingViewProvider } from "./method-modeling-view-provider"; import { Method } from "../method"; import { ModelingStore } from "../modeling-store"; import { ModelEditorViewTracker } from "../model-editor-view-tracker"; +import { ModelConfigListener } from "../../config"; export class MethodModelingPanel extends DisposableObject { private readonly provider: MethodModelingViewProvider; @@ -16,10 +17,16 @@ export class MethodModelingPanel extends DisposableObject { ) { super(); + // This is here instead of in MethodModelingViewProvider because we need to + // dispose this when the extension gets disposed, not when the webview gets + // disposed. + const modelConfig = this.push(new ModelConfigListener()); + this.provider = new MethodModelingViewProvider( app, modelingStore, editorViewTracker, + modelConfig, ); this.push( window.registerWebviewViewProvider( diff --git a/extensions/ql-vscode/src/model-editor/method-modeling/method-modeling-view-provider.ts b/extensions/ql-vscode/src/model-editor/method-modeling/method-modeling-view-provider.ts index e80c5528a..207807836 100644 --- a/extensions/ql-vscode/src/model-editor/method-modeling/method-modeling-view-provider.ts +++ b/extensions/ql-vscode/src/model-editor/method-modeling/method-modeling-view-provider.ts @@ -12,7 +12,7 @@ import { DbModelingState, ModelingStore } from "../modeling-store"; import { AbstractWebviewViewProvider } from "../../common/vscode/abstract-webview-view-provider"; import { assertNever } from "../../common/helpers-pure"; import { ModelEditorViewTracker } from "../model-editor-view-tracker"; -import { showMultipleModels } from "../../config"; +import { ModelConfigListener } from "../../config"; export class MethodModelingViewProvider extends AbstractWebviewViewProvider< ToMethodModelingMessage, @@ -26,6 +26,7 @@ export class MethodModelingViewProvider extends AbstractWebviewViewProvider< app: App, private readonly modelingStore: ModelingStore, private readonly editorViewTracker: ModelEditorViewTracker, + private readonly modelConfig: ModelConfigListener, ) { super(app, "method-modeling"); } @@ -33,13 +34,14 @@ export class MethodModelingViewProvider extends AbstractWebviewViewProvider< protected override async onWebViewLoaded(): Promise { await Promise.all([this.setViewState(), this.setInitialState()]); this.registerToModelingStoreEvents(); + this.registerToModelConfigEvents(); } private async setViewState(): Promise { await this.postMessage({ t: "setMethodModelingPanelViewState", viewState: { - showMultipleModels: showMultipleModels(), + showMultipleModels: this.modelConfig.showMultipleModels, }, }); } @@ -198,4 +200,12 @@ export class MethodModelingViewProvider extends AbstractWebviewViewProvider< }), ); } + + private registerToModelConfigEvents(): void { + this.push( + this.modelConfig.onDidChangeConfiguration(() => { + void this.setViewState(); + }), + ); + } } From 951bd1388196f1970f3d0334e2215be263a42da5 Mon Sep 17 00:00:00 2001 From: Koen Vlaswinkel Date: Fri, 6 Oct 2023 14:29:07 +0200 Subject: [PATCH 33/46] Use `ModelConfig` for all model settings This switches all places where we're retrieving some model configuration to use the `ModelConfig` or `ModelConfigListener` types. This makes it much easier to mock these settings in tests. This also adds a listener to the `ModelEditorView` to send the new view state when any of the settings is changed. This should make it easier to test settings changes in the model editor without having to re-open the model editor. --- extensions/ql-vscode/src/config.ts | 26 +---- .../src/model-editor/extension-pack-picker.ts | 7 +- .../src/model-editor/model-editor-module.ts | 12 +- .../src/model-editor/model-editor-queries.ts | 6 +- .../src/model-editor/model-editor-view.ts | 26 +++-- .../extension-pack-picker.test.ts | 109 ++++-------------- .../model-editor/model-editor-queries.test.ts | 11 +- .../model-editor/model-editor-view.test.ts | 5 + 8 files changed, 80 insertions(+), 122 deletions(-) diff --git a/extensions/ql-vscode/src/config.ts b/extensions/ql-vscode/src/config.ts index 19accd73c..1330de8a3 100644 --- a/extensions/ql-vscode/src/config.ts +++ b/extensions/ql-vscode/src/config.ts @@ -714,7 +714,7 @@ const SHOW_MULTIPLE_MODELS = new Setting("showMultipleModels", MODEL_SETTING); export interface ModelConfig { flowGeneration: boolean; llmGeneration: boolean; - extensionsDirectory: string | undefined; + getExtensionsDirectory(languageId: string): string | undefined; showMultipleModels: boolean; } @@ -731,29 +731,13 @@ export class ModelConfigListener extends ConfigListener implements ModelConfig { return !!LLM_GENERATION.getValue(); } - public get extensionsDirectory(): string | undefined { - return EXTENSIONS_DIRECTORY.getValue(); + public getExtensionsDirectory(languageId: string): string | undefined { + return EXTENSIONS_DIRECTORY.getValue({ + languageId, + }); } public get showMultipleModels(): boolean { return !!SHOW_MULTIPLE_MODELS.getValue(); } } - -export function showFlowGeneration(): boolean { - return !!FLOW_GENERATION.getValue(); -} - -export function showLlmGeneration(): boolean { - return !!LLM_GENERATION.getValue(); -} - -export function getExtensionsDirectory(languageId: string): string | undefined { - return EXTENSIONS_DIRECTORY.getValue({ - languageId, - }); -} - -export function showMultipleModels(): boolean { - return !!SHOW_MULTIPLE_MODELS.getValue(); -} diff --git a/extensions/ql-vscode/src/model-editor/extension-pack-picker.ts b/extensions/ql-vscode/src/model-editor/extension-pack-picker.ts index a02251723..c970002d0 100644 --- a/extensions/ql-vscode/src/model-editor/extension-pack-picker.ts +++ b/extensions/ql-vscode/src/model-editor/extension-pack-picker.ts @@ -11,7 +11,7 @@ import { getQlPackPath, QLPACK_FILENAMES } from "../common/ql"; import { getErrorMessage } from "../common/helpers-pure"; import { ExtensionPack } from "./shared/extension-pack"; import { NotificationLogger, showAndLogErrorMessage } from "../common/logging"; -import { getExtensionsDirectory } from "../config"; +import { ModelConfig } from "../config"; import { autoNameExtensionPack, ExtensionPackName, @@ -28,6 +28,7 @@ const extensionPackValidate = ajv.compile(extensionPackMetadataSchemaJson); export async function pickExtensionPack( cliServer: Pick, databaseItem: Pick, + modelConfig: ModelConfig, logger: NotificationLogger, progress: ProgressCallback, maxStep: number, @@ -56,7 +57,9 @@ export async function pickExtensionPack( }); // Get the `codeQL.model.extensionsDirectory` setting for the language - const userExtensionsDirectory = getExtensionsDirectory(databaseItem.language); + const userExtensionsDirectory = modelConfig.getExtensionsDirectory( + databaseItem.language, + ); // If the setting is not set, automatically pick a suitable directory const extensionsDirectory = userExtensionsDirectory diff --git a/extensions/ql-vscode/src/model-editor/model-editor-module.ts b/extensions/ql-vscode/src/model-editor/model-editor-module.ts index c5645de2c..1eb7c0f17 100644 --- a/extensions/ql-vscode/src/model-editor/model-editor-module.ts +++ b/extensions/ql-vscode/src/model-editor/model-editor-module.ts @@ -21,6 +21,7 @@ import { MethodModelingPanel } from "./method-modeling/method-modeling-panel"; import { ModelingStore } from "./modeling-store"; import { showResolvableLocation } from "../databases/local-databases/locations"; import { ModelEditorViewTracker } from "./model-editor-view-tracker"; +import { ModelConfigListener } from "../config"; const SUPPORTED_LANGUAGES: string[] = ["java", "csharp"]; @@ -150,9 +151,12 @@ export class ModelEditorModule extends DisposableObject { return; } + const modelConfig = this.push(new ModelConfigListener()); + const modelFile = await pickExtensionPack( this.cliServer, db, + modelConfig, this.app.logger, progress, maxStep, @@ -172,7 +176,12 @@ export class ModelEditorModule extends DisposableObject { unsafeCleanup: true, }); - const success = await setUpPack(this.cliServer, queryDir, language); + const success = await setUpPack( + this.cliServer, + queryDir, + language, + modelConfig, + ); if (!success) { await cleanupQueryDir(); return; @@ -188,6 +197,7 @@ export class ModelEditorModule extends DisposableObject { this.app, this.modelingStore, this.editorViewTracker, + modelConfig, this.databaseManager, this.cliServer, this.queryRunner, diff --git a/extensions/ql-vscode/src/model-editor/model-editor-queries.ts b/extensions/ql-vscode/src/model-editor/model-editor-queries.ts index 063452d03..7fc0e7562 100644 --- a/extensions/ql-vscode/src/model-editor/model-editor-queries.ts +++ b/extensions/ql-vscode/src/model-editor/model-editor-queries.ts @@ -4,7 +4,7 @@ import { writeFile } from "fs-extra"; import { dump } from "js-yaml"; import { prepareExternalApiQuery } from "./external-api-usage-queries"; import { CodeQLCliServer } from "../codeql-cli/cli"; -import { showLlmGeneration } from "../config"; +import { ModelConfig } from "../config"; import { Mode } from "./shared/mode"; import { resolveQueriesFromPacks } from "../local-queries"; import { modeTag } from "./mode-tag"; @@ -28,12 +28,14 @@ export const syntheticQueryPackName = "codeql/external-api-usage"; * @param cliServer The CodeQL CLI server to use. * @param queryDir The directory to set up. * @param language The language to use for the queries. + * @param modelConfig The model config to use. * @returns true if the setup was successful, false otherwise. */ export async function setUpPack( cliServer: CodeQLCliServer, queryDir: string, language: QueryLanguage, + modelConfig: ModelConfig, ): Promise { // Download the required query packs await cliServer.packDownload([`codeql/${language}-queries`]); @@ -84,7 +86,7 @@ export async function setUpPack( } // Download any other required packs - if (language === "java" && showLlmGeneration()) { + if (language === "java" && modelConfig.llmGeneration) { await cliServer.packDownload([`codeql/${language}-automodel-queries`]); } diff --git a/extensions/ql-vscode/src/model-editor/model-editor-view.ts b/extensions/ql-vscode/src/model-editor/model-editor-view.ts index a2a0e42ba..0c62dab0a 100644 --- a/extensions/ql-vscode/src/model-editor/model-editor-view.ts +++ b/extensions/ql-vscode/src/model-editor/model-editor-view.ts @@ -17,8 +17,8 @@ import { import { ProgressCallback, withProgress } from "../common/vscode/progress"; import { QueryRunner } from "../query-server"; import { - showAndLogExceptionWithTelemetry, showAndLogErrorMessage, + showAndLogExceptionWithTelemetry, } from "../common/logging"; import { DatabaseItem, DatabaseManager } from "../databases/local-databases"; import { CodeQLCliServer } from "../codeql-cli/cli"; @@ -34,11 +34,7 @@ import { import { Method, Usage } from "./method"; import { ModeledMethod } from "./modeled-method"; import { ExtensionPack } from "./shared/extension-pack"; -import { - showFlowGeneration, - showLlmGeneration, - showMultipleModels, -} from "../config"; +import { ModelConfigListener } from "../config"; import { Mode } from "./shared/mode"; import { loadModeledMethods, saveModeledMethods } from "./modeled-method-fs"; import { pickExtensionPack } from "./extension-pack-picker"; @@ -58,6 +54,7 @@ export class ModelEditorView extends AbstractWebview< protected readonly app: App, private readonly modelingStore: ModelingStore, private readonly viewTracker: ModelEditorViewTracker, + private readonly modelConfig: ModelConfigListener, private readonly databaseManager: DatabaseManager, private readonly cliServer: CodeQLCliServer, private readonly queryRunner: QueryRunner, @@ -71,6 +68,7 @@ export class ModelEditorView extends AbstractWebview< this.modelingStore.initializeStateForDb(databaseItem); this.registerToModelingStoreEvents(); + this.registerToModelConfigEvents(); this.viewTracker.registerView(this); @@ -334,15 +332,15 @@ export class ModelEditorView extends AbstractWebview< private async setViewState(): Promise { const showLlmButton = - this.databaseItem.language === "java" && showLlmGeneration(); + this.databaseItem.language === "java" && this.modelConfig.llmGeneration; await this.postMessage({ t: "setModelEditorViewState", viewState: { extensionPack: this.extensionPack, - showFlowGeneration: showFlowGeneration(), + showFlowGeneration: this.modelConfig.flowGeneration, showLlmButton, - showMultipleModels: showMultipleModels(), + showMultipleModels: this.modelConfig.showMultipleModels, mode: this.mode, }, }); @@ -481,6 +479,7 @@ export class ModelEditorView extends AbstractWebview< const modelFile = await pickExtensionPack( this.cliServer, addedDatabase, + this.modelConfig, this.app.logger, progress, 3, @@ -493,6 +492,7 @@ export class ModelEditorView extends AbstractWebview< this.app, this.modelingStore, this.viewTracker, + this.modelConfig, this.databaseManager, this.cliServer, this.queryRunner, @@ -614,6 +614,14 @@ export class ModelEditorView extends AbstractWebview< ); } + private registerToModelConfigEvents() { + this.push( + this.modelConfig.onDidChangeConfiguration(() => { + void this.setViewState(); + }), + ); + } + private addModeledMethods(modeledMethods: Record) { this.modelingStore.addModeledMethods(this.databaseItem, modeledMethods); diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/model-editor/extension-pack-picker.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/model-editor/extension-pack-picker.test.ts index 4ae3415ba..6898a32cc 100644 --- a/extensions/ql-vscode/test/vscode-tests/no-workspace/model-editor/extension-pack-picker.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/model-editor/extension-pack-picker.test.ts @@ -1,10 +1,4 @@ -import { - ConfigurationScope, - Uri, - workspace, - WorkspaceConfiguration as VSCodeWorkspaceConfiguration, - WorkspaceFolder, -} from "vscode"; +import { Uri, workspace, WorkspaceFolder } from "vscode"; import { dump as dumpYaml, load as loadYaml } from "js-yaml"; import { outputFile, readFile } from "fs-extra"; import { join } from "path"; @@ -14,7 +8,8 @@ import { QlpacksInfo } from "../../../../src/codeql-cli/cli"; import { pickExtensionPack } from "../../../../src/model-editor/extension-pack-picker"; import { ExtensionPack } from "../../../../src/model-editor/shared/extension-pack"; import { createMockLogger } from "../../../__mocks__/loggerMock"; -import { vscodeGetConfigurationMock } from "../../test-config"; +import { ModelConfig } from "../../../../src/config"; +import { mockedObject } from "../../utils/mocking.helpers"; describe("pickExtensionPack", () => { let tmpDir: string; @@ -32,6 +27,7 @@ describe("pickExtensionPack", () => { let workspaceFoldersSpy: jest.SpyInstance; let additionalPacks: string[]; let workspaceFolder: WorkspaceFolder; + let modelConfig: ModelConfig; const logger = createMockLogger(); const maxStep = 4; @@ -67,41 +63,20 @@ describe("pickExtensionPack", () => { workspaceFoldersSpy = jest .spyOn(workspace, "workspaceFolders", "get") .mockReturnValue([workspaceFolder]); + + modelConfig = mockedObject({ + getExtensionsDirectory: jest.fn().mockReturnValue(undefined), + }); }); it("selects an existing extension pack", async () => { - vscodeGetConfigurationMock.mockImplementation( - ( - section?: string, - scope?: ConfigurationScope | null, - ): VSCodeWorkspaceConfiguration => { - expect(section).toEqual("codeQL.model"); - expect((scope as any)?.languageId).toEqual("java"); - - return { - get: (key: string) => { - expect(key).toEqual("extensionsDirectory"); - return undefined; - }, - has: (key: string) => { - return key === "extensionsDirectory"; - }, - inspect: () => { - throw new Error("inspect not implemented"); - }, - update: () => { - throw new Error("update not implemented"); - }, - }; - }, - ); - const cliServer = mockCliServer(qlPacks); expect( await pickExtensionPack( cliServer, databaseItem, + modelConfig, logger, progress, maxStep, @@ -112,35 +87,10 @@ describe("pickExtensionPack", () => { additionalPacks, true, ); + expect(modelConfig.getExtensionsDirectory).toHaveBeenCalledWith("java"); }); it("creates a new extension pack using default extensions directory", async () => { - vscodeGetConfigurationMock.mockImplementation( - ( - section?: string, - scope?: ConfigurationScope | null, - ): VSCodeWorkspaceConfiguration => { - expect(section).toEqual("codeQL.model"); - expect((scope as any)?.languageId).toEqual("java"); - - return { - get: (key: string) => { - expect(key).toEqual("extensionsDirectory"); - return undefined; - }, - has: (key: string) => { - return key === "extensionsDirectory"; - }, - inspect: () => { - throw new Error("inspect not implemented"); - }, - update: () => { - throw new Error("update not implemented"); - }, - }; - }, - ); - const tmpDir = await dir({ unsafeCleanup: true, }); @@ -183,6 +133,7 @@ describe("pickExtensionPack", () => { await pickExtensionPack( cliServer, databaseItem, + modelConfig, logger, progress, maxStep, @@ -199,6 +150,7 @@ describe("pickExtensionPack", () => { dataExtensions: ["models/**/*.yml"], }); expect(cliServer.resolveQlpacks).toHaveBeenCalled(); + expect(modelConfig.getExtensionsDirectory).toHaveBeenCalledWith("java"); expect( loadYaml(await readFile(join(newPackDir, "codeql-pack.yml"), "utf8")), @@ -223,31 +175,9 @@ describe("pickExtensionPack", () => { "my-custom-extensions-directory", ); - vscodeGetConfigurationMock.mockImplementation( - ( - section?: string, - scope?: ConfigurationScope | null, - ): VSCodeWorkspaceConfiguration => { - expect(section).toEqual("codeQL.model"); - expect((scope as any)?.languageId).toEqual("java"); - - return { - get: (key: string) => { - expect(key).toEqual("extensionsDirectory"); - return configExtensionsDir; - }, - has: (key: string) => { - return key === "extensionsDirectory"; - }, - inspect: () => { - throw new Error("inspect not implemented"); - }, - update: () => { - throw new Error("update not implemented"); - }, - }; - }, - ); + const modelConfig = mockedObject({ + getExtensionsDirectory: jest.fn().mockReturnValue(configExtensionsDir), + }); const newPackDir = join(configExtensionsDir, "vscode-codeql-java"); @@ -257,6 +187,7 @@ describe("pickExtensionPack", () => { await pickExtensionPack( cliServer, databaseItem, + modelConfig, logger, progress, maxStep, @@ -273,6 +204,7 @@ describe("pickExtensionPack", () => { dataExtensions: ["models/**/*.yml"], }); expect(cliServer.resolveQlpacks).toHaveBeenCalled(); + expect(modelConfig.getExtensionsDirectory).toHaveBeenCalledWith("java"); expect( loadYaml(await readFile(join(newPackDir, "codeql-pack.yml"), "utf8")), @@ -299,6 +231,7 @@ describe("pickExtensionPack", () => { await pickExtensionPack( cliServer, databaseItem, + modelConfig, logger, progress, maxStep, @@ -324,6 +257,7 @@ describe("pickExtensionPack", () => { await pickExtensionPack( cliServer, databaseItem, + modelConfig, logger, progress, maxStep, @@ -351,6 +285,7 @@ describe("pickExtensionPack", () => { await pickExtensionPack( cliServer, databaseItem, + modelConfig, logger, progress, maxStep, @@ -388,6 +323,7 @@ describe("pickExtensionPack", () => { await pickExtensionPack( cliServer, databaseItem, + modelConfig, logger, progress, maxStep, @@ -425,6 +361,7 @@ describe("pickExtensionPack", () => { await pickExtensionPack( cliServer, databaseItem, + modelConfig, logger, progress, maxStep, @@ -465,6 +402,7 @@ describe("pickExtensionPack", () => { await pickExtensionPack( cliServer, databaseItem, + modelConfig, logger, progress, maxStep, @@ -522,6 +460,7 @@ describe("pickExtensionPack", () => { await pickExtensionPack( cliServer, databaseItem, + modelConfig, logger, progress, maxStep, diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/model-editor/model-editor-queries.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/model-editor/model-editor-queries.test.ts index f6d827b3e..ccbafc186 100644 --- a/extensions/ql-vscode/test/vscode-tests/no-workspace/model-editor/model-editor-queries.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/model-editor/model-editor-queries.test.ts @@ -8,6 +8,7 @@ import { QueryLanguage } from "../../../../src/common/query-language"; import { Mode } from "../../../../src/model-editor/shared/mode"; import { mockedObject } from "../../utils/mocking.helpers"; import { CodeQLCliServer } from "../../../../src/codeql-cli/cli"; +import { ModelConfig } from "../../../../src/config"; describe("setUpPack", () => { let queryDir: string; @@ -32,8 +33,11 @@ describe("setUpPack", () => { packInstall: jest.fn(), resolveQueriesInSuite: jest.fn().mockResolvedValue([]), }); + const modelConfig = mockedObject({ + llmGeneration: false, + }); - await setUpPack(cliServer, queryDir, language); + await setUpPack(cliServer, queryDir, language, modelConfig); const queryFiles = await readdir(queryDir); expect(queryFiles.sort()).toEqual( @@ -89,8 +93,11 @@ describe("setUpPack", () => { .fn() .mockResolvedValue(["/a/b/c/ApplicationModeEndpoints.ql"]), }); + const modelConfig = mockedObject({ + llmGeneration: false, + }); - await setUpPack(cliServer, queryDir, language); + await setUpPack(cliServer, queryDir, language, modelConfig); const queryFiles = await readdir(queryDir); expect(queryFiles.sort()).toEqual(["codeql-pack.yml"].sort()); diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/model-editor/model-editor-view.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/model-editor/model-editor-view.test.ts index 2abb9177a..b346de2e0 100644 --- a/extensions/ql-vscode/test/vscode-tests/no-workspace/model-editor/model-editor-view.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/model-editor/model-editor-view.test.ts @@ -10,11 +10,15 @@ import { QueryRunner } from "../../../../src/query-server"; import { ExtensionPack } from "../../../../src/model-editor/shared/extension-pack"; import { createMockModelingStore } from "../../../__mocks__/model-editor/modelingStoreMock"; import { createMockModelEditorViewTracker } from "../../../__mocks__/model-editor/modelEditorViewTrackerMock"; +import { ModelConfigListener } from "../../../../src/config"; describe("ModelEditorView", () => { const app = createMockApp({}); const modelingStore = createMockModelingStore(); const viewTracker = createMockModelEditorViewTracker(); + const modelConfig = mockedObject({ + onDidChangeConfiguration: jest.fn(), + }); const databaseManager = mockEmptyDatabaseManager(); const cliServer = mockedObject({}); const queryRunner = mockedObject({}); @@ -41,6 +45,7 @@ describe("ModelEditorView", () => { app, modelingStore, viewTracker, + modelConfig, databaseManager, cliServer, queryRunner, From 0e5551b6507fb8644cf0da14417972f56f6dd9ba Mon Sep 17 00:00:00 2001 From: Charis Kyriakou Date: Fri, 6 Oct 2023 14:25:33 +0100 Subject: [PATCH 34/46] Make method modeling panel available to canary users (#2920) --- 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 447f5b34c..abe9d4355 100644 --- a/extensions/ql-vscode/package.json +++ b/extensions/ql-vscode/package.json @@ -1996,7 +1996,7 @@ "id": "codeQLMethodModeling", "type": "webview", "name": "CodeQL Method Modeling", - "when": "config.codeQL.canary && config.codeQL.model.methodModelingView" + "when": "config.codeQL.canary" } ], "codeql-methods-usage": [ From ee1bf8896e87500600a3656774bd1a84753c6599 Mon Sep 17 00:00:00 2001 From: Koen Vlaswinkel Date: Fri, 6 Oct 2023 15:48:41 +0200 Subject: [PATCH 35/46] Change MethodModeling to accept multiple models --- .../stories/method-modeling/MethodModeling.stories.tsx | 4 +++- .../ql-vscode/src/view/method-modeling/MethodModeling.tsx | 8 +++++--- .../src/view/method-modeling/MethodModelingView.tsx | 2 +- .../method-modeling/__tests__/MethodModeling.spec.tsx | 2 +- 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/extensions/ql-vscode/src/stories/method-modeling/MethodModeling.stories.tsx b/extensions/ql-vscode/src/stories/method-modeling/MethodModeling.stories.tsx index 46d05c3d4..a447cd364 100644 --- a/extensions/ql-vscode/src/stories/method-modeling/MethodModeling.stories.tsx +++ b/extensions/ql-vscode/src/stories/method-modeling/MethodModeling.stories.tsx @@ -18,18 +18,20 @@ const method = createMethod(); export const MethodUnmodeled = Template.bind({}); MethodUnmodeled.args = { method, + modeledMethods: [], modelingStatus: "unmodeled", }; export const MethodModeled = Template.bind({}); MethodModeled.args = { method, - + modeledMethods: [], modelingStatus: "unsaved", }; export const MethodSaved = Template.bind({}); MethodSaved.args = { method, + modeledMethods: [], modelingStatus: "saved", }; diff --git a/extensions/ql-vscode/src/view/method-modeling/MethodModeling.tsx b/extensions/ql-vscode/src/view/method-modeling/MethodModeling.tsx index 4e8a993bc..5bdbda812 100644 --- a/extensions/ql-vscode/src/view/method-modeling/MethodModeling.tsx +++ b/extensions/ql-vscode/src/view/method-modeling/MethodModeling.tsx @@ -55,14 +55,14 @@ const UnsavedTag = ({ modelingStatus }: { modelingStatus: ModelingStatus }) => ( export type MethodModelingProps = { modelingStatus: ModelingStatus; method: Method; - modeledMethod: ModeledMethod | undefined; + modeledMethods: ModeledMethod[]; showMultipleModels?: boolean; onChange: (modeledMethod: ModeledMethod) => void; }; export const MethodModeling = ({ modelingStatus, - modeledMethod, + modeledMethods, method, onChange, }: MethodModelingProps): JSX.Element => { @@ -79,7 +79,9 @@ export const MethodModeling = ({ 0 ? modeledMethods[0] : undefined + } onChange={onChange} /> diff --git a/extensions/ql-vscode/src/view/method-modeling/MethodModelingView.tsx b/extensions/ql-vscode/src/view/method-modeling/MethodModelingView.tsx index 2c32c3ee5..ea7ddf26c 100644 --- a/extensions/ql-vscode/src/view/method-modeling/MethodModelingView.tsx +++ b/extensions/ql-vscode/src/view/method-modeling/MethodModelingView.tsx @@ -94,7 +94,7 @@ export function MethodModelingView({ initialViewState }: Props): JSX.Element { diff --git a/extensions/ql-vscode/src/view/method-modeling/__tests__/MethodModeling.spec.tsx b/extensions/ql-vscode/src/view/method-modeling/__tests__/MethodModeling.spec.tsx index be625f743..0dad23970 100644 --- a/extensions/ql-vscode/src/view/method-modeling/__tests__/MethodModeling.spec.tsx +++ b/extensions/ql-vscode/src/view/method-modeling/__tests__/MethodModeling.spec.tsx @@ -16,7 +16,7 @@ describe(MethodModeling.name, () => { render({ modelingStatus: "saved", method, - modeledMethod, + modeledMethods: [modeledMethod], onChange, }); From a29112e045e399416faf4a8b1a8a25845927c863 Mon Sep 17 00:00:00 2001 From: Koen Vlaswinkel Date: Fri, 6 Oct 2023 15:54:02 +0200 Subject: [PATCH 36/46] Add multiple models to MethodModeling story --- .../method-modeling/MethodModeling.stories.tsx | 18 ++++++++++++++++++ .../model-editor/modeled-method-factories.ts | 2 +- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/extensions/ql-vscode/src/stories/method-modeling/MethodModeling.stories.tsx b/extensions/ql-vscode/src/stories/method-modeling/MethodModeling.stories.tsx index a447cd364..eb9259412 100644 --- a/extensions/ql-vscode/src/stories/method-modeling/MethodModeling.stories.tsx +++ b/extensions/ql-vscode/src/stories/method-modeling/MethodModeling.stories.tsx @@ -4,6 +4,7 @@ import { Meta, StoryFn } from "@storybook/react"; import { MethodModeling as MethodModelingComponent } from "../../view/method-modeling/MethodModeling"; import { createMethod } from "../../../test/factories/model-editor/method-factories"; +import { createModeledMethod } from "../../../test/factories/model-editor/modeled-method-factories"; export default { title: "Method Modeling/Method Modeling", component: MethodModelingComponent, @@ -35,3 +36,20 @@ MethodSaved.args = { modeledMethods: [], modelingStatus: "saved", }; + +export const MultipleModelings = Template.bind({}); +MultipleModelings.args = { + method, + modeledMethods: [ + createModeledMethod(method), + createModeledMethod({ + ...method, + type: "source", + input: "", + output: "ReturnValue", + kind: "remote", + }), + ], + showMultipleModels: true, + modelingStatus: "saved", +}; diff --git a/extensions/ql-vscode/test/factories/model-editor/modeled-method-factories.ts b/extensions/ql-vscode/test/factories/model-editor/modeled-method-factories.ts index 0fff7d480..9c8b1207b 100644 --- a/extensions/ql-vscode/test/factories/model-editor/modeled-method-factories.ts +++ b/extensions/ql-vscode/test/factories/model-editor/modeled-method-factories.ts @@ -13,7 +13,7 @@ export function createModeledMethod( type: "sink", input: "Argument[0]", output: "", - kind: "jndi-injection", + kind: "path-injection", provenance: "manual", ...data, }; From a01a76cb73edac8d30f80afb00ce2e5bb300d249 Mon Sep 17 00:00:00 2001 From: Koen Vlaswinkel Date: Fri, 6 Oct 2023 16:25:25 +0200 Subject: [PATCH 37/46] Add initial multiple modelings panel --- .../MethodModeling.stories.tsx | 20 ++++- .../src/view/common/icon/CodiconButton.tsx | 50 +++++++++++ .../ql-vscode/src/view/common/icon/index.ts | 1 + .../view/method-modeling/MethodModeling.tsx | 14 ++-- .../method-modeling/ModeledMethodsPanel.tsx | 44 ++++++++++ .../MultipleModeledMethodsPanel.tsx | 84 +++++++++++++++++++ 6 files changed, 202 insertions(+), 11 deletions(-) create mode 100644 extensions/ql-vscode/src/view/common/icon/CodiconButton.tsx create mode 100644 extensions/ql-vscode/src/view/method-modeling/ModeledMethodsPanel.tsx create mode 100644 extensions/ql-vscode/src/view/method-modeling/MultipleModeledMethodsPanel.tsx diff --git a/extensions/ql-vscode/src/stories/method-modeling/MethodModeling.stories.tsx b/extensions/ql-vscode/src/stories/method-modeling/MethodModeling.stories.tsx index eb9259412..e611378ee 100644 --- a/extensions/ql-vscode/src/stories/method-modeling/MethodModeling.stories.tsx +++ b/extensions/ql-vscode/src/stories/method-modeling/MethodModeling.stories.tsx @@ -37,8 +37,24 @@ MethodSaved.args = { modelingStatus: "saved", }; -export const MultipleModelings = Template.bind({}); -MultipleModelings.args = { +export const MultipleModelingsUnmodeled = Template.bind({}); +MultipleModelingsUnmodeled.args = { + method, + modeledMethods: [], + showMultipleModels: true, + modelingStatus: "saved", +}; + +export const MultipleModelingsModeledSingle = Template.bind({}); +MultipleModelingsModeledSingle.args = { + method, + modeledMethods: [createModeledMethod(method)], + showMultipleModels: true, + modelingStatus: "saved", +}; + +export const MultipleModelingsModeledMultiple = Template.bind({}); +MultipleModelingsModeledMultiple.args = { method, modeledMethods: [ createModeledMethod(method), diff --git a/extensions/ql-vscode/src/view/common/icon/CodiconButton.tsx b/extensions/ql-vscode/src/view/common/icon/CodiconButton.tsx new file mode 100644 index 000000000..43aeeb3a7 --- /dev/null +++ b/extensions/ql-vscode/src/view/common/icon/CodiconButton.tsx @@ -0,0 +1,50 @@ +import * as React from "react"; +import { styled } from "styled-components"; +import classNames from "classnames"; + +type Size = "x-small" | "small" | "medium" | "large" | "x-large"; + +const StyledButton = styled.button<{ size: Size }>` + background: none; + color: var(--vscode-textLink-foreground); + border: none; + cursor: pointer; + font-size: ${(props) => props.size ?? "1em"}; + padding: 0; + vertical-align: text-bottom; + + &:disabled { + color: var(--vscode-disabledForeground); + cursor: default; + } +`; + +export const CodiconButton = ({ + size, + onClick, + className, + name, + label, + disabled, +}: { + size?: Size; + onClick: (e: React.MouseEvent) => void; + className?: string; + name: string; + label: string; + disabled?: boolean; +}) => ( + + + +); diff --git a/extensions/ql-vscode/src/view/common/icon/index.ts b/extensions/ql-vscode/src/view/common/icon/index.ts index 829bb273c..6ab766aab 100644 --- a/extensions/ql-vscode/src/view/common/icon/index.ts +++ b/extensions/ql-vscode/src/view/common/icon/index.ts @@ -1,4 +1,5 @@ export * from "./Codicon"; +export * from "./CodiconButton"; export * from "./ErrorIcon"; export * from "./LoadingIcon"; export * from "./SuccessIcon"; diff --git a/extensions/ql-vscode/src/view/method-modeling/MethodModeling.tsx b/extensions/ql-vscode/src/view/method-modeling/MethodModeling.tsx index 5bdbda812..bb46da9dc 100644 --- a/extensions/ql-vscode/src/view/method-modeling/MethodModeling.tsx +++ b/extensions/ql-vscode/src/view/method-modeling/MethodModeling.tsx @@ -5,9 +5,9 @@ import { ModelingStatusIndicator } from "../model-editor/ModelingStatusIndicator import { Method } from "../../model-editor/method"; import { MethodName } from "../model-editor/MethodName"; import { ModeledMethod } from "../../model-editor/modeled-method"; -import { MethodModelingInputs } from "./MethodModelingInputs"; import { VSCodeTag } from "@vscode/webview-ui-toolkit/react"; import { ReviewInEditorButton } from "./ReviewInEditorButton"; +import { ModeledMethodsPanel } from "./ModeledMethodsPanel"; const Container = styled.div` padding-top: 0.5rem; @@ -38,10 +38,6 @@ const DependencyContainer = styled.div` margin-bottom: 0.8rem; `; -const StyledMethodModelingInputs = styled(MethodModelingInputs)` - padding-bottom: 0.5rem; -`; - const StyledVSCodeTag = styled(VSCodeTag)<{ visible: boolean }>` visibility: ${(props) => (props.visible ? "visible" : "hidden")}; `; @@ -64,6 +60,7 @@ export const MethodModeling = ({ modelingStatus, modeledMethods, method, + showMultipleModels = false, onChange, }: MethodModelingProps): JSX.Element => { return ( @@ -77,11 +74,10 @@ export const MethodModeling = ({ - 0 ? modeledMethods[0] : undefined - } + modeledMethods={modeledMethods} + showMultipleModels={showMultipleModels} onChange={onChange} /> diff --git a/extensions/ql-vscode/src/view/method-modeling/ModeledMethodsPanel.tsx b/extensions/ql-vscode/src/view/method-modeling/ModeledMethodsPanel.tsx new file mode 100644 index 000000000..808b55c06 --- /dev/null +++ b/extensions/ql-vscode/src/view/method-modeling/ModeledMethodsPanel.tsx @@ -0,0 +1,44 @@ +import * as React from "react"; +import { ModeledMethod } from "../../model-editor/modeled-method"; +import { MethodModelingInputs } from "./MethodModelingInputs"; +import { Method } from "../../model-editor/method"; +import { styled } from "styled-components"; +import { MultipleModeledMethodsPanel } from "./MultipleModeledMethodsPanel"; + +type Props = { + method: Method; + modeledMethods: ModeledMethod[]; + showMultipleModels: boolean; + onChange: (modeledMethod: ModeledMethod) => void; +}; + +const SingleMethodModelingInputs = styled(MethodModelingInputs)` + padding-bottom: 0.5rem; +`; + +export const ModeledMethodsPanel = ({ + method, + modeledMethods, + showMultipleModels, + onChange, +}: Props) => { + if (!showMultipleModels) { + return ( + 0 ? modeledMethods[0] : undefined + } + onChange={onChange} + /> + ); + } + + return ( + + ); +}; diff --git a/extensions/ql-vscode/src/view/method-modeling/MultipleModeledMethodsPanel.tsx b/extensions/ql-vscode/src/view/method-modeling/MultipleModeledMethodsPanel.tsx new file mode 100644 index 000000000..5ca0292d6 --- /dev/null +++ b/extensions/ql-vscode/src/view/method-modeling/MultipleModeledMethodsPanel.tsx @@ -0,0 +1,84 @@ +import * as React from "react"; +import { useCallback, useState } from "react"; +import { Method } from "../../model-editor/method"; +import { ModeledMethod } from "../../model-editor/modeled-method"; +import { styled } from "styled-components"; +import { MethodModelingInputs } from "./MethodModelingInputs"; +import { CodiconButton } from "../common"; + +type Props = { + method: Method; + modeledMethods: ModeledMethod[]; + onChange: (modeledMethod: ModeledMethod) => void; +}; + +const Container = styled.div` + display: flex; + flex-direction: column; + gap: 0.25rem; + + padding-bottom: 0.5rem; + border-bottom: 0.05rem solid var(--vscode-panelSection-border); +`; + +const Footer = styled.div` + display: flex; + flex-direction: row; +`; + +const PaginationActions = styled.div` + display: flex; + flex-direction: row; + gap: 0.5rem; +`; + +export const MultipleModeledMethodsPanel = ({ + method, + modeledMethods, + onChange, +}: Props) => { + const [selectedIndex, setSelectedIndex] = useState(0); + + const handlePreviousClick = useCallback(() => { + setSelectedIndex((previousIndex) => previousIndex - 1); + }, []); + const handleNextClick = useCallback(() => { + setSelectedIndex((previousIndex) => previousIndex + 1); + }, []); + + return ( + + {modeledMethods.length > 0 && ( + + )} +
+ + + {modeledMethods.length > 1 && ( +
+ {selectedIndex + 1}/{modeledMethods.length} +
+ )} + + +
+ + ); +}; From 3a494dff365e3f8890d51b7223e4f4a2e166a596 Mon Sep 17 00:00:00 2001 From: Koen Vlaswinkel Date: Fri, 6 Oct 2023 16:26:53 +0200 Subject: [PATCH 38/46] Add support for unmodeled methods --- .../view/method-modeling/MultipleModeledMethodsPanel.tsx | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/extensions/ql-vscode/src/view/method-modeling/MultipleModeledMethodsPanel.tsx b/extensions/ql-vscode/src/view/method-modeling/MultipleModeledMethodsPanel.tsx index 5ca0292d6..c2b04e68c 100644 --- a/extensions/ql-vscode/src/view/method-modeling/MultipleModeledMethodsPanel.tsx +++ b/extensions/ql-vscode/src/view/method-modeling/MultipleModeledMethodsPanel.tsx @@ -48,12 +48,18 @@ export const MultipleModeledMethodsPanel = ({ return ( - {modeledMethods.length > 0 && ( + {modeledMethods.length > 0 ? ( + ) : ( + )}