Skip to content

Disallow destructured parameter properties #1673

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

Merged
merged 6 commits into from
Jan 15, 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
10 changes: 8 additions & 2 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7189,7 +7189,7 @@ module ts {

checkVariableLikeDeclaration(node);
var func = getContainingFunction(node);
if (node.flags & (NodeFlags.Public | NodeFlags.Private | NodeFlags.Protected)) {
if (node.flags & NodeFlags.AccessibilityModifier) {
func = getContainingFunction(node);
if (!(func.kind === SyntaxKind.Constructor && nodeIsPresent(func.body))) {
error(node, Diagnostics.A_parameter_property_is_only_allowed_in_a_constructor_implementation);
Expand All @@ -7208,17 +7208,20 @@ module ts {
checkGrammarIndexSignature(<SignatureDeclaration>node);
}
// TODO (yuisu): Remove this check in else-if when SyntaxKind.Construct is moved and ambient context is handled
else if (node.kind === SyntaxKind.FunctionType || node.kind === SyntaxKind.FunctionDeclaration || node.kind === SyntaxKind.ConstructorType ||
else if (node.kind === SyntaxKind.FunctionType || node.kind === SyntaxKind.FunctionDeclaration || node.kind === SyntaxKind.ConstructorType ||
node.kind === SyntaxKind.CallSignature || node.kind === SyntaxKind.Constructor ||
node.kind === SyntaxKind.ConstructSignature){
checkGrammarFunctionLikeDeclaration(<FunctionLikeDeclaration>node);
}

checkTypeParameters(node.typeParameters);

forEach(node.parameters, checkParameter);

if (node.type) {
checkSourceElement(node.type);
}

if (produceDiagnostics) {
checkCollisionWithArgumentsInGeneratedCode(node);
if (compilerOptions.noImplicitAny && !node.type) {
Expand Down Expand Up @@ -10169,6 +10172,9 @@ module ts {
else if (node.kind === SyntaxKind.InterfaceDeclaration && flags & NodeFlags.Ambient) {
return grammarErrorOnNode(lastDeclare, Diagnostics.A_declare_modifier_cannot_be_used_with_an_interface_declaration, "declare");
}
else if (node.kind === SyntaxKind.Parameter && (flags & NodeFlags.AccessibilityModifier) && isBindingPattern((<ParameterDeclaration>node).name)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

How can node.kind be Parameter if node is a SignatureDeclaration?

Copy link
Member Author

Choose a reason for hiding this comment

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

To clarify, it's not, GitHub diffs break after 10K lines.

return grammarErrorOnNode(node, Diagnostics.A_parameter_property_may_not_be_a_binding_pattern);
}
}

function checkGrammarForDisallowedTrailingComma(list: NodeArray<Node>): boolean {
Expand Down
1 change: 1 addition & 0 deletions src/compiler/diagnosticInformationMap.generated.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ module ts {
Modifiers_cannot_appear_here: { code: 1184, category: DiagnosticCategory.Error, key: "Modifiers cannot appear here." },
Merge_conflict_marker_encountered: { code: 1185, category: DiagnosticCategory.Error, key: "Merge conflict marker encountered." },
A_rest_element_cannot_have_an_initializer: { code: 1186, category: DiagnosticCategory.Error, key: "A rest element cannot have an initializer." },
A_parameter_property_may_not_be_a_binding_pattern: { code: 1187, category: DiagnosticCategory.Error, key: "A parameter property may not be a binding pattern." },
Duplicate_identifier_0: { code: 2300, category: DiagnosticCategory.Error, key: "Duplicate identifier '{0}'." },
Initializer_of_instance_member_variable_0_cannot_reference_identifier_1_declared_in_the_constructor: { code: 2301, category: DiagnosticCategory.Error, key: "Initializer of instance member variable '{0}' cannot reference identifier '{1}' declared in the constructor." },
Static_members_cannot_reference_class_type_parameters: { code: 2302, category: DiagnosticCategory.Error, key: "Static members cannot reference class type parameters." },
Expand Down
8 changes: 6 additions & 2 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@
"A 'declare' modifier cannot be used with an import declaration.": {
"category": "Error",
"code": 1079,
"isEarly": true
"isEarly": true
},
"Invalid 'reference' directive syntax.": {
"category": "Error",
Expand Down Expand Up @@ -659,7 +659,7 @@
"An implementation cannot be declared in ambient contexts.": {
"category": "Error",
"code": 1184,
"isEarly": true
"isEarly": true
},
"Modifiers cannot appear here.": {
"category": "Error",
Expand All @@ -673,6 +673,10 @@
"category": "Error",
"code": 1186
},
"A parameter property may not be a binding pattern.": {
"category": "Error",
"code": 1187
},

"Duplicate identifier '{0}'.": {
"category": "Error",
Expand Down
5 changes: 4 additions & 1 deletion src/compiler/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1395,7 +1395,10 @@ module ts {
}

function canFollowModifier(): boolean {
return token === SyntaxKind.OpenBracketToken || token === SyntaxKind.AsteriskToken || isLiteralPropertyName();
return token === SyntaxKind.OpenBracketToken
|| token === SyntaxKind.OpenBraceToken
|| token === SyntaxKind.AsteriskToken
|| isLiteralPropertyName();
}

// True if positioned at the start of a list element
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
tests/cases/conformance/es6/destructuring/destructuringParameterProperties1.ts(2,17): error TS1187: A parameter property may not be a binding pattern.
tests/cases/conformance/es6/destructuring/destructuringParameterProperties1.ts(9,17): error TS1187: A parameter property may not be a binding pattern.
tests/cases/conformance/es6/destructuring/destructuringParameterProperties1.ts(16,17): error TS1187: A parameter property may not be a binding pattern.
tests/cases/conformance/es6/destructuring/destructuringParameterProperties1.ts(22,26): error TS2339: Property 'x' does not exist on type 'C1'.
tests/cases/conformance/es6/destructuring/destructuringParameterProperties1.ts(22,35): error TS2339: Property 'y' does not exist on type 'C1'.
tests/cases/conformance/es6/destructuring/destructuringParameterProperties1.ts(22,43): error TS2339: Property 'y' does not exist on type 'C1'.
tests/cases/conformance/es6/destructuring/destructuringParameterProperties1.ts(22,52): error TS2339: Property 'z' does not exist on type 'C1'.
tests/cases/conformance/es6/destructuring/destructuringParameterProperties1.ts(25,30): error TS2339: Property 'x' does not exist on type 'C2'.
tests/cases/conformance/es6/destructuring/destructuringParameterProperties1.ts(25,36): error TS2339: Property 'y' does not exist on type 'C2'.
tests/cases/conformance/es6/destructuring/destructuringParameterProperties1.ts(25,42): error TS2339: Property 'z' does not exist on type 'C2'.
tests/cases/conformance/es6/destructuring/destructuringParameterProperties1.ts(29,30): error TS2339: Property 'x' does not exist on type 'C3'.
tests/cases/conformance/es6/destructuring/destructuringParameterProperties1.ts(29,36): error TS2339: Property 'y' does not exist on type 'C3'.
tests/cases/conformance/es6/destructuring/destructuringParameterProperties1.ts(29,42): error TS2339: Property 'z' does not exist on type 'C3'.


==== tests/cases/conformance/es6/destructuring/destructuringParameterProperties1.ts (13 errors) ====
class C1 {
constructor(public [x, y, z]: string[]) {
~~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS1187: A parameter property may not be a binding pattern.
}
}

type TupleType1 = [string, number, boolean];

class C2 {
constructor(public [x, y, z]: TupleType1) {
~~~~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS1187: A parameter property may not be a binding pattern.
}
}

type ObjType1 = { x: number; y: string; z: boolean }

class C3 {
constructor(public { x, y, z }: ObjType1) {
~~~~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS1187: A parameter property may not be a binding pattern.
}
}

var c1 = new C1([]);
c1 = new C1(["larry", "{curly}", "moe"]);
var useC1Properties = c1.x === c1.y && c1.y === c1.z;
~
!!! error TS2339: Property 'x' does not exist on type 'C1'.
~
!!! error TS2339: Property 'y' does not exist on type 'C1'.
~
!!! error TS2339: Property 'y' does not exist on type 'C1'.
~
!!! error TS2339: Property 'z' does not exist on type 'C1'.

var c2 = new C2(["10", 10, !!10]);
var [c2_x, c2_y, c2_z] = [c2.x, c2.y, c2.z];
~
!!! error TS2339: Property 'x' does not exist on type 'C2'.
~
!!! error TS2339: Property 'y' does not exist on type 'C2'.
~
!!! error TS2339: Property 'z' does not exist on type 'C2'.

var c3 = new C3({x: 0, y: "", z: false});
c3 = new C3({x: 0, "y": "y", z: true});
var [c3_x, c3_y, c3_z] = [c3.x, c3.y, c3.z];
~
!!! error TS2339: Property 'x' does not exist on type 'C3'.
~
!!! error TS2339: Property 'y' does not exist on type 'C3'.
~
!!! error TS2339: Property 'z' does not exist on type 'C3'.
61 changes: 61 additions & 0 deletions tests/baselines/reference/destructuringParameterProperties1.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
//// [destructuringParameterProperties1.ts]
class C1 {
constructor(public [x, y, z]: string[]) {
}
}

type TupleType1 = [string, number, boolean];

class C2 {
constructor(public [x, y, z]: TupleType1) {
}
}

type ObjType1 = { x: number; y: string; z: boolean }

class C3 {
constructor(public { x, y, z }: ObjType1) {
}
}

var c1 = new C1([]);
c1 = new C1(["larry", "{curly}", "moe"]);
var useC1Properties = c1.x === c1.y && c1.y === c1.z;

var c2 = new C2(["10", 10, !!10]);
var [c2_x, c2_y, c2_z] = [c2.x, c2.y, c2.z];

var c3 = new C3({x: 0, y: "", z: false});
c3 = new C3({x: 0, "y": "y", z: true});
var [c3_x, c3_y, c3_z] = [c3.x, c3.y, c3.z];

//// [destructuringParameterProperties1.js]
var C1 = (function () {
function C1(_a) {
var x = _a[0], y = _a[1], z = _a[2];
this.[x, y, z] = [x, y, z];
}
return C1;
})();
var C2 = (function () {
function C2(_a) {
var x = _a[0], y = _a[1], z = _a[2];
this.[x, y, z] = [x, y, z];
}
return C2;
})();
var C3 = (function () {
function C3(_a) {
var x = _a.x, y = _a.y, z = _a.z;
this.{ x, y, z } = { x, y, z };
}
return C3;
})();
var c1 = new C1([]);
c1 = new C1(["larry", "{curly}", "moe"]);
var useC1Properties = c1.x === c1.y && c1.y === c1.z;
var c2 = new C2(["10", 10, !!10]);
var _a = [c2.x, c2.y, c2.z], c2_x = _a[0], c2_y = _a[1], c2_z = _a[2];
var c3 = new C3({ x: 0, y: "", z: false });
c3 = new C3({ x: 0, "y": "y", z: true });
var _b = [c3.x, c3.y, c3.z], c3_x = _b[0], c3_y = _b[1], c3_z = _b[2];
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
tests/cases/conformance/es6/destructuring/destructuringParameterProperties2.ts(2,36): error TS1187: A parameter property may not be a binding pattern.
tests/cases/conformance/es6/destructuring/destructuringParameterProperties2.ts(3,59): error TS2339: Property 'b' does not exist on type 'C1'.
tests/cases/conformance/es6/destructuring/destructuringParameterProperties2.ts(3,83): error TS2339: Property 'c' does not exist on type 'C1'.
tests/cases/conformance/es6/destructuring/destructuringParameterProperties2.ts(4,18): error TS2339: Property 'a' does not exist on type 'C1'.
tests/cases/conformance/es6/destructuring/destructuringParameterProperties2.ts(9,21): error TS2339: Property 'a' does not exist on type 'C1'.
tests/cases/conformance/es6/destructuring/destructuringParameterProperties2.ts(13,21): error TS2339: Property 'b' does not exist on type 'C1'.
tests/cases/conformance/es6/destructuring/destructuringParameterProperties2.ts(17,21): error TS2339: Property 'c' does not exist on type 'C1'.
tests/cases/conformance/es6/destructuring/destructuringParameterProperties2.ts(21,27): error TS2345: Argument of type '[number, undefined, string]' is not assignable to parameter of type '[number, string, boolean]'.


==== tests/cases/conformance/es6/destructuring/destructuringParameterProperties2.ts (8 errors) ====
class C1 {
constructor(private k: number, private [a, b, c]: [number, string, boolean]) {
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS1187: A parameter property may not be a binding pattern.
if ((b === undefined && c === undefined) || (this.b === undefined && this.c === undefined)) {
~
!!! error TS2339: Property 'b' does not exist on type 'C1'.
~
!!! error TS2339: Property 'c' does not exist on type 'C1'.
this.a = a || k;
~
!!! error TS2339: Property 'a' does not exist on type 'C1'.
}
}

public getA() {
return this.a
~
!!! error TS2339: Property 'a' does not exist on type 'C1'.
}

public getB() {
return this.b
~
!!! error TS2339: Property 'b' does not exist on type 'C1'.
}

public getC() {
return this.c;
~
!!! error TS2339: Property 'c' does not exist on type 'C1'.
}
}

var x = new C1(undefined, [0, undefined, ""]);
~~~~~~~~~~~~~~~~~~
!!! error TS2345: Argument of type '[number, undefined, string]' is not assignable to parameter of type '[number, string, boolean]'.
var [x_a, x_b, x_c] = [x.getA(), x.getB(), x.getC()];

var y = new C1(10, [0, "", true]);
var [y_a, y_b, y_c] = [y.getA(), y.getB(), y.getC()];

var z = new C1(10, [undefined, "", null]);
var [z_a, z_b, z_c] = [z.getA(), z.getB(), z.getC()];

58 changes: 58 additions & 0 deletions tests/baselines/reference/destructuringParameterProperties2.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
//// [destructuringParameterProperties2.ts]
class C1 {
constructor(private k: number, private [a, b, c]: [number, string, boolean]) {
if ((b === undefined && c === undefined) || (this.b === undefined && this.c === undefined)) {
this.a = a || k;
}
}

public getA() {
return this.a
}

public getB() {
return this.b
}

public getC() {
return this.c;
}
}

var x = new C1(undefined, [0, undefined, ""]);
var [x_a, x_b, x_c] = [x.getA(), x.getB(), x.getC()];

var y = new C1(10, [0, "", true]);
var [y_a, y_b, y_c] = [y.getA(), y.getB(), y.getC()];

var z = new C1(10, [undefined, "", null]);
var [z_a, z_b, z_c] = [z.getA(), z.getB(), z.getC()];


//// [destructuringParameterProperties2.js]
var C1 = (function () {
function C1(k, _a) {
var a = _a[0], b = _a[1], c = _a[2];
this.k = k;
this.[a, b, c] = [a, b, c];
if ((b === undefined && c === undefined) || (this.b === undefined && this.c === undefined)) {
this.a = a || k;
}
}
C1.prototype.getA = function () {
return this.a;
};
C1.prototype.getB = function () {
return this.b;
};
C1.prototype.getC = function () {
return this.c;
};
return C1;
})();
var x = new C1(undefined, [0, undefined, ""]);
var _a = [x.getA(), x.getB(), x.getC()], x_a = _a[0], x_b = _a[1], x_c = _a[2];
var y = new C1(10, [0, "", true]);
var _b = [y.getA(), y.getB(), y.getC()], y_a = _b[0], y_b = _b[1], y_c = _b[2];
var z = new C1(10, [undefined, "", null]);
var _c = [z.getA(), z.getB(), z.getC()], z_a = _c[0], z_b = _c[1], z_c = _c[2];
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
tests/cases/conformance/es6/destructuring/destructuringParameterProperties3.ts(2,31): error TS1187: A parameter property may not be a binding pattern.
tests/cases/conformance/es6/destructuring/destructuringParameterProperties3.ts(3,59): error TS2339: Property 'b' does not exist on type 'C1<T, U, V>'.
tests/cases/conformance/es6/destructuring/destructuringParameterProperties3.ts(3,83): error TS2339: Property 'c' does not exist on type 'C1<T, U, V>'.
tests/cases/conformance/es6/destructuring/destructuringParameterProperties3.ts(4,18): error TS2339: Property 'a' does not exist on type 'C1<T, U, V>'.
tests/cases/conformance/es6/destructuring/destructuringParameterProperties3.ts(9,21): error TS2339: Property 'a' does not exist on type 'C1<T, U, V>'.
tests/cases/conformance/es6/destructuring/destructuringParameterProperties3.ts(13,21): error TS2339: Property 'b' does not exist on type 'C1<T, U, V>'.
tests/cases/conformance/es6/destructuring/destructuringParameterProperties3.ts(17,21): error TS2339: Property 'c' does not exist on type 'C1<T, U, V>'.


==== tests/cases/conformance/es6/destructuring/destructuringParameterProperties3.ts (7 errors) ====
class C1<T, U, V> {
constructor(private k: T, private [a, b, c]: [T,U,V]) {
~~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS1187: A parameter property may not be a binding pattern.
if ((b === undefined && c === undefined) || (this.b === undefined && this.c === undefined)) {
~
!!! error TS2339: Property 'b' does not exist on type 'C1<T, U, V>'.
~
!!! error TS2339: Property 'c' does not exist on type 'C1<T, U, V>'.
this.a = a || k;
~
!!! error TS2339: Property 'a' does not exist on type 'C1<T, U, V>'.
}
}

public getA() {
return this.a
~
!!! error TS2339: Property 'a' does not exist on type 'C1<T, U, V>'.
}

public getB() {
return this.b
~
!!! error TS2339: Property 'b' does not exist on type 'C1<T, U, V>'.
}

public getC() {
return this.c;
~
!!! error TS2339: Property 'c' does not exist on type 'C1<T, U, V>'.
}
}

var x = new C1(undefined, [0, true, ""]);
var [x_a, x_b, x_c] = [x.getA(), x.getB(), x.getC()];

var y = new C1(10, [0, true, true]);
var [y_a, y_b, y_c] = [y.getA(), y.getB(), y.getC()];

var z = new C1(10, [undefined, "", ""]);
var [z_a, z_b, z_c] = [z.getA(), z.getB(), z.getC()];

var w = new C1(10, [undefined, undefined, undefined]);
var [z_a, z_b, z_c] = [z.getA(), z.getB(), z.getC()];

Loading