From 39fb3b1065c5a8ee9cb8b0b94860d449672ed4cc Mon Sep 17 00:00:00 2001 From: Matt Bierner Date: Thu, 14 May 2020 22:28:20 -0700 Subject: [PATCH] Mark add missing imports as preferred fixes Allow auto fixing add missing imports if: - There is only one possible import - And there are no better fixes (such as spelling changes) --- .vscode/launch.json | 2 +- build/gulpfile.hygiene.js | 1 + .../src/features/quickFix.ts | 45 ++++++++------ .../src/test/quickFix.test.ts | 59 ++++++++++++++++++- .../test-workspace/bar.ts | 1 + .../test-workspace/foo.ts | 0 .../test-workspace/index.ts | 0 .../test-workspace/tsconfig.json | 5 ++ 8 files changed, 92 insertions(+), 21 deletions(-) create mode 100644 extensions/typescript-language-features/test-workspace/bar.ts create mode 100644 extensions/typescript-language-features/test-workspace/foo.ts create mode 100644 extensions/typescript-language-features/test-workspace/index.ts create mode 100644 extensions/typescript-language-features/test-workspace/tsconfig.json diff --git a/.vscode/launch.json b/.vscode/launch.json index a4bf62bf947..3fe3be010e9 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -310,7 +310,7 @@ "name": "TypeScript Extension Tests", "runtimeExecutable": "${execPath}", "args": [ - "${workspaceFolder}/extensions/typescript-language-features/test-fixtures", + "${workspaceFolder}/extensions/typescript-language-features/test-workspace", "--extensionDevelopmentPath=${workspaceFolder}/extensions/typescript-language-features", "--extensionTestsPath=${workspaceFolder}/extensions/typescript-language-features/out/test" ], diff --git a/build/gulpfile.hygiene.js b/build/gulpfile.hygiene.js index 75c8413ae5d..11172aa3362 100644 --- a/build/gulpfile.hygiene.js +++ b/build/gulpfile.hygiene.js @@ -59,6 +59,7 @@ const indentationFilter = [ // except specific folders '!test/automation/out/**', '!test/smoke/out/**', + '!extensions/typescript-language-features/test-workspace/**', '!extensions/vscode-api-tests/testWorkspace/**', '!extensions/vscode-api-tests/testWorkspace2/**', '!build/monaco/**', diff --git a/extensions/typescript-language-features/src/features/quickFix.ts b/extensions/typescript-language-features/src/features/quickFix.ts index 4de63dc3e13..d739d53349f 100644 --- a/extensions/typescript-language-features/src/features/quickFix.ts +++ b/extensions/typescript-language-features/src/features/quickFix.ts @@ -333,17 +333,18 @@ const fixAllErrorCodes = new Map([ [2345, 2339], ]); -const preferredFixes = new Map([ - [fixNames.annotateWithTypeFromJSDoc, 0], - [fixNames.constructorForDerivedNeedSuperCall, 0], - [fixNames.extendsInterfaceBecomesImplements, 0], - [fixNames.awaitInSyncFunction, 0], - [fixNames.classIncorrectlyImplementsInterface, 1], - [fixNames.unreachableCode, 0], - [fixNames.unusedIdentifier, 0], - [fixNames.forgottenThisPropertyAccess, 0], - [fixNames.spelling, 1], - [fixNames.addMissingAwait, 0], +const preferredFixes = new Map([ + [fixNames.annotateWithTypeFromJSDoc, { value: 1 }], + [fixNames.constructorForDerivedNeedSuperCall, { value: 1 }], + [fixNames.extendsInterfaceBecomesImplements, { value: 1 }], + [fixNames.awaitInSyncFunction, { value: 1 }], + [fixNames.classIncorrectlyImplementsInterface, { value: 1 }], + [fixNames.unreachableCode, { value: 1 }], + [fixNames.unusedIdentifier, { value: 1 }], + [fixNames.forgottenThisPropertyAccess, { value: 1 }], + [fixNames.spelling, { value: 2 }], + [fixNames.addMissingAwait, { value: 1 }], + [fixNames.fixImport, { value: 0, thereCanOnlyBeOne: true }], ]); function isPreferredFix( @@ -354,20 +355,30 @@ function isPreferredFix( return false; } - const priority = preferredFixes.get(action.tsAction.fixName); - if (typeof priority === 'undefined') { + const fixPriority = preferredFixes.get(action.tsAction.fixName); + if (!fixPriority) { return false; } return allActions.every(otherAction => { - if (otherAction.tsAction === action.tsAction) { + if (otherAction === action) { return true; } - const otherPriority = preferredFixes.get(otherAction.tsAction.fixName); - if (typeof otherPriority === 'undefined') { + + if (otherAction.isFixAll) { return true; } - return priority >= otherPriority; + + const otherFixPriority = preferredFixes.get(otherAction.tsAction.fixName); + if (!otherFixPriority || otherFixPriority.value < fixPriority.value) { + return true; + } + + if (fixPriority.thereCanOnlyBeOne && action.tsAction.fixName === otherAction.tsAction.fixName) { + return false; + } + + return true; }); } diff --git a/extensions/typescript-language-features/src/test/quickFix.test.ts b/extensions/typescript-language-features/src/test/quickFix.test.ts index c1e591bf2ba..6895ad65cf8 100644 --- a/extensions/typescript-language-features/src/test/quickFix.test.ts +++ b/extensions/typescript-language-features/src/test/quickFix.test.ts @@ -5,13 +5,11 @@ import * as assert from 'assert'; import 'mocha'; +import { join } from 'path'; import * as vscode from 'vscode'; import { disposeAll } from '../utils/dispose'; import { createTestEditor, joinLines, wait } from './testUtils'; -const testDocumentUri = vscode.Uri.parse('untitled:test.ts'); - - suite('TypeScript Quick Fix', () => { const _disposables: vscode.Disposable[] = []; @@ -23,6 +21,8 @@ suite('TypeScript Quick Fix', () => { }); test('Fix all should not be marked as preferred #97866', async () => { + const testDocumentUri = vscode.Uri.parse('untitled:test.ts'); + const editor = await createTestEditor(testDocumentUri, `export const _ = 1;`, `const a$0 = 1;`, @@ -40,4 +40,57 @@ suite('TypeScript Quick Fix', () => { `const b = 2;`, )); }); + + test('Add import should be a preferred fix if there is only one possible import', async () => { + await createTestEditor(workspaceFile('foo.ts'), + `export const foo = 1;`); + + const editor = await createTestEditor(workspaceFile('index.ts'), + `export const _ = 1;`, + `foo$0;` + ); + + await wait(3000); + + await vscode.commands.executeCommand('editor.action.autoFix'); + + await wait(500); + + assert.strictEqual(editor.document.getText(), joinLines( + `import { foo } from "./foo";`, + ``, + `export const _ = 1;`, + `foo;` + )); + }); + + test('Add import should not be a preferred fix if are multiple possible imports', async () => { + await createTestEditor(workspaceFile('foo.ts'), + `export const foo = 1;`); + + await createTestEditor(workspaceFile('bar.ts'), + `export const foo = 1;`); + + const editor = await createTestEditor(workspaceFile('index.ts'), + `export const _ = 1;`, + `foo$0;` + ); + + await wait(3000); + + await vscode.commands.executeCommand('editor.action.autoFix'); + + await wait(500); + + assert.strictEqual(editor.document.getText(), joinLines( + `export const _ = 1;`, + `foo;` + )); + }); }); + + +function workspaceFile(fileName: string) { + return vscode.Uri.file(join(vscode.workspace.rootPath!, fileName)); +} + diff --git a/extensions/typescript-language-features/test-workspace/bar.ts b/extensions/typescript-language-features/test-workspace/bar.ts new file mode 100644 index 00000000000..4098e94951d --- /dev/null +++ b/extensions/typescript-language-features/test-workspace/bar.ts @@ -0,0 +1 @@ +// export const foo = 1; \ No newline at end of file diff --git a/extensions/typescript-language-features/test-workspace/foo.ts b/extensions/typescript-language-features/test-workspace/foo.ts new file mode 100644 index 00000000000..e69de29bb2d diff --git a/extensions/typescript-language-features/test-workspace/index.ts b/extensions/typescript-language-features/test-workspace/index.ts new file mode 100644 index 00000000000..e69de29bb2d diff --git a/extensions/typescript-language-features/test-workspace/tsconfig.json b/extensions/typescript-language-features/test-workspace/tsconfig.json new file mode 100644 index 00000000000..4935a9037fa --- /dev/null +++ b/extensions/typescript-language-features/test-workspace/tsconfig.json @@ -0,0 +1,5 @@ +{ + "compilerOptions": { + "target": "es2018" + } +}