Skip to content

Commit 3c988e8

Browse files
rbucktonrbuckton
authored
Merge pull request #21215 from Microsoft/fix20461
Fixes var declaration shadowing in async functions
2 parents e248d08 + 5b45db7 commit 3c988e8

File tree

7 files changed

+1840
-15
lines changed

7 files changed

+1840
-15
lines changed

‎src/compiler/core.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1341,7 +1341,8 @@ namespace ts {
13411341

13421342
export function cloneMap(map: SymbolTable): SymbolTable;
13431343
export function cloneMap<T>(map: ReadonlyMap<T>): Map<T>;
1344-
export function cloneMap<T>(map: ReadonlyMap<T> | SymbolTable): Map<T> | SymbolTable {
1344+
export function cloneMap<T>(map: ReadonlyUnderscoreEscapedMap<T>): UnderscoreEscapedMap<T>;
1345+
export function cloneMap<T>(map: ReadonlyMap<T> | ReadonlyUnderscoreEscapedMap<T> | SymbolTable): Map<T> | UnderscoreEscapedMap<T> | SymbolTable {
13451346
const clone = createMap<T>();
13461347
copyEntries(map as Map<T>, clone);
13471348
return clone;

‎src/compiler/transformers/es2017.ts

Lines changed: 202 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,9 @@ namespace ts {
1212

1313
export function transformES2017(context: TransformationContext) {
1414
const {
15-
startLexicalEnvironment,
1615
resumeLexicalEnvironment,
17-
endLexicalEnvironment
16+
endLexicalEnvironment,
17+
hoistVariableDeclaration
1818
} = context;
1919

2020
const resolver = context.getEmitResolver();
@@ -33,6 +33,8 @@ namespace ts {
3333
*/
3434
let enclosingSuperContainerFlags: NodeCheckFlags = 0;
3535

36+
let enclosingFunctionParameterNames: UnderscoreEscapedMap<true>;
37+
3638
// Save the previous transformation hooks.
3739
const previousOnEmitNode = context.onEmitNode;
3840
const previousOnSubstituteNode = context.onSubstituteNode;
@@ -83,6 +85,108 @@ namespace ts {
8385
}
8486
}
8587

88+
function asyncBodyVisitor(node: Node): VisitResult<Node> {
89+
if (isNodeWithPossibleHoistedDeclaration(node)) {
90+
switch (node.kind) {
91+
case SyntaxKind.VariableStatement:
92+
return visitVariableStatementInAsyncBody(node);
93+
case SyntaxKind.ForStatement:
94+
return visitForStatementInAsyncBody(node);
95+
case SyntaxKind.ForInStatement:
96+
return visitForInStatementInAsyncBody(node);
97+
case SyntaxKind.ForOfStatement:
98+
return visitForOfStatementInAsyncBody(node);
99+
case SyntaxKind.CatchClause:
100+
return visitCatchClauseInAsyncBody(node);
101+
case SyntaxKind.Block:
102+
case SyntaxKind.SwitchStatement:
103+
case SyntaxKind.CaseBlock:
104+
case SyntaxKind.CaseClause:
105+
case SyntaxKind.DefaultClause:
106+
case SyntaxKind.TryStatement:
107+
case SyntaxKind.DoStatement:
108+
case SyntaxKind.WhileStatement:
109+
case SyntaxKind.IfStatement:
110+
case SyntaxKind.WithStatement:
111+
case SyntaxKind.LabeledStatement:
112+
return visitEachChild(node, asyncBodyVisitor, context);
113+
default:
114+
return Debug.assertNever(node, "Unhandled node.");
115+
}
116+
}
117+
return visitor(node);
118+
}
119+
120+
function visitCatchClauseInAsyncBody(node: CatchClause) {
121+
const catchClauseNames = createUnderscoreEscapedMap<true>();
122+
recordDeclarationName(node.variableDeclaration, catchClauseNames);
123+
124+
// names declared in a catch variable are block scoped
125+
let catchClauseUnshadowedNames: UnderscoreEscapedMap<true>;
126+
catchClauseNames.forEach((_, escapedName) => {
127+
if (enclosingFunctionParameterNames.has(escapedName)) {
128+
if (!catchClauseUnshadowedNames) {
129+
catchClauseUnshadowedNames = cloneMap(enclosingFunctionParameterNames);
130+
}
131+
catchClauseUnshadowedNames.delete(escapedName);
132+
}
133+
});
134+
135+
if (catchClauseUnshadowedNames) {
136+
const savedEnclosingFunctionParameterNames = enclosingFunctionParameterNames;
137+
enclosingFunctionParameterNames = catchClauseUnshadowedNames;
138+
const result = visitEachChild(node, asyncBodyVisitor, context);
139+
enclosingFunctionParameterNames = savedEnclosingFunctionParameterNames;
140+
return result;
141+
}
142+
else {
143+
return visitEachChild(node, asyncBodyVisitor, context);
144+
}
145+
}
146+
147+
function visitVariableStatementInAsyncBody(node: VariableStatement) {
148+
if (isVariableDeclarationListWithCollidingName(node.declarationList)) {
149+
const expression = visitVariableDeclarationListWithCollidingNames(node.declarationList, /*hasReceiver*/ false);
150+
return expression ? createStatement(expression) : undefined;
151+
}
152+
return visitEachChild(node, visitor, context);
153+
}
154+
155+
function visitForInStatementInAsyncBody(node: ForInStatement) {
156+
return updateForIn(
157+
node,
158+
isVariableDeclarationListWithCollidingName(node.initializer)
159+
? visitVariableDeclarationListWithCollidingNames(node.initializer, /*hasReceiver*/ true)
160+
: visitNode(node.initializer, visitor, isForInitializer),
161+
visitNode(node.expression, visitor, isExpression),
162+
visitNode(node.statement, asyncBodyVisitor, isStatement, liftToBlock)
163+
);
164+
}
165+
166+
function visitForOfStatementInAsyncBody(node: ForOfStatement) {
167+
return updateForOf(
168+
node,
169+
visitNode(node.awaitModifier, visitor, isToken),
170+
isVariableDeclarationListWithCollidingName(node.initializer)
171+
? visitVariableDeclarationListWithCollidingNames(node.initializer, /*hasReceiver*/ true)
172+
: visitNode(node.initializer, visitor, isForInitializer),
173+
visitNode(node.expression, visitor, isExpression),
174+
visitNode(node.statement, asyncBodyVisitor, isStatement, liftToBlock)
175+
);
176+
}
177+
178+
function visitForStatementInAsyncBody(node: ForStatement) {
179+
return updateFor(
180+
node,
181+
isVariableDeclarationListWithCollidingName(node.initializer)
182+
? visitVariableDeclarationListWithCollidingNames(node.initializer, /*hasReceiver*/ false)
183+
: visitNode(node.initializer, visitor, isForInitializer),
184+
visitNode(node.condition, visitor, isExpression),
185+
visitNode(node.incrementor, visitor, isExpression),
186+
visitNode((<ForStatement>node).statement, asyncBodyVisitor, isStatement, liftToBlock)
187+
);
188+
}
189+
86190
/**
87191
* Visits an AwaitExpression node.
88192
*
@@ -197,6 +301,82 @@ namespace ts {
197301
);
198302
}
199303

304+
function recordDeclarationName({ name }: ParameterDeclaration | VariableDeclaration | BindingElement, names: UnderscoreEscapedMap<true>) {
305+
if (isIdentifier(name)) {
306+
names.set(name.escapedText, true);
307+
}
308+
else {
309+
for (const element of name.elements) {
310+
if (!isOmittedExpression(element)) {
311+
recordDeclarationName(element, names);
312+
}
313+
}
314+
}
315+
}
316+
317+
function isVariableDeclarationListWithCollidingName(node: ForInitializer): node is VariableDeclarationList {
318+
return node
319+
&& isVariableDeclarationList(node)
320+
&& !(node.flags & NodeFlags.BlockScoped)
321+
&& forEach(node.declarations, collidesWithParameterName);
322+
}
323+
324+
function visitVariableDeclarationListWithCollidingNames(node: VariableDeclarationList, hasReceiver: boolean) {
325+
hoistVariableDeclarationList(node);
326+
327+
const variables = getInitializedVariables(node);
328+
if (variables.length === 0) {
329+
if (hasReceiver) {
330+
return visitNode(convertToAssignmentElementTarget(node.declarations[0].name), visitor, isExpression);
331+
}
332+
return undefined;
333+
}
334+
335+
return inlineExpressions(map(variables, transformInitializedVariable));
336+
}
337+
338+
function hoistVariableDeclarationList(node: VariableDeclarationList) {
339+
forEach(node.declarations, hoistVariable);
340+
}
341+
342+
function hoistVariable({ name }: VariableDeclaration | BindingElement) {
343+
if (isIdentifier(name)) {
344+
hoistVariableDeclaration(name);
345+
}
346+
else {
347+
for (const element of name.elements) {
348+
if (!isOmittedExpression(element)) {
349+
hoistVariable(element);
350+
}
351+
}
352+
}
353+
}
354+
355+
function transformInitializedVariable(node: VariableDeclaration) {
356+
const converted = setSourceMapRange(
357+
createAssignment(
358+
convertToAssignmentElementTarget(node.name),
359+
node.initializer
360+
),
361+
node
362+
);
363+
return visitNode(converted, visitor, isExpression);
364+
}
365+
366+
function collidesWithParameterName({ name }: VariableDeclaration | BindingElement): boolean {
367+
if (isIdentifier(name)) {
368+
return enclosingFunctionParameterNames.has(name.escapedText);
369+
}
370+
else {
371+
for (const element of name.elements) {
372+
if (!isOmittedExpression(element) && collidesWithParameterName(element)) {
373+
return true;
374+
}
375+
}
376+
}
377+
return false;
378+
}
379+
200380
function transformAsyncFunctionBody(node: MethodDeclaration | AccessorDeclaration | FunctionDeclaration | FunctionExpression): FunctionBody;
201381
function transformAsyncFunctionBody(node: ArrowFunction): ConciseBody;
202382
function transformAsyncFunctionBody(node: FunctionLikeDeclaration): ConciseBody {
@@ -214,6 +394,13 @@ namespace ts {
214394
// passed to `__awaiter` is executed inside of the callback to the
215395
// promise constructor.
216396

397+
const savedEnclosingFunctionParameterNames = enclosingFunctionParameterNames;
398+
enclosingFunctionParameterNames = createUnderscoreEscapedMap<true>();
399+
for (const parameter of node.parameters) {
400+
recordDeclarationName(parameter, enclosingFunctionParameterNames);
401+
}
402+
403+
let result: ConciseBody;
217404
if (!isArrowFunction) {
218405
const statements: Statement[] = [];
219406
const statementOffset = addPrologue(statements, (<Block>node.body).statements, /*ensureUseStrict*/ false, visitor);
@@ -223,7 +410,7 @@ namespace ts {
223410
context,
224411
hasLexicalArguments,
225412
promiseConstructor,
226-
transformFunctionBodyWorker(<Block>node.body, statementOffset)
413+
transformAsyncFunctionBodyWorker(<Block>node.body, statementOffset)
227414
)
228415
)
229416
);
@@ -246,35 +433,36 @@ namespace ts {
246433
}
247434
}
248435

249-
return block;
436+
result = block;
250437
}
251438
else {
252439
const expression = createAwaiterHelper(
253440
context,
254441
hasLexicalArguments,
255442
promiseConstructor,
256-
transformFunctionBodyWorker(node.body)
443+
transformAsyncFunctionBodyWorker(node.body)
257444
);
258445

259446
const declarations = endLexicalEnvironment();
260447
if (some(declarations)) {
261448
const block = convertToFunctionBody(expression);
262-
return updateBlock(block, setTextRange(createNodeArray(concatenate(block.statements, declarations)), block.statements));
449+
result = updateBlock(block, setTextRange(createNodeArray(concatenate(block.statements, declarations)), block.statements));
450+
}
451+
else {
452+
result = expression;
263453
}
264-
265-
return expression;
266454
}
455+
456+
enclosingFunctionParameterNames = savedEnclosingFunctionParameterNames;
457+
return result;
267458
}
268459

269-
function transformFunctionBodyWorker(body: ConciseBody, start?: number) {
460+
function transformAsyncFunctionBodyWorker(body: ConciseBody, start?: number) {
270461
if (isBlock(body)) {
271-
return updateBlock(body, visitLexicalEnvironment(body.statements, visitor, context, start));
462+
return updateBlock(body, visitNodes(body.statements, asyncBodyVisitor, isStatement, start));
272463
}
273464
else {
274-
startLexicalEnvironment();
275-
const visited = convertToFunctionBody(visitNode(body, visitor, isConciseBody));
276-
const declarations = endLexicalEnvironment();
277-
return updateBlock(visited, setTextRange(createNodeArray(concatenate(visited.statements, declarations)), visited.statements));
465+
return convertToFunctionBody(visitNode(body, asyncBodyVisitor, isConciseBody));
278466
}
279467
}
280468

‎src/compiler/utilities.ts

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1766,6 +1766,51 @@ namespace ts {
17661766
return getAssignmentTargetKind(node) !== AssignmentKind.None;
17671767
}
17681768

1769+
export type NodeWithPossibleHoistedDeclaration =
1770+
| Block
1771+
| VariableStatement
1772+
| WithStatement
1773+
| IfStatement
1774+
| SwitchStatement
1775+
| CaseBlock
1776+
| CaseClause
1777+
| DefaultClause
1778+
| LabeledStatement
1779+
| ForStatement
1780+
| ForInStatement
1781+
| ForOfStatement
1782+
| DoStatement
1783+
| WhileStatement
1784+
| TryStatement
1785+
| CatchClause;
1786+
1787+
/**
1788+
* Indicates whether a node could contain a `var` VariableDeclarationList that contributes to
1789+
* the same `var` declaration scope as the node's parent.
1790+
*/
1791+
export function isNodeWithPossibleHoistedDeclaration(node: Node): node is NodeWithPossibleHoistedDeclaration {
1792+
switch (node.kind) {
1793+
case SyntaxKind.Block:
1794+
case SyntaxKind.VariableStatement:
1795+
case SyntaxKind.WithStatement:
1796+
case SyntaxKind.IfStatement:
1797+
case SyntaxKind.SwitchStatement:
1798+
case SyntaxKind.CaseBlock:
1799+
case SyntaxKind.CaseClause:
1800+
case SyntaxKind.DefaultClause:
1801+
case SyntaxKind.LabeledStatement:
1802+
case SyntaxKind.ForStatement:
1803+
case SyntaxKind.ForInStatement:
1804+
case SyntaxKind.ForOfStatement:
1805+
case SyntaxKind.DoStatement:
1806+
case SyntaxKind.WhileStatement:
1807+
case SyntaxKind.TryStatement:
1808+
case SyntaxKind.CatchClause:
1809+
return true;
1810+
}
1811+
return false;
1812+
}
1813+
17691814
function walkUp(node: Node, kind: SyntaxKind) {
17701815
while (node && node.kind === kind) {
17711816
node = node.parent;

0 commit comments

Comments
 (0)