From f4123ac9cb53915fcb9c785876d17e51838bdd72 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Tue, 3 Oct 2017 12:48:17 -0700 Subject: [PATCH] noUnusedLocals: Warn for recursive call to private method --- src/compiler/checker.ts | 15 +++++++-- .../noUnusedLocals_selfReference.errors.txt | 12 ++++--- .../reference/noUnusedLocals_selfReference.js | 22 ++++++------- .../noUnusedLocals_selfReference.symbols | 31 +++++++++---------- .../noUnusedLocals_selfReference.types | 21 ++++++------- .../compiler/noUnusedLocals_selfReference.ts | 7 ++--- 6 files changed, 57 insertions(+), 51 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 310325d97d222..1aae927a04706 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -14910,7 +14910,7 @@ namespace ts { checkPropertyNotUsedBeforeDeclaration(prop, node, right); - markPropertyAsReferenced(prop, node); + markPropertyAsReferenced(prop, node, left.kind === SyntaxKind.ThisKeyword); getNodeLinks(node).resolvedSymbol = prop; @@ -15100,12 +15100,21 @@ namespace ts { return bestCandidate; } - function markPropertyAsReferenced(prop: Symbol, nodeForCheckWriteOnly: Node | undefined) { + function markPropertyAsReferenced(prop: Symbol, nodeForCheckWriteOnly: Node | undefined, isThisAccess: boolean) { if (prop && noUnusedIdentifiers && (prop.flags & SymbolFlags.ClassMember) && prop.valueDeclaration && hasModifier(prop.valueDeclaration, ModifierFlags.Private) && !(nodeForCheckWriteOnly && isWriteOnlyAccess(nodeForCheckWriteOnly))) { + + if (isThisAccess) { + // Find any FunctionLikeDeclaration because those create a new 'this' binding. But this should only matter for methods (or getters/setters). + const containingMethod = findAncestor(nodeForCheckWriteOnly, isFunctionLikeDeclaration); + if (containingMethod && containingMethod.symbol === prop) { + return; + } + } + if (getCheckFlags(prop) & CheckFlags.Instantiated) { getSymbolLinks(prop).target.isReferenced = true; } @@ -20571,7 +20580,7 @@ namespace ts { const parentType = getTypeForBindingElementParent(parent); const name = node.propertyName || node.name; const property = getPropertyOfType(parentType, getTextOfPropertyName(name)); - markPropertyAsReferenced(property, /*nodeForCheckWriteOnly*/ undefined); // A destructuring is never a write-only reference. + markPropertyAsReferenced(property, /*nodeForCheckWriteOnly*/ undefined, /*isThisAccess*/ false); // A destructuring is never a write-only reference. if (parent.initializer && property) { checkPropertyAccessibility(parent, parent.initializer, parentType, property); } diff --git a/tests/baselines/reference/noUnusedLocals_selfReference.errors.txt b/tests/baselines/reference/noUnusedLocals_selfReference.errors.txt index e4a0d478cb4fb..b51671077a7b4 100644 --- a/tests/baselines/reference/noUnusedLocals_selfReference.errors.txt +++ b/tests/baselines/reference/noUnusedLocals_selfReference.errors.txt @@ -1,9 +1,10 @@ tests/cases/compiler/noUnusedLocals_selfReference.ts(3,10): error TS6133: 'f' is declared but its value is never read. tests/cases/compiler/noUnusedLocals_selfReference.ts(4,7): error TS6133: 'C' is declared but its value is never read. tests/cases/compiler/noUnusedLocals_selfReference.ts(7,6): error TS6133: 'E' is declared but its value is never read. +tests/cases/compiler/noUnusedLocals_selfReference.ts(9,19): error TS6133: 'm' is declared but its value is never read. -==== tests/cases/compiler/noUnusedLocals_selfReference.ts (3 errors) ==== +==== tests/cases/compiler/noUnusedLocals_selfReference.ts (4 errors) ==== export {}; // Make this a module scope, so these are local variables. function f() { f; } @@ -18,11 +19,12 @@ tests/cases/compiler/noUnusedLocals_selfReference.ts(7,6): error TS6133: 'E' is ~ !!! error TS6133: 'E' is declared but its value is never read. + class P { private m() { this.m; } } + ~ +!!! error TS6133: 'm' is declared but its value is never read. + P; + // Does not detect mutual recursion. function g() { D; } class D { m() { g; } } - - // Does not work on private methods. - class P { private m() { this.m; } } - P; \ No newline at end of file diff --git a/tests/baselines/reference/noUnusedLocals_selfReference.js b/tests/baselines/reference/noUnusedLocals_selfReference.js index 5f206fbc3dc46..8bd59ebb0056b 100644 --- a/tests/baselines/reference/noUnusedLocals_selfReference.js +++ b/tests/baselines/reference/noUnusedLocals_selfReference.js @@ -7,13 +7,12 @@ class C { } enum E { A = 0, B = E.A } +class P { private m() { this.m; } } +P; + // Does not detect mutual recursion. function g() { D; } class D { m() { g; } } - -// Does not work on private methods. -class P { private m() { this.m; } } -P; //// [noUnusedLocals_selfReference.js] @@ -31,6 +30,13 @@ var E; E[E["A"] = 0] = "A"; E[E["B"] = 0] = "B"; })(E || (E = {})); +var P = /** @class */ (function () { + function P() { + } + P.prototype.m = function () { this.m; }; + return P; +}()); +P; // Does not detect mutual recursion. function g() { D; } var D = /** @class */ (function () { @@ -39,11 +45,3 @@ var D = /** @class */ (function () { D.prototype.m = function () { g; }; return D; }()); -// Does not work on private methods. -var P = /** @class */ (function () { - function P() { - } - P.prototype.m = function () { this.m; }; - return P; -}()); -P; diff --git a/tests/baselines/reference/noUnusedLocals_selfReference.symbols b/tests/baselines/reference/noUnusedLocals_selfReference.symbols index dcd815b261992..1c59e6effaafe 100644 --- a/tests/baselines/reference/noUnusedLocals_selfReference.symbols +++ b/tests/baselines/reference/noUnusedLocals_selfReference.symbols @@ -20,24 +20,23 @@ enum E { A = 0, B = E.A } >E : Symbol(E, Decl(noUnusedLocals_selfReference.ts, 5, 1)) >A : Symbol(E.A, Decl(noUnusedLocals_selfReference.ts, 6, 8)) +class P { private m() { this.m; } } +>P : Symbol(P, Decl(noUnusedLocals_selfReference.ts, 6, 25)) +>m : Symbol(P.m, Decl(noUnusedLocals_selfReference.ts, 8, 9)) +>this.m : Symbol(P.m, Decl(noUnusedLocals_selfReference.ts, 8, 9)) +>this : Symbol(P, Decl(noUnusedLocals_selfReference.ts, 6, 25)) +>m : Symbol(P.m, Decl(noUnusedLocals_selfReference.ts, 8, 9)) + +P; +>P : Symbol(P, Decl(noUnusedLocals_selfReference.ts, 6, 25)) + // Does not detect mutual recursion. function g() { D; } ->g : Symbol(g, Decl(noUnusedLocals_selfReference.ts, 6, 25)) ->D : Symbol(D, Decl(noUnusedLocals_selfReference.ts, 9, 19)) +>g : Symbol(g, Decl(noUnusedLocals_selfReference.ts, 9, 2)) +>D : Symbol(D, Decl(noUnusedLocals_selfReference.ts, 12, 19)) class D { m() { g; } } ->D : Symbol(D, Decl(noUnusedLocals_selfReference.ts, 9, 19)) ->m : Symbol(D.m, Decl(noUnusedLocals_selfReference.ts, 10, 9)) ->g : Symbol(g, Decl(noUnusedLocals_selfReference.ts, 6, 25)) - -// Does not work on private methods. -class P { private m() { this.m; } } ->P : Symbol(P, Decl(noUnusedLocals_selfReference.ts, 10, 22)) ->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, 10, 22)) ->m : Symbol(P.m, Decl(noUnusedLocals_selfReference.ts, 13, 9)) - -P; ->P : Symbol(P, Decl(noUnusedLocals_selfReference.ts, 10, 22)) +>D : Symbol(D, Decl(noUnusedLocals_selfReference.ts, 12, 19)) +>m : Symbol(D.m, Decl(noUnusedLocals_selfReference.ts, 13, 9)) +>g : Symbol(g, Decl(noUnusedLocals_selfReference.ts, 9, 2)) diff --git a/tests/baselines/reference/noUnusedLocals_selfReference.types b/tests/baselines/reference/noUnusedLocals_selfReference.types index 7d2741c5681a1..897c36b8bc545 100644 --- a/tests/baselines/reference/noUnusedLocals_selfReference.types +++ b/tests/baselines/reference/noUnusedLocals_selfReference.types @@ -21,17 +21,6 @@ enum E { A = 0, B = E.A } >E : typeof E >A : E -// Does not detect mutual recursion. -function g() { D; } ->g : () => void ->D : typeof D - -class D { m() { g; } } ->D : D ->m : () => void ->g : () => void - -// Does not work on private methods. class P { private m() { this.m; } } >P : P >m : () => void @@ -42,3 +31,13 @@ class P { private m() { this.m; } } P; >P : typeof P +// Does not detect mutual recursion. +function g() { D; } +>g : () => void +>D : typeof D + +class D { m() { g; } } +>D : D +>m : () => void +>g : () => void + diff --git a/tests/cases/compiler/noUnusedLocals_selfReference.ts b/tests/cases/compiler/noUnusedLocals_selfReference.ts index 8eb528743c0ba..c60cdffcdb384 100644 --- a/tests/cases/compiler/noUnusedLocals_selfReference.ts +++ b/tests/cases/compiler/noUnusedLocals_selfReference.ts @@ -8,10 +8,9 @@ class C { } enum E { A = 0, B = E.A } +class P { private m() { this.m; } } +P; + // Does not detect mutual recursion. function g() { D; } class D { m() { g; } } - -// Does not work on private methods. -class P { private m() { this.m; } } -P;