8000 Add useDefineForClassFields flag for Set -> Define property declaration by sandersn · Pull Request #33509 · microsoft/TypeScript · GitHub
[go: up one dir, main page]

Skip to content

Add useDefineForClassFields flag for Set -> Define property declaration #33509

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 57 commits into from
Sep 26, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
57 commits
Select commit Hold shift + click to select a range
f76e01f
Disallow property/accessor o 10000 verrides
sandersn Sep 12, 2019
ac96739
Disallow uninitialised property overrides
sandersn Sep 13, 2019
c26785e
Merge branch 'master' into disallow-property-accessor-override
sandersn Sep 13, 2019
c4f334a
Updates from design review + fix ancient bug
sandersn Sep 13, 2019
70a15bd
Need to add a couple of errors and squash one
sandersn Sep 16, 2019
feb0fb6
Everything works so far
sandersn Sep 16, 2019
8686242
Check for constructor initialisation
sandersn Sep 16, 2019
2916c29
change error wording
sandersn Sep 16, 2019
2e5f57c
Improve error wording
sandersn Sep 16, 2019
f11f2be
Add codefix to add missing 'declare'
sandersn Sep 17, 2019
526ceb7
Merge branch 'master' into disallow-property-accessor-override
sandersn Sep 17, 2019
a8f4610
Merge branch 'master' into disallow-uninitialised-property-override
sandersn Sep 17, 2019
2238b66
Always emit accessors in .d.ts files
sandersn Sep 17, 2019
fcf0ff1
Allow 'declare' on any uninitialised property decl
sandersn Sep 17, 2019
810f923
Undo code moves
sandersn Sep 17, 2019
6408d7a
Let sleeping dogs lie
sandersn Sep 17, 2019
5ee3271
Correctly set NodeFlags.Ambient
sandersn Sep 17, 2019
e0ddced
Remove more unneeded code
sandersn Sep 17, 2019
610a62d
Merge branch 'disallow-property-accessor-override' into add-property-…
sandersn Sep 17, 2019
e7ba1ae
Merge branch 'disallow-uninitialised-property-override' into add-prop…
sandersn Sep 17, 2019
6c94395
Merge branch 'always-emit-accessors-in-dts' into add-property-define-…
sandersn Sep 17, 2019
6a066b9
Update baselines
sandersn Sep 17, 2019
0548220
Merge branch 'disallow-uninitialised-property-override' into sandersn…
sandersn Sep 17, 2019
7f69be7
Update baselines
sandersn Sep 17, 2019
8f42126
Merge branch 'always-emit-accessors-in-dts' into sandersn/add-propert…
sandersn Sep 17, 2019
d46a0db
Update baselines
sandersn Sep 17, 2019
58e1746
Ignore this-property assignments
sandersn Sep 19, 2019
9b358b1
Merge branch 'disallow-property-accessor-override' into sandersn/add-…
sandersn Sep 19, 2019
832d51f
Fix base-in-interface check
sandersn Sep 19, 2019
9c6aa17
Merge branch 'disallow-property-accessor-override' into sandersn/add-…
sandersn Sep 19, 2019
a645ca9
Do not error when base parent is interface
sandersn Sep 19, 2019
030c768
Fix base interface check
sandersn Sep 19, 2019
003d041
Add missed baselines
sandersn Sep 19, 2019
6ec445f
Fix check
sandersn Sep 19, 2019
c283574
Fix new errors in services
sandersn Sep 19, 2019
1c5f699
Fix new errors in services
sandersn Sep 19, 2019
2403116
Merge branch 'disallow-property-accessor-override' into sandersn/add-…
sandersn Sep 19, 2019
b45b5ba
Merge branch 'disallow-uninitialised-property-override' into sandersn…
sandersn Sep 19, 2019
8d99856
Fix errors in testRunner
sandersn Sep 19, 2019
f400822
Add flag and turn off errors when on
sandersn Sep 19, 2019
1f68bea
Structure of new emit is correct, fake content
sandersn Sep 20, 2019
6c35ac2
Basically right emit
sandersn Sep 20, 2019
fbeaf01
Fix one last unitialised property declaration
sandersn Sep 20, 2019
a5b1d8f
Haha no I missed another one
sandersn Sep 20, 2019
32a73b3
Fix whitespace back to CRLF
sandersn Sep 20, 2019
5028bfa
Minor fix and code cleanup
sandersn Sep 20, 2019
8053e05
New test case
sandersn Sep 20, 2019
7fdb423
Fix bug in isInitializedProperty
sandersn Sep 23, 2019
18c622c
Updates from design meeting.
sandersn Sep 23, 2019
8268f9a
Update baselines
sandersn Sep 23, 2019
5eeb2b0
Object.defineProperty for methods too
sandersn Sep 23, 2019
5ea2bc1
Update slow baselines
sandersn Sep 23, 2019
088daa8
Improve error message
sandersn Sep 23, 2019
e600ebe
Update src/compiler/transformers/utilities.ts
sandersn Sep 23, 2019
6e3bf5c
Add test of computed properties
sandersn Sep 23, 2019
ba876cf
Remove done TODO
sandersn Sep 23, 2019
6a1e24c
Merge branch 'master' into sandersn/add-property-define-flag
sandersn Sep 26, 2019
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
Next Next commit
Disallow property/accessor overrides
Unless the base property or accessor is abstract
  • Loading branch information
sandersn committed Sep 12, 2019
commit f76e01fbd98f3c847d5ee84e949c88b45893ee5c
23 changes: 18 additions & 5 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29513,14 +29513,27 @@ namespace ts {
// either base or derived property is private - not override, skip it
continue;
}

if (isPrototypeProperty(base) || base.flags & SymbolFlags.PropertyOrAccessor && derived.flags & SymbolFlags.PropertyOrAccessor) {
// method is overridden with method or property/accessor is overridden with property/accessor - correct case
if (isPrototypeProperty(base)) {
// method is overridden with method - correct case
continue;
}

let errorMessage: DiagnosticMessage;
if (isPrototypeProperty(base)) {
const basePropertyFlags = base.flags & SymbolFlags.PropertyOrAccessor;
const derivedPropertyFlags = derived.flags & SymbolFlags.PropertyOrAccessor;
if (basePropertyFlags && derivedPropertyFlags) {
// property/accessor is overridden with property/accessor
if (!(baseDeclarationFlags & ModifierFlags.Abstract) && basePropertyFlags !== SymbolFlags.Property && derivedPropertyFlags === SymbolFlags.Property) {
errorMessage = Diagnostics.Class_0_defines_instance_member_accessor_1_but_extended_class_2_defines_it_as_instance_member_property;
}
else if (!(baseDeclarationFlags & ModifierFlags.Abstract) && basePropertyFlags === SymbolFlags.Property && derivedPropertyFlags !== SymbolFlags.Property) {
errorMessage = Diagnostics.Class_0_defines_instance_member_property_1_but_extended_class_2_defines_it_as_instance_member_accessor;
}
else {
// correct case
continue;
}
}
else if (isPrototypeProperty(base)) {
if (derived.flags & SymbolFlags.Accessor) {
errorMessage = Diagnostics.Class_0_defines_instance_member_function_1_but_extended_class_2_defines_it_as_instance_member_accessor;
}
Expand Down
9 changes: 9 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -2204,6 +2204,15 @@
"category": "Error",
"code": 2609
},
"Class '{0}' defines instance member accessor '{1}', but extended class '{2}' defines it as instance member property.": {
"category": "Error",
"code": 2610
},
"Class '{0}' defines instance member property '{1}', but extended class '{2}' defines it as instance member accessor.": {
"category": "Error",
"code": 2611
},

"Cannot augment module '{0}' with value exports because it resolves to a non-module entity.": {
"category": "Error",
"code": 2649
Expand Down
1 change: 0 additions & 1 deletion src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3972,7 +3972,6 @@ namespace ts {
}

export function getModifierFlagsNoCache(node: Node): ModifierFlags {

let flags = ModifierFlags.None;
if (node.modifiers) {
for (const modifier of node.modifiers) {
Expand Down
24 changes: 24 additions & 0 deletions tests/baselines/reference/accessorsOverrideProperty.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
tests/cases/conformance/classes/propertyMemberDeclarations/accessorsOverrideProperty.ts(5,9): error TS2611: Class 'A' defines instance member property 'p', but extended class 'B' defines it as instance member accessor.
tests/cases/conformance/classes/propertyMemberDeclarations/accessorsOverrideProperty.ts(12,9): error TS2611: Class 'C' defines instance member property 'p', but extended class 'D' defines it as instance member accessor.


==== tests/cases/conformance/classes/propertyMemberDeclarations/accessorsOverrideProperty.ts (2 errors) ====
class A {
p = 'yep'
}
class B extends A {
get p() { return 'oh no' } // error
~
!!! error TS2611: Class 'A' defines instance member property 'p', but extended class 'B' defines it as instance member accessor.
}
class C {
p = 101
}
class D extends C {
_secret = 11
get p() { return this._secret } // error
~
!!! error TS2611: Class 'C' defines instance member property 'p', but extended class 'D' defines it as instance member accessor.
set p(value) { this._secret = value } // error
}

39 changes: 39 additions & 0 deletions tests/baselines/reference/accessorsOverrideProperty.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
//// [accessorsOverrideProperty.ts]
class A {
p = 'yep'
}
class B extends A {
get p() { return 'oh no' } // error
}
class C {
p = 101
}
class D extends C {
_secret = 11
get p() { return this._secret } // error
set p(value) { this._secret = value } // error
}


//// [accessorsOverrideProperty.js]
class A {
constructor() {
this.p = 'yep';
}
}
class B extends A {
get p() { return 'oh no'; } // error
}
class C {
constructor() {
this.p = 101;
}
}
class D extends C {
constructor() {
super(...arguments);
this._secret = 11;
}
get p() { return this._secret; } // error
set p(value) { this._secret = value; } // error
}
42 changes: 42 additions & 0 deletions tests/baselines/reference/accessorsOverrideProperty.symbols
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
=== tests/cases/conformance/classes/propertyMemberDeclarations/accessorsOverrideProperty.ts ===
class A {
>A : Symbol(A, Decl(accessorsOverrideProperty.ts, 0, 0))

p = 'yep'
>p : Symbol(A.p, Decl(accessorsOverrideProperty.ts, 0, 9))
}
class B extends A {
>B : Symbol(B, Decl(accessorsOverrideProperty.ts, 2, 1))
>A : Symbol(A, Decl(accessorsOverrideProperty.ts, 0, 0))

get p() { return 'oh no' } // error
>p : Symbol(B.p, Decl(accessorsOverrideProperty.ts, 3, 19))
}
class C {
>C : Symbol(C, Decl(accessorsOverrideProperty.ts, 5, 1))

p = 101
>p : Symbol(C.p, Decl(accessorsOverrideProperty.ts, 6, 9))
}
class D extends C {
>D : Symbol(D, Decl(accessorsOverrideProperty.ts, 8, 1))
>C : Symbol(C, Decl(accessorsOverrideProperty.ts, 5, 1))

_secret = 11
>_secret : Symbol(D._secret, Decl(accessorsOverrideProperty.ts, 9, 19))

get p() { return this._secret } // error
>p : Symbol(D.p, Decl(accessorsOverrideProperty.ts, 10, 17), Decl(accessorsOverrideProperty.ts, 11, 35))
>this._secret : Symbol(D._secret, Decl(accessorsOverrideProperty.ts, 9, 19))
>this : Symbol(D, Decl(accessorsOverrideProperty.ts, 8, 1))
>_secret : Symbol(D._secret, Decl(accessorsOverrideProperty.ts, 9, 19))

set p(value) { this._secret = value } // error
>p : Symbol(D.p, Decl(accessorsOverrideProperty.ts, 10, 17), Decl(accessorsOverrideProperty.ts, 11, 35))
>value : Symbol(value, Decl(accessorsOverrideProperty.ts, 12, 10))
>this._secret : Symbol(D._secret, Decl(accessorsOverrideProperty.ts, 9, 19))
>this : Symbol(D, Decl(accessorsOverrideProperty.ts, 8, 1))
>_secret : Symbol(D._secret, Decl(accessorsOverrideProperty.ts, 9, 19))
>value : Symbol(value, Decl(accessorsOverrideProperty.ts, 12, 10))
}

47 changes: 47 additions & 0 deletions tests/baselines/reference/accessorsOverrideProperty.types
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
=== tests/cases/conformance/classes/propertyMemberDeclarations/accessorsOverrideProperty.ts ===
class A {
>A : A

p = 'yep'
>p : string
>'yep' : "yep"
}
class B extends A {
>B : B
>A : A

get p() { return 'oh no' } // error
>p : string
>'oh no' : "oh no"
}
class C {
>C : C

p = 101
>p : number
>101 : 101
}
class D extends C {
>D : D
>C : C

_secret = 11
>_secret : number
>11 : 11

get p() { return this._secret } // error
>p : number
>this._secret : number
>this : this
>_secret : number

set p(value) { this._secret = value } // error
>p : number
>value : number
>this._secret = value : number
>this._secret : number
>this : this
>_secret : number
>value : number
}

18 changes: 18 additions & 0 deletions tests/baselines/reference/accessorsOverrideProperty2.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
tests/cases/conformance/classes/propertyMemberDeclarations/accessorsOverrideProperty2.ts(6,7): error TS2611: Class 'Base' defines instance member property 'x', but extended class 'Derived' defines it as instance member accessor.


==== tests/cases/conformance/classes/propertyMemberDeclarations/accessorsOverrideProperty2.ts (1 errors) ====
class Base {
x = 1;
}

class Derived extends Base {
get x() { return 2; } // should be an error
~
!!! error TS2611: Class 'Base' defines instance member property 'x', but extended class 'Derived' defines it as instance member accessor.
set x(value) { console.log(`x was set to ${value}`); }
}

const obj = new Derived(); // nothing printed
console.log(obj.x); // 1

26 changes: 26 additions & 0 deletions tests/baselines/reference/accessorsOverrideProperty2.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
//// [accessorsOverrideProperty2.ts]
class Base {
x = 1;
}

class Derived extends Base {
get x() { return 2; } // should be an error
set x(value) { console.log(`x was set to ${value}`); }
}

const obj = new Derived(); // nothing printed
console.log(obj.x); // 1


//// [accessorsOverrideProperty2.js]
class Base {
constructor() {
this.x = 1;
}
}
class Derived extends Base {
get x() { return 2; } // should be an error
set x(value) { console.log(`x was set to ${value}`); }
}
const obj = new Derived(); // nothing printed
console.log(obj.x); // 1
36 changes: 36 additions & 0 deletions tests/baselines/reference/accessorsOverrideProperty2.symbols
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
=== tests/cases/conformance/classes/propertyMemberDeclarations/accessorsOverrideProperty2.ts ===
class Base {
>Base : Symbol(Base, Decl(accessorsOverrideProperty2.ts, 0, 0))

x = 1;
>x : Symbol(Base.x, Decl(accessorsOverrideProperty2.ts, 0, 12))
}

class Derived extends Base {
>Derived : Symbol(Derived, Decl(accessorsOverrideProperty2.ts, 2, 1))
>Base : Symbol(Base, Decl(accessorsOverrideProperty2.ts, 0, 0))

get x() { return 2; } // should be an error
>x : Symbol(Derived.x, Decl(accessorsOverrideProperty2.ts, 4, 28), Decl(accessorsOverrideProperty2.ts, 5, 23))

set x(value) { console.log(`x was set to ${value}`); }
>x : Symbol(Derived.x, Decl(accessorsOverrideProperty2.ts, 4, 28), Decl(accessorsOverrideProperty2.ts, 5, 23))
>value : Symbol(value, Decl(accessorsOverrideProperty2.ts, 6, 8))
>console.log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))
>console : Symbol(console, Decl(lib.dom.d.ts, --, --))
>log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))
>value : Symbol(value, Decl(accessorsOverrideProperty2.ts, 6, 8))
}

const obj = new Derived(); // nothing printed
>obj : Symbol(obj, Decl(accessorsOverrideProperty2.ts, 9, 5))
>Derived : Symbol(Derived, Decl(accessorsOverrideProperty2.ts, 2, 1))

console.log(obj.x); // 1
>console.log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))
>console : Symbol(console, Decl(lib.dom.d.ts, --, --))
>log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))
>obj.x : Symbol(Derived.x, Decl(accessorsOverrideProperty2.ts, 4, 28), Decl(accessorsOverrideProperty2.ts, 5, 23))
>obj : Symbol(obj, Decl(accessorsOverrideProperty2.ts, 9, 5))
>x : Symbol(Derived.x, Decl(accessorsOverrideProperty2.ts, 4, 28), Decl(accessorsOverrideProperty2.ts, 5, 23))

42 changes: 42 additions & 0 deletions tests/baselines/reference/accessorsOverrideProperty2.types
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
=== tests/cases/conformance/classes/propertyMemberDeclarations/accessorsOverrideProperty2.ts ===
class Base {
>Base : Base

x = 1;
>x : number
>1 : 1
}

class Derived extends Base {
>Derived : Derived
>Base : Base

get x() { return 2; } // should be an error
>x : number
>2 : 2

set x(value) { console.log(`x was set to ${value}`); }
>x : number
>value : number
>console.log(`x was set to ${value}`) : void
>console.log : (message?: any, ...optionalParams: any[]) => void
>console : Console
>log : (message?: any, ...optionalParams: any[]) => void
>`x was set to ${value}` : string
>value : number
}

const obj = new Derived(); // nothing printed
>obj : Derived
>new Derived() : Derived
>Derived : typeof Derived

console.log(obj.x); // 1
>console.log(obj.x) : void
>console.log : (message?: any, ...optionalParams: any[]) => void
>console : Console
>log : (message?: any, ...optionalParams: any[]) => void
>obj.x : number
>obj : Derived
>x : number

15 changes: 15 additions & 0 deletions tests/baselines/reference/accessorsOverrideProperty3.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
tests/ca 29C5 ses/conformance/classes/propertyMemberDeclarations/accessorsOverrideProperty3.ts(6,9): error TS2611: Class 'Animal' defines instance member property 'sound', but extended class 'Lion' defines it as instance member accessor.


==== tests/cases/conformance/classes/propertyMemberDeclarations/accessorsOverrideProperty3.ts (1 errors) ====
declare class Animal {
sound: string
}
class Lion extends Animal {
_sound = 'grrr'
get sound() { return this._sound } // error here
~~~~~
!!! error TS2611: Class 'Animal' defines instance member property 'sound', but extended class 'Lion' defines it as instance member accessor.
set sound(val) { this._sound = val }
}

916B 20 changes: 20 additions & 0 deletions tests/baselines/reference/accessorsOverrideProperty3.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
//// [accessorsOverrideProperty3.ts]
declare class Animal {
sound: string
}
class Lion extends Animal {
_sound = 'grrr'
get sound() { return this._sound } // error here
set sound(val) { this._sound = val }
}


//// [accessorsOverrideProperty3.js]
class Lion extends Animal {
constructor() {
super(...arguments);
this._sound = 'grrr';
}
get sound() { return this._sound; } // error here
set sound(val) { this._sound = val; }
}
Loading
0