feat: allow user to skip privatization for specific methods/properties

Author: calebdw

rules-tests/Privatization/Rector/ClassMethod/PrivatizeFinalClassMethodRector/CallbackTest.php

@@ -0,0 +1,28 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Tests\Privatization\Rector\ClassMethod\PrivatizeFinalClassMethodRector;
+
+use Iterator;
+use PHPUnit\Framework\Attributes\DataProvider;
+use Rector\Testing\PHPUnit\AbstractRectorTestCase;
+
+final class CallbackTest extends AbstractRectorTestCase
+{
+    #[DataProvider('provideData')]
+    public function test(string $filePath): void
+    {
+        $this->doTestFile($filePath);
+    }
+
+    public static function provideData(): Iterator
+    {
+        return self::yieldFilesFromDirectory(__DIR__ . '/FixtureCallback');
+    }
+
+    public function provideConfigFilePath(): string
+    {
+        return __DIR__ . '/config/configured_rule_callback.php';
+    }
+}

rules-tests/Privatization/Rector/ClassMethod/PrivatizeFinalClassMethodRector/FixtureCallback/callback_can_skip.php.inc

@@ -0,0 +1,25 @@
+<?php
+
+namespace Rector\Tests\Privatization\Rector\ClassMethod\PrivatizeFinalClassMethodRector\Fixture;
+
+final class FinalClassWithProtectedMethods
+{
+    protected function someMethod() {}
+
+    protected function shouldBeProtected() {}
+}
+
+?>
+-----
+<?php
+
+namespace Rector\Tests\Privatization\Rector\ClassMethod\PrivatizeFinalClassMethodRector\Fixture;
+
+final class FinalClassWithProtectedMethods
+{
+    private function someMethod() {}
+
+    protected function shouldBeProtected() {}
+}
+
+?>

rules-tests/Privatization/Rector/ClassMethod/PrivatizeFinalClassMethodRector/config/configured_rule_callback.php

@@ -0,0 +1,23 @@
+<?php
+
+declare(strict_types=1);
+
+use PHPStan\Reflection\ClassReflection;
+use PhpParser\Node\Stmt\ClassMethod;
+use Rector\Config\RectorConfig;
+use Rector\Privatization\Rector\ClassMethod\PrivatizeFinalClassMethodRector;
+
+return RectorConfig::configure()
+    ->withConfiguredRule(PrivatizeFinalClassMethodRector::class, [
+        PrivatizeFinalClassMethodRector::SHOULD_SKIP_CALLBACK => static function (
+            ClassMethod $classMethod,
+            ClassReflection $classReflection,
+        ): bool {
+            if (! str_contains($classReflection->getName(), 'FinalClassWithProtectedMethods')) {
+                return false;
+            }
+
+
+            return str_contains($classMethod->name->toString(), 'shouldBeProtected');
+        },
+    ]);

rules-tests/Privatization/Rector/Property/PrivatizeFinalClassPropertyRector/Fixture/callback_can_skip.php.inc

@@ -0,0 +1,33 @@
+<?php
+
+namespace Rector\Tests\Privatization\Rector\Property\PrivatizeFinalClassPropertyRector\Fixture;
+
+final class FinalClassWithProtectedProperties
+{
+    protected string $value;
+    protected string $shouldBeProtected;
+
+    public function __construct(
+        protected string $bar,
+        protected string $shouldBeProtectedAlso,
+    ) {}
+}
+
+?>
+-----
+<?php
+
+namespace Rector\Tests\Privatization\Rector\Property\PrivatizeFinalClassPropertyRector\Fixture;
+
+final class FinalClassWithProtectedProperties
+{
+    private string $value;
+    protected string $shouldBeProtected;
+
+    public function __construct(
+        private string $bar,
+        protected string $shouldBeProtectedAlso,
+    ) {}
+}
+
+?>

rules-tests/Privatization/Rector/Property/PrivatizeFinalClassPropertyRector/config/configured_rule.php

@@ -2,8 +2,25 @@
 
 declare(strict_types=1);
 
+use PhpParser\Node\Stmt\Property;
+use PHPStan\Reflection\ClassReflection;
 use Rector\Config\RectorConfig;
 use Rector\Privatization\Rector\Property\PrivatizeFinalClassPropertyRector;
 
 return RectorConfig::configure()
-    ->withRules([PrivatizeFinalClassPropertyRector::class]);
+    ->withConfiguredRule(PrivatizeFinalClassPropertyRector::class, [
+        PrivatizeFinalClassPropertyRector::SHOULD_SKIP_CALLBACK => static function (
+            Property|string $property,
+            ClassReflection $classReflection,
+        ): bool {
+            if (! str_contains($classReflection->getName(), 'FinalClassWithProtectedProperties')) {
+                return false;
+            }
+
+            $name = $property instanceof Property
+                ? (string) $property->props[0]->name
+                : $property;
+
+            return str_contains($name, 'shouldBeProtected');
+        },
+    ]);

rules/Privatization/Rector/ClassMethod/PrivatizeFinalClassMethodRector.php

@@ -9,20 +9,33 @@
 use PhpParser\Node\Stmt\Class_;
 use PhpParser\Node\Stmt\ClassMethod;
 use PHPStan\Reflection\ClassReflection;
+use Rector\Contract\Rector\ConfigurableRectorInterface;
 use Rector\PhpParser\Node\BetterNodeFinder;
 use Rector\PHPStan\ScopeFetcher;
 use Rector\Privatization\Guard\OverrideByParentClassGuard;
 use Rector\Privatization\NodeManipulator\VisibilityManipulator;
 use Rector\Privatization\VisibilityGuard\ClassMethodVisibilityGuard;
 use Rector\Rector\AbstractRector;
-use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
+use Symplify\RuleDocGenerator\ValueObject\CodeSample\ConfiguredCodeSample;
 use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
 
 /**
  * @see \Rector\Tests\Privatization\Rector\ClassMethod\PrivatizeFinalClassMethodRector\PrivatizeFinalClassMethodRectorTest
+ * @see \Rector\Tests\Privatization\Rector\ClassMethod\PrivatizeFinalClassMethodRector\CallbackTest
  */
-final class PrivatizeFinalClassMethodRector extends AbstractRector
+final class PrivatizeFinalClassMethodRector extends AbstractRector implements ConfigurableRectorInterface
 {
+    /**
+     * @api
+     * @var string
+     */
+    public const SHOULD_SKIP_CALLBACK = 'should_skip_callback';
+
+    /**
+     * @var ?callable(ClassMethod, ClassReflection): bool
+     */
+    private $shouldSkipCallback = null;
+
     public function __construct(
         private readonly ClassMethodVisibilityGuard $classMethodVisibilityGuard,
         private readonly VisibilityManipulator $visibilityManipulator,
@@ -36,8 +49,14 @@ public function getRuleDefinition(): RuleDefinition
         return new RuleDefinition(
             'Change protected class method to private if possible',
             [
-                new CodeSample(
+                new ConfiguredCodeSample(
                     <<<'CODE_SAMPLE'
+final class SomeOtherClass
+{
+    protected function someMethod()
+    {
+    }
+}
 final class SomeClass
 {
     protected function someMethod()
@@ -47,18 +66,41 @@ protected function someMethod()
 CODE_SAMPLE
                     ,
                     <<<'CODE_SAMPLE'
+final class SomeOtherClass
+{
+    protected function someMethod()
+    {
+    }
+}
 final class SomeClass
 {
     private function someMethod()
     {
     }
 }
 CODE_SAMPLE
+                    ,
+                    [
+                        self::SHOULD_SKIP_CALLBACK => static function (
+                            ClassMethod $classMethod,
+                            ClassReflection $classReflection,
+                        ): bool {
+                            return $classReflection->is('SomeOtherClass');
+                        },
+                    ],
                 ),
             ]
         );
     }
 
+    /**
+     * @param mixed[] $configuration
+     */
+    public function configure(array $configuration): void
+    {
+        $this->shouldSkipCallback = $configuration[self::SHOULD_SKIP_CALLBACK] ?? null;
+    }
+
     /**
      * @return array<class-string<Node>>
      */
@@ -107,6 +149,13 @@ public function refactor(Node $node): ?Node
                 continue;
             }
 
+            if (
+                is_callable($this->shouldSkipCallback)
+                && call_user_func($this->shouldSkipCallback, $classMethod, $classReflection)
+            ) {
+                continue;
+            }
+
             $this->visibilityManipulator->makePrivate($classMethod);
             $hasChanged = true;
         }

rules/Privatization/Rector/Property/PrivatizeFinalClassPropertyRector.php

@@ -9,20 +9,32 @@
 use PhpParser\Node\Stmt\ClassMethod;
 use PhpParser\Node\Stmt\Property;
 use PHPStan\Reflection\ClassReflection;
+use Rector\Contract\Rector\ConfigurableRectorInterface;
 use Rector\Privatization\Guard\ParentPropertyLookupGuard;
 use Rector\Privatization\NodeManipulator\VisibilityManipulator;
 use Rector\Rector\AbstractRector;
 use Rector\Reflection\ReflectionResolver;
 use Rector\ValueObject\MethodName;
 use Rector\ValueObject\Visibility;
-use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
+use Symplify\RuleDocGenerator\ValueObject\CodeSample\ConfiguredCodeSample;
 use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
 
 /**
  * @see \Rector\Tests\Privatization\Rector\Property\PrivatizeFinalClassPropertyRector\PrivatizeFinalClassPropertyRectorTest
  */
-final class PrivatizeFinalClassPropertyRector extends AbstractRector
+final class PrivatizeFinalClassPropertyRector extends AbstractRector implements ConfigurableRectorInterface
 {
+    /**
+     * @api
+     * @var string
+     */
+    public const SHOULD_SKIP_CALLBACK = 'should_skip_callback';
+
+    /**
+     * @var ?callable(Property|string, ClassReflection): bool
+     */
+    private $shouldSkipCallback = null;
+
     public function __construct(
         private readonly VisibilityManipulator $visibilityManipulator,
         private readonly ParentPropertyLookupGuard $parentPropertyLookupGuard,
@@ -33,24 +45,49 @@ public function __construct(
     public function getRuleDefinition(): RuleDefinition
     {
         return new RuleDefinition('Change property to private if possible', [
-            new CodeSample(
+            new ConfiguredCodeSample(
                 <<<'CODE_SAMPLE'
+final class SomeOtherClass
+{
+    protected $value;
+}
 final class SomeClass
 {
     protected $value;
 }
 CODE_SAMPLE
                 ,
                 <<<'CODE_SAMPLE'
+final class SomeOtherClass
+{
+    protected $value;
+}
 final class SomeClass
 {
     private $value;
 }
 CODE_SAMPLE
+                ,
+                [
+                    self::SHOULD_SKIP_CALLBACK => static function (
+                        Property|string $property,
+                        ClassReflection $classReflection,
+                    ): bool {
+                        return $classReflection->is('SomeOtherClass');
+                    },
+                ],
             ),
         ]);
     }
 
+    /**
+     * @param mixed[] $configuration
+     */
+    public function configure(array $configuration): void
+    {
+        $this->shouldSkipCallback = $configuration[self::SHOULD_SKIP_CALLBACK] ?? null;
+    }
+
     /**
      * @return array<class-string<Node>>
      */
@@ -84,6 +121,13 @@ public function refactor(Node $node): ?Node
                 continue;
             }
 
+            if (
+                is_callable($this->shouldSkipCallback)
+                && call_user_func($this->shouldSkipCallback, $property, $classReflection)
+            ) {
+                continue;
+            }
+
             $this->visibilityManipulator->makePrivate($property);
             $hasChanged = true;
         }
@@ -99,7 +143,16 @@ public function refactor(Node $node): ?Node
                     continue;
                 }
 
-                if (! $this->parentPropertyLookupGuard->isLegal((string) $this->getName($param), $classReflection)) {
+                $property = (string) $this->getName($param);
+
+                if (! $this->parentPropertyLookupGuard->isLegal($property, $classReflection)) {
+                    continue;
+                }
+
+                if (
+                    is_callable($this->shouldSkipCallback)
+                    && call_user_func($this->shouldSkipCallback, $property, $classReflection)
+                ) {
                     continue;
                 }