Skip to content

Commit

Permalink
isDynamicName skips parentheses for element access
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
sandersn committed Jun 23, 2020
1 parent 986e9dd commit 1ee1212
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 1 deletion.
2 changes: 1 addition & 1 deletion src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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))

Original file line number Diff line number Diff line change
@@ -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

Original file line number Diff line number Diff line change
@@ -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;

0 comments on commit 1ee1212

Please sign in to comment.