league/flysystem upgrade set not working

[lulco]

Bug Report

Subject	Details
Rector version	e.g. v0.11.8

Upgrade set for league/flystem not working. I believe the reason is that class Filesystem has a parent FilesystemInterface and that's why rename method rules are not executed.

Minimal PHP Code Causing Issue

https://getrector.org/demo/54879889-2385-4f0f-a150-0010face86e2 - not working https://getrector.org/demo/23bf620b-6a00-4f99-8485-937135df0388 - working

The only change is that in second demo Filesystem has no parent.

Is there any reason why RenameMethodRector checks if configured class has an interface or parent class?

https://github.com/rectorphp/rector-src/blob/1913747f014f5b4663dec6b32148a8d5046a74a5/rules/Renaming/Rector/MethodCall/RenameMethodRector.php#L86

Expected Behaviour

Methods should be renamed also if the rector config contains non-top class or interface

[TomasVotruba]

Hi, looking at the set this behavior is correct, because that would break parent interface contract. See: https://3v4l.org/9bYf6

The interface should be probably included in the set too.

[lulco]

Yeah I understand the current behavior, just thinking about it. Imho it should rename only usage not the declaration. Because the declaration is renamed in package itself. I remember some issue about the same thing in rename class rector.

But if you are not considering to change the bahavior, I'll prepare update for flysystem upgrade set and change Filesystem to FilesystemInterface

[TomasVotruba]

I haven't come across such troubles, so not sure if I'm the right person to decide. What would you suggest?

[lulco]

I don't want to create more troubles here :) I've prepared fix

[lulco]

I'm just thinking: how can we test sets like this? Require whole library to rector or create some stubs? Will tests work with stubs?

[TomasVotruba]

I usually decide depending how much needs to be stubbed. E.g. when I tested @ORM\Entity annotation for final classes, requiring whole Doctrine seemed to much, so I made a stub.

[lulco]

And is it possible to create and use multiple stubs for the same library? One for version 1.x, second for version 2.x etc. For example when I want to test upgrade to 2.x and also to 3.x I need both stubs. But in rector there are php files autoloded by classmap, so if there are same classes in both versions (but with different methods), I will probably get Fatal Error Cannot Redeclare Class, right?

[TomasVotruba]

I haven't tried that yet. To test configurable rule, the class can be named anything though. Not exactly the same as original.

[lulco]

Well, yes. That's why these bugs are happen :) you can also omit parent or interface and you are testing something else like you want :)