From b5dbd6db4844c34f99d40ebf718f72310239caff Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Mon, 27 Jan 2020 14:50:28 -0800 Subject: [PATCH 1/2] Fix use-before-def errors for ESNext property declarations Fixes #36441 Fixes #36442 --- src/compiler/checker.ts | 28 ++++++++++----- ...ertyToPropertyDeclarationESNext.errors.txt | 12 ++++++- ...eterPropertyToPropertyDeclarationESNext.js | 23 +++++++++++- ...ropertyToPropertyDeclarationESNext.symbols | 34 ++++++++++++++++-- ...rPropertyToPropertyDeclarationESNext.types | 35 ++++++++++++++++++- ...eterPropertyToPropertyDeclarationESNext.ts | 12 ++++++- 6 files changed, 130 insertions(+), 14 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 3499cedab45dc..72c2cced49fd0 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -1326,12 +1326,14 @@ namespace ts { } else if (isPropertyDeclaration(declaration)) { // still might be illegal if a self-referencing property initializer (eg private x = this.x) - return !isPropertyImmediatelyReferencedWithinDeclaration(declaration, usage); + return !isPropertyImmediatelyReferencedWithinDeclaration(declaration, usage, /*stopAtAllPropertyDeclarations*/ false); } else if (isParameterPropertyDeclaration(declaration, declaration.parent)) { + const container = getEnclosingBlockScopeContainer(declaration.parent); // foo = this.bar is illegal in esnext+useDefineForClassFields when bar is a parameter property return !(compilerOptions.target === ScriptTarget.ESNext && !!compilerOptions.useDefineForClassFields - && isUsedInFunctionOrInstanceProperty(usage, declaration)); + && getContainingClass(declaration) === getContainingClass(usage) + && isUsedInFunctionOrInstanceProperty(usage, declaration, container)); } return true; } @@ -1357,10 +1359,19 @@ namespace ts { } const container = getEnclosingBlockScopeContainer(declaration); - return !!(usage.flags & NodeFlags.JSDoc) - || isInTypeQuery(usage) - || isUsedInFunctionOrInstanceProperty(usage, declaration, container) - && !(compilerOptions.target === ScriptTarget.ESNext && !!compilerOptions.useDefineForClassFields); + if (!!(usage.flags & NodeFlags.JSDoc) || isInTypeQuery(usage)) { + return true; + } + if (isUsedInFunctionOrInstanceProperty(usage, declaration, container)) { + if (compilerOptions.target === ScriptTarget.ESNext && !!compilerOptions.useDefineForClassFields) { + return (isPropertyDeclaration(declaration) || isParameterPropertyDeclaration(declaration, declaration.parent)) && + !isPropertyImmediatelyReferencedWithinDeclaration(declaration, usage, /*stopAtAllPropertyDeclarations*/ true); + } + else { + return true; + } + } + return false; function isImmediatelyUsedInInitializerOfBlockScopedVariable(declaration: VariableDeclaration, usage: Node): boolean { const container = getEnclosingBlockScopeContainer(declaration); @@ -1412,7 +1423,7 @@ namespace ts { }); } - function isPropertyImmediatelyReferencedWithinDeclaration(declaration: PropertyDeclaration, usage: Node) { + function isPropertyImmediatelyReferencedWithinDeclaration(declaration: PropertyDeclaration | ParameterPropertyDeclaration, usage: Node, stopAtAllPropertyDeclarations: boolean) { // always legal if usage is after declaration if (usage.end > declaration.end) { return false; @@ -1427,8 +1438,9 @@ namespace ts { switch (node.kind) { case SyntaxKind.ArrowFunction: - case SyntaxKind.PropertyDeclaration: return true; + case SyntaxKind.PropertyDeclaration: + return stopAtAllPropertyDeclarations ? "quit": true; case SyntaxKind.Block: switch (node.parent.kind) { case SyntaxKind.GetAccessor: diff --git a/tests/baselines/reference/assignParameterPropertyToPropertyDeclarationESNext.errors.txt b/tests/baselines/reference/assignParameterPropertyToPropertyDeclarationESNext.errors.txt index e3eea9a6cc6c1..fc71f647423a3 100644 --- a/tests/baselines/reference/assignParameterPropertyToPropertyDeclarationESNext.errors.txt +++ b/tests/baselines/reference/assignParameterPropertyToPropertyDeclarationESNext.errors.txt @@ -18,7 +18,7 @@ tests/cases/conformance/classes/propertyMemberDeclarations/assignParameterProper m1() { this.foo // ok } - constructor(private foo: string) {} + constructor(public foo: string) {} quim = this.baz // should error ~~~ !!! error TS2729: Property 'baz' is used before its initialization. @@ -32,4 +32,14 @@ tests/cases/conformance/classes/propertyMemberDeclarations/assignParameterProper this.foo // ok } } + + class D extends C { + quill = this.foo // ok + } + + class E { + bar = () => this.foo1 + this.foo2; // both ok + foo1 = ''; + constructor(public foo2: string) {} + } \ No newline at end of file diff --git a/tests/baselines/reference/assignParameterPropertyToPropertyDeclarationESNext.js b/tests/baselines/reference/assignParameterPropertyToPropertyDeclarationESNext.js index de93aa1e43e8d..1685b349680be 100644 --- a/tests/baselines/reference/assignParameterPropertyToPropertyDeclarationESNext.js +++ b/tests/baselines/reference/assignParameterPropertyToPropertyDeclarationESNext.js @@ -6,7 +6,7 @@ class C { m1() { this.foo // ok } - constructor(private foo: string) {} + constructor(public foo: string) {} quim = this.baz // should error baz = this.foo; // should error quid = this.baz // ok @@ -14,6 +14,16 @@ class C { this.foo // ok } } + +class D extends C { + quill = this.foo // ok +} + +class E { + bar = () => this.foo1 + this.foo2; // both ok + foo1 = ''; + constructor(public foo2: string) {} +} //// [assignParameterPropertyToPropertyDeclarationESNext.js] @@ -35,3 +45,14 @@ class C { this.foo; // ok } } +class D extends C { + quill = this.foo; // ok +} +class E { + foo2; + bar = () => this.foo1 + this.foo2; // both ok + foo1 = ''; + constructor(foo2) { + this.foo2 = foo2; + } +} diff --git a/tests/baselines/reference/assignParameterPropertyToPropertyDeclarationESNext.symbols b/tests/baselines/reference/assignParameterPropertyToPropertyDeclarationESNext.symbols index ffd908df671be..652c09d7c0a23 100644 --- a/tests/baselines/reference/assignParameterPropertyToPropertyDeclarationESNext.symbols +++ b/tests/baselines/reference/assignParameterPropertyToPropertyDeclarationESNext.symbols @@ -28,11 +28,11 @@ class C { >this : Symbol(C, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 0, 0)) >foo : Symbol(C.foo, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 7, 16)) } - constructor(private foo: string) {} + constructor(public foo: string) {} >foo : Symbol(C.foo, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 7, 16)) quim = this.baz // should error ->quim : Symbol(C.quim, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 7, 39)) +>quim : Symbol(C.quim, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 7, 38)) >this.baz : Symbol(C.baz, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 8, 19)) >this : Symbol(C, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 0, 0)) >baz : Symbol(C.baz, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 8, 19)) @@ -59,3 +59,33 @@ class C { } } +class D extends C { +>D : Symbol(D, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 14, 1)) +>C : Symbol(C, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 0, 0)) + + quill = this.foo // ok +>quill : Symbol(D.quill, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 16, 19)) +>this.foo : Symbol(C.foo, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 7, 16)) +>this : Symbol(D, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 14, 1)) +>foo : Symbol(C.foo, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 7, 16)) +} + +class E { +>E : Symbol(E, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 18, 1)) + + bar = () => this.foo1 + this.foo2; // both ok +>bar : Symbol(E.bar, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 20, 9)) +>this.foo1 : Symbol(E.foo1, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 21, 38)) +>this : Symbol(E, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 18, 1)) +>foo1 : Symbol(E.foo1, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 21, 38)) +>this.foo2 : Symbol(E.foo2, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 23, 16)) +>this : Symbol(E, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 18, 1)) +>foo2 : Symbol(E.foo2, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 23, 16)) + + foo1 = ''; +>foo1 : Symbol(E.foo1, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 21, 38)) + + constructor(public foo2: string) {} +>foo2 : Symbol(E.foo2, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 23, 16)) +} + diff --git a/tests/baselines/reference/assignParameterPropertyToPropertyDeclarationESNext.types b/tests/baselines/reference/assignParameterPropertyToPropertyDeclarationESNext.types index 8379bda4ef700..c13f5ff64a0f6 100644 --- a/tests/baselines/reference/assignParameterPropertyToPropertyDeclarationESNext.types +++ b/tests/baselines/reference/assignParameterPropertyToPropertyDeclarationESNext.types @@ -28,7 +28,7 @@ class C { >this : this >foo : string } - constructor(private foo: string) {} + constructor(public foo: string) {} >foo : string quim = this.baz // should error @@ -59,3 +59,36 @@ class C { } } +class D extends C { +>D : D +>C : C + + quill = this.foo // ok +>quill : string +>this.foo : string +>this : this +>foo : string +} + +class E { +>E : E + + bar = () => this.foo1 + this.foo2; // both ok +>bar : () => string +>() => this.foo1 + this.foo2 : () => string +>this.foo1 + this.foo2 : string +>this.foo1 : string +>this : this +>foo1 : string +>this.foo2 : string +>this : this +>foo2 : string + + foo1 = ''; +>foo1 : string +>'' : "" + + constructor(public foo2: string) {} +>foo2 : string +} + diff --git a/tests/cases/conformance/classes/propertyMemberDeclarations/assignParameterPropertyToPropertyDeclarationESNext.ts b/tests/cases/conformance/classes/propertyMemberDeclarations/assignParameterPropertyToPropertyDeclarationESNext.ts index 8cb56c0cd9a35..ae538a22bc417 100644 --- a/tests/cases/conformance/classes/propertyMemberDeclarations/assignParameterPropertyToPropertyDeclarationESNext.ts +++ b/tests/cases/conformance/classes/propertyMemberDeclarations/assignParameterPropertyToPropertyDeclarationESNext.ts @@ -7,7 +7,7 @@ class C { m1() { this.foo // ok } - constructor(private foo: string) {} + constructor(public foo: string) {} quim = this.baz // should error baz = this.foo; // should error quid = this.baz // ok @@ -15,3 +15,13 @@ class C { this.foo // ok } } + +class D extends C { + quill = this.foo // ok +} + +class E { + bar = () => this.foo1 + this.foo2; // both ok + foo1 = ''; + constructor(public foo2: string) {} +} From ea587c4931aadd8eb9629dc2ae2582d472f72a5c Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Thu, 30 Jan 2020 09:48:27 -0800 Subject: [PATCH 2/2] Handle property declarations in nested classes --- src/compiler/checker.ts | 13 ++++--- ...ertyToPropertyDeclarationESNext.errors.txt | 13 +++++++ ...eterPropertyToPropertyDeclarationESNext.js | 28 +++++++++++++++ ...ropertyToPropertyDeclarationESNext.symbols | 33 +++++++++++++++++ ...rPropertyToPropertyDeclarationESNext.types | 36 +++++++++++++++++++ ...eterPropertyToPropertyDeclarationESNext.ts | 13 +++++++ 6 files changed, 132 insertions(+), 4 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 72c2cced49fd0..ad6199fa983f9 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -1326,7 +1326,7 @@ namespace ts { } else if (isPropertyDeclaration(declaration)) { // still might be illegal if a self-referencing property initializer (eg private x = this.x) - return !isPropertyImmediatelyReferencedWithinDeclaration(declaration, usage, /*stopAtAllPropertyDeclarations*/ false); + return !isPropertyImmediatelyReferencedWithinDeclaration(declaration, usage, /*stopAtAnyPropertyDeclaration*/ false); } else if (isParameterPropertyDeclaration(declaration, declaration.parent)) { const container = getEnclosingBlockScopeContainer(declaration.parent); @@ -1365,7 +1365,7 @@ namespace ts { if (isUsedInFunctionOrInstanceProperty(usage, declaration, container)) { if (compilerOptions.target === ScriptTarget.ESNext && !!compilerOptions.useDefineForClassFields) { return (isPropertyDeclaration(declaration) || isParameterPropertyDeclaration(declaration, declaration.parent)) && - !isPropertyImmediatelyReferencedWithinDeclaration(declaration, usage, /*stopAtAllPropertyDeclarations*/ true); + !isPropertyImmediatelyReferencedWithinDeclaration(declaration, usage, /*stopAtAnyPropertyDeclaration*/ true); } else { return true; @@ -1423,7 +1423,8 @@ namespace ts { }); } - function isPropertyImmediatelyReferencedWithinDeclaration(declaration: PropertyDeclaration | ParameterPropertyDeclaration, usage: Node, stopAtAllPropertyDeclarations: boolean) { + /** stopAtAnyPropertyDeclaration is used for detecting ES-standard class field use-before-def errors */ + function isPropertyImmediatelyReferencedWithinDeclaration(declaration: PropertyDeclaration | ParameterPropertyDeclaration, usage: Node, stopAtAnyPropertyDeclaration: boolean) { // always legal if usage is after declaration if (usage.end > declaration.end) { return false; @@ -1440,7 +1441,11 @@ namespace ts { case SyntaxKind.ArrowFunction: return true; case SyntaxKind.PropertyDeclaration: - return stopAtAllPropertyDeclarations ? "quit": true; + // even when stopping at any property declaration, they need to come from the same class + return stopAtAnyPropertyDeclaration && + (isPropertyDeclaration(declaration) && node.parent === declaration.parent + || isParameterPropertyDeclaration(declaration, declaration.parent) && node.parent === declaration.parent.parent) + ? "quit": true; case SyntaxKind.Block: switch (node.parent.kind) { case SyntaxKind.GetAccessor: diff --git a/tests/baselines/reference/assignParameterPropertyToPropertyDeclarationESNext.errors.txt b/tests/baselines/reference/assignParameterPropertyToPropertyDeclarationESNext.errors.txt index fc71f647423a3..2010e92a2d8eb 100644 --- a/tests/baselines/reference/assignParameterPropertyToPropertyDeclarationESNext.errors.txt +++ b/tests/baselines/reference/assignParameterPropertyToPropertyDeclarationESNext.errors.txt @@ -42,4 +42,17 @@ tests/cases/conformance/classes/propertyMemberDeclarations/assignParameterProper foo1 = ''; constructor(public foo2: string) {} } + + class F { + Inner = class extends F { + p2 = this.p1 + } + p1 = 0 + } + class G { + Inner = class extends G { + p2 = this.p1 + } + constructor(public p1: number) {} + } \ No newline at end of file diff --git a/tests/baselines/reference/assignParameterPropertyToPropertyDeclarationESNext.js b/tests/baselines/reference/assignParameterPropertyToPropertyDeclarationESNext.js index 1685b349680be..f71361eb570f5 100644 --- a/tests/baselines/reference/assignParameterPropertyToPropertyDeclarationESNext.js +++ b/tests/baselines/reference/assignParameterPropertyToPropertyDeclarationESNext.js @@ -24,6 +24,19 @@ class E { foo1 = ''; constructor(public foo2: string) {} } + +class F { + Inner = class extends F { + p2 = this.p1 + } + p1 = 0 +} +class G { + Inner = class extends G { + p2 = this.p1 + } + constructor(public p1: number) {} +} //// [assignParameterPropertyToPropertyDeclarationESNext.js] @@ -56,3 +69,18 @@ class E { this.foo2 = foo2; } } +class F { + Inner = class extends F { + p2 = this.p1; + }; + p1 = 0; +} +class G { + p1; + Inner = class extends G { + p2 = this.p1; + }; + constructor(p1) { + this.p1 = p1; + } +} diff --git a/tests/baselines/reference/assignParameterPropertyToPropertyDeclarationESNext.symbols b/tests/baselines/reference/assignParameterPropertyToPropertyDeclarationESNext.symbols index 652c09d7c0a23..5fd56137bb83e 100644 --- a/tests/baselines/reference/assignParameterPropertyToPropertyDeclarationESNext.symbols +++ b/tests/baselines/reference/assignParameterPropertyToPropertyDeclarationESNext.symbols @@ -89,3 +89,36 @@ class E { >foo2 : Symbol(E.foo2, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 23, 16)) } +class F { +>F : Symbol(F, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 24, 1)) + + Inner = class extends F { +>Inner : Symbol(F.Inner, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 26, 9)) +>F : Symbol(F, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 24, 1)) + + p2 = this.p1 +>p2 : Symbol((Anonymous class).p2, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 27, 29)) +>this.p1 : Symbol(F.p1, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 29, 5)) +>this : Symbol((Anonymous class), Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 27, 11)) +>p1 : Symbol(F.p1, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 29, 5)) + } + p1 = 0 +>p1 : Symbol(F.p1, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 29, 5)) +} +class G { +>G : Symbol(G, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 31, 1)) + + Inner = class extends G { +>Inner : Symbol(G.Inner, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 32, 9)) +>G : Symbol(G, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 31, 1)) + + p2 = this.p1 +>p2 : Symbol((Anonymous class).p2, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 33, 29)) +>this.p1 : Symbol(G.p1, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 36, 16)) +>this : Symbol((Anonymous class), Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 33, 11)) +>p1 : Symbol(G.p1, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 36, 16)) + } + constructor(public p1: number) {} +>p1 : Symbol(G.p1, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 36, 16)) +} + diff --git a/tests/baselines/reference/assignParameterPropertyToPropertyDeclarationESNext.types b/tests/baselines/reference/assignParameterPropertyToPropertyDeclarationESNext.types index c13f5ff64a0f6..631b0dba5a4a8 100644 --- a/tests/baselines/reference/assignParameterPropertyToPropertyDeclarationESNext.types +++ b/tests/baselines/reference/assignParameterPropertyToPropertyDeclarationESNext.types @@ -92,3 +92,39 @@ class E { >foo2 : string } +class F { +>F : F + + Inner = class extends F { +>Inner : typeof (Anonymous class) +>class extends F { p2 = this.p1 } : typeof (Anonymous class) +>F : F + + p2 = this.p1 +>p2 : number +>this.p1 : number +>this : this +>p1 : number + } + p1 = 0 +>p1 : number +>0 : 0 +} +class G { +>G : G + + Inner = class extends G { +>Inner : typeof (Anonymous class) +>class extends G { p2 = this.p1 } : typeof (Anonymous class) +>G : G + + p2 = this.p1 +>p2 : number +>this.p1 : number +>this : this +>p1 : number + } + constructor(public p1: number) {} +>p1 : number +} + diff --git a/tests/cases/conformance/classes/propertyMemberDeclarations/assignParameterPropertyToPropertyDeclarationESNext.ts b/tests/cases/conformance/classes/propertyMemberDeclarations/assignParameterPropertyToPropertyDeclarationESNext.ts index ae538a22bc417..f4475f09ddf24 100644 --- a/tests/cases/conformance/classes/propertyMemberDeclarations/assignParameterPropertyToPropertyDeclarationESNext.ts +++ b/tests/cases/conformance/classes/propertyMemberDeclarations/assignParameterPropertyToPropertyDeclarationESNext.ts @@ -25,3 +25,16 @@ class E { foo1 = ''; constructor(public foo2: string) {} } + +class F { + Inner = class extends F { + p2 = this.p1 + } + p1 = 0 +} +class G { + Inner = class extends G { + p2 = this.p1 + } + constructor(public p1: number) {} +}