Meta: Single git commits per rector rule

[cweiske]

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

• Option to list full class names of relevant/applying rules in "process" command
• optional: Separate command to list full class names of applying rules
• Option to "process" to apply only a single rule (full class name)

---

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]

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

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]

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.

[cweiske]

I've built a shell script that creates a single git commit for each rule that rector wants to apply: https://github.com/mogic-le/rector-single-commit.

An resulting example commit log:

* 2b2a3a80 SRH-1479: Ssch\TYPO3Rector\CodeQuality\General\ExtEmConfRector (vor 4 Minuten) <Christian Weiske>
* f4187593 SRH-1479: Rector\Php83\Rector\ClassMethod\AddOverrideAttributeToOverriddenMethodsRector (vor 4 Minuten) <Christian Weiske>
* 77c08663 SRH-1479: Rector\Php81\Rector\Property\ReadOnlyPropertyRector (vor 4 Minuten) <Christian Weiske>
* ed0cffce SRH-1479: Rector\Php81\Rector\FuncCall\NullToStrictStringFuncCallArgRector (vor 4 Minuten) <Christian Weiske>
* 21d9e3b7 SRH-1479: Rector\Php80\Rector\Switch_\ChangeSwitchToMatchRector (vor 4 Minuten) <Christian Weiske>
* fdb38de6 SRH-1479: Rector\Php80\Rector\NotIdentical\StrContainsRector (vor 4 Minuten) <Christian Weiske>
* 6fdd2460 SRH-1479: Rector\Php80\Rector\Identical\StrStartsWithRector (vor 4 Minuten) <Christian Weiske>
* 68ad968c SRH-1479: Rector\Php80\Rector\Class_\ClassPropertyAssignToConstructorPromotionRector (vor 5 Minuten) <Christian Weiske>
* 49219f13 SRH-1479: Rector\Php80\Rector\Catch_\RemoveUnusedVariableInCatchRector (vor 5 Minuten) <Christian Weiske>
* 41716a34 SRH-1479: Rector\CodeQuality\Rector\ClassMethod\OptionalParametersAfterRequiredRector (vor 5 Minuten) <Christian Weiske>
* f2707863 SRH-1479: Update php version in rector config (vor 6 Minuten) <Christian Weiske>
* bf663a23 SRH-1479: Change typo3-rector extension constraints (vor 63 Minuten) <Christian Weiske>

The only thing bothering me is that the commit messages are not very helpful; the rule classes are better than nothing but not optimal.

Should rector rules themselves contain a docblock that can then be used as commit message, or do you deem that out of scope for rector?

---

The rules overview has human readable descriptions of rules: https://github.com/rectorphp/rector-src/blob/2.0.3/build/target-repository/docs/rector_rules_overview.md?plain=1
Not quite ready for commit messages, but nearly.

[cweiske]

I've added support for reading messages from .txt files. The git log looks better now:

* 0260f7de SRH-1479: Reconfigure ext_emconf.php files (vor 8 Minuten) <Christian Weiske>
* 86d8a5f7 SRH-1479: Add "#[\Override]" attribute to overridden methods (vor 8 Minuten) <Christian Weiske>
* 152c9e53 SRH-1479: Decorate read-only properties with "readonly" attribute (vor 9 Minuten) <Christian Weiske>
* 3b2a47d3 SRH-1479: Change "null" to empty string for strictly defined function arguments (vor 9 Minuten) <Christian Weiske>
* 639210c4 SRH-1479: Change switch() to match() (vor 10 Minuten) <Christian Weiske>
* 16f4f02b SRH-1479: Replace strpos() !== false and strstr() with str_contains() (vor 11 Minuten) <Christian Weiske>
* 33bbffad SRH-1479: Convert substr() to str_starts_with() (vor 11 Minuten) <Christian Weiske>
* 51ce15fe SRH-1479: Convert simple property initialization to constructor promotion (vor 12 Minuten) <Christian Weiske>
* 19dabac3 SRH-1479: Remove unused variable in catch block (vor 16 Minuten) <Christian Weiske>
* f4e58f07 SRH-1479: Move optional parameters after required ones in method signatures (vor 16 Minuten) <Christian Weiske>
* f2707863 SRH-1479: Update php version in rector config (vor 45 Minuten) <Christian Weiske>
* bf663a23 SRH-1479: Change typo3-rector extension constraints (vor 2 Stunden) <Christian Weiske>

[staabm]

I think the commit messages should indicate that they are created with rector, e.g. by starting with Rector: or even the rule short-name (to ease debugging cases when rector does something wrong)

[cweiske]

The full rule class names are in the long commit message.
But yes, the author should be Rector.

[cweiske]

I'll close this issue since we have a solution now.