From 7cf4b12d8835ebaf590d8a29a23290500c756fec Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Fri, 3 Apr 2020 10:29:22 -0700 Subject: [PATCH] Fix crash for private identifier in expando assignments (#37764) * Fix crash for private identifier in expando assignments It does this by disallowing private identifiers from expando assignments entirely. I haven't thought of a scenario where they make sense, but I haven't thought about it exhaustively either. Fixes #37356 * Update baselines I think the new error is probably better. It's certainly different! --- src/compiler/utilities.ts | 4 ++-- .../baselines/reference/privateIdentifierExpando.js | 9 +++++++++ .../reference/privateIdentifierExpando.symbols | 7 +++++++ .../reference/privateIdentifierExpando.types | 13 +++++++++++++ .../reference/privateNameJsBadAssignment.errors.txt | 9 ++++++--- .../reference/privateNameJsBadAssignment.symbols | 6 ++---- .../reference/privateNameJsBadAssignment.types | 2 +- .../conformance/salsa/privateIdentifierExpando.ts | 8 ++++++++ 8 files changed, 48 insertions(+), 10 deletions(-) create mode 100644 tests/baselines/reference/privateIdentifierExpando.js create mode 100644 tests/baselines/reference/privateIdentifierExpando.symbols create mode 100644 tests/baselines/reference/privateIdentifierExpando.types create mode 100644 tests/cases/conformance/salsa/privateIdentifierExpando.ts diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 3431ae10db6..4948a4bb6e3 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -2155,7 +2155,7 @@ namespace ts { /** Any series of property and element accesses. */ export function isBindableStaticAccessExpression(node: Node, excludeThisKeyword?: boolean): node is BindableStaticAccessExpression { - return isPropertyAccessExpression(node) && (!excludeThisKeyword && node.expression.kind === SyntaxKind.ThisKeyword || isBindableStaticNameExpression(node.expression, /*excludeThisKeyword*/ true)) + return isPropertyAccessExpression(node) && (!excludeThisKeyword && node.expression.kind === SyntaxKind.ThisKeyword || isIdentifier(node.name) && isBindableStaticNameExpression(node.expression, /*excludeThisKeyword*/ true)) || isBindableStaticElementAccessExpression(node, excludeThisKeyword); } @@ -4432,7 +4432,7 @@ namespace ts { } export function isPropertyAccessEntityNameExpression(node: Node): node is PropertyAccessEntityNameExpression { - return isPropertyAccessExpression(node) && isEntityNameExpression(node.expression); + return isPropertyAccessExpression(node) && isIdentifier(node.name) && isEntityNameExpression(node.expression); } export function tryGetPropertyAccessOrIdentifierToString(expr: Expression): string | undefined { diff --git a/tests/baselines/reference/privateIdentifierExpando.js b/tests/baselines/reference/privateIdentifierExpando.js new file mode 100644 index 00000000000..edf89f9584f --- /dev/null +++ b/tests/baselines/reference/privateIdentifierExpando.js @@ -0,0 +1,9 @@ +//// [privateIdentifierExpando.js] +const x = {}; +x.#bar.baz = 20; + + + + +//// [privateIdentifierExpando.d.ts] +declare const x: {}; diff --git a/tests/baselines/reference/privateIdentifierExpando.symbols b/tests/baselines/reference/privateIdentifierExpando.symbols new file mode 100644 index 00000000000..b8de10f8ac2 --- /dev/null +++ b/tests/baselines/reference/privateIdentifierExpando.symbols @@ -0,0 +1,7 @@ +=== tests/cases/conformance/salsa/privateIdentifierExpando.js === +const x = {}; +>x : Symbol(x, Decl(privateIdentifierExpando.js, 0, 5)) + +x.#bar.baz = 20; +>x : Symbol(x, Decl(privateIdentifierExpando.js, 0, 5)) + diff --git a/tests/baselines/reference/privateIdentifierExpando.types b/tests/baselines/reference/privateIdentifierExpando.types new file mode 100644 index 00000000000..46c4115280a --- /dev/null +++ b/tests/baselines/reference/privateIdentifierExpando.types @@ -0,0 +1,13 @@ +=== tests/cases/conformance/salsa/privateIdentifierExpando.js === +const x = {}; +>x : {} +>{} : {} + +x.#bar.baz = 20; +>x.#bar.baz = 20 : 20 +>x.#bar.baz : any +>x.#bar : any +>x : {} +>baz : any +>20 : 20 + diff --git a/tests/baselines/reference/privateNameJsBadAssignment.errors.txt b/tests/baselines/reference/privateNameJsBadAssignment.errors.txt index faf9be59850..007ec176f20 100644 --- a/tests/baselines/reference/privateNameJsBadAssignment.errors.txt +++ b/tests/baselines/reference/privateNameJsBadAssignment.errors.txt @@ -1,13 +1,16 @@ -tests/cases/conformance/classes/members/privateNames/privateNameJsBadAssignment.js(1,9): error TS2339: Property '#nope' does not exist on type 'typeof import("tests/cases/conformance/classes/members/privateNames/privateNameJsBadAssignment")'. +tests/cases/conformance/classes/members/privateNames/privateNameJsBadAssignment.js(1,1): error TS2304: Cannot find name 'exports'. +tests/cases/conformance/classes/members/privateNames/privateNameJsBadAssignment.js(1,9): error TS18016: Private identifiers are not allowed outside class bodies. tests/cases/conformance/classes/members/privateNames/privateNameJsBadAssignment.js(3,13): error TS18016: Private identifiers are not allowed outside class bodies. tests/cases/conformance/classes/members/privateNames/privateNameJsBadAssignment.js(6,3): error TS2339: Property '#foo' does not exist on type 'typeof B'. tests/cases/conformance/classes/members/privateNames/privateNameJsBadAssignment.js(11,14): error TS2339: Property '#foo' does not exist on type 'C'. -==== tests/cases/conformance/classes/members/privateNames/privateNameJsBadAssignment.js (4 errors) ==== +==== tests/cases/conformance/classes/members/privateNames/privateNameJsBadAssignment.js (5 errors) ==== exports.#nope = 1; // Error (outside class body) + ~~~~~~~ +!!! error TS2304: Cannot find name 'exports'. ~~~~~ -!!! error TS2339: Property '#nope' does not exist on type 'typeof import("tests/cases/conformance/classes/members/privateNames/privateNameJsBadAssignment")'. +!!! error TS18016: Private identifiers are not allowed outside class bodies. function A() { } A.prototype.#no = 2; // Error (outside class body) ~~~ diff --git a/tests/baselines/reference/privateNameJsBadAssignment.symbols b/tests/baselines/reference/privateNameJsBadAssignment.symbols index 0f86dc2554c..42ea61897e4 100644 --- a/tests/baselines/reference/privateNameJsBadAssignment.symbols +++ b/tests/baselines/reference/privateNameJsBadAssignment.symbols @@ -1,7 +1,5 @@ === tests/cases/conformance/classes/members/privateNames/privateNameJsBadAssignment.js === exports.#nope = 1; // Error (outside class body) ->exports : Symbol(#nope, Decl(privateNameJsBadAssignment.js, 0, 0)) - function A() { } >A : Symbol(A, Decl(privateNameJsBadAssignment.js, 0, 18)) @@ -11,10 +9,10 @@ A.prototype.#no = 2; // Error (outside class body) >prototype : Symbol(Function.prototype, Decl(lib.es5.d.ts, --, --)) class B {} ->B : Symbol(B, Decl(privateNameJsBadAssignment.js, 2, 20), Decl(privateNameJsBadAssignment.js, 4, 10)) +>B : Symbol(B, Decl(privateNameJsBadAssignment.js, 2, 20)) B.#foo = 3; // Error (outside class body) ->B : Symbol(B, Decl(privateNameJsBadAssignment.js, 2, 20), Decl(privateNameJsBadAssignment.js, 4, 10)) +>B : Symbol(B, Decl(privateNameJsBadAssignment.js, 2, 20)) class C { >C : Symbol(C, Decl(privateNameJsBadAssignment.js, 5, 11)) diff --git a/tests/baselines/reference/privateNameJsBadAssignment.types b/tests/baselines/reference/privateNameJsBadAssignment.types index 39b34820c2f..d1a10791600 100644 --- a/tests/baselines/reference/privateNameJsBadAssignment.types +++ b/tests/baselines/reference/privateNameJsBadAssignment.types @@ -2,7 +2,7 @@ exports.#nope = 1; // Error (outside class body) >exports.#nope = 1 : 1 >exports.#nope : any ->exports : typeof import("tests/cases/conformance/classes/members/privateNames/privateNameJsBadAssignment") +>exports : any >1 : 1 function A() { } diff --git a/tests/cases/conformance/salsa/privateIdentifierExpando.ts b/tests/cases/conformance/salsa/privateIdentifierExpando.ts new file mode 100644 index 00000000000..2f31c3a3699 --- /dev/null +++ b/tests/cases/conformance/salsa/privateIdentifierExpando.ts @@ -0,0 +1,8 @@ +// @allowJs: true +// @checkJs: true +// @declaration: true +// @emitDeclarationOnly: true +// @Filename: privateIdentifierExpando.js + +const x = {}; +x.#bar.baz = 20;