switch to match on non native types

[staabm]

Bug Report

I think rector should skip cases in which the variable beeing "switch-ed" on is not natively typed.

atm rector tranforms code from switch to match, which means code which was using loose-equality before is turned into code with strict-equality.

Subject	Details
Rector version	e.g. v1.0.3

Minimal PHP Code Causing Issue

https://getrector.com/demo/d16eee48-2ca8-456c-963e-94f8e9142995

Expected Behaviour

no change

[staabm]

see different behaviour in

https://3v4l.org/NfY3V vs. https://3v4l.org/0bDjg

working on a fix.

[TomasVotruba]

Thanks for reporting. While this is technically correct, it would make rule much less practical. That's why the border here is to take switch to match, despite slightly different behavior.

Now always the values are known, as in https://github.com/rectorphp/rector-src/pull/5772/files/d61bb646707e59f2d60f7b89b8e43527c476e53e#diff-1a53e9c53d1de0ab41aa0d69433aaaf4aa6e3d7f048808d77d18c8e08391d6bf, yet match switch is clearly benefit there.

We had not bug reported on this particular case yet.

What is more useful to improve, is to check type differences where we can mitigate the risk knowingly: https://github.com/rectorphp/rector/issues/7128

[staabm]

Thanks for reporting. While this is technically correct, it would make rule much less practical.

I don't see how it can be "practical" to introduce bugs with such a rule. the rule is super risky as is. (I ran into errors today because of the wrong transformation)

I think the rule should be "correct" in the first place. relaxing it for some scenarios is something which can be added under certain circumstances, but just silently transforming code seems to be a misleading way.

[staabm]

We had not bug reported on this particular case yet.

as you pointed out with https://github.com/rectorphp/rector/issues/7128 it seems there was at least one additional report :-)

[staabm]

Now always the values are known, as in https://github.com/rectorphp/rector-src/pull/5772/files/d61bb646707e59f2d60f7b89b8e43527c476e53e#diff-1a53e9c53d1de0ab41aa0d69433aaaf4aa6e3d7f048808d77d18c8e08391d6b yet match switch is clearly benefit there.

whats the benefit of match in this case? saving 2 lines of code?

IMO the only benefit of match over switch is type-safety. and if you ignore this fact and turn everything into match, even though its not typesafe you just generate shorter code but with different behaviour

[TomasVotruba]

In short, I'd be open to solution that handles known type conflicts. Skipping all mixed types is to aggressive.

[staabm]

could you provide a example in which the switch-condition is mixed but the switch expression should still be transformed to match? I can't imagine a case in which we have type information available but the condition is still considered mixed.

[samsonasik]

@staabm when all cases cond are equal, the check should be ok, as it will compare type to type, if it by constant, and constant is mixed type, it seems the constant is not autoloaded, which got mixed, and should be resolved in autoload first.

If that need skip, then switch cond should be compared with cases cond.

[staabm]

loose comparison is way more complicated and hard to implement. its not even implemented in PHPStan yet because of its complexity.

I don't think you do rector or its users a favor with the assumption that match is the same thing as switch but just with less lines of code.

I will add this rector on my projects ignore list.