Prioritize implement interface over remove unused in JS/ts

Fixes #94212

If both `implement interface` and `remove unused` are possible, only mark `implement interface` as prefered.

Also changes our core code action sorting logic to prioritize autofixes
This commit is contained in:
Matt Bierner 2020-04-08 13:02:48 -07:00
parent 7c49df64ec
commit 74b4540bd1
2 changed files with 63 additions and 24 deletions

View file

@ -126,19 +126,29 @@ class DiagnosticsSet {
} }
} }
class CodeActionSet { class VsCodeCodeAction extends vscode.CodeAction {
private readonly _actions = new Set<vscode.CodeAction>(); constructor(
private readonly _fixAllActions = new Map<{}, vscode.CodeAction>(); public readonly tsAction: Proto.CodeFixAction,
title: string,
kind: vscode.CodeActionKind,
) {
super(title, kind);
}
}
public get values(): Iterable<vscode.CodeAction> { class CodeActionSet {
private readonly _actions = new Set<VsCodeCodeAction>();
private readonly _fixAllActions = new Map<{}, VsCodeCodeAction>();
public get values(): Iterable<VsCodeCodeAction> {
return this._actions; return this._actions;
} }
public addAction(action: vscode.CodeAction) { public addAction(action: VsCodeCodeAction) {
this._actions.add(action); this._actions.add(action);
} }
public addFixAllAction(fixId: {}, action: vscode.CodeAction) { public addFixAllAction(fixId: {}, action: VsCodeCodeAction) {
const existing = this._fixAllActions.get(fixId); const existing = this._fixAllActions.get(fixId);
if (existing) { if (existing) {
// reinsert action at back of actions list // reinsert action at back of actions list
@ -219,7 +229,13 @@ class TypeScriptQuickFixProvider implements vscode.CodeActionProvider {
for (const diagnostic of fixableDiagnostics.values) { for (const diagnostic of fixableDiagnostics.values) {
await this.getFixesForDiagnostic(document, file, diagnostic, results, token); await this.getFixesForDiagnostic(document, file, diagnostic, results, token);
} }
return Array.from(results.values);
const allActions = Array.from(results.values);
const allTsActions = allActions.map(x => x.tsAction);
for (const action of allActions) {
action.isPreferred = isPreferredFix(action.tsAction, allTsActions);
}
return allActions;
} }
private async getFixesForDiagnostic( private async getFixesForDiagnostic(
@ -259,8 +275,8 @@ class TypeScriptQuickFixProvider implements vscode.CodeActionProvider {
private getSingleFixForTsCodeAction( private getSingleFixForTsCodeAction(
diagnostic: vscode.Diagnostic, diagnostic: vscode.Diagnostic,
tsAction: Proto.CodeFixAction tsAction: Proto.CodeFixAction
): vscode.CodeAction { ): VsCodeCodeAction {
const codeAction = new vscode.CodeAction(tsAction.description, vscode.CodeActionKind.QuickFix); const codeAction = new VsCodeCodeAction(tsAction, tsAction.description, vscode.CodeActionKind.QuickFix);
codeAction.edit = getEditForCodeAction(this.client, tsAction); codeAction.edit = getEditForCodeAction(this.client, tsAction);
codeAction.diagnostics = [diagnostic]; codeAction.diagnostics = [diagnostic];
codeAction.command = { codeAction.command = {
@ -268,7 +284,6 @@ class TypeScriptQuickFixProvider implements vscode.CodeActionProvider {
arguments: [tsAction], arguments: [tsAction],
title: '' title: ''
}; };
codeAction.isPreferred = isPreferredFix(tsAction);
return codeAction; return codeAction;
} }
@ -294,7 +309,8 @@ class TypeScriptQuickFixProvider implements vscode.CodeActionProvider {
return results; return results;
} }
const action = new vscode.CodeAction( const action = new VsCodeCodeAction(
tsAction,
tsAction.fixAllDescription || localize('fixAllInFileLabel', '{0} (Fix all in file)', tsAction.description), tsAction.fixAllDescription || localize('fixAllInFileLabel', '{0} (Fix all in file)', tsAction.description),
vscode.CodeActionKind.QuickFix); vscode.CodeActionKind.QuickFix);
action.diagnostics = [diagnostic]; action.diagnostics = [diagnostic];
@ -317,20 +333,37 @@ const fixAllErrorCodes = new Map<number, number>([
]); ]);
const preferredFixes = new Set([ const preferredFixes = new Map<string, /* priorty */number>([
'annotateWithTypeFromJSDoc', ['annotateWithTypeFromJSDoc', 0],
'constructorForDerivedNeedSuperCall', ['constructorForDerivedNeedSuperCall', 0],
'extendsInterfaceBecomesImplements', ['extendsInterfaceBecomesImplements', 0],
'fixAwaitInSyncFunction', ['fixAwaitInSyncFunction', 0],
'fixClassIncorrectlyImplementsInterface', ['fixClassIncorrectlyImplementsInterface', 1],
'fixUnreachableCode', ['fixUnreachableCode', 0],
'forgottenThisPropertyAccess', ['unusedIdentifier', 0],
'spelling', ['forgottenThisPropertyAccess', 0],
'unusedIdentifier', ['spelling', 1],
'addMissingAwait', ['addMissingAwait', 0],
]); ]);
function isPreferredFix(tsAction: Proto.CodeFixAction): boolean {
return preferredFixes.has(tsAction.fixName); function isPreferredFix(
tsAction: Proto.CodeFixAction,
allActions: readonly Proto.CodeFixAction[]
): boolean {
const priority = preferredFixes.get(tsAction.fixName);
if (typeof priority === 'undefined') {
return false;
}
return allActions.every(otherAction => {
if (otherAction === tsAction) {
return true;
}
const otherPriority = preferredFixes.get(otherAction.fixName);
if (typeof otherPriority === 'undefined') {
return true;
}
return priority >= otherPriority;
});
} }
export function register( export function register(

View file

@ -35,6 +35,12 @@ export interface CodeActionSet extends IDisposable {
class ManagedCodeActionSet extends Disposable implements CodeActionSet { class ManagedCodeActionSet extends Disposable implements CodeActionSet {
private static codeActionsComparator(a: modes.CodeAction, b: modes.CodeAction): number { private static codeActionsComparator(a: modes.CodeAction, b: modes.CodeAction): number {
if (a.isPreferred && !b.isPreferred) {
return -1;
} else if (!a.isPreferred && b.isPreferred) {
return 1;
}
if (isNonEmptyArray(a.diagnostics)) { if (isNonEmptyArray(a.diagnostics)) {
if (isNonEmptyArray(b.diagnostics)) { if (isNonEmptyArray(b.diagnostics)) {
return a.diagnostics[0].message.localeCompare(b.diagnostics[0].message); return a.diagnostics[0].message.localeCompare(b.diagnostics[0].message);