Skip to content

Commit 8450901

Browse files
TypeScript BotDanielRosenwasser
TypeScript Bot
andauthored
Cherry-pick PR #47433 into release-4.5 (#47515)
Component commits: fce282b Add failing test. ea2c290 Update failing test. d954948 Finalized failing test case. f476e84 Separate our usages of utilities that expect variables initialized to require(...) and require(...).foo. 9f0810c Renamed types and utilities, removed accidental indentation. bf708bf Renamed both utilitiy functions uniformly. Co-authored-by: Daniel Rosenwasser <[email protected]>
1 parent 8a8048f commit 8450901

File tree

9 files changed

+49
-15
lines changed

9 files changed

+49
-15
lines changed

Diff for: src/compiler/binder.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -3288,7 +3288,7 @@ namespace ts {
32883288
}
32893289

32903290
if (!isBindingPattern(node.name)) {
3291-
if (isInJSFile(node) && isRequireVariableDeclaration(node) && !getJSDocTypeTag(node)) {
3291+
if (isInJSFile(node) && isVariableDeclarationInitializedToBareOrAccessedRequire(node) && !getJSDocTypeTag(node)) {
32923292
declareSymbolAndAddToSymbolTable(node as Declaration, SymbolFlags.Alias, SymbolFlags.AliasExcludes);
32933293
}
32943294
else if (isBlockOrCatchScoped(node)) {

Diff for: src/compiler/checker.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -2592,7 +2592,7 @@ namespace ts {
25922592
&& isAliasableOrJsExpression(node.parent.right)
25932593
|| node.kind === SyntaxKind.ShorthandPropertyAssignment
25942594
|| node.kind === SyntaxKind.PropertyAssignment && isAliasableOrJsExpression((node as PropertyAssignment).initializer)
2595-
|| isRequireVariableDeclaration(node);
2595+
|| isVariableDeclarationInitializedToBareOrAccessedRequire(node);
25962596
}
25972597

25982598
function isAliasableOrJsExpression(e: Expression) {
@@ -36831,7 +36831,7 @@ namespace ts {
3683136831
}
3683236832
// For a commonjs `const x = require`, validate the alias and exit
3683336833
const symbol = getSymbolOfNode(node);
36834-
if (symbol.flags & SymbolFlags.Alias && isRequireVariableDeclaration(node)) {
36834+
if (symbol.flags & SymbolFlags.Alias && isVariableDeclarationInitializedToBareOrAccessedRequire(node)) {
3683536835
checkAliasSymbol(node);
3683636836
return;
3683736837
}

Diff for: src/compiler/types.ts

+4-4
Original file line numberDiff line numberDiff line change
@@ -4637,7 +4637,7 @@ namespace ts {
46374637
export type AnyImportSyntax = ImportDeclaration | ImportEqualsDeclaration;
46384638

46394639
/* @internal */
4640-
export type AnyImportOrRequire = AnyImportSyntax | RequireVariableDeclaration;
4640+
export type AnyImportOrRequire = AnyImportSyntax | VariableDeclarationInitializedTo<RequireOrImportCall>;
46414641

46424642
/* @internal */
46434643
export type AnyImportOrRequireStatement = AnyImportSyntax | RequireVariableStatement;
@@ -4662,8 +4662,8 @@ namespace ts {
46624662
export type RequireOrImportCall = CallExpression & { expression: Identifier, arguments: [StringLiteralLike] };
46634663

46644664
/* @internal */
4665-
export interface RequireVariableDeclaration extends VariableDeclaration {
4666-
readonly initializer: RequireOrImportCall;
4665+
export interface VariableDeclarationInitializedTo<T extends Expression> extends VariableDeclaration {
4666+
readonly initializer: T;
46674667
}
46684668

46694669
/* @internal */
@@ -4673,7 +4673,7 @@ namespace ts {
46734673

46744674
/* @internal */
46754675
export interface RequireVariableDeclarationList extends VariableDeclarationList {
4676-
readonly declarations: NodeArray<RequireVariableDeclaration>;
4676+
readonly declarations: NodeArray<VariableDeclarationInitializedTo<RequireOrImportCall>>;
46774677
}
46784678

46794679
/* @internal */

Diff for: src/compiler/utilities.ts

+17-4
Original file line numberDiff line numberDiff line change
@@ -2046,7 +2046,7 @@ namespace ts {
20462046
}
20472047

20482048
export function getExternalModuleRequireArgument(node: Node) {
2049-
return isRequireVariableDeclaration(node) && (getLeftmostAccessExpression(node.initializer) as CallExpression).arguments[0] as StringLiteral;
2049+
return isVariableDeclarationInitializedToBareOrAccessedRequire(node) && (getLeftmostAccessExpression(node.initializer) as CallExpression).arguments[0] as StringLiteral;
20502050
}
20512051

20522052
export function isInternalModuleImportEqualsDeclaration(node: Node): node is ImportEqualsDeclaration {
@@ -2113,17 +2113,30 @@ namespace ts {
21132113
* Returns true if the node is a VariableDeclaration initialized to a require call (see `isRequireCall`).
21142114
* This function does not test if the node is in a JavaScript file or not.
21152115
*/
2116-
export function isRequireVariableDeclaration(node: Node): node is RequireVariableDeclaration {
2116+
export function isVariableDeclarationInitializedToRequire(node: Node): node is VariableDeclarationInitializedTo<RequireOrImportCall> {
2117+
return isVariableDeclarationInitializedWithRequireHelper(node, /*allowAccessedRequire*/ false);
2118+
}
2119+
2120+
/**
2121+
* Like {@link isVariableDeclarationInitializedToRequire} but allows things like `require("...").foo.bar` or `require("...")["baz"]`.
2122+
*/
2123+
export function isVariableDeclarationInitializedToBareOrAccessedRequire(node: Node): node is VariableDeclarationInitializedTo<RequireOrImportCall | AccessExpression> {
2124+
return isVariableDeclarationInitializedWithRequireHelper(node, /*allowAccessedRequire*/ true);
2125+
}
2126+
2127+
function isVariableDeclarationInitializedWithRequireHelper(node: Node, allowAccessedRequire: boolean) {
21172128
if (node.kind === SyntaxKind.BindingElement) {
21182129
node = node.parent.parent;
21192130
}
2120-
return isVariableDeclaration(node) && !!node.initializer && isRequireCall(getLeftmostAccessExpression(node.initializer), /*requireStringLiteralLikeArgument*/ true);
2131+
return isVariableDeclaration(node) &&
2132+
!!node.initializer &&
2133+
isRequireCall(allowAccessedRequire ? getLeftmostAccessExpression(node.initializer) : node.initializer, /*requireStringLiteralLikeArgument*/ true);
21212134
}
21222135

21232136
export function isRequireVariableStatement(node: Node): node is RequireVariableStatement {
21242137
return isVariableStatement(node)
21252138
&& node.declarationList.declarations.length > 0
2126-
&& every(node.declarationList.declarations, decl => isRequireVariableDeclaration(decl));
2139+
&& every(node.declarationList.declarations, decl => isVariableDeclarationInitializedToRequire(decl));
21272140
}
21282141

21292142
export function isSingleOrDoubleQuote(charCode: number) {

Diff for: src/services/codefixes/importFixes.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -535,7 +535,7 @@ namespace ts.codefix {
535535
const importKind = getImportKind(importingFile, exportKind, compilerOptions);
536536
return mapDefined(importingFile.imports, (moduleSpecifier): FixAddToExistingImportInfo | undefined => {
537537
const i = importFromModuleSpecifier(moduleSpecifier);
538-
if (isRequireVariableDeclaration(i.parent)) {
538+
if (isVariableDeclarationInitializedToRequire(i.parent)) {
539539
return checker.resolveExternalModuleName(moduleSpecifier) === moduleSymbol ? { declaration: i.parent, importKind, symbol, targetFlags } : undefined;
540540
}
541541
if (i.kind === SyntaxKind.ImportDeclaration || i.kind === SyntaxKind.ImportEqualsDeclaration) {

Diff for: src/services/findAllReferences.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -1531,7 +1531,7 @@ namespace ts.FindAllReferences {
15311531
// Use the parent symbol if the location is commonjs require syntax on javascript files only.
15321532
if (isInJSFile(referenceLocation)
15331533
&& referenceLocation.parent.kind === SyntaxKind.BindingElement
1534-
&& isRequireVariableDeclaration(referenceLocation.parent)) {
1534+
&& isVariableDeclarationInitializedToBareOrAccessedRequire(referenceLocation.parent)) {
15351535
referenceSymbol = referenceLocation.parent.symbol;
15361536
// The parent will not have a symbol if it's an ObjectBindingPattern (when destructuring is used). In
15371537
// this case, just skip it, since the bound identifiers are not an alias of the import.

Diff for: src/services/goToDefinition.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -287,7 +287,7 @@ namespace ts.GoToDefinition {
287287
return declaration.parent.kind === SyntaxKind.NamedImports;
288288
case SyntaxKind.BindingElement:
289289
case SyntaxKind.VariableDeclaration:
290-
return isInJSFile(declaration) && isRequireVariableDeclaration(declaration);
290+
return isInJSFile(declaration) && isVariableDeclarationInitializedToBareOrAccessedRequire(declaration);
291291
default:
292292
return false;
293293
}

Diff for: src/services/importTracker.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -621,7 +621,7 @@ namespace ts.FindAllReferences {
621621
Debug.assert((parent as ImportClause | NamespaceImport).name === node);
622622
return true;
623623
case SyntaxKind.BindingElement:
624-
return isInJSFile(node) && isRequireVariableDeclaration(parent);
624+
return isInJSFile(node) && isVariableDeclarationInitializedToBareOrAccessedRequire(parent);
625625
default:
626626
return false;
627627
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
/// <reference path="./fourslash.ts" />
2+
3+
// @module: commonjs
4+
// @checkJs: true
5+
6+
// @Filename: ./library.js
7+
//// module.exports.aaa = function() {}
8+
//// module.exports.bbb = function() {}
9+
10+
// @Filename: ./foo.js
11+
//// var aaa = require("./library.js").aaa;
12+
//// aaa();
13+
//// /*$*/bbb
14+
15+
goTo.marker("$")
16+
verify.codeFixAvailable([
17+
{ description: "Import 'bbb' from module \"./library.js\"" },
18+
{ description: "Ignore this error message" },
19+
{ description: "Disable checking for this file" },
20+
{ description: "Convert to ES module" },
21+
]);

0 commit comments

Comments
 (0)