Skip to content

Object rest #12028

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

Object rest #12028

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
a7c1836
Parse, bind and check rest elements
sandersn Nov 2, 2016
e3a08ed
Downlevel emit of rest elements
sandersn Nov 2, 2016
ac20b46
Add rest tests
sandersn Nov 2, 2016
9d4ddb6
Update baselines
sandersn Nov 2, 2016
7ff8876
Merge branch 'object-spread' into object-rest-syntax
sandersn Nov 3, 2016
7ed5204
Update objectRestAssignment test and baselines
sandersn Nov 3, 2016
14f8b99
Update objectRestAssignment test
sandersn Nov 3, 2016
c9c5f49
Improve readability of ES next destructuring emit
sandersn Nov 3, 2016
71f3157
Address PR comments
sandersn Nov 4, 2016
cc342d1
Move convertForOf to factory for esnext and es2015
sandersn Nov 4, 2016
4337369
Update improved baselines
sandersn Nov 4, 2016
4c365bd
Move transformFunctionBody to factory
sandersn Nov 4, 2016
609cd00
Merge branch 'object-spread' into object-rest
sandersn Nov 7, 2016
9977936
Do not emit __rest for --target esnext
sandersn Nov 7, 2016
214ce38
Downlevel array destructuring to ES6 in object rest
sandersn Nov 9, 2016
0196947
Improve nested destructuring test for ESNext emit
sandersn Nov 9, 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
72 changes: 61 additions & 11 deletions src/compiler/binder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1916,6 +1916,9 @@ namespace ts {
return bindParameter(<ParameterDeclaration>node);
case SyntaxKind.VariableDeclaration:
case SyntaxKind.BindingElement:
if ((node as BindingElement).dotDotDotToken && node.parent.kind === SyntaxKind.ObjectBindingPattern) {
emitFlags |= NodeFlags.HasRestAttribute;
}
return bindVariableDeclarationOrBindingElement(<VariableDeclaration | BindingElement>node);
case SyntaxKind.PropertyDeclaration:
case SyntaxKind.PropertySignature:
Expand All @@ -1931,7 +1934,11 @@ namespace ts {

case SyntaxKind.SpreadElementExpression:
case SyntaxKind.JsxSpreadAttribute:
emitFlags |= NodeFlags.HasSpreadAttribute;
let root = container;
while (root && root.kind !== SyntaxKind.BinaryExpression) {
root = root.parent;
}
emitFlags |= root && isDestructuringAssignment(root) ? NodeFlags.HasRestAttribute : NodeFlags.HasSpreadAttribute;
Copy link
Member

Choose a reason for hiding this comment

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

I'm not 100% sure this is right. You might be walking up from the right side of a destructuring assignment, in which case the spread is actually a spread.

Copy link
Member Author

Choose a reason for hiding this comment

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

I improved it to check that the relation between root and parent is that the object literal is the lefthand side.

return;

case SyntaxKind.CallSignature:
Expand Down Expand Up @@ -2542,10 +2549,13 @@ namespace ts {
const operatorTokenKind = node.operatorToken.kind;
const leftKind = node.left.kind;

if (operatorTokenKind === SyntaxKind.EqualsToken
&& (leftKind === SyntaxKind.ObjectLiteralExpression
|| leftKind === SyntaxKind.ArrayLiteralExpression)) {
// Destructuring assignments are ES6 syntax.
if (operatorTokenKind === SyntaxKind.EqualsToken && leftKind === SyntaxKind.ObjectLiteralExpression) {
// Destructuring object assignments with are ES2015 syntax
// and possibly ESNext if they contain rest
transformFlags |= TransformFlags.AssertESNext | TransformFlags.AssertES2015 | TransformFlags.AssertDestructuringAssignment;
}
else if (operatorTokenKind === SyntaxKind.EqualsToken && leftKind === SyntaxKind.ArrayLiteralExpression) {
// Destructuring assignments are ES2015 syntax.
transformFlags |= TransformFlags.AssertES2015 | TransformFlags.AssertDestructuringAssignment;
}
else if (operatorTokenKind === SyntaxKind.AsteriskAsteriskToken
Expand Down Expand Up @@ -2579,6 +2589,11 @@ namespace ts {
transformFlags |= TransformFlags.AssertTypeScript | TransformFlags.ContainsParameterPropertyAssignments;
}

// parameters with object rest destructuring are ES Next syntax
if (subtreeFlags & TransformFlags.ContainsSpreadExpression) {
transformFlags |= TransformFlags.AssertESNext;
}

// If a parameter has an initializer, a binding pattern or a dotDotDot token, then
// it is ES6 syntax and its container must emit default value assignments or parameter destructuring downlevel.
if (subtreeFlags & TransformFlags.ContainsBindingPattern || initializer || dotDotDotToken) {
Expand Down Expand Up @@ -2812,6 +2827,11 @@ namespace ts {
transformFlags |= TransformFlags.AssertES2017;
}

// function declarations with object rest destructuring are ES Next syntax
if (subtreeFlags & TransformFlags.ContainsSpreadExpression) {
transformFlags |= TransformFlags.AssertESNext;
}

// If a FunctionDeclaration's subtree has marked the container as needing to capture the
// lexical this, or the function contains parameters with initializers, then this node is
// ES6 syntax.
Expand Down Expand Up @@ -2849,6 +2869,12 @@ namespace ts {
transformFlags |= TransformFlags.AssertES2017;
}

// function expressions with object rest destructuring are ES Next syntax
if (subtreeFlags & TransformFlags.ContainsSpreadExpression) {
transformFlags |= TransformFlags.AssertESNext;
}


// If a FunctionExpression's subtree has marked the container as needing to capture the
// lexical this, or the function contains parameters with initializers, then this node is
// ES6 syntax.
Expand Down Expand Up @@ -2886,6 +2912,11 @@ namespace ts {
transformFlags |= TransformFlags.AssertES2017;
}

// arrow functions with object rest destructuring are ES Next syntax
if (subtreeFlags & TransformFlags.ContainsSpreadExpression) {
transformFlags |= TransformFlags.AssertESNext;
}

// If an ArrowFunction contains a lexical this, its container must capture the lexical this.
if (subtreeFlags & TransformFlags.ContainsLexicalThis) {
transformFlags |= TransformFlags.ContainsCapturedLexicalThis;
Expand Down Expand Up @@ -2914,8 +2945,13 @@ namespace ts {
let transformFlags = subtreeFlags;
const nameKind = node.name.kind;

// A VariableDeclaration with a binding pattern is ES6 syntax.
if (nameKind === SyntaxKind.ObjectBindingPattern || nameKind === SyntaxKind.ArrayBindingPattern) {
// A VariableDeclaration with an object binding pattern is ES2015 syntax
// and possibly ESNext syntax if it contains an object binding pattern
if (nameKind === SyntaxKind.ObjectBindingPattern) {
transformFlags |= TransformFlags.AssertESNext | TransformFlags.AssertES2015 | TransformFlags.ContainsBindingPattern;
}
// A VariableDeclaration with an object binding pattern is ES2015 syntax.
else if (nameKind === SyntaxKind.ArrayBindingPattern) {
transformFlags |= TransformFlags.AssertES2015 | TransformFlags.ContainsBindingPattern;
}

Expand Down Expand Up @@ -3056,14 +3092,17 @@ namespace ts {
transformFlags |= TransformFlags.AssertJsx;
break;

case SyntaxKind.ForOfStatement:
// for-of might be ESNext if it has a rest destructuring
transformFlags |= TransformFlags.AssertESNext;
// FALLTHROUGH
case SyntaxKind.NoSubstitutionTemplateLiteral:
case SyntaxKind.TemplateHead:
case SyntaxKind.TemplateMiddle:
case SyntaxKind.TemplateTail:
case SyntaxKind.TemplateExpression:
case SyntaxKind.TaggedTemplateExpression:
case SyntaxKind.ShorthandPropertyAssignment:
case SyntaxKind.ForOfStatement:
case SyntaxKind.StaticKeyword:
// These nodes are ES6 syntax.
transformFlags |= TransformFlags.AssertES2015;
Expand Down Expand Up @@ -3129,10 +3168,16 @@ namespace ts {

case SyntaxKind.SpreadExpression:
case SyntaxKind.SpreadElementExpression:
// This node is ES6 or ES future syntax, but is handled by a containing node.
// This node is ES2015 or ES next syntax, but is handled by a containing node.
transformFlags |= TransformFlags.ContainsSpreadExpression;
break;

case SyntaxKind.BindingElement:
if ((node as BindingElement).dotDotDotToken) {
// this node is ES2015 or ES next syntax, but is handled by a containing node.
transformFlags |= TransformFlags.ContainsSpreadExpression;
}

case SyntaxKind.SuperKeyword:
// This node is ES6 syntax.
transformFlags |= TransformFlags.AssertES2015;
Expand All @@ -3145,8 +3190,13 @@ namespace ts {

case SyntaxKind.ObjectBindingPattern:
case SyntaxKind.ArrayBindingPattern:
// These nodes are ES6 syntax.
transformFlags |= TransformFlags.AssertES2015 | TransformFlags.ContainsBindingPattern;
// These nodes are ES2015 or ES Next syntax.
if (subtreeFlags & TransformFlags.ContainsSpreadExpression) {
transformFlags |= TransformFlags.AssertESNext | TransformFlags.ContainsBindingPattern;
}
else {
transformFlags |= TransformFlags.AssertES2015 | TransformFlags.ContainsBindingPattern;
}
break;

case SyntaxKind.Decorator:
Expand Down
96 changes: 58 additions & 38 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2982,26 +2982,31 @@ namespace ts {
return symbol && getSymbolLinks(symbol).type || getTypeForVariableLikeDeclaration(node, /*includeOptionality*/ false);
}

function getTextOfPropertyName(name: PropertyName): string {
switch (name.kind) {
case SyntaxKind.Identifier:
return (<Identifier>name).text;
case SyntaxKind.StringLiteral:
case SyntaxKind.NumericLiteral:
return (<LiteralExpression>name).text;
case SyntaxKind.ComputedPropertyName:
if (isStringOrNumericLiteral((<ComputedPropertyName>name).expression.kind)) {
return (<LiteralExpression>(<ComputedPropertyName>name).expression).text;
}
}

return undefined;
}

function isComputedNonLiteralName(name: PropertyName): boolean {
return name.kind === SyntaxKind.ComputedPropertyName && !isStringOrNumericLiteral((<ComputedPropertyName>name).expression.kind);
}

function getRestType(source: Type, properties: PropertyName[], symbol: Symbol): Type {
Debug.assert(!!(source.flags & TypeFlags.Object), "Rest types only support object types right now.");
const members = createMap<Symbol>();
const names = createMap<true>();
for (const name of properties) {
names[getTextOfPropertyName(name)] = true;
}
for (const prop of getPropertiesOfType(source)) {
const inNamesToRemove = prop.name in names;
const isPrivate = getDeclarationModifierFlagsFromSymbol(prop) & (ModifierFlags.Private | ModifierFlags.Protected);
Copy link
Contributor

Choose a reason for hiding this comment

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

why are we removing private? it will exist at runtime, we do not create it as enumerable: false, and it not a parent property.

Copy link
Contributor

Choose a reason for hiding this comment

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

well.. this is actually different from what we do with keyof where we filter privates as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

Spread also removes privates. It's the right thing to do there, and I think it is here too:

class P { private p: number }
let p: P;
var { ...exposed } = p;

I don't think exposed should be an easy way to get all of P's private members.

const isMethod = prop.flags & SymbolFlags.Method;
const isSetOnlyAccessor = prop.flags & SymbolFlags.SetAccessor && !(prop.flags & SymbolFlags.GetAccessor);
if (!inNamesToRemove && !isPrivate && !isMethod && !isSetOnlyAccessor) {
members[prop.name] = prop;
}
}
const stringIndexInfo = getIndexInfoOfType(source, IndexKind.String);
const numberIndexInfo = getIndexInfoOfType(source, IndexKind.Number);
return createAnonymousType(symbol, members, emptyArray, emptyArray, stringIndexInfo, numberIndexInfo);
}

/** Return the inferred type for a binding element */
function getTypeForBindingElement(declaration: BindingElement): Type {
const pattern = <BindingPattern>declaration.parent;
Expand All @@ -3022,26 +3027,41 @@ namespace ts {

let type: Type;
if (pattern.kind === SyntaxKind.ObjectBindingPattern) {
// Use explicitly specified property name ({ p: xxx } form), or otherwise the implied name ({ p } form)
const name = declaration.propertyName || <Identifier>declaration.name;
if (isComputedNonLiteralName(name)) {
// computed properties with non-literal names are treated as 'any'
return anyType;
}
if (declaration.initializer) {
getContextualType(declaration.initializer);
if (declaration.dotDotDotToken) {
if (!(parentType.flags & TypeFlags.Object)) {
error(declaration, Diagnostics.Rest_types_may_only_be_created_from_object_types);
return unknownType;
}
const literalMembers: PropertyName[] = [];
for (const element of pattern.elements) {
if (element.kind !== SyntaxKind.OmittedExpression && !(element as BindingElement).dotDotDotToken) {
literalMembers.push(element.propertyName || element.name as Identifier);
}
}
type = getRestType(parentType, literalMembers, declaration.symbol);
}
else {
// Use explicitly specified property name ({ p: xxx } form), or otherwise the implied name ({ p } form)
const name = declaration.propertyName || <Identifier>declaration.name;
if (isComputedNonLiteralName(name)) {
// computed properties with non-literal names are treated as 'any'
return anyType;
}
if (declaration.initializer) {
getContextualType(declaration.initializer);
}

// Use type of the specified property, or otherwise, for a numeric name, the type of the numeric index signature,
// or otherwise the type of the string index signature.
const text = getTextOfPropertyName(name);
// Use type of the specified property, or otherwise, for a numeric name, the type of the numeric index signature,
// or otherwise the type of the string index signature.
const text = getTextOfPropertyName(name);

type = getTypeOfPropertyOfType(parentType, text) ||
isNumericLiteralName(text) && getIndexTypeOfType(parentType, IndexKind.Number) ||
getIndexTypeOfType(parentType, IndexKind.String);
if (!type) {
error(name, Diagnostics.Type_0_has_no_property_1_and_no_string_index_signature, typeToString(parentType), declarationNameToString(name));
return unknownType;
type = getTypeOfPropertyOfType(parentType, text) ||
isNumericLiteralName(text) && getIndexTypeOfType(parentType, IndexKind.Number) ||
getIndexTypeOfType(parentType, IndexKind.String);
if (!type) {
error(name, Diagnostics.Type_0_has_no_property_1_and_no_string_index_signature, typeToString(parentType), declarationNameToString(name));
return unknownType;
}
}
}
else {
Expand Down Expand Up @@ -3249,8 +3269,8 @@ namespace ts {
let hasComputedProperties = false;
forEach(pattern.elements, e => {
const name = e.propertyName || <Identifier>e.name;
if (isComputedNonLiteralName(name)) {
// do not include computed properties in the implied type
if (isComputedNonLiteralName(name) || e.dotDotDotToken) {
// do not include computed properties or rests in the implied type
hasComputedProperties = true;
return;
}
Expand Down Expand Up @@ -13868,7 +13888,7 @@ namespace ts {
error(name, Diagnostics.Type_0_has_no_property_1_and_no_string_index_signature, typeToString(objectLiteralType), declarationNameToString(name));
}
}
else {
else if (property.kind !== SyntaxKind.SpreadElementExpression) {
error(property, Diagnostics.Property_assignment_expected);
}
}
Expand Down Expand Up @@ -13914,7 +13934,7 @@ namespace ts {
}
else {
if (elementIndex < elements.length - 1) {
error(element, Diagnostics.A_rest_element_must_be_last_in_an_array_destructuring_pattern);
error(element, Diagnostics.A_rest_element_must_be_last_in_a_destructuring_pattern);
}
else {
const restExpression = (<SpreadExpression>element).expression;
Expand Down Expand Up @@ -20814,7 +20834,7 @@ namespace ts {
if (node.dotDotDotToken) {
const elements = (<BindingPattern>node.parent).elements;
if (node !== lastOrUndefined(elements)) {
return grammarErrorOnNode(node, Diagnostics.A_rest_element_must_be_last_in_an_array_destructuring_pattern);
return grammarErrorOnNode(node, Diagnostics.A_rest_element_must_be_last_in_a_destructuring_pattern);
}

if (node.name.kind === SyntaxKind.ArrayBindingPattern || node.name.kind === SyntaxKind.ObjectBindingPattern) {
Expand Down
6 changes: 5 additions & 1 deletion src/compiler/diagnosticMessages.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1455,7 +1455,7 @@
"category": "Error",
"code": 2461
},
"A rest element must be last in an array destructuring pattern": {
"A rest element must be last in a destructuring pattern": {
"category": "Error",
"code": 2462
},
Expand Down Expand Up @@ -1987,6 +1987,10 @@
"category": "Error",
"code": 2698
},
"Rest types may only be created from object types.": {
"category": "Error",
"code": 2700
},

"Import declaration '{0}' is using private name '{1}'.": {
"category": "Error",
Expand Down
15 changes: 15 additions & 0 deletions src/compiler/emitter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,15 @@ var __assign = (this && this.__assign) || Object.assign || function(t) {
return t;
};`;

const restHelper = `
var __rest = (this && this.__rest) || function (s, e) {
Copy link
Contributor

Choose a reason for hiding this comment

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

we will need to add this to https://github.com/Microsoft/tslib

Copy link
Contributor

Choose a reason for hiding this comment

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

also you will need a check in checker.ts for __rest and __assign, and add some tests for these.

var t = {};

Copy link
Member

Choose a reason for hiding this comment

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

No blank lines in the helpers.

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

Copy link
Contributor

Choose a reason for hiding this comment

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

nit. remove empty line

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

for (var p in s) if (Object.prototype.hasOwnProperty.call(s, p) && !e.indexOf(p))
t[p] = s[p];
return t;
};`;

// emit output for the __decorate helper function
const decorateHelper = `
var __decorate = (this && this.__decorate) || function (decorators, target, key, desc) {
Expand Down Expand Up @@ -226,6 +235,7 @@ const _super = (function (geti, seti) {
let currentFileIdentifiers: Map<string>;
let extendsEmitted: boolean;
let assignEmitted: boolean;
let restEmitted: boolean;
let decorateEmitted: boolean;
let paramEmitted: boolean;
let awaiterEmitted: boolean;
Expand Down Expand Up @@ -2210,6 +2220,11 @@ const _super = (function (geti, seti) {
assignEmitted = true;
}

if (!restEmitted && node.flags & NodeFlags.HasRestAttribute) {
writeLines(restHelper);
restEmitted = true;
}

if (!decorateEmitted && node.flags & NodeFlags.HasDecorators) {
writeLines(decorateHelper);
if (compilerOptions.emitDecoratorMetadata) {
Expand Down
Loading