rector-src icon indicating copy to clipboard operation
rector-src copied to clipboard

[draft] RenameClassRector with callback support

Open dorrogeray opened this issue 3 years ago • 0 comments

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?. This is not (even remotely) complete solution, but it illustrates from user perspective how such callback might look like, as well as some technical obstacles that I have stumbled upon.

Callback enforcing Exception suffix on classes which extend Exception:

<?php

declare(strict_types = 1);

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

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

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

        return null;
    }
}

In final solution the NodeNameResolver 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.

However, I have realized that there is a bigger problem with this prototype, and that is the fact that it would be necessary to somehow visit all classes and process the renaming callbacks before the actual rectifying starts, because otherwise the uses / imports of the class which will be identified via user callback will not be rectified if they are in files processed before the file which contains the actual exception declaration. This would be even bigger problem in parallel processing, where only one of the workers would actually stumble upon the class, and register it for renaming, while others would not even notice..

In other words, as it is, this solution is useless. To make it work would require two passes over the codebase. First analytical, which would visit classes, process callbacks, and identify classes for renaming, and second, which would use this dynamically generated list for the renaming..

Any ideas on how to approach this? I really don't see a solution without two passes, and that just does not seem to be worth it.. I mean, it would essentially be a pre-processing of the rule configuration, and I guess anyone can write something like that without any PR to rector itself..

dorrogeray avatar Oct 28 '22 21:10 dorrogeray