noUnusedLocals: Warn for recursive call to private method (#18920)

src/compiler/checker.ts

@@ -15028,7 +15028,7 @@ namespace ts {
 
             checkPropertyNotUsedBeforeDeclaration(prop, node, right);
 
-            markPropertyAsReferenced(prop, node);
+            markPropertyAsReferenced(prop, node, left.kind === SyntaxKind.ThisKeyword);
 
             getNodeLinks(node).resolvedSymbol = prop;
 
@@ -15218,12 +15218,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;
                 }
@@ -20716,7 +20725,7 @@ namespace ts {
                 const parentType = getTypeForBindingElementParent(parent);
                 const name = node.propertyName || <Identifier>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);
                 }

tests/baselines/reference/noUnusedLocals_selfReference.errors.txt

@@ -2,9 +2,10 @@ 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 (4 errors) ====
+==== tests/cases/compiler/noUnusedLocals_selfReference.ts (5 errors) ====
     export {}; // Make this a module scope, so these are local variables.
     
     function f() {
@@ -26,11 +27,12 @@ tests/cases/compiler/noUnusedLocals_selfReference.ts(12,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;
     

tests/baselines/reference/noUnusedLocals_selfReference.js

@@ -12,13 +12,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]
@@ -41,6 +40,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 () {
@@ -49,11 +55,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;

tests/baselines/reference/noUnusedLocals_selfReference.symbols

@@ -29,24 +29,23 @@ 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))
 
-// Does not detect mutual recursion.
-function g() { D; }
->g : Symbol(g, Decl(noUnusedLocals_selfReference.ts, 11, 25))
->D : Symbol(D, Decl(noUnusedLocals_selfReference.ts, 14, 19))
-
-class D { m() { g; } }
->D : Symbol(D, Decl(noUnusedLocals_selfReference.ts, 14, 19))
->m : Symbol(D.m, Decl(noUnusedLocals_selfReference.ts, 15, 9))
->g : Symbol(g, Decl(noUnusedLocals_selfReference.ts, 11, 25))
-
-// Does not work on private methods.
 class P { private m() { this.m; } }
->P : Symbol(P, Decl(noUnusedLocals_selfReference.ts, 15, 22))
->m : Symbol(P.m, Decl(noUnusedLocals_selfReference.ts, 18, 9))
->this.m : Symbol(P.m, Decl(noUnusedLocals_selfReference.ts, 18, 9))
->this : Symbol(P, Decl(noUnusedLocals_selfReference.ts, 15, 22))
->m : Symbol(P.m, Decl(noUnusedLocals_selfReference.ts, 18, 9))
+>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;
->P : Symbol(P, Decl(noUnusedLocals_selfReference.ts, 15, 22))
+>P : Symbol(P, Decl(noUnusedLocals_selfReference.ts, 11, 25))
+
+// 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))
+
+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))
 

tests/baselines/reference/noUnusedLocals_selfReference.types

@@ -30,17 +30,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
@@ -51,3 +40,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
+

tests/cases/compiler/noUnusedLocals_selfReference.ts

@@ -13,10 +13,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;