Detect dead properties

README.md

@@ -107,7 +107,7 @@ parameters:
 ## Generic usage providers:
 
 #### Reflection:
-- Any enum, constant or method accessed via `ReflectionClass` is detected as used
+- Any property, enum, constant or method accessed via `ReflectionClass` is detected as used
   - e.g. `$reflection->getConstructor()`, `$reflection->getConstant('NAME')`, `$reflection->getMethods()`, `$reflection->getCases()`...
 
 #### Vendor:
@@ -332,6 +332,8 @@ class UserFacade
    ! Excluded usage at tests/User/UserFacadeTest.php:241 left intact
 ```
 
+- Also, removing dead properties currently only removes its definition (leaving all write usages as is).
+
 
 ## Calls over unknown types
 - In order to prevent false positives, we support even calls over unknown types (e.g. `$unknown->method()`) by marking all methods named `method` as used
@@ -376,10 +378,14 @@ parameters:
         detect:
             deadMethods: true
             deadConstants: true
-            deadEnumCases: false
+            deadProperties: false # opt-in
+            deadEnumCases: false # opt-in
 ```
 
-Enum cases are disabled by default as those are often used in API input objects (using custom deserialization, which typically require custom usage provider).
+Enum cases and properties are disabled by default as those are often used in API input objects (using custom deserialization, which typically require custom usage provider).
+But libraries should be able to enable those easily.
+
+Properties are considered dead if they are never read.
 
 
 ## Comparison with tomasvotruba/unused-public
@@ -477,8 +483,9 @@ If you set up `editorUrl` [parameter](https://phpstan.org/user-guide/output-form
   - You can also mark whole class or interface with `@api` to mark all its methods as entrypoints
 
 ## Future scope:
-- Dead class property detection
 - Dead class detection
+- Dead parameters detection
+- Useless public/protected visibility
 
 ## Contributing
 - Check your code by `composer check`

composer.json

@@ -90,7 +90,7 @@
         ],
         "check:coverage": [
             "XDEBUG_MODE=coverage phpunit tests --coverage-clover cache/clover.xml",
-            "coverage-guard check cache/clover.xml"
+            "coverage-guard check cache/clover.xml --color"
         ],
         "check:cs": "phpcs",
         "check:dependencies": "composer-dependency-analyser",

coverage-guard.php

@@ -48,11 +48,7 @@ public function inspect(
      */
     private function getRequiredCoverage(ReflectionClass $classReflection): int
     {
-        $isPoor = in_array($classReflection->getName(), [
-            BackwardCompatibilityChecker::class,
-            ReflectionHelper::class,
-        ], true);
-
+        $isPoor = $classReflection->getName() === BackwardCompatibilityChecker::class;
         $isCore = $classReflection->implementsInterface(MemberUsageProvider::class)
             || $classReflection->implementsInterface(Collector::class)
             || $classReflection->implementsInterface(Rule::class);

rules.neon

@@ -144,13 +144,21 @@ services:
         arguments:
             memberUsageExcluders: tagged(shipmonk.deadCode.memberUsageExcluder)
 
+    -
+        class: ShipMonk\PHPStan\DeadCode\Collector\PropertyAccessCollector
+        tags:
+            - phpstan.collector
+        arguments:
+            memberUsageExcluders: tagged(shipmonk.deadCode.memberUsageExcluder)
+
     -
         class: ShipMonk\PHPStan\DeadCode\Collector\ClassDefinitionCollector
         tags:
             - phpstan.collector
         arguments:
             detectDeadConstants: %shipmonkDeadCode.detect.deadConstants%
             detectDeadEnumCases: %shipmonkDeadCode.detect.deadEnumCases%
+            detectDeadProperties: %shipmonkDeadCode.detect.deadProperties%
 
     -
         class: ShipMonk\PHPStan\DeadCode\Collector\ProvidedUsagesCollector
@@ -175,7 +183,6 @@ services:
             servicesWithOldTag: tagged(shipmonk.deadCode.entrypointProvider)
             trackMixedAccessParameterValue: %shipmonkDeadCode.trackMixedAccess%
 
-
 parameters:
     parametersNotInvalidatingCache:
         - parameters.shipmonkDeadCode.debug.usagesOf
@@ -187,6 +194,7 @@ parameters:
             deadMethods: true
             deadConstants: true
             deadEnumCases: false
+            deadProperties: false
         usageProviders:
             apiPhpDoc:
                 enabled: true
@@ -238,6 +246,7 @@ parametersSchema:
             deadMethods: bool()
             deadConstants: bool()
             deadEnumCases: bool()
+            deadProperties: bool()
         ])
         usageProviders: structure([
             apiPhpDoc: structure([

src/Collector/ClassDefinitionCollector.php

@@ -4,6 +4,7 @@
 
 use LogicException;
 use PhpParser\Node;
+use PhpParser\Node\Expr\Variable;
 use PhpParser\Node\Stmt\Class_;
 use PhpParser\Node\Stmt\ClassLike;
 use PhpParser\Node\Stmt\Enum_;
@@ -21,13 +22,15 @@
 use function array_fill_keys;
 use function array_map;
 use function count;
+use function is_string;
 
 /**
  * @implements Collector<ClassLike, array{
  *       kind: string,
  *       name: string,
  *       cases: array<string, array{line: int}>,
  *       constants: array<string, array{line: int}>,
+ *       properties: array<string, array{line: int}>,
  *       methods: array<string, array{line: int, params: int, abstract: bool, visibility: int-mask-of<Visibility::*>}>,
  *       parents: array<string, null>,
  *       traits: array<string, array{excluded?: list<string>, aliases?: array<string, string>}>,
@@ -43,15 +46,19 @@ final class ClassDefinitionCollector implements Collector
 
     private bool $detectDeadEnumCases;
 
+    private bool $detectDeadProperties;
+
     public function __construct(
         ReflectionProvider $reflectionProvider,
         bool $detectDeadConstants,
-        bool $detectDeadEnumCases
+        bool $detectDeadEnumCases,
+        bool $detectDeadProperties
     )
     {
         $this->reflectionProvider = $reflectionProvider;
         $this->detectDeadConstants = $detectDeadConstants;
         $this->detectDeadEnumCases = $detectDeadEnumCases;
+        $this->detectDeadProperties = $detectDeadProperties;
     }
 
     public function getNodeType(): string
@@ -66,6 +73,7 @@ public function getNodeType(): string
      *      name: string,
      *      cases: array<string, array{line: int}>,
      *      constants: array<string, array{line: int}>,
+     *      properties: array<string, array{line: int}>,
      *      methods: array<string, array{line: int, params: int, abstract: bool, visibility: int-mask-of<Visibility::*>}>,
      *      parents: array<string, null>,
      *      traits: array<string, array{excluded?: list<string>, aliases?: array<string, string>}>,
@@ -88,40 +96,69 @@ public function processNode(
         $methods = [];
         $constants = [];
         $cases = [];
+        $properties = [];
 
         foreach ($node->getMethods() as $method) {
-            $methods[$method->name->toString()] = [
+            $methodName = $method->name->toString();
+            $methods[$methodName] = [
                 'line' => $method->name->getStartLine(),
                 'params' => count($method->params),
                 'abstract' => $method->isAbstract() || $node instanceof Interface_,
                 'visibility' => $method->flags & (Visibility::PUBLIC | Visibility::PROTECTED | Visibility::PRIVATE),
             ];
-        }
 
-        if ($this->detectDeadConstants) {
-            foreach ($node->getConstants() as $constant) {
-                foreach ($constant->consts as $const) {
-                    $constants[$const->name->toString()] = [
-                        'line' => $const->getStartLine(),
-                    ];
+            if ($methodName === '__construct') {
+                foreach ($method->getParams() as $param) {
+                    if ($param->isPromoted() && $param->var instanceof Variable && is_string($param->var->name)) {
+                        $properties[$param->var->name] = [
+                            'line' => $param->var->getStartLine(),
+                        ];
+                    }
                 }
             }
         }
 
-        if ($this->detectDeadEnumCases) {
-            foreach ($this->getEnumCases($node) as $case) {
-                $cases[$case->name->toString()] = [
-                    'line' => $case->name->getStartLine(),
+        foreach ($node->getConstants() as $constant) {
+            foreach ($constant->consts as $const) {
+                $constants[$const->name->toString()] = [
+                    'line' => $const->getStartLine(),
+                ];
+            }
+        }
+
+        foreach ($this->getEnumCases($node) as $case) {
+            $cases[$case->name->toString()] = [
+                'line' => $case->name->getStartLine(),
+            ];
+        }
+
+        foreach ($node->getProperties() as $property) {
+            foreach ($property->props as $prop) {
+                $properties[$prop->name->toString()] = [
+                    'line' => $prop->getStartLine(),
                 ];
             }
         }
 
+        if (!$this->detectDeadConstants) {
+            $constants = [];
+        }
+
+        if (!$this->detectDeadEnumCases) {
+            $cases = [];
+        }
+
+        if (!$this->detectDeadProperties) {
+            $properties = [];
+        }
+
         return [
             'kind' => $kind,
             'name' => $typeName,
             'methods' => $methods,
             'cases' => $cases,
             'constants' => $constants,
+            'properties' => $properties,
             'parents' => $this->getParents($reflection),
             'traits' => $this->getTraits($node),
             'interfaces' => $this->getInterfaces($reflection),