Merge pull request microsoft#8555 from Microsoft/readonly_ctr

Andy · Andy · commit 7806de0f45ac · 2016-05-12T10:29:22.000-07:00
Fix microsoft#7590: Allow 'readonly' to be used in constructor parameters
diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
@@ -12680,7 +12680,7 @@ namespace ts {
 
             checkVariableLikeDeclaration(node);
             let func = getContainingFunction(node);
-            if (node.flags & NodeFlags.AccessibilityModifier) {
+            if (node.flags & NodeFlags.ParameterPropertyModifier) {
                 func = getContainingFunction(node);
                 if (!(func.kind === SyntaxKind.Constructor && nodeIsPresent(func.body))) {
                     error(node, Diagnostics.A_parameter_property_is_only_allowed_in_a_constructor_implementation);
@@ -13016,7 +13016,7 @@ namespace ts {
                     //   or the containing class declares instance member variables with initializers.
                     const superCallShouldBeFirst =
                         forEach((<ClassDeclaration>node.parent).members, isInstancePropertyWithInitializer) ||
-                        forEach(node.parameters, p => p.flags & (NodeFlags.Public | NodeFlags.Private | NodeFlags.Protected));
+                        forEach(node.parameters, p => p.flags & NodeFlags.ParameterPropertyModifier);
 
                     // Skip past any prologue directives to find the first statement
                     // to ensure that it was a super call.
@@ -17673,7 +17673,8 @@ namespace ts {
                         if (flags & NodeFlags.Readonly) {
                             return grammarErrorOnNode(modifier, Diagnostics._0_modifier_already_seen, "readonly");
                         }
-                        else if (node.kind !== SyntaxKind.PropertyDeclaration && node.kind !== SyntaxKind.PropertySignature && node.kind !== SyntaxKind.IndexSignature) {
+                        else if (node.kind !== SyntaxKind.PropertyDeclaration && node.kind !== SyntaxKind.PropertySignature && node.kind !== SyntaxKind.IndexSignature && node.kind !== SyntaxKind.Parameter) {
+                            // If node.kind === SyntaxKind.Parameter, checkParameter report an error if it's not a parameter property.
                             return grammarErrorOnNode(modifier, Diagnostics.readonly_modifier_can_only_appear_on_a_property_declaration_or_index_signature);
                         }
                         flags |= NodeFlags.Readonly;
@@ -17781,7 +17782,7 @@ namespace ts {
             else if ((node.kind === SyntaxKind.ImportDeclaration || node.kind === SyntaxKind.ImportEqualsDeclaration) && flags & NodeFlags.Ambient) {
                 return grammarErrorOnNode(lastDeclare, Diagnostics.A_0_modifier_cannot_be_used_with_an_import_declaration, "declare");
             }
-            else if (node.kind === SyntaxKind.Parameter && (flags & NodeFlags.AccessibilityModifier) && isBindingPattern((<ParameterDeclaration>node).name)) {
+            else if (node.kind === SyntaxKind.Parameter && (flags & NodeFlags.ParameterPropertyModifier) && isBindingPattern((<ParameterDeclaration>node).name)) {
                 return grammarErrorOnNode(node, Diagnostics.A_parameter_property_may_not_be_a_binding_pattern);
             }
             if (flags & NodeFlags.Async) {
@@ -18246,9 +18247,6 @@ namespace ts {
                     if (parameter.dotDotDotToken) {
                         return grammarErrorOnNode(parameter.dotDotDotToken, Diagnostics.A_set_accessor_cannot_have_rest_parameter);
                     }
-                    else if (parameter.flags & NodeFlags.Modifier) {
-                        return grammarErrorOnNode(accessor.name, Diagnostics.A_parameter_property_is_only_allowed_in_a_constructor_implementation);
-                    }
                     else if (parameter.questionToken) {
                         return grammarErrorOnNode(parameter.questionToken, Diagnostics.A_set_accessor_cannot_have_an_optional_parameter);
                     }
diff --git a/src/compiler/declarationEmitter.ts b/src/compiler/declarationEmitter.ts
@@ -1051,7 +1051,7 @@ namespace ts {
             function emitParameterProperties(constructorDeclaration: ConstructorDeclaration) {
                 if (constructorDeclaration) {
                     forEach(constructorDeclaration.parameters, param => {
-                        if (param.flags & NodeFlags.AccessibilityModifier) {
+                        if (param.flags & NodeFlags.ParameterPropertyModifier) {
                             emitPropertyDeclaration(param);
                         }
                     });
diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts
@@ -4979,7 +4979,7 @@ const _super = (function (geti, seti) {
 
             function emitParameterPropertyAssignments(node: ConstructorDeclaration) {
                 forEach(node.parameters, param => {
-                    if (param.flags & NodeFlags.AccessibilityModifier) {
+                    if (param.flags & NodeFlags.ParameterPropertyModifier) {
                         writeLine();
                         emitStart(param);
                         emitStart(param.name);
diff --git a/src/compiler/types.ts b/src/compiler/types.ts
@@ -407,8 +407,10 @@ namespace ts {
         HasAggregatedChildData = 1 << 29,  // If we've computed data from children and cached it in this node
         HasJsxSpreadAttribute = 1 << 30,
 
-        Modifier = Export | Ambient | Public | Private | Protected | Static | Abstract | Default | Async,
+        Modifier = Export | Ambient | Public | Private | Protected | Static | Abstract | Default | Async | Readonly,
         AccessibilityModifier = Public | Private | Protected,
+        // Accessibility modifiers and 'readonly' can be attached to a parameter in a constructor to make it a property.
+        ParameterPropertyModifier = AccessibilityModifier | Readonly,
         BlockScoped = Let | Const,
 
         ReachabilityCheckFlags = HasImplicitReturn | HasExplicitReturn,
diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts
@@ -3040,7 +3040,7 @@ namespace ts {
     }
 
     export function isParameterPropertyDeclaration(node: ParameterDeclaration): boolean {
-        return node.flags & NodeFlags.AccessibilityModifier && node.parent.kind === SyntaxKind.Constructor && isClassLike(node.parent.parent);
+        return node.flags & NodeFlags.ParameterPropertyModifier && node.parent.kind === SyntaxKind.Constructor && isClassLike(node.parent.parent);
     }
 
     export function startsWith(str: string, prefix: string): boolean {
diff --git a/tests/baselines/reference/accessorParameterAccessibilityModifier.errors.txt b/tests/baselines/reference/accessorParameterAccessibilityModifier.errors.txt
@@ -1,20 +1,14 @@
-tests/cases/compiler/accessorParameterAccessibilityModifier.ts(3,9): error TS2369: A parameter property is only allowed in a constructor implementation.
 tests/cases/compiler/accessorParameterAccessibilityModifier.ts(3,11): error TS2369: A parameter property is only allowed in a constructor implementation.
-tests/cases/compiler/accessorParameterAccessibilityModifier.ts(4,16): error TS2369: A parameter property is only allowed in a constructor implementation.
 tests/cases/compiler/accessorParameterAccessibilityModifier.ts(4,18): error TS2369: A parameter property is only allowed in a constructor implementation.
 
 
-==== tests/cases/compiler/accessorParameterAccessibilityModifier.ts (4 errors) ====
+==== tests/cases/compiler/accessorParameterAccessibilityModifier.ts (2 errors) ====
     
     class C {
         set X(public v) { }
-            ~
-!!! error TS2369: A parameter property is only allowed in a constructor implementation.
               ~~~~~~~~
 !!! error TS2369: A parameter property is only allowed in a constructor implementation.
         static set X(public v2) { }
-                   ~
-!!! error TS2369: A parameter property is only allowed in a constructor implementation.
                      ~~~~~~~~~
 !!! error TS2369: A parameter property is only allowed in a constructor implementation.
     }
diff --git a/tests/baselines/reference/declarationEmit_readonly.js b/tests/baselines/reference/declarationEmit_readonly.js
@@ -0,0 +1,20 @@
+//// [declarationEmit_readonly.ts]
+
+class C {
+    constructor(readonly x: number) {}
+}
+
+//// [declarationEmit_readonly.js]
+var C = (function () {
+    function C(x) {
+        this.x = x;
+    }
+    return C;
+}());
+
+
+//// [declarationEmit_readonly.d.ts]
+declare class C {
+    readonly x: number;
+    constructor(x: number);
+}
diff --git a/tests/baselines/reference/declarationEmit_readonly.symbols b/tests/baselines/reference/declarationEmit_readonly.symbols
@@ -0,0 +1,8 @@
+=== tests/cases/conformance/classes/constructorDeclarations/constructorParameters/declarationEmit_readonly.ts ===
+
+class C {
+>C : Symbol(C, Decl(declarationEmit_readonly.ts, 0, 0))
+
+    constructor(readonly x: number) {}
+>x : Symbol(C.x, Decl(declarationEmit_readonly.ts, 2, 16))
+}
diff --git a/tests/baselines/reference/declarationEmit_readonly.types b/tests/baselines/reference/declarationEmit_readonly.types
@@ -0,0 +1,8 @@
+=== tests/cases/conformance/classes/constructorDeclarations/constructorParameters/declarationEmit_readonly.ts ===
+
+class C {
+>C : C
+
+    constructor(readonly x: number) {}
+>x : number
+}
diff --git a/tests/baselines/reference/parserMemberAccessorDeclaration15.errors.txt b/tests/baselines/reference/parserMemberAccessorDeclaration15.errors.txt
@@ -1,12 +1,9 @@
-tests/cases/conformance/parser/ecmascript5/MemberAccessorDeclarations/parserMemberAccessorDeclaration15.ts(2,8): error TS2369: A parameter property is only allowed in a constructor implementation.
 tests/cases/conformance/parser/ecmascript5/MemberAccessorDeclarations/parserMemberAccessorDeclaration15.ts(2,12): error TS2369: A parameter property is only allowed in a constructor implementation.
 
 
-==== tests/cases/conformance/parser/ecmascript5/MemberAccessorDeclarations/parserMemberAccessorDeclaration15.ts (2 errors) ====
+==== tests/cases/conformance/parser/ecmascript5/MemberAccessorDeclarations/parserMemberAccessorDeclaration15.ts (1 errors) ====
     class C {
        set Foo(public a: number) { }
-           ~~~
-!!! error TS2369: A parameter property is only allowed in a constructor implementation.
                ~~~~~~~~~~~~~~~~
 !!! error TS2369: A parameter property is only allowed in a constructor implementation.
     }
diff --git a/tests/baselines/reference/readonlyInAmbientClass.errors.txt b/tests/baselines/reference/readonlyInAmbientClass.errors.txt
@@ -0,0 +1,13 @@
+tests/cases/conformance/classes/constructorDeclarations/constructorParameters/readonlyInAmbientClass.ts(2,14): error TS2369: A parameter property is only allowed in a constructor implementation.
+tests/cases/conformance/classes/constructorDeclarations/constructorParameters/readonlyInAmbientClass.ts(3,9): error TS2369: A parameter property is only allowed in a constructor implementation.
+
+
+==== tests/cases/conformance/classes/constructorDeclarations/constructorParameters/readonlyInAmbientClass.ts (2 errors) ====
+    declare class C{
+    	constructor(readonly x: number);
+    	            ~~~~~~~~~~~~~~~~~~
+!!! error TS2369: A parameter property is only allowed in a constructor implementation.
+    	method(readonly x: number);
+    	       ~~~~~~~~~~~~~~~~~~
+!!! error TS2369: A parameter property is only allowed in a constructor implementation.
+    }
diff --git a/tests/baselines/reference/readonlyInAmbientClass.js b/tests/baselines/reference/readonlyInAmbientClass.js
@@ -0,0 +1,7 @@
+//// [readonlyInAmbientClass.ts]
+declare class C{
+	constructor(readonly x: number);
+	method(readonly x: number);
+}
+
+//// [readonlyInAmbientClass.js]
diff --git a/tests/baselines/reference/readonlyInConstructorParameters.errors.txt b/tests/baselines/reference/readonlyInConstructorParameters.errors.txt
@@ -0,0 +1,25 @@
+tests/cases/conformance/classes/constructorDeclarations/constructorParameters/readonlyInConstructorParameters.ts(4,1): error TS2450: Left-hand side of assignment expression cannot be a constant or a read-only property.
+tests/cases/conformance/classes/constructorDeclarations/constructorParameters/readonlyInConstructorParameters.ts(7,26): error TS1029: 'public' modifier must precede 'readonly' modifier.
+tests/cases/conformance/classes/constructorDeclarations/constructorParameters/readonlyInConstructorParameters.ts(13,10): error TS2341: Property 'x' is private and only accessible within class 'F'.
+
+
+==== tests/cases/conformance/classes/constructorDeclarations/constructorParameters/readonlyInConstructorParameters.ts (3 errors) ====
+    class C {
+        constructor(readonly x: number) {}
+    }
+    new C(1).x = 2;
+    ~~~~~~~~~~
+!!! error TS2450: Left-hand side of assignment expression cannot be a constant or a read-only property.
+    
+    class E {
+        constructor(readonly public x: number) {}
+                             ~~~~~~
+!!! error TS1029: 'public' modifier must precede 'readonly' modifier.
+    }
+    
+    class F {
+        constructor(private readonly x: number) {}
+    }
+    new F(1).x;
+             ~
+!!! error TS2341: Property 'x' is private and only accessible within class 'F'.
diff --git a/tests/baselines/reference/readonlyInConstructorParameters.js b/tests/baselines/reference/readonlyInConstructorParameters.js
@@ -0,0 +1,36 @@
+//// [readonlyInConstructorParameters.ts]
+class C {
+    constructor(readonly x: number) {}
+}
+new C(1).x = 2;
+
+class E {
+    constructor(readonly public x: number) {}
+}
+
+class F {
+    constructor(private readonly x: number) {}
+}
+new F(1).x;
+
+//// [readonlyInConstructorParameters.js]
+var C = (function () {
+    function C(x) {
+        this.x = x;
+    }
+    return C;
+}());
+new C(1).x = 2;
+var E = (function () {
+    function E(x) {
+        this.x = x;
+    }
+    return E;
+}());
+var F = (function () {
+    function F(x) {
+        this.x = x;
+    }
+    return F;
+}());
+new F(1).x;
diff --git a/tests/baselines/reference/readonlyInNonPropertyParameters.errors.txt b/tests/baselines/reference/readonlyInNonPropertyParameters.errors.txt
@@ -0,0 +1,21 @@
+tests/cases/compiler/readonlyInNonPropertyParameters.ts(4,9): error TS2369: A parameter property is only allowed in a constructor implementation.
+tests/cases/compiler/readonlyInNonPropertyParameters.ts(5,8): error TS2369: A parameter property is only allowed in a constructor implementation.
+tests/cases/compiler/readonlyInNonPropertyParameters.ts(7,2): error TS2369: A parameter property is only allowed in a constructor implementation.
+
+
+==== tests/cases/compiler/readonlyInNonPropertyParameters.ts (3 errors) ====
+    
+    // `readonly` won't work outside of property parameters
+    class X {
+    	method(readonly x: number) {}
+    	       ~~~~~~~~~~~~~~~~~~
+!!! error TS2369: A parameter property is only allowed in a constructor implementation.
+    	set x(readonly value: number) {}
+    	      ~~~~~~~~~~~~~~~~~~~~~~
+!!! error TS2369: A parameter property is only allowed in a constructor implementation.
+    }
+    (readonly x) => 0;
+     ~~~~~~~~~~
+!!! error TS2369: A parameter property is only allowed in a constructor implementation.
+    // OK to use `readonly` as a name
+    (readonly) => 0;
diff --git a/tests/baselines/reference/readonlyInNonPropertyParameters.js b/tests/baselines/reference/readonlyInNonPropertyParameters.js
@@ -0,0 +1,27 @@
+//// [readonlyInNonPropertyParameters.ts]
+
+// `readonly` won't work outside of property parameters
+class X {
+	method(readonly x: number) {}
+	set x(readonly value: number) {}
+}
+(readonly x) => 0;
+// OK to use `readonly` as a name
+(readonly) => 0;
+
+//// [readonlyInNonPropertyParameters.js]
+// `readonly` won't work outside of property parameters
+var X = (function () {
+    function X() {
+    }
+    X.prototype.method = function (x) { };
+    Object.defineProperty(X.prototype, "x", {
+        set: function (value) { },
+        enumerable: true,
+        configurable: true
+    });
+    return X;
+}());
+(function (x) { return 0; });
+// OK to use `readonly` as a name
+(function (readonly) { return 0; });
diff --git a/tests/baselines/reference/readonlyReadonly.errors.txt b/tests/baselines/reference/readonlyReadonly.errors.txt
@@ -0,0 +1,13 @@
+tests/cases/conformance/classes/constructorDeclarations/constructorParameters/readonlyReadonly.ts(2,14): error TS1030: 'readonly' modifier already seen.
+tests/cases/conformance/classes/constructorDeclarations/constructorParameters/readonlyReadonly.ts(3,26): error TS1030: 'readonly' modifier already seen.
+
+
+==== tests/cases/conformance/classes/constructorDeclarations/constructorParameters/readonlyReadonly.ts (2 errors) ====
+    class C {
+        readonly readonly x: number;
+                 ~~~~~~~~
+!!! error TS1030: 'readonly' modifier already seen.
+        constructor(readonly readonly y: number) {}
+                             ~~~~~~~~
+!!! error TS1030: 'readonly' modifier already seen.
+    }
diff --git a/tests/baselines/reference/readonlyReadonly.js b/tests/baselines/reference/readonlyReadonly.js
@@ -0,0 +1,13 @@
+//// [readonlyReadonly.ts]
+class C {
+    readonly readonly x: number;
+    constructor(readonly readonly y: number) {}
+}
+
+//// [readonlyReadonly.js]
+var C = (function () {
+    function C(y) {
+        this.y = y;
+    }
+    return C;
+}());
diff --git a/tests/cases/compiler/readonlyInNonPropertyParameters.ts b/tests/cases/compiler/readonlyInNonPropertyParameters.ts
@@ -0,0 +1,10 @@
+//@target: ES5
+
+// `readonly` won't work outside of property parameters
+class X {
+	method(readonly x: number) {}
+	set x(readonly value: number) {}
+}
+(readonly x) => 0;
+// OK to use `readonly` as a name
+(readonly) => 0;
diff --git a/tests/cases/conformance/classes/constructorDeclarations/constructorParameters/declarationEmit_readonly.ts b/tests/cases/conformance/classes/constructorDeclarations/constructorParameters/declarationEmit_readonly.ts
@@ -0,0 +1,5 @@
+// @declaration: true
+
+class C {
+    constructor(readonly x: number) {}
+}
diff --git a/tests/cases/conformance/classes/constructorDeclarations/constructorParameters/readonlyInAmbientClass.ts b/tests/cases/conformance/classes/constructorDeclarations/constructorParameters/readonlyInAmbientClass.ts
@@ -0,0 +1,4 @@
+declare class C{
+	constructor(readonly x: number);
+	method(readonly x: number);
+}
diff --git a/tests/cases/conformance/classes/constructorDeclarations/constructorParameters/readonlyInConstructorParameters.ts b/tests/cases/conformance/classes/constructorDeclarations/constructorParameters/readonlyInConstructorParameters.ts
@@ -0,0 +1,13 @@
+class C {
+    constructor(readonly x: number) {}
+}
+new C(1).x = 2;
+
+class E {
+    constructor(readonly public x: number) {}
+}
+
+class F {
+    constructor(private readonly x: number) {}
+}
+new F(1).x;
diff --git a/tests/cases/conformance/classes/constructorDeclarations/constructorParameters/readonlyReadonly.ts b/tests/cases/conformance/classes/constructorDeclarations/constructorParameters/readonlyReadonly.ts
@@ -0,0 +1,4 @@
+class C {
+    readonly readonly x: number;
+    constructor(readonly readonly y: number) {}
+}

Original file line number	Diff line number	Diff line change
`@@ -1051,7 +1051,7 @@ namespace ts {`
`1051`	`1051`	`function emitParameterProperties(constructorDeclaration: ConstructorDeclaration) {`
`1052`	`1052`	`if (constructorDeclaration) {`
`1053`	`1053`	`forEach(constructorDeclaration.parameters, param => {`
`1054`		`- if (param.flags & NodeFlags.AccessibilityModifier) {`
	`1054`	`+ if (param.flags & NodeFlags.ParameterPropertyModifier) {`
`1055`	`1055`	`emitPropertyDeclaration(param);`
`1056`	`1056`	`}`
`1057`	`1057`	`});`
Original file line number	Diff line number	Diff line change
`@@ -3040,7 +3040,7 @@ namespace ts {`
`3040`	`3040`	`}`
`3041`	`3041`
`3042`	`3042`	`export function isParameterPropertyDeclaration(node: ParameterDeclaration): boolean {`
`3043`		`- return node.flags & NodeFlags.AccessibilityModifier && node.parent.kind === SyntaxKind.Constructor && isClassLike(node.parent.parent);`
	`3043`	`+ return node.flags & NodeFlags.ParameterPropertyModifier && node.parent.kind === SyntaxKind.Constructor && isClassLike(node.parent.parent);`
`3044`	`3044`	`}`
`3045`	`3045`
`3046`	`3046`	`export function startsWith(str: string, prefix: string): boolean {`