Skip to content

Commit

Permalink
Prototyping new CodeAction API (#36316)
Browse files Browse the repository at this point in the history
* Add CodeAction Type

Adds skeleton on a new CodeActionType and allows codeActionProvider to return either `Command`s or `CodeAction`s

Move proposed CodeAction API to proposed and try using it in TS

Split CodeAction into quickfix and refactoring classes

Update proposed interface

Update for new API

Adding basic docs

* Support workspace edits and text edits in codeactions

* Remove placeholders

* Resolving conflicts and making PR suggested changes

* Fix quick fix test

* Revert change to only use `CodeAction` instead of `CodeAction | Command` in modes since this will break `vscode.executeCodeActionProvider`
  • Loading branch information
mjbvz authored Nov 9, 2017
1 parent 8a79656 commit be88547
Show file tree
Hide file tree
Showing 23 changed files with 274 additions and 104 deletions.
2 changes: 1 addition & 1 deletion build/monaco/monaco.d.ts.recipe
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ declare module monaco.languages {

#includeAll(vs/editor/standalone/browser/standaloneLanguages;modes.=>;editorCommon.=>editor.;IMarkerData=>editor.IMarkerData):
#includeAll(vs/editor/common/modes/languageConfiguration):
#includeAll(vs/editor/common/modes;editorCommon.IRange=>IRange;editorCommon.IPosition=>IPosition;editorCommon.=>editor.):
#includeAll(vs/editor/common/modes;editorCommon.IRange=>IRange;editorCommon.IPosition=>IPosition;editorCommon.=>editor.;IMarkerData=>editor.IMarkerData):
#include(vs/editor/common/services/modeService): ILanguageExtensionPoint
#includeAll(vs/editor/standalone/common/monarch/monarchTypes):

Expand Down
42 changes: 17 additions & 25 deletions extensions/typescript/src/features/codeActionProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,44 +9,36 @@ import * as Proto from '../protocol';
import { ITypeScriptServiceClient } from '../typescriptService';
import { vsRangeToTsFileRange } from '../utils/convert';
import FormattingConfigurationManager from './formattingConfigurationManager';
import { applyCodeAction } from '../utils/codeAction';
import { CommandManager, Command } from '../utils/commandManager';
import { getEditForCodeAction } from '../utils/codeAction';

interface NumberSet {
[key: number]: boolean;
}

class ApplyCodeActionCommand implements Command {

public static readonly ID: string = '_typescript.applyCodeAction';
public readonly id: string = ApplyCodeActionCommand.ID;

constructor(
private readonly client: ITypeScriptServiceClient
) { }

execute(action: Proto.CodeAction, file: string): void {
applyCodeAction(this.client, action, file);
}
}

export default class TypeScriptCodeActionProvider implements vscode.CodeActionProvider {
private _supportedCodeActions?: Thenable<NumberSet>;

constructor(
private readonly client: ITypeScriptServiceClient,
private readonly formattingConfigurationManager: FormattingConfigurationManager,
commandManager: CommandManager
private readonly formattingConfigurationManager: FormattingConfigurationManager
) { }

public provideCodeActions(
_document: vscode.TextDocument,
_range: vscode.Range,
_context: vscode.CodeActionContext,
_token: vscode.CancellationToken
) {
commandManager.register(new ApplyCodeActionCommand(this.client));
// Uses provideCodeActions2 instead
return [];
}

public async provideCodeActions(
public async provideCodeActions2(
document: vscode.TextDocument,
range: vscode.Range,
context: vscode.CodeActionContext,
token: vscode.CancellationToken
): Promise<vscode.Command[]> {
): Promise<vscode.CodeAction[]> {
if (!this.client.apiVersion.has213Features()) {
return [];
}
Expand All @@ -68,7 +60,7 @@ export default class TypeScriptCodeActionProvider implements vscode.CodeActionPr
errorCodes: Array.from(supportedActions)
};
const response = await this.client.execute('getCodeFixes', args, token);
return (response.body || []).map(action => this.getCommandForAction(action, file));
return (response.body || []).map(action => this.getCommandForAction(action));
}

private get supportedCodeActions(): Thenable<NumberSet> {
Expand All @@ -92,11 +84,11 @@ export default class TypeScriptCodeActionProvider implements vscode.CodeActionPr
.filter(code => supportedActions[code]));
}

private getCommandForAction(action: Proto.CodeAction, file: string): vscode.Command {
private getCommandForAction(action: Proto.CodeAction): vscode.CodeAction {
return {
title: action.description,
command: ApplyCodeActionCommand.ID,
arguments: [action, file]
edits: getEditForCodeAction(this.client, action),
diagnostics: []
};
}
}
27 changes: 19 additions & 8 deletions extensions/typescript/src/features/refactorProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,12 +106,17 @@ export default class TypeScriptRefactorProvider implements vscode.CodeActionProv
commandManager.register(new SelectRefactorCommand(doRefactoringCommand));
}

public async provideCodeActions(
public async provideCodeActions() {
// Uses provideCodeActions2 instead
return [];
}

public async provideCodeActions2(
document: vscode.TextDocument,
range: vscode.Range,
_context: vscode.CodeActionContext,
token: vscode.CancellationToken
): Promise<vscode.Command[]> {
): Promise<vscode.CodeAction[]> {
if (!this.client.apiVersion.has240Features()) {
return [];
}
Expand All @@ -128,20 +133,26 @@ export default class TypeScriptRefactorProvider implements vscode.CodeActionProv
return [];
}

const actions: vscode.Command[] = [];
const actions: vscode.CodeAction[] = [];
for (const info of response.body) {
if (info.inlineable === false) {
actions.push({
title: info.description,
command: SelectRefactorCommand.ID,
arguments: [document, file, info, range]
command: {
title: info.description,
command: SelectRefactorCommand.ID,
arguments: [document, file, info, range]
}
});
} else {
for (const action of info.actions) {
actions.push({
title: action.description,
command: ApplyRefactoringCommand.ID,
arguments: [document, file, info.name, action.name, range]
title: info.description,
command: {
title: info.description,
command: ApplyRefactoringCommand.ID,
arguments: [document, file, info.name, action.name, range]
}
});
}
}
Expand Down
2 changes: 1 addition & 1 deletion extensions/typescript/src/typescriptMain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ class LanguageProvider {
this.disposables.push(languages.registerDocumentSymbolProvider(selector, new (await import('./features/documentSymbolProvider')).default(client)));
this.disposables.push(languages.registerSignatureHelpProvider(selector, new (await import('./features/signatureHelpProvider')).default(client), '(', ','));
this.disposables.push(languages.registerRenameProvider(selector, new (await import('./features/renameProvider')).default(client)));
this.disposables.push(languages.registerCodeActionsProvider(selector, new (await import('./features/codeActionProvider')).default(client, this.formattingOptionsManager, this.commandManager)));
this.disposables.push(languages.registerCodeActionsProvider(selector, new (await import('./features/codeActionProvider')).default(client, this.formattingOptionsManager)));
this.disposables.push(languages.registerCodeActionsProvider(selector, new (await import('./features/refactorProvider')).default(client, this.formattingOptionsManager, this.commandManager)));
this.registerVersionDependentProviders();

Expand Down
27 changes: 22 additions & 5 deletions extensions/typescript/src/utils/codeAction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,10 @@ import * as Proto from '../protocol';
import { tsTextSpanToVsRange } from './convert';
import { ITypeScriptServiceClient } from '../typescriptService';


export async function applyCodeAction(
export function getEditForCodeAction(
client: ITypeScriptServiceClient,
action: Proto.CodeAction,
file: string
): Promise<boolean> {
action: Proto.CodeAction
): WorkspaceEdit | undefined {
if (action.changes && action.changes.length) {
const workspaceEdit = new WorkspaceEdit();
for (const change of action.changes) {
Expand All @@ -24,11 +22,30 @@ export async function applyCodeAction(
}
}

return workspaceEdit;
}
return undefined;
}

export async function applyCodeAction(
client: ITypeScriptServiceClient,
action: Proto.CodeAction,
file: string
): Promise<boolean> {
const workspaceEdit = getEditForCodeAction(client, action);
if (workspaceEdit) {
if (!(await workspace.applyEdit(workspaceEdit))) {
return false;
}
}
return applyCodeActionCommands(client, action, file);
}

export async function applyCodeActionCommands(
client: ITypeScriptServiceClient,
action: Proto.CodeAction,
file: string
): Promise<boolean> {
if (action.commands && action.commands.length) {
for (const command of action.commands) {
const response = await client.execute('applyCodeActionCommand', { file, command });
Expand Down
10 changes: 9 additions & 1 deletion src/vs/editor/common/modes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { Range, IRange } from 'vs/editor/common/core/range';
import Event from 'vs/base/common/event';
import { TokenizationRegistryImpl } from 'vs/editor/common/modes/tokenizationRegistry';
import { Color } from 'vs/base/common/color';
import { IMarkerData } from 'vs/platform/markers/common/markers';

/**
* Open ended enum at runtime
Expand Down Expand Up @@ -275,6 +276,13 @@ export interface ISuggestSupport {
resolveCompletionItem?(model: editorCommon.IModel, position: Position, item: ISuggestion, token: CancellationToken): ISuggestion | Thenable<ISuggestion>;
}

export interface CodeAction {
title: string;
command?: Command;
edits?: WorkspaceEdit;
diagnostics?: IMarkerData[];
}

/**
* The code action interface defines the contract between extensions and
* the [light bulb](https://code.visualstudio.com/docs/editor/editingevolved#_code-action) feature.
Expand All @@ -284,7 +292,7 @@ export interface CodeActionProvider {
/**
* Provide commands for the given document and range.
*/
provideCodeActions(model: editorCommon.IReadOnlyModel, range: Range, token: CancellationToken): Command[] | Thenable<Command[]>;
provideCodeActions(model: editorCommon.IReadOnlyModel, range: Range, token: CancellationToken): (CodeAction | Command)[] | Thenable<(Command | CodeAction)[]>;
}

/**
Expand Down
6 changes: 3 additions & 3 deletions src/vs/editor/contrib/quickFix/quickFix.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,16 @@
import URI from 'vs/base/common/uri';
import { IReadOnlyModel } from 'vs/editor/common/editorCommon';
import { Range } from 'vs/editor/common/core/range';
import { Command, CodeActionProviderRegistry } from 'vs/editor/common/modes';
import { CodeActionProviderRegistry, CodeAction, Command } from 'vs/editor/common/modes';
import { asWinJsPromise } from 'vs/base/common/async';
import { TPromise } from 'vs/base/common/winjs.base';
import { onUnexpectedExternalError, illegalArgument } from 'vs/base/common/errors';
import { IModelService } from 'vs/editor/common/services/modelService';
import { CommonEditorRegistry } from 'vs/editor/common/editorCommonExtensions';

export function getCodeActions(model: IReadOnlyModel, range: Range): TPromise<Command[]> {
export function getCodeActions(model: IReadOnlyModel, range: Range): TPromise<(CodeAction | Command)[]> {

const allResults: Command[] = [];
const allResults: (CodeAction | Command)[] = [];
const promises = CodeActionProviderRegistry.all(model).map(support => {
return asWinJsPromise(token => support.provideCodeActions(model, range, token)).then(result => {
if (Array.isArray(result)) {
Expand Down
25 changes: 22 additions & 3 deletions src/vs/editor/contrib/quickFix/quickFixCommands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@ import { registerEditorContribution } from 'vs/editor/browser/editorBrowserExten
import { QuickFixContextMenu } from './quickFixWidget';
import { LightBulbWidget } from './lightBulbWidget';
import { QuickFixModel, QuickFixComputeEvent } from './quickFixModel';
import { TPromise } from 'vs/base/common/winjs.base';
import { CodeAction } from 'vs/editor/common/modes';
import { createBulkEdit } from 'vs/editor/common/services/bulkEdit';
import { IFileService } from 'vs/platform/files/common/files';
import { ITextModelService } from 'vs/editor/common/services/resolverService';

export class QuickFixController implements IEditorContribution {

Expand All @@ -38,13 +43,15 @@ export class QuickFixController implements IEditorContribution {
constructor(editor: ICodeEditor,
@IMarkerService markerService: IMarkerService,
@IContextKeyService contextKeyService: IContextKeyService,
@ICommandService commandService: ICommandService,
@ICommandService private readonly _commandService: ICommandService,
@IContextMenuService contextMenuService: IContextMenuService,
@IKeybindingService private _keybindingService: IKeybindingService
@IKeybindingService private readonly _keybindingService: IKeybindingService,
@ITextModelService private readonly _textModelService: ITextModelService,
@IFileService private _fileService: IFileService
) {
this._editor = editor;
this._model = new QuickFixModel(this._editor, markerService);
this._quickFixContextMenu = new QuickFixContextMenu(editor, contextMenuService, commandService);
this._quickFixContextMenu = new QuickFixContextMenu(editor, contextMenuService, action => this._onApplyCodeAction(action));
this._lightBulbWidget = new LightBulbWidget(editor);

this._updateLightBulbTitle();
Expand Down Expand Up @@ -102,6 +109,18 @@ export class QuickFixController implements IEditorContribution {
}
this._lightBulbWidget.title = title;
}

private async _onApplyCodeAction(action: CodeAction): TPromise<void> {
if (action.edits) {
const edit = createBulkEdit(this._textModelService, this._editor, this._fileService);
edit.add(action.edits.edits);
await edit.finish();
}

if (action.command) {
await this._commandService.executeCommand(action.command.id, ...action.command.arguments);
}
}
}

export class QuickFixAction extends EditorAction {
Expand Down
17 changes: 14 additions & 3 deletions src/vs/editor/contrib/quickFix/quickFixModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { IMarkerService } from 'vs/platform/markers/common/markers';
import { Range } from 'vs/editor/common/core/range';
import { Selection } from 'vs/editor/common/core/selection';
import { ICommonCodeEditor } from 'vs/editor/common/editorCommon';
import { CodeActionProviderRegistry, Command } from 'vs/editor/common/modes';
import { CodeActionProviderRegistry, CodeAction, Command } from 'vs/editor/common/modes';
import { getCodeActions } from './quickFix';
import { Position } from 'vs/editor/common/core/position';

Expand Down Expand Up @@ -112,11 +112,22 @@ export class QuickFixOracle {
const model = this._editor.getModel();
const range = model.validateRange(rangeOrSelection);
const position = rangeOrSelection instanceof Selection ? rangeOrSelection.getPosition() : rangeOrSelection.getStartPosition();

const fixes = getCodeActions(model, range).then(actions =>
actions.map(action => {
if ('id' in action) {
// must be a command
const command = action as Command;
return { title: command.title, command: command } as CodeAction;
}
return action;
}));

this._signalChange({
type,
range,
position,
fixes: getCodeActions(model, range)
fixes
});
}
}
Expand All @@ -126,7 +137,7 @@ export interface QuickFixComputeEvent {
type: 'auto' | 'manual';
range: Range;
position: Position;
fixes: TPromise<Command[]>;
fixes: TPromise<CodeAction[]>;
}

export class QuickFixModel {
Expand Down
27 changes: 11 additions & 16 deletions src/vs/editor/contrib/quickFix/quickFixWidget.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,38 +10,33 @@ import { always } from 'vs/base/common/async';
import { getDomNodePagePosition } from 'vs/base/browser/dom';
import { Position } from 'vs/editor/common/core/position';
import { ICodeEditor } from 'vs/editor/browser/editorBrowser';
import { Command } from 'vs/editor/common/modes';
import { CodeAction } from 'vs/editor/common/modes';
import { IContextMenuService } from 'vs/platform/contextview/browser/contextView';
import { ICommandService } from 'vs/platform/commands/common/commands';
import { Action } from 'vs/base/common/actions';
import Event, { Emitter } from 'vs/base/common/event';
import { ScrollType } from 'vs/editor/common/editorCommon';

export class QuickFixContextMenu {

private _editor: ICodeEditor;
private _contextMenuService: IContextMenuService;
private _commandService: ICommandService;
private _visible: boolean;
private _onDidExecuteCodeAction = new Emitter<void>();

readonly onDidExecuteCodeAction: Event<void> = this._onDidExecuteCodeAction.event;

constructor(editor: ICodeEditor, contextMenuService: IContextMenuService, commandService: ICommandService) {
this._editor = editor;
this._contextMenuService = contextMenuService;
this._commandService = commandService;
}
constructor(
private readonly _editor: ICodeEditor,
private readonly _contextMenuService: IContextMenuService,
private readonly _onApplyCodeAction: (action: CodeAction) => TPromise<any>
) { }

show(fixes: TPromise<Command[]>, at: { x: number; y: number } | Position) {
show(fixes: TPromise<CodeAction[]>, at: { x: number; y: number } | Position) {

const actions = fixes.then(value => {
return value.map(command => {
return new Action(command.id, command.title, undefined, true, () => {
return value.map(action => {
return new Action(action.command ? action.command.id : action.title, action.title, undefined, true, () => {
return always(
this._commandService.executeCommand(command.id, ...command.arguments),
() => this._onDidExecuteCodeAction.fire(undefined)
);
this._onApplyCodeAction(action),
() => this._onDidExecuteCodeAction.fire(undefined));
});
});
});
Expand Down
Loading

0 comments on commit be88547

Please sign in to comment.