Fix infinite loop: module.exports alias detection (#31436)

* Fix infinite loop: module.exports alias detection Previously, module.exports alias detection in the binder could enter an infinite recursion. Now it does not. Notably, there are *two* safeguards: a counter limiter that I set at 100, and an already-seen set. I actually prefer the counter limiter code because it's foolproof and uses less memory. But it takes 100 iterations to escape from loops. * fix space lint * Remove already-seen map
microsoft · May 17, 2019 · eeba30a · eeba30a
1 parent f4b83ef
commit eeba30a
Show file tree

Hide file tree

Showing 4 changed files with 61 additions and 16 deletions.
diff --git a/src/compiler/binder.ts b/src/compiler/binder.ts
@@ -2581,7 +2581,7 @@ namespace ts {
             // Fix up parent pointers since we're going to use these nodes before we bind into them
             node.left.parent = node;
             node.right.parent = node;
-            if (isIdentifier(lhs.expression) && container === file && isNameOfExportsOrModuleExportsAliasDeclaration(file, lhs.expression)) {
+            if (isIdentifier(lhs.expression) && container === file && isExportsOrModuleExportsOrAlias(file, lhs.expression)) {
                 // This can be an alias for the 'exports' or 'module.exports' names, e.g.
                 //    var util = module.exports;
                 //    util.property = function ...
@@ -2975,21 +2975,27 @@ namespace ts {
     }
 
     export function isExportsOrModuleExportsOrAlias(sourceFile: SourceFile, node: Expression): boolean {
-        return isExportsIdentifier(node) ||
-            isModuleExportsPropertyAccessExpression(node) ||
-            isIdentifier(node) && isNameOfExportsOrModuleExportsAliasDeclaration(sourceFile, node);
-    }
-
-    function isNameOfExportsOrModuleExportsAliasDeclaration(sourceFile: SourceFile, node: Identifier): boolean {
-        const symbol = lookupSymbolForNameWorker(sourceFile, node.escapedText);
-        return !!symbol && !!symbol.valueDeclaration && isVariableDeclaration(symbol.valueDeclaration) &&
-            !!symbol.valueDeclaration.initializer && isExportsOrModuleExportsOrAliasOrAssignment(sourceFile, symbol.valueDeclaration.initializer);
-    }
-
-    function isExportsOrModuleExportsOrAliasOrAssignment(sourceFile: SourceFile, node: Expression): boolean {
-        return isExportsOrModuleExportsOrAlias(sourceFile, node) ||
-            (isAssignmentExpression(node, /*excludeCompoundAssignment*/ true) && (
-                isExportsOrModuleExportsOrAliasOrAssignment(sourceFile, node.left) || isExportsOrModuleExportsOrAliasOrAssignment(sourceFile, node.right)));
+        let i = 0;
+        const q = [node];
+        while (q.length && i < 100) {
+            i++;
+            node = q.shift()!;
+            if (isExportsIdentifier(node) || isModuleExportsPropertyAccessExpression(node)) {
+                return true;
+            }
+            else if (isIdentifier(node)) {
+                const symbol = lookupSymbolForNameWorker(sourceFile, node.escapedText);
+                if (!!symbol && !!symbol.valueDeclaration && isVariableDeclaration(symbol.valueDeclaration) && !!symbol.valueDeclaration.initializer) {
+                    const init = symbol.valueDeclaration.initializer;
+                    q.push(init);
+                    if (isAssignmentExpression(init, /*excludeCompoundAssignment*/ true)) {
+                        q.push(init.left);
+                        q.push(init.right);
+                    }
+                }
+            }
+        }
+        return false;
     }
 
     function lookupSymbolForNameWorker(container: Node, name: __String): Symbol | undefined {

diff --git a/tests/baselines/reference/binderUninitializedModuleExportsAssignment.symbols b/tests/baselines/reference/binderUninitializedModuleExportsAssignment.symbols
@@ -0,0 +1,15 @@
+=== tests/cases/conformance/salsa/loop.js ===
+var loop1 = loop2;
+>loop1 : Symbol(loop1, Decl(loop.js, 0, 3))
+>loop2 : Symbol(loop2, Decl(loop.js, 1, 3))
+
+var loop2 = loop1;
+>loop2 : Symbol(loop2, Decl(loop.js, 1, 3))
+>loop1 : Symbol(loop1, Decl(loop.js, 0, 3))
+
+module.exports = loop2;
+>module.exports : Symbol("tests/cases/conformance/salsa/loop", Decl(loop.js, 0, 0))
+>module : Symbol(export=, Decl(loop.js, 1, 18))
+>exports : Symbol(export=, Decl(loop.js, 1, 18))
+>loop2 : Symbol(loop2, Decl(loop.js, 1, 3))
+
diff --git a/tests/baselines/reference/binderUninitializedModuleExportsAssignment.types b/tests/baselines/reference/binderUninitializedModuleExportsAssignment.types
@@ -0,0 +1,16 @@
+=== tests/cases/conformance/salsa/loop.js ===
+var loop1 = loop2;
+>loop1 : any
+>loop2 : any
+
+var loop2 = loop1;
+>loop2 : any
+>loop1 : any
+
+module.exports = loop2;
+>module.exports = loop2 : any
+>module.exports : any
+>module : { "tests/cases/conformance/salsa/loop": any; }
+>exports : any
+>loop2 : any
+
diff --git a/tests/cases/conformance/salsa/binderUninitializedModuleExportsAssignment.ts b/tests/cases/conformance/salsa/binderUninitializedModuleExportsAssignment.ts
@@ -0,0 +1,8 @@
+// @noEmit: true
+// @allowJs: true
+// @checkJs: true
+// @Filename: loop.js
+var loop1 = loop2;
+var loop2 = loop1;
+
+module.exports = loop2;