Incorrect behavior of RenameClassRector

[grandmaster44]

Bug Report

Subject	Details
Rector version	last dev-main
Installed as	composer dependency

Minimal PHP Code Causing Issue

See https://getrector.org/demo/224e1a17-d4f4-4d71-bafe-f1d4cf129846

<?php

final class DemoFile extends Demo
{
    /**
     * @var int
     */
    private $test = 5;
    
    public function run()
    {
        $demo = new Demo();
        return 5;

        // we never get here
        return 10;
    }
}

final class Demo {
}

final class Demo2 {
}

Responsible rules

<?php

use Rector\Config\RectorConfig;
use Rector\Php74\Rector\Property\TypedPropertyRector;
use Rector\Set\ValueObject\SetList;
use Rector\Renaming\Rector\Name\RenameClassRector;
use Rector\DeadCode\Rector\Stmt\RemoveUnreachableStatementRector;

return static function (RectorConfig $rectorConfig): void {
    $rectorConfig->skip([
        TypedPropertyRector::class => ['/*'], 
        RenameClassRector::class => ['/*']
    ]);
    
    $rectorConfig->rule(TypedPropertyRector::class);
    $rectorConfig->rule(RemoveUnreachableStatementRector::class);
    $rectorConfig->ruleWithConfiguration(RenameClassRector::class, [
        Demo::class => Demo2::class,
    ]);
};

Expected Behavior

Rector should skip RenameClassRector rule (same as TypedPropertyRector was skipped)

[samsonasik]

The change is expected, the extends Demo not changed because it is final class, if the class is not final, it will change extends and new instantion, ref :

https://getrector.org/demo/8402e93c-63be-44f0-a7bd-509f904a9061

if you want to skip the RenameClassRector, you can do skip it with provide correct path:

    $rectorConfig->skip([
        RenameClassRector::class => __DIR__ . '/app/Entity'
    ]);

[grandmaster44]

Thanks for the reply.

The problem is that I want to skip RenameClassRector rule, but event I provide a full path to the file, it isn't skipped:

https://getrector.org/demo/94c60b5b-3228-4719-8128-9cec1cf8e7dd

[samsonasik]

RenameClassRector seems use post rector, which skip by path may overlapped, if you don't need, you can skip it entirely without path: $rectorConfig->skip([ RenameClassRector::class, //... ]); Pada tanggal Kam, 25 Agu 2022 15:22, grandmaster44 ***@***.***> menulis:

…

Thanks for the reply. The problem is that I want to skip RenameClassRector rule, but event I provide a full path to the file, it isn't skipped: https://getrector.org/demo/94c60b5b-3228-4719-8128-9cec1cf8e7dd — Reply to this email directly, view it on GitHub <#7417 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADQHAEDKMQIJ252XRSVGT3V24UMXANCNFSM57R7OKHQ> . You are receiving this because you were assigned.Message ID: ***@***.***>

[samsonasik]

see https://getrector.org/demo/4b5954ac-1563-456c-9ad0-f96913f0b784 Pada tanggal Kam, 25 Agu 2022 15:36, abdul malik ikhsan < ***@***.***> menulis:

…

RenameClassRector seems use post rector, which skip by path may overlapped, if you don't need, you can skip it entirely without path: $rectorConfig->skip([ RenameClassRector::class, //... ]); Pada tanggal Kam, 25 Agu 2022 15:22, grandmaster44 < ***@***.***> menulis: > Thanks for the reply. > > The problem is that I want to skip RenameClassRector rule, but event I > provide a full path to the file, it isn't skipped: > > https://getrector.org/demo/94c60b5b-3228-4719-8128-9cec1cf8e7dd > > — > Reply to this email directly, view it on GitHub > <#7417 (comment)>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/AADQHAEDKMQIJ252XRSVGT3V24UMXANCNFSM57R7OKHQ> > . > You are receiving this because you were assigned.Message ID: > ***@***.***> >

[grandmaster44]

Skipping the RenameClassRector rule is not a solution.

I want to rename class in every file except the single one. How can I achieve this ?

[TomasVotruba]

See README: https://github.com/rectorphp/rector/blob/main/docs/how_to_ignore_rule_or_paths.md

[samsonasik]

It seems not possible, as now, as it applied in post rector as well, see ClassRenamingPostRector

rector/packages/PostRector/Rector/ClassRenamingPostRector.php

Lines 35 to 39 in dbe8618

	$oldToNewClasses = $this->renamedClassesDataCollector->getOldToNewClasses();
	if ($oldToNewClasses === []) {
	return $node;
	}
	return $this->classRenamer->renameNode($node, $oldToNewClasses);

that's executed if $this->renamedClassesDataCollector->getOldToNewClasses() not empty.

The question is why you're doing that, because changing only in specific file for RenameClassRector seems not right, because when class used, it will point different object.

[grandmaster44]

I discovered that I can skip the rule by combination of two rules (RenameClassRector and ClassRenamingPostRector)

https://getrector.org/demo/acfd37ce-819f-4248-b13c-e229c438cfa7

<?php

use Rector\Config\RectorConfig;
use Rector\Php74\Rector\Property\TypedPropertyRector;
use Rector\Set\ValueObject\SetList;
use Rector\Renaming\Rector\Name\RenameClassRector;
use Rector\DeadCode\Rector\Stmt\RemoveUnreachableStatementRector;
use Rector\PostRector\Rector\ClassRenamingPostRector;

return static function (RectorConfig $rectorConfig): void {
    $rectorConfig->skip([
        TypedPropertyRector::class => ['/rector_analyzed_file.php'],
       	RenameClassRector::class => ['/rector_analyzed_file.php'],
        ClassRenamingPostRector::class => ['/rector_analyzed_file.php'],
    ]);
    
    $rectorConfig->rule(TypedPropertyRector::class);
    $rectorConfig->rule(RemoveUnreachableStatementRector::class);
    $rectorConfig->ruleWithConfiguration(RenameClassRector::class, [
        Demo::class => Demo2::class,
    ]);
};

The reason for skipping a single file from RenameClassRector rule may be that I want to force the usage of NewMailer in new features and thus allow OldMailer to be used in old ones.

Old Feature (skip class rename):

//@deprecated
final class DeprecatedVendorMailer{}

//@deprecated
final class OldSendMailService {
    public function __construct(DeprecatedVendorMailer $mailer) {}
}

New Feature (check that NewMailer has been used - if not - rename class - from DeprecatedVendorMailer to NewVendorMailer):

final class NewVendorMailer{}

final class NewSendMailService {
    public function __construct(NewVendorMailer $mailer) {}
}

[samsonasik]

@grandmaster44 awesome 👍 , yes, it seems skip both RenameClassRector and ClassRenamingPostRector seems the way to go as they share the $this->renamedClassesDataCollector->getOldToNewClasses() data

[samsonasik]

Re-open, I will create PR to properly fix it.