From 1ee1212952668c671fe9a7a3827a08b1473bd845 Mon Sep 17 00:00:00 2001
From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com>
Date: Thu, 11 Jun 2020 09:08:51 -0700
Subject: [PATCH] isDynamicName skips parentheses for element access

Neither `x[0]` nor `x[(0)]` should be dynamic names. Previously, the
latter was because `isDynamicName` didn't skip parentheses.

Since the binder treats dynamic names in property assignments as
assignment declarations, this incorrectly tried to create a binding for
expressions like `x[(0)] = 1`.

This caused an assert because `x[(0)]` would not take the dynamic name
code path during binding (`hasDynamicName` returned false), but the
normal code path for static names.
---
 src/compiler/utilities.ts                          |  2 +-
 ...propertyAssignmentOnParenthesizedNumber.symbols |  8 ++++++++
 .../propertyAssignmentOnParenthesizedNumber.types  | 14 ++++++++++++++
 .../propertyAssignmentOnParenthesizedNumber.ts     |  8 ++++++++
 4 files changed, 31 insertions(+), 1 deletion(-)
 create mode 100644 tests/baselines/reference/propertyAssignmentOnParenthesizedNumber.symbols
 create mode 100644 tests/baselines/reference/propertyAssignmentOnParenthesizedNumber.types
 create mode 100644 tests/cases/conformance/salsa/propertyAssignmentOnParenthesizedNumber.ts

diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts
index 1e019ec69ad9a..56bb67e80bc16 100644
--- a/src/compiler/utilities.ts
+++ b/src/compiler/utilities.ts
@@ -3061,7 +3061,7 @@ namespace ts {
         if (!(name.kind === SyntaxKind.ComputedPropertyName || name.kind === SyntaxKind.ElementAccessExpression)) {
             return false;
         }
-        const expr = isElementAccessExpression(name) ? name.argumentExpression : name.expression;
+        const expr = isElementAccessExpression(name) ? skipParentheses(name.argumentExpression) : name.expression;
         return !isStringOrNumericLiteralLike(expr) &&
             !isSignedNumericLiteral(expr) &&
             !isWellKnownSymbolSyntactically(expr);
diff --git a/tests/baselines/reference/propertyAssignmentOnParenthesizedNumber.symbols b/tests/baselines/reference/propertyAssignmentOnParenthesizedNumber.symbols
new file mode 100644
index 0000000000000..9cdc888f1f9fd
--- /dev/null
+++ b/tests/baselines/reference/propertyAssignmentOnParenthesizedNumber.symbols
@@ -0,0 +1,8 @@
+=== tests/cases/conformance/salsa/bug38934.js ===
+var x = {};
+>x : Symbol(x, Decl(bug38934.js, 0, 3))
+
+// should not crash and also should not result in a property '0' on x.
+x[(0)] = 1;
+>x : Symbol(x, Decl(bug38934.js, 0, 3))
+
diff --git a/tests/baselines/reference/propertyAssignmentOnParenthesizedNumber.types b/tests/baselines/reference/propertyAssignmentOnParenthesizedNumber.types
new file mode 100644
index 0000000000000..78c24be33cda9
--- /dev/null
+++ b/tests/baselines/reference/propertyAssignmentOnParenthesizedNumber.types
@@ -0,0 +1,14 @@
+=== tests/cases/conformance/salsa/bug38934.js ===
+var x = {};
+>x : {}
+>{} : {}
+
+// should not crash and also should not result in a property '0' on x.
+x[(0)] = 1;
+>x[(0)] = 1 : 1
+>x[(0)] : any
+>x : {}
+>(0) : 0
+>0 : 0
+>1 : 1
+
diff --git a/tests/cases/conformance/salsa/propertyAssignmentOnParenthesizedNumber.ts b/tests/cases/conformance/salsa/propertyAssignmentOnParenthesizedNumber.ts
new file mode 100644
index 0000000000000..7bfd76959a01a
--- /dev/null
+++ b/tests/cases/conformance/salsa/propertyAssignmentOnParenthesizedNumber.ts
@@ -0,0 +1,8 @@
+// @allowJs: true
+// @checkJs: true
+// @noEmit: true
+// @Filename: bug38934.js
+
+var x = {};
+// should not crash and also should not result in a property '0' on x.
+x[(0)] = 1;