From f84b2d20a9856d0797abae4d4bcb69039ee798eb Mon Sep 17 00:00:00 2001 From: Joey Watts Date: Tue, 7 Jan 2020 19:00:15 -0500 Subject: [PATCH] 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. --- src/compiler/checker.ts | 4 ---- src/compiler/parser.ts | 4 +++- .../reference/privateIdentifierChain.1.symbols | 3 +++ .../reference/privateIdentifierChain.1.types | 6 +++--- ...vateNameUncheckedJsOptionalChain.errors.txt | 17 +++++++++++++++++ ...privateNameUncheckedJsOptionalChain.symbols | 17 +++++++++++++++++ .../privateNameUncheckedJsOptionalChain.types | 18 ++++++++++++++++++ .../privateNameUncheckedJsOptionalChain.ts | 13 +++++++++++++ 8 files changed, 74 insertions(+), 8 deletions(-) create mode 100644 tests/baselines/reference/privateNameUncheckedJsOptionalChain.errors.txt create mode 100644 tests/baselines/reference/privateNameUncheckedJsOptionalChain.symbols create mode 100644 tests/baselines/reference/privateNameUncheckedJsOptionalChain.types create mode 100644 tests/cases/conformance/classes/members/privateNames/privateNameUncheckedJsOptionalChain.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 8100e62b12733..19824e6eb96db 100644 --- 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 index 28fc666a8da40..a10c2cbd6d42a 100644 --- a/src/compiler/parser.ts +++ b/src/compiler/parser.ts @@ -4680,10 +4680,12 @@ namespace ts { const propertyAccess = 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 index 65c6b7392fc59..775437414de96 100644 --- 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 index 02a8bdffe11c6..faf9a763c04f3 100644 --- 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 new file mode 100644 index 0000000000000..1cc99bc8aecbe --- /dev/null +++ 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. + } + } + \ No newline at end of file diff --git a/tests/baselines/reference/privateNameUncheckedJsOptionalChain.symbols b/tests/baselines/reference/privateNameUncheckedJsOptionalChain.symbols new file mode 100644 index 0000000000000..b23eae11f9649 --- /dev/null +++ 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 new file mode 100644 index 0000000000000..bc6acf5a45891 --- /dev/null +++ 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 new file mode 100644 index 0000000000000..e3d20ed4212ad --- /dev/null +++ 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; + } +}