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

Meta: Single git commits per rector rule #8898

Open
cweiske opened this issue Nov 15, 2024 · 2 comments
Open

Meta: Single git commits per rector rule #8898

cweiske opened this issue Nov 15, 2024 · 2 comments
Labels

Comments

@cweiske
Copy link

cweiske commented Nov 15, 2024

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
Copy link
Member

TomasVotruba commented Nov 15, 2024

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
Copy link
Author

cweiske commented Nov 15, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants