Skip to content
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.

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

Support spread operator in call expressions #1931

Merged
merged 3 commits into from
Feb 10, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
265 changes: 134 additions & 131 deletions src/compiler/checker.ts

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions src/compiler/diagnosticInformationMap.generated.ts
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,7 @@ module ts {
this_cannot_be_referenced_in_a_computed_property_name: { code: 2465, category: DiagnosticCategory.Error, key: "'this' cannot be referenced in a computed property name." },
super_cannot_be_referenced_in_a_computed_property_name: { code: 2466, category: DiagnosticCategory.Error, key: "'super' cannot be referenced in a computed property name." },
A_computed_property_name_cannot_reference_a_type_parameter_from_its_containing_type: { code: 2466, category: DiagnosticCategory.Error, key: "A computed property name cannot reference a type parameter from its containing type." },
Spread_operator_in_new_expressions_is_only_available_when_targeting_ECMAScript_6_and_higher: { code: 2468, category: DiagnosticCategory.Error, key: "Spread operator in 'new' expressions is only available when targeting ECMAScript 6 and higher." },
Import_declaration_0_is_using_private_name_1: { code: 4000, category: DiagnosticCategory.Error, key: "Import declaration '{0}' is using private name '{1}'." },
Type_parameter_0_of_exported_class_has_or_is_using_private_name_1: { code: 4002, category: DiagnosticCategory.Error, key: "Type parameter '{0}' of exported class has or is using private name '{1}'." },
Type_parameter_0_of_exported_interface_has_or_is_using_private_name_1: { code: 4004, category: DiagnosticCategory.Error, key: "Type parameter '{0}' of exported interface has or is using private name '{1}'." },
Expand Down
4 changes: 4 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -1204,6 +1204,10 @@
"category": "Error",
"code": 2466
},
"Spread operator in 'new' expressions is only available when targeting ECMAScript 6 and higher.": {
"category": "Error",
"code": 2468
},

"Import declaration '{0}' is using private name '{1}'.": {
"category": "Error",
Expand Down
111 changes: 94 additions & 17 deletions src/compiler/emitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1921,7 +1921,7 @@ module ts {
break;
}
// _a .. _h, _j ... _z, _0, _1, ...
name = "_" + (tempCount < 25 ? String.fromCharCode(tempCount + (tempCount < 8 ? 0: 1) + CharacterCodes.a) : tempCount - 25);
name = "_" + (tempCount < 25 ? String.fromCharCode(tempCount + (tempCount < 8 ? 0 : 1) + CharacterCodes.a) : tempCount - 25);
Copy link
Contributor

Choose a reason for hiding this comment

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

This is pretty confusing. Can you put a comment explaining what this is doing? Also, I think it makes sense to replace the numbers with named constants. This can be addressed separately from this change.

Copy link
Member Author

Choose a reason for hiding this comment

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

Unrelated to this checkin, all that changed was formatting (guess VS must have formatted the file).

Copy link
Contributor

Choose a reason for hiding this comment

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

Yup, just pointing out that it would be good to clarify it, but in a separate change.

tempCount++;
}
var result = <Identifier>createNode(SyntaxKind.Identifier);
Expand Down Expand Up @@ -2350,22 +2350,10 @@ module ts {
return true;
}

function emitArrayLiteral(node: ArrayLiteralExpression) {
var elements = node.elements;
var length = elements.length;
if (length === 0) {
write("[]");
return;
}
if (languageVersion >= ScriptTarget.ES6) {
write("[");
emitList(elements, 0, elements.length, /*multiLine*/(node.flags & NodeFlags.MultiLine) !== 0,
/*trailingComma*/ elements.hasTrailingComma);
write("]");
return;
}
function emitListWithSpread(elements: Expression[], multiLine: boolean, trailingComma: boolean) {
var pos = 0;
var group = 0;
var length = elements.length;
while (pos < length) {
// Emit using the pattern <group0>.concat(<group1>, <group2>, ...)
if (group === 1) {
Expand All @@ -2386,8 +2374,7 @@ module ts {
i++;
}
write("[");
emitList(elements, pos, i - pos, /*multiLine*/ (node.flags & NodeFlags.MultiLine) !== 0,
/*trailingComma*/ elements.hasTrailingComma);
emitList(elements, pos, i - pos, multiLine, trailingComma && i === length);
write("]");
pos = i;
}
Expand All @@ -2398,6 +2385,23 @@ module ts {
}
}

function emitArrayLiteral(node: ArrayLiteralExpression) {
var elements = node.elements;
if (elements.length === 0) {
write("[]");
}
else if (languageVersion >= ScriptTarget.ES6) {
write("[");
emitList(elements, 0, elements.length, /*multiLine*/ (node.flags & NodeFlags.MultiLine) !== 0,
/*trailingComma*/ elements.hasTrailingComma);
write("]");
}
else {
emitListWithSpread(elements, /*multiLine*/ (node.flags & NodeFlags.MultiLine) !== 0,
/*trailingComma*/ elements.hasTrailingComma);
}
}

function emitObjectLiteral(node: ObjectLiteralExpression) {
write("{");
var properties = node.properties;
Expand Down Expand Up @@ -2492,7 +2496,80 @@ module ts {
write("]");
}

function hasSpreadElement(elements: Expression[]) {
return forEach(elements, e => e.kind === SyntaxKind.SpreadElementExpression);
}

function skipParentheses(node: Expression): Expression {
while (node.kind === SyntaxKind.ParenthesizedExpression || node.kind === SyntaxKind.TypeAssertionExpression) {
node = (<ParenthesizedExpression | TypeAssertion>node).expression;
}
return node;
}

function emitCallTarget(node: Expression): Expression {
if (node.kind === SyntaxKind.Identifier || node.kind === SyntaxKind.ThisKeyword || node.kind === SyntaxKind.SuperKeyword) {
emit(node);
return node;
}
var temp = createTempVariable(node);
recordTempDeclaration(temp);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can recordTempDeclaration be folded into createTempVariable?

Copy link
Member Author

Choose a reason for hiding this comment

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

It probably could, but I prefer keeping them separate instead of adding another boolean argument.

Copy link
Contributor

Choose a reason for hiding this comment

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

My concern is that you could do one without the other and forget. In general I don't really like void functions that are just called imperatively like this, because somebody will forget.

write("(");
emit(temp);
write(" = ");
emit(node);
write(")");
return temp;
}

function emitCallWithSpread(node: CallExpression) {
var target: Expression;
var expr = skipParentheses(node.expression);
if (expr.kind === SyntaxKind.PropertyAccessExpression) {
// Target will be emitted as "this" argument
target = emitCallTarget((<PropertyAccessExpression>expr).expression);
write(".");
emit((<PropertyAccessExpression>expr).name);
Copy link
Contributor

Choose a reason for hiding this comment

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

Please comment to explain why it is like this, and not simply emitTarget(expr). I realize that it's to make sure the temp receives the correct this value, but it took me a few minutes to understand.

}
else if (expr.kind === SyntaxKind.ElementAccessExpression) {
// Target will be emitted as "this" argument
target = emitCallTarget((<PropertyAccessExpression>expr).expression);
write("[");
emit((<ElementAccessExpression>expr).argumentExpression);
write("]");
}
else if (expr.kind === SyntaxKind.SuperKeyword) {
target = expr;
write("_super");
}
else {
emit(node.expression);
}
write(".apply(");
if (target) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a sample call for each case to see what it corresponds to?

if (target.kind === SyntaxKind.SuperKeyword) {
// Calls of form super(...) and super.foo(...)
emitThis(target);
}
else {
// Calls of form obj.foo(...)
emit(target);
}
}
else {
// Calls of form foo(...)
write("void 0");
}
write(", ");
emitListWithSpread(node.arguments, /*multiLine*/ false, /*trailingComma*/ false);
write(")");
}

function emitCallExpression(node: CallExpression) {
if (languageVersion < ScriptTarget.ES6 && hasSpreadElement(node.arguments)) {
emitCallWithSpread(node);
return;
}
var superCall = false;
if (node.expression.kind === SyntaxKind.SuperKeyword) {
write("_super");
Expand Down
17 changes: 6 additions & 11 deletions src/compiler/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1440,7 +1440,6 @@ module ts {
case ParsingContext.TypeParameters:
return isIdentifier();
case ParsingContext.ArgumentExpressions:
return token === SyntaxKind.CommaToken || isStartOfExpression();
case ParsingContext.ArrayLiteralMembers:
return token === SyntaxKind.CommaToken || token === SyntaxKind.DotDotDotToken || isStartOfExpression();
case ParsingContext.Parameters:
Expand Down Expand Up @@ -3531,32 +3530,28 @@ module ts {
return finishNode(node);
}

function parseAssignmentExpressionOrOmittedExpression(): Expression {
return token === SyntaxKind.CommaToken
? <Expression>createNode(SyntaxKind.OmittedExpression)
: parseAssignmentExpressionOrHigher();
}

function parseSpreadElement(): Expression {
var node = <SpreadElementExpression>createNode(SyntaxKind.SpreadElementExpression);
parseExpected(SyntaxKind.DotDotDotToken);
node.expression = parseAssignmentExpressionOrHigher();
return finishNode(node);
}

function parseArrayLiteralElement(): Expression {
return token === SyntaxKind.DotDotDotToken ? parseSpreadElement() : parseAssignmentExpressionOrOmittedExpression();
function parseArgumentOrArrayLiteralElement(): Expression {
return token === SyntaxKind.DotDotDotToken ? parseSpreadElement() :
token === SyntaxKind.CommaToken ? <Expression>createNode(SyntaxKind.OmittedExpression) :
parseAssignmentExpressionOrHigher();
}

function parseArgumentExpression(): Expression {
return allowInAnd(parseAssignmentExpressionOrOmittedExpression);
return allowInAnd(parseArgumentOrArrayLiteralElement);
}

function parseArrayLiteralExpression(): ArrayLiteralExpression {
var node = <ArrayLiteralExpression>createNode(SyntaxKind.ArrayLiteralExpression);
parseExpected(SyntaxKind.OpenBracketToken);
if (scanner.hasPrecedingLineBreak()) node.flags |= NodeFlags.MultiLine;
node.elements = parseDelimitedList(ParsingContext.ArrayLiteralMembers, parseArrayLiteralElement);
node.elements = parseDelimitedList(ParsingContext.ArrayLiteralMembers, parseArgumentOrArrayLiteralElement);
parseExpected(SyntaxKind.CloseBracketToken);
return finishNode(node);
}
Expand Down
59 changes: 59 additions & 0 deletions tests/baselines/reference/callWithSpread.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
tests/cases/conformance/expressions/functionCalls/callWithSpread.ts(52,21): error TS2468: Spread operator in 'new' expressions is only available when targeting ECMAScript 6 and higher.


==== tests/cases/conformance/expressions/functionCalls/callWithSpread.ts (1 errors) ====
interface X {
foo(x: number, y: number, ...z: string[]);
}

function foo(x: number, y: number, ...z: string[]) {
}

var a: string[];
var z: number[];
var obj: X;
var xa: X[];

foo(1, 2, "abc");
foo(1, 2, ...a);
foo(1, 2, ...a, "abc");

obj.foo(1, 2, "abc");
obj.foo(1, 2, ...a);
obj.foo(1, 2, ...a, "abc");

(obj.foo)(1, 2, "abc");
(obj.foo)(1, 2, ...a);
(obj.foo)(1, 2, ...a, "abc");

xa[1].foo(1, 2, "abc");
xa[1].foo(1, 2, ...a);
xa[1].foo(1, 2, ...a, "abc");

(<Function>xa[1].foo)(...[1, 2, "abc"]);

class C {
constructor(x: number, y: number, ...z: string[]) {
this.foo(x, y);
this.foo(x, y, ...z);
}
foo(x: number, y: number, ...z: string[]) {
}
}

class D extends C {
constructor() {
super(1, 2);
super(1, 2, ...a);
}
foo() {
super.foo(1, 2);
super.foo(1, 2, ...a);
}
}

// Only supported in when target is ES6
var c = new C(1, 2, ...a);
~~~~
!!! error TS2468: Spread operator in 'new' expressions is only available when targeting ECMAScript 6 and higher.

117 changes: 117 additions & 0 deletions tests/baselines/reference/callWithSpread.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
//// [callWithSpread.ts]
interface X {
foo(x: number, y: number, ...z: string[]);
}

function foo(x: number, y: number, ...z: string[]) {
}

var a: string[];
var z: number[];
var obj: X;
var xa: X[];

foo(1, 2, "abc");
foo(1, 2, ...a);
foo(1, 2, ...a, "abc");

obj.foo(1, 2, "abc");
obj.foo(1, 2, ...a);
obj.foo(1, 2, ...a, "abc");

(obj.foo)(1, 2, "abc");
(obj.foo)(1, 2, ...a);
(obj.foo)(1, 2, ...a, "abc");

xa[1].foo(1, 2, "abc");
xa[1].foo(1, 2, ...a);
xa[1].foo(1, 2, ...a, "abc");

(<Function>xa[1].foo)(...[1, 2, "abc"]);

class C {
constructor(x: number, y: number, ...z: string[]) {
this.foo(x, y);
this.foo(x, y, ...z);
}
foo(x: number, y: number, ...z: string[]) {
}
}

class D extends C {
constructor() {
super(1, 2);
super(1, 2, ...a);
}
foo() {
super.foo(1, 2);
super.foo(1, 2, ...a);
}
}

// Only supported in when target is ES6
var c = new C(1, 2, ...a);


//// [callWithSpread.js]
var __extends = this.__extends || function (d, b) {
for (var p in b) if (b.hasOwnProperty(p)) d[p] = b[p];
function __() { this.constructor = d; }
__.prototype = b.prototype;
d.prototype = new __();
};
function foo(x, y) {
var z = [];
for (var _i = 2; _i < arguments.length; _i++) {
z[_i - 2] = arguments[_i];
}
}
var a;
var z;
var obj;
var xa;
foo(1, 2, "abc");
foo.apply(void 0, [1, 2].concat(a));
foo.apply(void 0, [1, 2].concat(a, ["abc"]));
obj.foo(1, 2, "abc");
obj.foo.apply(obj, [1, 2].concat(a));
obj.foo.apply(obj, [1, 2].concat(a, ["abc"]));
(obj.foo)(1, 2, "abc");
obj.foo.apply(obj, [1, 2].concat(a));
obj.foo.apply(obj, [1, 2].concat(a, ["abc"]));
xa[1].foo(1, 2, "abc");
(_a = xa[1]).foo.apply(_a, [1, 2].concat(a));
(_b = xa[1]).foo.apply(_b, [1, 2].concat(a, ["abc"]));
(_c = xa[1]).foo.apply(_c, [1, 2, "abc"]);
var C = (function () {
function C(x, y) {
var z = [];
for (var _i = 2; _i < arguments.length; _i++) {
z[_i - 2] = arguments[_i];
}
this.foo(x, y);
this.foo.apply(this, [x, y].concat(z));
}
C.prototype.foo = function (x, y) {
var z = [];
for (var _i = 2; _i < arguments.length; _i++) {
z[_i - 2] = arguments[_i];
}
};
return C;
})();
var D = (function (_super) {
__extends(D, _super);
function D() {
_super.call(this, 1, 2);
_super.apply(this, [1, 2].concat(a));
}
D.prototype.foo = function () {
_super.prototype.foo.call(this, 1, 2);
_super.prototype.foo.apply(this, [1, 2].concat(a));
};
return D;
})(C);
// Only supported in when target is ES6
var c = new C(1, 2, ...a);
var _a, _b, _c;
Loading