diff --git a/src/compiler/binder.ts b/src/compiler/binder.ts index eefc41fdaf8b5..5e4ed7af65031 100644 --- a/src/compiler/binder.ts +++ b/src/compiler/binder.ts @@ -3282,7 +3282,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 578313bc7993c..1a8b1d0fc50e4 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -2596,7 +2596,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) { @@ -36984,7 +36984,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 dcea2e7e38526..32b316137b3f6 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -4639,7 +4639,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; @@ -4664,8 +4664,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 */ @@ -4675,7 +4675,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 98d89d4612824..d634192b9365b 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 aebc52b2fb0f8..9f6baf0cc73c4 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 9ae8dda40f84c..bc1b2f64d5ab6 100644 --- a/src/services/goToDefinition.ts +++ b/src/services/goToDefinition.ts @@ -290,7 +290,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" }, +]);