Skip to content

Commit 5c8def9

Browse files
elibarzilayelibarzilay
committed
Fix emitting ?.
Use `emit()` for writing `questionDotToken`, leading to properly calling the emit hooks (which `emitTokenWithComment` doesn't) and printing the comments. This fixes #35372 by calling its hooks to set the `.__pos` and `.__end` fields. Also, remove `getDotOrQuestionDotToken` which was used only here -- mainly because it seems likely to encourage misusing the `questionDotToken` again. Also, fix a bunch of `visitor` -> `tokenVisiton` calls in `visitorPublic.ts`.
1 parent 67930fc commit 5c8def9

10 files changed

+159
-15
lines changed

‎src/compiler/emitter.ts

+7-2
Original file line numberDiff line numberDiff line change
@@ -2273,7 +2273,7 @@ namespace ts {
22732273

22742274
function emitPropertyAccessExpression(node: PropertyAccessExpression) {
22752275
const expression = cast(emitExpression(node.expression), isExpression);
2276-
const token = getDotOrQuestionDotToken(node);
2276+
const token = node.questionDotToken || createNode(SyntaxKind.DotToken, node.expression.end, node.name.pos) as DotToken;
22772277
const indentBeforeDot = needsIndentation(node, node.expression, token);
22782278
const indentAfterDot = needsIndentation(node, token, node.name);
22792279

@@ -2289,7 +2289,12 @@ namespace ts {
22892289
writePunctuation(".");
22902290
}
22912291

2292-
emitTokenWithComment(token.kind, node.expression.end, writePunctuation, node);
2292+
if (node.questionDotToken) {
2293+
emit(token);
2294+
}
2295+
else {
2296+
emitTokenWithComment(token.kind, node.expression.end, writePunctuation, node);
2297+
}
22932298
increaseIndentIf(indentAfterDot, /*writeSpaceIfNotIndenting*/ false);
22942299
emit(node.name);
22952300
decreaseIndentIf(indentBeforeDot, indentAfterDot);

‎src/compiler/utilities.ts

-4
Original file line numberDiff line numberDiff line change
@@ -5052,10 +5052,6 @@ namespace ts {
50525052
}
50535053
}
50545054

5055-
export function getDotOrQuestionDotToken(node: PropertyAccessExpression) {
5056-
return node.questionDotToken || createNode(SyntaxKind.DotToken, node.expression.end, node.name.pos) as DotToken;
5057-
}
5058-
50595055
export function isNamedImportsOrExports(node: Node): node is NamedImportsOrExports {
50605056
return node.kind === SyntaxKind.NamedImports || node.kind === SyntaxKind.NamedExports;
50615057
}

‎src/compiler/visitorPublic.ts

+8-8
Original file line numberDiff line numberDiff line change
@@ -459,7 +459,7 @@ namespace ts {
459459
if (node.flags & NodeFlags.OptionalChain) {
460460
return updatePropertyAccessChain(<PropertyAccessChain>node,
461461
visitNode((<PropertyAccessChain>node).expression, visitor, isExpression),
462-
visitNode((<PropertyAccessChain>node).questionDotToken, visitor, isToken),
462+
visitNode((<PropertyAccessChain>node).questionDotToken, tokenVisitor, isToken),
463463
visitNode((<PropertyAccessChain>node).name, visitor, isIdentifier));
464464
}
465465
return updatePropertyAccess(<PropertyAccessExpression>node,
@@ -470,7 +470,7 @@ namespace ts {
470470
if (node.flags & NodeFlags.OptionalChain) {
471471
return updateElementAccessChain(<ElementAccessChain>node,
472472
visitNode((<ElementAccessChain>node).expression, visitor, isExpression),
473-
visitNode((<ElementAccessChain>node).questionDotToken, visitor, isToken),
473+
visitNode((<ElementAccessChain>node).questionDotToken, tokenVisitor, isToken),
474474
visitNode((<ElementAccessChain>node).argumentExpression, visitor, isExpression));
475475
}
476476
return updateElementAccess(<ElementAccessExpression>node,
@@ -481,7 +481,7 @@ namespace ts {
481481
if (node.flags & NodeFlags.OptionalChain) {
482482
return updateCallChain(<CallChain>node,
483483
visitNode((<CallChain>node).expression, visitor, isExpression),
484-
visitNode((<CallChain>node).questionDotToken, visitor, isToken),
484+
visitNode((<CallChain>node).questionDotToken, tokenVisitor, isToken),
485485
nodesVisitor((<CallChain>node).typeArguments, visitor, isTypeNode),
486486
nodesVisitor((<CallChain>node).arguments, visitor, isExpression));
487487
}
@@ -527,7 +527,7 @@ namespace ts {
527527
nodesVisitor((<ArrowFunction>node).typeParameters, visitor, isTypeParameterDeclaration),
528528
visitParameterList((<ArrowFunction>node).parameters, visitor, context, nodesVisitor),
529529
visitNode((<ArrowFunction>node).type, visitor, isTypeNode),
530-
visitNode((<ArrowFunction>node).equalsGreaterThanToken, visitor, isToken),
530+
visitNode((<ArrowFunction>node).equalsGreaterThanToken, tokenVisitor, isToken),
531531
visitFunctionBody((<ArrowFunction>node).body, visitor, context));
532532

533533
case SyntaxKind.DeleteExpression:
@@ -558,14 +558,14 @@ namespace ts {
558558
return updateBinary(<BinaryExpression>node,
559559
visitNode((<BinaryExpression>node).left, visitor, isExpression),
560560
visitNode((<BinaryExpression>node).right, visitor, isExpression),
561-
visitNode((<BinaryExpression>node).operatorToken, visitor, isToken));
561+
visitNode((<BinaryExpression>node).operatorToken, tokenVisitor, isToken));
562562

563563
case SyntaxKind.ConditionalExpression:
564564
return updateConditional(<ConditionalExpression>node,
565565
visitNode((<ConditionalExpression>node).condition, visitor, isExpression),
566-
visitNode((<ConditionalExpression>node).questionToken, visitor, isToken),
566+
visitNode((<ConditionalExpression>node).questionToken, tokenVisitor, isToken),
567567
visitNode((<ConditionalExpression>node).whenTrue, visitor, isExpression),
568-
visitNode((<ConditionalExpression>node).colonToken, visitor, isToken),
568+
visitNode((<ConditionalExpression>node).colonToken, tokenVisitor, isToken),
569569
visitNode((<ConditionalExpression>node).whenFalse, visitor, isExpression));
570570

571571
case SyntaxKind.TemplateExpression:
@@ -659,7 +659,7 @@ namespace ts {
659659

660660
case SyntaxKind.ForOfStatement:
661661
return updateForOf(<ForOfStatement>node,
662-
visitNode((<ForOfStatement>node).awaitModifier, visitor, isToken),
662+
visitNode((<ForOfStatement>node).awaitModifier, tokenVisitor, isToken),
663663
visitNode((<ForOfStatement>node).initializer, visitor, isForInitializer),
664664
visitNode((<ForOfStatement>node).expression, visitor, isExpression),
665665
visitNode((<ForOfStatement>node).statement, visitor, isStatement, liftToBlock));

‎src/services/textChanges.ts

-1
Original file line numberDiff line numberDiff line change
@@ -966,7 +966,6 @@ namespace ts.textChanges {
966966
function createWriter(newLine: string): TextChangesWriter {
967967
let lastNonTriviaPosition = 0;
968968

969-
970969
const writer = createTextWriter(newLine);
971970
const onEmitNode: PrintHandlers["onEmitNode"] = (hint, node, printCallback) => {
972971
if (node) {

‎tests/baselines/reference/propertyAccessExpressionInnerComments.js

+29
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,23 @@
1313
/*1*/Array
1414
// Single-line comment
1515
/*2*/./*3*/toString/*4*/
16+
17+
/* Existing issue: the "2" comments below are duplicated and "3"s are missing */
18+
19+
/*1*/Array/*2*/?./*3*/toString/*4*/
20+
21+
/*1*/Array
22+
/*2*/?./*3*/
23+
// Single-line comment
24+
toString/*4*/
25+
26+
/*1*/Array/*2*/?./*3*/
27+
// Single-line comment
28+
toString/*4*/
29+
30+
/*1*/Array
31+
// Single-line comment
32+
/*2*/?./*3*/toString/*4*/
1633

1734

1835
//// [propertyAccessExpressionInnerComments.js]
@@ -27,3 +44,15 @@
2744
/*1*/ Array
2845
// Single-line comment
2946
/*2*/ . /*3*/toString; /*4*/
47+
/* Existing issue: the "2" comments below are duplicated and "3"s are missing */
48+
/*1*/ Array /*2*/ === null || Array /*2*/ === void 0 ? void 0 : Array /*2*/.toString; /*4*/
49+
/*1*/ Array === null || Array === void 0 ? void 0 : Array
50+
/*2*/ .
51+
// Single-line comment
52+
toString; /*4*/
53+
/*1*/ Array /*2*/ === null || Array /*2*/ === void 0 ? void 0 : Array /*2*/.
54+
// Single-line comment
55+
toString; /*4*/
56+
/*1*/ Array === null || Array === void 0 ? void 0 : Array
57+
// Single-line comment
58+
/*2*/ .toString; /*4*/

‎tests/baselines/reference/propertyAccessExpressionInnerComments.symbols

+32
Original file line numberDiff line numberDiff line change
@@ -29,3 +29,35 @@
2929
/*2*/./*3*/toString/*4*/
3030
>toString : Symbol(Function.toString, Decl(lib.es5.d.ts, --, --))
3131

32+
/* Existing issue: the "2" comments below are duplicated and "3"s are missing */
33+
34+
/*1*/Array/*2*/?./*3*/toString/*4*/
35+
>Array/*2*/?./*3*/toString : Symbol(Function.toString, Decl(lib.es5.d.ts, --, --))
36+
>Array : Symbol(Array, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --))
37+
>toString : Symbol(Function.toString, Decl(lib.es5.d.ts, --, --))
38+
39+
/*1*/Array
40+
>Array/*2*/?./*3*/ // Single-line comment toString : Symbol(Function.toString, Decl(lib.es5.d.ts, --, --))
41+
>Array : Symbol(Array, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --))
42+
43+
/*2*/?./*3*/
44+
// Single-line comment
45+
toString/*4*/
46+
>toString : Symbol(Function.toString, Decl(lib.es5.d.ts, --, --))
47+
48+
/*1*/Array/*2*/?./*3*/
49+
>Array/*2*/?./*3*/ // Single-line comment toString : Symbol(Function.toString, Decl(lib.es5.d.ts, --, --))
50+
>Array : Symbol(Array, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --))
51+
52+
// Single-line comment
53+
toString/*4*/
54+
>toString : Symbol(Function.toString, Decl(lib.es5.d.ts, --, --))
55+
56+
/*1*/Array
57+
>Array // Single-line comment /*2*/?./*3*/toString : Symbol(Function.toString, Decl(lib.es5.d.ts, --, --))
58+
>Array : Symbol(Array, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --))
59+
60+
// Single-line comment
61+
/*2*/?./*3*/toString/*4*/
62+
>toString : Symbol(Function.toString, Decl(lib.es5.d.ts, --, --))
63+

‎tests/baselines/reference/propertyAccessExpressionInnerComments.types

+32
Original file line numberDiff line numberDiff line change
@@ -29,3 +29,35 @@
2929
/*2*/./*3*/toString/*4*/
3030
>toString : () => string
3131

32+
/* Existing issue: the "2" comments below are duplicated and "3"s are missing */
33+
34+
/*1*/Array/*2*/?./*3*/toString/*4*/
35+
>Array/*2*/?./*3*/toString : () => string
36+
>Array : ArrayConstructor
37+
>toString : () => string
38+
39+
/*1*/Array
40+
>Array/*2*/?./*3*/ // Single-line comment toString : () => string
41+
>Array : ArrayConstructor
42+
43+
/*2*/?./*3*/
44+
// Single-line comment
45+
toString/*4*/
46+
>toString : () => string
47+
48+
/*1*/Array/*2*/?./*3*/
49+
>Array/*2*/?./*3*/ // Single-line comment toString : () => string
50+
>Array : ArrayConstructor
51+
52+
// Single-line comment
53+
toString/*4*/
54+
>toString : () => string
55+
56+
/*1*/Array
57+
>Array // Single-line comment /*2*/?./*3*/toString : () => string
58+
>Array : ArrayConstructor
59+
60+
// Single-line comment
61+
/*2*/?./*3*/toString/*4*/
62+
>toString : () => string
63+

‎tests/cases/compiler/propertyAccessExpressionInnerComments.ts

+17
Original file line numberDiff line numberDiff line change
@@ -12,3 +12,20 @@
1212
/*1*/Array
1313
// Single-line comment
1414
/*2*/./*3*/toString/*4*/
15+
16+
/* Existing issue: the "2" comments below are duplicated and "3"s are missing */
17+
18+
/*1*/Array/*2*/?./*3*/toString/*4*/
19+
20+
/*1*/Array
21+
/*2*/?./*3*/
22+
// Single-line comment
23+
toString/*4*/
24+
25+
/*1*/Array/*2*/?./*3*/
26+
// Single-line comment
27+
toString/*4*/
28+
29+
/*1*/Array
30+
// Single-line comment
31+
/*2*/?./*3*/toString/*4*/

‎tests/cases/fourslash/extract-const2.ts

+16
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
////function foo(bar: number) {
4+
//// /*a*/bar.toString();/*b*/
5+
////}
6+
7+
goTo.select("a", "b");
8+
edit.applyRefactor({
9+
refactorName: "Extract Symbol",
10+
actionName: "constant_scope_0",
11+
actionDescription: "Extract to constant in enclosing scope",
12+
newContent:
13+
`function foo(bar: number) {
14+
const /*RENAME*/newLocal = bar.toString();
15+
}`
16+
});

‎tests/cases/fourslash/extract-const3.ts

+18
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
// GH#35372
4+
5+
////function foo(bar?: number) {
6+
//// /*a*/bar?.toString();/*b*/
7+
////}
8+
9+
goTo.select("a", "b");
10+
edit.applyRefactor({
11+
refactorName: "Extract Symbol",
12+
actionName: "constant_scope_0",
13+
actionDescription: "Extract to constant in enclosing scope",
14+
newContent:
15+
`function foo(bar?: number) {
16+
const /*RENAME*/newLocal = bar?.toString();
17+
}`
18+
});

0 commit comments

Comments
 (0)