From 29f6c487705955787adb21aabce0ceb0ff9b70e2 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Fri, 24 Jan 2020 08:43:53 -0800 Subject: [PATCH 1/3] Class fields w/esnext+[[Define]]:no shadow error With useDefineForClassFields: true and ESNext target, initializer expressions for property declarations are evaluated in the scope of the class body and are permitted to reference parameters or local variables of the constructor. This is different from classic Typescript behaviour, with useDefineForClassFields: false. There, initialisers of property declarations are evaluated in the scope of the constructor body. Note that when class fields are accepted in the ECMAScript standard, the target will become that year's ES20xx --- src/compiler/checker.ts | 2 +- ...constructorParameterShadowsOuterScopes2.js | 51 +++++++++++++++++++ ...ructorParameterShadowsOuterScopes2.symbols | 45 ++++++++++++++++ ...structorParameterShadowsOuterScopes2.types | 48 +++++++++++++++++ ...constructorParameterShadowsOuterScopes2.ts | 29 +++++++++++ 5 files changed, 174 insertions(+), 1 deletion(-) create mode 100644 tests/baselines/reference/constructorParameterShadowsOuterScopes2.js create mode 100644 tests/baselines/reference/constructorParameterShadowsOuterScopes2.symbols create mode 100644 tests/baselines/reference/constructorParameterShadowsOuterScopes2.types create mode 100644 tests/cases/conformance/classes/propertyMemberDeclarations/constructorParameterShadowsOuterScopes2.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 409d785a2274d..2cc9f08d8737e 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -1811,7 +1811,7 @@ namespace ts { // Perform extra checks only if error reporting was requested if (nameNotFoundMessage) { - if (propertyWithInvalidInitializer) { + if (propertyWithInvalidInitializer && !(compilerOptions.target === ScriptTarget.ESNext && compilerOptions.useDefineForClassFields)) { // We have a match, but the reference occurred within a property initializer and the identifier also binds // to a local variable in the constructor where the code will be emitted. const propertyName = (propertyWithInvalidInitializer).name; diff --git a/tests/baselines/reference/constructorParameterShadowsOuterScopes2.js b/tests/baselines/reference/constructorParameterShadowsOuterScopes2.js new file mode 100644 index 0000000000000..aa1096303d900 --- /dev/null +++ b/tests/baselines/reference/constructorParameterShadowsOuterScopes2.js @@ -0,0 +1,51 @@ +//// [constructorParameterShadowsOuterScopes2.ts] +// With useDefineForClassFields: true and ESNext target, initializer +// expressions for property declarations are evaluated in the scope of +// the class body and are permitted to reference parameters or local +// variables of the constructor. This is different from classic +// Typescript behaviour, with useDefineForClassFields: false. There, +// initialisers of property declarations are evaluated in the scope of +// the constructor body. + +// Note that when class fields are accepted in the ECMAScript +// standard, the target will become that year's ES20xx + +var x = 1; +class C { + b = x; // ok + constructor(x: string) { + } +} + +var y = 1; +class D { + b = y; // ok + constructor(x: string) { + var y = ""; + } +} + + +//// [constructorParameterShadowsOuterScopes2.js] +// With useDefineForClassFields: true and ESNext target, initializer +// expressions for property declarations are evaluated in the scope of +// the class body and are permitted to reference parameters or local +// variables of the constructor. This is different from classic +// Typescript behaviour, with useDefineForClassFields: false. There, +// initialisers of property declarations are evaluated in the scope of +// the constructor body. +// Note that when class fields are accepted in the ECMAScript +// standard, the target will become that year's ES20xx +var x = 1; +class C { + b = x; // ok + constructor(x) { + } +} +var y = 1; +class D { + b = y; // ok + constructor(x) { + var y = ""; + } +} diff --git a/tests/baselines/reference/constructorParameterShadowsOuterScopes2.symbols b/tests/baselines/reference/constructorParameterShadowsOuterScopes2.symbols new file mode 100644 index 0000000000000..4a24bb89f692a --- /dev/null +++ b/tests/baselines/reference/constructorParameterShadowsOuterScopes2.symbols @@ -0,0 +1,45 @@ +=== tests/cases/conformance/classes/propertyMemberDeclarations/constructorParameterShadowsOuterScopes2.ts === +// With useDefineForClassFields: true and ESNext target, initializer +// expressions for property declarations are evaluated in the scope of +// the class body and are permitted to reference parameters or local +// variables of the constructor. This is different from classic +// Typescript behaviour, with useDefineForClassFields: false. There, +// initialisers of property declarations are evaluated in the scope of +// the constructor body. + +// Note that when class fields are accepted in the ECMAScript +// standard, the target will become that year's ES20xx + +var x = 1; +>x : Symbol(x, Decl(constructorParameterShadowsOuterScopes2.ts, 11, 3)) + +class C { +>C : Symbol(C, Decl(constructorParameterShadowsOuterScopes2.ts, 11, 10)) + + b = x; // ok +>b : Symbol(C.b, Decl(constructorParameterShadowsOuterScopes2.ts, 12, 9)) +>x : Symbol(x, Decl(constructorParameterShadowsOuterScopes2.ts, 11, 3)) + + constructor(x: string) { +>x : Symbol(x, Decl(constructorParameterShadowsOuterScopes2.ts, 14, 16)) + } +} + +var y = 1; +>y : Symbol(y, Decl(constructorParameterShadowsOuterScopes2.ts, 18, 3)) + +class D { +>D : Symbol(D, Decl(constructorParameterShadowsOuterScopes2.ts, 18, 10)) + + b = y; // ok +>b : Symbol(D.b, Decl(constructorParameterShadowsOuterScopes2.ts, 19, 9)) +>y : Symbol(y, Decl(constructorParameterShadowsOuterScopes2.ts, 18, 3)) + + constructor(x: string) { +>x : Symbol(x, Decl(constructorParameterShadowsOuterScopes2.ts, 21, 16)) + + var y = ""; +>y : Symbol(y, Decl(constructorParameterShadowsOuterScopes2.ts, 22, 11)) + } +} + diff --git a/tests/baselines/reference/constructorParameterShadowsOuterScopes2.types b/tests/baselines/reference/constructorParameterShadowsOuterScopes2.types new file mode 100644 index 0000000000000..ffcb4a591d692 --- /dev/null +++ b/tests/baselines/reference/constructorParameterShadowsOuterScopes2.types @@ -0,0 +1,48 @@ +=== tests/cases/conformance/classes/propertyMemberDeclarations/constructorParameterShadowsOuterScopes2.ts === +// With useDefineForClassFields: true and ESNext target, initializer +// expressions for property declarations are evaluated in the scope of +// the class body and are permitted to reference parameters or local +// variables of the constructor. This is different from classic +// Typescript behaviour, with useDefineForClassFields: false. There, +// initialisers of property declarations are evaluated in the scope of +// the constructor body. + +// Note that when class fields are accepted in the ECMAScript +// standard, the target will become that year's ES20xx + +var x = 1; +>x : number +>1 : 1 + +class C { +>C : C + + b = x; // ok +>b : number +>x : number + + constructor(x: string) { +>x : string + } +} + +var y = 1; +>y : number +>1 : 1 + +class D { +>D : D + + b = y; // ok +>b : number +>y : number + + constructor(x: string) { +>x : string + + var y = ""; +>y : string +>"" : "" + } +} + diff --git a/tests/cases/conformance/classes/propertyMemberDeclarations/constructorParameterShadowsOuterScopes2.ts b/tests/cases/conformance/classes/propertyMemberDeclarations/constructorParameterShadowsOuterScopes2.ts new file mode 100644 index 0000000000000..72e16c916d654 --- /dev/null +++ b/tests/cases/conformance/classes/propertyMemberDeclarations/constructorParameterShadowsOuterScopes2.ts @@ -0,0 +1,29 @@ +// @target: esnext +// @useDefineForClassFields: true + + +// With useDefineForClassFields: true and ESNext target, initializer +// expressions for property declarations are evaluated in the scope of +// the class body and are permitted to reference parameters or local +// variables of the constructor. This is different from classic +// Typescript behaviour, with useDefineForClassFields: false. There, +// initialisers of property declarations are evaluated in the scope of +// the constructor body. + +// Note that when class fields are accepted in the ECMAScript +// standard, the target will become that year's ES20xx + +var x = 1; +class C { + b = x; // ok + constructor(x: string) { + } +} + +var y = 1; +class D { + b = y; // ok + constructor(x: string) { + var y = ""; + } +} From 70ecf92a40ce32e8e85a8cfd9efa5067ff09340d Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Wed, 29 Jan 2020 14:10:19 -0800 Subject: [PATCH 2/3] add negative test case --- ...torParameterShadowsOuterScopes2.errors.txt | 38 +++++++++++++++++++ ...constructorParameterShadowsOuterScopes2.js | 11 ++++++ ...ructorParameterShadowsOuterScopes2.symbols | 11 ++++++ ...structorParameterShadowsOuterScopes2.types | 12 ++++++ ...constructorParameterShadowsOuterScopes2.ts | 6 +++ 5 files changed, 78 insertions(+) create mode 100644 tests/baselines/reference/constructorParameterShadowsOuterScopes2.errors.txt diff --git a/tests/baselines/reference/constructorParameterShadowsOuterScopes2.errors.txt b/tests/baselines/reference/constructorParameterShadowsOuterScopes2.errors.txt new file mode 100644 index 0000000000000..7b2f3c6121a4b --- /dev/null +++ b/tests/baselines/reference/constructorParameterShadowsOuterScopes2.errors.txt @@ -0,0 +1,38 @@ +tests/cases/conformance/classes/propertyMemberDeclarations/constructorParameterShadowsOuterScopes2.ts(28,9): error TS2304: Cannot find name 'z'. + + +==== tests/cases/conformance/classes/propertyMemberDeclarations/constructorParameterShadowsOuterScopes2.ts (1 errors) ==== + // With useDefineForClassFields: true and ESNext target, initializer + // expressions for property declarations are evaluated in the scope of + // the class body and are permitted to reference parameters or local + // variables of the constructor. This is different from classic + // Typescript behaviour, with useDefineForClassFields: false. There, + // initialisers of property declarations are evaluated in the scope of + // the constructor body. + + // Note that when class fields are accepted in the ECMAScript + // standard, the target will become that year's ES20xx + + var x = 1; + class C { + b = x; // ok + constructor(x: string) { + } + } + + var y = 1; + class D { + b = y; // ok + constructor(x: string) { + var y = ""; + } + } + + class E { + b = z; // not ok + ~ +!!! error TS2304: Cannot find name 'z'. + constructor(z: string) { + } + } + \ No newline at end of file diff --git a/tests/baselines/reference/constructorParameterShadowsOuterScopes2.js b/tests/baselines/reference/constructorParameterShadowsOuterScopes2.js index aa1096303d900..a266bc5b0fb19 100644 --- a/tests/baselines/reference/constructorParameterShadowsOuterScopes2.js +++ b/tests/baselines/reference/constructorParameterShadowsOuterScopes2.js @@ -24,6 +24,12 @@ class D { var y = ""; } } + +class E { + b = z; // not ok + constructor(z: string) { + } +} //// [constructorParameterShadowsOuterScopes2.js] @@ -49,3 +55,8 @@ class D { var y = ""; } } +class E { + b = z; // not ok + constructor(z) { + } +} diff --git a/tests/baselines/reference/constructorParameterShadowsOuterScopes2.symbols b/tests/baselines/reference/constructorParameterShadowsOuterScopes2.symbols index 4a24bb89f692a..01bb0bb7613fa 100644 --- a/tests/baselines/reference/constructorParameterShadowsOuterScopes2.symbols +++ b/tests/baselines/reference/constructorParameterShadowsOuterScopes2.symbols @@ -43,3 +43,14 @@ class D { } } +class E { +>E : Symbol(E, Decl(constructorParameterShadowsOuterScopes2.ts, 24, 1)) + + b = z; // not ok +>b : Symbol(E.b, Decl(constructorParameterShadowsOuterScopes2.ts, 26, 9)) + + constructor(z: string) { +>z : Symbol(z, Decl(constructorParameterShadowsOuterScopes2.ts, 28, 16)) + } +} + diff --git a/tests/baselines/reference/constructorParameterShadowsOuterScopes2.types b/tests/baselines/reference/constructorParameterShadowsOuterScopes2.types index ffcb4a591d692..c9c24a45e5e6f 100644 --- a/tests/baselines/reference/constructorParameterShadowsOuterScopes2.types +++ b/tests/baselines/reference/constructorParameterShadowsOuterScopes2.types @@ -46,3 +46,15 @@ class D { } } +class E { +>E : E + + b = z; // not ok +>b : any +>z : any + + constructor(z: string) { +>z : string + } +} + diff --git a/tests/cases/conformance/classes/propertyMemberDeclarations/constructorParameterShadowsOuterScopes2.ts b/tests/cases/conformance/classes/propertyMemberDeclarations/constructorParameterShadowsOuterScopes2.ts index 72e16c916d654..d6c5be3bd7957 100644 --- a/tests/cases/conformance/classes/propertyMemberDeclarations/constructorParameterShadowsOuterScopes2.ts +++ b/tests/cases/conformance/classes/propertyMemberDeclarations/constructorParameterShadowsOuterScopes2.ts @@ -27,3 +27,9 @@ class D { var y = ""; } } + +class E { + b = z; // not ok + constructor(z: string) { + } +} From 56275715ecc3a9cc13f54a48719bc22b5fc4563f Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Wed, 29 Jan 2020 14:12:20 -0800 Subject: [PATCH 3/3] Add explanatory comment --- src/compiler/checker.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 1bb0a8af3dee2..db0b3a6db9141 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -1824,7 +1824,8 @@ namespace ts { if (nameNotFoundMessage) { if (propertyWithInvalidInitializer && !(compilerOptions.target === ScriptTarget.ESNext && compilerOptions.useDefineForClassFields)) { // We have a match, but the reference occurred within a property initializer and the identifier also binds - // to a local variable in the constructor where the code will be emitted. + // to a local variable in the constructor where the code will be emitted. Note that this is actually allowed + // with ESNext+useDefineForClassFields because the scope semantics are different. const propertyName = (propertyWithInvalidInitializer).name; error(errorLocation, Diagnostics.Initializer_of_instance_member_variable_0_cannot_reference_identifier_1_declared_in_the_constructor, declarationNameToString(propertyName), diagnosticName(nameArg!));