Check for unused getter/setter in classes

src/compiler/checker.ts

@@ -16034,27 +16034,27 @@ namespace ts {
         }
 
         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 (!prop || !noUnusedIdentifiers || !(prop.flags & SymbolFlags.ClassMember) || !prop.valueDeclaration || !hasModifier(prop.valueDeclaration, ModifierFlags.Private)) {
+                return;
+            }
+            if (nodeForCheckWriteOnly && isWriteOnlyAccess(nodeForCheckWriteOnly) && !(prop.flags & SymbolFlags.SetAccessor && !(prop.flags & SymbolFlags.GetAccessor))) {
+                return;
+            }
 
-                if (getCheckFlags(prop) & CheckFlags.Instantiated) {
-                    getSymbolLinks(prop).target.isReferenced = true;
-                }
-                else {
-                    prop.isReferenced = true;
+            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;
+            }
+            else {
+                prop.isReferenced = true;
+            }
         }
 
         function isValidPropertyAccess(node: PropertyAccessExpression | QualifiedName, propertyName: __String): boolean {
@@ -21222,17 +21222,31 @@ namespace ts {
             if (compilerOptions.noUnusedLocals && !(node.flags & NodeFlags.Ambient)) {
                 if (node.members) {
                     for (const member of node.members) {
-                        if (member.kind === SyntaxKind.MethodDeclaration || member.kind === SyntaxKind.PropertyDeclaration) {
-                            if (!member.symbol.isReferenced && hasModifier(member, ModifierFlags.Private)) {
-                                error(member.name, Diagnostics._0_is_declared_but_its_value_is_never_read, symbolName(member.symbol));
-                            }
-                        }
-                        else if (member.kind === SyntaxKind.Constructor) {
-                            for (const parameter of (<ConstructorDeclaration>member).parameters) {
-                                if (!parameter.symbol.isReferenced && hasModifier(parameter, ModifierFlags.Private)) {
-                                    error(parameter.name, Diagnostics.Property_0_is_declared_but_its_value_is_never_read, symbolName(parameter.symbol));
+                        switch (member.kind) {
+                            case SyntaxKind.MethodDeclaration:
+                            case SyntaxKind.PropertyDeclaration:
+                            case SyntaxKind.GetAccessor:
+                            case SyntaxKind.SetAccessor:
+                                if (member.kind === SyntaxKind.SetAccessor && member.symbol.flags & SymbolFlags.GetAccessor) {
+                                    // Already would have reported an error on the getter.
+                                    break;
                                 }
-                            }
+                                if (!member.symbol.isReferenced && hasModifier(member, ModifierFlags.Private)) {
+                                    error(member.name, Diagnostics._0_is_declared_but_its_value_is_never_read, symbolName(member.symbol));
+                                }
+                                break;
+                            case SyntaxKind.Constructor:
+                                for (const parameter of (<ConstructorDeclaration>member).parameters) {
+                                    if (!parameter.symbol.isReferenced && hasModifier(parameter, ModifierFlags.Private)) {
+                                        error(parameter.name, Diagnostics.Property_0_is_declared_but_its_value_is_never_read, symbolName(parameter.symbol));
+                                    }
+                                }
+                                break;
+                            case SyntaxKind.IndexSignature:
+                                // Can't be private
+                                break;
+                            default:
+                                Debug.fail();
                         }
                     }
                 }

tests/baselines/reference/unusedGetterInClass.errors.txt

@@ -0,0 +1,15 @@
+tests/cases/compiler/unusedGetterInClass.ts(4,17): error TS6133: 'fullName' is declared but its value is never read.
+
+
+==== tests/cases/compiler/unusedGetterInClass.ts (1 errors) ====
+    class Employee {
+        private _fullName: string;
+
+        private get fullName(): string {
+                    ~~~~~~~~
+!!! error TS6133: 'fullName' is declared but its value is never read.
+            return this._fullName;
+        }
+        // Will not also error on the setter
+        private set fullName(_: string) {}
+    }

tests/baselines/reference/unusedGetterInClass.js

@@ -2,9 +2,11 @@
 class Employee {
     private _fullName: string;
 
-    get fullName(): string {
+    private get fullName(): string {
         return this._fullName;
     }
+    // Will not also error on the setter
+    private set fullName(_: string) {}
 }
 
 //// [unusedGetterInClass.js]
@@ -15,6 +17,8 @@ var Employee = /** @class */ (function () {
         get: function () {
             return this._fullName;
         },
+        // Will not also error on the setter
+        set: function (_) { },
         enumerable: true,
         configurable: true
     });

tests/baselines/reference/unusedGetterInClass.symbols

@@ -5,12 +5,16 @@ class Employee {
     private _fullName: string;
 >_fullName : Symbol(Employee._fullName, Decl(unusedGetterInClass.ts, 0, 16))
 
-    get fullName(): string {
->fullName : Symbol(Employee.fullName, Decl(unusedGetterInClass.ts, 1, 30))
+    private get fullName(): string {
+>fullName : Symbol(Employee.fullName, Decl(unusedGetterInClass.ts, 1, 30), Decl(unusedGetterInClass.ts, 5, 5))
 
         return this._fullName;
 >this._fullName : Symbol(Employee._fullName, Decl(unusedGetterInClass.ts, 0, 16))
 >this : Symbol(Employee, Decl(unusedGetterInClass.ts, 0, 0))
 >_fullName : Symbol(Employee._fullName, Decl(unusedGetterInClass.ts, 0, 16))
     }
+    // Will not also error on the setter
+    private set fullName(_: string) {}
+>fullName : Symbol(Employee.fullName, Decl(unusedGetterInClass.ts, 1, 30), Decl(unusedGetterInClass.ts, 5, 5))
+>_ : Symbol(_, Decl(unusedGetterInClass.ts, 7, 25))
 }

tests/baselines/reference/unusedGetterInClass.types

@@ -5,12 +5,16 @@ class Employee {
     private _fullName: string;
 >_fullName : string
 
-    get fullName(): string {
+    private get fullName(): string {
 >fullName : string
 
         return this._fullName;
 >this._fullName : string
 >this : this
 >_fullName : string
     }
+    // Will not also error on the setter
+    private set fullName(_: string) {}
+>fullName : string
+>_ : string
 }

tests/baselines/reference/unusedSetterInClass.errors.txt

@@ -1,13 +1,16 @@
 tests/cases/compiler/unusedSetterInClass.ts(2,13): error TS6133: '_fullName' is declared but its value is never read.
+tests/cases/compiler/unusedSetterInClass.ts(4,17): error TS6133: 'fullName' is declared but its value is never read.
 
 
-==== tests/cases/compiler/unusedSetterInClass.ts (1 errors) ====
+==== tests/cases/compiler/unusedSetterInClass.ts (2 errors) ====
     class Employee {
         private _fullName: string;
                 ~~~~~~~~~
 !!! error TS6133: '_fullName' is declared but its value is never read.
 
-        set fullName(newName: string) {
+        private set fullName(newName: string) {
+                    ~~~~~~~~
+!!! error TS6133: 'fullName' is declared but its value is never read.
             this._fullName = newName;
         }
     }

tests/baselines/reference/unusedSetterInClass.js

@@ -2,7 +2,7 @@
 class Employee {
     private _fullName: string;
 
-    set fullName(newName: string) {
+    private set fullName(newName: string) {
         this._fullName = newName;
     }
 }

tests/baselines/reference/unusedSetterInClass.symbols

@@ -5,14 +5,14 @@ class Employee {
     private _fullName: string;
 >_fullName : Symbol(Employee._fullName, Decl(unusedSetterInClass.ts, 0, 16))
 
-    set fullName(newName: string) {
+    private set fullName(newName: string) {
 >fullName : Symbol(Employee.fullName, Decl(unusedSetterInClass.ts, 1, 30))
->newName : Symbol(newName, Decl(unusedSetterInClass.ts, 3, 17))
+>newName : Symbol(newName, Decl(unusedSetterInClass.ts, 3, 25))
 
         this._fullName = newName;
 >this._fullName : Symbol(Employee._fullName, Decl(unusedSetterInClass.ts, 0, 16))
 >this : Symbol(Employee, Decl(unusedSetterInClass.ts, 0, 0))
 >_fullName : Symbol(Employee._fullName, Decl(unusedSetterInClass.ts, 0, 16))
->newName : Symbol(newName, Decl(unusedSetterInClass.ts, 3, 17))
+>newName : Symbol(newName, Decl(unusedSetterInClass.ts, 3, 25))
     }
 }

tests/baselines/reference/unusedSetterInClass.types

@@ -5,7 +5,7 @@ class Employee {
     private _fullName: string;
 >_fullName : string
 
-    set fullName(newName: string) {
+    private set fullName(newName: string) {
 >fullName : string
 >newName : string
 

tests/baselines/reference/unusedSetterInClass2.errors.txt

@@ -0,0 +1,14 @@
+tests/cases/compiler/unusedSetterInClass2.ts(3,17): error TS1056: Accessors are only available when targeting ECMAScript 5 and higher.
+
+
+==== tests/cases/compiler/unusedSetterInClass2.ts (1 errors) ====
+    // Unlike everything else, a setter without a getter is used by a write access.
+    class Employee {
+        private set p(_: number) {}
+                    ~
+!!! error TS1056: Accessors are only available when targeting ECMAScript 5 and higher.
+
+        m() {
+            this.p = 0;
+        }
+    }

tests/baselines/reference/unusedSetterInClass2.js

@@ -0,0 +1,25 @@
+//// [unusedSetterInClass2.ts]
+// Unlike everything else, a setter without a getter is used by a write access.
+class Employee {
+    private set p(_: number) {}
+
+    m() {
+        this.p = 0;
+    }
+}
+
+//// [unusedSetterInClass2.js]
+// Unlike everything else, a setter without a getter is used by a write access.
+var Employee = /** @class */ (function () {
+    function Employee() {
+    }
+    Object.defineProperty(Employee.prototype, "p", {
+        set: function (_) { },
+        enumerable: true,
+        configurable: true
+    });
+    Employee.prototype.m = function () {
+        this.p = 0;
+    };
+    return Employee;
+}());

tests/baselines/reference/unusedSetterInClass2.symbols

@@ -0,0 +1,18 @@
+=== tests/cases/compiler/unusedSetterInClass2.ts ===
+// Unlike everything else, a setter without a getter is used by a write access.
+class Employee {
+>Employee : Symbol(Employee, Decl(unusedSetterInClass2.ts, 0, 0))
+
+    private set p(_: number) {}
+>p : Symbol(Employee.p, Decl(unusedSetterInClass2.ts, 1, 16))
+>_ : Symbol(_, Decl(unusedSetterInClass2.ts, 2, 18))
+
+    m() {
+>m : Symbol(Employee.m, Decl(unusedSetterInClass2.ts, 2, 31))
+
+        this.p = 0;
+>this.p : Symbol(Employee.p, Decl(unusedSetterInClass2.ts, 1, 16))
+>this : Symbol(Employee, Decl(unusedSetterInClass2.ts, 0, 0))
+>p : Symbol(Employee.p, Decl(unusedSetterInClass2.ts, 1, 16))
+    }
+}

tests/baselines/reference/unusedSetterInClass2.types

@@ -0,0 +1,20 @@
+=== tests/cases/compiler/unusedSetterInClass2.ts ===
+// Unlike everything else, a setter without a getter is used by a write access.
+class Employee {
+>Employee : Employee
+
+    private set p(_: number) {}
+>p : number
+>_ : number
+
+    m() {
+>m : () => void
+
+        this.p = 0;
+>this.p = 0 : 0
+>this.p : number
+>this : this
+>p : number
+>0 : 0
+    }
+}

tests/cases/compiler/unusedGetterInClass.ts

@@ -5,7 +5,9 @@
 class Employee {
     private _fullName: string;
 
-    get fullName(): string {
+    private get fullName(): string {
         return this._fullName;
     }
+    // Will not also error on the setter
+    private set fullName(_: string) {}
 }

tests/cases/compiler/unusedSetterInClass.ts

@@ -5,7 +5,7 @@
 class Employee {
     private _fullName: string;
 
-    set fullName(newName: string) {
+    private set fullName(newName: string) {
         this._fullName = newName;
     }
 }

tests/cases/compiler/unusedSetterInClass2.ts

@@ -0,0 +1,10 @@
+// @noUnusedLocals:true
+
+// Unlike everything else, a setter without a getter is used by a write access.
+class Employee {
+    private set p(_: number) {}
+
+    m() {
+        this.p = 0;
+    }
+}