Prioritize implement interface over remove unused in JS/ts

mjbvz · mjbvz · commit 74b4540bd132 · 2020-04-08T13:05:44.000-07:00
Fixes microsoft#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
diff --git a/extensions/typescript-language-features/src/features/quickFix.ts b/extensions/typescript-language-features/src/features/quickFix.ts
@@ -126,19 +126,29 @@ class DiagnosticsSet {
 	}
 }
 
+class VsCodeCodeAction extends vscode.CodeAction {
+	constructor(
+		public readonly tsAction: Proto.CodeFixAction,
+		title: string,
+		kind: vscode.CodeActionKind,
+	) {
+		super(title, kind);
+	}
+}
+
 class CodeActionSet {
-	private readonly _actions = new Set<vscode.CodeAction>();
-	private readonly _fixAllActions = new Map<{}, vscode.CodeAction>();
+	private readonly _actions = new Set<VsCodeCodeAction>();
+	private readonly _fixAllActions = new Map<{}, VsCodeCodeAction>();
 
-	public get values(): Iterable<vscode.CodeAction> {
+	public get values(): Iterable<VsCodeCodeAction> {
 		return this._actions;
 	}
 
-	public addAction(action: vscode.CodeAction) {
+	public addAction(action: VsCodeCodeAction) {
 		this._actions.add(action);
 	}
 
-	public addFixAllAction(fixId: {}, action: vscode.CodeAction) {
+	public addFixAllAction(fixId: {}, action: VsCodeCodeAction) {
 		const existing = this._fixAllActions.get(fixId);
 		if (existing) {
 			// reinsert action at back of actions list
@@ -219,7 +229,13 @@ class TypeScriptQuickFixProvider implements vscode.CodeActionProvider {
 		for (const diagnostic of fixableDiagnostics.values) {
 			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(
@@ -259,16 +275,15 @@ class TypeScriptQuickFixProvider implements vscode.CodeActionProvider {
 	private getSingleFixForTsCodeAction(
 		diagnostic: vscode.Diagnostic,
 		tsAction: Proto.CodeFixAction
-	): vscode.CodeAction {
-		const codeAction = new vscode.CodeAction(tsAction.description, vscode.CodeActionKind.QuickFix);
+	): VsCodeCodeAction {
+		const codeAction = new VsCodeCodeAction(tsAction, tsAction.description, vscode.CodeActionKind.QuickFix);
 		codeAction.edit = getEditForCodeAction(this.client, tsAction);
 		codeAction.diagnostics = [diagnostic];
 		codeAction.command = {
 			command: ApplyCodeActionCommand.ID,
 			arguments: [tsAction],
 			title: ''
 		};
-		codeAction.isPreferred = isPreferredFix(tsAction);
 		return codeAction;
 	}
 
@@ -294,7 +309,8 @@ class TypeScriptQuickFixProvider implements vscode.CodeActionProvider {
 			return results;
 		}
 
-		const action = new vscode.CodeAction(
+		const action = new VsCodeCodeAction(
+			tsAction,
 			tsAction.fixAllDescription || localize('fixAllInFileLabel', '{0} (Fix all in file)', tsAction.description),
 			vscode.CodeActionKind.QuickFix);
 		action.diagnostics = [diagnostic];
@@ -317,20 +333,37 @@ const fixAllErrorCodes = new Map<number, number>([
 ]);
 
 
-const preferredFixes = new Set([
-	'annotateWithTypeFromJSDoc',
-	'constructorForDerivedNeedSuperCall',
-	'extendsInterfaceBecomesImplements',
-	'fixAwaitInSyncFunction',
-	'fixClassIncorrectlyImplementsInterface',
-	'fixUnreachableCode',
-	'forgottenThisPropertyAccess',
-	'spelling',
-	'unusedIdentifier',
-	'addMissingAwait',
+const preferredFixes = new Map<string, /* priorty */number>([
+	['annotateWithTypeFromJSDoc', 0],
+	['constructorForDerivedNeedSuperCall', 0],
+	['extendsInterfaceBecomesImplements', 0],
+	['fixAwaitInSyncFunction', 0],
+	['fixClassIncorrectlyImplementsInterface', 1],
+	['fixUnreachableCode', 0],
+	['unusedIdentifier', 0],
+	['forgottenThisPropertyAccess', 0],
+	['spelling', 1],
+	['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(
diff --git a/src/vs/editor/contrib/codeAction/codeAction.ts b/src/vs/editor/contrib/codeAction/codeAction.ts
@@ -35,6 +35,12 @@ export interface CodeActionSet extends IDisposable {
 class ManagedCodeActionSet extends Disposable implements CodeActionSet {
 
 	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(b.diagnostics)) {
 				return a.diagnostics[0].message.localeCompare(b.diagnostics[0].message);
