[DeadCode] Skip count() on mixed on RemoveUnusedNonEmptyArrayBeforeForeachRector

[samsonasik]

Fixes rectorphp/rector#8265

[samsonasik]

Fixed 🎉 /cc @staabm

[samsonasik]

@staabm while it fixed the issue, could you create a https://3v4l.org/ demo for how it can cause invalid usage without skipping?

Otherwise, the patch is useless :)

[staabm]

see https://3v4l.org/bavqQ where we get fatal errors on the count variant, but not the foreach variant

[samsonasik]

The error is exists on the first place, so removing count() actually unrelated to side effect.

see https://3v4l.org/eGBHn

when no change yet, it already fatal error.

In real app, actually, removing count() should never used when only used on loop next.

[staabm]

when no change yet, it already fatal error.

I don't get your point. IMO we can't change code which fatal errors (so from a runtime perspective will stop execution and will not proceed) with a different code which no longer fatal errors and keeps executing.

sure the code not running into fatal errors sounds better but if rector is doing such change it completely changes the code beeing executed and because of procssing is not running into a fatal error anymore it might lead to e.g. other serious problem in code after that, which was never executed before rector did the change

[samsonasik]

That seems just hiding the real issue then. If the code after that must not be executed, then code should be modified to fix that.

The TypeError check will need to be catched on the first place, and user should see that.

[staabm]

back to square one: the only thing I am saying is, that rector cannot do a change which fundamentally changes the execution path.

of course such a code should be fixed by a human.

the RemoveUnusedNonEmptyArrayBeforeForeachRector can of course remove e.g. a count() statement if it knows whatever passed to it is countable.

[samsonasik]

Ok, the rule is named RemoveUnusedNonEmptyArrayBeforeForeachRector, so it should only care about ArrayType, so your reason is reasonable.

I am merging it then ;)