From dcbaadaa606585c8aedef8565fb284afb2621c0c Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Fri, 20 May 2016 08:54:05 -0700 Subject: [PATCH 1/3] Allow assignment to readonly parameter property within the constructor --- src/compiler/checker.ts | 14 ++++++++++++-- .../reference/readonlyConstructorAssignment.js | 16 ++++++++++++++++ .../readonlyConstructorAssignment.symbols | 14 ++++++++++++++ .../readonlyConstructorAssignment.types | 16 ++++++++++++++++ .../readonlyConstructorAssignment.ts | 5 +++++ 5 files changed, 63 insertions(+), 2 deletions(-) create mode 100644 tests/baselines/reference/readonlyConstructorAssignment.js create mode 100644 tests/baselines/reference/readonlyConstructorAssignment.symbols create mode 100644 tests/baselines/reference/readonlyConstructorAssignment.types create mode 100644 tests/cases/conformance/classes/constructorDeclarations/constructorParameters/readonlyConstructorAssignment.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 476695e3ec4..8b15d7291bf 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -11882,8 +11882,18 @@ namespace ts { if (symbol.flags & SymbolFlags.Property && (expr.kind === SyntaxKind.PropertyAccessExpression || expr.kind === SyntaxKind.ElementAccessExpression) && (expr as PropertyAccessExpression | ElementAccessExpression).expression.kind === SyntaxKind.ThisKeyword) { - const func = getContainingFunction(expr); - return !(func && func.kind === SyntaxKind.Constructor && func.parent === symbol.valueDeclaration.parent); + return !isInConstructor(getContainingFunction(expr)); + function isInConstructor(func: FunctionLikeDeclaration) { + if (!func) + return false; + if (func.kind !== SyntaxKind.Constructor) + return false; + if (func.parent === symbol.valueDeclaration.parent) // If the symbol was declared in the class: + return true; + if (func === symbol.valueDeclaration.parent) // If symbolDecl is a parameter property of the constructor: + return true; + return false; + } } return true; } diff --git a/tests/baselines/reference/readonlyConstructorAssignment.js b/tests/baselines/reference/readonlyConstructorAssignment.js new file mode 100644 index 00000000000..3d00f829a78 --- /dev/null +++ b/tests/baselines/reference/readonlyConstructorAssignment.js @@ -0,0 +1,16 @@ +//// [readonlyConstructorAssignment.ts] +class A { + constructor(readonly x: number) { + this.x = 7; + } +} + + +//// [readonlyConstructorAssignment.js] +var A = (function () { + function A(x) { + this.x = x; + this.x = 7; + } + return A; +}()); diff --git a/tests/baselines/reference/readonlyConstructorAssignment.symbols b/tests/baselines/reference/readonlyConstructorAssignment.symbols new file mode 100644 index 00000000000..594f9e02b69 --- /dev/null +++ b/tests/baselines/reference/readonlyConstructorAssignment.symbols @@ -0,0 +1,14 @@ +=== tests/cases/conformance/classes/constructorDeclarations/constructorParameters/readonlyConstructorAssignment.ts === +class A { +>A : Symbol(A, Decl(readonlyConstructorAssignment.ts, 0, 0)) + + constructor(readonly x: number) { +>x : Symbol(A.x, Decl(readonlyConstructorAssignment.ts, 1, 16)) + + this.x = 7; +>this.x : Symbol(A.x, Decl(readonlyConstructorAssignment.ts, 1, 16)) +>this : Symbol(A, Decl(readonlyConstructorAssignment.ts, 0, 0)) +>x : Symbol(A.x, Decl(readonlyConstructorAssignment.ts, 1, 16)) + } +} + diff --git a/tests/baselines/reference/readonlyConstructorAssignment.types b/tests/baselines/reference/readonlyConstructorAssignment.types new file mode 100644 index 00000000000..10f1cd20470 --- /dev/null +++ b/tests/baselines/reference/readonlyConstructorAssignment.types @@ -0,0 +1,16 @@ +=== tests/cases/conformance/classes/constructorDeclarations/constructorParameters/readonlyConstructorAssignment.ts === +class A { +>A : A + + constructor(readonly x: number) { +>x : number + + this.x = 7; +>this.x = 7 : number +>this.x : number +>this : this +>x : number +>7 : number + } +} + diff --git a/tests/cases/conformance/classes/constructorDeclarations/constructorParameters/readonlyConstructorAssignment.ts b/tests/cases/conformance/classes/constructorDeclarations/constructorParameters/readonlyConstructorAssignment.ts new file mode 100644 index 00000000000..a81bf69249c --- /dev/null +++ b/tests/cases/conformance/classes/constructorDeclarations/constructorParameters/readonlyConstructorAssignment.ts @@ -0,0 +1,5 @@ +class A { + constructor(readonly x: number) { + this.x = 7; + } +} From d3b4c6a93eaf848be27a34997d517e438883e8dd Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Fri, 20 May 2016 10:29:08 -0700 Subject: [PATCH 2/3] Improve readability --- src/compiler/checker.ts | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 8b15d7291bf..b3861ed9657 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -11882,18 +11882,14 @@ namespace ts { if (symbol.flags & SymbolFlags.Property && (expr.kind === SyntaxKind.PropertyAccessExpression || expr.kind === SyntaxKind.ElementAccessExpression) && (expr as PropertyAccessExpression | ElementAccessExpression).expression.kind === SyntaxKind.ThisKeyword) { - return !isInConstructor(getContainingFunction(expr)); - function isInConstructor(func: FunctionLikeDeclaration) { - if (!func) - return false; - if (func.kind !== SyntaxKind.Constructor) - return false; - if (func.parent === symbol.valueDeclaration.parent) // If the symbol was declared in the class: - return true; - if (func === symbol.valueDeclaration.parent) // If symbolDecl is a parameter property of the constructor: - return true; - return false; - } + // Look for if this is the constructor for the class that `symbol` is a property of. + const func = getContainingFunction(expr); + if (!(func && func.kind === SyntaxKind.Constructor)) + return true; + // If func.parent is a class and symbol is a (readonly) property of that class, or + // if func is a constructor and symbol is a (readonly) parameter property declared in it, + // then symbol is writeable here. + return !(func.parent === symbol.valueDeclaration.parent || func === symbol.valueDeclaration.parent); } return true; } From 17009e41d4eb84fe319b52c6925ae845888e2ba4 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Mon, 23 May 2016 06:06:01 -0700 Subject: [PATCH 3/3] Expand tests --- .../readonlyConstructorAssignment.errors.txt | 50 ++++++++++++ .../readonlyConstructorAssignment.js | 80 ++++++++++++++++++- .../readonlyConstructorAssignment.symbols | 14 ---- .../readonlyConstructorAssignment.types | 16 ---- .../readonlyConstructorAssignment.ts | 35 +++++++- 5 files changed, 162 insertions(+), 33 deletions(-) create mode 100644 tests/baselines/reference/readonlyConstructorAssignment.errors.txt delete mode 100644 tests/baselines/reference/readonlyConstructorAssignment.symbols delete mode 100644 tests/baselines/reference/readonlyConstructorAssignment.types diff --git a/tests/baselines/reference/readonlyConstructorAssignment.errors.txt b/tests/baselines/reference/readonlyConstructorAssignment.errors.txt new file mode 100644 index 00000000000..e764fe21eda --- /dev/null +++ b/tests/baselines/reference/readonlyConstructorAssignment.errors.txt @@ -0,0 +1,50 @@ +tests/cases/conformance/classes/constructorDeclarations/constructorParameters/readonlyConstructorAssignment.ts(13,9): error TS2450: Left-hand side of assignment expression cannot be a constant or a read-only property. +tests/cases/conformance/classes/constructorDeclarations/constructorParameters/readonlyConstructorAssignment.ts(33,7): error TS2415: Class 'E' incorrectly extends base class 'D'. + Property 'x' is private in type 'D' but not in type 'E'. + + +==== tests/cases/conformance/classes/constructorDeclarations/constructorParameters/readonlyConstructorAssignment.ts (2 errors) ==== + // Tests that readonly parameter properties behave like regular readonly properties + + class A { + constructor(readonly x: number) { + this.x = 0; + } + } + + class B extends A { + constructor(x: number) { + super(x); + // Fails, x is readonly + this.x = 1; + ~~~~~~ +!!! error TS2450: Left-hand side of assignment expression cannot be a constant or a read-only property. + } + } + + class C extends A { + // This is the usual behavior of readonly properties: + // if one is redeclared in a base class, then it can be assigned to. + constructor(readonly x: number) { + super(x); + this.x = 1; + } + } + + class D { + constructor(private readonly x: number) { + this.x = 0; + } + } + + // Fails, can't redeclare readonly property + class E extends D { + ~ +!!! error TS2415: Class 'E' incorrectly extends base class 'D'. +!!! error TS2415: Property 'x' is private in type 'D' but not in type 'E'. + constructor(readonly x: number) { + super(x); + this.x = 1; + } + } + \ No newline at end of file diff --git a/tests/baselines/reference/readonlyConstructorAssignment.js b/tests/baselines/reference/readonlyConstructorAssignment.js index 3d00f829a78..40c292e4ce8 100644 --- a/tests/baselines/reference/readonlyConstructorAssignment.js +++ b/tests/baselines/reference/readonlyConstructorAssignment.js @@ -1,16 +1,92 @@ //// [readonlyConstructorAssignment.ts] +// Tests that readonly parameter properties behave like regular readonly properties + class A { constructor(readonly x: number) { - this.x = 7; + this.x = 0; + } +} + +class B extends A { + constructor(x: number) { + super(x); + // Fails, x is readonly + this.x = 1; + } +} + +class C extends A { + // This is the usual behavior of readonly properties: + // if one is redeclared in a base class, then it can be assigned to. + constructor(readonly x: number) { + super(x); + this.x = 1; + } +} + +class D { + constructor(private readonly x: number) { + this.x = 0; + } +} + +// Fails, can't redeclare readonly property +class E extends D { + constructor(readonly x: number) { + super(x); + this.x = 1; } } //// [readonlyConstructorAssignment.js] +// Tests that readonly parameter properties behave like regular readonly properties +var __extends = (this && this.__extends) || function (d, b) { + for (var p in b) if (b.hasOwnProperty(p)) d[p] = b[p]; + function __() { this.constructor = d; } + d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __()); +}; var A = (function () { function A(x) { this.x = x; - this.x = 7; + this.x = 0; } return A; }()); +var B = (function (_super) { + __extends(B, _super); + function B(x) { + _super.call(this, x); + // Fails, x is readonly + this.x = 1; + } + return B; +}(A)); +var C = (function (_super) { + __extends(C, _super); + // This is the usual behavior of readonly properties: + // if one is redeclared in a base class, then it can be assigned to. + function C(x) { + _super.call(this, x); + this.x = x; + this.x = 1; + } + return C; +}(A)); +var D = (function () { + function D(x) { + this.x = x; + this.x = 0; + } + return D; +}()); +// Fails, can't redeclare readonly property +var E = (function (_super) { + __extends(E, _super); + function E(x) { + _super.call(this, x); + this.x = x; + this.x = 1; + } + return E; +}(D)); diff --git a/tests/baselines/reference/readonlyConstructorAssignment.symbols b/tests/baselines/reference/readonlyConstructorAssignment.symbols deleted file mode 100644 index 594f9e02b69..00000000000 --- a/tests/baselines/reference/readonlyConstructorAssignment.symbols +++ /dev/null @@ -1,14 +0,0 @@ -=== tests/cases/conformance/classes/constructorDeclarations/constructorParameters/readonlyConstructorAssignment.ts === -class A { ->A : Symbol(A, Decl(readonlyConstructorAssignment.ts, 0, 0)) - - constructor(readonly x: number) { ->x : Symbol(A.x, Decl(readonlyConstructorAssignment.ts, 1, 16)) - - this.x = 7; ->this.x : Symbol(A.x, Decl(readonlyConstructorAssignment.ts, 1, 16)) ->this : Symbol(A, Decl(readonlyConstructorAssignment.ts, 0, 0)) ->x : Symbol(A.x, Decl(readonlyConstructorAssignment.ts, 1, 16)) - } -} - diff --git a/tests/baselines/reference/readonlyConstructorAssignment.types b/tests/baselines/reference/readonlyConstructorAssignment.types deleted file mode 100644 index 10f1cd20470..00000000000 --- a/tests/baselines/reference/readonlyConstructorAssignment.types +++ /dev/null @@ -1,16 +0,0 @@ -=== tests/cases/conformance/classes/constructorDeclarations/constructorParameters/readonlyConstructorAssignment.ts === -class A { ->A : A - - constructor(readonly x: number) { ->x : number - - this.x = 7; ->this.x = 7 : number ->this.x : number ->this : this ->x : number ->7 : number - } -} - diff --git a/tests/cases/conformance/classes/constructorDeclarations/constructorParameters/readonlyConstructorAssignment.ts b/tests/cases/conformance/classes/constructorDeclarations/constructorParameters/readonlyConstructorAssignment.ts index a81bf69249c..aeecf085f2f 100644 --- a/tests/cases/conformance/classes/constructorDeclarations/constructorParameters/readonlyConstructorAssignment.ts +++ b/tests/cases/conformance/classes/constructorDeclarations/constructorParameters/readonlyConstructorAssignment.ts @@ -1,5 +1,38 @@ +// Tests that readonly parameter properties behave like regular readonly properties + class A { constructor(readonly x: number) { - this.x = 7; + this.x = 0; + } +} + +class B extends A { + constructor(x: number) { + super(x); + // Fails, x is readonly + this.x = 1; + } +} + +class C extends A { + // This is the usual behavior of readonly properties: + // if one is redeclared in a base class, then it can be assigned to. + constructor(readonly x: number) { + super(x); + this.x = 1; + } +} + +class D { + constructor(private readonly x: number) { + this.x = 0; + } +} + +// Fails, can't redeclare readonly property +class E extends D { + constructor(readonly x: number) { + super(x); + this.x = 1; } }