Skip to content

Commit c85ae74

Browse files
mheibermheiber
committed
Disallow PrivateIdentifier in Optional Chains
1 parent e781a6b commit c85ae74

File tree

12 files changed

+212
-14
lines changed

12 files changed

+212
-14
lines changed

‎src/compiler/checker.ts‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22920,6 +22920,10 @@ namespace ts {
2292022920
const assignmentKind = getAssignmentTargetKind(node);
2292122921
const apparentType = getApparentType(assignmentKind !== AssignmentKind.None || isMethodAccessForCall(node) ? getWidenedType(leftType) : leftType);
2292222922
if (isPrivateIdentifier(right)) {
22923+
if (isOptionalChain(node)) {
22924+
grammarErrorOnNode(right, Diagnostics.An_optional_chain_cannot_contain_private_identifiers);
22925+
return anyType;
22926+
}
2292322927
checkExternalEmitHelpers(node, ExternalEmitHelpers.ClassPrivateFieldGet);
2292422928
}
2292522929
const isAnyLike = isTypeAny(apparentType) || apparentType === silentNeverType;
@@ -23301,9 +23305,14 @@ namespace ts {
2330123305
return true;
2330223306
}
2330323307
const prop = getPropertyOfType(type, propertyName);
23304-
return prop ? checkPropertyAccessibility(node, isSuper, type, prop)
23305-
// In js files properties of unions are allowed in completion
23306-
: isInJSFile(node) && (type.flags & TypeFlags.Union) !== 0 && (<UnionType>type).types.some(elementType => isValidPropertyAccessWithType(node, isSuper, propertyName, elementType));
23308+
if (prop) {
23309+
if (isOptionalChain(node) && isPrivateIdentifierPropertyDeclaration(prop.valueDeclaration)) {
23310+
return false;
23311+
}
23312+
return checkPropertyAccessibility(node, isSuper, type, prop);
23313+
}
23314+
// In js files properties of unions are allowed in completion
23315+
return isInJSFile(node) && (type.flags & TypeFlags.Union) !== 0 && (<UnionType>type).types.some(elementType => isValidPropertyAccessWithType(node, isSuper, propertyName, elementType));
2330723316
}
2330823317

2330923318
/**

‎src/compiler/diagnosticMessages.json‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5366,5 +5366,9 @@
53665366
"Private identifiers are only available when targeting ECMAScript 2015 and higher.": {
53675367
"category": "Error",
53685368
"code": 18028
5369+
},
5370+
"An optional chain cannot contain private identifiers.": {
5371+
"category": "Error",
5372+
"code": 18029
53695373
}
53705374
}

‎src/compiler/factory.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ namespace ts {
115115
return node;
116116
}
117117

118-
function createLiteralFromNode(sourceNode: PropertyNameLiteral): StringLiteral {
118+
function createLiteralFromNode(sourceNode: Exclude<PropertyNameLiteral, PrivateIdentifier>): StringLiteral {
119119
const node = createStringLiteral(getTextOfIdentifierOrLiteral(sourceNode));
120120
node.textSourceNode = sourceNode;
121121
return node;

‎src/compiler/parser.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4655,7 +4655,8 @@ namespace ts {
46554655
const propertyAccess = <PropertyAccessExpression>createNode(SyntaxKind.PropertyAccessExpression, expression.pos);
46564656
propertyAccess.expression = expression;
46574657
propertyAccess.questionDotToken = questionDotToken;
4658-
propertyAccess.name = parseRightSideOfDot(/*allowIdentifierNames*/ true, /*allowPrivateIdentifiers*/ !questionDotToken);
4658+
// checker will error on private identifiers in optional chains, so don't have to catch them here
4659+
propertyAccess.name = parseRightSideOfDot(/*allowIdentifierNames*/ true, /*allowPrivateIdentifiers*/ true);
46594660
if (questionDotToken || expression.flags & NodeFlags.OptionalChain) {
46604661
propertyAccess.flags |= NodeFlags.OptionalChain;
46614662
}

‎src/compiler/transformers/esnext.ts‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,13 @@ namespace ts {
4242
function flattenChain(chain: OptionalChain) {
4343
const links: OptionalChain[] = [chain];
4444
while (!chain.questionDotToken && !isTaggedTemplateExpression(chain)) {
45-
chain = cast(chain.expression, isOptionalChain);
46-
links.unshift(chain);
45+
if (isOptionalChain(chain.expression)) {
46+
chain = chain.expression;
47+
links.unshift(chain);
48+
}
49+
else {
50+
return { expression: chain.expression, chain: links };
51+
}
4752
}
4853
return { expression: chain.expression, chain: links };
4954
}

‎src/compiler/utilities.ts‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2957,10 +2957,11 @@ namespace ts {
29572957
}
29582958
}
29592959

2960-
export type PropertyNameLiteral = Identifier | StringLiteralLike | NumericLiteral;
2960+
export type PropertyNameLiteral = Identifier | PrivateIdentifier | StringLiteralLike | NumericLiteral;
29612961
export function isPropertyNameLiteral(node: Node): node is PropertyNameLiteral {
29622962
switch (node.kind) {
29632963
case SyntaxKind.Identifier:
2964+
case SyntaxKind.PrivateIdentifier:
29642965
case SyntaxKind.StringLiteral:
29652966
case SyntaxKind.NoSubstitutionTemplateLiteral:
29662967
case SyntaxKind.NumericLiteral:
@@ -2970,11 +2971,23 @@ namespace ts {
29702971
}
29712972
}
29722973
export function getTextOfIdentifierOrLiteral(node: PropertyNameLiteral): string {
2973-
return node.kind === SyntaxKind.Identifier ? idText(node) : node.text;
2974+
switch (node.kind) {
2975+
case SyntaxKind.Identifier:
2976+
case SyntaxKind.PrivateIdentifier:
2977+
return idText(node);
2978+
default:
2979+
return node.text;
2980+
}
29742981
}
29752982

29762983
export function getEscapedTextOfIdentifierOrLiteral(node: PropertyNameLiteral): __String {
2977-
return node.kind === SyntaxKind.Identifier ? node.escapedText : escapeLeadingUnderscores(node.text);
2984+
switch(node.kind) {
2985+
case SyntaxKind.Identifier:
2986+
case SyntaxKind.PrivateIdentifier:
2987+
return node.escapedText;
2988+
default:
2989+
return escapeLeadingUnderscores(node.text);
2990+
}
29782991
}
29792992

29802993
export function getPropertyNameForUniqueESSymbol(symbol: Symbol): __String {

‎tests/baselines/reference/privateIdentifierChain.1.errors.txt‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
tests/cases/conformance/expressions/optionalChaining/privateIdentifierChain/privateIdentifierChain.1.ts(8,15): error TS18029: An optional chain cannot contain private identifiers.
2+
tests/cases/conformance/expressions/optionalChaining/privateIdentifierChain/privateIdentifierChain.1.ts(9,9): error TS2532: Object is possibly 'undefined'.
3+
tests/cases/conformance/expressions/optionalChaining/privateIdentifierChain/privateIdentifierChain.1.ts(9,17): error TS18029: An optional chain cannot contain private identifiers.
4+
tests/cases/conformance/expressions/optionalChaining/privateIdentifierChain/privateIdentifierChain.1.ts(10,22): error TS18029: An optional chain cannot contain private identifiers.
5+
6+
7+
==== tests/cases/conformance/expressions/optionalChaining/privateIdentifierChain/privateIdentifierChain.1.ts (4 errors) ====
8+
class A {
9+
a?: A
10+
#b?: A;
11+
getA(): A {
12+
return new A();
13+
}
14+
constructor() {
15+
this?.#b; // Error
16+
~~
17+
!!! error TS18029: An optional chain cannot contain private identifiers.
18+
this?.a.#b; // Error
19+
~~~~~~~
20+
!!! error TS2532: Object is possibly 'undefined'.
21+
~~
22+
!!! error TS18029: An optional chain cannot contain private identifiers.
23+
this?.getA().#b; // Error
24+
~~
25+
!!! error TS18029: An optional chain cannot contain private identifiers.
26+
}
27+
}
28+

‎tests/baselines/reference/privateIdentifierChain.1.js‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
//// [privateIdentifierChain.1.ts]
2+
class A {
3+
a?: A
4+
#b?: A;
5+
getA(): A {
6+
return new A();
7+
}
8+
constructor() {
9+
this?.#b; // Error
10+
this?.a.#b; // Error
11+
this?.getA().#b; // Error
12+
}
13+
}
14+
15+
16+
//// [privateIdentifierChain.1.js]
17+
"use strict";
18+
class A {
19+
constructor() {
20+
this?.#b; // Error
21+
this?.a.#b; // Error
22+
this?.getA().#b; // Error
23+
}
24+
#b;
25+
getA() {
26+
return new A();
27+
}
28+
}

‎tests/baselines/reference/privateIdentifierChain.1.symbols‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
=== tests/cases/conformance/expressions/optionalChaining/privateIdentifierChain/privateIdentifierChain.1.ts ===
2+
class A {
3+
>A : Symbol(A, Decl(privateIdentifierChain.1.ts, 0, 0))
4+
5+
a?: A
6+
>a : Symbol(A.a, Decl(privateIdentifierChain.1.ts, 0, 9))
7+
>A : Symbol(A, Decl(privateIdentifierChain.1.ts, 0, 0))
8+
9+
#b?: A;
10+
>#b : Symbol(A.#b, Decl(privateIdentifierChain.1.ts, 1, 9))
11+
>A : Symbol(A, Decl(privateIdentifierChain.1.ts, 0, 0))
12+
13+
getA(): A {
14+
>getA : Symbol(A.getA, Decl(privateIdentifierChain.1.ts, 2, 11))
15+
>A : Symbol(A, Decl(privateIdentifierChain.1.ts, 0, 0))
16+
17+
return new A();
18+
>A : Symbol(A, Decl(privateIdentifierChain.1.ts, 0, 0))
19+
}
20+
constructor() {
21+
this?.#b; // Error
22+
>this : Symbol(A, Decl(privateIdentifierChain.1.ts, 0, 0))
23+
24+
this?.a.#b; // Error
25+
>this?.a : Symbol(A.a, Decl(privateIdentifierChain.1.ts, 0, 9))
26+
>this : Symbol(A, Decl(privateIdentifierChain.1.ts, 0, 0))
27+
>a : Symbol(A.a, Decl(privateIdentifierChain.1.ts, 0, 9))
28+
29+
this?.getA().#b; // Error
30+
>this?.getA : Symbol(A.getA, Decl(privateIdentifierChain.1.ts, 2, 11))
31+
>this : Symbol(A, Decl(privateIdentifierChain.1.ts, 0, 0))
32+
>getA : Symbol(A.getA, Decl(privateIdentifierChain.1.ts, 2, 11))
33+
}
34+
}
35+

‎tests/baselines/reference/privateIdentifierChain.1.types‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
=== tests/cases/conformance/expressions/optionalChaining/privateIdentifierChain/privateIdentifierChain.1.ts ===
2+
class A {
3+
>A : A
4+
5+
a?: A
6+
>a : A | undefined
7+
8+
#b?: A;
9+
>#b : A | undefined
10+
11+
getA(): A {
12+
>getA : () => A
13+
14+
return new A();
15+
>new A() : A
16+
>A : typeof A
17+
}
18+
constructor() {
19+
this?.#b; // Error
20+
>this?.#b : any
21+
>this : this
22+
23+
this?.a.#b; // Error
24+
>this?.a.#b : any
25+
>this?.a : A | undefined
26+
>this : this
27+
>a : A | undefined
28+
29+
this?.getA().#b; // Error
30+
>this?.getA().#b : any
31+
>this?.getA() : A
32+
>this?.getA : () => A
33+
>this : this
34+
>getA : () => A
35+
}
36+
}
37+

0 commit comments

Comments
 (0)