From 79d3058b83bec8aca924d2ddb2864a4e45f3d05c Mon Sep 17 00:00:00 2001 From: Ryan Cavanaugh Date: Thu, 11 Mar 2021 07:23:19 -0800 Subject: [PATCH] Don't crash when renaming a JS property declared via module.exports (#40297) Fixes #38070 When the originating definition was of the form ```js module.exports.foo = expr ``` we were incorrectly trying to call `resolveName` on just the `foo` portion to get the "local" symbol, which simply failed to resolve (or would have resolved to the wrong thing), but for this form, the local symbol is just the containing property access expression --- src/services/importTracker.ts | 3 +- ...findAllRefsCommonJsRequire2.baseline.jsonc | 59 ++++---- .../indirectJsRequireRename.baseline.jsonc | 135 ++++++++++++++++++ .../fourslash/indirectJsRequireRename.ts | 15 ++ 4 files changed, 185 insertions(+), 27 deletions(-) create mode 100644 tests/baselines/reference/indirectJsRequireRename.baseline.jsonc create mode 100644 tests/cases/fourslash/indirectJsRequireRename.ts diff --git a/src/services/importTracker.ts b/src/services/importTracker.ts index 34158f5f71f..e93b99ef63b 100644 --- a/src/services/importTracker.ts +++ b/src/services/importTracker.ts @@ -644,7 +644,8 @@ namespace ts.FindAllReferences { return checker.getExportSpecifierLocalTargetSymbol(declaration)!; } else if (isPropertyAccessExpression(declaration) && isModuleExportsAccessExpression(declaration.expression) && !isPrivateIdentifier(declaration.name)) { - return checker.getExportSpecifierLocalTargetSymbol(declaration.name)!; + // Export of form 'module.exports.propName = expr'; + return checker.getSymbolAtLocation(declaration)!; } else if (isShorthandPropertyAssignment(declaration) && isBinaryExpression(declaration.parent.parent) diff --git a/tests/baselines/reference/findAllRefsCommonJsRequire2.baseline.jsonc b/tests/baselines/reference/findAllRefsCommonJsRequire2.baseline.jsonc index 3ea2975673f..d904da0af89 100644 --- a/tests/baselines/reference/findAllRefsCommonJsRequire2.baseline.jsonc +++ b/tests/baselines/reference/findAllRefsCommonJsRequire2.baseline.jsonc @@ -3,8 +3,8 @@ // /*FIND ALL REFS*/[|f|] // === /a.js === -// function [|f|]() { } -// module.exports.f = [|f|] +// function f() { } +// module.exports.[|f|] = f [ { @@ -119,16 +119,24 @@ "containerKind": "", "containerName": "", "fileName": "/a.js", - "kind": "function", - "name": "function f(): void", + "kind": "property", + "name": "(property) f: () => void", "textSpan": { - "start": 9, + "start": 32, "length": 1 }, "displayParts": [ { - "text": "function", - "kind": "keyword" + "text": "(", + "kind": "punctuation" + }, + { + "text": "property", + "kind": "text" + }, + { + "text": ")", + "kind": "punctuation" }, { "text": " ", @@ -136,7 +144,15 @@ }, { "text": "f", - "kind": "functionName" + "kind": "propertyName" + }, + { + "text": ":", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" }, { "text": "(", @@ -147,7 +163,11 @@ "kind": "punctuation" }, { - "text": ":", + "text": " ", + "kind": "space" + }, + { + "text": "=>", "kind": "punctuation" }, { @@ -160,27 +180,14 @@ } ], "contextSpan": { - "start": 0, + "start": 17, "length": 16 } }, "references": [ { "textSpan": { - "start": 9, - "length": 1 - }, - "fileName": "/a.js", - "contextSpan": { - "start": 0, - "length": 16 - }, - "isWriteAccess": true, - "isDefinition": true - }, - { - "textSpan": { - "start": 36, + "start": 32, "length": 1 }, "fileName": "/a.js", @@ -188,8 +195,8 @@ "start": 17, "length": 20 }, - "isWriteAccess": false, - "isDefinition": false + "isWriteAccess": true, + "isDefinition": true } ] } diff --git a/tests/baselines/reference/indirectJsRequireRename.baseline.jsonc b/tests/baselines/reference/indirectJsRequireRename.baseline.jsonc new file mode 100644 index 00000000000..264235c40c8 --- /dev/null +++ b/tests/baselines/reference/indirectJsRequireRename.baseline.jsonc @@ -0,0 +1,135 @@ +// === /lib/plugins/aws/package/compile/events/httpApi/index.js === +// const { [|logWarning|] } = require('../../../../../../classes/Error'); + +// === /lib/classes/Error.js === +// module.exports.[|logWarning|] = message => { }; + +// === /bin/serverless.js === +// /*FIND ALL REFS*/require('../lib/classes/Error').[|logWarning|](`CLI triage crashed with: ${error.stack}`); + +[ + { + "definition": { + "containerKind": "", + "containerName": "", + "fileName": "/lib/classes/Error.js", + "kind": "property", + "name": "(property) logWarning: (message: any) => void", + "textSpan": { + "start": 15, + "length": 10 + }, + "displayParts": [ + { + "text": "(", + "kind": "punctuation" + }, + { + "text": "property", + "kind": "text" + }, + { + "text": ")", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "logWarning", + "kind": "propertyName" + }, + { + "text": ":", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "(", + "kind": "punctuation" + }, + { + "text": "message", + "kind": "parameterName" + }, + { + "text": ":", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "any", + "kind": "keyword" + }, + { + "text": ")", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "=>", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "void", + "kind": "keyword" + } + ], + "contextSpan": { + "start": 0, + "length": 25 + } + }, + "references": [ + { + "textSpan": { + "start": 15, + "length": 10 + }, + "fileName": "/lib/classes/Error.js", + "contextSpan": { + "start": 0, + "length": 43 + }, + "isWriteAccess": true, + "isDefinition": true + }, + { + "textSpan": { + "start": 32, + "length": 10 + }, + "fileName": "/bin/serverless.js", + "isWriteAccess": false, + "isDefinition": false + }, + { + "textSpan": { + "start": 8, + "length": 10 + }, + "fileName": "/lib/plugins/aws/package/compile/events/httpApi/index.js", + "contextSpan": { + "start": 0, + "length": 66 + }, + "isWriteAccess": true, + "isDefinition": true + } + ] + } +] \ No newline at end of file diff --git a/tests/cases/fourslash/indirectJsRequireRename.ts b/tests/cases/fourslash/indirectJsRequireRename.ts new file mode 100644 index 00000000000..845ce491629 --- /dev/null +++ b/tests/cases/fourslash/indirectJsRequireRename.ts @@ -0,0 +1,15 @@ +/// + +// @allowJs: true + +// @Filename: /bin/serverless.js +//// require('../lib/classes/Error').log/**/Warning(`CLI triage crashed with: ${error.stack}`); + +// @Filename: /lib/plugins/aws/package/compile/events/httpApi/index.js +//// const { logWarning } = require('../../../../../../classes/Error'); + +// @Filename: /lib/classes/Error.js +//// module.exports.logWarning = message => { }; + +goTo.marker(); +verify.baselineFindAllReferences(""); \ No newline at end of file