Skip to content

Commit 79d3058

Browse files
RyanCavanaughRyanCavanaugh
authored
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
1 parent 3c576f1 commit 79d3058

File tree

4 files changed

+185
-27
lines changed

4 files changed

+185
-27
lines changed

‎src/services/importTracker.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -644,7 +644,8 @@ namespace ts.FindAllReferences {
644644
return checker.getExportSpecifierLocalTargetSymbol(declaration)!;
645645
}
646646
else if (isPropertyAccessExpression(declaration) && isModuleExportsAccessExpression(declaration.expression) && !isPrivateIdentifier(declaration.name)) {
647-
return checker.getExportSpecifierLocalTargetSymbol(declaration.name)!;
647+
// Export of form 'module.exports.propName = expr';
648+
return checker.getSymbolAtLocation(declaration)!;
648649
}
649650
else if (isShorthandPropertyAssignment(declaration)
650651
&& isBinaryExpression(declaration.parent.parent)

‎tests/baselines/reference/findAllRefsCommonJsRequire2.baseline.jsonc

Lines changed: 33 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33
// /*FIND ALL REFS*/[|f|]
44

55
// === /a.js ===
6-
// function [|f|]() { }
7-
// module.exports.f = [|f|]
6+
// function f() { }
7+
// module.exports.[|f|] = f
88

99
[
1010
{
@@ -119,24 +119,40 @@
119119
"containerKind": "",
120120
"containerName": "",
121121
"fileName": "/a.js",
122-
"kind": "function",
123-
"name": "function f(): void",
122+
"kind": "property",
123+
"name": "(property) f: () => void",
124124
"textSpan": {
125-
"start": 9,
125+
"start": 32,
126126
"length": 1
127127
},
128128
"displayParts": [
129129
{
130-
"text": "function",
131-
"kind": "keyword"
130+
"text": "(",
131+
"kind": "punctuation"
132+
},
133+
{
134+
"text": "property",
135+
"kind": "text"
136+
},
137+
{
138+
"text": ")",
139+
"kind": "punctuation"
132140
},
133141
{
134142
"text": " ",
135143
"kind": "space"
136144
},
137145
{
138146
"text": "f",
139-
"kind": "functionName"
147+
"kind": "propertyName"
148+
},
149+
{
150+
"text": ":",
151+
"kind": "punctuation"
152+
},
153+
{
154+
"text": " ",
155+
"kind": "space"
140156
},
141157
{
142158
"text": "(",
@@ -147,7 +163,11 @@
147163
"kind": "punctuation"
148164
},
149165
{
150-
"text": ":",
166+
"text": " ",
167+
"kind": "space"
168+
},
169+
{
170+
"text": "=>",
151171
"kind": "punctuation"
152172
},
153173
{
@@ -160,36 +180,23 @@
160180
}
161181
],
162182
"contextSpan": {
163-
"start": 0,
183+
"start": 17,
164184
"length": 16
165185
}
166186
},
167187
"references": [
168188
{
169189
"textSpan": {
170-
"start": 9,
171-
"length": 1
172-
},
173-
"fileName": "/a.js",
174-
"contextSpan": {
175-
"start": 0,
176-
"length": 16
177-
},
178-
"isWriteAccess": true,
179-
"isDefinition": true
180-
},
181-
{
182-
"textSpan": {
183-
"start": 36,
190+
"start": 32,
184191
"length": 1
185192
},
186193
"fileName": "/a.js",
187194
"contextSpan": {
188195
"start": 17,
189196
"length": 20
190197
},
191-
"isWriteAccess": false,
192-
"isDefinition": false
198+
"isWriteAccess": true,
199+
"isDefinition": true
193200
}
194201
]
195202
}

‎tests/baselines/reference/indirectJsRequireRename.baseline.jsonc

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
// === /lib/plugins/aws/package/compile/events/httpApi/index.js ===
2+
// const { [|logWarning|] } = require('../../../../../../classes/Error');
3+
4+
// === /lib/classes/Error.js ===
5+
// module.exports.[|logWarning|] = message => { };
6+
7+
// === /bin/serverless.js ===
8+
// /*FIND ALL REFS*/require('../lib/classes/Error').[|logWarning|](`CLI triage crashed with: ${error.stack}`);
9+
10+
[
11+
{
12+
"definition": {
13+
"containerKind": "",
14+
"containerName": "",
15+
"fileName": "/lib/classes/Error.js",
16+
"kind": "property",
17+
"name": "(property) logWarning: (message: any) => void",
18+
"textSpan": {
19+
"start": 15,
20+
"length": 10
21+
},
22+
"displayParts": [
23+
{
24+
"text": "(",
25+
"kind": "punctuation"
26+
},
27+
{
28+
"text": "property",
29+
"kind": "text"
30+
},
31+
{
32+
"text": ")",
33+
"kind": "punctuation"
34+
},
35+
{
36+
"text": " ",
37+
"kind": "space"
38+
},
39+
{
40+
"text": "logWarning",
41+
"kind": "propertyName"
42+
},
43+
{
44+
"text": ":",
45+
"kind": "punctuation"
46+
},
47+
{
48+
"text": " ",
49+
"kind": "space"
50+
},
51+
{
52+
"text": "(",
53+
"kind": "punctuation"
54+
},
55+
{
56+
"text": "message",
57+
"kind": "parameterName"
58+
},
59+
{
60+
"text": ":",
61+
"kind": "punctuation"
62+
},
63+
{
64+
"text": " ",
65+
"kind": "space"
66+
},
67+
{
68+
"text": "any",
69+
"kind": "keyword"
70+
},
71+
{
72+
"text": ")",
73+
"kind": "punctuation"
74+
},
75+
{
76+
"text": " ",
77+
"kind": "space"
78+
},
79+
{
80+
"text": "=>",
81+
"kind": "punctuation"
82+
},
83+
{
84+
"text": " ",
85+
"kind": "space"
86+
},
87+
{
88+
"text": "void",
89+
"kind": "keyword"
90+
}
91+
],
92+
"contextSpan": {
93+
"start": 0,
94+
"length": 25
95+
}
96+
},
97+
"references": [
98+
{
99+
"textSpan": {
100+
"start": 15,
101+
"length": 10
102+
},
103+
"fileName": "/lib/classes/Error.js",
104+
"contextSpan": {
105+
"start": 0,
106+
"length": 43
107+
},
108+
"isWriteAccess": true,
109+
"isDefinition": true
110+
},
111+
{
112+
"textSpan": {
113+
"start": 32,
114+
"length": 10
115+
},
116+
"fileName": "/bin/serverless.js",
117+
"isWriteAccess": false,
118+
"isDefinition": false
119+
},
120+
{
121+
"textSpan": {
122+
"start": 8,
123+
"length": 10
124+
},
125+
"fileName": "/lib/plugins/aws/package/compile/events/httpApi/index.js",
126+
"contextSpan": {
127+
"start": 0,
128+
"length": 66
129+
},
130+
"isWriteAccess": true,
131+
"isDefinition": true
132+
}
133+
]
134+
}
135+
]

‎tests/cases/fourslash/indirectJsRequireRename.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
/// <reference path="fourslash.ts" />
2+
3+
// @allowJs: true
4+
5+
// @Filename: /bin/serverless.js
6+
//// require('../lib/classes/Error').log/**/Warning(`CLI triage crashed with: ${error.stack}`);
7+
8+
// @Filename: /lib/plugins/aws/package/compile/events/httpApi/index.js
9+
//// const { logWarning } = require('../../../../../../classes/Error');
10+
11+
// @Filename: /lib/classes/Error.js
12+
//// module.exports.logWarning = message => { };
13+
14+
goTo.marker();
15+
verify.baselineFindAllReferences("");

0 commit comments

Comments
 (0)