Skip to content

Object spread rest fixes #12248

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.

Sign up for GitHub

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 10 commits into from
Nov 15, 2016
Merged

Object spread rest fixes #12248

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
6f274ea
Spread+rest fixes
sandersn Nov 15, 2016
fe66d9e
Add spread tests and update rest tests
sandersn Nov 15, 2016
7c72f3d
Update baselines
sandersn Nov 15, 2016
b9fa06f
Address PR commennts
sandersn Nov 15, 2016
47f3319
Improve rest/spread emit for symbols
sandersn Nov 15, 2016
0690bf4
Update baselines
sandersn Nov 15, 2016
2496756
Merge branch 'master' into object-spread-rest-fixes
sandersn Nov 15, 2016
c011b36
Update baseline of newly merged test
sandersn Nov 15, 2016
bb46e78
Fix spread/rest emit for symbols.
sandersn Nov 15, 2016
6c1f733
Update baselines with fixed emit
sandersn Nov 15, 2016
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
13 changes: 8 additions & 5 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11164,10 +11164,10 @@ namespace ts {
return links.resolvedType;
}

function getObjectLiteralIndexInfo(node: ObjectLiteralExpression, properties: Symbol[], kind: IndexKind): IndexInfo {
function getObjectLiteralIndexInfo(propertyNodes: NodeArray<ObjectLiteralElementLike>, offset: number, properties: Symbol[], kind: IndexKind): IndexInfo {
const propTypes: Type[] = [];
for (let i = 0; i < properties.length; i++) {
if (kind === IndexKind.String || isNumericName(node.properties[i].name)) {
if (kind === IndexKind.String || isNumericName(propertyNodes[i + offset].name)) {
propTypes.push(getTypeOfSymbol(properties[i]));
}
}
Expand All @@ -11193,7 +11193,9 @@ namespace ts {
let hasComputedStringProperty = false;
let hasComputedNumberProperty = false;

for (const memberDecl of node.properties) {
let offset = 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like it can probably be part of the following for declaration rather than on its own.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, left over from an earlier single-variable attempt.

for (let i = 0; i < node.properties.length; i++) {
const memberDecl = node.properties[i];
let member = memberDecl.symbol;
if (memberDecl.kind === SyntaxKind.PropertyAssignment ||
memberDecl.kind === SyntaxKind.ShorthandPropertyAssignment ||
Expand Down Expand Up @@ -11262,6 +11264,7 @@ namespace ts {
return unknownType;
}
spread = getSpreadType(spread, type, /*isFromObjectLiteral*/ false);
offset = i + 1;
continue;
}
else {
Expand Down Expand Up @@ -11315,8 +11318,8 @@ namespace ts {
return createObjectLiteralType();

function createObjectLiteralType() {
const stringIndexInfo = hasComputedStringProperty ? getObjectLiteralIndexInfo(node, propertiesArray, IndexKind.String) : undefined;
const numberIndexInfo = hasComputedNumberProperty ? getObjectLiteralIndexInfo(node, propertiesArray, IndexKind.Number) : undefined;
const stringIndexInfo = hasComputedStringProperty ? getObjectLiteralIndexInfo(node.properties, offset, propertiesArray, IndexKind.String) : undefined;
const numberIndexInfo = hasComputedNumberProperty ? getObjectLiteralIndexInfo(node.properties, offset, propertiesArray, IndexKind.Number) : undefined;
const result = createAnonymousType(node.symbol, propertiesTable, emptyArray, emptyArray, stringIndexInfo, numberIndexInfo);
const freshObjectLiteralFlag = compilerOptions.suppressExcessPropertyErrors ? 0 : TypeFlags.FreshLiteral;
result.flags |= TypeFlags.ContainsObjectLiteral | freshObjectLiteralFlag | (typeFlags & TypeFlags.PropagatingFlags);
Expand Down
8 changes: 7 additions & 1 deletion src/compiler/emitter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,21 @@ var __assign = (this && this.__assign) || Object.assign || function(t) {
s = arguments[i];
for (var p in s) if (Object.prototype.hasOwnProperty.call(s, p))
t[p] = s[p];
if (typeof Object.getOwnPropertySymbols === "function")
for (var i = 0, p = Object.getOwnPropertySymbols(s); i < p.length; i++)
t[p[i]] = s[p[i]];
}
return t;
};`;

const restHelper = `
var __rest = (this && this.__rest) || function (s, e) {
var t = {};
for (var p in s) if (Object.prototype.hasOwnProperty.call(s, p) && !e.indexOf(p))
for (var p in s) if (Object.prototype.hasOwnProperty.call(s, p) && e.indexOf(p) < 0)
t[p] = s[p];
if (typeof Object.getOwnPropertySymbols === "function")
for (var i = 0, p = Object.getOwnPropertySymbols(s); i < p.length; i++) if (e.indexOf(p[i]) < 0)
t[p[i]] = s[p[i]];
return t;
};`;

Expand Down
57 changes: 41 additions & 16 deletions src/compiler/transformers/destructuring.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,25 +304,31 @@ namespace ts {
if (properties.length !== 1) {
// For anything but a single element destructuring we need to generate a temporary
// to ensure value is evaluated exactly once.
// When doing so we want to hightlight the passed in source map node since thats the one needing this temp assignment
// When doing so we want to highlight the passed in source map node since that's the one needing this temp assignment
value = ensureIdentifier(value, /*reuseIdentifierExpressions*/ true, location, emitTempVariableAssignment);
}

let bindingElements: ObjectLiteralElementLike[] = [];
let computedTempVariables: Expression[];
for (let i = 0; i < properties.length; i++) {
const p = properties[i];
if (p.kind === SyntaxKind.PropertyAssignment || p.kind === SyntaxKind.ShorthandPropertyAssignment) {
if (!transformRest ||
p.transformFlags & TransformFlags.ContainsSpreadExpression ||
(p.kind === SyntaxKind.PropertyAssignment && p.initializer.transformFlags & TransformFlags.ContainsSpreadExpression)) {
(p.kind === SyntaxKind.PropertyAssignment && p.initializer.transformFlags & TransformFlags.ContainsSpreadExpression) ||
isComputedPropertyName(p.name)) {
if (bindingElements.length) {
emitRestAssignment(bindingElements, value, location, target);
bindingElements = [];
}
const propName = <Identifier | LiteralExpression>(<PropertyAssignment>p).name;
const bindingTarget = p.kind === SyntaxKind.ShorthandPropertyAssignment ? <ShorthandPropertyAssignment>p : (<PropertyAssignment>p).initializer || propName;
// Assignment for bindingTarget = value.propName should highlight whole property, hence use p as source map node
emitDestructuringAssignment(bindingTarget, createDestructuringPropertyAccess(value, propName), p);
const propAccess = createDestructuringPropertyAccess(value, propName);
if (isComputedPropertyName(propName)) {
computedTempVariables = append(computedTempVariables, (propAccess as ElementAccessExpression).argumentExpression);
}
emitDestructuringAssignment(bindingTarget, propAccess, p);
}
else {
bindingElements.push(p);
Expand All @@ -336,7 +342,7 @@ namespace ts {
bindingElements = [];
}
const propName = (p as SpreadAssignment).expression as Identifier;
const restCall = createRestCall(value, target.properties, p => p.name, target);
const restCall = createRestCall(value, target.properties, p => p.name, target, computedTempVariables);
emitDestructuringAssignment(propName, restCall, p);
}
}
Expand Down Expand Up @@ -413,17 +419,28 @@ namespace ts {

/** Given value: o, propName: p, pattern: { a, b, ...p } from the original statement
* `{ a, b, ...p } = o`, create `p = __rest(o, ["a", "b"]);`*/
function createRestCall<T extends Node>(value: Expression, elements: T[], getPropertyName: (element: T) => PropertyName, location: TextRange): Expression {
const propertyNames: LiteralExpression[] = [];
function createRestCall<T extends Node>(value: Expression, elements: T[], getPropertyName: (element: T) => PropertyName, location: TextRange, computedTempVariables: Expression[]): Expression {
const propertyNames: Expression[] = [];
for (let i = 0; i < elements.length - 1; i++) {
if (isOmittedExpression(elements[i])) {
const element = elements[i];
if (isOmittedExpression(element)) {
continue;
}
const str = <StringLiteral>createSynthesizedNode(SyntaxKind.StringLiteral);
str.pos = location.pos;
str.end = location.end;
str.text = getTextOfPropertyName(getPropertyName(elements[i]));
propertyNames.push(str);
if (isComputedPropertyName(getPropertyName(element))) {
// get the temp name and put that in there instead, like `_tmp + ""`
const temp = computedTempVariables.shift();
propertyNames.push(createConditional(createBinary(createTypeOf(temp),
SyntaxKind.EqualsEqualsEqualsToken,
createLiteral("symbol")),
createToken(SyntaxKind.QuestionToken),
temp,
createToken(SyntaxKind.ColonToken),
createBinary(temp, SyntaxKind.PlusToken, createLiteral(""))));
}
else {
const propName = getTextOfPropertyName(getPropertyName(element));
propertyNames.push(createLiteral(propName, location));
}
}
const args = createSynthesizedNodeArray([value, createArrayLiteral(propertyNames, location)]);
return createCall(createIdentifier("__rest"), undefined, args);
Expand Down Expand Up @@ -522,6 +539,7 @@ namespace ts {
const elements = name.elements;
const numElements = elements.length;
let bindingElements: BindingElement[] = [];
let computedTempVariables: Expression[];
for (let i = 0; i < numElements; i++) {
const element = elements[i];
if (isOmittedExpression(element)) {
Expand All @@ -533,12 +551,15 @@ namespace ts {
bindingElements = [];
}
const restCall = createRestCall(value,
name.elements,
elements, // name.elements,
element => (element as BindingElement).propertyName || <Identifier>(element as BindingElement).name,
name);
name,
computedTempVariables);
emitBindingElement(element, restCall);
}
else if (transformRest && !(element.transformFlags & TransformFlags.ContainsSpreadExpression)) {
else if (transformRest &&
!(element.transformFlags & TransformFlags.ContainsSpreadExpression) &&
!isComputedPropertyName(element.propertyName || element.name)) {
// do not emit until we have a complete bundle of ES2015 syntax
bindingElements.push(element);
}
Expand All @@ -549,7 +570,11 @@ namespace ts {
}
// Rewrite element to a declaration with an initializer that fetches property
const propName = element.propertyName || <Identifier>element.name;
emitBindingElement(element, createDestructuringPropertyAccess(value, propName));
const propAccess = createDestructuringPropertyAccess(value, propName);
if (isComputedPropertyName(propName)) {
computedTempVariables = append(computedTempVariables, (propAccess as ElementAccessExpression).argumentExpression);
}
emitBindingElement(element, propAccess);
}
}
if (bindingElements.length) {
Expand Down
1 change: 1 addition & 0 deletions src/compiler/utilities.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1262,6 +1262,7 @@ namespace ts {
case SyntaxKind.Decorator:
case SyntaxKind.JsxExpression:
case SyntaxKind.JsxSpreadAttribute:
case SyntaxKind.SpreadAssignment:
return true;
case SyntaxKind.ExpressionWithTypeArguments:
return (<ExpressionWithTypeArguments>parent).expression === node && isExpressionWithTypeArgumentsInClassExtendsClause(parent);
Expand Down
3 changes: 3 additions & 0 deletions tests/baselines/reference/importHelpersInTsx.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ var __assign = (this && this.__assign) || Object.assign || function(t) {
s = arguments[i];
for (var p in s) if (Object.prototype.hasOwnProperty.call(s, p))
t[p] = s[p];
if (typeof Object.getOwnPropertySymbols === "function")
for (var i = 0, p = Object.getOwnPropertySymbols(s); i < p.length; i++)
t[p[i]] = s[p[i]];
}
return t;
};
Expand Down
32 changes: 22 additions & 10 deletions tests/baselines/reference/objectRest.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
//// [objectRest.ts]
let o = { a: 1, b: 'no' }
var o = { a: 1, b: 'no' }
var { ...clone } = o;
var { a, ...justB } = o;
var { a, b: renamed, ...empty } = o;
Expand Down Expand Up @@ -31,31 +31,39 @@ class Removable {
}
var removable = new Removable();
var { removed, ...removableRest } = removable;

let computed = 'b';
let computed2 = 'a';
var { [computed]: stillNotGreat, [computed2]: soSo, ...o } = o;
({ [computed]: stillNotGreat, [computed2]: soSo, ...o } = o);


//// [objectRest.js]
var __rest = (this && this.__rest) || function (s, e) {
var t = {};
for (var p in s) if (Object.prototype.hasOwnProperty.call(s, p) && !e.indexOf(p))
for (var p in s) if (Object.prototype.hasOwnProperty.call(s, p) && e.indexOf(p) < 0)
t[p] = s[p];
if (typeof Object.getOwnPropertySymbols === "function")
for (var i = 0, p = Object.getOwnPropertySymbols(s); i < p.length; i++) if (e.indexOf(p[i]) < 0)
t[p[i]] = s[p[i]];
return t;
};
let o = { a: 1, b: 'no' };
var o = { a: 1, b: 'no' };
var clone = __rest(o, []);
var { a } = o, justB = __rest(o, ["a"]);
var { a, b: renamed } = o, empty = __rest(o, ["a", "b"]);
var { ['b']: renamed } = o, justA = __rest(o, ["b"]);
var _a = 'b', renamed = o[_a], justA = __rest(o, [typeof _a === "symbol" ? _a : _a + ""]);
var { 'b': renamed } = o, justA = __rest(o, ["b"]);
var { b: { '0': n, '1': oooo } } = o, justA = __rest(o, ["b"]);
let o2 = { c: 'terrible idea?', d: 'yes' };
var { d: renamed } = o2, d = __rest(o2, ["d"]);
let nestedrest;
var { x } = nestedrest, _a = nestedrest.n1, { y } = _a, _b = _a.n2, { z } = _b, nr = __rest(_b.n3, []), restrest = __rest(nestedrest, ["x", "n1"]);
var { x } = nestedrest, _b = nestedrest.n1, { y } = _b, _c = _b.n2, { z } = _c, nr = __rest(_c.n3, []), restrest = __rest(nestedrest, ["x", "n1"]);
let complex;
var _c = complex.x, { ka } = _c, nested = __rest(_c, ["ka"]), { y: other } = complex, rest = __rest(complex, ["x", "y"]);
(_d = complex.x, { ka } = _d, nested = __rest(_d, ["ka"]), { y: other } = complex, rest = __rest(complex, ["x", "y"]), complex);
var _e = { x: 1, y: 2 }, { x } = _e, fresh = __rest(_e, ["x"]);
(_f = { x: 1, y: 2 }, { x } = _f, fresh = __rest(_f, ["x"]), _f);
var _d = complex.x, { ka } = _d, nested = __rest(_d, ["ka"]), { y: other } = complex, rest = __rest(complex, ["x", "y"]);
(_e = complex.x, { ka } = _e, nested = __rest(_e, ["ka"]), { y: other } = complex, rest = __rest(complex, ["x", "y"]), complex);
var _f = { x: 1, y: 2 }, { x } = _f, fresh = __rest(_f, ["x"]);
(_g = { x: 1, y: 2 }, { x } = _g, fresh = __rest(_g, ["x"]), _g);
class Removable {
set z(value) { }
get both() { return 12; }
Expand All @@ -64,4 +72,8 @@ class Removable {
}
var removable = new Removable();
var { removed } = removable, removableRest = __rest(removable, ["removed"]);
var _d, _f;
let computed = 'b';
let computed2 = 'a';
var _h = computed, stillNotGreat = o[_h], _j = computed2, soSo = o[_j], o = __rest(o, [typeof _h === "symbol" ? _h : _h + "", typeof _j === "symbol" ? _j : _j + ""]);
(_k = computed, stillNotGreat = o[_k], _l = computed2, soSo = o[_l], o = __rest(o, [typeof _k === "symbol" ? _k : _k + "", typeof _l === "symbol" ? _l : _l + ""]), o);
var _e, _g, _k, _l;
Loading