Skip to content

Commit

Permalink
Fix bug: Interface type parameter merged with property is not unused (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
Andy authored Feb 15, 2018
1 parent 1b6aa13 commit a133cec
Show file tree
Hide file tree
Showing 7 changed files with 115 additions and 29 deletions.
30 changes: 15 additions & 15 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1159,7 +1159,7 @@ namespace ts {
const originalLocation = location; // needed for did-you-mean error reporting, which gathers candidates starting from the original location
let result: Symbol;
let lastLocation: Node;
let lastNonBlockLocation: Node;
let lastSelfReferenceLocation: Node;
let propertyWithInvalidInitializer: Node;
const errorLocation = location;
let grandparent: Node;
Expand Down Expand Up @@ -1383,17 +1383,17 @@ namespace ts {
}
break;
}
if (isNonBlockLocation(location)) {
lastNonBlockLocation = location;
if (isSelfReferenceLocation(location)) {
lastSelfReferenceLocation = location;
}
lastLocation = location;
location = location.parent;
}

// We just climbed up parents looking for the name, meaning that we started in a descendant node of `lastLocation`.
// If `result === lastNonBlockLocation.symbol`, that means that we are somewhere inside `lastNonBlockLocation` looking up a name, and resolving to `lastLocation` itself.
// If `result === lastSelfReferenceLocation.symbol`, that means that we are somewhere inside `lastSelfReferenceLocation` looking up a name, and resolving to `lastLocation` itself.
// That means that this is a self-reference of `lastLocation`, and shouldn't count this when considering whether `lastLocation` is used.
if (isUse && result && nameNotFoundMessage && noUnusedIdentifiers && result !== lastNonBlockLocation.symbol) {
if (isUse && result && nameNotFoundMessage && noUnusedIdentifiers && (!lastSelfReferenceLocation || result !== lastSelfReferenceLocation.symbol)) {
result.isReferenced = true;
}

Expand Down Expand Up @@ -1476,17 +1476,17 @@ namespace ts {
return result;
}

function isNonBlockLocation({ kind }: Node): boolean {
switch (kind) {
case SyntaxKind.Block:
case SyntaxKind.ModuleBlock:
case SyntaxKind.SwitchStatement:
case SyntaxKind.CaseBlock:
case SyntaxKind.CaseClause:
case SyntaxKind.DefaultClause:
return false;
default:
function isSelfReferenceLocation(node: Node): boolean {
switch (node.kind) {
case SyntaxKind.FunctionDeclaration:
case SyntaxKind.ClassDeclaration:
case SyntaxKind.InterfaceDeclaration:
case SyntaxKind.EnumDeclaration:
case SyntaxKind.TypeAliasDeclaration:
case SyntaxKind.ModuleDeclaration: // For `namespace N { N; }`
return true;
default:
return false;
}
}

Expand Down
21 changes: 19 additions & 2 deletions tests/baselines/reference/noUnusedLocals_selfReference.errors.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,13 @@ tests/cases/compiler/noUnusedLocals_selfReference.ts(3,10): error TS6133: 'f' is
tests/cases/compiler/noUnusedLocals_selfReference.ts(5,14): error TS6133: 'g' is declared but its value is never read.
tests/cases/compiler/noUnusedLocals_selfReference.ts(9,7): error TS6133: 'C' is declared but its value is never read.
tests/cases/compiler/noUnusedLocals_selfReference.ts(12,6): error TS6133: 'E' is declared but its value is never read.
tests/cases/compiler/noUnusedLocals_selfReference.ts(14,19): error TS6133: 'm' is declared but its value is never read.
tests/cases/compiler/noUnusedLocals_selfReference.ts(13,11): error TS6133: 'I' is declared but its value is never read.
tests/cases/compiler/noUnusedLocals_selfReference.ts(14,6): error TS6133: 'T' is declared but its value is never read.
tests/cases/compiler/noUnusedLocals_selfReference.ts(15,11): error TS6133: 'N' is declared but its value is never read.
tests/cases/compiler/noUnusedLocals_selfReference.ts(22,19): error TS6133: 'm' is declared but its value is never read.


==== tests/cases/compiler/noUnusedLocals_selfReference.ts (5 errors) ====
==== tests/cases/compiler/noUnusedLocals_selfReference.ts (8 errors) ====
export {}; // Make this a module scope, so these are local variables.

function f() {
Expand All @@ -26,6 +29,20 @@ tests/cases/compiler/noUnusedLocals_selfReference.ts(14,19): error TS6133: 'm' i
enum E { A = 0, B = E.A }
~
!!! error TS6133: 'E' is declared but its value is never read.
interface I { x: I };
~
!!! error TS6133: 'I' is declared but its value is never read.
type T = { x: T };
~
!!! error TS6133: 'T' is declared but its value is never read.
namespace N { N; }
~
!!! error TS6133: 'N' is declared but its value is never read.

// Avoid a false positive.
// Previously `T` was considered unused due to merging with the property,
// back when all non-blocks were checked for recursion.
export interface A<T> { T: T }

class P { private m() { this.m; } }
~
Expand Down
13 changes: 13 additions & 0 deletions tests/baselines/reference/noUnusedLocals_selfReference.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,14 @@ class C {
m() { C; }
}
enum E { A = 0, B = E.A }
interface I { x: I };
type T = { x: T };
namespace N { N; }

// Avoid a false positive.
// Previously `T` was considered unused due to merging with the property,
// back when all non-blocks were checked for recursion.
export interface A<T> { T: T }

class P { private m() { this.m; } }
P;
Expand Down Expand Up @@ -40,6 +48,11 @@ var E;
E[E["A"] = 0] = "A";
E[E["B"] = 0] = "B";
})(E || (E = {}));
;
var N;
(function (N) {
N;
})(N || (N = {}));
var P = /** @class */ (function () {
function P() {
}
Expand Down
45 changes: 34 additions & 11 deletions tests/baselines/reference/noUnusedLocals_selfReference.symbols
Original file line number Diff line number Diff line change
Expand Up @@ -29,23 +29,46 @@ enum E { A = 0, B = E.A }
>E : Symbol(E, Decl(noUnusedLocals_selfReference.ts, 10, 1))
>A : Symbol(E.A, Decl(noUnusedLocals_selfReference.ts, 11, 8))

interface I { x: I };
>I : Symbol(I, Decl(noUnusedLocals_selfReference.ts, 11, 25))
>x : Symbol(I.x, Decl(noUnusedLocals_selfReference.ts, 12, 13))
>I : Symbol(I, Decl(noUnusedLocals_selfReference.ts, 11, 25))

type T = { x: T };
>T : Symbol(T, Decl(noUnusedLocals_selfReference.ts, 12, 21))
>x : Symbol(x, Decl(noUnusedLocals_selfReference.ts, 13, 10))
>T : Symbol(T, Decl(noUnusedLocals_selfReference.ts, 12, 21))

namespace N { N; }
>N : Symbol(N, Decl(noUnusedLocals_selfReference.ts, 13, 18))
>N : Symbol(N, Decl(noUnusedLocals_selfReference.ts, 13, 18))

// Avoid a false positive.
// Previously `T` was considered unused due to merging with the property,
// back when all non-blocks were checked for recursion.
export interface A<T> { T: T }
>A : Symbol(A, Decl(noUnusedLocals_selfReference.ts, 14, 18))
>T : Symbol(T, Decl(noUnusedLocals_selfReference.ts, 19, 19), Decl(noUnusedLocals_selfReference.ts, 19, 23))
>T : Symbol(T, Decl(noUnusedLocals_selfReference.ts, 19, 19), Decl(noUnusedLocals_selfReference.ts, 19, 23))
>T : Symbol(T, Decl(noUnusedLocals_selfReference.ts, 19, 19), Decl(noUnusedLocals_selfReference.ts, 19, 23))

class P { private m() { this.m; } }
>P : Symbol(P, Decl(noUnusedLocals_selfReference.ts, 11, 25))
>m : Symbol(P.m, Decl(noUnusedLocals_selfReference.ts, 13, 9))
>this.m : Symbol(P.m, Decl(noUnusedLocals_selfReference.ts, 13, 9))
>this : Symbol(P, Decl(noUnusedLocals_selfReference.ts, 11, 25))
>m : Symbol(P.m, Decl(noUnusedLocals_selfReference.ts, 13, 9))
>P : Symbol(P, Decl(noUnusedLocals_selfReference.ts, 19, 30))
>m : Symbol(P.m, Decl(noUnusedLocals_selfReference.ts, 21, 9))
>this.m : Symbol(P.m, Decl(noUnusedLocals_selfReference.ts, 21, 9))
>this : Symbol(P, Decl(noUnusedLocals_selfReference.ts, 19, 30))
>m : Symbol(P.m, Decl(noUnusedLocals_selfReference.ts, 21, 9))

P;
>P : Symbol(P, Decl(noUnusedLocals_selfReference.ts, 11, 25))
>P : Symbol(P, Decl(noUnusedLocals_selfReference.ts, 19, 30))

// Does not detect mutual recursion.
function g() { D; }
>g : Symbol(g, Decl(noUnusedLocals_selfReference.ts, 14, 2))
>D : Symbol(D, Decl(noUnusedLocals_selfReference.ts, 17, 19))
>g : Symbol(g, Decl(noUnusedLocals_selfReference.ts, 22, 2))
>D : Symbol(D, Decl(noUnusedLocals_selfReference.ts, 25, 19))

class D { m() { g; } }
>D : Symbol(D, Decl(noUnusedLocals_selfReference.ts, 17, 19))
>m : Symbol(D.m, Decl(noUnusedLocals_selfReference.ts, 18, 9))
>g : Symbol(g, Decl(noUnusedLocals_selfReference.ts, 14, 2))
>D : Symbol(D, Decl(noUnusedLocals_selfReference.ts, 25, 19))
>m : Symbol(D.m, Decl(noUnusedLocals_selfReference.ts, 26, 9))
>g : Symbol(g, Decl(noUnusedLocals_selfReference.ts, 22, 2))

23 changes: 23 additions & 0 deletions tests/baselines/reference/noUnusedLocals_selfReference.types
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,29 @@ enum E { A = 0, B = E.A }
>E : typeof E
>A : E

interface I { x: I };
>I : I
>x : I
>I : I

type T = { x: T };
>T : { x: T; }
>x : { x: T; }
>T : { x: T; }

namespace N { N; }
>N : typeof N
>N : typeof N

// Avoid a false positive.
// Previously `T` was considered unused due to merging with the property,
// back when all non-blocks were checked for recursion.
export interface A<T> { T: T }
>A : A<T>
>T : T
>T : T
>T : T

class P { private m() { this.m; } }
>P : P
>m : () => void
Expand Down
9 changes: 9 additions & 0 deletions tests/cases/compiler/noUnusedLocals_selfReference.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// @noUnusedLocals: true
// @noUnusedParameters: true

export {}; // Make this a module scope, so these are local variables.

Expand All @@ -12,6 +13,14 @@ class C {
m() { C; }
}
enum E { A = 0, B = E.A }
interface I { x: I };
type T = { x: T };
namespace N { N; }

// Avoid a false positive.
// Previously `T` was considered unused due to merging with the property,
// back when all non-blocks were checked for recursion.
export interface A<T> { T: T }

class P { private m() { this.m; } }
P;
Expand Down
3 changes: 2 additions & 1 deletion tests/cases/compiler/nounusedTypeParameterConstraint.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
//@noUnusedLocals:true
// @noUnusedLocals: true
// @noUnusedParameters:true

//@filename: bar.ts
export interface IEventSourcedEntity { }
Expand Down

0 comments on commit a133cec

Please sign in to comment.