Skip to content

Evaluate RHS of binding/assignment pattern first #34806

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 30, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions src/compiler/factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5389,6 +5389,12 @@ namespace ts {
* Gets the property name of a BindingOrAssignmentElement
*/
export function getPropertyNameOfBindingOrAssignmentElement(bindingElement: BindingOrAssignmentElement): PropertyName | undefined {
const propertyName = tryGetPropertyNameOfBindingOrAssignmentElement(bindingElement);
Debug.assert(!!propertyName || isSpreadAssignment(bindingElement), "Invalid property name for binding element.");
return propertyName;
}

export function tryGetPropertyNameOfBindingOrAssignmentElement(bindingElement: BindingOrAssignmentElement): PropertyName | undefined {
switch (bindingElement.kind) {
case SyntaxKind.BindingElement:
// `a` in `let { a: b } = ...`
Expand Down Expand Up @@ -5429,8 +5435,6 @@ namespace ts {
? target.expression
: target;
}

Debug.fail("Invalid property name for binding element.");
}

function isStringOrNumericLiteral(node: Node): node is StringLiteral | NumericLiteral {
Expand Down
19 changes: 17 additions & 2 deletions src/compiler/transformers/destructuring.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,8 @@ namespace ts {
if (value) {
value = visitNode(value, visitor, isExpression);

if (isIdentifier(value) && bindingOrAssignmentElementAssignsToName(node, value.escapedText)) {
if (isIdentifier(value) && bindingOrAssignmentElementAssignsToName(node, value.escapedText) ||
bindingOrAssignmentElementContainsNonLiteralComputedName(node)) {
// If the right-hand value of the assignment is also an assignment target then
// we need to cache the right-hand value.
value = ensureIdentifier(flattenContext, value, /*reuseIdentifierExpressions*/ false, location);
Expand Down Expand Up @@ -147,6 +148,19 @@ namespace ts {
return false;
}

function bindingOrAssignmentElementContainsNonLiteralComputedName(element: BindingOrAssignmentElement): boolean {
const propertyName = tryGetPropertyNameOfBindingOrAssignmentElement(element);
if (propertyName && isComputedPropertyName(propertyName) && !isLiteralExpression(propertyName.expression)) {
return true;
}
const target = getTargetOfBindingOrAssignmentElement(element);
return !!target && isBindingOrAssignmentPattern(target) && bindingOrAssignmentPatternContainsNonLiteralComputedName(target);
}

function bindingOrAssignmentPatternContainsNonLiteralComputedName(pattern: BindingOrAssignmentPattern): boolean {
return !!forEach(getElementsOfBindingOrAssignmentPattern(pattern), bindingOrAssignmentElementContainsNonLiteralComputedName);
}

/**
* Flattens a VariableDeclaration or ParameterDeclaration to one or more variable declarations.
*
Expand Down Expand Up @@ -184,7 +198,8 @@ namespace ts {

if (isVariableDeclaration(node)) {
let initializer = getInitializerOfBindingOrAssignmentElement(node);
if (initializer && isIdentifier(initializer) && bindingOrAssignmentElementAssignsToName(node, initializer.escapedText)) {
if (initializer && (isIdentifier(initializer) && bindingOrAssignmentElementAssignsToName(node, initializer.escapedText) ||
bindingOrAssignmentElementContainsNonLiteralComputedName(node))) {
// If the right-hand value of the assignment is also an assignment target then
// we need to cache the right-hand value.
initializer = ensureIdentifier(flattenContext, initializer, /*reuseIdentifierExpressions*/ false, initializer);
Expand Down
4 changes: 2 additions & 2 deletions tests/baselines/reference/blockScopedBindingUsedBeforeDef.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ for (var _i = 0, _a = [{}]; _i < _a.length; _i++) {
continue;
}
// 2:
for (var _c = a, a = {}[_c]; false;)
for (var _c = {}, _d = a, a = _c[_d]; false;)
continue;
// 3:
var _d = b, b = {}[_d];
var _e = {}, _f = b, b = _e[_f];
26 changes: 13 additions & 13 deletions tests/baselines/reference/computedPropertiesInDestructuring1.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,15 @@ let [{[foo.toExponential()]: bar7}] = [{bar: "bar"}];


//// [computedPropertiesInDestructuring1.js]
var _a, _b, _c, _d, _e, _f;
var _a, _b, _c, _d, _e, _f, _g, _h, _j, _k, _l, _m;
// destructuring in variable declarations
var foo = "bar";
var _g = foo, bar = { bar: "bar" }[_g];
var _o = { bar: "bar" }, _p = foo, bar = _o[_p];
var bar2 = { bar: "bar" }["bar"];
var foo2 = function () { return "bar"; };
var _h = foo2(), bar3 = { bar: "bar" }[_h];
var _j = foo, bar4 = [{ bar: "bar" }][0][_j];
var _k = foo2(), bar5 = [{ bar: "bar" }][0][_k];
var _q = { bar: "bar" }, _r = foo2(), bar3 = _q[_r];
var _s = [{ bar: "bar" }], _t = foo, bar4 = _s[0][_t];
var _u = [{ bar: "bar" }], _v = foo2(), bar5 = _u[0][_v];
function f1(_a) {
var x = _a["bar"];
}
Expand All @@ -63,13 +63,13 @@ function f5(_a) {
var _b = foo2(), x = _a[0][_b];
}
// report errors on type errors in computed properties used in destructuring
var _l = foo(), bar6 = [{ bar: "bar" }][0][_l];
var _m = foo.toExponential(), bar7 = [{ bar: "bar" }][0][_m];
var _w = [{ bar: "bar" }], _x = foo(), bar6 = _w[0][_x];
var _y = [{ bar: "bar" }], _z = foo.toExponential(), bar7 = _y[0][_z];
// destructuring assignment
(_a = foo, bar = { bar: "bar" }[_a]);
(_a = { bar: "bar" }, _b = foo, bar = _a[_b]);
(bar2 = { bar: "bar" }["bar"]);
(_b = foo2(), bar3 = { bar: "bar" }[_b]);
_c = foo, bar4 = [{ bar: "bar" }][0][_c];
_d = foo2(), bar5 = [{ bar: "bar" }][0][_d];
_e = foo(), bar4 = [{ bar: "bar" }][0][_e];
_f = (1 + {}), bar4 = [{ bar: "bar" }][0][_f];
(_c = { bar: "bar" }, _d = foo2(), bar3 = _c[_d]);
_e = [{ bar: "bar" }], _f = foo, bar4 = _e[0][_f];
_g = [{ bar: "bar" }], _h = foo2(), bar5 = _g[0][_h];
_j = [{ bar: "bar" }], _k = foo(), bar4 = _j[0][_k];
_l = [{ bar: "bar" }], _m = (1 + {}), bar4 = _l[0][_m];
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@ let {[foo2()]: bar3} = {};

//// [computedPropertiesInDestructuring2.js]
var foo2 = function () { return "bar"; };
var _a = foo2(), bar3 = {}[_a];
var _a = {}, _b = foo2(), bar3 = _a[_b];
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,12 @@ exports.__esModule = true;
var itemId = 'some-id';
// --- test on first level ---
var items = {};
var _a = itemId, itemOk1 = items[_a];
var _a = items, _b = itemId, itemOk1 = _a[_b];
typeof itemOk1; // pass
var objWithItems = { items: {} };
var itemOk2 = objWithItems.items[itemId];
typeof itemOk2; // pass
var _b = objWithItems.items /*happens when default value is provided*/, _c = itemId, itemWithTSError = (_b === void 0 ? {} /*happens when default value is provided*/ : _b)[_c];
var _c = objWithItems, _d = _c.items /*happens when default value is provided*/, _e = itemId, itemWithTSError = (_d === void 0 ? {} /*happens when default value is provided*/ : _d)[_e];
// in order to re-produce the error, uncomment next line:
typeof itemWithTSError; // :(
// will result in:
Expand Down
4 changes: 2 additions & 2 deletions tests/baselines/reference/destructureComputedProperty.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const { p: p3 } = new C();

//// [destructureComputedProperty.js]
var nameN = "n";
var _a = nameN, n = ab[_a];
var _a = ab, _b = nameN, n = _a[_b];
var C = /** @class */ (function () {
function C() {
}
Expand All @@ -22,5 +22,5 @@ var C = /** @class */ (function () {
var nameP = "p";
var p0 = new C()["p"];
var p1 = new C()["p"];
var _b = nameP, p2 = new C()[_b];
var _c = new C(), _d = nameP, p2 = _c[_d];
var p3 = new C().p;
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ function foo<T extends string>(key: T, obj: { [_ in T]: number }) {

//// [destructuredMaappedTypeIsNotImplicitlyAny.js]
function foo(key, obj) {
var _a = key, bar = obj[_a]; // Element implicitly has an 'any' type because type '{ [_ in T]: number; }' has no index signature.
var _a = obj, _b = key, bar = _a[_b]; // Element implicitly has an 'any' type because type '{ [_ in T]: number; }' has no index signature.
bar; // bar : any
// Note: this does work:
var lorem = obj[key];
Expand Down
6 changes: 3 additions & 3 deletions tests/baselines/reference/destructuringAssignment_private.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ const nameO = "o";


//// [destructuringAssignment_private.js]
var _a, _b;
var _a, _b, _c, _d;
var C = /** @class */ (function () {
function C() {
this.x = 0;
Expand All @@ -27,6 +27,6 @@ var x;
(x = [{ a: new C() }][0].a.x);
(x = new C().o[0].a);
var nameX = "x";
(_a = nameX, x = [{ a: new C() }][0].a[_a]);
(_a = [{ a: new C() }], _b = nameX, x = _a[0].a[_b]);
var nameO = "o";
(_b = nameO, x = new C()[_b][0].a);
(_c = new C(), _d = nameO, x = _c[_d][0].a);
4 changes: 2 additions & 2 deletions tests/baselines/reference/destructuringControlFlow.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,11 +69,11 @@ function f3(obj) {
}
}
function f4() {
var _a;
var _a, _b;
var x;
(x = 0..x); // Error
(x = 0["x"]); // Error
(_a = "x" + "", x = 0[_a]); // Errpr
(_a = 0, _b = "x" + "", x = _a[_b]); // Errpr
}
var _a = ["foo"], key = _a[0], value = _a[1];
value.toUpperCase(); // Error
8 changes: 4 additions & 4 deletions tests/baselines/reference/genericObjectRest.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,16 +48,16 @@ function f1(obj) {
let { a: a2, b: b2 } = obj, r2 = __rest(obj, ["a", "b"]);
let { 'a': a3 } = obj, r3 = __rest(obj, ['a']);
let { ['a']: a4 } = obj, r4 = __rest(obj, ['a']);
let _a = a, a5 = obj[_a], r5 = __rest(obj, [typeof _a === "symbol" ? _a : _a + ""]);
let _a = obj, _b = a, a5 = _a[_b], r5 = __rest(_a, [typeof _b === "symbol" ? _b : _b + ""]);
}
const sa = Symbol();
const sb = Symbol();
function f2(obj) {
let _a = sa, a1 = obj[_a], _b = sb, b1 = obj[_b], r1 = __rest(obj, [typeof _a === "symbol" ? _a : _a + "", typeof _b === "symbol" ? _b : _b + ""]);
let _a = obj, _b = sa, a1 = _a[_b], _c = sb, b1 = _a[_c], r1 = __rest(_a, [typeof _b === "symbol" ? _b : _b + "", typeof _c === "symbol" ? _c : _c + ""]);
}
function f3(obj, k1, k2) {
let _a = k1, a1 = obj[_a], _b = k2, a2 = obj[_b], r1 = __rest(obj, [typeof _a === "symbol" ? _a : _a + "", typeof _b === "symbol" ? _b : _b + ""]);
let _a = obj, _b = k1, a1 = _a[_b], _c = k2, a2 = _a[_c], r1 = __rest(_a, [typeof _b === "symbol" ? _b : _b + "", typeof _c === "symbol" ? _c : _c + ""]);
}
function f4(obj, k1, k2) {
let _a = k1, a1 = obj[_a], _b = k2, a2 = obj[_b], r1 = __rest(obj, [typeof _a === "symbol" ? _a : _a + "", typeof _b === "symbol" ? _b : _b + ""]);
let _a = obj, _b = k1, a1 = _a[_b], _c = k2, a2 = _a[_c], r1 = __rest(_a, [typeof _b === "symbol" ? _b : _b + "", typeof _c === "symbol" ? _c : _c + ""]);
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,26 +30,26 @@ void prop9;

//// [lateBoundDestructuringImplicitAnyError.js]
var named = "foo";
var _a = named, prop = { prop: "foo" }[_a];
var _a = { prop: "foo" }, _b = named, prop = _a[_b];
void prop;
var numIndexed = null;
var strIndexed = null;
var numed = 6;
var symed = Symbol();
var symed2 = Symbol();
var _b = named, prop2 = numIndexed[_b];
var _c = numIndexed, _d = named, prop2 = _c[_d];
void prop2;
var _c = numed, prop3 = numIndexed[_c];
var _e = numIndexed, _f = numed, prop3 = _e[_f];
void prop3;
var _d = named, prop4 = strIndexed[_d];
var _g = strIndexed, _h = named, prop4 = _g[_h];
void prop4;
var _e = numed, prop5 = strIndexed[_e];
var _j = strIndexed, _k = numed, prop5 = _j[_k];
void prop5;
var _f = symed, prop6 = numIndexed[_f];
var _l = numIndexed, _m = symed, prop6 = _l[_m];
void prop6;
var _g = symed, prop7 = strIndexed[_g];
var _o = strIndexed, _p = symed, prop7 = _o[_p];
void prop7;
var _h = symed2, prop8 = numIndexed[_h];
var _q = numIndexed, _r = symed2, prop8 = _q[_r];
void prop8;
var _j = symed2, prop9 = strIndexed[_j];
var _s = strIndexed, _t = symed2, prop9 = _s[_t];
void prop9;