Skip to content

Commit

Permalink
Merge pull request #14952 from Microsoft/fix-scope-checks-of-class-pr…
Browse files Browse the repository at this point in the history
…operties

Fix scope checks of class properties
  • Loading branch information
sandersn authored Apr 4, 2017
2 parents dd48dd1 + f65819a commit 013d52a
Show file tree
Hide file tree
Showing 5 changed files with 89 additions and 5 deletions.
10 changes: 5 additions & 5 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -721,7 +721,7 @@ namespace ts {
}
// declaration is after usage
// can be legal if usage is deferred (i.e. inside function or in initializer of instance property)
if (isUsedInFunctionOrInstanceProperty(usage)) {
if (isUsedInFunctionOrInstanceProperty(usage, declaration)) {
return true;
}
const sourceFiles = host.getSourceFiles();
Expand Down Expand Up @@ -752,8 +752,7 @@ namespace ts {
// 1. inside a function
// 2. inside an instance property initializer, a reference to a non-instance property
const container = getEnclosingBlockScopeContainer(declaration);
const isInstanceProperty = declaration.kind === SyntaxKind.PropertyDeclaration && !(getModifierFlags(declaration) & ModifierFlags.Static);
return isUsedInFunctionOrInstanceProperty(usage, isInstanceProperty, container);
return isUsedInFunctionOrInstanceProperty(usage, declaration, container);

function isImmediatelyUsedInInitializerOfBlockScopedVariable(declaration: VariableDeclaration, usage: Node): boolean {
const container = getEnclosingBlockScopeContainer(declaration);
Expand Down Expand Up @@ -782,7 +781,7 @@ namespace ts {
return false;
}

function isUsedInFunctionOrInstanceProperty(usage: Node, isDeclarationInstanceProperty?: boolean, container?: Node): boolean {
function isUsedInFunctionOrInstanceProperty(usage: Node, declaration: Node, container?: Node): boolean {
let current = usage;
while (current) {
if (current === container) {
Expand All @@ -799,7 +798,8 @@ namespace ts {
(<PropertyDeclaration>current.parent).initializer === current;

if (initializerOfInstanceProperty) {
return !isDeclarationInstanceProperty;
const isDeclarationInstanceProperty = declaration.kind === SyntaxKind.PropertyDeclaration && !(getModifierFlags(declaration) & ModifierFlags.Static);
return !isDeclarationInstanceProperty || getContainingClass(usage) !== getContainingClass(declaration);
}

current = current.parent;
Expand Down
26 changes: 26 additions & 0 deletions tests/baselines/reference/scopeCheckClassProperty.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
//// [scopeCheckClassProperty.ts]
class C {
constructor() {
new A().p; // ok
}
public x = new A().p; // should also be ok
}
class A {
public p = '';
}


//// [scopeCheckClassProperty.js]
var C = (function () {
function C() {
this.x = new A().p; // should also be ok
new A().p; // ok
}
return C;
}());
var A = (function () {
function A() {
this.p = '';
}
return A;
}());
23 changes: 23 additions & 0 deletions tests/baselines/reference/scopeCheckClassProperty.symbols
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
=== tests/cases/compiler/scopeCheckClassProperty.ts ===
class C {
>C : Symbol(C, Decl(scopeCheckClassProperty.ts, 0, 0))

constructor() {
new A().p; // ok
>new A().p : Symbol(A.p, Decl(scopeCheckClassProperty.ts, 6, 9))
>A : Symbol(A, Decl(scopeCheckClassProperty.ts, 5, 1))
>p : Symbol(A.p, Decl(scopeCheckClassProperty.ts, 6, 9))
}
public x = new A().p; // should also be ok
>x : Symbol(C.x, Decl(scopeCheckClassProperty.ts, 3, 3))
>new A().p : Symbol(A.p, Decl(scopeCheckClassProperty.ts, 6, 9))
>A : Symbol(A, Decl(scopeCheckClassProperty.ts, 5, 1))
>p : Symbol(A.p, Decl(scopeCheckClassProperty.ts, 6, 9))
}
class A {
>A : Symbol(A, Decl(scopeCheckClassProperty.ts, 5, 1))

public p = '';
>p : Symbol(A.p, Decl(scopeCheckClassProperty.ts, 6, 9))
}

26 changes: 26 additions & 0 deletions tests/baselines/reference/scopeCheckClassProperty.types
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
=== tests/cases/compiler/scopeCheckClassProperty.ts ===
class C {
>C : C

constructor() {
new A().p; // ok
>new A().p : string
>new A() : A
>A : typeof A
>p : string
}
public x = new A().p; // should also be ok
>x : string
>new A().p : string
>new A() : A
>A : typeof A
>p : string
}
class A {
>A : A

public p = '';
>p : string
>'' : ""
}

9 changes: 9 additions & 0 deletions tests/cases/compiler/scopeCheckClassProperty.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
class C {
constructor() {
new A().p; // ok
}
public x = new A().p; // should also be ok
}
class A {
public p = '';
}

0 comments on commit 013d52a

Please sign in to comment.