Fix crash from inaccurate type guard implementation (#49252)

* Fix `isVariableDeclarationInitializedToBareOrAccessedRequire` returning true on binding elements * Undo auto format change
microsoft · May 26, 2022 · 5b86612 · 5b86612
1 parent 8497483
commit 5b86612
Show file tree

Hide file tree

Showing 6 changed files with 21 additions and 10 deletions.
diff --git a/src/compiler/binder.ts b/src/compiler/binder.ts
@@ -3291,7 +3291,12 @@ namespace ts {
             }
 
             if (!isBindingPattern(node.name)) {
-                if (isInJSFile(node) && isVariableDeclarationInitializedToBareOrAccessedRequire(node) && !getJSDocTypeTag(node) && !(getCombinedModifierFlags(node) & ModifierFlags.Export)) {
+                const possibleVariableDecl = node.kind === SyntaxKind.VariableDeclaration ? node : node.parent.parent;
+                if (isInJSFile(node) &&
+                    isVariableDeclarationInitializedToBareOrAccessedRequire(possibleVariableDecl) &&
+                    !getJSDocTypeTag(node) &&
+                    !(getCombinedModifierFlags(node) & ModifierFlags.Export)
+                ) {
                     declareSymbolAndAddToSymbolTable(node as Declaration, SymbolFlags.Alias, SymbolFlags.AliasExcludes);
                 }
                 else if (isBlockOrCatchScoped(node)) {

diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
@@ -2641,7 +2641,8 @@ namespace ts {
                     && isAliasableOrJsExpression(node.parent.right)
                 || node.kind === SyntaxKind.ShorthandPropertyAssignment
                 || node.kind === SyntaxKind.PropertyAssignment && isAliasableOrJsExpression((node as PropertyAssignment).initializer)
-                || isVariableDeclarationInitializedToBareOrAccessedRequire(node);
+                || node.kind === SyntaxKind.VariableDeclaration && isVariableDeclarationInitializedToBareOrAccessedRequire(node)
+                || node.kind === SyntaxKind.BindingElement && isVariableDeclarationInitializedToBareOrAccessedRequire(node.parent.parent);
         }
 
         function isAliasableOrJsExpression(e: Expression) {
@@ -37819,8 +37820,8 @@ namespace ts {
             }
             // For a commonjs `const x = require`, validate the alias and exit
             const symbol = getSymbolOfNode(node);
-            if (symbol.flags & SymbolFlags.Alias && isVariableDeclarationInitializedToBareOrAccessedRequire(node)) {
-                checkAliasSymbol(node);
+            if (symbol.flags & SymbolFlags.Alias && isVariableDeclarationInitializedToBareOrAccessedRequire(node.kind === SyntaxKind.BindingElement ? node.parent.parent : node)) {
+                checkAliasSymbol(node as BindingElement | VariableDeclaration);
                 return;
             }
 
@@ -40684,7 +40685,7 @@ namespace ts {
             return true;
         }
 
-        function checkAliasSymbol(node: ImportEqualsDeclaration | VariableDeclaration | ImportClause | NamespaceImport | ImportSpecifier | ExportSpecifier | NamespaceExport) {
+        function checkAliasSymbol(node: ImportEqualsDeclaration | VariableDeclaration | ImportClause | NamespaceImport | ImportSpecifier | ExportSpecifier | NamespaceExport | BindingElement) {
             let symbol = getSymbolOfNode(node);
             const target = resolveAlias(symbol);
 

diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts
@@ -2197,9 +2197,6 @@ namespace ts {
     }
 
     function isVariableDeclarationInitializedWithRequireHelper(node: Node, allowAccessedRequire: boolean) {
-        if (node.kind === SyntaxKind.BindingElement) {
-            node = node.parent.parent;
-        }
         return isVariableDeclaration(node) &&
             !!node.initializer &&
             isRequireCall(allowAccessedRequire ? getLeftmostAccessExpression(node.initializer) : node.initializer, /*requireStringLiteralLikeArgument*/ true);

diff --git a/src/services/findAllReferences.ts b/src/services/findAllReferences.ts
@@ -1601,7 +1601,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
-                && isVariableDeclarationInitializedToBareOrAccessedRequire(referenceLocation.parent)) {
+                && isVariableDeclarationInitializedToBareOrAccessedRequire(referenceLocation.parent.parent.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/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) && isVariableDeclarationInitializedToBareOrAccessedRequire(parent);
+                return isInJSFile(node) && isVariableDeclarationInitializedToBareOrAccessedRequire(parent.parent.parent);
             default:
                 return false;
         }

diff --git a/tests/cases/fourslash/server/goToSource14_unresolvedRequireDestructuring.ts b/tests/cases/fourslash/server/goToSource14_unresolvedRequireDestructuring.ts
@@ -0,0 +1,8 @@
+/// <reference path="../fourslash.ts" />
+
+// @allowJs: true
+
+// @Filename: /index.js
+//// const { blah/**/ } = require("unresolved");
+
+verify.goToSourceDefinition("", []);