Skip to content

Conversation

@alfredbez
Copy link
Contributor

This is an updated version of #2332

Is it possible to define parent and child classes somehow in the test fixtures? That would make it possible to define some scenarios for non-final classes or final classes that extend a class. Maybe we can merge this (simple scenario) as a first step and I will have a look at how we can have a more sophisticated rule with config options afterward.

@alfredbez alfredbez force-pushed the replace-static-with-self-for-private-constants branch from 41937e9 to 6649307 Compare December 9, 2022 17:06
@alfredbez
Copy link
Contributor Author

If you prefer the diff from the previous MR have a look into this:

Diff
diff --git a/rules-tests/CodeQuality/Rector/ClassConstFetch/ConvertStaticPrivateConstantToSelfRector/ConvertStaticPrivateConstantToSelfRectorTest.php b/rules-tests/CodeQuality/Rector/ClassConstFetch/ConvertStaticPrivateConstantToSelfRector/ConvertStaticPrivateConstantToSelfRectorTest.php
index 49afc206b5..feb5921a44 100644
--- a/rules-tests/CodeQuality/Rector/ClassConstFetch/ConvertStaticPrivateConstantToSelfRector/ConvertStaticPrivateConstantToSelfRectorTest.php
+++ b/rules-tests/CodeQuality/Rector/ClassConstFetch/ConvertStaticPrivateConstantToSelfRector/ConvertStaticPrivateConstantToSelfRectorTest.php
@@ -6,18 +6,20 @@
 
 use Iterator;
 use Rector\Testing\PHPUnit\AbstractRectorTestCase;
-use Symplify\SmartFileSystem\SmartFileInfo;
 
 final class ConvertStaticPrivateConstantToSelfRectorTest extends AbstractRectorTestCase
 {
     /**
      * @dataProvider provideData()
      */
-    public function test(SmartFileInfo $fileInfo): void
+    public function test(string $filePath): void
     {
-        $this->doTestFileInfo($fileInfo);
+        $this->doTestFile($filePath);
     }
 
+    /**
+     * @return Iterator<array<string>>
+     */
     public function provideData(): Iterator
     {
         return $this->yieldFilesFromDirectory(__DIR__ . '/Fixture');
diff --git a/rules-tests/CodeQuality/Rector/ClassConstFetch/ConvertStaticPrivateConstantToSelfRector/Fixture/const-in-non-final-class-will-not-be-changed.php.inc b/rules-tests/CodeQuality/Rector/ClassConstFetch/ConvertStaticPrivateConstantToSelfRector/Fixture/const-in-non-final-class-will-not-be-changed.php.inc
new file mode 100644
index 0000000000..4ec84989c7
--- /dev/null
+++ b/rules-tests/CodeQuality/Rector/ClassConstFetch/ConvertStaticPrivateConstantToSelfRector/Fixture/const-in-non-final-class-will-not-be-changed.php.inc
@@ -0,0 +1,13 @@
+<?php
+
+namespace Utils\Rector\Tests\Rector\UseDateTimeImmutableRector\Fixture;
+
+class Foo
+{
+    private const BAR = 1;
+    public function baz(): void
+    {
+        echo static::BAR;
+    }
+}
+?>
diff --git a/rules/CodeQuality/Rector/ClassConstFetch/ConvertStaticPrivateConstantToSelfRector.php b/rules/CodeQuality/Rector/ClassConstFetch/ConvertStaticPrivateConstantToSelfRector.php
index 2b7adc59d7..e61bbb7b60 100644
--- a/rules/CodeQuality/Rector/ClassConstFetch/ConvertStaticPrivateConstantToSelfRector.php
+++ b/rules/CodeQuality/Rector/ClassConstFetch/ConvertStaticPrivateConstantToSelfRector.php
@@ -12,6 +12,7 @@
 use PHPStan\Analyser\Scope;
 use PHPStan\Reflection\ClassReflection;
 use Rector\Core\Rector\AbstractRector;
+use Rector\Core\Reflection\ReflectionResolver;
 use Rector\NodeTypeResolver\Node\AttributeKey;
 use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
 use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
@@ -23,14 +24,19 @@
  */
 final class ConvertStaticPrivateConstantToSelfRector extends AbstractRector
 {
+    public function __construct(
+        private readonly ReflectionResolver $reflectionResolver
+    ) {
+    }
+
     public function getRuleDefinition(): RuleDefinition
     {
         return new RuleDefinition(
-            'Replaces static::* access to private constants with self::*',
+            'Replaces static::* access to private constants with self::* on final classes',
             [
                 new CodeSample(
                     <<<'CODE_SAMPLE'
-class Foo {
+final class Foo {
     private const BAR = 'bar';
     public function run()
     {
@@ -38,9 +44,9 @@ public function run()
     }
 }
 CODE_SAMPLE
-,
+                    ,
                     <<<'CODE_SAMPLE'
-class Foo {
+final class Foo {
     private const BAR = 'bar';
     public function run()
     {
@@ -48,7 +54,7 @@ public function run()
     }
 }
 CODE_SAMPLE
-,
+                    ,
                 ),
             ],
         );
@@ -61,10 +67,8 @@ public function getNodeTypes(): array
 
     /**
      * @param \PhpParser\Node\Expr\ClassConstFetch $node
-     *
-     * @return \PhpParser\Node|null
      */
-    public function refactor(Node $node)
+    public function refactor(Node $node): ?ClassConstFetch
     {
         if (! $this->isUsingStatic($node)) {
             return null;
@@ -74,7 +78,7 @@ public function refactor(Node $node)
             return null;
         }
 
-        return new ClassConstFetch(new Name('self'), $node->name,);
+        return new ClassConstFetch(new Name('self'), $node->name);
     }
 
     private function isUsingStatic(ClassConstFetch $node): bool
@@ -92,10 +96,13 @@ private function isPrivateConstant(ClassConstFetch $node): bool
         if (! $scope instanceof Scope) {
             return false;
         }
-        $classReflection = $scope->getClassReflection();
+        $classReflection = $this->reflectionResolver->resolveClassReflection($node);
         if (! $classReflection instanceof ClassReflection) {
             return false;
         }
+        if (! $classReflection->isClass() || ! $classReflection->isFinal()) {
+            return false;
+        }
         $constantName = $node->name;
         if ($constantName instanceof Error) {
             return false;

@alfredbez alfredbez force-pushed the replace-static-with-self-for-private-constants branch from 6649307 to ce4b240 Compare December 12, 2022 10:06
@alfredbez alfredbez force-pushed the replace-static-with-self-for-private-constants branch from ce4b240 to 3121674 Compare December 12, 2022 11:13
@alfredbez alfredbez force-pushed the replace-static-with-self-for-private-constants branch from 3121674 to 59ccb1d Compare December 12, 2022 11:31
Copy link
Member

@samsonasik samsonasik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good 👍

@alfredbez alfredbez force-pushed the replace-static-with-self-for-private-constants branch from 59ccb1d to 0d79204 Compare December 12, 2022 12:20
@samsonasik samsonasik merged commit de00876 into rectorphp:main Dec 12, 2022
@samsonasik
Copy link
Member

Thank you @alfredbez

@alfredbez
Copy link
Contributor Author

Is it possible to define parent and child classes somehow in the test fixtures? That would make it possible to define some scenarios for non-final classes or final classes that extend a class.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants