Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for resolving code actions #106856

Merged
merged 5 commits into from
Sep 18, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/vs/editor/common/modes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -654,6 +654,11 @@ export interface CodeActionProvider {
*/
provideCodeActions(model: model.ITextModel, range: Range | Selection, context: CodeActionContext, token: CancellationToken): ProviderResult<CodeActionList>;

/**
* Given a code action fill in the edit. Will only invoked when missing.
*/
resolveCodeAction?(codeAction: CodeAction, token: CancellationToken): ProviderResult<CodeAction>;

/**
* Optional list of CodeActionKinds that this provider returns.
*/
Expand Down
63 changes: 50 additions & 13 deletions src/vs/editor/contrib/codeAction/codeAction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,17 +24,40 @@ 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<this> {
if (this.provider?.resolveCodeAction && !this.action.edit) {
let action: modes.CodeAction | undefined | null;
try {
action = await this.provider.resolveCodeAction(this.action, token);
} catch (err) {
onUnexpectedExternalError(err);
}
if (action) {
this.action.edit = action.edit;
}
}
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[];
}

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) {
Expand All @@ -54,27 +77,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,
Expand Down Expand Up @@ -108,7 +131,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;
Expand Down Expand Up @@ -198,7 +224,7 @@ function getDocumentation(
}

registerLanguageCommand('_executeCodeActionProvider', async function (accessor, args): Promise<ReadonlyArray<modes.CodeAction>> {
const { resource, rangeOrSelection, kind } = args;
const { resource, rangeOrSelection, kind, itemResolveCount } = args;
if (!(resource instanceof URI)) {
throw illegalArgument();
}
Expand All @@ -225,6 +251,17 @@ registerLanguageCommand('_executeCodeActionProvider', async function (accessor,
Progress.None,
CancellationToken.None);

setTimeout(() => codeActionSet.dispose(), 100);
return codeActionSet.validActions;

const resolving: Promise<any>[] = [];
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);
}
});
25 changes: 14 additions & 11 deletions src/vs/editor/contrib/codeAction/codeActionCommands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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';
Expand Down Expand Up @@ -130,14 +131,14 @@ export class QuickFixController extends Disposable implements IEditorContributio
return this._model.trigger(trigger);
}

private _applyCodeAction(action: CodeAction): Promise<void> {
private _applyCodeAction(action: CodeActionItem): Promise<void> {
return this._instantiationService.invokeFunction(applyCodeAction, action, this._editor);
}
}

export async function applyCodeAction(
accessor: ServicesAccessor,
action: CodeAction,
item: CodeActionItem,
editor?: ICodeEditor,
): Promise<void> {
const bulkEditService = accessor.get(IBulkEditService);
Expand All @@ -157,18 +158,20 @@ export async function applyCodeAction(
};

telemetryService.publicLog2<ApplyCodeActionEvent, ApplyCodeEventClassification>('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(
Expand Down
14 changes: 7 additions & 7 deletions src/vs/editor/contrib/codeAction/codeActionMenu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<any>;
onSelectCodeAction: (action: CodeActionItem) => Promise<any>;
}

interface ResolveCodeActionKeybinding {
Expand Down Expand Up @@ -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);
Expand All @@ -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;
Expand Down
16 changes: 8 additions & 8 deletions src/vs/editor/contrib/codeAction/codeActionUi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -29,7 +29,7 @@ export class CodeActionUi extends Disposable {
quickFixActionId: string,
preferredFixActionId: string,
private readonly delegate: {
applyCodeAction: (action: CodeAction, regtriggerAfterApply: boolean) => Promise<void>
applyCodeAction: (action: CodeActionItem, regtriggerAfterApply: boolean) => Promise<void>
},
@IInstantiationService instantiationService: IInstantiationService,
) {
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -114,21 +114,21 @@ 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;
}

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;
}
Expand Down
31 changes: 15 additions & 16 deletions src/vs/editor/contrib/codeAction/test/codeAction.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
Expand All @@ -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');
}

{
Expand All @@ -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 () {
Expand All @@ -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');
}
});

Expand All @@ -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');
}
});

Expand Down Expand Up @@ -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');
}
});

Expand All @@ -282,4 +282,3 @@ suite('CodeAction', () => {
assert.strictEqual(wasInvoked, false);
});
});

Loading