Errors are only fixed partially

[kkmuffme]

Bug Report

Errors are only partially fixed and require x reruns to auto-fix everything.

Subject	Details
Rector version	e.g. v0.14

I know I can just loop over it myself in CI until no more fixable issues.

However it would be great if rector, like e.g. phpcbf, would internally re-run until:

• no more fixable changes
• no changes applied anymore (despite fixable) - not sure if this scenario exists in rector (it does in phpcbf, where rules conflict and undo/revert it eachother)
• limit of X runs reached (I think phpcbf has a limit of 50)

[samsonasik]

Could you narrow it down to specific rules and the code that cause it? For multiple rules applied, you can create failing test case PR to
https://github.com/rectorphp/rector-src/tree/main/tests/Issues

You can copy and modify on of the tests

[kkmuffme]

It seems this is a general issue that can happen with many rules.
Here's 2 examples:

SplitDoubleAssignRector

$user_zip = $user_city = $user_state = $user_keywords = $user_focus = $user_remarks = $user_monetized = $user_importance_scale = $user_country = $user_affiliate_coupon = '';

It always only splits off 1 multi-assignment. This means I have to run it x - 1 times where x is the number of variables assigned in this line.

---

Another one:
NewlineAfterStatementRector
=> the newline is only applied to changes done by rector on the next run in some (?) cases.

Applied rules:
 * CommonNotEqualRector (https://stackoverflow.com/a/4294663/1348344)
 * NewlineAfterStatementRector
 * UseClassKeywordForClassNameResolutionRector

---

Furthermore:
In the diff only "NewlineAfterStatementRector" is applied. The others weren't applied at all, since there was not "<>" operator or "::class" in it at all.

Exact same issue here:

``

• CommonNotEqualRector (https://stackoverflow.com/a/4294663/1348344)
• SplitDoubleAssignRector
• UseClassKeywordForClassNameResolutionRector

    
Only SplitDoubleAssignRector was actually applied, the other 2 rules don't make any sense given the diff snippet.
These 2 other rules will show until "SplitDoubleAssignRector" has been fixed for all assignments in the line, by calling rector multiple times.

---

It starts off like:

6/26 [▓▓▓▓▓▓░░░░░░░░░░░░░░░░░░░░░░] 23%�[1G�[2K 26/26 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%
24 files with changes

5/25 [▓▓▓▓▓░░░░░░░░░░░░░░░░░░░░░░░] 20%�[1G�[2K 25/25 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%
10 files with changes

11/11 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%
4 files with changes

5/5 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%
2 files with changes

3/3 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%
2 files with changes

3/3 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%
1 file with changes

2/2 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%
1 file with changes

...
repeat for X times until all SplitDoubleAssignRector are fixed

What exactly does this partially filled progress bar mean in the first 2 runs?

[kkmuffme]

Fyi this also happens when using $rectorConfig->disableParallel();, so it's unrelated to that.

[kkmuffme]

I now checked the code rector creates once and realized it's even dumber than this.
It splits it up one by one, but in the end we have each variable declared multiple times.

e.g.

$foo = $bar = $hello = $world = '';

After having to run x times until "Rector is done", rector has created this 😂

$foo = '';
$world = '';
$hello = '';
$world = '';
$bar = '';
$world = '';
$hello = '';
$world = '';

[samsonasik]

For NewlineAfterStatementRector, that's expected since AST doesn't know about spaces so the space need to be applied first so multiple run is expected when combined with other rules.

For SplitDoubleAssignRector, seems a bug, please create failing test case PR for it, you can start from https://getrector.org/demo, fill the sample and config, click "Process", then click "Create a Test" button if you expect different result.

[kkmuffme]

For NewlineAfterStatementRector, that's expected since AST doesn't know about spaces so the space need to be applied first so multiple run is expected when combined with other rules.

Expected by who? This isn't documented anywhere.
All other auto-fix tools (phpcbf, psalter) will rerun internally until all fixable issues are fixed. Therefore, this is definitely unexpected.

For SplitDoubleAssignRector, seems a bug, please create failing test case PR for it, you can start from https://getrector.org/demo, fill the sample and config, click "Process", then click "Create a Test" button if you expect different result.

This is the most basic case for this rule. Not sure if slapping on another test is the way to go, I think existing (if any) tests should be revisited here, and the rule fixed.
https://getrector.org/demo/29717998-4b0e-4cde-bfaf-e73fa0de3593

This rule atm works exactly for 1 case, where you have exactly 2 variables:
https://getrector.org/demo/22e389bd-4614-4635-929d-59d9f03e3bf6

[TomasVotruba]

@kkmuffme I recommend not to discuss 2 rules at a time, as now the conversations splits into 2 rules and got pretty confusing.

Closing to avoud more confusion. Please create a separate issue for the single rule you want to talk about first. So we can have focused discussion and solve it 👍