diff --git a/package-lock.json b/package-lock.json index dd95b904d..26bbb7e3a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "aws-iac-vscode", - "version": "1.0.0", + "version": "1.1.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "aws-iac-vscode", - "version": "1.0.0", + "version": "1.1.0", "license": "Apache-2.0", "dependencies": { "@base2/pretty-print-object": "1.0.2", diff --git a/package.json b/package.json index 2706e09a6..c1a576745 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "aws-iac-vscode", "displayName": "AWS CloudFormation", - "version": "1.0.0", + "version": "1.1.0", "description": "AWS CloudFormation language support: validation, completion, hover documentation, and cfn-lint diagnostics for JSON and YAML templates.", "publisher": "amazonwebservices", "author": "Amazon Web Services (AWS)", @@ -16,6 +16,7 @@ }, "qna": "https://github.com/aws-cloudformation/cloudformation-vscode/issues", "pricing": "Free", + "icon": "resources/marketplace/aws-cloudformation-icon.png", "galleryBanner": { "color": "#232F3E", "theme": "dark" diff --git a/resources/marketplace/aws-cloudformation-icon.png b/resources/marketplace/aws-cloudformation-icon.png new file mode 100644 index 000000000..f4210093b Binary files /dev/null and b/resources/marketplace/aws-cloudformation-icon.png differ diff --git a/src/extension.ts b/src/extension.ts index 2b504d2f6..5b4ccc30e 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -4,6 +4,7 @@ import { AwsCredentialsService } from './auth/AwsCredentials'; import { restartCommand, RestartServerCommand, updateRegion } from './commands/Commands'; import { CfnInlineCompletionProvider } from './inlineCompletion/InlineCompletionProvider'; import { CfnDevLspServerProvider } from './lsp-server/CfnDevLspServerProvider'; +import { CfnLanguageClient } from './lsp-server/CfnLanguageClient'; import { CfnRemoteLspServerProvider } from './lsp-server/CfnRemoteLspServerProvider'; import { CfnDocumentSelector, cfnInitializationOptions, cfnServerOptions } from './lsp-server/LspClientConfig'; import { LspLauncher, LspResolver } from './lsp-server/LspLauncher'; @@ -129,7 +130,7 @@ async function initialize(context: ExtensionContext) { errorHandler, }; - return new LanguageClient(ExtensionId, ExtensionName, cfnServerOptions(serverFile), clientOptions); + return new CfnLanguageClient(ExtensionId, ExtensionName, cfnServerOptions(serverFile), clientOptions); }; const launcher = new LspLauncher(resolver, clientFactory, { diff --git a/src/lsp-server/CfnLanguageClient.ts b/src/lsp-server/CfnLanguageClient.ts new file mode 100644 index 000000000..8e80b2658 --- /dev/null +++ b/src/lsp-server/CfnLanguageClient.ts @@ -0,0 +1,30 @@ +import { ExecuteCommandRequest } from 'vscode-languageclient'; +import { + DynamicFeature, + LanguageClient, + LanguageClientOptions, + ServerOptions, + StaticFeature, +} from 'vscode-languageclient/node'; +import { advertisedCommands } from './LspCommands'; +import { suffixCodeActionCommands, SuffixedExecuteCommandFeature } from './SuffixedExecuteCommandFeature'; + +/** A `LanguageClient` that registers the server's command ids with VS Code under this client's suffix. */ +export class CfnLanguageClient extends LanguageClient { + constructor(id: string, name: string, serverOptions: ServerOptions, clientOptions: LanguageClientOptions) { + super(id, name, serverOptions, clientOptions); + this.middleware.provideCodeActions = async (document, range, context, token, next) => { + const actions = await next(document, range, context, token); + if (actions) { + suffixCodeActionCommands(actions, advertisedCommands(this)); + } + return actions; + }; + } + + override registerFeature(feature: StaticFeature | DynamicFeature): void { + const replaced = + 'registrationType' in feature && feature.registrationType.method === ExecuteCommandRequest.method; + super.registerFeature(replaced ? new SuffixedExecuteCommandFeature(this) : feature); + } +} diff --git a/src/lsp-server/LspCommands.ts b/src/lsp-server/LspCommands.ts index 076bb5afe..ad494bc05 100644 --- a/src/lsp-server/LspCommands.ts +++ b/src/lsp-server/LspCommands.ts @@ -3,16 +3,28 @@ import { CfnLspClientName } from '../utils/ExtensionConfig'; export const UpdateRegionCommand = '/command/region/update'; +const CommandSuffix = `.${CfnLspClientName}`; + +/** The ids the running server advertised through `executeCommandProvider`. */ +export function advertisedCommands(client: LanguageClient): string[] { + return client.initializeResult?.capabilities.executeCommandProvider?.commands ?? []; +} + /** * Returns the id under which the running server registered `baseCommand`: suffixed when the server honours * `aws.commandSuffix`, unsuffixed when it predates the option. */ export function advertisedCommand(client: LanguageClient, baseCommand: string): string { - const advertised = client.initializeResult?.capabilities.executeCommandProvider?.commands ?? []; - const suffixed = `${baseCommand}.${CfnLspClientName}`; + const advertised = advertisedCommands(client); + const suffixed = suffixedCommand(baseCommand); const command = [suffixed, baseCommand].find((candidate) => advertised.includes(candidate)); if (!command) { throw new Error(`Language server does not advertise ${suffixed} (advertised: ${advertised.join(', ')})`); } return command; } + +/** The id this client registers `command` with VS Code under, whether or not the server already suffixed it. */ +export function suffixedCommand(command: string): string { + return command.endsWith(CommandSuffix) ? command : `${command}${CommandSuffix}`; +} diff --git a/src/lsp-server/SuffixedExecuteCommandFeature.ts b/src/lsp-server/SuffixedExecuteCommandFeature.ts new file mode 100644 index 000000000..bed9a678a --- /dev/null +++ b/src/lsp-server/SuffixedExecuteCommandFeature.ts @@ -0,0 +1,97 @@ +import { randomUUID } from 'crypto'; +import { CodeAction, Command, commands, Disposable } from 'vscode'; +import { ExecuteCommandRequest } from 'vscode-languageclient'; +import { + ClientCapabilities, + DynamicFeature, + ExecuteCommandRegistrationOptions, + FeatureState, + LanguageClient, + RegistrationData, + ServerCapabilities, +} from 'vscode-languageclient/node'; +import { suffixedCommand } from './LspCommands'; + +type CommandClient = Pick; + +/** + * Replaces the client library's execute-command feature. The server's command ids are registered with VS Code under + * this client's suffix even when the server predates `aws.commandSuffix` and advertises unsuffixed ids, so another + * extension hosting the same server in the window cannot collide with ours. Executions are forwarded to the server + * under the id it advertised. + */ +export class SuffixedExecuteCommandFeature implements DynamicFeature { + private readonly registrations = new Map(); + + constructor(private readonly client: CommandClient) {} + + get registrationType() { + return ExecuteCommandRequest.type; + } + + getState(): FeatureState { + return { kind: 'workspace', id: ExecuteCommandRequest.method, registrations: this.registrations.size > 0 }; + } + + fillClientCapabilities(capabilities: ClientCapabilities): void { + (capabilities.workspace ??= {}).executeCommand = { dynamicRegistration: true }; + } + + initialize(capabilities: ServerCapabilities): void { + if (capabilities.executeCommandProvider) { + this.register({ id: randomUUID(), registerOptions: capabilities.executeCommandProvider }); + } + } + + register(data: RegistrationData): void { + this.registrations.set( + data.id, + data.registerOptions.commands.map((command) => + commands.registerCommand( + suffixedCommand(command), + async (...args: unknown[]) => await this.execute(command, args), + ), + ), + ); + } + + unregister(id: string): void { + for (const registration of this.registrations.get(id) ?? []) { + registration.dispose(); + } + this.registrations.delete(id); + } + + clear(): void { + for (const id of this.registrations.keys()) { + this.unregister(id); + } + } + + private async execute(command: string, args: unknown[]): Promise { + try { + return await this.client.sendRequest(ExecuteCommandRequest.method, { command, arguments: args }); + } catch (error) { + // `defaultValue` is a required parameter: the client returns it for errors it swallows and rethrows the rest. + // eslint-disable-next-line unicorn/no-useless-undefined + return this.client.handleFailedRequest(ExecuteCommandRequest.type, undefined, error, undefined); + } + } +} + +/** + * Points the commands of server-provided code actions at the ids {@link SuffixedExecuteCommandFeature} registered. + * Commands the server did not advertise, such as ones contributed by the extension itself, are left alone. + */ +export function suffixCodeActionCommands(actions: (Command | CodeAction)[], advertised: readonly string[]): void { + for (const action of actions) { + const command = isCommand(action) ? action : action.command; + if (command && advertised.includes(command.command)) { + command.command = suffixedCommand(command.command); + } + } +} + +function isCommand(action: Command | CodeAction): action is Command { + return typeof action.command === 'string'; +} diff --git a/tst/lsp-server/LspCommands.test.ts b/tst/lsp-server/LspCommands.test.ts index 5f0c0d3aa..bcf60619b 100644 --- a/tst/lsp-server/LspCommands.test.ts +++ b/tst/lsp-server/LspCommands.test.ts @@ -1,6 +1,11 @@ import { describe, it, expect } from 'vitest'; import { LanguageClient } from 'vscode-languageclient/node'; -import { advertisedCommand, UpdateRegionCommand } from '../../src/lsp-server/LspCommands'; +import { + advertisedCommand, + advertisedCommands, + suffixedCommand, + UpdateRegionCommand, +} from '../../src/lsp-server/LspCommands'; function clientAdvertising(commands: string[] | undefined): LanguageClient { return { @@ -46,3 +51,28 @@ describe('advertisedCommand', () => { expect(() => advertisedCommand(client, UpdateRegionCommand)).toThrow('does not advertise'); }); }); + +describe('suffixedCommand', () => { + it('appends the client suffix to an unsuffixed id', () => { + expect(suffixedCommand(UpdateRegionCommand)).toBe('/command/region/update.iac-vscode'); + }); + + it('keeps an id the server already suffixed for this client', () => { + expect(suffixedCommand('/command/region/update.iac-vscode')).toBe('/command/region/update.iac-vscode'); + }); + + it('suffixes an id carrying another client suffix rather than replacing it', () => { + expect(suffixedCommand('/command/region/update.other')).toBe('/command/region/update.other.iac-vscode'); + }); +}); + +describe('advertisedCommands', () => { + it('returns the ids the server advertised', () => { + expect(advertisedCommands(clientAdvertising(['/command/region/update']))).toEqual(['/command/region/update']); + }); + + it('returns no ids before initialization or when the server provides no commands', () => { + expect(advertisedCommands({} as unknown as LanguageClient)).toEqual([]); + expect(advertisedCommands(clientAdvertising(undefined))).toEqual([]); + }); +}); diff --git a/tst/lsp-server/SuffixedExecuteCommandFeature.test.ts b/tst/lsp-server/SuffixedExecuteCommandFeature.test.ts new file mode 100644 index 000000000..39519f0f5 --- /dev/null +++ b/tst/lsp-server/SuffixedExecuteCommandFeature.test.ts @@ -0,0 +1,155 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { CodeAction, commands } from 'vscode'; +import { ExecuteCommandRequest } from 'vscode-languageclient'; +import { LanguageClient } from 'vscode-languageclient/node'; +import { + suffixCodeActionCommands, + SuffixedExecuteCommandFeature, +} from '../../src/lsp-server/SuffixedExecuteCommandFeature'; + +vi.mock('vscode'); +vi.mock('vscode-languageclient/node'); +vi.mock('vscode-languageclient'); + +const ClearDiagnostic = '/command/template/clear-diagnostic'; +const UpdateRegion = '/command/region/update'; +const SuffixedClearDiagnostic = `${ClearDiagnostic}.iac-vscode`; +const SuffixedUpdateRegion = `${UpdateRegion}.iac-vscode`; + +type CommandHandler = (...args: unknown[]) => Promise; + +function registeredCommands(): Map { + return new Map( + vi.mocked(commands.registerCommand).mock.calls.map(([id, handler]) => [id, handler as CommandHandler]), + ); +} + +describe('SuffixedExecuteCommandFeature', () => { + let client: { sendRequest: ReturnType; handleFailedRequest: ReturnType }; + let feature: SuffixedExecuteCommandFeature; + + beforeEach(() => { + vi.mocked(commands.registerCommand) + .mockReset() + .mockImplementation(() => ({ dispose: vi.fn() })); + client = { + sendRequest: vi.fn().mockResolvedValue('result'), + handleFailedRequest: vi.fn(), + }; + feature = new SuffixedExecuteCommandFeature(client as unknown as LanguageClient); + }); + + it('registers for workspace/executeCommand and asks for dynamic registration', () => { + const capabilities: { workspace?: { executeCommand?: { dynamicRegistration?: boolean } } } = {}; + + feature.fillClientCapabilities(capabilities); + + expect(feature.registrationType).toBe(ExecuteCommandRequest.type); + expect(capabilities.workspace?.executeCommand).toEqual({ dynamicRegistration: true }); + expect(feature.getState()).toEqual({ kind: 'workspace', id: 'workspace/executeCommand', registrations: false }); + }); + + it('registers unsuffixed ids from a server that predates the command suffix under the suffixed id', () => { + feature.initialize({ executeCommandProvider: { commands: [ClearDiagnostic, UpdateRegion] } }); + + expect([...registeredCommands().keys()]).toEqual([SuffixedClearDiagnostic, SuffixedUpdateRegion]); + expect(feature.getState()).toMatchObject({ registrations: true }); + }); + + it('registers ids a server that honours the command suffix advertised as they are', () => { + feature.initialize({ executeCommandProvider: { commands: [SuffixedClearDiagnostic, SuffixedUpdateRegion] } }); + + expect([...registeredCommands().keys()]).toEqual([SuffixedClearDiagnostic, SuffixedUpdateRegion]); + }); + + it('registers nothing when the server provides no commands', () => { + feature.initialize({}); + + expect(commands.registerCommand).not.toHaveBeenCalled(); + expect(feature.getState()).toMatchObject({ registrations: false }); + }); + + it('forwards an execution to the server under the id the server advertised', async () => { + feature.initialize({ executeCommandProvider: { commands: [ClearDiagnostic] } }); + + const result = await registeredCommands().get(SuffixedClearDiagnostic)?.('file:///t.yaml', { id: 1 }); + + expect(result).toBe('result'); + expect(client.sendRequest).toHaveBeenCalledWith('workspace/executeCommand', { + command: ClearDiagnostic, + arguments: ['file:///t.yaml', { id: 1 }], + }); + }); + + it('hands a failed execution to the client', async () => { + const error = new Error('boom'); + client.sendRequest.mockRejectedValue(error); + client.handleFailedRequest.mockReturnValue('fallback'); + feature.initialize({ executeCommandProvider: { commands: [ClearDiagnostic] } }); + + const result = await registeredCommands().get(SuffixedClearDiagnostic)?.(); + + expect(result).toBe('fallback'); + expect(client.handleFailedRequest).toHaveBeenCalledWith( + ExecuteCommandRequest.type, + undefined, + error, + undefined, + ); + }); + + it('disposes the registrations of an id on unregister and all of them on clear', () => { + feature.register({ id: 'first', registerOptions: { commands: [ClearDiagnostic] } }); + feature.register({ id: 'second', registerOptions: { commands: [UpdateRegion] } }); + const [first, second] = vi.mocked(commands.registerCommand).mock.results.map((result) => result.value); + + feature.unregister('first'); + + expect(first.dispose).toHaveBeenCalledTimes(1); + expect(second.dispose).not.toHaveBeenCalled(); + expect(feature.getState()).toMatchObject({ registrations: true }); + + feature.clear(); + + expect(second.dispose).toHaveBeenCalledTimes(1); + expect(feature.getState()).toMatchObject({ registrations: false }); + }); +}); + +describe('suffixCodeActionCommands', () => { + const advertised = [ClearDiagnostic, UpdateRegion]; + + it('points code actions at the suffixed id of an advertised command', () => { + const action = { title: 'Remove', command: { title: 'Remove', command: ClearDiagnostic, arguments: ['u'] } }; + + suffixCodeActionCommands([action], advertised); + + expect(action.command).toEqual({ title: 'Remove', command: SuffixedClearDiagnostic, arguments: ['u'] }); + }); + + it('rewrites a bare command item', () => { + const command = { title: 'Update', command: UpdateRegion }; + + suffixCodeActionCommands([command], advertised); + + expect(command.command).toBe(SuffixedUpdateRegion); + }); + + it('leaves commands the server did not advertise and actions without a command alone', () => { + const own = { title: 'Extract', command: { title: 'Extract', command: 'aws.cloudformation.extract' } }; + const editOnly = { title: 'Fix', edit: {} }; + + suffixCodeActionCommands([own, editOnly as CodeAction], advertised); + + expect(own.command.command).toBe('aws.cloudformation.extract'); + expect(editOnly).toEqual({ title: 'Fix', edit: {} }); + }); + + it('is a no-op for a server that already suffixes its commands', () => { + const action = { title: 'Remove', command: { title: 'Remove', command: SuffixedClearDiagnostic } }; + + suffixCodeActionCommands([action], [SuffixedClearDiagnostic, SuffixedUpdateRegion]); + + expect(action.command.command).toBe(SuffixedClearDiagnostic); + }); +});