Skip to content

Commit

Permalink
Fix crash when checking invalid object rest (#31530)
Browse files Browse the repository at this point in the history
  • Loading branch information
rbuckton committed May 22, 2019
1 parent 3d2af9f commit 6a559e3
Show file tree
Hide file tree
Showing 6 changed files with 136 additions and 14 deletions.
39 changes: 25 additions & 14 deletions src/compiler/checker.ts
Expand Up @@ -23370,14 +23370,16 @@ namespace ts {
if (strictNullChecks && properties.length === 0) {
return checkNonNullType(sourceType, node);
}
for (const p of properties) {
checkObjectLiteralDestructuringPropertyAssignment(sourceType, p, properties, rightIsThis);
for (let i = 0; i < properties.length; i++) {
checkObjectLiteralDestructuringPropertyAssignment(node, sourceType, i, properties, rightIsThis);
}
return sourceType;
}

/** Note: If property cannot be a SpreadAssignment, then allProperties does not need to be provided */
function checkObjectLiteralDestructuringPropertyAssignment(objectLiteralType: Type, property: ObjectLiteralElementLike, allProperties?: NodeArray<ObjectLiteralElementLike>, rightIsThis = false) {
function checkObjectLiteralDestructuringPropertyAssignment(node: ObjectLiteralExpression, objectLiteralType: Type, propertyIndex: number, allProperties?: NodeArray<ObjectLiteralElementLike>, rightIsThis = false) {
const properties = node.properties;
const property = properties[propertyIndex];
if (property.kind === SyntaxKind.PropertyAssignment || property.kind === SyntaxKind.ShorthandPropertyAssignment) {
const name = property.name;
const exprType = getLiteralTypeFromPropertyName(name);
Expand All @@ -23394,18 +23396,25 @@ namespace ts {
return checkDestructuringAssignment(property.kind === SyntaxKind.ShorthandPropertyAssignment ? property : property.initializer, type);
}
else if (property.kind === SyntaxKind.SpreadAssignment) {
if (languageVersion < ScriptTarget.ESNext) {
checkExternalEmitHelpers(property, ExternalEmitHelpers.Rest);
if (propertyIndex < properties.length - 1) {
error(property, Diagnostics.A_rest_element_must_be_last_in_a_destructuring_pattern);
}
const nonRestNames: PropertyName[] = [];
if (allProperties) {
for (let i = 0; i < allProperties.length - 1; i++) {
nonRestNames.push(allProperties[i].name!);
else {
if (languageVersion < ScriptTarget.ESNext) {
checkExternalEmitHelpers(property, ExternalEmitHelpers.Rest);
}
const nonRestNames: PropertyName[] = [];
if (allProperties) {
for (const otherProperty of allProperties) {
if (!isSpreadAssignment(otherProperty)) {
nonRestNames.push(otherProperty.name);
}
}
}
const type = getRestType(objectLiteralType, nonRestNames, objectLiteralType.symbol);
checkGrammarForDisallowedTrailingComma(allProperties, Diagnostics.A_rest_parameter_or_binding_pattern_may_not_have_a_trailing_comma);
return checkDestructuringAssignment(property.expression, type);
}
const type = getRestType(objectLiteralType, nonRestNames, objectLiteralType.symbol);
checkGrammarForDisallowedTrailingComma(allProperties, Diagnostics.A_rest_parameter_or_binding_pattern_may_not_have_a_trailing_comma);
return checkDestructuringAssignment(property.expression, type);
}
else {
error(property, Diagnostics.Property_assignment_expected);
Expand Down Expand Up @@ -29964,8 +29973,10 @@ namespace ts {
// If this is from nested object binding pattern
// for ({ skills: { primary, secondary } } = multiRobot, i = 0; i < 1; i++) {
if (expr.parent.kind === SyntaxKind.PropertyAssignment) {
const typeOfParentObjectLiteral = getTypeOfArrayLiteralOrObjectLiteralDestructuringAssignment(<Expression>expr.parent.parent);
return checkObjectLiteralDestructuringPropertyAssignment(typeOfParentObjectLiteral || errorType, <ObjectLiteralElementLike>expr.parent)!; // TODO: GH#18217
const node = cast(expr.parent.parent, isObjectLiteralExpression);
const typeOfParentObjectLiteral = getTypeOfArrayLiteralOrObjectLiteralDestructuringAssignment(node);
const propertyIndex = indexOfNode(node.properties, expr.parent);
return checkObjectLiteralDestructuringPropertyAssignment(node, typeOfParentObjectLiteral || errorType, propertyIndex)!; // TODO: GH#18217
}
// Array literal assignment - array destructuring pattern
Debug.assert(expr.parent.kind === SyntaxKind.ArrayLiteralExpression);
Expand Down
21 changes: 21 additions & 0 deletions tests/baselines/reference/objectRestPropertyMustBeLast.errors.txt
@@ -0,0 +1,21 @@
tests/cases/conformance/types/rest/objectRestPropertyMustBeLast.ts(1,9): error TS2462: A rest element must be last in a destructuring pattern.
tests/cases/conformance/types/rest/objectRestPropertyMustBeLast.ts(2,3): error TS2462: A rest element must be last in a destructuring pattern.
tests/cases/conformance/types/rest/objectRestPropertyMustBeLast.ts(4,9): error TS2462: A rest element must be last in a destructuring pattern.
tests/cases/conformance/types/rest/objectRestPropertyMustBeLast.ts(5,3): error TS2462: A rest element must be last in a destructuring pattern.


==== tests/cases/conformance/types/rest/objectRestPropertyMustBeLast.ts (4 errors) ====
var {...a, x } = { x: 1 }; // Error, rest must be last property
~
!!! error TS2462: A rest element must be last in a destructuring pattern.
({...a, x } = { x: 1 }); // Error, rest must be last property
~~~~
!!! error TS2462: A rest element must be last in a destructuring pattern.

var {...a, x, ...b } = { x: 1 }; // Error, rest must be last property
~
!!! error TS2462: A rest element must be last in a destructuring pattern.
({...a, x, ...b } = { x: 1 }); // Error, rest must be last property
~~~~
!!! error TS2462: A rest element must be last in a destructuring pattern.

25 changes: 25 additions & 0 deletions tests/baselines/reference/objectRestPropertyMustBeLast.js
@@ -0,0 +1,25 @@
//// [objectRestPropertyMustBeLast.ts]
var {...a, x } = { x: 1 }; // Error, rest must be last property
({...a, x } = { x: 1 }); // Error, rest must be last property

var {...a, x, ...b } = { x: 1 }; // Error, rest must be last property
({...a, x, ...b } = { x: 1 }); // Error, rest must be last property


//// [objectRestPropertyMustBeLast.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) < 0)
t[p] = s[p];
if (s != null && typeof Object.getOwnPropertySymbols === "function")
for (var i = 0, p = Object.getOwnPropertySymbols(s); i < p.length; i++) {
if (e.indexOf(p[i]) < 0 && Object.prototype.propertyIsEnumerable.call(s, p[i]))
t[p[i]] = s[p[i]];
}
return t;
};
var _a, _b;
var _c = { x: 1 }, x = _c.x; // Error, rest must be last property
(_a = { x: 1 }, (x = _a.x, _a)); // Error, rest must be last property
var _d = { x: 1 }, x = _d.x, b = __rest(_d, ["a", "x"]); // Error, rest must be last property
(_b = { x: 1 }, (x = _b.x, _b), b = __rest(_b, ["x"])); // Error, rest must be last property
23 changes: 23 additions & 0 deletions tests/baselines/reference/objectRestPropertyMustBeLast.symbols
@@ -0,0 +1,23 @@
=== tests/cases/conformance/types/rest/objectRestPropertyMustBeLast.ts ===
var {...a, x } = { x: 1 }; // Error, rest must be last property
>a : Symbol(a, Decl(objectRestPropertyMustBeLast.ts, 0, 5), Decl(objectRestPropertyMustBeLast.ts, 3, 5))
>x : Symbol(x, Decl(objectRestPropertyMustBeLast.ts, 0, 10), Decl(objectRestPropertyMustBeLast.ts, 3, 10))
>x : Symbol(x, Decl(objectRestPropertyMustBeLast.ts, 0, 18))

({...a, x } = { x: 1 }); // Error, rest must be last property
>a : Symbol(a, Decl(objectRestPropertyMustBeLast.ts, 0, 5), Decl(objectRestPropertyMustBeLast.ts, 3, 5))
>x : Symbol(x, Decl(objectRestPropertyMustBeLast.ts, 1, 7))
>x : Symbol(x, Decl(objectRestPropertyMustBeLast.ts, 1, 15))

var {...a, x, ...b } = { x: 1 }; // Error, rest must be last property
>a : Symbol(a, Decl(objectRestPropertyMustBeLast.ts, 0, 5), Decl(objectRestPropertyMustBeLast.ts, 3, 5))
>x : Symbol(x, Decl(objectRestPropertyMustBeLast.ts, 0, 10), Decl(objectRestPropertyMustBeLast.ts, 3, 10))
>b : Symbol(b, Decl(objectRestPropertyMustBeLast.ts, 3, 13))
>x : Symbol(x, Decl(objectRestPropertyMustBeLast.ts, 3, 24))

({...a, x, ...b } = { x: 1 }); // Error, rest must be last property
>a : Symbol(a, Decl(objectRestPropertyMustBeLast.ts, 0, 5), Decl(objectRestPropertyMustBeLast.ts, 3, 5))
>x : Symbol(x, Decl(objectRestPropertyMustBeLast.ts, 4, 7))
>b : Symbol(b, Decl(objectRestPropertyMustBeLast.ts, 3, 13))
>x : Symbol(x, Decl(objectRestPropertyMustBeLast.ts, 4, 21))

37 changes: 37 additions & 0 deletions tests/baselines/reference/objectRestPropertyMustBeLast.types
@@ -0,0 +1,37 @@
=== tests/cases/conformance/types/rest/objectRestPropertyMustBeLast.ts ===
var {...a, x } = { x: 1 }; // Error, rest must be last property
>a : {}
>x : number
>{ x: 1 } : { x: number; }
>x : number
>1 : 1

({...a, x } = { x: 1 }); // Error, rest must be last property
>({...a, x } = { x: 1 }) : { x: number; }
>{...a, x } = { x: 1 } : { x: number; }
>{...a, x } : { x: number; }
>a : {}
>x : number
>{ x: 1 } : { x: number; }
>x : number
>1 : 1

var {...a, x, ...b } = { x: 1 }; // Error, rest must be last property
>a : {}
>x : number
>b : {}
>{ x: 1 } : { x: number; }
>x : number
>1 : 1

({...a, x, ...b } = { x: 1 }); // Error, rest must be last property
>({...a, x, ...b } = { x: 1 }) : { x: number; }
>{...a, x, ...b } = { x: 1 } : { x: number; }
>{...a, x, ...b } : { x: number; }
>a : {}
>x : number
>b : {}
>{ x: 1 } : { x: number; }
>x : number
>1 : 1

@@ -0,0 +1,5 @@
var {...a, x } = { x: 1 }; // Error, rest must be last property
({...a, x } = { x: 1 }); // Error, rest must be last property

var {...a, x, ...b } = { x: 1 }; // Error, rest must be last property
({...a, x, ...b } = { x: 1 }); // Error, rest must be last property

0 comments on commit 6a559e3

Please sign in to comment.