Skip to content

Commit 1737598

Browse files
mhegazymhegazy
authored
Merge pull request microsoft#15704 from Microsoft/Fix15062-2
Fix microsoft#15062: report errors for parameter properties in private constructors
2 parents 25796f0 + 0349caa commit 1737598

13 files changed

+248
-7
lines changed

‎src/compiler/declarationEmitter.ts

+9-2
Original file line numberDiff line numberDiff line change
@@ -1266,7 +1266,9 @@ namespace ts {
12661266
Diagnostics.Exported_variable_0_has_or_is_using_private_name_1;
12671267
}
12681268
// This check is to ensure we don't report error on constructor parameter property as that error would be reported during parameter emit
1269-
else if (node.kind === SyntaxKind.PropertyDeclaration || node.kind === SyntaxKind.PropertySignature) {
1269+
// The only exception here is if the constructor was marked as private. we are not emitting the constructor parameters at all.
1270+
else if (node.kind === SyntaxKind.PropertyDeclaration || node.kind === SyntaxKind.PropertySignature ||
1271+
(node.kind === SyntaxKind.Parameter && hasModifier(node.parent, ModifierFlags.Private))) {
12701272
// TODO(jfreeman): Deal with computed properties in error reporting.
12711273
if (hasModifier(node, ModifierFlags.Static)) {
12721274
return symbolAccessibilityResult.errorModuleName ?
@@ -1275,7 +1277,7 @@ namespace ts {
12751277
Diagnostics.Public_static_property_0_of_exported_class_has_or_is_using_name_1_from_private_module_2 :
12761278
Diagnostics.Public_static_property_0_of_exported_class_has_or_is_using_private_name_1;
12771279
}
1278-
else if (node.parent.kind === SyntaxKind.ClassDeclaration) {
1280+
else if (node.parent.kind === SyntaxKind.ClassDeclaration || node.kind === SyntaxKind.Parameter) {
12791281
return symbolAccessibilityResult.errorModuleName ?
12801282
symbolAccessibilityResult.accessibility === SymbolAccessibility.CannotBeNamed ?
12811283
Diagnostics.Public_property_0_of_exported_class_has_or_is_using_name_1_from_external_module_2_but_cannot_be_named :
@@ -1501,6 +1503,11 @@ namespace ts {
15011503
write("[");
15021504
}
15031505
else {
1506+
if (node.kind === SyntaxKind.Constructor && hasModifier(node, ModifierFlags.Private)) {
1507+
write("();");
1508+
writeLine();
1509+
return;
1510+
}
15041511
// Construct signature or constructor type write new Signature
15051512
if (node.kind === SyntaxKind.ConstructSignature || node.kind === SyntaxKind.ConstructorType) {
15061513
write("new ");

‎tests/baselines/reference/classConstructorAccessibility.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ declare class C {
8989
}
9090
declare class D {
9191
x: number;
92-
private constructor(x);
92+
private constructor();
9393
}
9494
declare class E {
9595
x: number;

‎tests/baselines/reference/classConstructorAccessibility2.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ declare class BaseB {
135135
}
136136
declare class BaseC {
137137
x: number;
138-
private constructor(x);
138+
private constructor();
139139
createInstance(): void;
140140
static staticInstance(): void;
141141
}

‎tests/baselines/reference/classConstructorAccessibility3.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ declare class Baz {
9090
}
9191
declare class Qux {
9292
x: number;
93-
private constructor(x);
93+
private constructor();
9494
}
9595
declare let a: typeof Foo;
9696
declare let b: typeof Baz;

‎tests/baselines/reference/classConstructorOverloadsAccessibility.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ var D = (function () {
5959
declare class A {
6060
constructor(a: boolean);
6161
protected constructor(a: number);
62-
private constructor(a);
62+
private constructor();
6363
}
6464
declare class B {
6565
protected constructor(a: number);

‎tests/baselines/reference/declarationEmitClassPrivateConstructor.js

+72
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
//// [declarationEmitClassPrivateConstructor.ts]
2+
interface PrivateInterface {
3+
}
4+
5+
export class ExportedClass1 {
6+
private constructor(data: PrivateInterface) { }
7+
}
8+
9+
export class ExportedClass2 {
10+
private constructor(private data: PrivateInterface) { }
11+
}
12+
13+
export class ExportedClass3 {
14+
private constructor(private data: PrivateInterface, private n: number) { }
15+
}
16+
17+
export class ExportedClass4 {
18+
private constructor(private data: PrivateInterface, public n:number) { }
19+
}
20+
21+
//// [declarationEmitClassPrivateConstructor.js]
22+
"use strict";
23+
Object.defineProperty(exports, "__esModule", { value: true });
24+
var ExportedClass1 = (function () {
25+
function ExportedClass1(data) {
26+
}
27+
return ExportedClass1;
28+
}());
29+
exports.ExportedClass1 = ExportedClass1;
30+
var ExportedClass2 = (function () {
31+
function ExportedClass2(data) {
32+
this.data = data;
33+
}
34+
return ExportedClass2;
35+
}());
36+
exports.ExportedClass2 = ExportedClass2;
37+
var ExportedClass3 = (function () {
38+
function ExportedClass3(data, n) {
39+
this.data = data;
40+
this.n = n;
41+
}
42+
return ExportedClass3;
43+
}());
44+
exports.ExportedClass3 = ExportedClass3;
45+
var ExportedClass4 = (function () {
46+
function ExportedClass4(data, n) {
47+
this.data = data;
48+
this.n = n;
49+
}
50+
return ExportedClass4;
51+
}());
52+
exports.ExportedClass4 = ExportedClass4;
53+
54+
55+
//// [declarationEmitClassPrivateConstructor.d.ts]
56+
export declare class ExportedClass1 {
57+
private constructor();
58+
}
59+
export declare class ExportedClass2 {
60+
private data;
61+
private constructor();
62+
}
63+
export declare class ExportedClass3 {
64+
private data;
65+
private n;
66+
private constructor();
67+
}
68+
export declare class ExportedClass4 {
69+
private data;
70+
n: number;
71+
private constructor();
72+
}

‎tests/baselines/reference/declarationEmitClassPrivateConstructor.symbols

+38
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
=== tests/cases/compiler/declarationEmitClassPrivateConstructor.ts ===
2+
interface PrivateInterface {
3+
>PrivateInterface : Symbol(PrivateInterface, Decl(declarationEmitClassPrivateConstructor.ts, 0, 0))
4+
}
5+
6+
export class ExportedClass1 {
7+
>ExportedClass1 : Symbol(ExportedClass1, Decl(declarationEmitClassPrivateConstructor.ts, 1, 1))
8+
9+
private constructor(data: PrivateInterface) { }
10+
>data : Symbol(data, Decl(declarationEmitClassPrivateConstructor.ts, 4, 24))
11+
>PrivateInterface : Symbol(PrivateInterface, Decl(declarationEmitClassPrivateConstructor.ts, 0, 0))
12+
}
13+
14+
export class ExportedClass2 {
15+
>ExportedClass2 : Symbol(ExportedClass2, Decl(declarationEmitClassPrivateConstructor.ts, 5, 1))
16+
17+
private constructor(private data: PrivateInterface) { }
18+
>data : Symbol(ExportedClass2.data, Decl(declarationEmitClassPrivateConstructor.ts, 8, 24))
19+
>PrivateInterface : Symbol(PrivateInterface, Decl(declarationEmitClassPrivateConstructor.ts, 0, 0))
20+
}
21+
22+
export class ExportedClass3 {
23+
>ExportedClass3 : Symbol(ExportedClass3, Decl(declarationEmitClassPrivateConstructor.ts, 9, 1))
24+
25+
private constructor(private data: PrivateInterface, private n: number) { }
26+
>data : Symbol(ExportedClass3.data, Decl(declarationEmitClassPrivateConstructor.ts, 12, 24))
27+
>PrivateInterface : Symbol(PrivateInterface, Decl(declarationEmitClassPrivateConstructor.ts, 0, 0))
28+
>n : Symbol(ExportedClass3.n, Decl(declarationEmitClassPrivateConstructor.ts, 12, 55))
29+
}
30+
31+
export class ExportedClass4 {
32+
>ExportedClass4 : Symbol(ExportedClass4, Decl(declarationEmitClassPrivateConstructor.ts, 13, 1))
33+
34+
private constructor(private data: PrivateInterface, public n:number) { }
35+
>data : Symbol(ExportedClass4.data, Decl(declarationEmitClassPrivateConstructor.ts, 16, 24))
36+
>PrivateInterface : Symbol(PrivateInterface, Decl(declarationEmitClassPrivateConstructor.ts, 0, 0))
37+
>n : Symbol(ExportedClass4.n, Decl(declarationEmitClassPrivateConstructor.ts, 16, 55))
38+
}

‎tests/baselines/reference/declarationEmitClassPrivateConstructor.types

+38
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
=== tests/cases/compiler/declarationEmitClassPrivateConstructor.ts ===
2+
interface PrivateInterface {
3+
>PrivateInterface : PrivateInterface
4+
}
5+
6+
export class ExportedClass1 {
7+
>ExportedClass1 : ExportedClass1
8+
9+
private constructor(data: PrivateInterface) { }
10+
>data : PrivateInterface
11+
>PrivateInterface : PrivateInterface
12+
}
13+
14+
export class ExportedClass2 {
15+
>ExportedClass2 : ExportedClass2
16+
17+
private constructor(private data: PrivateInterface) { }
18+
>data : PrivateInterface
19+
>PrivateInterface : PrivateInterface
20+
}
21+
22+
export class ExportedClass3 {
23+
>ExportedClass3 : ExportedClass3
24+
25+
private constructor(private data: PrivateInterface, private n: number) { }
26+
>data : PrivateInterface
27+
>PrivateInterface : PrivateInterface
28+
>n : number
29+
}
30+
31+
export class ExportedClass4 {
32+
>ExportedClass4 : ExportedClass4
33+
34+
private constructor(private data: PrivateInterface, public n:number) { }
35+
>data : PrivateInterface
36+
>PrivateInterface : PrivateInterface
37+
>n : number
38+
}

‎tests/baselines/reference/declarationEmitClassPrivateConstructor2.errors.txt

+20
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
tests/cases/compiler/declarationEmitClassPrivateConstructor2.ts(5,38): error TS4031: Public property 'data' of exported class has or is using private name 'PrivateInterface'.
2+
tests/cases/compiler/declarationEmitClassPrivateConstructor2.ts(10,33): error TS4063: Parameter 'data' of constructor from exported class has or is using private name 'PrivateInterface'.
3+
4+
5+
==== tests/cases/compiler/declarationEmitClassPrivateConstructor2.ts (2 errors) ====
6+
interface PrivateInterface {
7+
}
8+
9+
export class ExportedClass1 {
10+
private constructor(public data: PrivateInterface) { }
11+
~~~~~~~~~~~~~~~~
12+
!!! error TS4031: Public property 'data' of exported class has or is using private name 'PrivateInterface'.
13+
}
14+
15+
16+
export class ExportedClass2 {
17+
protected constructor(data: PrivateInterface) { }
18+
~~~~~~~~~~~~~~~~
19+
!!! error TS4063: Parameter 'data' of constructor from exported class has or is using private name 'PrivateInterface'.
20+
}

‎tests/baselines/reference/declarationEmitClassPrivateConstructor2.js

+29
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
//// [declarationEmitClassPrivateConstructor2.ts]
2+
interface PrivateInterface {
3+
}
4+
5+
export class ExportedClass1 {
6+
private constructor(public data: PrivateInterface) { }
7+
}
8+
9+
10+
export class ExportedClass2 {
11+
protected constructor(data: PrivateInterface) { }
12+
}
13+
14+
//// [declarationEmitClassPrivateConstructor2.js]
15+
"use strict";
16+
Object.defineProperty(exports, "__esModule", { value: true });
17+
var ExportedClass1 = (function () {
18+
function ExportedClass1(data) {
19+
this.data = data;
20+
}
21+
return ExportedClass1;
22+
}());
23+
exports.ExportedClass1 = ExportedClass1;
24+
var ExportedClass2 = (function () {
25+
function ExportedClass2(data) {
26+
}
27+
return ExportedClass2;
28+
}());
29+
exports.ExportedClass2 = ExportedClass2;

‎tests/baselines/reference/typesWithPrivateConstructor.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ declare class C {
3838
declare var c: any;
3939
declare var r: () => void;
4040
declare class C2 {
41-
private constructor(x);
41+
private constructor();
4242
}
4343
declare var c2: any;
4444
declare var r2: (x: number) => void;

‎tests/cases/compiler/declarationEmitClassPrivateConstructor.ts

+22
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
// @target: es5
2+
// @module: commonjs
3+
// @declaration: true
4+
5+
interface PrivateInterface {
6+
}
7+
8+
export class ExportedClass1 {
9+
private constructor(data: PrivateInterface) { }
10+
}
11+
12+
export class ExportedClass2 {
13+
private constructor(private data: PrivateInterface) { }
14+
}
15+
16+
export class ExportedClass3 {
17+
private constructor(private data: PrivateInterface, private n: number) { }
18+
}
19+
20+
export class ExportedClass4 {
21+
private constructor(private data: PrivateInterface, public n:number) { }
22+
}

‎tests/cases/compiler/declarationEmitClassPrivateConstructor2.ts

+15
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
// @target: es5
2+
// @module: commonjs
3+
// @declaration: true
4+
5+
interface PrivateInterface {
6+
}
7+
8+
export class ExportedClass1 {
9+
private constructor(public data: PrivateInterface) { }
10+
}
11+
12+
13+
export class ExportedClass2 {
14+
protected constructor(data: PrivateInterface) { }
15+
}

0 commit comments

Comments
 (0)