Autoremove: warn about kept excluded usages (#175)

Author: janedbal

.ecrc

@@ -1,5 +1,5 @@
 {
     "Exclude": [
-        "^tests/Rule/data/debug/expected_output.txt"
+        "output.txt$"
     ]
 }

README.md

@@ -280,6 +280,12 @@ class UserFacade
 ```
 
 - If you are excluding tests usages (see above), this will not cause the related tests to be removed alongside.
+  - But you will see all those kept usages in output (with links to your IDE if you set up `editorUrl` [parameter](https://phpstan.org/user-guide/output-format#opening-file-in-an-editor))
+
+```txt
+ • Removed method UserFacade::deadMethod
+   ! Excluded usage at tests/User/UserFacadeTest.php:241 left intact
+```
 
 
 ## Calls over unknown types

rules.neon

@@ -6,11 +6,14 @@ services:
         class: ShipMonk\PHPStan\DeadCode\Hierarchy\ClassHierarchy
     -
         class: ShipMonk\PHPStan\DeadCode\Transformer\FileSystem
+    -
+        class: ShipMonk\PHPStan\DeadCode\Output\OutputEnhancer
+        arguments:
+            editorUrl: %editorUrl%
 
     -
         class: ShipMonk\PHPStan\DeadCode\Debug\DebugUsagePrinter
         arguments:
-            editorUrl: %editorUrl%
             mixedExcluderEnabled: %shipmonkDeadCode.usageExcluders.usageOverMixed.enabled%
 
     -

src/Debug/DebugUsagePrinter.php

@@ -5,7 +5,6 @@
 use LogicException;
 use PHPStan\Command\Output;
 use PHPStan\DependencyInjection\Container;
-use PHPStan\File\RelativePathHelper;
 use PHPStan\Reflection\ClassReflection;
 use PHPStan\Reflection\ReflectionProvider;
 use ReflectionException;
@@ -15,7 +14,7 @@
 use ShipMonk\PHPStan\DeadCode\Graph\ClassMemberUsage;
 use ShipMonk\PHPStan\DeadCode\Graph\ClassMethodRef;
 use ShipMonk\PHPStan\DeadCode\Graph\CollectedUsage;
-use ShipMonk\PHPStan\DeadCode\Graph\UsageOrigin;
+use ShipMonk\PHPStan\DeadCode\Output\OutputEnhancer;
 use function array_map;
 use function array_sum;
 use function array_unique;
@@ -27,18 +26,15 @@
 use function reset;
 use function sprintf;
 use function str_repeat;
-use function str_replace;
 use function strpos;
 
 class DebugUsagePrinter
 {
 
-    private RelativePathHelper $relativePathHelper;
+    private OutputEnhancer $outputEnhancer;
 
     private ReflectionProvider $reflectionProvider;
 
-    private ?string $editorUrl;
-
     /**
      * memberKey => usage info
      *
@@ -50,16 +46,14 @@ class DebugUsagePrinter
 
     public function __construct(
         Container $container,
-        RelativePathHelper $relativePathHelper,
+        OutputEnhancer $outputEnhancer,
         ReflectionProvider $reflectionProvider,
-        ?string $editorUrl,
         bool $mixedExcluderEnabled
     )
     {
-        $this->relativePathHelper = $relativePathHelper;
+        $this->outputEnhancer = $outputEnhancer;
         $this->reflectionProvider = $reflectionProvider;
         $this->mixedExcluderEnabled = $mixedExcluderEnabled;
-        $this->editorUrl = $editorUrl;
         $this->debugMembers = $this->buildDebugMemberKeys(
             // @phpstan-ignore offsetAccess.nonOffsetAccessible, offsetAccess.nonOffsetAccessible, missingType.checkedException, argument.type
             $container->getParameter('shipmonkDeadCode')['debug']['usagesOf'], // prevents https://github.com/phpstan/phpstan/issues/12740
@@ -119,7 +113,7 @@ private function getExampleCaller(array $usages): ?string
             $origin = $usage->getUsage()->getOrigin();
 
             if ($origin->getFile() !== null) {
-                return $this->getOriginReference($origin);
+                return $this->outputEnhancer->getOriginReference($origin);
             }
         }
 
@@ -148,7 +142,7 @@ public function printDebugMemberUsages(Output $output, array $analysedClasses):
 
                 foreach ($debugMember['eliminationPath'] as $fragmentKey => $fragmentUsages) {
                     if ($depth === 1) {
-                        $entrypoint = $this->getOriginReference($fragmentUsages[0]->getOrigin(), false);
+                        $entrypoint = $this->outputEnhancer->getOriginReference($fragmentUsages[0]->getOrigin(), false);
                         $output->writeLineFormatted(sprintf('| <fg=gray>entry</> <fg=white>%s</>', $entrypoint));
                     }
 
@@ -160,7 +154,7 @@ public function printDebugMemberUsages(Output $output, array $analysedClasses):
 
                     $pathFragment = $nextFragmentFirstUsageOrigin === null
                         ? $this->prettyMemberKey($fragmentKey)
-                        : $this->getOriginLink($nextFragmentFirstUsageOrigin, $this->prettyMemberKey($fragmentKey));
+                        : $this->outputEnhancer->getOriginLink($nextFragmentFirstUsageOrigin, $this->prettyMemberKey($fragmentKey));
 
                     $output->writeLineFormatted(sprintf('| %s<fg=white>%s</>', $indent, $pathFragment));
 
@@ -185,7 +179,7 @@ public function printDebugMemberUsages(Output $output, array $analysedClasses):
 
                 foreach ($debugMember['usages'] as $collectedUsage) {
                     $origin = $collectedUsage->getUsage()->getOrigin();
-                    $output->writeFormatted(sprintf('|  • <fg=white>%s</>', $this->getOriginReference($origin)));
+                    $output->writeFormatted(sprintf('|  • <fg=white>%s</>', $this->outputEnhancer->getOriginReference($origin)));
 
                     if ($collectedUsage->isExcluded()) {
                         $output->writeFormatted(sprintf(' - <fg=yellow>excluded by %s excluder</>', $collectedUsage->getExcludedBy()));
@@ -214,59 +208,6 @@ private function prettyMemberKey(string $memberKey): string
         return $replaced;
     }
 
-    private function getOriginReference(UsageOrigin $origin, bool $preferFileLine = true): string
-    {
-        $file = $origin->getFile();
-        $line = $origin->getLine();
-
-        if ($file !== null && $line !== null) {
-            $relativeFile = $this->relativePathHelper->getRelativePath($file);
-
-            $title = $origin->getClassName() !== null && $origin->getMethodName() !== null && !$preferFileLine
-                ? sprintf('%s::%s:%d', $origin->getClassName(), $origin->getMethodName(), $line)
-                : sprintf('%s:%s', $relativeFile, $line);
-
-            if ($this->editorUrl === null) {
-                return $title;
-            }
-
-            return sprintf(
-                '<href=%s>%s</>',
-                str_replace(['%file%', '%relFile%', '%line%'], [$file, $relativeFile, (string) $line], $this->editorUrl),
-                $title,
-            );
-        }
-
-        if ($origin->getProvider() !== null) {
-            $note = $origin->getNote() !== null ? " ({$origin->getNote()})" : '';
-            return 'virtual usage from ' . $origin->getProvider() . $note;
-        }
-
-        throw new LogicException('Unknown state of usage origin');
-    }
-
-    private function getOriginLink(UsageOrigin $origin, string $title): string
-    {
-        $file = $origin->getFile();
-        $line = $origin->getLine();
-
-        if ($line !== null) {
-            $title = sprintf('%s:%s', $title, $line);
-        }
-
-        if ($this->editorUrl !== null && $file !== null && $line !== null) {
-            $relativeFile = $this->relativePathHelper->getRelativePath($file);
-
-            return sprintf(
-                '<href=%s>%s</>',
-                str_replace(['%file%', '%relFile%', '%line%'], [$file, $relativeFile, (string) $line], $this->editorUrl),
-                $title,
-            );
-        }
-
-        return $title;
-    }
-
     /**
      * @param list<string> $alternativeKeys
      */

src/Error/BlackMember.php

@@ -5,6 +5,8 @@
 use LogicException;
 use ShipMonk\PHPStan\DeadCode\Graph\ClassConstantRef;
 use ShipMonk\PHPStan\DeadCode\Graph\ClassMemberRef;
+use ShipMonk\PHPStan\DeadCode\Graph\ClassMemberUsage;
+use ShipMonk\PHPStan\DeadCode\Graph\CollectedUsage;
 use ShipMonk\PHPStan\DeadCode\Rule\DeadCodeRule;
 use function array_keys;
 use function count;
@@ -20,9 +22,9 @@ final class BlackMember
     private int $line;
 
     /**
-     * @var array<string, true>
+     * @var array<string, list<ClassMemberUsage>>
      */
-    private array $excluders = [];
+    private array $excludedUsages = [];
 
     public function __construct(
         ClassMemberRef $member,
@@ -58,9 +60,15 @@ public function getLine(): int
         return $this->line;
     }
 
-    public function markHasExcludedUsage(string $excludedBy): void
+    public function addExcludedUsage(CollectedUsage $excludedUsage): void
     {
-        $this->excluders[$excludedBy] = true;
+        if (!$excludedUsage->isExcluded()) {
+            throw new LogicException('Given usage is not excluded!');
+        }
+
+        $excludedBy = $excludedUsage->getExcludedBy();
+
+        $this->excludedUsages[$excludedBy][] = $excludedUsage->getUsage();
     }
 
     public function getErrorIdentifier(): string
@@ -72,14 +80,30 @@ public function getErrorIdentifier(): string
 
     public function getExclusionMessage(): string
     {
-        if (count($this->excluders) === 0) {
+        if (count($this->excludedUsages) === 0) {
             return '';
         }
 
-        $excluderNames = implode(', ', array_keys($this->excluders));
-        $plural = count($this->excluders) > 1 ? 's' : '';
+        $excluderNames = implode(', ', array_keys($this->excludedUsages));
+        $plural = count($this->excludedUsages) > 1 ? 's' : '';
 
         return " (all usages excluded by {$excluderNames} excluder{$plural})";
     }
 
+    /**
+     * @return list<ClassMemberUsage>
+     */
+    public function getExcludedUsages(): array
+    {
+        $result = [];
+
+        foreach ($this->excludedUsages as $usages) {
+            foreach ($usages as $usage) {
+                $result[] = $usage;
+            }
+        }
+
+        return $result;
+    }
+
 }

src/Formatter/RemoveDeadCodeFormatter.php

@@ -5,19 +5,29 @@
 use PHPStan\Command\AnalysisResult;
 use PHPStan\Command\ErrorFormatter\ErrorFormatter;
 use PHPStan\Command\Output;
+use ShipMonk\PHPStan\DeadCode\Graph\ClassMemberUsage;
+use ShipMonk\PHPStan\DeadCode\Graph\UsageOrigin;
+use ShipMonk\PHPStan\DeadCode\Output\OutputEnhancer;
 use ShipMonk\PHPStan\DeadCode\Rule\DeadCodeRule;
 use ShipMonk\PHPStan\DeadCode\Transformer\FileSystem;
 use ShipMonk\PHPStan\DeadCode\Transformer\RemoveDeadCodeTransformer;
+use function array_keys;
 use function count;
 
 class RemoveDeadCodeFormatter implements ErrorFormatter
 {
 
     private FileSystem $fileSystem;
 
-    public function __construct(FileSystem $fileSystem)
+    private OutputEnhancer $outputEnhancer;
+
+    public function __construct(
+        FileSystem $fileSystem,
+        OutputEnhancer $outputEnhancer
+    )
     {
         $this->fileSystem = $fileSystem;
+        $this->outputEnhancer = $outputEnhancer;
     }
 
     public function formatErrors(
@@ -37,6 +47,7 @@ public function formatErrors(
             return 1;
         }
 
+        /** @var array<string, array<string, array<string, list<ClassMemberUsage>>>> $deadMembersByFiles file => [identifier => [key => excludedUsages[]]] */
         $deadMembersByFiles = [];
 
         foreach ($analysisResult->getFileSpecificErrors() as $fileSpecificError) {
@@ -47,32 +58,74 @@ public function formatErrors(
                 continue;
             }
 
-            /** @var array<string, array{file: string}> $metadata */
+            /** @var array<string, array{file: string, excludedUsages: list<ClassMemberUsage>}> $metadata */
             $metadata = $fileSpecificError->getMetadata();
             $type = $fileSpecificError->getIdentifier();
 
-            foreach ($metadata as $key => $data) {
-                $deadMembersByFiles[$data['file']][$type][] = $key;
+            foreach ($metadata as $memberKey => $data) {
+                $deadMembersByFiles[$data['file']][$type][$memberKey] = $data['excludedUsages'];
             }
         }
 
-        $count = 0;
+        $membersCount = 0;
+        $filesCount = count($deadMembersByFiles);
 
         foreach ($deadMembersByFiles as $file => $deadMembersByType) {
+            /** @var array<string, list<ClassMemberUsage>> $deadConstants */
             $deadConstants = $deadMembersByType[DeadCodeRule::IDENTIFIER_CONSTANT] ?? [];
+            /** @var array<string, list<ClassMemberUsage>> $deadMethods */
             $deadMethods = $deadMembersByType[DeadCodeRule::IDENTIFIER_METHOD] ?? [];
 
-            $count += count($deadConstants) + count($deadMethods);
+            $membersCount += count($deadConstants) + count($deadMethods);
 
-            $transformer = new RemoveDeadCodeTransformer($deadMethods, $deadConstants);
+            $transformer = new RemoveDeadCodeTransformer(array_keys($deadMethods), array_keys($deadConstants));
             $oldCode = $this->fileSystem->read($file);
             $newCode = $transformer->transformCode($oldCode);
             $this->fileSystem->write($file, $newCode);
+
+            foreach ($deadConstants as $constant => $excludedUsages) {
+                $output->writeLineFormatted(" • Removed constant <fg=white>$constant</>");
+                $this->printExcludedUsages($output, $excludedUsages);
+            }
+
+            foreach ($deadMethods as $method => $excludedUsages) {
+                $output->writeLineFormatted(" • Removed method <fg=white>$method</>");
+                $this->printExcludedUsages($output, $excludedUsages);
+            }
         }
 
-        $output->writeLineFormatted('Removed ' . $count . ' dead methods in ' . count($deadMembersByFiles) . ' files.');
+        $memberPlural = $membersCount === 1 ? '' : 's';
+        $filePlural = $filesCount === 1 ? '' : 's';
+
+        $output->writeLineFormatted('');
+        $output->writeLineFormatted("Removed $membersCount dead member$memberPlural in $filesCount file$filePlural.");
 
         return 0;
     }
 
+    /**
+     * @param list<ClassMemberUsage> $excludedUsages
+     */
+    private function printExcludedUsages(Output $output, array $excludedUsages): void
+    {
+        foreach ($excludedUsages as $excludedUsage) {
+            $originLink = $this->getOriginLink($excludedUsage->getOrigin());
+
+            if ($originLink === null) {
+                continue;
+            }
+
+            $output->writeLineFormatted("   ! Excluded usage at {$originLink} left intact");
+        }
+    }
+
+    private function getOriginLink(UsageOrigin $origin): ?string
+    {
+        if ($origin->getFile() === null || $origin->getLine() === null) {
+            return null;
+        }
+
+        return $this->outputEnhancer->getOriginReference($origin);
+    }
+
 }