From 1bd78649e78b83578c6db62bd031751ab66a7565 Mon Sep 17 00:00:00 2001 From: Robert Date: Fri, 9 Jun 2023 09:57:23 +0100 Subject: [PATCH 1/4] Remove workspaceFolders from app --- extensions/ql-vscode/src/common/app.ts | 7 +------ extensions/ql-vscode/src/common/vscode/vscode-app.ts | 4 ---- extensions/ql-vscode/test/__mocks__/appMock.ts | 9 +-------- 3 files changed, 2 insertions(+), 18 deletions(-) diff --git a/extensions/ql-vscode/src/common/app.ts b/extensions/ql-vscode/src/common/app.ts index 4f61cd12a6f..fe9bc7ed485 100644 --- a/extensions/ql-vscode/src/common/app.ts +++ b/extensions/ql-vscode/src/common/app.ts @@ -4,11 +4,7 @@ import { AppEventEmitter } from "./events"; import { Logger } from "./logging"; import { Memento } from "./memento"; import { AppCommandManager } from "./commands"; -import type { - WorkspaceFolder, - Event, - WorkspaceFoldersChangeEvent, -} from "vscode"; +import type { Event, WorkspaceFoldersChangeEvent } from "vscode"; export interface App { createEventEmitter(): AppEventEmitter; @@ -19,7 +15,6 @@ export interface App { readonly globalStoragePath: string; readonly workspaceStoragePath?: string; readonly workspaceState: Memento; - readonly workspaceFolders: readonly WorkspaceFolder[] | undefined; readonly onDidChangeWorkspaceFolders: Event; readonly credentials: Credentials; readonly commands: AppCommandManager; diff --git a/extensions/ql-vscode/src/common/vscode/vscode-app.ts b/extensions/ql-vscode/src/common/vscode/vscode-app.ts index 45e3f9d8409..99bb83baf78 100644 --- a/extensions/ql-vscode/src/common/vscode/vscode-app.ts +++ b/extensions/ql-vscode/src/common/vscode/vscode-app.ts @@ -40,10 +40,6 @@ export class ExtensionApp implements App { return this.extensionContext.workspaceState; } - public get workspaceFolders(): readonly vscode.WorkspaceFolder[] | undefined { - return vscode.workspace.workspaceFolders; - } - public get onDidChangeWorkspaceFolders(): vscode.Event { return vscode.workspace.onDidChangeWorkspaceFolders; } diff --git a/extensions/ql-vscode/test/__mocks__/appMock.ts b/extensions/ql-vscode/test/__mocks__/appMock.ts index 260e5a25888..ec0d112fb60 100644 --- a/extensions/ql-vscode/test/__mocks__/appMock.ts +++ b/extensions/ql-vscode/test/__mocks__/appMock.ts @@ -8,11 +8,7 @@ import { testCredentialsWithStub } from "../factories/authentication"; import { Credentials } from "../../src/common/authentication"; import { AppCommandManager } from "../../src/common/commands"; import { createMockCommandManager } from "./commandsMock"; -import type { - Event, - WorkspaceFolder, - WorkspaceFoldersChangeEvent, -} from "vscode"; +import type { Event, WorkspaceFoldersChangeEvent } from "vscode"; export function createMockApp({ extensionPath = "/mock/extension/path", @@ -20,7 +16,6 @@ export function createMockApp({ globalStoragePath = "/mock/global/storage/path", createEventEmitter = () => new MockAppEventEmitter(), workspaceState = createMockMemento(), - workspaceFolders = [], onDidChangeWorkspaceFolders = jest.fn(), credentials = testCredentialsWithStub(), commands = createMockCommandManager(), @@ -31,7 +26,6 @@ export function createMockApp({ globalStoragePath?: string; createEventEmitter?: () => AppEventEmitter; workspaceState?: Memento; - workspaceFolders?: readonly WorkspaceFolder[] | undefined; onDidChangeWorkspaceFolders?: Event; credentials?: Credentials; commands?: AppCommandManager; @@ -45,7 +39,6 @@ export function createMockApp({ workspaceStoragePath, globalStoragePath, workspaceState, - workspaceFolders, onDidChangeWorkspaceFolders, createEventEmitter, credentials, From f0cf4a01058499a106ff2ce42363a0661095f38d Mon Sep 17 00:00:00 2001 From: Robert Date: Fri, 9 Jun 2023 10:18:22 +0100 Subject: [PATCH 2/4] Remove onDidChangeWorkspaceFolders from app --- extensions/ql-vscode/src/common/app.ts | 2 -- extensions/ql-vscode/src/common/vscode/vscode-app.ts | 4 ---- .../ql-vscode/src/queries-panel/query-discovery.ts | 10 ++++++++-- extensions/ql-vscode/test/__mocks__/appMock.ts | 4 ---- .../queries-panel/query-discovery.test.ts | 4 +++- 5 files changed, 11 insertions(+), 13 deletions(-) diff --git a/extensions/ql-vscode/src/common/app.ts b/extensions/ql-vscode/src/common/app.ts index fe9bc7ed485..73badc6eeb4 100644 --- a/extensions/ql-vscode/src/common/app.ts +++ b/extensions/ql-vscode/src/common/app.ts @@ -4,7 +4,6 @@ import { AppEventEmitter } from "./events"; import { Logger } from "./logging"; import { Memento } from "./memento"; import { AppCommandManager } from "./commands"; -import type { Event, WorkspaceFoldersChangeEvent } from "vscode"; export interface App { createEventEmitter(): AppEventEmitter; @@ -15,7 +14,6 @@ export interface App { readonly globalStoragePath: string; readonly workspaceStoragePath?: string; readonly workspaceState: Memento; - readonly onDidChangeWorkspaceFolders: Event; readonly credentials: Credentials; readonly commands: AppCommandManager; readonly environment: EnvironmentContext; diff --git a/extensions/ql-vscode/src/common/vscode/vscode-app.ts b/extensions/ql-vscode/src/common/vscode/vscode-app.ts index 99bb83baf78..2e006e47743 100644 --- a/extensions/ql-vscode/src/common/vscode/vscode-app.ts +++ b/extensions/ql-vscode/src/common/vscode/vscode-app.ts @@ -40,10 +40,6 @@ export class ExtensionApp implements App { return this.extensionContext.workspaceState; } - public get onDidChangeWorkspaceFolders(): vscode.Event { - return vscode.workspace.onDidChangeWorkspaceFolders; - } - public get subscriptions(): Disposable[] { return this.extensionContext.subscriptions; } diff --git a/extensions/ql-vscode/src/queries-panel/query-discovery.ts b/extensions/ql-vscode/src/queries-panel/query-discovery.ts index 43bfd295587..5c0fe6f680d 100644 --- a/extensions/ql-vscode/src/queries-panel/query-discovery.ts +++ b/extensions/ql-vscode/src/queries-panel/query-discovery.ts @@ -1,7 +1,13 @@ import { dirname, basename, normalize, relative } from "path"; import { Discovery } from "../common/discovery"; import { CodeQLCliServer } from "../codeql-cli/cli"; -import { Event, RelativePattern, Uri, WorkspaceFolder } from "vscode"; +import { + Event, + RelativePattern, + Uri, + WorkspaceFolder, + workspace, +} from "vscode"; import { MultiFileSystemWatcher } from "../common/vscode/multi-file-system-watcher"; import { App } from "../common/app"; import { FileTreeDirectory, FileTreeLeaf } from "../common/file-tree-nodes"; @@ -48,7 +54,7 @@ export class QueryDiscovery super("Query Discovery", extLogger); this.onDidChangeQueriesEmitter = this.push(app.createEventEmitter()); - this.push(app.onDidChangeWorkspaceFolders(this.refresh.bind(this))); + this.push(workspace.onDidChangeWorkspaceFolders(this.refresh.bind(this))); this.push(this.watcher.onDidChange(this.refresh.bind(this))); } diff --git a/extensions/ql-vscode/test/__mocks__/appMock.ts b/extensions/ql-vscode/test/__mocks__/appMock.ts index ec0d112fb60..03bda4dc329 100644 --- a/extensions/ql-vscode/test/__mocks__/appMock.ts +++ b/extensions/ql-vscode/test/__mocks__/appMock.ts @@ -8,7 +8,6 @@ import { testCredentialsWithStub } from "../factories/authentication"; import { Credentials } from "../../src/common/authentication"; import { AppCommandManager } from "../../src/common/commands"; import { createMockCommandManager } from "./commandsMock"; -import type { Event, WorkspaceFoldersChangeEvent } from "vscode"; export function createMockApp({ extensionPath = "/mock/extension/path", @@ -16,7 +15,6 @@ export function createMockApp({ globalStoragePath = "/mock/global/storage/path", createEventEmitter = () => new MockAppEventEmitter(), workspaceState = createMockMemento(), - onDidChangeWorkspaceFolders = jest.fn(), credentials = testCredentialsWithStub(), commands = createMockCommandManager(), environment = createMockEnvironmentContext(), @@ -26,7 +24,6 @@ export function createMockApp({ globalStoragePath?: string; createEventEmitter?: () => AppEventEmitter; workspaceState?: Memento; - onDidChangeWorkspaceFolders?: Event; credentials?: Credentials; commands?: AppCommandManager; environment?: EnvironmentContext; @@ -39,7 +36,6 @@ export function createMockApp({ workspaceStoragePath, globalStoragePath, workspaceState, - onDidChangeWorkspaceFolders, createEventEmitter, credentials, commands, diff --git a/extensions/ql-vscode/test/vscode-tests/minimal-workspace/queries-panel/query-discovery.test.ts b/extensions/ql-vscode/test/vscode-tests/minimal-workspace/queries-panel/query-discovery.test.ts index 563ee6119a8..f4b2b3524ad 100644 --- a/extensions/ql-vscode/test/vscode-tests/minimal-workspace/queries-panel/query-discovery.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/minimal-workspace/queries-panel/query-discovery.test.ts @@ -180,11 +180,13 @@ describe("QueryDiscovery", () => { it("should refresh when workspace folders change", async () => { const onDidChangeWorkspaceFoldersEvent = new EventEmitter(); + jest + .spyOn(workspace, "onDidChangeWorkspaceFolders") + .mockImplementation(onDidChangeWorkspaceFoldersEvent.event); const discovery = new QueryDiscovery( createMockApp({ createEventEmitter: () => new EventEmitter(), - onDidChangeWorkspaceFolders: onDidChangeWorkspaceFoldersEvent.event, }), mockedObject({ resolveQueries: jest.fn().mockResolvedValue([]), From a19c40bd661f4d4a74ef7ee794e5002edfbb555f Mon Sep 17 00:00:00 2001 From: Robert Date: Fri, 9 Jun 2023 10:24:07 +0100 Subject: [PATCH 3/4] Avoid using app.createEventEmitter --- .../ql-vscode/src/queries-panel/query-discovery.ts | 3 ++- .../queries-panel/query-discovery.test.ts | 11 ++--------- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/extensions/ql-vscode/src/queries-panel/query-discovery.ts b/extensions/ql-vscode/src/queries-panel/query-discovery.ts index 5c0fe6f680d..dde498a28f4 100644 --- a/extensions/ql-vscode/src/queries-panel/query-discovery.ts +++ b/extensions/ql-vscode/src/queries-panel/query-discovery.ts @@ -3,6 +3,7 @@ import { Discovery } from "../common/discovery"; import { CodeQLCliServer } from "../codeql-cli/cli"; import { Event, + EventEmitter, RelativePattern, Uri, WorkspaceFolder, @@ -53,7 +54,7 @@ export class QueryDiscovery ) { super("Query Discovery", extLogger); - this.onDidChangeQueriesEmitter = this.push(app.createEventEmitter()); + this.onDidChangeQueriesEmitter = this.push(new EventEmitter()); this.push(workspace.onDidChangeWorkspaceFolders(this.refresh.bind(this))); this.push(this.watcher.onDidChange(this.refresh.bind(this))); } diff --git a/extensions/ql-vscode/test/vscode-tests/minimal-workspace/queries-panel/query-discovery.test.ts b/extensions/ql-vscode/test/vscode-tests/minimal-workspace/queries-panel/query-discovery.test.ts index f4b2b3524ad..b5a67d655fc 100644 --- a/extensions/ql-vscode/test/vscode-tests/minimal-workspace/queries-panel/query-discovery.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/minimal-workspace/queries-panel/query-discovery.test.ts @@ -153,12 +153,7 @@ describe("QueryDiscovery", () => { .mockResolvedValue([join(workspaceRoot, "query1.ql")]), }); - const discovery = new QueryDiscovery( - createMockApp({ - createEventEmitter: () => new EventEmitter(), - }), - cli, - ); + const discovery = new QueryDiscovery(createMockApp({}), cli); const onDidChangeQueriesSpy = jest.fn(); discovery.onDidChangeQueries(onDidChangeQueriesSpy); @@ -185,9 +180,7 @@ describe("QueryDiscovery", () => { .mockImplementation(onDidChangeWorkspaceFoldersEvent.event); const discovery = new QueryDiscovery( - createMockApp({ - createEventEmitter: () => new EventEmitter(), - }), + createMockApp({}), mockedObject({ resolveQueries: jest.fn().mockResolvedValue([]), }), From eb938034fb231bb19169c97b776682cf1563e3e6 Mon Sep 17 00:00:00 2001 From: Robert Date: Fri, 9 Jun 2023 10:26:50 +0100 Subject: [PATCH 4/4] Pass in just the environment instead of full app --- .../ql-vscode/src/queries-panel/queries-module.ts | 2 +- .../ql-vscode/src/queries-panel/query-discovery.ts | 6 +++--- .../queries-panel/query-discovery.test.ts | 14 +++++++------- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/extensions/ql-vscode/src/queries-panel/queries-module.ts b/extensions/ql-vscode/src/queries-panel/queries-module.ts index aede56a8d9b..ee39f5d37e0 100644 --- a/extensions/ql-vscode/src/queries-panel/queries-module.ts +++ b/extensions/ql-vscode/src/queries-panel/queries-module.ts @@ -19,7 +19,7 @@ export class QueriesModule extends DisposableObject { } void extLogger.log("Initializing queries panel."); - const queryDiscovery = new QueryDiscovery(app, cliServer); + const queryDiscovery = new QueryDiscovery(app.environment, cliServer); this.push(queryDiscovery); void queryDiscovery.refresh(); diff --git a/extensions/ql-vscode/src/queries-panel/query-discovery.ts b/extensions/ql-vscode/src/queries-panel/query-discovery.ts index dde498a28f4..0b3e559c460 100644 --- a/extensions/ql-vscode/src/queries-panel/query-discovery.ts +++ b/extensions/ql-vscode/src/queries-panel/query-discovery.ts @@ -10,7 +10,7 @@ import { workspace, } from "vscode"; import { MultiFileSystemWatcher } from "../common/vscode/multi-file-system-watcher"; -import { App } from "../common/app"; +import { EnvironmentContext } from "../common/app"; import { FileTreeDirectory, FileTreeLeaf } from "../common/file-tree-nodes"; import { getOnDiskWorkspaceFoldersObjects } from "../helpers"; import { AppEventEmitter } from "../common/events"; @@ -49,7 +49,7 @@ export class QueryDiscovery ); constructor( - private readonly app: App, + private readonly env: EnvironmentContext, private readonly cliServer: CodeQLCliServer, ) { super("Query Discovery", extLogger); @@ -137,7 +137,7 @@ export class QueryDiscovery const rootDirectory = new FileTreeDirectory( fullPath, name, - this.app.environment, + this.env, ); for (const queryPath of resolvedQueries) { const relativePath = normalize(relative(fullPath, queryPath)); diff --git a/extensions/ql-vscode/test/vscode-tests/minimal-workspace/queries-panel/query-discovery.test.ts b/extensions/ql-vscode/test/vscode-tests/minimal-workspace/queries-panel/query-discovery.test.ts index b5a67d655fc..67f7dd4a176 100644 --- a/extensions/ql-vscode/test/vscode-tests/minimal-workspace/queries-panel/query-discovery.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/minimal-workspace/queries-panel/query-discovery.test.ts @@ -7,7 +7,7 @@ import { } from "vscode"; import { CodeQLCliServer } from "../../../../src/codeql-cli/cli"; import { QueryDiscovery } from "../../../../src/queries-panel/query-discovery"; -import { createMockApp } from "../../../__mocks__/appMock"; +import { createMockEnvironmentContext } from "../../../__mocks__/appMock"; import { mockedObject } from "../../utils/mocking.helpers"; import { basename, join, sep } from "path"; @@ -23,7 +23,7 @@ describe("QueryDiscovery", () => { resolveQueries, }); - const discovery = new QueryDiscovery(createMockApp({}), cli); + const discovery = new QueryDiscovery(createMockEnvironmentContext(), cli); await discovery.refresh(); const queries = discovery.queries; @@ -43,7 +43,7 @@ describe("QueryDiscovery", () => { ]), }); - const discovery = new QueryDiscovery(createMockApp({}), cli); + const discovery = new QueryDiscovery(createMockEnvironmentContext(), cli); await discovery.refresh(); const queries = discovery.queries; expect(queries).toBeDefined(); @@ -69,7 +69,7 @@ describe("QueryDiscovery", () => { ]), }); - const discovery = new QueryDiscovery(createMockApp({}), cli); + const discovery = new QueryDiscovery(createMockEnvironmentContext(), cli); await discovery.refresh(); const queries = discovery.queries; expect(queries).toBeDefined(); @@ -114,7 +114,7 @@ describe("QueryDiscovery", () => { resolveQueries, }); - const discovery = new QueryDiscovery(createMockApp({}), cli); + const discovery = new QueryDiscovery(createMockEnvironmentContext(), cli); await discovery.refresh(); const queries = discovery.queries; expect(queries).toBeDefined(); @@ -153,7 +153,7 @@ describe("QueryDiscovery", () => { .mockResolvedValue([join(workspaceRoot, "query1.ql")]), }); - const discovery = new QueryDiscovery(createMockApp({}), cli); + const discovery = new QueryDiscovery(createMockEnvironmentContext(), cli); const onDidChangeQueriesSpy = jest.fn(); discovery.onDidChangeQueries(onDidChangeQueriesSpy); @@ -180,7 +180,7 @@ describe("QueryDiscovery", () => { .mockImplementation(onDidChangeWorkspaceFoldersEvent.event); const discovery = new QueryDiscovery( - createMockApp({}), + createMockEnvironmentContext(), mockedObject({ resolveQueries: jest.fn().mockResolvedValue([]), }),