Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove buggy rules that depend on false condition of all class and calls analysis, to improve static reflection #5958

Merged
merged 15 commits into from
Mar 23, 2021

Conversation

TomasVotruba
Copy link
Member

@TomasVotruba TomasVotruba commented Mar 22, 2021

Due to static reflection in Rector 0.10 #3490, the autoloading of whole project is not require anymore. On the other hand, some rules required full composer autoload of all classes.

These rules attempted to detect unused public methods, yet it's hard to determine if that is true or not. E.g. in case of magic methods without parent contract interface, Doctrine entities, controller/presenter methods, template calls of objects etc. Magical limit is too high.

This caused false positives on removed methods, privatized constants od method that were actually used.
Another bad side effect was slow performance, as whole project had to be analysed with clear-cache. In the end we didn't even used this rules on Rector itself.

One more problem: these rules cripled parallel run idea, as big amount of metadata had to be passed between runs, compared to other rules that were independent on global context of the project.

Saying that, this is out of Rector scope. It's a place for advanced too, another package or even private service.

This is concerned ~10 rules. To avoid more problems caused to Rector users, we remove them.

Feel free to re-use them, making own packages or wrapper tool. It has a potential.


Similar tool doing similar work phpdcd was deprecated couple of years go, possibly due to similar "too-magical" reason: https://github.com/sebastianbergmann/phpdcd


PHPStorm does much better job in this matter, suggesting dead methods right in the code. It's also easier to verify it there by looking for a string name of the method.

image

See Related Sources

@TomasVotruba
Copy link
Member Author

cc @JanMikes This will make paralel much easier to implement :)

@TomasVotruba TomasVotruba force-pushed the no-zero branch 3 times, most recently from 18bf84e to 2300f3e Compare March 23, 2021 20:08
@TomasVotruba TomasVotruba changed the title no zero Remove buggy rules that depend on false condition of all class and calls analysis, to improve static reflection Mar 23, 2021
*/
interface ZeroCacheRectorInterface
{
}
Copy link
Member Author

Choose a reason for hiding this comment

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

@mssimi Hey, we've finally cleanup the broken dead-code rules :) thanks for the interface, it's easier to find them now

@TomasVotruba TomasVotruba force-pushed the no-zero branch 3 times, most recently from 4531059 to 3138e6c Compare March 23, 2021 21:57
@TomasVotruba TomasVotruba merged commit 27e9ad0 into main Mar 23, 2021
@TomasVotruba TomasVotruba deleted the no-zero branch March 23, 2021 22:13
@TomasVotruba
Copy link
Member Author

Ref #2971

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.

1 participant