Skip to content

Commit d3d088f

Browse files
a-tarasyukrbuckton
a-tarasyuk
and
rbuckton
authored
fix(44639): Transpilation of optional chaining combined with type casting results in function call losing its context (microsoft#44666)
* fix(44639): Fix emit for optional chain with type assertions * Move ASI-blocking parens out of ts transform * Add missing comments from PartiallyEmittedExpression+minor cleanup * Avoid comment duplication on copied receiver Co-authored-by: Ron Buckton <[email protected]>
1 parent 68bf5a5 commit d3d088f

10 files changed

+280
-18
lines changed

‎src/compiler/emitter.ts

Lines changed: 53 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2766,7 +2766,7 @@ namespace ts {
27662766
function emitYieldExpression(node: YieldExpression) {
27672767
emitTokenWithComment(SyntaxKind.YieldKeyword, node.pos, writeKeyword, node);
27682768
emit(node.asteriskToken);
2769-
emitExpressionWithLeadingSpace(node.expression, parenthesizer.parenthesizeExpressionForDisallowedComma);
2769+
emitExpressionWithLeadingSpace(node.expression && parenthesizeExpressionForNoAsi(node.expression), parenthesizeExpressionForNoAsiAndDisallowedComma);
27702770
}
27712771

27722772
function emitSpreadElement(node: SpreadElement) {
@@ -2990,9 +2990,49 @@ namespace ts {
29902990
return pos;
29912991
}
29922992

2993+
function commentWillEmitNewLine(node: CommentRange) {
2994+
return node.kind === SyntaxKind.SingleLineCommentTrivia || !!node.hasTrailingNewLine;
2995+
}
2996+
2997+
function willEmitLeadingNewLine(node: Expression): boolean {
2998+
if (!currentSourceFile) return false;
2999+
if (some(getLeadingCommentRanges(currentSourceFile.text, node.pos), commentWillEmitNewLine)) return true;
3000+
if (some(getSyntheticLeadingComments(node), commentWillEmitNewLine)) return true;
3001+
if (isPartiallyEmittedExpression(node)) {
3002+
if (node.pos !== node.expression.pos) {
3003+
if (some(getTrailingCommentRanges(currentSourceFile.text, node.expression.pos), commentWillEmitNewLine)) return true;
3004+
}
3005+
return willEmitLeadingNewLine(node.expression);
3006+
}
3007+
return false;
3008+
}
3009+
3010+
/**
3011+
* Wraps an expression in parens if we would emit a leading comment that would introduce a line separator
3012+
* between the node and its parent.
3013+
*/
3014+
function parenthesizeExpressionForNoAsi(node: Expression) {
3015+
if (!commentsDisabled && isPartiallyEmittedExpression(node) && willEmitLeadingNewLine(node)) {
3016+
const parseNode = getParseTreeNode(node);
3017+
if (parseNode && isParenthesizedExpression(parseNode)) {
3018+
// If the original node was a parenthesized expression, restore it to preserve comment and source map emit
3019+
const parens = factory.createParenthesizedExpression(node.expression);
3020+
setOriginalNode(parens, node);
3021+
setTextRange(parens, parseNode);
3022+
return parens;
3023+
}
3024+
return factory.createParenthesizedExpression(node);
3025+
}
3026+
return node;
3027+
}
3028+
3029+
function parenthesizeExpressionForNoAsiAndDisallowedComma(node: Expression) {
3030+
return parenthesizeExpressionForNoAsi(parenthesizer.parenthesizeExpressionForDisallowedComma(node));
3031+
}
3032+
29933033
function emitReturnStatement(node: ReturnStatement) {
29943034
emitTokenWithComment(SyntaxKind.ReturnKeyword, node.pos, writeKeyword, /*contextNode*/ node);
2995-
emitExpressionWithLeadingSpace(node.expression);
3035+
emitExpressionWithLeadingSpace(node.expression && parenthesizeExpressionForNoAsi(node.expression), parenthesizeExpressionForNoAsi);
29963036
writeTrailingSemicolon();
29973037
}
29983038

@@ -3024,7 +3064,7 @@ namespace ts {
30243064

30253065
function emitThrowStatement(node: ThrowStatement) {
30263066
emitTokenWithComment(SyntaxKind.ThrowKeyword, node.pos, writeKeyword, node);
3027-
emitExpressionWithLeadingSpace(node.expression);
3067+
emitExpressionWithLeadingSpace(parenthesizeExpressionForNoAsi(node.expression), parenthesizeExpressionForNoAsi);
30283068
writeTrailingSemicolon();
30293069
}
30303070

@@ -3974,7 +4014,14 @@ namespace ts {
39744014
// Transformation nodes
39754015

39764016
function emitPartiallyEmittedExpression(node: PartiallyEmittedExpression) {
4017+
const emitFlags = getEmitFlags(node);
4018+
if (!(emitFlags & EmitFlags.NoLeadingComments) && node.pos !== node.expression.pos) {
4019+
emitTrailingCommentsOfPosition(node.expression.pos);
4020+
}
39774021
emitExpression(node.expression);
4022+
if (!(emitFlags & EmitFlags.NoTrailingComments) && node.end !== node.expression.end) {
4023+
emitLeadingCommentsOfPosition(node.expression.end);
4024+
}
39784025
}
39794026

39804027
function emitCommaList(node: CommaListExpression) {
@@ -4362,10 +4409,8 @@ namespace ts {
43624409
// Emit this child.
43634410
previousSourceFileTextKind = recordBundleFileInternalSectionStart(child);
43644411
if (shouldEmitInterveningComments) {
4365-
if (emitTrailingCommentsOfPosition) {
4366-
const commentRange = getCommentRange(child);
4367-
emitTrailingCommentsOfPosition(commentRange.pos);
4368-
}
4412+
const commentRange = getCommentRange(child);
4413+
emitTrailingCommentsOfPosition(commentRange.pos);
43694414
}
43704415
else {
43714416
shouldEmitInterveningComments = mayEmitInterveningComments;
@@ -4743,7 +4788,7 @@ namespace ts {
47434788
function writeLineSeparatorsAndIndentBefore(node: Node, parent: Node): boolean {
47444789
const leadingNewlines = preserveSourceNewlines && getLeadingLineTerminatorCount(parent, [node], ListFormat.None);
47454790
if (leadingNewlines) {
4746-
writeLinesAndIndent(leadingNewlines, /*writeLinesIfNotIndenting*/ false);
4791+
writeLinesAndIndent(leadingNewlines, /*writeSpaceIfNotIndenting*/ false);
47474792
}
47484793
return !!leadingNewlines;
47494794
}

‎src/compiler/transformers/es2020.ts

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -122,11 +122,11 @@ namespace ts {
122122

123123
function visitOptionalExpression(node: OptionalChain, captureThisArg: boolean, isDelete: boolean): Expression {
124124
const { expression, chain } = flattenChain(node);
125-
const left = visitNonOptionalExpression(expression, isCallChain(chain[0]), /*isDelete*/ false);
126-
const leftThisArg = isSyntheticReference(left) ? left.thisArg : undefined;
127-
let leftExpression = isSyntheticReference(left) ? left.expression : left;
128-
let capturedLeft: Expression = leftExpression;
129-
if (!isSimpleCopiableExpression(leftExpression)) {
125+
const left = visitNonOptionalExpression(skipPartiallyEmittedExpressions(expression), isCallChain(chain[0]), /*isDelete*/ false);
126+
let leftThisArg = isSyntheticReference(left) ? left.thisArg : undefined;
127+
let capturedLeft = isSyntheticReference(left) ? left.expression : left;
128+
let leftExpression = factory.restoreOuterExpressions(expression, capturedLeft, OuterExpressionKinds.PartiallyEmittedExpressions);
129+
if (!isSimpleCopiableExpression(capturedLeft)) {
130130
capturedLeft = factory.createTempVariable(hoistVariableDeclaration);
131131
leftExpression = factory.createAssignment(capturedLeft, leftExpression);
132132
}
@@ -152,6 +152,10 @@ namespace ts {
152152
break;
153153
case SyntaxKind.CallExpression:
154154
if (i === 0 && leftThisArg) {
155+
if (!isGeneratedIdentifier(leftThisArg)) {
156+
leftThisArg = factory.cloneNode(leftThisArg);
157+
addEmitFlags(leftThisArg, EmitFlags.NoComments);
158+
}
155159
rightExpression = factory.createFunctionCallCall(
156160
rightExpression,
157161
leftThisArg.kind === SyntaxKind.SuperKeyword ? factory.createThis() : leftThisArg,

‎src/compiler/transformers/ts.ts

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2237,11 +2237,10 @@ namespace ts {
22372237
// we can safely elide the parentheses here, as a new synthetic
22382238
// ParenthesizedExpression will be inserted if we remove parentheses too
22392239
// aggressively.
2240-
// HOWEVER - if there are leading comments on the expression itself, to handle ASI
2241-
// correctly for return and throw, we must keep the parenthesis
2242-
if (length(getLeadingCommentRangesOfNode(expression, currentSourceFile))) {
2243-
return factory.updateParenthesizedExpression(node, expression);
2244-
}
2240+
//
2241+
// If there are leading comments on the expression itself, the emitter will handle ASI
2242+
// for return, throw, and yield by re-introducing parenthesis during emit on an as-need
2243+
// basis.
22452244
return factory.createPartiallyEmittedExpression(expression, node);
22462245
}
22472246

‎tests/baselines/reference/optionalChainingInTypeAssertions(target=es2015).js

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
//// [optionalChainingInTypeAssertions.ts]
2+
class Foo {
3+
m() {}
4+
}
5+
6+
const foo = new Foo();
7+
8+
(foo.m as any)?.();
9+
(<any>foo.m)?.();
10+
11+
/*a1*/(/*a2*/foo.m as any/*a3*/)/*a4*/?.();
12+
/*b1*/(/*b2*/<any>foo.m/*b3*/)/*b4*/?.();
13+
14+
15+
//// [optionalChainingInTypeAssertions.js]
16+
var _a, _b, _c, _d;
17+
class Foo {
18+
m() { }
19+
}
20+
const foo = new Foo();
21+
(_a = foo.m) === null || _a === void 0 ? void 0 : _a.call(foo);
22+
(_b = foo.m) === null || _b === void 0 ? void 0 : _b.call(foo);
23+
/*a1*/ (_c = /*a2*/ foo.m /*a3*/ /*a4*/) === null || _c === void 0 ? void 0 : _c.call(foo);
24+
/*b1*/ (_d = /*b2*/ foo.m /*b3*/ /*b4*/) === null || _d === void 0 ? void 0 : _d.call(foo);

‎tests/baselines/reference/optionalChainingInTypeAssertions(target=es2015).symbols

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
=== tests/cases/conformance/expressions/optionalChaining/optionalChainingInTypeAssertions.ts ===
2+
class Foo {
3+
>Foo : Symbol(Foo, Decl(optionalChainingInTypeAssertions.ts, 0, 0))
4+
5+
m() {}
6+
>m : Symbol(Foo.m, Decl(optionalChainingInTypeAssertions.ts, 0, 11))
7+
}
8+
9+
const foo = new Foo();
10+
>foo : Symbol(foo, Decl(optionalChainingInTypeAssertions.ts, 4, 5))
11+
>Foo : Symbol(Foo, Decl(optionalChainingInTypeAssertions.ts, 0, 0))
12+
13+
(foo.m as any)?.();
14+
>foo.m : Symbol(Foo.m, Decl(optionalChainingInTypeAssertions.ts, 0, 11))
15+
>foo : Symbol(foo, Decl(optionalChainingInTypeAssertions.ts, 4, 5))
16+
>m : Symbol(Foo.m, Decl(optionalChainingInTypeAssertions.ts, 0, 11))
17+
18+
(<any>foo.m)?.();
19+
>foo.m : Symbol(Foo.m, Decl(optionalChainingInTypeAssertions.ts, 0, 11))
20+
>foo : Symbol(foo, Decl(optionalChainingInTypeAssertions.ts, 4, 5))
21+
>m : Symbol(Foo.m, Decl(optionalChainingInTypeAssertions.ts, 0, 11))
22+
23+
/*a1*/(/*a2*/foo.m as any/*a3*/)/*a4*/?.();
24+
>foo.m : Symbol(Foo.m, Decl(optionalChainingInTypeAssertions.ts, 0, 11))
25+
>foo : Symbol(foo, Decl(optionalChainingInTypeAssertions.ts, 4, 5))
26+
>m : Symbol(Foo.m, Decl(optionalChainingInTypeAssertions.ts, 0, 11))
27+
28+
/*b1*/(/*b2*/<any>foo.m/*b3*/)/*b4*/?.();
29+
>foo.m : Symbol(Foo.m, Decl(optionalChainingInTypeAssertions.ts, 0, 11))
30+
>foo : Symbol(foo, Decl(optionalChainingInTypeAssertions.ts, 4, 5))
31+
>m : Symbol(Foo.m, Decl(optionalChainingInTypeAssertions.ts, 0, 11))
32+

‎tests/baselines/reference/optionalChainingInTypeAssertions(target=es2015).types

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
=== tests/cases/conformance/expressions/optionalChaining/optionalChainingInTypeAssertions.ts ===
2+
class Foo {
3+
>Foo : Foo
4+
5+
m() {}
6+
>m : () => void
7+
}
8+
9+
const foo = new Foo();
10+
>foo : Foo
11+
>new Foo() : Foo
12+
>Foo : typeof Foo
13+
14+
(foo.m as any)?.();
15+
>(foo.m as any)?.() : any
16+
>(foo.m as any) : any
17+
>foo.m as any : any
18+
>foo.m : () => void
19+
>foo : Foo
20+
>m : () => void
21+
22+
(<any>foo.m)?.();
23+
>(<any>foo.m)?.() : any
24+
>(<any>foo.m) : any
25+
><any>foo.m : any
26+
>foo.m : () => void
27+
>foo : Foo
28+
>m : () => void
29+
30+
/*a1*/(/*a2*/foo.m as any/*a3*/)/*a4*/?.();
31+
>(/*a2*/foo.m as any/*a3*/)/*a4*/?.() : any
32+
>(/*a2*/foo.m as any/*a3*/) : any
33+
>foo.m as any : any
34+
>foo.m : () => void
35+
>foo : Foo
36+
>m : () => void
37+
38+
/*b1*/(/*b2*/<any>foo.m/*b3*/)/*b4*/?.();
39+
>(/*b2*/<any>foo.m/*b3*/)/*b4*/?.() : any
40+
>(/*b2*/<any>foo.m/*b3*/) : any
41+
><any>foo.m : any
42+
>foo.m : () => void
43+
>foo : Foo
44+
>m : () => void
45+

‎tests/baselines/reference/optionalChainingInTypeAssertions(target=esnext).js

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
//// [optionalChainingInTypeAssertions.ts]
2+
class Foo {
3+
m() {}
4+
}
5+
6+
const foo = new Foo();
7+
8+
(foo.m as any)?.();
9+
(<any>foo.m)?.();
10+
11+
/*a1*/(/*a2*/foo.m as any/*a3*/)/*a4*/?.();
12+
/*b1*/(/*b2*/<any>foo.m/*b3*/)/*b4*/?.();
13+
14+
15+
//// [optionalChainingInTypeAssertions.js]
16+
class Foo {
17+
m() { }
18+
}
19+
const foo = new Foo();
20+
foo.m?.();
21+
foo.m?.();
22+
/*a1*/ /*a2*/ foo.m /*a3*/ /*a4*/?.();
23+
/*b1*/ /*b2*/ foo.m /*b3*/ /*b4*/?.();

‎tests/baselines/reference/optionalChainingInTypeAssertions(target=esnext).symbols

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
=== tests/cases/conformance/expressions/optionalChaining/optionalChainingInTypeAssertions.ts ===
2+
class Foo {
3+
>Foo : Symbol(Foo, Decl(optionalChainingInTypeAssertions.ts, 0, 0))
4+
5+
m() {}
6+
>m : Symbol(Foo.m, Decl(optionalChainingInTypeAssertions.ts, 0, 11))
7+
}
8+
9+
const foo = new Foo();
10+
>foo : Symbol(foo, Decl(optionalChainingInTypeAssertions.ts, 4, 5))
11+
>Foo : Symbol(Foo, Decl(optionalChainingInTypeAssertions.ts, 0, 0))
12+
13+
(foo.m as any)?.();
14+
>foo.m : Symbol(Foo.m, Decl(optionalChainingInTypeAssertions.ts, 0, 11))
15+
>foo : Symbol(foo, Decl(optionalChainingInTypeAssertions.ts, 4, 5))
16+
>m : Symbol(Foo.m, Decl(optionalChainingInTypeAssertions.ts, 0, 11))
17+
18+
(<any>foo.m)?.();
19+
>foo.m : Symbol(Foo.m, Decl(optionalChainingInTypeAssertions.ts, 0, 11))
20+
>foo : Symbol(foo, Decl(optionalChainingInTypeAssertions.ts, 4, 5))
21+
>m : Symbol(Foo.m, Decl(optionalChainingInTypeAssertions.ts, 0, 11))
22+
23+
/*a1*/(/*a2*/foo.m as any/*a3*/)/*a4*/?.();
24+
>foo.m : Symbol(Foo.m, Decl(optionalChainingInTypeAssertions.ts, 0, 11))
25+
>foo : Symbol(foo, Decl(optionalChainingInTypeAssertions.ts, 4, 5))
26+
>m : Symbol(Foo.m, Decl(optionalChainingInTypeAssertions.ts, 0, 11))
27+
28+
/*b1*/(/*b2*/<any>foo.m/*b3*/)/*b4*/?.();
29+
>foo.m : Symbol(Foo.m, Decl(optionalChainingInTypeAssertions.ts, 0, 11))
30+
>foo : Symbol(foo, Decl(optionalChainingInTypeAssertions.ts, 4, 5))
31+
>m : Symbol(Foo.m, Decl(optionalChainingInTypeAssertions.ts, 0, 11))
32+

‎tests/baselines/reference/optionalChainingInTypeAssertions(target=esnext).types

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
=== tests/cases/conformance/expressions/optionalChaining/optionalChainingInTypeAssertions.ts ===
2+
class Foo {
3+
>Foo : Foo
4+
5+
m() {}
6+
>m : () => void
7+
}
8+
9+
const foo = new Foo();
10+
>foo : Foo
11+
>new Foo() : Foo
12+
>Foo : typeof Foo
13+
14+
(foo.m as any)?.();
15+
>(foo.m as any)?.() : any
16+
>(foo.m as any) : any
17+
>foo.m as any : any
18+
>foo.m : () => void
19+
>foo : Foo
20+
>m : () => void
21+
22+
(<any>foo.m)?.();
23+
>(<any>foo.m)?.() : any
24+
>(<any>foo.m) : any
25+
><any>foo.m : any
26+
>foo.m : () => void
27+
>foo : Foo
28+
>m : () => void
29+
30+
/*a1*/(/*a2*/foo.m as any/*a3*/)/*a4*/?.();
31+
>(/*a2*/foo.m as any/*a3*/)/*a4*/?.() : any
32+
>(/*a2*/foo.m as any/*a3*/) : any
33+
>foo.m as any : any
34+
>foo.m : () => void
35+
>foo : Foo
36+
>m : () => void
37+
38+
/*b1*/(/*b2*/<any>foo.m/*b3*/)/*b4*/?.();
39+
>(/*b2*/<any>foo.m/*b3*/)/*b4*/?.() : any
40+
>(/*b2*/<any>foo.m/*b3*/) : any
41+
><any>foo.m : any
42+
>foo.m : () => void
43+
>foo : Foo
44+
>m : () => void
45+

‎tests/cases/conformance/expressions/optionalChaining/optionalChainingInTypeAssertions.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// @target: es2015, esnext
2+
3+
class Foo {
4+
m() {}
5+
}
6+
7+
const foo = new Foo();
8+
9+
(foo.m as any)?.();
10+
(<any>foo.m)?.();
11+
12+
/*a1*/(/*a2*/foo.m as any/*a3*/)/*a4*/?.();
13+
/*b1*/(/*b2*/<any>foo.m/*b3*/)/*b4*/?.();

0 commit comments

Comments
 (0)