From 90093fb9f5be153ead19757ac2479405522f53ad Mon Sep 17 00:00:00 2001 From: Robert Date: Fri, 15 Sep 2023 15:01:32 +0100 Subject: [PATCH 01/47] 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/47] 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/47] 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/47] 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/47] 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/47] 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/47] 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/47] 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/47] 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/47] 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/47] 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/47] 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/47] 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/47] 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 c5175e040ec2e54dba0d9937b24892951a6511c3 Mon Sep 17 00:00:00 2001 From: Koen Vlaswinkel Date: Wed, 4 Oct 2023 13:47:30 +0200 Subject: [PATCH 15/47] 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 e75eccb4168798e258fcb1c3931f2459b5be56be Mon Sep 17 00:00:00 2001 From: Robert Date: Wed, 4 Oct 2023 12:34:40 +0100 Subject: [PATCH 16/47] Change MethodRow to accept ModelEditorViewState object instead of just Mode --- .../ql-vscode/src/view/model-editor/LibraryRow.tsx | 2 +- .../ql-vscode/src/view/model-editor/MethodRow.tsx | 11 ++++++----- .../src/view/model-editor/ModeledMethodDataGrid.tsx | 8 ++++---- .../view/model-editor/__tests__/MethodRow.spec.tsx | 12 +++++++++++- .../__tests__/ModeledMethodDataGrid.spec.tsx | 12 +++++++++++- 5 files changed, 33 insertions(+), 12 deletions(-) diff --git a/extensions/ql-vscode/src/view/model-editor/LibraryRow.tsx b/extensions/ql-vscode/src/view/model-editor/LibraryRow.tsx index 2c7871a69..74c2e3fbc 100644 --- a/extensions/ql-vscode/src/view/model-editor/LibraryRow.tsx +++ b/extensions/ql-vscode/src/view/model-editor/LibraryRow.tsx @@ -235,7 +235,7 @@ export const LibraryRow = ({ modeledMethods={modeledMethods} modifiedSignatures={modifiedSignatures} inProgressMethods={inProgressMethods} - mode={viewState.mode} + viewState={viewState} hideModeledMethods={hideModeledMethods} revealedMethodSignature={revealedMethodSignature} onChange={onChange} diff --git a/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx b/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx index b1880355c..21fde2801 100644 --- a/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx +++ b/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx @@ -21,6 +21,7 @@ import { MethodName } from "./MethodName"; import { ModelTypeDropdown } from "./ModelTypeDropdown"; import { ModelInputDropdown } from "./ModelInputDropdown"; import { ModelOutputDropdown } from "./ModelOutputDropdown"; +import { ModelEditorViewState } from "../../model-editor/shared/view-state"; const ApiOrMethodCell = styled(VSCodeDataGridCell)` display: flex; @@ -58,7 +59,7 @@ export type MethodRowProps = { modeledMethod: ModeledMethod | undefined; methodIsUnsaved: boolean; modelingInProgress: boolean; - mode: Mode; + viewState: ModelEditorViewState; revealedMethodSignature: string | null; onChange: (modeledMethod: ModeledMethod) => void; }; @@ -90,7 +91,7 @@ const ModelableMethodRow = forwardRef( method, modeledMethod, methodIsUnsaved, - mode, + viewState, revealedMethodSignature, onChange, } = props; @@ -112,7 +113,7 @@ const ModelableMethodRow = forwardRef( - {mode === Mode.Application && ( + {viewState.mode === Mode.Application && ( {method.usages.length} @@ -178,7 +179,7 @@ const UnmodelableMethodRow = forwardRef< HTMLElement | undefined, MethodRowProps >((props, ref) => { - const { method, mode, revealedMethodSignature } = props; + const { method, viewState, revealedMethodSignature } = props; const jumpToUsage = useCallback( () => sendJumpToUsageMessage(method), @@ -194,7 +195,7 @@ const UnmodelableMethodRow = forwardRef< - {mode === Mode.Application && ( + {viewState.mode === Mode.Application && ( {method.usages.length} diff --git a/extensions/ql-vscode/src/view/model-editor/ModeledMethodDataGrid.tsx b/extensions/ql-vscode/src/view/model-editor/ModeledMethodDataGrid.tsx index 11044c5ff..9ff27ad21 100644 --- a/extensions/ql-vscode/src/view/model-editor/ModeledMethodDataGrid.tsx +++ b/extensions/ql-vscode/src/view/model-editor/ModeledMethodDataGrid.tsx @@ -8,10 +8,10 @@ import { MethodRow } from "./MethodRow"; import { Method } from "../../model-editor/method"; import { ModeledMethod } from "../../model-editor/modeled-method"; import { useMemo } from "react"; -import { Mode } from "../../model-editor/shared/mode"; import { sortMethods } from "../../model-editor/shared/sorting"; import { InProgressMethods } from "../../model-editor/shared/in-progress-methods"; import { HiddenMethodsRow } from "./HiddenMethodsRow"; +import { ModelEditorViewState } from "../../model-editor/shared/view-state"; export const GRID_TEMPLATE_COLUMNS = "0.5fr 0.125fr 0.125fr 0.125fr 0.125fr"; @@ -21,7 +21,7 @@ export type ModeledMethodDataGridProps = { modeledMethods: Record; modifiedSignatures: Set; inProgressMethods: InProgressMethods; - mode: Mode; + viewState: ModelEditorViewState; hideModeledMethods: boolean; revealedMethodSignature: string | null; onChange: (modeledMethod: ModeledMethod) => void; @@ -33,7 +33,7 @@ export const ModeledMethodDataGrid = ({ modeledMethods, modifiedSignatures, inProgressMethods, - mode, + viewState, hideModeledMethods, revealedMethodSignature, onChange, @@ -95,7 +95,7 @@ export const ModeledMethodDataGrid = ({ packageName, method.signature, )} - mode={mode} + viewState={viewState} revealedMethodSignature={revealedMethodSignature} onChange={onChange} /> diff --git a/extensions/ql-vscode/src/view/model-editor/__tests__/MethodRow.spec.tsx b/extensions/ql-vscode/src/view/model-editor/__tests__/MethodRow.spec.tsx index 2c22455c0..64874f60c 100644 --- a/extensions/ql-vscode/src/view/model-editor/__tests__/MethodRow.spec.tsx +++ b/extensions/ql-vscode/src/view/model-editor/__tests__/MethodRow.spec.tsx @@ -9,6 +9,8 @@ import { Mode } from "../../../model-editor/shared/mode"; import { MethodRow, MethodRowProps } from "../MethodRow"; import { ModeledMethod } from "../../../model-editor/modeled-method"; import userEvent from "@testing-library/user-event"; +import { ModelEditorViewState } from "../../../model-editor/shared/view-state"; +import { createMockExtensionPack } from "../../../../test/factories/model-editor/extension-pack"; describe(MethodRow.name, () => { const method = createMethod({ @@ -31,6 +33,14 @@ describe(MethodRow.name, () => { }; const onChange = jest.fn(); + const viewState: ModelEditorViewState = { + mode: Mode.Application, + showFlowGeneration: false, + showLlmButton: false, + showMultipleModels: false, + extensionPack: createMockExtensionPack(), + }; + const render = (props: Partial = {}) => reactRender( { methodIsUnsaved={false} modelingInProgress={false} revealedMethodSignature={null} - mode={Mode.Application} + viewState={viewState} onChange={onChange} {...props} />, diff --git a/extensions/ql-vscode/src/view/model-editor/__tests__/ModeledMethodDataGrid.spec.tsx b/extensions/ql-vscode/src/view/model-editor/__tests__/ModeledMethodDataGrid.spec.tsx index 25cb0268c..a9ae87ba6 100644 --- a/extensions/ql-vscode/src/view/model-editor/__tests__/ModeledMethodDataGrid.spec.tsx +++ b/extensions/ql-vscode/src/view/model-editor/__tests__/ModeledMethodDataGrid.spec.tsx @@ -7,6 +7,8 @@ import { ModeledMethodDataGrid, ModeledMethodDataGridProps, } from "../ModeledMethodDataGrid"; +import { ModelEditorViewState } from "../../../model-editor/shared/view-state"; +import { createMockExtensionPack } from "../../../../test/factories/model-editor/extension-pack"; describe(ModeledMethodDataGrid.name, () => { const method1 = createMethod({ @@ -41,6 +43,14 @@ describe(ModeledMethodDataGrid.name, () => { }); const onChange = jest.fn(); + const viewState: ModelEditorViewState = { + mode: Mode.Application, + showFlowGeneration: false, + showLlmButton: false, + showMultipleModels: false, + extensionPack: createMockExtensionPack(), + }; + const render = (props: Partial = {}) => reactRender( { }} modifiedSignatures={new Set([method1.signature])} inProgressMethods={new InProgressMethods()} - mode={Mode.Application} + viewState={viewState} hideModeledMethods={false} revealedMethodSignature={null} onChange={onChange} From a704cd7baed8c4179c8142c4cea3c16469613224 Mon Sep 17 00:00:00 2001 From: Robert Date: Wed, 4 Oct 2023 14:27:59 +0100 Subject: [PATCH 17/47] Convert getModelingStatus to take ModeledMethod[] --- .../methods-usage/methods-usage-data-provider.ts | 5 ++++- .../ql-vscode/src/model-editor/shared/modeling-status.ts | 6 +++--- .../src/view/method-modeling/MethodModelingView.tsx | 3 ++- extensions/ql-vscode/src/view/model-editor/MethodRow.tsx | 5 ++++- 4 files changed, 13 insertions(+), 6 deletions(-) diff --git a/extensions/ql-vscode/src/model-editor/methods-usage/methods-usage-data-provider.ts b/extensions/ql-vscode/src/model-editor/methods-usage/methods-usage-data-provider.ts index fe8ee875d..444276579 100644 --- a/extensions/ql-vscode/src/model-editor/methods-usage/methods-usage-data-provider.ts +++ b/extensions/ql-vscode/src/model-editor/methods-usage/methods-usage-data-provider.ts @@ -102,7 +102,10 @@ export class MethodsUsageDataProvider const modeledMethod = this.modeledMethods[method.signature]; const modifiedMethod = this.modifiedMethodSignatures.has(method.signature); - const status = getModelingStatus(modeledMethod, modifiedMethod); + const status = getModelingStatus( + modeledMethod ? [modeledMethod] : [], + modifiedMethod, + ); switch (status) { case "unmodeled": return new ThemeIcon("error", new ThemeColor("errorForeground")); diff --git a/extensions/ql-vscode/src/model-editor/shared/modeling-status.ts b/extensions/ql-vscode/src/model-editor/shared/modeling-status.ts index 496b4c0e7..f0737ec32 100644 --- a/extensions/ql-vscode/src/model-editor/shared/modeling-status.ts +++ b/extensions/ql-vscode/src/model-editor/shared/modeling-status.ts @@ -3,13 +3,13 @@ import { ModeledMethod } from "../modeled-method"; export type ModelingStatus = "unmodeled" | "unsaved" | "saved"; export function getModelingStatus( - modeledMethod: ModeledMethod | undefined, + modeledMethods: ModeledMethod[], methodIsUnsaved: boolean, ): ModelingStatus { - if (modeledMethod) { + if (modeledMethods.length > 0) { if (methodIsUnsaved) { return "unsaved"; - } else if (modeledMethod.type !== "none") { + } else if (modeledMethods.some((m) => m.type !== "none")) { return "saved"; } } diff --git a/extensions/ql-vscode/src/view/method-modeling/MethodModelingView.tsx b/extensions/ql-vscode/src/view/method-modeling/MethodModelingView.tsx index 390d2fde9..9125fbe6a 100644 --- a/extensions/ql-vscode/src/view/method-modeling/MethodModelingView.tsx +++ b/extensions/ql-vscode/src/view/method-modeling/MethodModelingView.tsx @@ -18,7 +18,8 @@ export function MethodModelingView(): JSX.Element { const [isMethodModified, setIsMethodModified] = useState(false); const modelingStatus = useMemo( - () => getModelingStatus(modeledMethod, isMethodModified), + () => + getModelingStatus(modeledMethod ? [modeledMethod] : [], isMethodModified), [modeledMethod, isMethodModified], ); diff --git a/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx b/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx index 21fde2801..182366448 100644 --- a/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx +++ b/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx @@ -101,7 +101,10 @@ const ModelableMethodRow = forwardRef( [method], ); - const modelingStatus = getModelingStatus(modeledMethod, methodIsUnsaved); + const modelingStatus = getModelingStatus( + modeledMethod ? [modeledMethod] : [], + methodIsUnsaved, + ); return ( Date: Wed, 4 Oct 2023 15:41:08 +0100 Subject: [PATCH 18/47] Convert ApiOrMethodCell to ApiOrMethodRow --- .../src/view/model-editor/MethodRow.tsx | 53 ++++++++++--------- 1 file changed, 29 insertions(+), 24 deletions(-) diff --git a/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx b/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx index 182366448..7d7e0c66d 100644 --- a/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx +++ b/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx @@ -23,7 +23,8 @@ import { ModelInputDropdown } from "./ModelInputDropdown"; import { ModelOutputDropdown } from "./ModelOutputDropdown"; import { ModelEditorViewState } from "../../model-editor/shared/view-state"; -const ApiOrMethodCell = styled(VSCodeDataGridCell)` +const ApiOrMethodRow = styled.div` + min-height: calc(var(--input-height) * 1px); display: flex; flex-direction: row; align-items: center; @@ -112,18 +113,20 @@ const ModelableMethodRow = forwardRef( ref={ref} focused={revealedMethodSignature === method.signature} > - - - - - {viewState.mode === Mode.Application && ( - - {method.usages.length} - - )} - View - {props.modelingInProgress && } - + + + + + + {viewState.mode === Mode.Application && ( + + {method.usages.length} + + )} + View + {props.modelingInProgress && } + + {props.modelingInProgress && ( <> @@ -195,17 +198,19 @@ const UnmodelableMethodRow = forwardRef< ref={ref} focused={revealedMethodSignature === method.signature} > - - - - {viewState.mode === Mode.Application && ( - - {method.usages.length} - - )} - View - - + + + + + {viewState.mode === Mode.Application && ( + + {method.usages.length} + + )} + View + + + Method already modeled From 252c7a20c4560ef029a6090394bbd86c3544acf6 Mon Sep 17 00:00:00 2001 From: Robert Date: Wed, 4 Oct 2023 15:42:13 +0100 Subject: [PATCH 19/47] Convert MethodRow to display multiple modelings --- .../model-editor/MethodRow.stories.tsx | 32 ++++-- .../src/view/model-editor/MethodRow.tsx | 98 ++++++++++++------- .../model-editor/ModeledMethodDataGrid.tsx | 35 ++++--- .../model-editor/__tests__/MethodRow.spec.tsx | 6 +- 4 files changed, 111 insertions(+), 60 deletions(-) diff --git a/extensions/ql-vscode/src/stories/model-editor/MethodRow.stories.tsx b/extensions/ql-vscode/src/stories/model-editor/MethodRow.stories.tsx index 9c2118914..2e15420a1 100644 --- a/extensions/ql-vscode/src/stories/model-editor/MethodRow.stories.tsx +++ b/extensions/ql-vscode/src/stories/model-editor/MethodRow.stories.tsx @@ -7,6 +7,9 @@ import { CallClassification, Method } from "../../model-editor/method"; import { ModeledMethod } from "../../model-editor/modeled-method"; import { VSCodeDataGrid } from "@vscode/webview-ui-toolkit/react"; import { GRID_TEMPLATE_COLUMNS } from "../../view/model-editor/ModeledMethodDataGrid"; +import { ModelEditorViewState } from "../../model-editor/shared/view-state"; +import { createMockExtensionPack } from "../../../test/factories/model-editor/extension-pack"; +import { Mode } from "../../model-editor/shared/mode"; export default { title: "CodeQL Model Editor/Method Row", @@ -66,51 +69,66 @@ const modeledMethod: ModeledMethod = { methodParameters: "()", }; +const viewState: ModelEditorViewState = { + extensionPack: createMockExtensionPack(), + showFlowGeneration: true, + showLlmButton: true, + showMultipleModels: true, + mode: Mode.Application, +}; + export const Unmodeled = Template.bind({}); Unmodeled.args = { method, - modeledMethod: undefined, + modeledMethods: [], methodCanBeModeled: true, + viewState, }; export const Source = Template.bind({}); Source.args = { method, - modeledMethod: { ...modeledMethod, type: "source" }, + modeledMethods: [{ ...modeledMethod, type: "source" }], methodCanBeModeled: true, + viewState, }; export const Sink = Template.bind({}); Sink.args = { method, - modeledMethod: { ...modeledMethod, type: "sink" }, + modeledMethods: [{ ...modeledMethod, type: "sink" }], methodCanBeModeled: true, + viewState, }; export const Summary = Template.bind({}); Summary.args = { method, - modeledMethod: { ...modeledMethod, type: "summary" }, + modeledMethods: [{ ...modeledMethod, type: "summary" }], methodCanBeModeled: true, + viewState, }; export const Neutral = Template.bind({}); Neutral.args = { method, - modeledMethod: { ...modeledMethod, type: "neutral" }, + modeledMethods: [{ ...modeledMethod, type: "neutral" }], methodCanBeModeled: true, + viewState, }; export const AlreadyModeled = Template.bind({}); AlreadyModeled.args = { method: { ...method, supported: true }, - modeledMethod: undefined, + modeledMethods: [], + viewState, }; export const ModelingInProgress = Template.bind({}); ModelingInProgress.args = { method, - modeledMethod, + modeledMethods: [modeledMethod], modelingInProgress: true, methodCanBeModeled: true, + viewState, }; diff --git a/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx b/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx index 7d7e0c66d..32be92cda 100644 --- a/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx +++ b/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx @@ -23,6 +23,12 @@ import { ModelInputDropdown } from "./ModelInputDropdown"; import { ModelOutputDropdown } from "./ModelOutputDropdown"; import { ModelEditorViewState } from "../../model-editor/shared/view-state"; +const MultiModelColumn = styled(VSCodeDataGridCell)` + display: flex; + flex-direction: column; + gap: 0.5em; +`; + const ApiOrMethodRow = styled.div` min-height: calc(var(--input-height) * 1px); display: flex; @@ -57,7 +63,7 @@ const DataGridRow = styled(VSCodeDataGridRow)<{ focused?: boolean }>` export type MethodRowProps = { method: Method; methodCanBeModeled: boolean; - modeledMethod: ModeledMethod | undefined; + modeledMethods: ModeledMethod[]; methodIsUnsaved: boolean; modelingInProgress: boolean; viewState: ModelEditorViewState; @@ -90,22 +96,23 @@ const ModelableMethodRow = forwardRef( (props, ref) => { const { method, - modeledMethod, + modeledMethods: modeledMethodsArg, methodIsUnsaved, viewState, revealedMethodSignature, onChange, } = props; + const modeledMethods = viewState.showMultipleModels + ? modeledMethodsArg + : modeledMethodsArg.slice(0, 1); + const jumpToUsage = useCallback( () => sendJumpToUsageMessage(method), [method], ); - const modelingStatus = getModelingStatus( - modeledMethod ? [modeledMethod] : [], - methodIsUnsaved, - ); + const modelingStatus = getModelingStatus(modeledMethods, methodIsUnsaved); return ( ( )} {!props.modelingInProgress && ( <> - - - - - - - - - - - - + + {forEachModeledMethod(modeledMethods, (modeledMethod) => ( + + ))} + + + {forEachModeledMethod(modeledMethods, (modeledMethod) => ( + + ))} + + + {forEachModeledMethod(modeledMethods, (modeledMethod) => ( + + ))} + + + {forEachModeledMethod(modeledMethods, (modeledMethod) => ( + + ))} + )} @@ -227,3 +246,14 @@ function sendJumpToUsageMessage(method: Method) { usage: method.usages[0], }); } + +function forEachModeledMethod( + modeledMethods: ModeledMethod[], + renderer: (modeledMethod: ModeledMethod | undefined) => JSX.Element, +): JSX.Element | JSX.Element[] { + if (modeledMethods.length === 0) { + return renderer(undefined); + } else { + return modeledMethods.map(renderer); + } +} diff --git a/extensions/ql-vscode/src/view/model-editor/ModeledMethodDataGrid.tsx b/extensions/ql-vscode/src/view/model-editor/ModeledMethodDataGrid.tsx index 9ff27ad21..bf53b479c 100644 --- a/extensions/ql-vscode/src/view/model-editor/ModeledMethodDataGrid.tsx +++ b/extensions/ql-vscode/src/view/model-editor/ModeledMethodDataGrid.tsx @@ -84,22 +84,25 @@ export const ModeledMethodDataGrid = ({ Kind - {methodsWithModelability.map(({ method, methodCanBeModeled }) => ( - - ))} + {methodsWithModelability.map(({ method, methodCanBeModeled }) => { + const modeledMethod = modeledMethods[method.signature]; + return ( + + ); + })} )} { { it("shows the modeling status indicator when unmodeled", () => { render({ - modeledMethod: undefined, + modeledMethods: [], }); expect(screen.getByLabelText("Method not modeled")).toBeInTheDocument(); @@ -137,7 +137,7 @@ describe(MethodRow.name, () => { it("renders an unmodelable method", () => { render({ methodCanBeModeled: false, - modeledMethod: undefined, + modeledMethods: [], }); expect(screen.queryByRole("combobox")).not.toBeInTheDocument(); From 8eef4eb4f25b4c65f02c6e0794cd545c8c3466f1 Mon Sep 17 00:00:00 2001 From: Robert Date: Wed, 4 Oct 2023 15:42:35 +0100 Subject: [PATCH 20/47] Add story with multiple modelings --- .../src/stories/model-editor/MethodRow.stories.tsx | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/extensions/ql-vscode/src/stories/model-editor/MethodRow.stories.tsx b/extensions/ql-vscode/src/stories/model-editor/MethodRow.stories.tsx index 2e15420a1..9ea43028a 100644 --- a/extensions/ql-vscode/src/stories/model-editor/MethodRow.stories.tsx +++ b/extensions/ql-vscode/src/stories/model-editor/MethodRow.stories.tsx @@ -132,3 +132,15 @@ ModelingInProgress.args = { methodCanBeModeled: true, viewState, }; + +export const MultipleModelings = Template.bind({}); +MultipleModelings.args = { + method, + modeledMethods: [ + { ...modeledMethod, type: "source" }, + { ...modeledMethod, type: "sink" }, + { ...modeledMethod }, + ], + methodCanBeModeled: true, + viewState, +}; From f87b1c46de5d6580626a152cd32ea0db2d90ddbc Mon Sep 17 00:00:00 2001 From: Robert Date: Wed, 4 Oct 2023 18:26:52 +0100 Subject: [PATCH 21/47] Add some simple tests of rendering multiple models --- .../model-editor/__tests__/MethodRow.spec.tsx | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/extensions/ql-vscode/src/view/model-editor/__tests__/MethodRow.spec.tsx b/extensions/ql-vscode/src/view/model-editor/__tests__/MethodRow.spec.tsx index 11d435021..fb1b99601 100644 --- a/extensions/ql-vscode/src/view/model-editor/__tests__/MethodRow.spec.tsx +++ b/extensions/ql-vscode/src/view/model-editor/__tests__/MethodRow.spec.tsx @@ -134,6 +134,44 @@ describe(MethodRow.name, () => { expect(screen.getByLabelText("Loading")).toBeInTheDocument(); }); + it("can render multiple models", () => { + render({ + modeledMethods: [ + { ...modeledMethod, type: "source" }, + { ...modeledMethod, type: "sink" }, + { ...modeledMethod, type: "summary" }, + ], + viewState: { + ...viewState, + showMultipleModels: true, + }, + }); + + const kindInputs = screen.getAllByRole("combobox", { name: "Model type" }); + expect(kindInputs).toHaveLength(3); + expect(kindInputs[0]).toHaveValue("source"); + expect(kindInputs[1]).toHaveValue("sink"); + expect(kindInputs[2]).toHaveValue("summary"); + }); + + it("renders only first model when showMultipleModels feature flag is disabled", () => { + render({ + modeledMethods: [ + { ...modeledMethod, type: "source" }, + { ...modeledMethod, type: "sink" }, + { ...modeledMethod, type: "summary" }, + ], + viewState: { + ...viewState, + showMultipleModels: false, + }, + }); + + const kindInputs = screen.getAllByRole("combobox", { name: "Model type" }); + expect(kindInputs.length).toBe(1); + expect(kindInputs[0]).toHaveValue("source"); + }); + it("renders an unmodelable method", () => { render({ methodCanBeModeled: false, From 5ba64a1db3553c3844c9428afa255676f5983ddc Mon Sep 17 00:00:00 2001 From: Robert Date: Wed, 4 Oct 2023 18:45:56 +0100 Subject: [PATCH 22/47] Add test for when there's no modeled method --- .../src/view/model-editor/__tests__/MethodRow.spec.tsx | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/extensions/ql-vscode/src/view/model-editor/__tests__/MethodRow.spec.tsx b/extensions/ql-vscode/src/view/model-editor/__tests__/MethodRow.spec.tsx index fb1b99601..4f34ab1e7 100644 --- a/extensions/ql-vscode/src/view/model-editor/__tests__/MethodRow.spec.tsx +++ b/extensions/ql-vscode/src/view/model-editor/__tests__/MethodRow.spec.tsx @@ -64,6 +64,14 @@ describe(MethodRow.name, () => { expect(screen.queryByLabelText("Loading")).not.toBeInTheDocument(); }); + it("renders when there is no modeled method", () => { + render({ modeledMethods: [] }); + + expect(screen.queryAllByRole("combobox")).toHaveLength(4); + expect(screen.getByLabelText("Method not modeled")).toBeInTheDocument(); + expect(screen.queryByLabelText("Loading")).not.toBeInTheDocument(); + }); + it("can change the kind", async () => { render(); From 2b0bd840e616d941459b0451905588856f9c0240 Mon Sep 17 00:00:00 2001 From: Robert Date: Thu, 5 Oct 2023 15:25:42 +0100 Subject: [PATCH 23/47] 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/47] 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/47] 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 9c275130a5a36281da1486d3daaafbd83de0e9f3 Mon Sep 17 00:00:00 2001 From: Koen Vlaswinkel Date: Fri, 6 Oct 2023 12:09:00 +0200 Subject: [PATCH 26/47] 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 08944a292c79d5c4ffdb6c6218999e190a84dd7a Mon Sep 17 00:00:00 2001 From: Koen Vlaswinkel Date: Fri, 6 Oct 2023 14:15:23 +0200 Subject: [PATCH 27/47] 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 28/47] 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 ee1bf8896e87500600a3656774bd1a84753c6599 Mon Sep 17 00:00:00 2001 From: Koen Vlaswinkel Date: Fri, 6 Oct 2023 15:48:41 +0200 Subject: [PATCH 29/47] 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 30/47] 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 31/47] 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 32/47] 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 ? ( + ) : ( + )}