From 5b86612f68905f35e893c0edc2f054df94b909d0 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Thu, 26 May 2022 16:43:22 -0700 Subject: [PATCH] Fix crash from inaccurate type guard implementation (#49252) * Fix `isVariableDeclarationInitializedToBareOrAccessedRequire` returning true on binding elements * Undo auto format change --- src/compiler/binder.ts | 7 ++++++- src/compiler/checker.ts | 9 +++++---- src/compiler/utilities.ts | 3 --- src/services/findAllReferences.ts | 2 +- src/services/importTracker.ts | 2 +- .../goToSource14_unresolvedRequireDestructuring.ts | 8 ++++++++ 6 files changed, 21 insertions(+), 10 deletions(-) create mode 100644 tests/cases/fourslash/server/goToSource14_unresolvedRequireDestructuring.ts diff --git a/src/compiler/binder.ts b/src/compiler/binder.ts index 66a9f6393dc75..10e40cf4cecbd 100644 --- 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 index 974220eb2d423..779affe2c90d4 100644 --- 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 index a554f1f54d02d..e5ff167dcc93e 100644 --- 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 index a6253eeafa5da..b9ba94daef0fa 100644 --- 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 index a3fcfb76680de..6ea4de1e9520a 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) && 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 new file mode 100644 index 0000000000000..426dc11107488 --- /dev/null +++ b/tests/cases/fourslash/server/goToSource14_unresolvedRequireDestructuring.ts @@ -0,0 +1,8 @@ +/// + +// @allowJs: true + +// @Filename: /index.js +//// const { blah/**/ } = require("unresolved"); + +verify.goToSourceDefinition("", []);