Add initial source actions for remove unused and add missing imports

Fixes #95831
This commit is contained in:
Matt Bierner 2020-05-11 23:06:22 -07:00
parent 6efd86a30c
commit b312fa2d33
4 changed files with 226 additions and 89 deletions

View file

@ -9,30 +9,187 @@ import type * as Proto from '../protocol';
import { ITypeScriptServiceClient } from '../typescriptService'; import { ITypeScriptServiceClient } from '../typescriptService';
import API from '../utils/api'; import API from '../utils/api';
import { VersionDependentRegistration } from '../utils/dependentRegistration'; import { VersionDependentRegistration } from '../utils/dependentRegistration';
import * as errorCodes from '../utils/errorCodes';
import * as fixNames from '../utils/fixNames';
import * as typeConverters from '../utils/typeConverters'; import * as typeConverters from '../utils/typeConverters';
import { DiagnosticsManager } from './diagnostics'; import { DiagnosticsManager } from './diagnostics';
import FileConfigurationManager from './fileConfigurationManager'; import FileConfigurationManager from './fileConfigurationManager';
import * as errorCodes from '../utils/errorCodes';
import * as fixNames from '../utils/fixNames';
const localize = nls.loadMessageBundle(); const localize = nls.loadMessageBundle();
interface AutoFixableError { interface AutoFix {
readonly code: number; readonly code: number;
readonly fixName: string; readonly fixName: string;
} }
const fixImplementInterface = Object.freeze<AutoFixableError>({ code: errorCodes.incorrectlyImplementsInterface, fixName: fixNames.classIncorrectlyImplementsInterface }); async function buildIndividualFixes(
const fixUnreachable = Object.freeze<AutoFixableError>({ code: errorCodes.unreachableCode, fixName: fixNames.unreachableCode }); fixes: readonly AutoFix[],
const fixAsync = Object.freeze<AutoFixableError>({ code: errorCodes.asyncOnlyAllowedInAsyncFunctions, fixName: fixNames.awaitInSyncFunction }); edit: vscode.WorkspaceEdit,
client: ITypeScriptServiceClient,
file: string,
diagnostics: readonly vscode.Diagnostic[],
token: vscode.CancellationToken,
): Promise<void> {
for (const diagnostic of diagnostics) {
for (const { code, fixName } of fixes) {
if (token.isCancellationRequested) {
return;
}
if (diagnostic.code !== code) {
continue;
}
const args: Proto.CodeFixRequestArgs = {
...typeConverters.Range.toFileRangeRequestArgs(file, diagnostic.range),
errorCodes: [+(diagnostic.code!)]
};
const response = await client.execute('getCodeFixes', args, token);
if (response.type !== 'response') {
continue;
}
const fix = response.body?.find(fix => fix.fixName === fixName);
if (fix) {
typeConverters.WorkspaceEdit.withFileCodeEdits(edit, client, fix.changes);
break;
}
}
}
}
async function buildCombinedFix(
fixes: readonly AutoFix[],
edit: vscode.WorkspaceEdit,
client: ITypeScriptServiceClient,
file: string,
diagnostics: readonly vscode.Diagnostic[],
token: vscode.CancellationToken,
): Promise<void> {
for (const diagnostic of diagnostics) {
for (const { code, fixName } of fixes) {
if (token.isCancellationRequested) {
return;
}
if (diagnostic.code !== code) {
continue;
}
const args: Proto.CodeFixRequestArgs = {
...typeConverters.Range.toFileRangeRequestArgs(file, diagnostic.range),
errorCodes: [+(diagnostic.code!)]
};
const response = await client.execute('getCodeFixes', args, token);
if (response.type !== 'response' || !response.body?.length) {
continue;
}
const fix = response.body?.find(fix => fix.fixName === fixName);
if (!fix) {
continue;
}
if (!fix.fixId) {
typeConverters.WorkspaceEdit.withFileCodeEdits(edit, client, fix.changes);
return;
}
const combinedArgs: Proto.GetCombinedCodeFixRequestArgs = {
scope: {
type: 'file',
args: { file }
},
fixId: fix.fixId,
};
const combinedResponse = await client.execute('getCombinedCodeFix', combinedArgs, token);
if (combinedResponse.type !== 'response' || !combinedResponse.body) {
return;
}
typeConverters.WorkspaceEdit.withFileCodeEdits(edit, client, combinedResponse.body.changes);
return;
}
}
}
// #region Source Actions
abstract class SourceAction extends vscode.CodeAction {
abstract async build(
client: ITypeScriptServiceClient,
file: string,
diagnostics: readonly vscode.Diagnostic[],
token: vscode.CancellationToken,
): Promise<void>;
}
class SourceFixAll extends SourceAction {
static readonly kind = vscode.CodeActionKind.SourceFixAll.append('ts');
constructor() {
super(localize('autoFix.label', 'Fix All'), SourceFixAll.kind);
}
async build(client: ITypeScriptServiceClient, file: string, diagnostics: readonly vscode.Diagnostic[], token: vscode.CancellationToken): Promise<void> {
this.edit = new vscode.WorkspaceEdit();
await buildIndividualFixes([
{ code: errorCodes.incorrectlyImplementsInterface, fixName: fixNames.classIncorrectlyImplementsInterface },
{ code: errorCodes.asyncOnlyAllowedInAsyncFunctions, fixName: fixNames.awaitInSyncFunction },
], this.edit, client, file, diagnostics, token);
await buildCombinedFix([
{ code: errorCodes.unreachableCode, fixName: fixNames.unreachableCode }
], this.edit, client, file, diagnostics, token);
}
}
class SourceRemoveUnused extends SourceAction {
static readonly kind = vscode.CodeActionKind.Source.append('removeUnused').append('ts');
constructor() {
super(localize('autoFix.unused.label', 'Remove all unused code'), SourceRemoveUnused.kind);
}
async build(client: ITypeScriptServiceClient, file: string, diagnostics: readonly vscode.Diagnostic[], token: vscode.CancellationToken): Promise<void> {
this.edit = new vscode.WorkspaceEdit();
await buildCombinedFix([
{ code: errorCodes.variableDeclaredButNeverUsed, fixName: fixNames.unusedIdentifier },
], this.edit, client, file, diagnostics, token);
}
}
class SourceAddMissingImports extends SourceAction {
static readonly kind = vscode.CodeActionKind.Source.append('addMissingImports').append('ts');
constructor() {
super(localize('autoFix.missingImports.label', 'Add all missing imports'), SourceRemoveUnused.kind);
}
async build(client: ITypeScriptServiceClient, file: string, diagnostics: readonly vscode.Diagnostic[], token: vscode.CancellationToken): Promise<void> {
this.edit = new vscode.WorkspaceEdit();
await buildCombinedFix(
errorCodes.cannotFindName.map(code => ({ code, fixName: fixNames.fixImport })),
this.edit, client, file, diagnostics, token);
}
}
//#endregion
class TypeScriptAutoFixProvider implements vscode.CodeActionProvider { class TypeScriptAutoFixProvider implements vscode.CodeActionProvider {
private static readonly fixAllKind = vscode.CodeActionKind.SourceFixAll.append('ts');
public static readonly metadata: vscode.CodeActionProviderMetadata = { public static readonly metadata: vscode.CodeActionProviderMetadata = {
providedCodeActionKinds: [ providedCodeActionKinds: [
TypeScriptAutoFixProvider.fixAllKind, SourceFixAll.kind,
SourceRemoveUnused.kind,
SourceAddMissingImports.kind,
] ]
}; };
@ -48,7 +205,7 @@ class TypeScriptAutoFixProvider implements vscode.CodeActionProvider {
context: vscode.CodeActionContext, context: vscode.CodeActionContext,
token: vscode.CancellationToken token: vscode.CancellationToken
): Promise<vscode.CodeAction[] | undefined> { ): Promise<vscode.CodeAction[] | undefined> {
if (!context.only || !vscode.CodeActionKind.SourceFixAll.intersects(context.only)) { if (!context.only || !vscode.CodeActionKind.Source.intersects(context.only)) {
return undefined; return undefined;
} }
@ -57,95 +214,44 @@ class TypeScriptAutoFixProvider implements vscode.CodeActionProvider {
return undefined; return undefined;
} }
const autoFixes = this.getAutoFixes(); const actions = this.getFixAllActions(context.only);
const autoFixableDiagnostics = this.getAutoFixableDiagnostics(document, autoFixes);
if (!autoFixableDiagnostics.length) {
return undefined;
}
const fixAllAction = await this.getFixAllCodeAction(document, file, autoFixableDiagnostics, autoFixes, token);
return fixAllAction ? [fixAllAction] : undefined;
}
private getAutoFixableDiagnostics(
document: vscode.TextDocument,
autoFixes: readonly AutoFixableError[],
): vscode.Diagnostic[] {
if (this.client.bufferSyncSupport.hasPendingDiagnostics(document.uri)) { if (this.client.bufferSyncSupport.hasPendingDiagnostics(document.uri)) {
return []; return actions;
} }
const fixableCodes = new Set(autoFixes.map(x => x.code)); const diagnostics = this.diagnosticsManager.getDiagnostics(document.uri);
return this.diagnosticsManager.getDiagnostics(document.uri) if (!diagnostics.length) {
.filter(x => fixableCodes.has(+(x.code as number))); // Actions are a no-op in this case but we still want to return them
} return actions;
}
private async getFixAllCodeAction(
document: vscode.TextDocument,
file: string,
diagnostics: ReadonlyArray<vscode.Diagnostic>,
autoFixes: readonly AutoFixableError[],
token: vscode.CancellationToken,
): Promise<vscode.CodeAction | undefined> {
await this.fileConfigurationManager.ensureConfigurationForDocument(document, token); await this.fileConfigurationManager.ensureConfigurationForDocument(document, token);
const autoFixResponse = await this.getAutoFixEdit(file, diagnostics, autoFixes, token); if (token.isCancellationRequested) {
if (!autoFixResponse) {
return undefined;
}
const { edit, fixedDiagnostics } = autoFixResponse;
if (!edit.size) {
return undefined; return undefined;
} }
const codeAction = new vscode.CodeAction( await Promise.all(actions.map(action => action.build(this.client, file, diagnostics, token)));
localize('autoFix.label', 'Auto fix'),
TypeScriptAutoFixProvider.fixAllKind);
codeAction.edit = edit;
codeAction.diagnostics = fixedDiagnostics;
return codeAction; return actions;
} }
private async getAutoFixEdit( private getFixAllActions(only: vscode.CodeActionKind): SourceAction[] {
file: string, const fixes = [];
diagnostics: ReadonlyArray<vscode.Diagnostic>,
autoFixes: readonly AutoFixableError[],
token: vscode.CancellationToken,
): Promise<{ edit: vscode.WorkspaceEdit, fixedDiagnostics: vscode.Diagnostic[] } | undefined> {
const edit = new vscode.WorkspaceEdit();
const fixedDiagnostics: vscode.Diagnostic[] = [];
for (const diagnostic of diagnostics) {
const args: Proto.CodeFixRequestArgs = {
...typeConverters.Range.toFileRangeRequestArgs(file, diagnostic.range),
errorCodes: [+(diagnostic.code!)]
};
const response = await this.client.execute('getCodeFixes', args, token);
if (response.type !== 'response' || !response.body || response.body.length > 1) {
continue;
}
const fix = response.body[0]; if (only.intersects(SourceFixAll.kind)) {
if (autoFixes.some(autoFix => autoFix.fixName.includes(fix.fixName))) { fixes.push(new SourceFixAll());
typeConverters.WorkspaceEdit.withFileCodeEdits(edit, this.client, fix.changes);
fixedDiagnostics.push(diagnostic);
}
} }
if (!fixedDiagnostics.length) { if (only.intersects(SourceRemoveUnused.kind)) {
return undefined; fixes.push(new SourceRemoveUnused());
} }
return { edit, fixedDiagnostics }; if (only.intersects(SourceAddMissingImports.kind)) {
} fixes.push(new SourceAddMissingImports());
}
private getAutoFixes(): readonly AutoFixableError[] { return fixes;
return [
fixImplementInterface,
fixUnreachable,
fixAsync,
];
} }
} }
@ -153,7 +259,8 @@ export function register(
selector: vscode.DocumentSelector, selector: vscode.DocumentSelector,
client: ITypeScriptServiceClient, client: ITypeScriptServiceClient,
fileConfigurationManager: FileConfigurationManager, fileConfigurationManager: FileConfigurationManager,
diagnosticsManager: DiagnosticsManager) { diagnosticsManager: DiagnosticsManager,
) {
return new VersionDependentRegistration(client, API.v300, () => return new VersionDependentRegistration(client, API.v300, () =>
vscode.languages.registerCodeActionsProvider(selector, vscode.languages.registerCodeActionsProvider(selector,
new TypeScriptAutoFixProvider(client, fileConfigurationManager, diagnosticsManager), new TypeScriptAutoFixProvider(client, fileConfigurationManager, diagnosticsManager),

View file

@ -23,7 +23,7 @@ suite('TypeScript Fix All', () => {
await vscode.commands.executeCommand('workbench.action.closeAllEditors'); await vscode.commands.executeCommand('workbench.action.closeAllEditors');
}); });
test('Should fix unreachable code', async () => { test('Fix all should remove unreachable code', async () => {
const editor = await createTestEditor(testDocumentUri, const editor = await createTestEditor(testDocumentUri,
`function foo() {`, `function foo() {`,
` return 1;`, ` return 1;`,
@ -35,16 +35,16 @@ suite('TypeScript Fix All', () => {
`};`, `};`,
); );
await wait(2500); await wait(2000);
const fixes = await vscode.commands.executeCommand<vscode.CodeAction[]>('vscode.executeCodeActionProvider', const fixes = await vscode.commands.executeCommand<vscode.CodeAction[]>('vscode.executeCodeActionProvider',
testDocumentUri, testDocumentUri,
emptyRange, emptyRange,
vscode.CodeActionKind.SourceFixAll vscode.CodeActionKind.SourceFixAll
); );
assert.strictEqual(fixes?.length, 1);
await vscode.workspace.applyEdit(fixes![0].edit!); await vscode.workspace.applyEdit(fixes![0].edit!);
assert.strictEqual(editor.document.getText(), joinLines( assert.strictEqual(editor.document.getText(), joinLines(
`function foo() {`, `function foo() {`,
` return 1;`, ` return 1;`,
@ -53,9 +53,10 @@ suite('TypeScript Fix All', () => {
` return 3;`, ` return 3;`,
`};`, `};`,
)); ));
}); });
test('Should implement interfaces', async () => { test('Fix all should implement interfaces', async () => {
const editor = await createTestEditor(testDocumentUri, const editor = await createTestEditor(testDocumentUri,
`interface I {`, `interface I {`,
` x: number;`, ` x: number;`,
@ -64,14 +65,13 @@ suite('TypeScript Fix All', () => {
`class B implements I {}`, `class B implements I {}`,
); );
await wait(2500); await wait(2000);
const fixes = await vscode.commands.executeCommand<vscode.CodeAction[]>('vscode.executeCodeActionProvider', const fixes = await vscode.commands.executeCommand<vscode.CodeAction[]>('vscode.executeCodeActionProvider',
testDocumentUri, testDocumentUri,
emptyRange, emptyRange,
vscode.CodeActionKind.SourceFixAll vscode.CodeActionKind.SourceFixAll
); );
assert.strictEqual(fixes?.length, 1);
await vscode.workspace.applyEdit(fixes![0].edit!); await vscode.workspace.applyEdit(fixes![0].edit!);
assert.strictEqual(editor.document.getText(), joinLines( assert.strictEqual(editor.document.getText(), joinLines(
@ -86,4 +86,33 @@ suite('TypeScript Fix All', () => {
`}`, `}`,
)); ));
}); });
test('Remove unused should handle nested ununused', async () => {
const editor = await createTestEditor(testDocumentUri,
`export const _ = 1;`,
`function unused() {`,
` const a = 1;`,
`}`,
`function used() {`,
` const a = 1;`,
`}`,
`used();`
);
await wait(2000);
const fixes = await vscode.commands.executeCommand<vscode.CodeAction[]>('vscode.executeCodeActionProvider',
testDocumentUri,
emptyRange,
vscode.CodeActionKind.Source.append('removeUnused')
);
await vscode.workspace.applyEdit(fixes![0].edit!);
assert.strictEqual(editor.document.getText(), joinLines(
`export const _ = 1;`,
`function used() {`,
`}`,
`used();`
));
});
}); });

View file

@ -11,6 +11,6 @@ export const unusedLabel = 7028;
export const fallThroughCaseInSwitch = 7029; export const fallThroughCaseInSwitch = 7029;
export const notAllCodePathsReturnAValue = 7030; export const notAllCodePathsReturnAValue = 7030;
export const incorrectlyImplementsInterface = 2420; export const incorrectlyImplementsInterface = 2420;
export const cannotFindName = 2552; export const cannotFindName = [2552, 2304];
export const extendsShouldBeImplements = 2689; export const extendsShouldBeImplements = 2689;
export const asyncOnlyAllowedInAsyncFunctions = 1308; export const asyncOnlyAllowedInAsyncFunctions = 1308;

View file

@ -12,4 +12,5 @@ export const unreachableCode = 'fixUnreachableCode';
export const unusedIdentifier = 'unusedIdentifier'; export const unusedIdentifier = 'unusedIdentifier';
export const forgottenThisPropertyAccess = 'forgottenThisPropertyAccess'; export const forgottenThisPropertyAccess = 'forgottenThisPropertyAccess';
export const spelling = 'spelling'; export const spelling = 'spelling';
export const fixImport = 'import';
export const addMissingAwait = 'addMissingAwait'; export const addMissingAwait = 'addMissingAwait';