From 368db997ed52b0000920ca8d33ebfe84935e2eef Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Fri, 24 Jan 2020 14:53:28 -0800 Subject: [PATCH] ESNext+[[Define]]: reference to param props illegal (#36425) * ESNext+[[Define]]: reference to param props illegal When target: "esnext" and useDefineForClassFields: true, property declaration initialisers should not be able to reference parameter properties; class fields are initialised before the constructor runs, but parameter properties are not: ```ts class C { foo = this.bar constructor(public bar: string) { } } ``` emits code that looks like this: ```js class C { bar foo = this.bar constructor(bar) { this.bar = bar } } new C('x').foo.length // crashes; foo is undefined ``` This PR adds an error on foo's declaration with ESNext+[[Define]]. * improve test --- src/compiler/checker.ts | 11 +++- ...ertyToPropertyDeclarationESNext.errors.txt | 35 +++++++++++ ...eterPropertyToPropertyDeclarationESNext.js | 37 +++++++++++ ...ropertyToPropertyDeclarationESNext.symbols | 61 +++++++++++++++++++ ...rPropertyToPropertyDeclarationESNext.types | 61 +++++++++++++++++++ .../defineProperty(target=esnext).errors.txt | 43 +++++++++++++ ...eterPropertyToPropertyDeclarationESNext.ts | 17 ++++++ 7 files changed, 264 insertions(+), 1 deletion(-) create mode 100644 tests/baselines/reference/assignParameterPropertyToPropertyDeclarationESNext.errors.txt create mode 100644 tests/baselines/reference/assignParameterPropertyToPropertyDeclarationESNext.js create mode 100644 tests/baselines/reference/assignParameterPropertyToPropertyDeclarationESNext.symbols create mode 100644 tests/baselines/reference/assignParameterPropertyToPropertyDeclarationESNext.types create mode 100644 tests/baselines/reference/defineProperty(target=esnext).errors.txt create mode 100644 tests/cases/conformance/classes/propertyMemberDeclarations/assignParameterPropertyToPropertyDeclarationESNext.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index b719fcb9b6..6e70c19e00 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -1328,6 +1328,11 @@ namespace ts { // still might be illegal if a self-referencing property initializer (eg private x = this.x) return !isPropertyImmediatelyReferencedWithinDeclaration(declaration, usage); } + else if (isParameterPropertyDeclaration(declaration, 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)); + } return true; } @@ -1336,6 +1341,7 @@ namespace ts { // 1. inside an export specifier // 2. inside a function // 3. inside an instance property initializer, a reference to a non-instance property + // (except when target: "esnext" and useDefineForClassFields: true and the reference is to a parameter property) // 4. inside a static property initializer, a reference to a static method in the same class // 5. inside a TS export= declaration (since we will move the export statement during emit to avoid TDZ) // or if usage is in a type context: @@ -1351,7 +1357,10 @@ namespace ts { } const container = getEnclosingBlockScopeContainer(declaration); - return !!(usage.flags & NodeFlags.JSDoc) || isInTypeQuery(usage) || isUsedInFunctionOrInstanceProperty(usage, declaration, container); + return !!(usage.flags & NodeFlags.JSDoc) + || isInTypeQuery(usage) + || isUsedInFunctionOrInstanceProperty(usage, declaration, container) + && !(compilerOptions.target === ScriptTarget.ESNext && !!compilerOptions.useDefineForClassFields); function isImmediatelyUsedInInitializerOfBlockScopedVariable(declaration: VariableDeclaration, usage: Node): boolean { const container = getEnclosingBlockScopeContainer(declaration); diff --git a/tests/baselines/reference/assignParameterPropertyToPropertyDeclarationESNext.errors.txt b/tests/baselines/reference/assignParameterPropertyToPropertyDeclarationESNext.errors.txt new file mode 100644 index 0000000000..e3eea9a6cc --- /dev/null +++ b/tests/baselines/reference/assignParameterPropertyToPropertyDeclarationESNext.errors.txt @@ -0,0 +1,35 @@ +tests/cases/conformance/classes/propertyMemberDeclarations/assignParameterPropertyToPropertyDeclarationESNext.ts(2,16): error TS2729: Property 'bar' is used before its initialization. +tests/cases/conformance/classes/propertyMemberDeclarations/assignParameterPropertyToPropertyDeclarationESNext.ts(3,16): error TS2729: Property 'foo' is used before its initialization. +tests/cases/conformance/classes/propertyMemberDeclarations/assignParameterPropertyToPropertyDeclarationESNext.ts(9,17): error TS2729: Property 'baz' is used before its initialization. +tests/cases/conformance/classes/propertyMemberDeclarations/assignParameterPropertyToPropertyDeclarationESNext.ts(10,16): error TS2729: Property 'foo' is used before its initialization. + + +==== tests/cases/conformance/classes/propertyMemberDeclarations/assignParameterPropertyToPropertyDeclarationESNext.ts (4 errors) ==== + class C { + qux = this.bar // should error + ~~~ +!!! error TS2729: Property 'bar' is used before its initialization. +!!! related TS2728 tests/cases/conformance/classes/propertyMemberDeclarations/assignParameterPropertyToPropertyDeclarationESNext.ts:3:5: 'bar' is declared here. + bar = this.foo // should error + ~~~ +!!! error TS2729: Property 'foo' is used before its initialization. +!!! related TS2728 tests/cases/conformance/classes/propertyMemberDeclarations/assignParameterPropertyToPropertyDeclarationESNext.ts:8:17: 'foo' is declared here. + quiz = this.bar // ok + m1() { + this.foo // ok + } + constructor(private foo: string) {} + quim = this.baz // should error + ~~~ +!!! error TS2729: Property 'baz' is used before its initialization. +!!! related TS2728 tests/cases/conformance/classes/propertyMemberDeclarations/assignParameterPropertyToPropertyDeclarationESNext.ts:10:5: 'baz' is declared here. + baz = this.foo; // should error + ~~~ +!!! error TS2729: Property 'foo' is used before its initialization. +!!! related TS2728 tests/cases/conformance/classes/propertyMemberDeclarations/assignParameterPropertyToPropertyDeclarationESNext.ts:8:17: 'foo' is declared here. + quid = this.baz // ok + m2() { + this.foo // ok + } + } + \ No newline at end of file diff --git a/tests/baselines/reference/assignParameterPropertyToPropertyDeclarationESNext.js b/tests/baselines/reference/assignParameterPropertyToPropertyDeclarationESNext.js new file mode 100644 index 0000000000..de93aa1e43 --- /dev/null +++ b/tests/baselines/reference/assignParameterPropertyToPropertyDeclarationESNext.js @@ -0,0 +1,37 @@ +//// [assignParameterPropertyToPropertyDeclarationESNext.ts] +class C { + qux = this.bar // should error + bar = this.foo // should error + quiz = this.bar // ok + m1() { + this.foo // ok + } + constructor(private foo: string) {} + quim = this.baz // should error + baz = this.foo; // should error + quid = this.baz // ok + m2() { + this.foo // ok + } +} + + +//// [assignParameterPropertyToPropertyDeclarationESNext.js] +class C { + foo; + qux = this.bar; // should error + bar = this.foo; // should error + quiz = this.bar; // ok + m1() { + this.foo; // ok + } + constructor(foo) { + this.foo = foo; + } + quim = this.baz; // should error + baz = this.foo; // should error + quid = this.baz; // ok + m2() { + this.foo; // ok + } +} diff --git a/tests/baselines/reference/assignParameterPropertyToPropertyDeclarationESNext.symbols b/tests/baselines/reference/assignParameterPropertyToPropertyDeclarationESNext.symbols new file mode 100644 index 0000000000..ffd908df67 --- /dev/null +++ b/tests/baselines/reference/assignParameterPropertyToPropertyDeclarationESNext.symbols @@ -0,0 +1,61 @@ +=== tests/cases/conformance/classes/propertyMemberDeclarations/assignParameterPropertyToPropertyDeclarationESNext.ts === +class C { +>C : Symbol(C, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 0, 0)) + + qux = this.bar // should error +>qux : Symbol(C.qux, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 0, 9)) +>this.bar : Symbol(C.bar, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 1, 18)) +>this : Symbol(C, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 0, 0)) +>bar : Symbol(C.bar, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 1, 18)) + + bar = this.foo // should error +>bar : Symbol(C.bar, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 1, 18)) +>this.foo : Symbol(C.foo, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 7, 16)) +>this : Symbol(C, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 0, 0)) +>foo : Symbol(C.foo, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 7, 16)) + + quiz = this.bar // ok +>quiz : Symbol(C.quiz, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 2, 18)) +>this.bar : Symbol(C.bar, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 1, 18)) +>this : Symbol(C, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 0, 0)) +>bar : Symbol(C.bar, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 1, 18)) + + m1() { +>m1 : Symbol(C.m1, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 3, 19)) + + this.foo // ok +>this.foo : Symbol(C.foo, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 7, 16)) +>this : Symbol(C, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 0, 0)) +>foo : Symbol(C.foo, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 7, 16)) + } + constructor(private 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)) +>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)) + + baz = this.foo; // should error +>baz : Symbol(C.baz, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 8, 19)) +>this.foo : Symbol(C.foo, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 7, 16)) +>this : Symbol(C, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 0, 0)) +>foo : Symbol(C.foo, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 7, 16)) + + quid = this.baz // ok +>quid : Symbol(C.quid, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 9, 19)) +>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)) + + m2() { +>m2 : Symbol(C.m2, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 10, 19)) + + this.foo // ok +>this.foo : Symbol(C.foo, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 7, 16)) +>this : Symbol(C, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 0, 0)) +>foo : Symbol(C.foo, Decl(assignParameterPropertyToPropertyDeclarationESNext.ts, 7, 16)) + } +} + diff --git a/tests/baselines/reference/assignParameterPropertyToPropertyDeclarationESNext.types b/tests/baselines/reference/assignParameterPropertyToPropertyDeclarationESNext.types new file mode 100644 index 0000000000..8379bda4ef --- /dev/null +++ b/tests/baselines/reference/assignParameterPropertyToPropertyDeclarationESNext.types @@ -0,0 +1,61 @@ +=== tests/cases/conformance/classes/propertyMemberDeclarations/assignParameterPropertyToPropertyDeclarationESNext.ts === +class C { +>C : C + + qux = this.bar // should error +>qux : string +>this.bar : string +>this : this +>bar : string + + bar = this.foo // should error +>bar : string +>this.foo : string +>this : this +>foo : string + + quiz = this.bar // ok +>quiz : string +>this.bar : string +>this : this +>bar : string + + m1() { +>m1 : () => void + + this.foo // ok +>this.foo : string +>this : this +>foo : string + } + constructor(private foo: string) {} +>foo : string + + quim = this.baz // should error +>quim : string +>this.baz : string +>this : this +>baz : string + + baz = this.foo; // should error +>baz : string +>this.foo : string +>this : this +>foo : string + + quid = this.baz // ok +>quid : string +>this.baz : string +>this : this +>baz : string + + m2() { +>m2 : () => void + + this.foo // ok +>this.foo : string +>this : this +>foo : string + } +} + diff --git a/tests/baselines/reference/defineProperty(target=esnext).errors.txt b/tests/baselines/reference/defineProperty(target=esnext).errors.txt new file mode 100644 index 0000000000..2a4e17a1b5 --- /dev/null +++ b/tests/baselines/reference/defineProperty(target=esnext).errors.txt @@ -0,0 +1,43 @@ +tests/cases/conformance/classes/propertyMemberDeclarations/defineProperty.ts(3,14): error TS2729: Property 'y' is used before its initialization. +tests/cases/conformance/classes/propertyMemberDeclarations/defineProperty.ts(10,14): error TS2729: Property 'y' is used before its initialization. +tests/cases/conformance/classes/propertyMemberDeclarations/defineProperty.ts(18,14): error TS2729: Property 'ka' is used before its initialization. +tests/cases/conformance/classes/propertyMemberDeclarations/defineProperty.ts(22,15): error TS2729: Property 'ka' is used before its initialization. + + +==== tests/cases/conformance/classes/propertyMemberDeclarations/defineProperty.ts (4 errors) ==== + var x: "p" = "p" + class A { + a = this.y + ~ +!!! error TS2729: Property 'y' is used before its initialization. +!!! related TS2728 tests/cases/conformance/classes/propertyMemberDeclarations/defineProperty.ts:9:17: 'y' is declared here. + b + public c; + ["computed"] = 13 + ;[x] = 14 + m() { } + constructor(public readonly y: number) { } + z = this.y + ~ +!!! error TS2729: Property 'y' is used before its initialization. +!!! related TS2728 tests/cases/conformance/classes/propertyMemberDeclarations/defineProperty.ts:9:17: 'y' is declared here. + declare notEmitted; + } + class B { + public a; + } + class C extends B { + declare public a; + z = this.ka + ~~ +!!! error TS2729: Property 'ka' is used before its initialization. +!!! related TS2728 tests/cases/conformance/classes/propertyMemberDeclarations/defineProperty.ts:19:17: 'ka' is declared here. + constructor(public ka: number) { + super() + } + ki = this.ka + ~~ +!!! error TS2729: Property 'ka' is used before its initialization. +!!! related TS2728 tests/cases/conformance/classes/propertyMemberDeclarations/defineProperty.ts:19:17: 'ka' is declared here. + } + \ No newline at end of file diff --git a/tests/cases/conformance/classes/propertyMemberDeclarations/assignParameterPropertyToPropertyDeclarationESNext.ts b/tests/cases/conformance/classes/propertyMemberDeclarations/assignParameterPropertyToPropertyDeclarationESNext.ts new file mode 100644 index 0000000000..8cb56c0cd9 --- /dev/null +++ b/tests/cases/conformance/classes/propertyMemberDeclarations/assignParameterPropertyToPropertyDeclarationESNext.ts @@ -0,0 +1,17 @@ +// @useDefineForClassFields: true +// @target: esnext +class C { + qux = this.bar // should error + bar = this.foo // should error + quiz = this.bar // ok + m1() { + this.foo // ok + } + constructor(private foo: string) {} + quim = this.baz // should error + baz = this.foo; // should error + quid = this.baz // ok + m2() { + this.foo // ok + } +}