RenameClassRector with callback support

[dorrogeray]

Hi, sorry for the delayed response. I put this together in order to try to answer question in discussion: Make RenameClassRector more flexible by supporting callbacks?:

Callback enforcing Exception suffix on classes which extend Exception:

<?php

declare(strict_types = 1);

namespace Rector\Tests\Renaming\Rector\Name\RenameClassRector\Source;

use Exception;
use PhpParser\Node\Stmt\ClassLike;
use PHPStan\Reflection\ReflectionProvider;
use Rector\NodeNameResolver\NodeNameResolver;

class EnforceExceptionSuffixCallback
{
    public function __invoke(
        ClassLike $class,
        NodeNameResolver $nodeNameResolver,
        ReflectionProvider $reflectionProvider
    ): ?string {
        $fullyQualifiedClassName = (string) $nodeNameResolver->getName($class);
        $classReflection = $reflectionProvider->getClass($fullyQualifiedClassName);
        if (! $classReflection->isSubclassOf(Exception::class)) {
            return null;
        }

        if (!str_ends_with($fullyQualifiedClassName, 'Exception')) {
            return $fullyQualifiedClassName . 'Exception';
        }

        return null;
    }
}

Callback enforcing Interface suffix on classes which are interfaces:

<?php

declare(strict_types = 1);

namespace Rector\Tests\Renaming\Rector\Name\RenameClassRector\Source;

use PhpParser\Node\Stmt\ClassLike;
use PhpParser\Node\Stmt\Interface_;
use Rector\NodeNameResolver\NodeNameResolver;

class EnforceInterfaceSuffixCallback
{
    public function __invoke(ClassLike $class, NodeNameResolver $nodeNameResolver): ?string {
        $fullyQualifiedClassName = (string) $nodeNameResolver->getName($class);
        if (
            $class instanceof Interface_ &&
            !str_ends_with($fullyQualifiedClassName, 'Interface')
        ) {
            return $fullyQualifiedClassName . 'Interface';
        }

        return null;
    }
}

In final solution the NodeNameResolver and ReflectionProvider could be injected via DI, but it did not work in the unit test out of the box so I have passed it via argument for simplicity.

I don't like the magical callbacks configuration key, but I am not sure what is the best approach here, to keep configuration BC while adding this new feature..

[dorrogeray]

Hi, please let me know if I need to make any additional changes or if the current implementation is acceptable. Thanks! 🙏

[samsonasik]

Could you add fixture and with existing configured rule test an verify rename class named __callbacks__, eg:

'__callbacks__' => 'NewSomeOtherClass'

will be renamed working ok?

it seems class named __callbacks__ is valid https://3v4l.org/TJlNv

[dorrogeray]

Could you add fixture and with existing configured rule test an verify rename class named __callbacks__, eg:

'__callbacks__' => 'NewSomeOtherClass'

will be renamed working ok?

it seems class named __callbacks__ is valid https://3v4l.org/TJlNv

Hi, good point. 👍

I have changed the value of RenameClassRector::CALLBACKS from __callbacks__ to #callbacks#, which is not a valid classname: https://3v4l.org/ZuC21

[samsonasik]

Looks good 👍

[TomasVotruba]

Thank you 👏

[TomasVotruba]

Hi @dorrogeray

I'm thinking about this rule in retrospect and it seems it's crossing border with PHPStan and IDE fetaures.
While it might be benefitical for automated fixes of architecture standard, it's leaking feature of dynamic coding to the tool.

It doesn't seem useful to public nor standard sets. Instead these should be handled in a custom Rector rule or better required by PHPStan, as probably is already :)

I wan to keep IDE features and dynamic code sepeate, as we focus mainly on strict and clear no-brainer refactorings. Morover with upcoming GPT coding that goes the opposite way.
Saying that I'll withdraw this feature to keep Rector robuts and focuses.

Despite that, I want to thank you for your input and work 🙏