From 84509012f3857929e5dec4463558c67b3d5d6e31 Mon Sep 17 00:00:00 2001 From: TypeScript Bot Date: Wed, 19 Jan 2022 15:05:16 -0800 Subject: [PATCH] Cherry-pick PR #47433 into release-4.5 (#47515) Component commits: fce282b5bd Add failing test. ea2c290ba8 Update failing test. d954948394 Finalized failing test case. f476e8426b Separate our usages of utilities that expect variables initialized to require(...) and require(...).foo. 9f0810c26d Renamed types and utilities, removed accidental indentation. bf708bfde7 Renamed both utilitiy functions uniformly. Co-authored-by: Daniel Rosenwasser --- src/compiler/binder.ts | 2 +- src/compiler/checker.ts | 4 ++-- src/compiler/types.ts | 8 +++---- src/compiler/utilities.ts | 21 +++++++++++++++---- src/services/codefixes/importFixes.ts | 2 +- src/services/findAllReferences.ts | 2 +- src/services/goToDefinition.ts | 2 +- src/services/importTracker.ts | 2 +- .../importFixesWithExistingDottedRequire.ts | 21 +++++++++++++++++++ 9 files changed, 49 insertions(+), 15 deletions(-) create mode 100644 tests/cases/fourslash/importFixesWithExistingDottedRequire.ts diff --git a/src/compiler/binder.ts b/src/compiler/binder.ts index 8ff6712ad30ac..5d09f069770fe 100644 --- a/src/compiler/binder.ts +++ b/src/compiler/binder.ts @@ -3288,7 +3288,7 @@ namespace ts { } if (!isBindingPattern(node.name)) { - if (isInJSFile(node) && isRequireVariableDeclaration(node) && !getJSDocTypeTag(node)) { + if (isInJSFile(node) && isVariableDeclarationInitializedToBareOrAccessedRequire(node) && !getJSDocTypeTag(node)) { declareSymbolAndAddToSymbolTable(node as Declaration, SymbolFlags.Alias, SymbolFlags.AliasExcludes); } else if (isBlockOrCatchScoped(node)) { diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 5c965a813a5b1..e504553233140 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -2592,7 +2592,7 @@ namespace ts { && isAliasableOrJsExpression(node.parent.right) || node.kind === SyntaxKind.ShorthandPropertyAssignment || node.kind === SyntaxKind.PropertyAssignment && isAliasableOrJsExpression((node as PropertyAssignment).initializer) - || isRequireVariableDeclaration(node); + || isVariableDeclarationInitializedToBareOrAccessedRequire(node); } function isAliasableOrJsExpression(e: Expression) { @@ -36831,7 +36831,7 @@ namespace ts { } // For a commonjs `const x = require`, validate the alias and exit const symbol = getSymbolOfNode(node); - if (symbol.flags & SymbolFlags.Alias && isRequireVariableDeclaration(node)) { + if (symbol.flags & SymbolFlags.Alias && isVariableDeclarationInitializedToBareOrAccessedRequire(node)) { checkAliasSymbol(node); return; } diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 92225facae6c6..86d1e17c10d9b 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -4637,7 +4637,7 @@ namespace ts { export type AnyImportSyntax = ImportDeclaration | ImportEqualsDeclaration; /* @internal */ - export type AnyImportOrRequire = AnyImportSyntax | RequireVariableDeclaration; + export type AnyImportOrRequire = AnyImportSyntax | VariableDeclarationInitializedTo; /* @internal */ export type AnyImportOrRequireStatement = AnyImportSyntax | RequireVariableStatement; @@ -4662,8 +4662,8 @@ namespace ts { export type RequireOrImportCall = CallExpression & { expression: Identifier, arguments: [StringLiteralLike] }; /* @internal */ - export interface RequireVariableDeclaration extends VariableDeclaration { - readonly initializer: RequireOrImportCall; + export interface VariableDeclarationInitializedTo extends VariableDeclaration { + readonly initializer: T; } /* @internal */ @@ -4673,7 +4673,7 @@ namespace ts { /* @internal */ export interface RequireVariableDeclarationList extends VariableDeclarationList { - readonly declarations: NodeArray; + readonly declarations: NodeArray>; } /* @internal */ diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 66975e89ad052..0e84f0b47a25f 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -2046,7 +2046,7 @@ namespace ts { } export function getExternalModuleRequireArgument(node: Node) { - return isRequireVariableDeclaration(node) && (getLeftmostAccessExpression(node.initializer) as CallExpression).arguments[0] as StringLiteral; + return isVariableDeclarationInitializedToBareOrAccessedRequire(node) && (getLeftmostAccessExpression(node.initializer) as CallExpression).arguments[0] as StringLiteral; } export function isInternalModuleImportEqualsDeclaration(node: Node): node is ImportEqualsDeclaration { @@ -2113,17 +2113,30 @@ namespace ts { * Returns true if the node is a VariableDeclaration initialized to a require call (see `isRequireCall`). * This function does not test if the node is in a JavaScript file or not. */ - export function isRequireVariableDeclaration(node: Node): node is RequireVariableDeclaration { + export function isVariableDeclarationInitializedToRequire(node: Node): node is VariableDeclarationInitializedTo { + return isVariableDeclarationInitializedWithRequireHelper(node, /*allowAccessedRequire*/ false); + } + + /** + * Like {@link isVariableDeclarationInitializedToRequire} but allows things like `require("...").foo.bar` or `require("...")["baz"]`. + */ + export function isVariableDeclarationInitializedToBareOrAccessedRequire(node: Node): node is VariableDeclarationInitializedTo { + return isVariableDeclarationInitializedWithRequireHelper(node, /*allowAccessedRequire*/ true); + } + + function isVariableDeclarationInitializedWithRequireHelper(node: Node, allowAccessedRequire: boolean) { if (node.kind === SyntaxKind.BindingElement) { node = node.parent.parent; } - return isVariableDeclaration(node) && !!node.initializer && isRequireCall(getLeftmostAccessExpression(node.initializer), /*requireStringLiteralLikeArgument*/ true); + return isVariableDeclaration(node) && + !!node.initializer && + isRequireCall(allowAccessedRequire ? getLeftmostAccessExpression(node.initializer) : node.initializer, /*requireStringLiteralLikeArgument*/ true); } export function isRequireVariableStatement(node: Node): node is RequireVariableStatement { return isVariableStatement(node) && node.declarationList.declarations.length > 0 - && every(node.declarationList.declarations, decl => isRequireVariableDeclaration(decl)); + && every(node.declarationList.declarations, decl => isVariableDeclarationInitializedToRequire(decl)); } export function isSingleOrDoubleQuote(charCode: number) { diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index 6284cb6775569..bf78d043e1084 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -535,7 +535,7 @@ namespace ts.codefix { const importKind = getImportKind(importingFile, exportKind, compilerOptions); return mapDefined(importingFile.imports, (moduleSpecifier): FixAddToExistingImportInfo | undefined => { const i = importFromModuleSpecifier(moduleSpecifier); - if (isRequireVariableDeclaration(i.parent)) { + if (isVariableDeclarationInitializedToRequire(i.parent)) { return checker.resolveExternalModuleName(moduleSpecifier) === moduleSymbol ? { declaration: i.parent, importKind, symbol, targetFlags } : undefined; } if (i.kind === SyntaxKind.ImportDeclaration || i.kind === SyntaxKind.ImportEqualsDeclaration) { diff --git a/src/services/findAllReferences.ts b/src/services/findAllReferences.ts index 0c04f00e6a87b..a744ff64545db 100644 --- a/src/services/findAllReferences.ts +++ b/src/services/findAllReferences.ts @@ -1531,7 +1531,7 @@ namespace ts.FindAllReferences { // Use the parent symbol if the location is commonjs require syntax on javascript files only. if (isInJSFile(referenceLocation) && referenceLocation.parent.kind === SyntaxKind.BindingElement - && isRequireVariableDeclaration(referenceLocation.parent)) { + && isVariableDeclarationInitializedToBareOrAccessedRequire(referenceLocation.parent)) { referenceSymbol = referenceLocation.parent.symbol; // The parent will not have a symbol if it's an ObjectBindingPattern (when destructuring is used). In // this case, just skip it, since the bound identifiers are not an alias of the import. diff --git a/src/services/goToDefinition.ts b/src/services/goToDefinition.ts index af03eb260f507..80894d1b21203 100644 --- a/src/services/goToDefinition.ts +++ b/src/services/goToDefinition.ts @@ -287,7 +287,7 @@ namespace ts.GoToDefinition { return declaration.parent.kind === SyntaxKind.NamedImports; case SyntaxKind.BindingElement: case SyntaxKind.VariableDeclaration: - return isInJSFile(declaration) && isRequireVariableDeclaration(declaration); + return isInJSFile(declaration) && isVariableDeclarationInitializedToBareOrAccessedRequire(declaration); default: return false; } diff --git a/src/services/importTracker.ts b/src/services/importTracker.ts index d3c95854f9bbe..7bc8ffc36947e 100644 --- a/src/services/importTracker.ts +++ b/src/services/importTracker.ts @@ -621,7 +621,7 @@ namespace ts.FindAllReferences { Debug.assert((parent as ImportClause | NamespaceImport).name === node); return true; case SyntaxKind.BindingElement: - return isInJSFile(node) && isRequireVariableDeclaration(parent); + return isInJSFile(node) && isVariableDeclarationInitializedToBareOrAccessedRequire(parent); default: return false; } diff --git a/tests/cases/fourslash/importFixesWithExistingDottedRequire.ts b/tests/cases/fourslash/importFixesWithExistingDottedRequire.ts new file mode 100644 index 0000000000000..8dae9bdac8aed --- /dev/null +++ b/tests/cases/fourslash/importFixesWithExistingDottedRequire.ts @@ -0,0 +1,21 @@ +/// + +// @module: commonjs +// @checkJs: true + +// @Filename: ./library.js +//// module.exports.aaa = function() {} +//// module.exports.bbb = function() {} + +// @Filename: ./foo.js +//// var aaa = require("./library.js").aaa; +//// aaa(); +//// /*$*/bbb + +goTo.marker("$") +verify.codeFixAvailable([ + { description: "Import 'bbb' from module \"./library.js\"" }, + { description: "Ignore this error message" }, + { description: "Disable checking for this file" }, + { description: "Convert to ES module" }, +]);