diff --git a/extensions/ql-vscode/src/common/app.ts b/extensions/ql-vscode/src/common/app.ts index 67506a5fa..278ea2b9d 100644 --- a/extensions/ql-vscode/src/common/app.ts +++ b/extensions/ql-vscode/src/common/app.ts @@ -3,6 +3,7 @@ import { Disposable } from "../pure/disposable-object"; import { AppEventEmitter } from "./events"; import { Logger } from "./logging"; import { Memento } from "./memento"; +import { AppCommandManager } from "./commands"; export interface App { createEventEmitter(): AppEventEmitter; @@ -15,6 +16,7 @@ export interface App { readonly workspaceStoragePath?: string; readonly workspaceState: Memento; readonly credentials: Credentials; + readonly commands: AppCommandManager; } export enum AppMode { diff --git a/extensions/ql-vscode/src/common/commands.ts b/extensions/ql-vscode/src/common/commands.ts new file mode 100644 index 000000000..d4ceada59 --- /dev/null +++ b/extensions/ql-vscode/src/common/commands.ts @@ -0,0 +1,24 @@ +import { CommandManager } from "../packages/commands"; + +/** + * Contains type definitions for all commands used by the extension. + * + * To add a new command first define its type here, then provide + * the implementation in the corresponding `getCommands` function. + */ + +// Base commands not tied directly to a module like e.g. variant analysis. +export type BaseCommands = { + "codeQL.openDocumentation": () => Promise; +}; + +// Commands tied to variant analysis +export type VariantAnalysisCommands = { + "codeQL.openVariantAnalysisLogs": ( + variantAnalysisId: number, + ) => Promise; +}; + +export type AllCommands = BaseCommands & VariantAnalysisCommands; + +export type AppCommandManager = CommandManager; diff --git a/extensions/ql-vscode/src/common/vscode/commands.ts b/extensions/ql-vscode/src/common/vscode/commands.ts new file mode 100644 index 000000000..86e32faa4 --- /dev/null +++ b/extensions/ql-vscode/src/common/vscode/commands.ts @@ -0,0 +1,32 @@ +import { commands } from "vscode"; +import { commandRunner } from "../../commandRunner"; +import { CommandFunction, CommandManager } from "../../packages/commands"; + +/** + * Create a command manager for VSCode, wrapping the commandRunner + * and vscode.executeCommand. + */ +export function createVSCodeCommandManager< + Commands extends Record, +>(): CommandManager { + return new CommandManager(commandRunner, wrapExecuteCommand); +} + +/** + * wrapExecuteCommand wraps commands.executeCommand to satisfy that the + * type is a Promise. Type script does not seem to be smart enough + * to figure out that `ReturnType` is actually + * a Promise, so we need to add a second layer of wrapping and unwrapping + * (The `Promise, + CommandName extends keyof Commands & string = keyof Commands & string, +>( + commandName: CommandName, + ...args: Parameters +): Promise>> { + return await commands.executeCommand< + Awaited> + >(commandName, ...args); +} diff --git a/extensions/ql-vscode/src/common/vscode/vscode-app.ts b/extensions/ql-vscode/src/common/vscode/vscode-app.ts index 8f54341b6..1ae588278 100644 --- a/extensions/ql-vscode/src/common/vscode/vscode-app.ts +++ b/extensions/ql-vscode/src/common/vscode/vscode-app.ts @@ -6,14 +6,19 @@ import { AppEventEmitter } from "../events"; import { extLogger, Logger } from "../logging"; import { Memento } from "../memento"; import { VSCodeAppEventEmitter } from "./events"; +import { AppCommandManager } from "../commands"; +import { createVSCodeCommandManager } from "./commands"; export class ExtensionApp implements App { public readonly credentials: VSCodeCredentials; + public readonly commands: AppCommandManager; public constructor( public readonly extensionContext: vscode.ExtensionContext, ) { this.credentials = new VSCodeCredentials(); + this.commands = createVSCodeCommandManager(); + extensionContext.subscriptions.push(this.commands); } public get extensionPath(): string { diff --git a/extensions/ql-vscode/src/extension.ts b/extensions/ql-vscode/src/extension.ts index 5b4edf7cd..4620d49f4 100644 --- a/extensions/ql-vscode/src/extension.ts +++ b/extensions/ql-vscode/src/extension.ts @@ -136,6 +136,7 @@ import { RepositoriesFilterSortStateWithIds } from "./pure/variant-analysis-filt import { DbModule } from "./databases/db-module"; import { redactableError } from "./pure/errors"; import { QueryHistoryDirs } from "./query-history/query-history-dirs"; +import { AllCommands, BaseCommands } from "./common/commands"; /** * extension.ts @@ -167,6 +168,17 @@ let isInstallingOrUpdatingDistribution = false; const extensionId = "GitHub.vscode-codeql"; const extension = extensions.getExtension(extensionId); +/** + * Return all commands that are not tied to the more specific managers. + */ +function getCommands(): BaseCommands { + return { + "codeQL.openDocumentation": async () => { + await env.openExternal(Uri.parse("https://codeql.github.com/docs/")); + }, + }; +} + /** * If the user tries to execute vscode commands after extension activation is failed, give * a sensible error message. @@ -1191,14 +1203,14 @@ async function activateWithInstalledDistribution( ), ); - ctx.subscriptions.push( - commandRunner( - "codeQL.openVariantAnalysisLogs", - async (variantAnalysisId: number) => { - await variantAnalysisManager.openVariantAnalysisLogs(variantAnalysisId); - }, - ), - ); + const allCommands: AllCommands = { + ...getCommands(), + ...variantAnalysisManager.getCommands(), + }; + + for (const [commandName, command] of Object.entries(allCommands)) { + app.commands.register(commandName as keyof AllCommands, command); + } ctx.subscriptions.push( commandRunner( @@ -1410,12 +1422,6 @@ async function activateWithInstalledDistribution( ), ); - ctx.subscriptions.push( - commandRunner("codeQL.openDocumentation", async () => - env.openExternal(Uri.parse("https://codeql.github.com/docs/")), - ), - ); - ctx.subscriptions.push( commandRunner("codeQL.copyVersion", async () => { const text = `CodeQL extension version: ${ diff --git a/extensions/ql-vscode/src/packages/commands/CommandManager.ts b/extensions/ql-vscode/src/packages/commands/CommandManager.ts index c0b1d38f2..c328d9bed 100644 --- a/extensions/ql-vscode/src/packages/commands/CommandManager.ts +++ b/extensions/ql-vscode/src/packages/commands/CommandManager.ts @@ -1 +1,70 @@ -export class CommandManager {} +/** + * Contains a generic implementation of typed commands. + * + * This allows different parts of the extension to register commands with a certain type, + * and then allow other parts to call those commands in a well-typed manner. + */ + +import { Disposable } from "./Disposable"; + +/** + * A command function is a completely untyped command. + */ +export type CommandFunction = (...args: any[]) => Promise; + +/** + * The command manager basically takes a single input, the type + * of all the known commands. The second parameter is provided by + * default (and should not be needed by the caller) it is a + * technicality to allow the type system to look up commands. + */ +export class CommandManager< + Commands extends Record, + CommandName extends keyof Commands & string = keyof Commands & string, +> implements Disposable +{ + // TODO: should this be a map? + // TODO: handle multiple command names + private commands: Disposable[] = []; + + constructor( + private readonly commandRegister: ( + commandName: T, + fn: Commands[T], + ) => Disposable, + private readonly commandExecute: ( + commandName: T, + ...args: Parameters + ) => Promise>>, + ) {} + + /** + * Register a command with the specified name and implementation. + */ + register( + commandName: T, + definition: Commands[T], + ): void { + this.commands.push(this.commandRegister(commandName, definition)); + } + + /** + * Execute a command with the specified name and the provided arguments. + */ + execute( + commandName: T, + ...args: Parameters + ): Promise>> { + return this.commandExecute(commandName, ...args); + } + + /** + * Dispose the manager, disposing all the registered commands. + */ + dispose(): void { + this.commands.forEach((cmd) => { + cmd.dispose(); + }); + this.commands = []; + } +} diff --git a/extensions/ql-vscode/src/packages/commands/Disposable.ts b/extensions/ql-vscode/src/packages/commands/Disposable.ts new file mode 100644 index 000000000..3a6dd5c17 --- /dev/null +++ b/extensions/ql-vscode/src/packages/commands/Disposable.ts @@ -0,0 +1,7 @@ +/** + * This interface mirrors the vscode.Disaposable class, so that + * the command manager does not depend on vscode directly. + */ +export interface Disposable { + dispose(): void; +} diff --git a/extensions/ql-vscode/src/variant-analysis/variant-analysis-manager.ts b/extensions/ql-vscode/src/variant-analysis/variant-analysis-manager.ts index af42da85c..c7ebbebf1 100644 --- a/extensions/ql-vscode/src/variant-analysis/variant-analysis-manager.ts +++ b/extensions/ql-vscode/src/variant-analysis/variant-analysis-manager.ts @@ -62,6 +62,7 @@ import { URLSearchParams } from "url"; import { DbManager } from "../databases/db-manager"; import { App } from "../common/app"; import { redactableError } from "../pure/errors"; +import { AppCommandManager, VariantAnalysisCommands } from "../common/commands"; export class VariantAnalysisManager extends DisposableObject @@ -123,6 +124,18 @@ export class VariantAnalysisManager ); } + getCommands(): VariantAnalysisCommands { + return { + "codeQL.openVariantAnalysisLogs": async (variantAnalysisId: number) => { + await this.openVariantAnalysisLogs(variantAnalysisId); + }, + }; + } + + get commandManager(): AppCommandManager { + return this.app.commands; + } + public async runVariantAnalysis( uri: Uri | undefined, progress: ProgressCallback, diff --git a/extensions/ql-vscode/src/variant-analysis/variant-analysis-view-manager.ts b/extensions/ql-vscode/src/variant-analysis/variant-analysis-view-manager.ts index 7f307c42d..9f17a4bd4 100644 --- a/extensions/ql-vscode/src/variant-analysis/variant-analysis-view-manager.ts +++ b/extensions/ql-vscode/src/variant-analysis/variant-analysis-view-manager.ts @@ -2,6 +2,7 @@ import { VariantAnalysis, VariantAnalysisScannedRepositoryState, } from "./shared/variant-analysis"; +import { AppCommandManager } from "../common/commands"; export interface VariantAnalysisViewInterface { variantAnalysisId: number; @@ -11,6 +12,8 @@ export interface VariantAnalysisViewInterface { export interface VariantAnalysisViewManager< T extends VariantAnalysisViewInterface, > { + commandManager: AppCommandManager; + registerView(view: T): void; unregisterView(view: T): void; getView(variantAnalysisId: number): T | undefined; diff --git a/extensions/ql-vscode/src/variant-analysis/variant-analysis-view.ts b/extensions/ql-vscode/src/variant-analysis/variant-analysis-view.ts index e41a85205..ef9a47bc5 100644 --- a/extensions/ql-vscode/src/variant-analysis/variant-analysis-view.ts +++ b/extensions/ql-vscode/src/variant-analysis/variant-analysis-view.ts @@ -145,7 +145,7 @@ export class VariantAnalysisView ); break; case "openLogs": - await commands.executeCommand( + await this.manager.commandManager.execute( "codeQL.openVariantAnalysisLogs", this.variantAnalysisId, ); diff --git a/extensions/ql-vscode/test/__mocks__/appMock.ts b/extensions/ql-vscode/test/__mocks__/appMock.ts index e5f74be29..6a4b05ba8 100644 --- a/extensions/ql-vscode/test/__mocks__/appMock.ts +++ b/extensions/ql-vscode/test/__mocks__/appMock.ts @@ -6,6 +6,8 @@ import { createMockLogger } from "./loggerMock"; import { createMockMemento } from "../mock-memento"; import { testCredentialsWithStub } from "../factories/authentication"; import { Credentials } from "../../src/common/authentication"; +import { AppCommandManager } from "../../src/common/commands"; +import { createMockCommandManager } from "./commandsMock"; export function createMockApp({ extensionPath = "/mock/extension/path", @@ -15,6 +17,7 @@ export function createMockApp({ executeCommand = jest.fn(() => Promise.resolve()), workspaceState = createMockMemento(), credentials = testCredentialsWithStub(), + commands = createMockCommandManager(), }: { extensionPath?: string; workspaceStoragePath?: string; @@ -23,6 +26,7 @@ export function createMockApp({ executeCommand?: () => Promise; workspaceState?: Memento; credentials?: Credentials; + commands?: AppCommandManager; }): App { return { mode: AppMode.Test, @@ -35,6 +39,7 @@ export function createMockApp({ createEventEmitter, executeCommand, credentials, + commands, }; } diff --git a/extensions/ql-vscode/test/__mocks__/commandsMock.ts b/extensions/ql-vscode/test/__mocks__/commandsMock.ts new file mode 100644 index 000000000..16e92606b --- /dev/null +++ b/extensions/ql-vscode/test/__mocks__/commandsMock.ts @@ -0,0 +1,13 @@ +import { AppCommandManager } from "../../src/common/commands"; +import { CommandFunction, CommandManager } from "../../src/packages/commands"; +import { Disposable } from "../../src/packages/commands/Disposable"; + +export function createMockCommandManager({ + registerCommand = jest.fn(), + executeCommand = jest.fn(), +}: { + registerCommand?: (commandName: string, fn: CommandFunction) => Disposable; + executeCommand?: (commandName: string, ...args: any[]) => Promise; +} = {}): AppCommandManager { + return new CommandManager(registerCommand, executeCommand); +} diff --git a/extensions/ql-vscode/test/factories/extension-context.ts b/extensions/ql-vscode/test/factories/extension-context.ts index d871e77d6..8c90b330c 100644 --- a/extensions/ql-vscode/test/factories/extension-context.ts +++ b/extensions/ql-vscode/test/factories/extension-context.ts @@ -19,5 +19,6 @@ export function createMockExtensionContext({ globalStorageUri: vscode.Uri.file(globalStoragePath), storageUri: vscode.Uri.file(workspaceStoragePath), workspaceState: createMockMemento(), + subscriptions: [], } as any as vscode.ExtensionContext; } diff --git a/extensions/ql-vscode/test/unit-tests/packages/commands/CommandManager.test.ts b/extensions/ql-vscode/test/unit-tests/packages/commands/CommandManager.test.ts index 40b78d932..96bede0b6 100644 --- a/extensions/ql-vscode/test/unit-tests/packages/commands/CommandManager.test.ts +++ b/extensions/ql-vscode/test/unit-tests/packages/commands/CommandManager.test.ts @@ -1,8 +1,111 @@ -import { CommandManager } from "../../../../src/packages/commands"; +import { + CommandFunction, + CommandManager, +} from "../../../../src/packages/commands"; -describe(CommandManager.name, () => { - it("can create a command manager", () => { - const commandManager = new CommandManager(); - expect(commandManager).not.toBeUndefined(); +describe("CommandManager", () => { + it("can register a command", () => { + const commandRegister = jest.fn(); + const commandManager = new CommandManager>( + commandRegister, + jest.fn(), + ); + const myCommand = jest.fn(); + commandManager.register("abc", myCommand); + expect(commandRegister).toHaveBeenCalledTimes(1); + expect(commandRegister).toHaveBeenCalledWith("abc", myCommand); + }); + + it("can register typed commands", async () => { + const commands = { + "codeQL.openVariantAnalysisLogs": async (variantAnalysisId: number) => { + return variantAnalysisId * 10; + }, + }; + const commandManager = new CommandManager( + jest.fn(), + jest.fn(), + ); + + // @ts-expect-error wrong command name should give a type error + commandManager.register("abc", jest.fn()); + + commandManager.register( + "codeQL.openVariantAnalysisLogs", + // @ts-expect-error wrong function parameter type should give a type error + async (variantAnalysisId: string): Promise => 10, + ); + + commandManager.register( + "codeQL.openVariantAnalysisLogs", + // @ts-expect-error wrong function return type should give a type error + async (variantAnalysisId: number): Promise => "hello", + ); + + // Working types + commandManager.register( + "codeQL.openVariantAnalysisLogs", + async (variantAnalysisId: number): Promise => + variantAnalysisId * 10, + ); + }); + + it("can dispose of its commands", () => { + const dispose1 = jest.fn(); + const dispose2 = jest.fn(); + const commandRegister = jest + .fn() + .mockReturnValueOnce({ dispose: dispose1 }) + .mockReturnValueOnce({ dispose: dispose2 }); + const commandManager = new CommandManager>( + commandRegister, + jest.fn(), + ); + commandManager.register("abc", jest.fn()); + commandManager.register("def", jest.fn()); + expect(dispose1).not.toHaveBeenCalled(); + expect(dispose2).not.toHaveBeenCalled(); + commandManager.dispose(); + expect(dispose1).toHaveBeenCalledTimes(1); + expect(dispose2).toHaveBeenCalledTimes(1); + }); + + it("can execute a command", async () => { + const commandExecute = jest.fn().mockReturnValue(7); + const commandManager = new CommandManager>( + jest.fn(), + commandExecute, + ); + const result = await commandManager.execute("abc", "hello", true); + expect(result).toEqual(7); + expect(commandExecute).toHaveBeenCalledTimes(1); + expect(commandExecute).toHaveBeenCalledWith("abc", "hello", true); + }); + + it("can execute typed commands", async () => { + const commands = { + "codeQL.openVariantAnalysisLogs": async (variantAnalysisId: number) => { + return variantAnalysisId * 10; + }, + }; + const commandManager = new CommandManager( + jest.fn(), + jest.fn(), + ); + + // @ts-expect-error wrong command name should give a type error + await commandManager.execute("abc", 4); + + await commandManager.execute( + "codeQL.openVariantAnalysisLogs", + // @ts-expect-error wrong argument type should give a type error + "xyz", + ); + + // @ts-expect-error wrong number of arguments should give a type error + await commandManager.execute("codeQL.openVariantAnalysisLogs", 2, 3); + + // Working types + await commandManager.execute("codeQL.openVariantAnalysisLogs", 7); }); });