rectorphp / rector

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

Meta: Single git commits per rector rule #8898

Open cweiske opened 1 week ago

cweiske commented 1 week ago

Feature Request

When upgrading old code bases, rector currently generates a single diff with changes of all rules. For a more readable and reviewable git history, it is beneficial to do commits for each single rector rule applied.

Integrating automated git commits is currently (v1.2.10) not possible and deemed out of scope: #8505. Unfortunately, it is also not possible to automate this process with rector.

Possible automation

If rector would provide the necessary features, it would be possible to automate this process:

  1. Get a list of all rules that apply to the code base
  2. Run rector with a single rule only
  3. Create a commit for the changes of each rule

1. List of all relevant rules

Currently it is possible to get a list of the short names of all applying rules:

$ ./vendor/bin/rector process --dry-run --no-progress-bar | grep '^ \* ' | sort | uniq | sed 's/^ \* //'
AddVoidReturnTypeWhereNoReturnRector
ClassConstantToSelfClassRector
DirNameFileConstantToDirConstantRector
ExtEmConfRector
LongArrayToShortArrayRector

Unfortunately, this short names do not help very much, since we need the full class in the configuration file.

rector would at least need a way to obtain the full names of the applied rules. Ideally, it would have a separate command that does not generate any diff output, but simply lists the full names of the relevant rules.

2. Run rector for a single rule

This is currently only possible by modifying the rector.php configuration file.

A feature request for an option/parameter #7366 was rejected because short rule names are ambiguous. When passing full class names, it should not be a problem to find the rule and apply only that.

New feature request: #8899.

3. Create a commit

When 1 and 2 are available, creating commits would be easily possible with external scripts. This does not need to be part of rector.

Missing features


What do you think? Would that be a feasible approach to this problem?

Would the separate command to list applying rules be better, or should we go the option-to-list-full-rule-names-in-process route?

TomasVotruba commented 1 week ago

Thanks for proposal. We've talked about such a feature for couple year now and it would be welcomed :+1:

Ideally, it would have a separate command that does not generate any diff output, but simply lists the full names of the relevant rules.

I think the --output-format=json should be able to provide FQN names. Feel free to update the JsonOutputFormatter, if not yet.

It could also get matrix of rule class + applied on files and run Rector only on those files, to make the run fast.

I'm curious about proof of concept now :)

cweiske commented 1 week ago

With --output-format=json we can extract the full rule names:

$ ./vendor/bin/rector process --dry-run --output-format=json | jq -r .file_diffs[].applied_rectors[] | sort | uniq
Rector\Php70\Rector\MethodCall\ThisCallOnStaticMethodToStaticCallRector
Rector\Php71\Rector\TryCatch\MultiExceptionCatchRector
Rector\Php74\Rector\Assign\NullCoalescingOperatorRector
Rector\Php74\Rector\Ternary\ParenthesizeNestedTernaryRector
Ssch\TYPO3Rector\CodeQuality\General\AddErrorCodeToExceptionRector
Ssch\TYPO3Rector\CodeQuality\General\ExtEmConfRector
Ssch\TYPO3Rector\CodeQuality\General\InjectMethodToConstructorInjectionRector

So indeed only feature number 2 is missing, executing a single rule! -> #8899.