From 9813ffdf82718c2fec28d6a58f3063a2cd06e71a Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Wed, 16 Sep 2020 11:39:58 +0200 Subject: [PATCH 1/5] Add resolveCodeAction to internal API, introduce CodeActionItem which knows a CodeAction and the provider, adopt CodeActitemItem in codeAction-land --- src/vs/editor/common/modes.ts | 5 +++ .../editor/contrib/codeAction/codeAction.ts | 45 ++++++++++++++----- .../contrib/codeAction/codeActionCommands.ts | 25 ++++++----- .../contrib/codeAction/codeActionMenu.ts | 14 +++--- .../editor/contrib/codeAction/codeActionUi.ts | 16 +++---- .../codeAction/test/codeAction.test.ts | 31 +++++++------ .../codeEditor/browser/saveParticipants.ts | 2 +- .../markers/browser/markersTreeViewer.ts | 8 ++-- .../api/extHostLanguageFeatures.test.ts | 16 +++---- 9 files changed, 96 insertions(+), 66 deletions(-) diff --git a/src/vs/editor/common/modes.ts b/src/vs/editor/common/modes.ts index 3d34f627b1cf0..492bab678970f 100644 --- a/src/vs/editor/common/modes.ts +++ b/src/vs/editor/common/modes.ts @@ -654,6 +654,11 @@ export interface CodeActionProvider { */ provideCodeActions(model: model.ITextModel, range: Range | Selection, context: CodeActionContext, token: CancellationToken): ProviderResult; + /** + * Given a code action fill in the edit or command. Will only invoked when missing. + */ + resolveCodeAction?(codeAction: CodeAction, token: CancellationToken): ProviderResult; + /** * Optional list of CodeActionKinds that this provider returns. */ diff --git a/src/vs/editor/contrib/codeAction/codeAction.ts b/src/vs/editor/contrib/codeAction/codeAction.ts index f54e722566e26..24c60641ee9cb 100644 --- a/src/vs/editor/contrib/codeAction/codeAction.ts +++ b/src/vs/editor/contrib/codeAction/codeAction.ts @@ -24,9 +24,29 @@ export const sourceActionCommandId = 'editor.action.sourceAction'; export const organizeImportsCommandId = 'editor.action.organizeImports'; export const fixAllCommandId = 'editor.action.fixAll'; +export class CodeActionItem { + + constructor( + readonly action: modes.CodeAction, + readonly provider: modes.CodeActionProvider | undefined, + ) { } + + async resolve(token: CancellationToken): Promise { + // TODO@jrieken when is an item resolved already? + if (this.provider?.resolveCodeAction && !this.action.edit && !this.action.command) { + try { + this.provider.resolveCodeAction(this.action, token); + } catch (err) { + onUnexpectedExternalError(err); + } + } + return this; + } +} + export interface CodeActionSet extends IDisposable { - readonly validActions: readonly modes.CodeAction[]; - readonly allActions: readonly modes.CodeAction[]; + readonly validActions: readonly CodeActionItem[]; + readonly allActions: readonly CodeActionItem[]; readonly hasAutoFix: boolean; readonly documentation: readonly modes.Command[]; @@ -34,7 +54,7 @@ export interface CodeActionSet extends IDisposable { class ManagedCodeActionSet extends Disposable implements CodeActionSet { - private static codeActionsComparator(a: modes.CodeAction, b: modes.CodeAction): number { + private static codeActionsComparator({ action: a }: CodeActionItem, { action: b }: CodeActionItem): number { if (a.isPreferred && !b.isPreferred) { return -1; } else if (!a.isPreferred && b.isPreferred) { @@ -54,27 +74,27 @@ class ManagedCodeActionSet extends Disposable implements CodeActionSet { } } - public readonly validActions: readonly modes.CodeAction[]; - public readonly allActions: readonly modes.CodeAction[]; + public readonly validActions: readonly CodeActionItem[]; + public readonly allActions: readonly CodeActionItem[]; public constructor( - actions: readonly modes.CodeAction[], + actions: readonly CodeActionItem[], public readonly documentation: readonly modes.Command[], disposables: DisposableStore, ) { super(); this._register(disposables); this.allActions = mergeSort([...actions], ManagedCodeActionSet.codeActionsComparator); - this.validActions = this.allActions.filter(action => !action.disabled); + this.validActions = this.allActions.filter(({ action }) => !action.disabled); } public get hasAutoFix() { - return this.validActions.some(fix => !!fix.kind && CodeActionKind.QuickFix.contains(new CodeActionKind(fix.kind)) && !!fix.isPreferred); + return this.validActions.some(({ action: fix }) => !!fix.kind && CodeActionKind.QuickFix.contains(new CodeActionKind(fix.kind)) && !!fix.isPreferred); } } -const emptyCodeActionsResponse = { actions: [] as modes.CodeAction[], documentation: undefined }; +const emptyCodeActionsResponse = { actions: [] as CodeActionItem[], documentation: undefined }; export function getCodeActions( model: ITextModel, @@ -108,7 +128,10 @@ export function getCodeActions( const filteredActions = (providedCodeActions?.actions || []).filter(action => action && filtersAction(filter, action)); const documentation = getDocumentation(provider, filteredActions, filter.include); - return { actions: filteredActions, documentation }; + return { + actions: filteredActions.map(action => new CodeActionItem(action, provider)), + documentation + }; } catch (err) { if (isPromiseCanceledError(err)) { throw err; @@ -226,5 +249,5 @@ registerLanguageCommand('_executeCodeActionProvider', async function (accessor, CancellationToken.None); setTimeout(() => codeActionSet.dispose(), 100); - return codeActionSet.validActions; + return codeActionSet.validActions.map(item => item.action); }); diff --git a/src/vs/editor/contrib/codeAction/codeActionCommands.ts b/src/vs/editor/contrib/codeAction/codeActionCommands.ts index 699e05a469904..65c19a781f620 100644 --- a/src/vs/editor/contrib/codeAction/codeActionCommands.ts +++ b/src/vs/editor/contrib/codeAction/codeActionCommands.ts @@ -4,6 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { IAnchor } from 'vs/base/browser/ui/contextview/contextview'; +import { CancellationToken } from 'vs/base/common/cancellation'; import { IJSONSchema } from 'vs/base/common/jsonSchema'; import { KeyCode, KeyMod } from 'vs/base/common/keyCodes'; import { Lazy } from 'vs/base/common/lazy'; @@ -15,8 +16,8 @@ import { IBulkEditService, ResourceEdit } from 'vs/editor/browser/services/bulkE import { IPosition } from 'vs/editor/common/core/position'; import { IEditorContribution } from 'vs/editor/common/editorCommon'; import { EditorContextKeys } from 'vs/editor/common/editorContextKeys'; -import { CodeAction, CodeActionTriggerType } from 'vs/editor/common/modes'; -import { codeActionCommandId, CodeActionSet, fixAllCommandId, organizeImportsCommandId, refactorCommandId, sourceActionCommandId } from 'vs/editor/contrib/codeAction/codeAction'; +import { CodeActionTriggerType } from 'vs/editor/common/modes'; +import { codeActionCommandId, CodeActionItem, CodeActionSet, fixAllCommandId, organizeImportsCommandId, refactorCommandId, sourceActionCommandId } from 'vs/editor/contrib/codeAction/codeAction'; import { CodeActionUi } from 'vs/editor/contrib/codeAction/codeActionUi'; import { MessageController } from 'vs/editor/contrib/message/messageController'; import * as nls from 'vs/nls'; @@ -130,14 +131,14 @@ export class QuickFixController extends Disposable implements IEditorContributio return this._model.trigger(trigger); } - private _applyCodeAction(action: CodeAction): Promise { + private _applyCodeAction(action: CodeActionItem): Promise { return this._instantiationService.invokeFunction(applyCodeAction, action, this._editor); } } export async function applyCodeAction( accessor: ServicesAccessor, - action: CodeAction, + item: CodeActionItem, editor?: ICodeEditor, ): Promise { const bulkEditService = accessor.get(IBulkEditService); @@ -157,18 +158,20 @@ export async function applyCodeAction( }; telemetryService.publicLog2('codeAction.applyCodeAction', { - codeActionTitle: action.title, - codeActionKind: action.kind, - codeActionIsPreferred: !!action.isPreferred, + codeActionTitle: item.action.title, + codeActionKind: item.action.kind, + codeActionIsPreferred: !!item.action.isPreferred, }); - if (action.edit) { - await bulkEditService.apply(ResourceEdit.convert(action.edit), { editor, label: action.title }); + await item.resolve(CancellationToken.None); + + if (item.action.edit) { + await bulkEditService.apply(ResourceEdit.convert(item.action.edit), { editor, label: item.action.title }); } - if (action.command) { + if (item.action.command) { try { - await commandService.executeCommand(action.command.id, ...(action.command.arguments || [])); + await commandService.executeCommand(item.action.command.id, ...(item.action.command.arguments || [])); } catch (err) { const message = asMessage(err); notificationService.error( diff --git a/src/vs/editor/contrib/codeAction/codeActionMenu.ts b/src/vs/editor/contrib/codeAction/codeActionMenu.ts index 0c9ef9ec71aa9..186eacd2e34aa 100644 --- a/src/vs/editor/contrib/codeAction/codeActionMenu.ts +++ b/src/vs/editor/contrib/codeAction/codeActionMenu.ts @@ -14,14 +14,14 @@ import { ICodeEditor } from 'vs/editor/browser/editorBrowser'; import { IPosition, Position } from 'vs/editor/common/core/position'; import { ScrollType } from 'vs/editor/common/editorCommon'; import { CodeAction, CodeActionProviderRegistry, Command } from 'vs/editor/common/modes'; -import { codeActionCommandId, CodeActionSet, fixAllCommandId, organizeImportsCommandId, refactorCommandId, sourceActionCommandId } from 'vs/editor/contrib/codeAction/codeAction'; +import { codeActionCommandId, CodeActionItem, CodeActionSet, fixAllCommandId, organizeImportsCommandId, refactorCommandId, sourceActionCommandId } from 'vs/editor/contrib/codeAction/codeAction'; import { CodeActionAutoApply, CodeActionCommandArgs, CodeActionTrigger, CodeActionKind } from 'vs/editor/contrib/codeAction/types'; import { IContextMenuService } from 'vs/platform/contextview/browser/contextView'; import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding'; import { ResolvedKeybindingItem } from 'vs/platform/keybinding/common/resolvedKeybindingItem'; interface CodeActionWidgetDelegate { - onSelectCodeAction: (action: CodeAction) => Promise; + onSelectCodeAction: (action: CodeActionItem) => Promise; } interface ResolveCodeActionKeybinding { @@ -103,10 +103,10 @@ export class CodeActionMenu extends Disposable { private getMenuActions( trigger: CodeActionTrigger, - actionsToShow: readonly CodeAction[], + actionsToShow: readonly CodeActionItem[], documentation: readonly Command[] ): IAction[] { - const toCodeActionAction = (action: CodeAction): CodeActionAction => new CodeActionAction(action, () => this._delegate.onSelectCodeAction(action)); + const toCodeActionAction = (item: CodeActionItem): CodeActionAction => new CodeActionAction(item.action, () => this._delegate.onSelectCodeAction(item)); const result: IAction[] = actionsToShow .map(toCodeActionAction); @@ -117,16 +117,16 @@ export class CodeActionMenu extends Disposable { if (model && result.length) { for (const provider of CodeActionProviderRegistry.all(model)) { if (provider._getAdditionalMenuItems) { - allDocumentation.push(...provider._getAdditionalMenuItems({ trigger: trigger.type, only: trigger.filter?.include?.value }, actionsToShow)); + allDocumentation.push(...provider._getAdditionalMenuItems({ trigger: trigger.type, only: trigger.filter?.include?.value }, actionsToShow.map(item => item.action))); } } } if (allDocumentation.length) { - result.push(new Separator(), ...allDocumentation.map(command => toCodeActionAction({ + result.push(new Separator(), ...allDocumentation.map(command => toCodeActionAction(new CodeActionItem({ title: command.title, command: command, - }))); + }, undefined)))); } return result; diff --git a/src/vs/editor/contrib/codeAction/codeActionUi.ts b/src/vs/editor/contrib/codeAction/codeActionUi.ts index 6419e5e42f608..7a645a7bd0ba3 100644 --- a/src/vs/editor/contrib/codeAction/codeActionUi.ts +++ b/src/vs/editor/contrib/codeAction/codeActionUi.ts @@ -9,8 +9,8 @@ import { Lazy } from 'vs/base/common/lazy'; import { Disposable, MutableDisposable } from 'vs/base/common/lifecycle'; import { ICodeEditor } from 'vs/editor/browser/editorBrowser'; import { IPosition } from 'vs/editor/common/core/position'; -import { CodeAction, CodeActionTriggerType } from 'vs/editor/common/modes'; -import { CodeActionSet } from 'vs/editor/contrib/codeAction/codeAction'; +import { CodeActionTriggerType } from 'vs/editor/common/modes'; +import { CodeActionItem, CodeActionSet } from 'vs/editor/contrib/codeAction/codeAction'; import { MessageController } from 'vs/editor/contrib/message/messageController'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import { CodeActionMenu, CodeActionShowOptions } from './codeActionMenu'; @@ -29,7 +29,7 @@ export class CodeActionUi extends Disposable { quickFixActionId: string, preferredFixActionId: string, private readonly delegate: { - applyCodeAction: (action: CodeAction, regtriggerAfterApply: boolean) => Promise + applyCodeAction: (action: CodeActionItem, regtriggerAfterApply: boolean) => Promise }, @IInstantiationService instantiationService: IInstantiationService, ) { @@ -83,8 +83,8 @@ export class CodeActionUi extends Disposable { // Check to see if there is an action that we would have applied were it not invalid if (newState.trigger.context) { const invalidAction = this.getInvalidActionThatWouldHaveBeenApplied(newState.trigger, actions); - if (invalidAction && invalidAction.disabled) { - MessageController.get(this._editor).showMessage(invalidAction.disabled, newState.trigger.context.position); + if (invalidAction && invalidAction.action.disabled) { + MessageController.get(this._editor).showMessage(invalidAction.action.disabled, newState.trigger.context.position); actions.dispose(); return; } @@ -114,7 +114,7 @@ export class CodeActionUi extends Disposable { } } - private getInvalidActionThatWouldHaveBeenApplied(trigger: CodeActionTrigger, actions: CodeActionSet): CodeAction | undefined { + private getInvalidActionThatWouldHaveBeenApplied(trigger: CodeActionTrigger, actions: CodeActionSet): CodeActionItem | undefined { if (!actions.allActions.length) { return undefined; } @@ -122,13 +122,13 @@ export class CodeActionUi extends Disposable { if ((trigger.autoApply === CodeActionAutoApply.First && actions.validActions.length === 0) || (trigger.autoApply === CodeActionAutoApply.IfSingle && actions.allActions.length === 1) ) { - return actions.allActions.find(action => action.disabled); + return actions.allActions.find(({ action }) => action.disabled); } return undefined; } - private tryGetValidActionToApply(trigger: CodeActionTrigger, actions: CodeActionSet): CodeAction | undefined { + private tryGetValidActionToApply(trigger: CodeActionTrigger, actions: CodeActionSet): CodeActionItem | undefined { if (!actions.validActions.length) { return undefined; } diff --git a/src/vs/editor/contrib/codeAction/test/codeAction.test.ts b/src/vs/editor/contrib/codeAction/test/codeAction.test.ts index 85510ab2a01c3..53a80a511083a 100644 --- a/src/vs/editor/contrib/codeAction/test/codeAction.test.ts +++ b/src/vs/editor/contrib/codeAction/test/codeAction.test.ts @@ -8,7 +8,7 @@ import { URI } from 'vs/base/common/uri'; import { Range } from 'vs/editor/common/core/range'; import { TextModel } from 'vs/editor/common/model/textModel'; import * as modes from 'vs/editor/common/modes'; -import { getCodeActions } from 'vs/editor/contrib/codeAction/codeAction'; +import { CodeActionItem, getCodeActions } from 'vs/editor/contrib/codeAction/codeAction'; import { CodeActionKind } from 'vs/editor/contrib/codeAction/types'; import { IMarkerData, MarkerSeverity } from 'vs/platform/markers/common/markers'; import { CancellationToken } from 'vs/base/common/cancellation'; @@ -117,14 +117,14 @@ suite('CodeAction', () => { const expected = [ // CodeActions with a diagnostics array are shown first ordered by diagnostics.message - testData.diagnostics.abc, - testData.diagnostics.bcd, + new CodeActionItem(testData.diagnostics.abc, provider), + new CodeActionItem(testData.diagnostics.bcd, provider), // CodeActions without diagnostics are shown in the given order without any further sorting - testData.command.abc, - testData.spelling.bcd, // empty diagnostics array - testData.tsLint.bcd, - testData.tsLint.abc + new CodeActionItem(testData.command.abc, provider), + new CodeActionItem(testData.spelling.bcd, provider), // empty diagnostics array + new CodeActionItem(testData.tsLint.bcd, provider), + new CodeActionItem(testData.tsLint.abc, provider) ]; const { validActions: actions } = await getCodeActions(model, new Range(1, 1, 2, 1), { type: modes.CodeActionTriggerType.Manual }, Progress.None, CancellationToken.None); @@ -144,14 +144,14 @@ suite('CodeAction', () => { { const { validActions: actions } = await getCodeActions(model, new Range(1, 1, 2, 1), { type: modes.CodeActionTriggerType.Auto, filter: { include: new CodeActionKind('a') } }, Progress.None, CancellationToken.None); assert.equal(actions.length, 2); - assert.strictEqual(actions[0].title, 'a'); - assert.strictEqual(actions[1].title, 'a.b'); + assert.strictEqual(actions[0].action.title, 'a'); + assert.strictEqual(actions[1].action.title, 'a.b'); } { const { validActions: actions } = await getCodeActions(model, new Range(1, 1, 2, 1), { type: modes.CodeActionTriggerType.Auto, filter: { include: new CodeActionKind('a.b') } }, Progress.None, CancellationToken.None); assert.equal(actions.length, 1); - assert.strictEqual(actions[0].title, 'a.b'); + assert.strictEqual(actions[0].action.title, 'a.b'); } { @@ -176,7 +176,7 @@ suite('CodeAction', () => { const { validActions: actions } = await getCodeActions(model, new Range(1, 1, 2, 1), { type: modes.CodeActionTriggerType.Auto, filter: { include: new CodeActionKind('a') } }, Progress.None, CancellationToken.None); assert.equal(actions.length, 1); - assert.strictEqual(actions[0].title, 'a'); + assert.strictEqual(actions[0].action.title, 'a'); }); test('getCodeActions should not return source code action by default', async function () { @@ -190,13 +190,13 @@ suite('CodeAction', () => { { const { validActions: actions } = await getCodeActions(model, new Range(1, 1, 2, 1), { type: modes.CodeActionTriggerType.Auto }, Progress.None, CancellationToken.None); assert.equal(actions.length, 1); - assert.strictEqual(actions[0].title, 'b'); + assert.strictEqual(actions[0].action.title, 'b'); } { const { validActions: actions } = await getCodeActions(model, new Range(1, 1, 2, 1), { type: modes.CodeActionTriggerType.Auto, filter: { include: CodeActionKind.Source, includeSourceActions: true } }, Progress.None, CancellationToken.None); assert.equal(actions.length, 1); - assert.strictEqual(actions[0].title, 'a'); + assert.strictEqual(actions[0].action.title, 'a'); } }); @@ -218,7 +218,7 @@ suite('CodeAction', () => { } }, Progress.None, CancellationToken.None); assert.equal(actions.length, 1); - assert.strictEqual(actions[0].title, 'b'); + assert.strictEqual(actions[0].action.title, 'b'); } }); @@ -255,7 +255,7 @@ suite('CodeAction', () => { }, Progress.None, CancellationToken.None); assert.strictEqual(didInvoke, false); assert.equal(actions.length, 1); - assert.strictEqual(actions[0].title, 'a'); + assert.strictEqual(actions[0].action.title, 'a'); } }); @@ -282,4 +282,3 @@ suite('CodeAction', () => { assert.strictEqual(wasInvoked, false); }); }); - diff --git a/src/vs/workbench/contrib/codeEditor/browser/saveParticipants.ts b/src/vs/workbench/contrib/codeEditor/browser/saveParticipants.ts index 83a504c0135c8..698828c06d76e 100644 --- a/src/vs/workbench/contrib/codeEditor/browser/saveParticipants.ts +++ b/src/vs/workbench/contrib/codeEditor/browser/saveParticipants.ts @@ -344,7 +344,7 @@ class CodeActionOnSaveParticipant implements ITextFileSaveParticipant { const actionsToRun = await this.getActionsToRun(model, codeActionKind, excludes, getActionProgress, token); try { for (const action of actionsToRun.validActions) { - progress.report({ message: localize('codeAction.apply', "Applying code action '{0}'.", action.title) }); + progress.report({ message: localize('codeAction.apply', "Applying code action '{0}'.", action.action.title) }); await this.instantiationService.invokeFunction(applyCodeAction, action); } } catch { diff --git a/src/vs/workbench/contrib/markers/browser/markersTreeViewer.ts b/src/vs/workbench/contrib/markers/browser/markersTreeViewer.ts index c1f4dd65b2f35..67f945f0b0d00 100644 --- a/src/vs/workbench/contrib/markers/browser/markersTreeViewer.ts +++ b/src/vs/workbench/contrib/markers/browser/markersTreeViewer.ts @@ -691,14 +691,14 @@ export class MarkerViewModel extends Disposable { } private toActions(codeActions: CodeActionSet): IAction[] { - return codeActions.validActions.map(codeAction => new Action( - codeAction.command ? codeAction.command.id : codeAction.title, - codeAction.title, + return codeActions.validActions.map(item => new Action( + item.action.command ? item.action.command.id : item.action.title, + item.action.title, undefined, true, () => { return this.openFileAtMarker(this.marker) - .then(() => this.instantiationService.invokeFunction(applyCodeAction, codeAction)); + .then(() => this.instantiationService.invokeFunction(applyCodeAction, item)); })); } diff --git a/src/vs/workbench/test/browser/api/extHostLanguageFeatures.test.ts b/src/vs/workbench/test/browser/api/extHostLanguageFeatures.test.ts index df33e67802a62..efc8b65b1503c 100644 --- a/src/vs/workbench/test/browser/api/extHostLanguageFeatures.test.ts +++ b/src/vs/workbench/test/browser/api/extHostLanguageFeatures.test.ts @@ -594,10 +594,10 @@ suite('ExtHostLanguageFeatures', function () { const { validActions: actions } = await getCodeActions(model, model.getFullModelRange(), { type: modes.CodeActionTriggerType.Manual }, Progress.None, CancellationToken.None); assert.equal(actions.length, 2); const [first, second] = actions; - assert.equal(first.title, 'Testing1'); - assert.equal(first.command!.id, 'test1'); - assert.equal(second.title, 'Testing2'); - assert.equal(second.command!.id, 'test2'); + assert.equal(first.action.title, 'Testing1'); + assert.equal(first.action.command!.id, 'test1'); + assert.equal(second.action.title, 'Testing2'); + assert.equal(second.action.command!.id, 'test2'); }); test('Quick Fix, code action data conversion', async () => { @@ -618,10 +618,10 @@ suite('ExtHostLanguageFeatures', function () { const { validActions: actions } = await getCodeActions(model, model.getFullModelRange(), { type: modes.CodeActionTriggerType.Manual }, Progress.None, CancellationToken.None); assert.equal(actions.length, 1); const [first] = actions; - assert.equal(first.title, 'Testing1'); - assert.equal(first.command!.title, 'Testing1Command'); - assert.equal(first.command!.id, 'test1'); - assert.equal(first.kind, 'test.scope'); + assert.equal(first.action.title, 'Testing1'); + assert.equal(first.action.command!.title, 'Testing1Command'); + assert.equal(first.action.command!.id, 'test1'); + assert.equal(first.action.kind, 'test.scope'); }); From 136cc276d105e7050db6e7a254362c2465184cde Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Wed, 16 Sep 2020 12:34:20 +0200 Subject: [PATCH 2/5] proposed API for CodeActionProvider#resolveCodeAction and all the wiring --- .../editor/contrib/codeAction/codeAction.ts | 6 ++++- src/vs/vscode.proposed.d.ts | 11 ++++++++ .../api/browser/mainThreadLanguageFeatures.ts | 16 +++++++++--- .../workbench/api/common/extHost.protocol.ts | 6 +++-- .../api/common/extHostLanguageFeatures.ts | 25 +++++++++++++++++-- 5 files changed, 56 insertions(+), 8 deletions(-) diff --git a/src/vs/editor/contrib/codeAction/codeAction.ts b/src/vs/editor/contrib/codeAction/codeAction.ts index 24c60641ee9cb..cd22fd6ca6049 100644 --- a/src/vs/editor/contrib/codeAction/codeAction.ts +++ b/src/vs/editor/contrib/codeAction/codeAction.ts @@ -34,11 +34,15 @@ export class CodeActionItem { async resolve(token: CancellationToken): Promise { // TODO@jrieken when is an item resolved already? if (this.provider?.resolveCodeAction && !this.action.edit && !this.action.command) { + let action: modes.CodeAction | undefined | null; try { - this.provider.resolveCodeAction(this.action, token); + action = await this.provider.resolveCodeAction(this.action, token); } catch (err) { onUnexpectedExternalError(err); } + if (action) { + this.action.edit = action.edit; + } } return this; } diff --git a/src/vs/vscode.proposed.d.ts b/src/vs/vscode.proposed.d.ts index 10e23905a66be..55e858830d7c2 100644 --- a/src/vs/vscode.proposed.d.ts +++ b/src/vs/vscode.proposed.d.ts @@ -16,6 +16,17 @@ declare module 'vscode' { + //#region https://github.com/microsoft/vscode/issues/106410 + + export interface CodeActionProvider { + // TODO@jrieken make it clear that there is no support for commands, only code action + // TODO@jrieken only edit can be set + resolveCodeAction?(codeAction: T, token: CancellationToken): ProviderResult; + } + + //#endregion + + // #region auth provider: https://github.com/microsoft/vscode/issues/88309 /** diff --git a/src/vs/workbench/api/browser/mainThreadLanguageFeatures.ts b/src/vs/workbench/api/browser/mainThreadLanguageFeatures.ts index b3133ba44c927..635730ba75b15 100644 --- a/src/vs/workbench/api/browser/mainThreadLanguageFeatures.ts +++ b/src/vs/workbench/api/browser/mainThreadLanguageFeatures.ts @@ -292,8 +292,8 @@ export class MainThreadLanguageFeatures implements MainThreadLanguageFeaturesSha // --- quick fix - $registerQuickFixSupport(handle: number, selector: IDocumentFilterDto[], metadata: ICodeActionProviderMetadataDto, displayName: string): void { - this._registrations.set(handle, modes.CodeActionProviderRegistry.register(selector, { + $registerQuickFixSupport(handle: number, selector: IDocumentFilterDto[], metadata: ICodeActionProviderMetadataDto, displayName: string, supportsResolve: boolean): void { + const provider: modes.CodeActionProvider = { provideCodeActions: async (model: ITextModel, rangeOrSelection: EditorRange | Selection, context: modes.CodeActionContext, token: CancellationToken): Promise => { const listDto = await this._proxy.$provideCodeActions(handle, model.uri, rangeOrSelection, context, token); if (!listDto) { @@ -311,7 +311,17 @@ export class MainThreadLanguageFeatures implements MainThreadLanguageFeaturesSha providedCodeActionKinds: metadata.providedKinds, documentation: metadata.documentation, displayName - })); + }; + + if (supportsResolve) { + provider.resolveCodeAction = async (codeAction: modes.CodeAction, token: CancellationToken): Promise => { + const data = await this._proxy.$resolveCodeAction(handle, (codeAction).cacheId!, token); + codeAction.edit = reviveWorkspaceEditDto(data); + return codeAction; + }; + } + + this._registrations.set(handle, modes.CodeActionProviderRegistry.register(selector, provider)); } // --- formatting diff --git a/src/vs/workbench/api/common/extHost.protocol.ts b/src/vs/workbench/api/common/extHost.protocol.ts index c42c42c29b29f..f02c0aacd2071 100644 --- a/src/vs/workbench/api/common/extHost.protocol.ts +++ b/src/vs/workbench/api/common/extHost.protocol.ts @@ -380,7 +380,7 @@ export interface MainThreadLanguageFeaturesShape extends IDisposable { $registerDocumentHighlightProvider(handle: number, selector: IDocumentFilterDto[]): void; $registerOnTypeRenameProvider(handle: number, selector: IDocumentFilterDto[], stopPattern: IRegExpDto | undefined): void; $registerReferenceSupport(handle: number, selector: IDocumentFilterDto[]): void; - $registerQuickFixSupport(handle: number, selector: IDocumentFilterDto[], metadata: ICodeActionProviderMetadataDto, displayName: string): void; + $registerQuickFixSupport(handle: number, selector: IDocumentFilterDto[], metadata: ICodeActionProviderMetadataDto, displayName: string, supportsResolve: boolean): void; $registerDocumentFormattingSupport(handle: number, selector: IDocumentFilterDto[], extensionId: ExtensionIdentifier, displayName: string): void; $registerRangeFormattingSupport(handle: number, selector: IDocumentFilterDto[], extensionId: ExtensionIdentifier, displayName: string): void; $registerOnTypeFormattingSupport(handle: number, selector: IDocumentFilterDto[], autoFormatTriggerCharacters: string[], extensionId: ExtensionIdentifier): void; @@ -1310,6 +1310,7 @@ export function reviveWorkspaceEditDto(data: IWorkspaceEditDto | undefined): mod export type ICommandDto = ObjectIdentifier & modes.Command; export interface ICodeActionDto { + cacheId?: ChainedCacheId; title: string; edit?: IWorkspaceEditDto; diagnostics?: IMarkerData[]; @@ -1320,7 +1321,7 @@ export interface ICodeActionDto { } export interface ICodeActionListDto { - cacheId: number; + cacheId: CacheId; actions: ReadonlyArray; } @@ -1388,6 +1389,7 @@ export interface ExtHostLanguageFeaturesShape { $provideOnTypeRenameRanges(handle: number, resource: UriComponents, position: IPosition, token: CancellationToken): Promise<{ ranges: IRange[]; wordPattern?: IRegExpDto; } | undefined>; $provideReferences(handle: number, resource: UriComponents, position: IPosition, context: modes.ReferenceContext, token: CancellationToken): Promise; $provideCodeActions(handle: number, resource: UriComponents, rangeOrSelection: IRange | ISelection, context: modes.CodeActionContext, token: CancellationToken): Promise; + $resolveCodeAction(handle: number, id: ChainedCacheId, token: CancellationToken): Promise; $releaseCodeActions(handle: number, cacheId: number): void; $provideDocumentFormattingEdits(handle: number, resource: UriComponents, options: modes.FormattingOptions, token: CancellationToken): Promise; $provideDocumentRangeFormattingEdits(handle: number, resource: UriComponents, range: IRange, options: modes.FormattingOptions, token: CancellationToken): Promise; diff --git a/src/vs/workbench/api/common/extHostLanguageFeatures.ts b/src/vs/workbench/api/common/extHostLanguageFeatures.ts index 80258e2b7d7de..853bd792e47a2 100644 --- a/src/vs/workbench/api/common/extHostLanguageFeatures.ts +++ b/src/vs/workbench/api/common/extHostLanguageFeatures.ts @@ -413,7 +413,8 @@ class CodeActionAdapter { this._disposables.set(cacheId, disposables); const actions: CustomCodeAction[] = []; - for (const candidate of commandsOrActions) { + for (let i = 0; i < commandsOrActions.length; i++) { + const candidate = commandsOrActions[i]; if (!candidate) { continue; } @@ -439,6 +440,7 @@ class CodeActionAdapter { // new school: convert code action actions.push({ + cacheId: [cacheId, i], title: candidate.title, command: candidate.command && this._commands.toInternal(candidate.command, disposables), diagnostics: candidate.diagnostics && candidate.diagnostics.map(typeConvert.Diagnostic.from), @@ -454,6 +456,21 @@ class CodeActionAdapter { }); } + public async resolveCodeAction(id: extHostProtocol.ChainedCacheId, token: CancellationToken): Promise { + const [sessionId, itemId] = id; + const item = this._cache.get(sessionId, itemId); + if (!item || CodeActionAdapter._isCommand(item)) { + return undefined; // code actions only! + } + if (!this._provider.resolveCodeAction) { + return; // this should not happen... + } + const resolvedItem = await this._provider.resolveCodeAction(item, token); + return resolvedItem?.edit + ? typeConvert.WorkspaceEdit.from(resolvedItem.edit) + : undefined; + } + public releaseCodeActions(cachedId: number): void { this._disposables.get(cachedId)?.dispose(); this._disposables.delete(cachedId); @@ -1595,7 +1612,7 @@ export class ExtHostLanguageFeatures implements extHostProtocol.ExtHostLanguageF kind: x.kind.value, command: this._commands.converter.toInternal(x.command, store), })) - }, ExtHostLanguageFeatures._extLabel(extension)); + }, ExtHostLanguageFeatures._extLabel(extension), Boolean(extension.enableProposedApi && provider.resolveCodeAction)); store.add(this._createDisposable(handle)); return store; } @@ -1605,6 +1622,10 @@ export class ExtHostLanguageFeatures implements extHostProtocol.ExtHostLanguageF return this._withAdapter(handle, CodeActionAdapter, adapter => adapter.provideCodeActions(URI.revive(resource), rangeOrSelection, context, token), undefined); } + $resolveCodeAction(handle: number, id: extHostProtocol.ChainedCacheId, token: CancellationToken): Promise { + return this._withAdapter(handle, CodeActionAdapter, adapter => adapter.resolveCodeAction(id, token), undefined); + } + $releaseCodeActions(handle: number, cacheId: number): void { this._withAdapter(handle, CodeActionAdapter, adapter => Promise.resolve(adapter.releaseCodeActions(cacheId)), undefined); } From f8ad845310290d1e8788edb0c785d6fdae9dd3f0 Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Wed, 16 Sep 2020 13:52:26 +0200 Subject: [PATCH 3/5] Allow executeCodeActionProvider API command to set the number of items to resolve, unit test to check that and the whole resolve machinery --- .../editor/contrib/codeAction/codeAction.ts | 17 ++++++++-- .../api/common/extHostApiCommands.ts | 7 ++-- .../api/common/extHostLanguageFeatures.ts | 2 +- .../browser/api/extHostApiCommands.test.ts | 32 +++++++++++++++++++ 4 files changed, 52 insertions(+), 6 deletions(-) diff --git a/src/vs/editor/contrib/codeAction/codeAction.ts b/src/vs/editor/contrib/codeAction/codeAction.ts index cd22fd6ca6049..eb0bdba64af16 100644 --- a/src/vs/editor/contrib/codeAction/codeAction.ts +++ b/src/vs/editor/contrib/codeAction/codeAction.ts @@ -225,7 +225,7 @@ function getDocumentation( } registerLanguageCommand('_executeCodeActionProvider', async function (accessor, args): Promise> { - const { resource, rangeOrSelection, kind } = args; + const { resource, rangeOrSelection, kind, itemResolveCount } = args; if (!(resource instanceof URI)) { throw illegalArgument(); } @@ -252,6 +252,17 @@ registerLanguageCommand('_executeCodeActionProvider', async function (accessor, Progress.None, CancellationToken.None); - setTimeout(() => codeActionSet.dispose(), 100); - return codeActionSet.validActions.map(item => item.action); + + const resolving: Promise[] = []; + const resolveCount = Math.min(codeActionSet.validActions.length, typeof itemResolveCount === 'number' ? itemResolveCount : 0); + for (let i = 0; i < resolveCount; i++) { + resolving.push(codeActionSet.validActions[i].resolve(CancellationToken.None)); + } + + try { + await Promise.all(resolving); + return codeActionSet.validActions.map(item => item.action); + } finally { + setTimeout(() => codeActionSet.dispose(), 100); + } }); diff --git a/src/vs/workbench/api/common/extHostApiCommands.ts b/src/vs/workbench/api/common/extHostApiCommands.ts index e7cac07ee3bf1..956f358bcd5a5 100644 --- a/src/vs/workbench/api/common/extHostApiCommands.ts +++ b/src/vs/workbench/api/common/extHostApiCommands.ts @@ -282,6 +282,8 @@ export class ExtHostApiCommands { { name: 'uri', description: 'Uri of a text document', constraint: URI }, { name: 'rangeOrSelection', description: 'Range in a text document. Some refactoring provider requires Selection object.', constraint: types.Range }, { name: 'kind', description: '(optional) Code action kind to return code actions for', constraint: (value: any) => !value || typeof value.value === 'string' }, + { name: 'itemResolveCount', description: '(optional) Number of code actions to resolve (too large numbers slow down code actions)', constraint: (value: any) => value === undefined || typeof value === 'number' } + ], returns: 'A promise that resolves to an array of Command-instances.' }); @@ -436,13 +438,14 @@ export class ExtHostApiCommands { } - private _executeCodeActionProvider(resource: URI, rangeOrSelection: types.Range | types.Selection, kind?: string): Promise<(vscode.CodeAction | vscode.Command | undefined)[] | undefined> { + private _executeCodeActionProvider(resource: URI, rangeOrSelection: types.Range | types.Selection, kind?: string, itemResolveCount?: number): Promise<(vscode.CodeAction | vscode.Command | undefined)[] | undefined> { const args = { resource, rangeOrSelection: types.Selection.isSelection(rangeOrSelection) ? typeConverters.Selection.from(rangeOrSelection) : typeConverters.Range.from(rangeOrSelection), - kind + kind, + itemResolveCount, }; return this._commands.executeCommand('_executeCodeActionProvider', args) .then(tryMapWith(codeAction => { diff --git a/src/vs/workbench/api/common/extHostLanguageFeatures.ts b/src/vs/workbench/api/common/extHostLanguageFeatures.ts index 853bd792e47a2..faab8cd649157 100644 --- a/src/vs/workbench/api/common/extHostLanguageFeatures.ts +++ b/src/vs/workbench/api/common/extHostLanguageFeatures.ts @@ -1612,7 +1612,7 @@ export class ExtHostLanguageFeatures implements extHostProtocol.ExtHostLanguageF kind: x.kind.value, command: this._commands.converter.toInternal(x.command, store), })) - }, ExtHostLanguageFeatures._extLabel(extension), Boolean(extension.enableProposedApi && provider.resolveCodeAction)); + }, ExtHostLanguageFeatures._extLabel(extension), Boolean(provider.resolveCodeAction)); store.add(this._createDisposable(handle)); return store; } diff --git a/src/vs/workbench/test/browser/api/extHostApiCommands.test.ts b/src/vs/workbench/test/browser/api/extHostApiCommands.test.ts index f5290dd2f7423..e13e1e2b73677 100644 --- a/src/vs/workbench/test/browser/api/extHostApiCommands.test.ts +++ b/src/vs/workbench/test/browser/api/extHostApiCommands.test.ts @@ -925,6 +925,38 @@ suite('ExtHostLanguageFeatureCommands', function () { }); }); + test('resolving code action', async function () { + + let didCallResolve = 0; + class MyAction extends types.CodeAction { } + + disposables.push(extHost.registerCodeActionProvider(nullExtensionDescription, defaultSelector, { + provideCodeActions(document, rangeOrSelection): vscode.CodeAction[] { + return [new MyAction('title', types.CodeActionKind.Empty.append('foo'))]; + }, + resolveCodeAction(action): vscode.CodeAction { + assert.ok(action instanceof MyAction); + + didCallResolve += 1; + action.title = 'resolved title'; + action.edit = new types.WorkspaceEdit(); + return action; + } + })); + + const selection = new types.Selection(0, 0, 1, 1); + + await rpcProtocol.sync(); + + const value = await commands.executeCommand('vscode.executeCodeActionProvider', model.uri, selection, undefined, 1000); + assert.equal(didCallResolve, 1); + assert.equal(value.length, 1); + + const [first] = value; + assert.equal(first.title, 'title'); // does NOT change + assert.ok(first.edit); // is set + }); + // --- code lens test('CodeLens, back and forth', function () { From 74ad416ebc70b161d70df18f70ef4e7c2b6ca07d Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Thu, 17 Sep 2020 09:06:16 +0200 Subject: [PATCH 4/5] update doc comment and resolve logic --- src/vs/editor/contrib/codeAction/codeAction.ts | 3 +-- src/vs/vscode.proposed.d.ts | 17 +++++++++++++++-- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/src/vs/editor/contrib/codeAction/codeAction.ts b/src/vs/editor/contrib/codeAction/codeAction.ts index eb0bdba64af16..a488d79cae1d2 100644 --- a/src/vs/editor/contrib/codeAction/codeAction.ts +++ b/src/vs/editor/contrib/codeAction/codeAction.ts @@ -32,8 +32,7 @@ export class CodeActionItem { ) { } async resolve(token: CancellationToken): Promise { - // TODO@jrieken when is an item resolved already? - if (this.provider?.resolveCodeAction && !this.action.edit && !this.action.command) { + if (this.provider?.resolveCodeAction && !this.action.edit) { let action: modes.CodeAction | undefined | null; try { action = await this.provider.resolveCodeAction(this.action, token); diff --git a/src/vs/vscode.proposed.d.ts b/src/vs/vscode.proposed.d.ts index 55e858830d7c2..8c7e69b22ac09 100644 --- a/src/vs/vscode.proposed.d.ts +++ b/src/vs/vscode.proposed.d.ts @@ -19,8 +19,21 @@ declare module 'vscode' { //#region https://github.com/microsoft/vscode/issues/106410 export interface CodeActionProvider { - // TODO@jrieken make it clear that there is no support for commands, only code action - // TODO@jrieken only edit can be set + + /** + * Given a code action fill in its [`edit`](#CodeAction.edit)-property, changes to + * all other properties, like title, are ignored. A code action that has an edit + * will not be resolved. + * + * *Note* that a code action provider that returns commands, not code actions, cannot successfully + * implement this function. Returning commands is deprecated and instead code actions should be + * returned. + * + * @param codeAction A code action. + * @param token A cancellation token. + * @return The resolved code action or a thenable that resolve to such. It is OK to return the given + * `item`. When no result is returned, the given `item` will be used. + */ resolveCodeAction?(codeAction: T, token: CancellationToken): ProviderResult; } From cb38e0c22a529e2f05eb8072f50e83990a188a90 Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Thu, 17 Sep 2020 09:08:40 +0200 Subject: [PATCH 5/5] update jsdoc for internal API --- src/vs/editor/common/modes.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vs/editor/common/modes.ts b/src/vs/editor/common/modes.ts index 492bab678970f..2b864b92dd1e5 100644 --- a/src/vs/editor/common/modes.ts +++ b/src/vs/editor/common/modes.ts @@ -655,7 +655,7 @@ export interface CodeActionProvider { provideCodeActions(model: model.ITextModel, range: Range | Selection, context: CodeActionContext, token: CancellationToken): ProviderResult; /** - * Given a code action fill in the edit or command. Will only invoked when missing. + * Given a code action fill in the edit. Will only invoked when missing. */ resolveCodeAction?(codeAction: CodeAction, token: CancellationToken): ProviderResult;