From 6a559e37ee0d660fcc94f086a34370e79e94b17a Mon Sep 17 00:00:00 2001 From: Ron Buckton Date: Wed, 22 May 2019 11:20:57 -0700 Subject: [PATCH] Fix crash when checking invalid object rest (#31530) --- src/compiler/checker.ts | 39 ++++++++++++------- .../objectRestPropertyMustBeLast.errors.txt | 21 ++++++++++ .../reference/objectRestPropertyMustBeLast.js | 25 ++++++++++++ .../objectRestPropertyMustBeLast.symbols | 23 +++++++++++ .../objectRestPropertyMustBeLast.types | 37 ++++++++++++++++++ .../rest/objectRestPropertyMustBeLast.ts | 5 +++ 6 files changed, 136 insertions(+), 14 deletions(-) create mode 100644 tests/baselines/reference/objectRestPropertyMustBeLast.errors.txt create mode 100644 tests/baselines/reference/objectRestPropertyMustBeLast.js create mode 100644 tests/baselines/reference/objectRestPropertyMustBeLast.symbols create mode 100644 tests/baselines/reference/objectRestPropertyMustBeLast.types create mode 100644 tests/cases/conformance/types/rest/objectRestPropertyMustBeLast.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 129a79a7c11fb..05b1015b12428 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -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, rightIsThis = false) { + function checkObjectLiteralDestructuringPropertyAssignment(node: ObjectLiteralExpression, objectLiteralType: Type, propertyIndex: number, allProperties?: NodeArray, 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); @@ -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); @@ -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(expr.parent.parent); - return checkObjectLiteralDestructuringPropertyAssignment(typeOfParentObjectLiteral || errorType, 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); diff --git a/tests/baselines/reference/objectRestPropertyMustBeLast.errors.txt b/tests/baselines/reference/objectRestPropertyMustBeLast.errors.txt new file mode 100644 index 0000000000000..bdcb420dc833a --- /dev/null +++ b/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. + \ No newline at end of file diff --git a/tests/baselines/reference/objectRestPropertyMustBeLast.js b/tests/baselines/reference/objectRestPropertyMustBeLast.js new file mode 100644 index 0000000000000..dc91f72dbc7dc --- /dev/null +++ b/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 diff --git a/tests/baselines/reference/objectRestPropertyMustBeLast.symbols b/tests/baselines/reference/objectRestPropertyMustBeLast.symbols new file mode 100644 index 0000000000000..7163f2a98c9a2 --- /dev/null +++ b/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)) + diff --git a/tests/baselines/reference/objectRestPropertyMustBeLast.types b/tests/baselines/reference/objectRestPropertyMustBeLast.types new file mode 100644 index 0000000000000..8f8aafe6e0f19 --- /dev/null +++ b/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 + diff --git a/tests/cases/conformance/types/rest/objectRestPropertyMustBeLast.ts b/tests/cases/conformance/types/rest/objectRestPropertyMustBeLast.ts new file mode 100644 index 0000000000000..2e4e17dc302ce --- /dev/null +++ b/tests/cases/conformance/types/rest/objectRestPropertyMustBeLast.ts @@ -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