From 64b595ea0e3e327ba226f7ee91b4fe1f94931639 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Thu, 26 Apr 2018 09:05:04 -0700 Subject: [PATCH 1/3] Add code fix to convert 'require' in a '.ts' file to an 'import' --- src/compiler/diagnosticMessages.json | 12 ++++++++ src/harness/fourslash.ts | 6 ++-- src/harness/tsconfig.json | 1 + src/server/tsconfig.json | 1 + src/server/tsconfig.library.json | 1 + src/services/codefixes/requireInTs.ts | 29 +++++++++++++++++++ src/services/suggestionDiagnostics.ts | 13 +++++++++ src/services/tsconfig.json | 1 + tests/cases/fourslash/codeFixRequireInTs.ts | 15 ++++++++++ .../cases/fourslash/codeFixRequireInTs_all.ts | 13 +++++++++ ...equireInTs_allowSyntheticDefaultImports.ts | 11 +++++++ 11 files changed, 101 insertions(+), 2 deletions(-) create mode 100644 src/services/codefixes/requireInTs.ts create mode 100644 tests/cases/fourslash/codeFixRequireInTs.ts create mode 100644 tests/cases/fourslash/codeFixRequireInTs_all.ts create mode 100644 tests/cases/fourslash/codeFixRequireInTs_allowSyntheticDefaultImports.ts diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index e81cd4fbc221a..6cc4e1ea1bcbd 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -3898,6 +3898,10 @@ "category": "Suggestion", "code": 80004 }, + "'require' call may be converted to an import.": { + "category": "Suggestion", + "code": 80005 + }, "Add missing 'super()' call": { "category": "Message", @@ -4174,5 +4178,13 @@ "Generate 'get' and 'set' accessors": { "category": "Message", "code": 95046 + }, + "Convert 'require' to 'import'": { + "category": "Message", + "code": 95047 + }, + "Convert all 'require' to 'import'": { + "category": "Message", + "code": 95048 } } diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index 7dfff7dd18995..9fa6dad39b354 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -324,8 +324,10 @@ namespace FourSlash { this.languageServiceAdapterHost.addScript(fileName, file, /*isRootFile*/ true); } }); - this.languageServiceAdapterHost.addScript(Harness.Compiler.defaultLibFileName, - Harness.Compiler.getDefaultLibrarySourceFile().text, /*isRootFile*/ false); + if (!compilationOptions.noLib) { + this.languageServiceAdapterHost.addScript(Harness.Compiler.defaultLibFileName, + Harness.Compiler.getDefaultLibrarySourceFile().text, /*isRootFile*/ false); + } } for (const file of testData.files) { diff --git a/src/harness/tsconfig.json b/src/harness/tsconfig.json index a4ad8cb37975f..faeff1d4bcf1c 100644 --- a/src/harness/tsconfig.json +++ b/src/harness/tsconfig.json @@ -112,6 +112,7 @@ "../services/codefixes/inferFromUsage.ts", "../services/codefixes/fixInvalidImportSyntax.ts", "../services/codefixes/fixStrictClassInitialization.ts", + "../services/codefixes/requireInTs.ts", "../services/codefixes/useDefaultImport.ts", "../services/refactors/extractSymbol.ts", "../services/refactors/generateGetAccessorAndSetAccessor.ts", diff --git a/src/server/tsconfig.json b/src/server/tsconfig.json index e6768f4edeed1..71ff00e6ac1ab 100644 --- a/src/server/tsconfig.json +++ b/src/server/tsconfig.json @@ -108,6 +108,7 @@ "../services/codefixes/inferFromUsage.ts", "../services/codefixes/fixInvalidImportSyntax.ts", "../services/codefixes/fixStrictClassInitialization.ts", + "../services/codefixes/requireInTs.ts", "../services/codefixes/useDefaultImport.ts", "../services/refactors/extractSymbol.ts", "../services/refactors/generateGetAccessorAndSetAccessor.ts", diff --git a/src/server/tsconfig.library.json b/src/server/tsconfig.library.json index a167d3b39a2ff..95489ff0ea357 100644 --- a/src/server/tsconfig.library.json +++ b/src/server/tsconfig.library.json @@ -114,6 +114,7 @@ "../services/codefixes/inferFromUsage.ts", "../services/codefixes/fixInvalidImportSyntax.ts", "../services/codefixes/fixStrictClassInitialization.ts", + "../services/codefixes/requireInTs.ts", "../services/codefixes/useDefaultImport.ts", "../services/refactors/extractSymbol.ts", "../services/refactors/generateGetAccessorAndSetAccessor.ts", diff --git a/src/services/codefixes/requireInTs.ts b/src/services/codefixes/requireInTs.ts new file mode 100644 index 0000000000000..7efd9bd7fa6fc --- /dev/null +++ b/src/services/codefixes/requireInTs.ts @@ -0,0 +1,29 @@ +/* @internal */ +namespace ts.codefix { + const fixId = "requireInTs"; + const errorCodes = [Diagnostics.require_call_may_be_converted_to_an_import.code]; + registerCodeFix({ + errorCodes, + getCodeActions(context) { + const changes = textChanges.ChangeTracker.with(context, t => doChange(t, context.sourceFile, context.span.start, context.program)); + return [createCodeFixAction(fixId, changes, Diagnostics.Convert_require_to_import, fixId, Diagnostics.Convert_all_require_to_import)]; + }, + fixIds: [fixId], + getAllCodeActions: context => codeFixAll(context, errorCodes, (changes, diag) => doChange(changes, diag.file, diag.start, context.program)), + }); + + function doChange(changes: textChanges.ChangeTracker, sourceFile: SourceFile, pos: number, program: Program) { + const { statement, name, required } = getInfo(sourceFile, pos); + changes.replaceNode(sourceFile, statement, getAllowSyntheticDefaultImports(program.getCompilerOptions()) + ? createImportDeclaration(/*decorators*/ undefined, /*modifiers*/ undefined, createImportClause(name, /*namedBindings*/ undefined), required) + : createImportEqualsDeclaration(/*decorators*/ undefined, /*modifiers*/ undefined, name, createExternalModuleReference(required))); + } + + interface Info { readonly statement: VariableStatement; readonly name: Identifier; readonly required: StringLiteralLike; } + function getInfo(sourceFile: SourceFile, pos: number): Info { + const { parent } = getTokenAtPosition(sourceFile, pos, /*includeJsDocComment*/ false); + if (!isRequireCall(parent, /*checkArgumentIsStringLiteralLike*/ true)) throw Debug.failBadSyntaxKind(parent); + const decl = cast(parent.parent, isVariableDeclaration); + return { statement: cast(decl.parent.parent, isVariableStatement), name: cast(decl.name, isIdentifier), required: parent.arguments[0] }; + } +} diff --git a/src/services/suggestionDiagnostics.ts b/src/services/suggestionDiagnostics.ts index d04f3f15cf909..0c8ec58798c94 100644 --- a/src/services/suggestionDiagnostics.ts +++ b/src/services/suggestionDiagnostics.ts @@ -32,6 +32,19 @@ namespace ts { } check(sourceFile); + if (!isJsFile) { + for (const statement of sourceFile.statements) { + if (isVariableStatement(statement) && + statement.declarationList.flags & NodeFlags.Const && + statement.declarationList.declarations.length === 1) { + const init = statement.declarationList.declarations[0].initializer; + if (init && isRequireCall(init, /*checkArgumentIsStringLiteralLike*/ true)) { + diags.push(createDiagnosticForNode(init, Diagnostics.require_call_may_be_converted_to_an_import)); + } + } + } + } + if (getAllowSyntheticDefaultImports(program.getCompilerOptions())) { for (const moduleSpecifier of sourceFile.imports) { const importNode = importFromModuleSpecifier(moduleSpecifier); diff --git a/src/services/tsconfig.json b/src/services/tsconfig.json index 75b46a5c49bbe..be46c21e06b12 100644 --- a/src/services/tsconfig.json +++ b/src/services/tsconfig.json @@ -105,6 +105,7 @@ "codefixes/inferFromUsage.ts", "codefixes/fixInvalidImportSyntax.ts", "codefixes/fixStrictClassInitialization.ts", + "codefixes/requireInTs.ts", "codefixes/useDefaultImport.ts", "refactors/extractSymbol.ts", "refactors/generateGetAccessorAndSetAccessor.ts", diff --git a/tests/cases/fourslash/codeFixRequireInTs.ts b/tests/cases/fourslash/codeFixRequireInTs.ts new file mode 100644 index 0000000000000..345b15f69f250 --- /dev/null +++ b/tests/cases/fourslash/codeFixRequireInTs.ts @@ -0,0 +1,15 @@ +/// + +// @Filename: /a.ts +////const a = [|require("a")|]; + +verify.getSuggestionDiagnostics([{ + message: "'require' call may be converted to an import.", + code: 80005, +}]); + +verify.codeFix({ + description: "Convert 'require' to 'import'", + newFileContent: +`import a = require("a");`, +}); diff --git a/tests/cases/fourslash/codeFixRequireInTs_all.ts b/tests/cases/fourslash/codeFixRequireInTs_all.ts new file mode 100644 index 0000000000000..9f9fe4824a8f5 --- /dev/null +++ b/tests/cases/fourslash/codeFixRequireInTs_all.ts @@ -0,0 +1,13 @@ +/// + +// @Filename: /a.ts +////const a = [|require("a")|]; +////const b = [|require("b")|]; + +verify.codeFixAll({ + fixId: "requireInTs", + fixAllDescription: "Convert all 'require' to 'import'", + newFileContent: +`import a = require("a"); +import b = require("b");`, +}); diff --git a/tests/cases/fourslash/codeFixRequireInTs_allowSyntheticDefaultImports.ts b/tests/cases/fourslash/codeFixRequireInTs_allowSyntheticDefaultImports.ts new file mode 100644 index 0000000000000..949387af2e542 --- /dev/null +++ b/tests/cases/fourslash/codeFixRequireInTs_allowSyntheticDefaultImports.ts @@ -0,0 +1,11 @@ +/// + +// @allowSyntheticDefaultImports: true + +// @Filename: /a.ts +////const a = [|require("a")|]; + +verify.codeFix({ + description: "Convert 'require' to 'import'", + newFileContent: `import a from "a";`, +}); From b1a728fdacacf7089e8995f6a5caddac27ef5e62 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Mon, 30 Apr 2018 10:11:52 -0700 Subject: [PATCH 2/3] Only add suggestion for modules --- src/services/suggestionDiagnostics.ts | 2 +- tests/cases/fourslash/codeFixRequireInTs.ts | 8 ++++++-- tests/cases/fourslash/codeFixRequireInTs_all.ts | 6 ++++-- .../codeFixRequireInTs_allowSyntheticDefaultImports.ts | 9 +++++++-- tests/cases/fourslash/codeFixRequireInTs_onlyIfModule.ts | 5 +++++ 5 files changed, 23 insertions(+), 7 deletions(-) create mode 100644 tests/cases/fourslash/codeFixRequireInTs_onlyIfModule.ts diff --git a/src/services/suggestionDiagnostics.ts b/src/services/suggestionDiagnostics.ts index 0c8ec58798c94..8bc363169086b 100644 --- a/src/services/suggestionDiagnostics.ts +++ b/src/services/suggestionDiagnostics.ts @@ -32,7 +32,7 @@ namespace ts { } check(sourceFile); - if (!isJsFile) { + if (!isJsFile && sourceFile.externalModuleIndicator) { for (const statement of sourceFile.statements) { if (isVariableStatement(statement) && statement.declarationList.flags & NodeFlags.Const && diff --git a/tests/cases/fourslash/codeFixRequireInTs.ts b/tests/cases/fourslash/codeFixRequireInTs.ts index 345b15f69f250..176a01ebe4aeb 100644 --- a/tests/cases/fourslash/codeFixRequireInTs.ts +++ b/tests/cases/fourslash/codeFixRequireInTs.ts @@ -1,7 +1,8 @@ /// -// @Filename: /a.ts +////export {}; ////const a = [|require("a")|]; +////a; verify.getSuggestionDiagnostics([{ message: "'require' call may be converted to an import.", @@ -11,5 +12,8 @@ verify.getSuggestionDiagnostics([{ verify.codeFix({ description: "Convert 'require' to 'import'", newFileContent: -`import a = require("a");`, +// TODO: GH#23781 +`export {}; + import a = require("a"); +a;`, }); diff --git a/tests/cases/fourslash/codeFixRequireInTs_all.ts b/tests/cases/fourslash/codeFixRequireInTs_all.ts index 9f9fe4824a8f5..a23215cb3643b 100644 --- a/tests/cases/fourslash/codeFixRequireInTs_all.ts +++ b/tests/cases/fourslash/codeFixRequireInTs_all.ts @@ -1,6 +1,6 @@ /// -// @Filename: /a.ts +////export {}; ////const a = [|require("a")|]; ////const b = [|require("b")|]; @@ -8,6 +8,8 @@ verify.codeFixAll({ fixId: "requireInTs", fixAllDescription: "Convert all 'require' to 'import'", newFileContent: -`import a = require("a"); +// TODO: GH#23781 +`export {}; + import a = require("a"); import b = require("b");`, }); diff --git a/tests/cases/fourslash/codeFixRequireInTs_allowSyntheticDefaultImports.ts b/tests/cases/fourslash/codeFixRequireInTs_allowSyntheticDefaultImports.ts index 949387af2e542..634d701215c80 100644 --- a/tests/cases/fourslash/codeFixRequireInTs_allowSyntheticDefaultImports.ts +++ b/tests/cases/fourslash/codeFixRequireInTs_allowSyntheticDefaultImports.ts @@ -2,10 +2,15 @@ // @allowSyntheticDefaultImports: true -// @Filename: /a.ts +////export {}; ////const a = [|require("a")|]; +////a; verify.codeFix({ description: "Convert 'require' to 'import'", - newFileContent: `import a from "a";`, + newFileContent: +// TODO: GH#23781 +`export {}; + import a from "a"; +a;`, }); diff --git a/tests/cases/fourslash/codeFixRequireInTs_onlyIfModule.ts b/tests/cases/fourslash/codeFixRequireInTs_onlyIfModule.ts new file mode 100644 index 0000000000000..b3c923c356c34 --- /dev/null +++ b/tests/cases/fourslash/codeFixRequireInTs_onlyIfModule.ts @@ -0,0 +1,5 @@ +/// + +////const a = [|require("a")|]; + +verify.getSuggestionDiagnostics([]); From 51a4d9c494b7f5bf65ebae28caa858e1d0519bc4 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Tue, 1 May 2018 08:14:24 -0700 Subject: [PATCH 3/3] Revert "Only add suggestion for modules" This reverts commit b1a728fdacacf7089e8995f6a5caddac27ef5e62. --- src/services/suggestionDiagnostics.ts | 2 +- tests/cases/fourslash/codeFixRequireInTs.ts | 8 ++------ tests/cases/fourslash/codeFixRequireInTs_all.ts | 6 ++---- .../codeFixRequireInTs_allowSyntheticDefaultImports.ts | 9 ++------- tests/cases/fourslash/codeFixRequireInTs_onlyIfModule.ts | 5 ----- 5 files changed, 7 insertions(+), 23 deletions(-) delete mode 100644 tests/cases/fourslash/codeFixRequireInTs_onlyIfModule.ts diff --git a/src/services/suggestionDiagnostics.ts b/src/services/suggestionDiagnostics.ts index 8bc363169086b..0c8ec58798c94 100644 --- a/src/services/suggestionDiagnostics.ts +++ b/src/services/suggestionDiagnostics.ts @@ -32,7 +32,7 @@ namespace ts { } check(sourceFile); - if (!isJsFile && sourceFile.externalModuleIndicator) { + if (!isJsFile) { for (const statement of sourceFile.statements) { if (isVariableStatement(statement) && statement.declarationList.flags & NodeFlags.Const && diff --git a/tests/cases/fourslash/codeFixRequireInTs.ts b/tests/cases/fourslash/codeFixRequireInTs.ts index 176a01ebe4aeb..345b15f69f250 100644 --- a/tests/cases/fourslash/codeFixRequireInTs.ts +++ b/tests/cases/fourslash/codeFixRequireInTs.ts @@ -1,8 +1,7 @@ /// -////export {}; +// @Filename: /a.ts ////const a = [|require("a")|]; -////a; verify.getSuggestionDiagnostics([{ message: "'require' call may be converted to an import.", @@ -12,8 +11,5 @@ verify.getSuggestionDiagnostics([{ verify.codeFix({ description: "Convert 'require' to 'import'", newFileContent: -// TODO: GH#23781 -`export {}; - import a = require("a"); -a;`, +`import a = require("a");`, }); diff --git a/tests/cases/fourslash/codeFixRequireInTs_all.ts b/tests/cases/fourslash/codeFixRequireInTs_all.ts index a23215cb3643b..9f9fe4824a8f5 100644 --- a/tests/cases/fourslash/codeFixRequireInTs_all.ts +++ b/tests/cases/fourslash/codeFixRequireInTs_all.ts @@ -1,6 +1,6 @@ /// -////export {}; +// @Filename: /a.ts ////const a = [|require("a")|]; ////const b = [|require("b")|]; @@ -8,8 +8,6 @@ verify.codeFixAll({ fixId: "requireInTs", fixAllDescription: "Convert all 'require' to 'import'", newFileContent: -// TODO: GH#23781 -`export {}; - import a = require("a"); +`import a = require("a"); import b = require("b");`, }); diff --git a/tests/cases/fourslash/codeFixRequireInTs_allowSyntheticDefaultImports.ts b/tests/cases/fourslash/codeFixRequireInTs_allowSyntheticDefaultImports.ts index 634d701215c80..949387af2e542 100644 --- a/tests/cases/fourslash/codeFixRequireInTs_allowSyntheticDefaultImports.ts +++ b/tests/cases/fourslash/codeFixRequireInTs_allowSyntheticDefaultImports.ts @@ -2,15 +2,10 @@ // @allowSyntheticDefaultImports: true -////export {}; +// @Filename: /a.ts ////const a = [|require("a")|]; -////a; verify.codeFix({ description: "Convert 'require' to 'import'", - newFileContent: -// TODO: GH#23781 -`export {}; - import a from "a"; -a;`, + newFileContent: `import a from "a";`, }); diff --git a/tests/cases/fourslash/codeFixRequireInTs_onlyIfModule.ts b/tests/cases/fourslash/codeFixRequireInTs_onlyIfModule.ts deleted file mode 100644 index b3c923c356c34..0000000000000 --- a/tests/cases/fourslash/codeFixRequireInTs_onlyIfModule.ts +++ /dev/null @@ -1,5 +0,0 @@ -/// - -////const a = [|require("a")|]; - -verify.getSuggestionDiagnostics([]);