diff --git a/extensions/typescript-language-features/src/features/fixAll.ts b/extensions/typescript-language-features/src/features/fixAll.ts index f52b7f76071..256173ec73e 100644 --- a/extensions/typescript-language-features/src/features/fixAll.ts +++ b/extensions/typescript-language-features/src/features/fixAll.ts @@ -9,30 +9,187 @@ import type * as Proto from '../protocol'; import { ITypeScriptServiceClient } from '../typescriptService'; import API from '../utils/api'; import { VersionDependentRegistration } from '../utils/dependentRegistration'; +import * as errorCodes from '../utils/errorCodes'; +import * as fixNames from '../utils/fixNames'; import * as typeConverters from '../utils/typeConverters'; import { DiagnosticsManager } from './diagnostics'; import FileConfigurationManager from './fileConfigurationManager'; -import * as errorCodes from '../utils/errorCodes'; -import * as fixNames from '../utils/fixNames'; const localize = nls.loadMessageBundle(); -interface AutoFixableError { +interface AutoFix { readonly code: number; readonly fixName: string; } -const fixImplementInterface = Object.freeze({ code: errorCodes.incorrectlyImplementsInterface, fixName: fixNames.classIncorrectlyImplementsInterface }); -const fixUnreachable = Object.freeze({ code: errorCodes.unreachableCode, fixName: fixNames.unreachableCode }); -const fixAsync = Object.freeze({ code: errorCodes.asyncOnlyAllowedInAsyncFunctions, fixName: fixNames.awaitInSyncFunction }); +async function buildIndividualFixes( + fixes: readonly AutoFix[], + edit: vscode.WorkspaceEdit, + client: ITypeScriptServiceClient, + file: string, + diagnostics: readonly vscode.Diagnostic[], + token: vscode.CancellationToken, +): Promise { + 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 { + 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; +} + +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 { + 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 { + 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 { + 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 { - private static readonly fixAllKind = vscode.CodeActionKind.SourceFixAll.append('ts'); - public static readonly metadata: vscode.CodeActionProviderMetadata = { providedCodeActionKinds: [ - TypeScriptAutoFixProvider.fixAllKind, + SourceFixAll.kind, + SourceRemoveUnused.kind, + SourceAddMissingImports.kind, ] }; @@ -48,7 +205,7 @@ class TypeScriptAutoFixProvider implements vscode.CodeActionProvider { context: vscode.CodeActionContext, token: vscode.CancellationToken ): Promise { - if (!context.only || !vscode.CodeActionKind.SourceFixAll.intersects(context.only)) { + if (!context.only || !vscode.CodeActionKind.Source.intersects(context.only)) { return undefined; } @@ -57,95 +214,44 @@ class TypeScriptAutoFixProvider implements vscode.CodeActionProvider { return undefined; } - const autoFixes = this.getAutoFixes(); - - 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[] { + const actions = this.getFixAllActions(context.only); if (this.client.bufferSyncSupport.hasPendingDiagnostics(document.uri)) { - return []; + return actions; } - const fixableCodes = new Set(autoFixes.map(x => x.code)); - return this.diagnosticsManager.getDiagnostics(document.uri) - .filter(x => fixableCodes.has(+(x.code as number))); - } + const diagnostics = this.diagnosticsManager.getDiagnostics(document.uri); + if (!diagnostics.length) { + // 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, - autoFixes: readonly AutoFixableError[], - token: vscode.CancellationToken, - ): Promise { await this.fileConfigurationManager.ensureConfigurationForDocument(document, token); - const autoFixResponse = await this.getAutoFixEdit(file, diagnostics, autoFixes, token); - if (!autoFixResponse) { - return undefined; - } - const { edit, fixedDiagnostics } = autoFixResponse; - if (!edit.size) { + if (token.isCancellationRequested) { return undefined; } - const codeAction = new vscode.CodeAction( - localize('autoFix.label', 'Auto fix'), - TypeScriptAutoFixProvider.fixAllKind); - codeAction.edit = edit; - codeAction.diagnostics = fixedDiagnostics; + await Promise.all(actions.map(action => action.build(this.client, file, diagnostics, token))); - return codeAction; + return actions; } - private async getAutoFixEdit( - file: string, - diagnostics: ReadonlyArray, - 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; - } + private getFixAllActions(only: vscode.CodeActionKind): SourceAction[] { + const fixes = []; - const fix = response.body[0]; - if (autoFixes.some(autoFix => autoFix.fixName.includes(fix.fixName))) { - typeConverters.WorkspaceEdit.withFileCodeEdits(edit, this.client, fix.changes); - fixedDiagnostics.push(diagnostic); - } + if (only.intersects(SourceFixAll.kind)) { + fixes.push(new SourceFixAll()); } - if (!fixedDiagnostics.length) { - return undefined; + if (only.intersects(SourceRemoveUnused.kind)) { + fixes.push(new SourceRemoveUnused()); } - return { edit, fixedDiagnostics }; - } + if (only.intersects(SourceAddMissingImports.kind)) { + fixes.push(new SourceAddMissingImports()); + } - private getAutoFixes(): readonly AutoFixableError[] { - return [ - fixImplementInterface, - fixUnreachable, - fixAsync, - ]; + return fixes; } } @@ -153,7 +259,8 @@ export function register( selector: vscode.DocumentSelector, client: ITypeScriptServiceClient, fileConfigurationManager: FileConfigurationManager, - diagnosticsManager: DiagnosticsManager) { + diagnosticsManager: DiagnosticsManager, +) { return new VersionDependentRegistration(client, API.v300, () => vscode.languages.registerCodeActionsProvider(selector, new TypeScriptAutoFixProvider(client, fileConfigurationManager, diagnosticsManager), diff --git a/extensions/typescript-language-features/src/test/fixAll.test.ts b/extensions/typescript-language-features/src/test/fixAll.test.ts index eb5448a1f90..0675c2f16cf 100644 --- a/extensions/typescript-language-features/src/test/fixAll.test.ts +++ b/extensions/typescript-language-features/src/test/fixAll.test.ts @@ -23,7 +23,7 @@ suite('TypeScript Fix All', () => { 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, `function foo() {`, ` return 1;`, @@ -35,16 +35,16 @@ suite('TypeScript Fix All', () => { `};`, ); - await wait(2500); + await wait(2000); const fixes = await vscode.commands.executeCommand('vscode.executeCodeActionProvider', testDocumentUri, emptyRange, vscode.CodeActionKind.SourceFixAll ); - assert.strictEqual(fixes?.length, 1); await vscode.workspace.applyEdit(fixes![0].edit!); + assert.strictEqual(editor.document.getText(), joinLines( `function foo() {`, ` return 1;`, @@ -53,9 +53,10 @@ suite('TypeScript Fix All', () => { ` return 3;`, `};`, )); + }); - test('Should implement interfaces', async () => { + test('Fix all should implement interfaces', async () => { const editor = await createTestEditor(testDocumentUri, `interface I {`, ` x: number;`, @@ -64,14 +65,13 @@ suite('TypeScript Fix All', () => { `class B implements I {}`, ); - await wait(2500); + await wait(2000); const fixes = await vscode.commands.executeCommand('vscode.executeCodeActionProvider', testDocumentUri, emptyRange, vscode.CodeActionKind.SourceFixAll ); - assert.strictEqual(fixes?.length, 1); await vscode.workspace.applyEdit(fixes![0].edit!); 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.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();` + )); + }); }); diff --git a/extensions/typescript-language-features/src/utils/errorCodes.ts b/extensions/typescript-language-features/src/utils/errorCodes.ts index fa29edc6e4f..1d066d9d196 100644 --- a/extensions/typescript-language-features/src/utils/errorCodes.ts +++ b/extensions/typescript-language-features/src/utils/errorCodes.ts @@ -11,6 +11,6 @@ export const unusedLabel = 7028; export const fallThroughCaseInSwitch = 7029; export const notAllCodePathsReturnAValue = 7030; export const incorrectlyImplementsInterface = 2420; -export const cannotFindName = 2552; +export const cannotFindName = [2552, 2304]; export const extendsShouldBeImplements = 2689; export const asyncOnlyAllowedInAsyncFunctions = 1308; diff --git a/extensions/typescript-language-features/src/utils/fixNames.ts b/extensions/typescript-language-features/src/utils/fixNames.ts index f14dbdbe6ac..0bda0c3bd90 100644 --- a/extensions/typescript-language-features/src/utils/fixNames.ts +++ b/extensions/typescript-language-features/src/utils/fixNames.ts @@ -12,4 +12,5 @@ export const unreachableCode = 'fixUnreachableCode'; export const unusedIdentifier = 'unusedIdentifier'; export const forgottenThisPropertyAccess = 'forgottenThisPropertyAccess'; export const spelling = 'spelling'; +export const fixImport = 'import'; export const addMissingAwait = 'addMissingAwait';