rectorphp / rector

Instant Upgrades and Automated Refactoring of any PHP 5.3+ code
https://getrector.com
MIT License
8.59k stars 680 forks source link

Add a flag for exclude rules which violates backward compatibility #7666

Closed alexander-schranz closed 1 year ago

alexander-schranz commented 1 year ago

Feature Request

I'm using rector as a code quality tool, and want to use it also inside open source projects. There are lot of helpfull rules in rector in the PHP Upgrade rules strpos -> str_starts_with and so on.

But there are also rules in it like the MixedTypeRector here: https://github.com/rectorphp/rector/blob/ff75474d25c9bfc8c11935905f9c2027b46507d9/config/set/php80.php#L57

Adding a type to a public or protected method on a not final class is a bc break. It would be nice to have some flags available to say I want to include: LevelSetList::UP_TO_PHP_81 but without any bc break.

Our rector.php mostly look like this:

rector.php ``` paths([__DIR__ . '/src', __DIR__ . '/tests']); $rectorConfig->phpstanConfig(__DIR__ . '/phpstan.neon'); // basic rules $rectorConfig->importNames(); $rectorConfig->importShortClasses(false); $rectorConfig->sets([ SetList::CODE_QUALITY, LevelSetList::UP_TO_PHP_81, ]); // symfony rules $rectorConfig->symfonyContainerPhp(__DIR__ . '/var/cache/website/dev/App_KernelDevDebugContainer.xml'); $rectorConfig->sets([ SymfonySetList::SYMFONY_CODE_QUALITY, SymfonySetList::SYMFONY_CONSTRUCTOR_INJECTION, SymfonyLevelSetList::UP_TO_SYMFONY_60, ]); // doctrine rules $rectorConfig->sets([ DoctrineSetList::DOCTRINE_CODE_QUALITY, ]); // phpunit rules $rectorConfig->sets([ PHPUnitLevelSetList::UP_TO_PHPUNIT_90, PHPUnitSetList::PHPUNIT_91, ]); // sulu rules $rectorConfig->sets([ SuluLevelSetList::UP_TO_SULU_25, ]); }; ```

Diff

Currently UP_TO_PHP_81 includes breaking changes like:

 class SomeClass
 {
-    /**
-     * @param mixed $param
-     */
-    public function run($param)
+    public function run(mixed $param)
     {
     }
 }

Which would be nice to make configurable via a flag when using rector inside libs :) I think in php-cs-fixer two rulesets exist php80migration and php80migration:risky. Maybe something similar could be included into rector. Maybe risky is the false name as it is not risky in point of view run for a project, which is the main users of rector. Maybe keep_backward_compatible flag is better name then. So maybe something like UP_TO_PHP_81_KEEP_BACKWARD_COMPATIBLE. It could also be handled as a general flag and a rule itself defines if it violates bc promise or not, or behaves differently based on that flag. Example the MixedTypeRector could still be run on final classes.

samsonasik commented 1 year ago

Mixed type is safe to apply, even it has a child, see https://3v4l.org/Qosp3

alexander-schranz commented 1 year ago

@samsonasik 🤯

Mindblown

samsonasik commented 1 year ago

Yes, it is :)