Parse error on private identifier optional chain (#35987)

Previously, this error was reported in the checker, so JS files with checkJs: false were not erroring on this invalid syntax.
microsoft · Jan 8, 2020 · f84b2d2 · f84b2d2
1 parent 9fbcdb1
commit f84b2d2
Show file tree

Hide file tree

Showing 8 changed files with 74 additions and 8 deletions.
diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
@@ -23234,10 +23234,6 @@ namespace ts {
             const assignmentKind = getAssignmentTargetKind(node);
             const apparentType = getApparentType(assignmentKind !== AssignmentKind.None || isMethodAccessForCall(node) ? getWidenedType(leftType) : leftType);
             if (isPrivateIdentifier(right)) {
-                if (isOptionalChain(node)) {
-                    grammarErrorOnNode(right, Diagnostics.An_optional_chain_cannot_contain_private_identifiers);
-                    return anyType;
-                }
                 checkExternalEmitHelpers(node, ExternalEmitHelpers.ClassPrivateFieldGet);
             }
             const isAnyLike = isTypeAny(apparentType) || apparentType === silentNeverType;

diff --git a/src/compiler/parser.ts b/src/compiler/parser.ts
@@ -4680,10 +4680,12 @@ namespace ts {
             const propertyAccess = <PropertyAccessExpression>createNode(SyntaxKind.PropertyAccessExpression, expression.pos);
             propertyAccess.expression = expression;
             propertyAccess.questionDotToken = questionDotToken;
-            // checker will error on private identifiers in optional chains, so don't have to catch them here
             propertyAccess.name = parseRightSideOfDot(/*allowIdentifierNames*/ true, /*allowPrivateIdentifiers*/ true);
             if (questionDotToken || expression.flags & NodeFlags.OptionalChain) {
                 propertyAccess.flags |= NodeFlags.OptionalChain;
+                if (isPrivateIdentifier(propertyAccess.name)) {
+                    parseErrorAtRange(propertyAccess.name, Diagnostics.An_optional_chain_cannot_contain_private_identifiers);
+                }
             }
             return finishNode(propertyAccess);
         }

diff --git a/tests/baselines/reference/privateIdentifierChain.1.symbols b/tests/baselines/reference/privateIdentifierChain.1.symbols
@@ -19,14 +19,17 @@ class A {
     }
     constructor() {
         this?.#b;           // Error
+>this?.#b : Symbol(A.#b, Decl(privateIdentifierChain.1.ts, 1, 9))
 >this : Symbol(A, Decl(privateIdentifierChain.1.ts, 0, 0))
 
         this?.a.#b;         // Error
+>this?.a.#b : Symbol(A.#b, Decl(privateIdentifierChain.1.ts, 1, 9))
 >this?.a : Symbol(A.a, Decl(privateIdentifierChain.1.ts, 0, 9))
 >this : Symbol(A, Decl(privateIdentifierChain.1.ts, 0, 0))
 >a : Symbol(A.a, Decl(privateIdentifierChain.1.ts, 0, 9))
 
         this?.getA().#b;    // Error
+>this?.getA().#b : Symbol(A.#b, Decl(privateIdentifierChain.1.ts, 1, 9))
 >this?.getA : Symbol(A.getA, Decl(privateIdentifierChain.1.ts, 2, 11))
 >this : Symbol(A, Decl(privateIdentifierChain.1.ts, 0, 0))
 >getA : Symbol(A.getA, Decl(privateIdentifierChain.1.ts, 2, 11))

diff --git a/tests/baselines/reference/privateIdentifierChain.1.types b/tests/baselines/reference/privateIdentifierChain.1.types
@@ -17,17 +17,17 @@ class A {
     }
     constructor() {
         this?.#b;           // Error
->this?.#b : any
+>this?.#b : A | undefined
 >this : this
 
         this?.a.#b;         // Error
->this?.a.#b : any
+>this?.a.#b : A | undefined
 >this?.a : A | undefined
 >this : this
 >a : A | undefined
 
         this?.getA().#b;    // Error
->this?.getA().#b : any
+>this?.getA().#b : A | undefined
 >this?.getA() : A | undefined
 >this?.getA : (() => A) | undefined
 >this : this

diff --git a/tests/baselines/reference/privateNameUncheckedJsOptionalChain.errors.txt b/tests/baselines/reference/privateNameUncheckedJsOptionalChain.errors.txt
@@ -0,0 +1,17 @@
+tests/cases/conformance/classes/members/privateNames/privateNameUncheckedJsOptionalChain.js(4,15): error TS18030: An optional chain cannot contain private identifiers.
+tests/cases/conformance/classes/members/privateNames/privateNameUncheckedJsOptionalChain.js(5,15): error TS18030: An optional chain cannot contain private identifiers.
+
+
+==== tests/cases/conformance/classes/members/privateNames/privateNameUncheckedJsOptionalChain.js (2 errors) ====
+    class C {
+        #bar;
+        constructor () {
+            this?.#foo;
+                  ~~~~
+!!! error TS18030: An optional chain cannot contain private identifiers.
+            this?.#bar;
+                  ~~~~
+!!! error TS18030: An optional chain cannot contain private identifiers.
+        }
+    }
+
diff --git a/tests/baselines/reference/privateNameUncheckedJsOptionalChain.symbols b/tests/baselines/reference/privateNameUncheckedJsOptionalChain.symbols
@@ -0,0 +1,17 @@
+=== tests/cases/conformance/classes/members/privateNames/privateNameUncheckedJsOptionalChain.js ===
+class C {
+>C : Symbol(C, Decl(privateNameUncheckedJsOptionalChain.js, 0, 0))
+
+    #bar;
+>#bar : Symbol(C.#bar, Decl(privateNameUncheckedJsOptionalChain.js, 0, 9))
+
+    constructor () {
+        this?.#foo;
+>this : Symbol(C, Decl(privateNameUncheckedJsOptionalChain.js, 0, 0))
+
+        this?.#bar;
+>this?.#bar : Symbol(C.#bar, Decl(privateNameUncheckedJsOptionalChain.js, 0, 9))
+>this : Symbol(C, Decl(privateNameUncheckedJsOptionalChain.js, 0, 0))
+    }
+}
+
diff --git a/tests/baselines/reference/privateNameUncheckedJsOptionalChain.types b/tests/baselines/reference/privateNameUncheckedJsOptionalChain.types
@@ -0,0 +1,18 @@
+=== tests/cases/conformance/classes/members/privateNames/privateNameUncheckedJsOptionalChain.js ===
+class C {
+>C : C
+
+    #bar;
+>#bar : any
+
+    constructor () {
+        this?.#foo;
+>this?.#foo : any
+>this : this
+
+        this?.#bar;
+>this?.#bar : any
+>this : this
+    }
+}
+
diff --git a/tests/cases/conformance/classes/members/privateNames/privateNameUncheckedJsOptionalChain.ts b/tests/cases/conformance/classes/members/privateNames/privateNameUncheckedJsOptionalChain.ts
@@ -0,0 +1,13 @@
+// @allowJs: true
+// @checkJs: false
+// @noEmit: true
+// @Filename: privateNameUncheckedJsOptionalChain.js
+// @target: es2015
+
+class C {
+    #bar;
+    constructor () {
+        this?.#foo;
+        this?.#bar;
+    }
+}