Skip to content

Commit 6a559e3

Browse files
rbucktonrbuckton
authored
Fix crash when checking invalid object rest (#31530)
1 parent 3d2af9f commit 6a559e3

File tree

6 files changed

+136
-14
lines changed

6 files changed

+136
-14
lines changed

‎src/compiler/checker.ts

+25-14
Original file line numberDiff line numberDiff line change
@@ -23370,14 +23370,16 @@ namespace ts {
2337023370
if (strictNullChecks && properties.length === 0) {
2337123371
return checkNonNullType(sourceType, node);
2337223372
}
23373-
for (const p of properties) {
23374-
checkObjectLiteralDestructuringPropertyAssignment(sourceType, p, properties, rightIsThis);
23373+
for (let i = 0; i < properties.length; i++) {
23374+
checkObjectLiteralDestructuringPropertyAssignment(node, sourceType, i, properties, rightIsThis);
2337523375
}
2337623376
return sourceType;
2337723377
}
2337823378

2337923379
/** Note: If property cannot be a SpreadAssignment, then allProperties does not need to be provided */
23380-
function checkObjectLiteralDestructuringPropertyAssignment(objectLiteralType: Type, property: ObjectLiteralElementLike, allProperties?: NodeArray<ObjectLiteralElementLike>, rightIsThis = false) {
23380+
function checkObjectLiteralDestructuringPropertyAssignment(node: ObjectLiteralExpression, objectLiteralType: Type, propertyIndex: number, allProperties?: NodeArray<ObjectLiteralElementLike>, rightIsThis = false) {
23381+
const properties = node.properties;
23382+
const property = properties[propertyIndex];
2338123383
if (property.kind === SyntaxKind.PropertyAssignment || property.kind === SyntaxKind.ShorthandPropertyAssignment) {
2338223384
const name = property.name;
2338323385
const exprType = getLiteralTypeFromPropertyName(name);
@@ -23394,18 +23396,25 @@ namespace ts {
2339423396
return checkDestructuringAssignment(property.kind === SyntaxKind.ShorthandPropertyAssignment ? property : property.initializer, type);
2339523397
}
2339623398
else if (property.kind === SyntaxKind.SpreadAssignment) {
23397-
if (languageVersion < ScriptTarget.ESNext) {
23398-
checkExternalEmitHelpers(property, ExternalEmitHelpers.Rest);
23399+
if (propertyIndex < properties.length - 1) {
23400+
error(property, Diagnostics.A_rest_element_must_be_last_in_a_destructuring_pattern);
2339923401
}
23400-
const nonRestNames: PropertyName[] = [];
23401-
if (allProperties) {
23402-
for (let i = 0; i < allProperties.length - 1; i++) {
23403-
nonRestNames.push(allProperties[i].name!);
23402+
else {
23403+
if (languageVersion < ScriptTarget.ESNext) {
23404+
checkExternalEmitHelpers(property, ExternalEmitHelpers.Rest);
23405+
}
23406+
const nonRestNames: PropertyName[] = [];
23407+
if (allProperties) {
23408+
for (const otherProperty of allProperties) {
23409+
if (!isSpreadAssignment(otherProperty)) {
23410+
nonRestNames.push(otherProperty.name);
23411+
}
23412+
}
2340423413
}
23414+
const type = getRestType(objectLiteralType, nonRestNames, objectLiteralType.symbol);
23415+
checkGrammarForDisallowedTrailingComma(allProperties, Diagnostics.A_rest_parameter_or_binding_pattern_may_not_have_a_trailing_comma);
23416+
return checkDestructuringAssignment(property.expression, type);
2340523417
}
23406-
const type = getRestType(objectLiteralType, nonRestNames, objectLiteralType.symbol);
23407-
checkGrammarForDisallowedTrailingComma(allProperties, Diagnostics.A_rest_parameter_or_binding_pattern_may_not_have_a_trailing_comma);
23408-
return checkDestructuringAssignment(property.expression, type);
2340923418
}
2341023419
else {
2341123420
error(property, Diagnostics.Property_assignment_expected);
@@ -29964,8 +29973,10 @@ namespace ts {
2996429973
// If this is from nested object binding pattern
2996529974
// for ({ skills: { primary, secondary } } = multiRobot, i = 0; i < 1; i++) {
2996629975
if (expr.parent.kind === SyntaxKind.PropertyAssignment) {
29967-
const typeOfParentObjectLiteral = getTypeOfArrayLiteralOrObjectLiteralDestructuringAssignment(<Expression>expr.parent.parent);
29968-
return checkObjectLiteralDestructuringPropertyAssignment(typeOfParentObjectLiteral || errorType, <ObjectLiteralElementLike>expr.parent)!; // TODO: GH#18217
29976+
const node = cast(expr.parent.parent, isObjectLiteralExpression);
29977+
const typeOfParentObjectLiteral = getTypeOfArrayLiteralOrObjectLiteralDestructuringAssignment(node);
29978+
const propertyIndex = indexOfNode(node.properties, expr.parent);
29979+
return checkObjectLiteralDestructuringPropertyAssignment(node, typeOfParentObjectLiteral || errorType, propertyIndex)!; // TODO: GH#18217
2996929980
}
2997029981
// Array literal assignment - array destructuring pattern
2997129982
Debug.assert(expr.parent.kind === SyntaxKind.ArrayLiteralExpression);

‎tests/baselines/reference/objectRestPropertyMustBeLast.errors.txt

+21
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
tests/cases/conformance/types/rest/objectRestPropertyMustBeLast.ts(1,9): error TS2462: A rest element must be last in a destructuring pattern.
2+
tests/cases/conformance/types/rest/objectRestPropertyMustBeLast.ts(2,3): error TS2462: A rest element must be last in a destructuring pattern.
3+
tests/cases/conformance/types/rest/objectRestPropertyMustBeLast.ts(4,9): error TS2462: A rest element must be last in a destructuring pattern.
4+
tests/cases/conformance/types/rest/objectRestPropertyMustBeLast.ts(5,3): error TS2462: A rest element must be last in a destructuring pattern.
5+
6+
7+
==== tests/cases/conformance/types/rest/objectRestPropertyMustBeLast.ts (4 errors) ====
8+
var {...a, x } = { x: 1 }; // Error, rest must be last property
9+
~
10+
!!! error TS2462: A rest element must be last in a destructuring pattern.
11+
({...a, x } = { x: 1 }); // Error, rest must be last property
12+
~~~~
13+
!!! error TS2462: A rest element must be last in a destructuring pattern.
14+
15+
var {...a, x, ...b } = { x: 1 }; // Error, rest must be last property
16+
~
17+
!!! error TS2462: A rest element must be last in a destructuring pattern.
18+
({...a, x, ...b } = { x: 1 }); // Error, rest must be last property
19+
~~~~
20+
!!! error TS2462: A rest element must be last in a destructuring pattern.
21+

‎tests/baselines/reference/objectRestPropertyMustBeLast.js

+25
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
//// [objectRestPropertyMustBeLast.ts]
2+
var {...a, x } = { x: 1 }; // Error, rest must be last property
3+
({...a, x } = { x: 1 }); // Error, rest must be last property
4+
5+
var {...a, x, ...b } = { x: 1 }; // Error, rest must be last property
6+
({...a, x, ...b } = { x: 1 }); // Error, rest must be last property
7+
8+
9+
//// [objectRestPropertyMustBeLast.js]
10+
var __rest = (this && this.__rest) || function (s, e) {
11+
var t = {};
12+
for (var p in s) if (Object.prototype.hasOwnProperty.call(s, p) && e.indexOf(p) < 0)
13+
t[p] = s[p];
14+
if (s != null && typeof Object.getOwnPropertySymbols === "function")
15+
for (var i = 0, p = Object.getOwnPropertySymbols(s); i < p.length; i++) {
16+
if (e.indexOf(p[i]) < 0 && Object.prototype.propertyIsEnumerable.call(s, p[i]))
17+
t[p[i]] = s[p[i]];
18+
}
19+
return t;
20+
};
21+
var _a, _b;
22+
var _c = { x: 1 }, x = _c.x; // Error, rest must be last property
23+
(_a = { x: 1 }, (x = _a.x, _a)); // Error, rest must be last property
24+
var _d = { x: 1 }, x = _d.x, b = __rest(_d, ["a", "x"]); // Error, rest must be last property
25+
(_b = { x: 1 }, (x = _b.x, _b), b = __rest(_b, ["x"])); // Error, rest must be last property

‎tests/baselines/reference/objectRestPropertyMustBeLast.symbols

+23
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
=== tests/cases/conformance/types/rest/objectRestPropertyMustBeLast.ts ===
2+
var {...a, x } = { x: 1 }; // Error, rest must be last property
3+
>a : Symbol(a, Decl(objectRestPropertyMustBeLast.ts, 0, 5), Decl(objectRestPropertyMustBeLast.ts, 3, 5))
4+
>x : Symbol(x, Decl(objectRestPropertyMustBeLast.ts, 0, 10), Decl(objectRestPropertyMustBeLast.ts, 3, 10))
5+
>x : Symbol(x, Decl(objectRestPropertyMustBeLast.ts, 0, 18))
6+
7+
({...a, x } = { x: 1 }); // Error, rest must be last property
8+
>a : Symbol(a, Decl(objectRestPropertyMustBeLast.ts, 0, 5), Decl(objectRestPropertyMustBeLast.ts, 3, 5))
9+
>x : Symbol(x, Decl(objectRestPropertyMustBeLast.ts, 1, 7))
10+
>x : Symbol(x, Decl(objectRestPropertyMustBeLast.ts, 1, 15))
11+
12+
var {...a, x, ...b } = { x: 1 }; // Error, rest must be last property
13+
>a : Symbol(a, Decl(objectRestPropertyMustBeLast.ts, 0, 5), Decl(objectRestPropertyMustBeLast.ts, 3, 5))
14+
>x : Symbol(x, Decl(objectRestPropertyMustBeLast.ts, 0, 10), Decl(objectRestPropertyMustBeLast.ts, 3, 10))
15+
>b : Symbol(b, Decl(objectRestPropertyMustBeLast.ts, 3, 13))
16+
>x : Symbol(x, Decl(objectRestPropertyMustBeLast.ts, 3, 24))
17+
18+
({...a, x, ...b } = { x: 1 }); // Error, rest must be last property
19+
>a : Symbol(a, Decl(objectRestPropertyMustBeLast.ts, 0, 5), Decl(objectRestPropertyMustBeLast.ts, 3, 5))
20+
>x : Symbol(x, Decl(objectRestPropertyMustBeLast.ts, 4, 7))
21+
>b : Symbol(b, Decl(objectRestPropertyMustBeLast.ts, 3, 13))
22+
>x : Symbol(x, Decl(objectRestPropertyMustBeLast.ts, 4, 21))
23+

‎tests/baselines/reference/objectRestPropertyMustBeLast.types

+37
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
=== tests/cases/conformance/types/rest/objectRestPropertyMustBeLast.ts ===
2+
var {...a, x } = { x: 1 }; // Error, rest must be last property
3+
>a : {}
4+
>x : number
5+
>{ x: 1 } : { x: number; }
6+
>x : number
7+
>1 : 1
8+
9+
({...a, x } = { x: 1 }); // Error, rest must be last property
10+
>({...a, x } = { x: 1 }) : { x: number; }
11+
>{...a, x } = { x: 1 } : { x: number; }
12+
>{...a, x } : { x: number; }
13+
>a : {}
14+
>x : number
15+
>{ x: 1 } : { x: number; }
16+
>x : number
17+
>1 : 1
18+
19+
var {...a, x, ...b } = { x: 1 }; // Error, rest must be last property
20+
>a : {}
21+
>x : number
22+
>b : {}
23+
>{ x: 1 } : { x: number; }
24+
>x : number
25+
>1 : 1
26+
27+
({...a, x, ...b } = { x: 1 }); // Error, rest must be last property
28+
>({...a, x, ...b } = { x: 1 }) : { x: number; }
29+
>{...a, x, ...b } = { x: 1 } : { x: number; }
30+
>{...a, x, ...b } : { x: number; }
31+
>a : {}
32+
>x : number
33+
>b : {}
34+
>{ x: 1 } : { x: number; }
35+
>x : number
36+
>1 : 1
37+

‎tests/cases/conformance/types/rest/objectRestPropertyMustBeLast.ts

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
var {...a, x } = { x: 1 }; // Error, rest must be last property
2+
({...a, x } = { x: 1 }); // Error, rest must be last property
3+
4+
var {...a, x, ...b } = { x: 1 }; // Error, rest must be last property
5+
({...a, x, ...b } = { x: 1 }); // Error, rest must be last property

0 commit comments

Comments
 (0)