Php-cs-fixer: Fixer for logical operators?

[Anahkiasen]

Would it be possible to add a fixer that changes and and or to && and ||? Thought that was already in contrib but seems I was mistaken. Might have time for a PR later if the idea is ok?

[GrahamCampbell]

They'd be quite risky as they could totally change the code behaviour.

[gharlan]

http://php.net/manual/en/language.operators.precedence.php

[gharlan]

Example:

if ($a = foo() and bar()) {
}

// it is the same as:
if (($a = foo()) && bar()) {
}

// and not:
if ($a = foo() && bar()) {
}

[Anahkiasen]

They'd be quite risky as they could totally change the code behaviour.

Well no, if it'd be a fixer the other way around yes but not a fixer to && ||

[gharlan]

Well no, if it'd be a fixer the other way around yes but not a fixer to && ||

Hmm, look at my example..

[Anahkiasen]

No I understand your example, but my point is most of the time when people use and they don't know about the differences and fixing it to && would most of the time make the code work as the developer originally intended. The fact that the fixer would be breaking shouldn't matter, we have a lot of breaking fixers already (strict, strict_params etc)

[GrahamCampbell]

but my point is most of the time when people use and they don't know about the differences and fixing it to && would most of the time make the code work as the developer originally intended.

Waaaaa. I disagree. If people don't know the difference, they need to get a grip. We can't work on the assumption that everyone else is stupid!

[Anahkiasen]

I feel like I'm not making myself clear here, this would be a completely _optional_ fixer, in contrib. This is PHP _CS_ Fixer, a lot of CS define whether to use and/or or &&/|| so it seemed logical to me to have a fixer in that regard. We don't have to make any assumptions about the user base or anything because it _doesn't matter_, since it's optional

[gharlan]

I'm not against the fixer, but it should be marked as risky and in contrib level.
And I wouldn't use this fixer. ;)

[SpacePossum]

It would be tricky and could possible lead to troubles.
However I agree with @Anahkiasen; the new fixer would be as risky to use as the strict comparison-fixer. I would use the fixer, but I wouldn't blindly rely on the changes it made ;)

[soullivaneuh]

:+1: For the fixer.

@gharlan According to your sample, could be possible for the fixer to adapt condition?

I mean, before fixer:

if ($a = foo() and bar()) {
}

After fixer:

if (($a = foo()) && bar()) {
}

Should be possible IMHO. We just have to found all rules for not be wrong.

And if we can do this, the fixer will not be risky at all.

[keradus]

@Soullivaneuh I'm really looking forward for your solution in PR ;)

[gharlan]

Me too. And yes, could be possible. :)

http://php.net/manual/en/language.operators.precedence.php
You have to pay extra attention to all operators between &&, || and and, or:

• all assignment operators, example: $a = foo() and bar()
• ternary operator, example: $a and $b ? $c : $d
• combinations of xor and or, because xor is between or and ||, example: $a or $b xor $c

[Anahkiasen]

Again I didn't ask for the fixer to not be risky, we have tons of fixers that can screw up your codebase way worse than that, I just asked for a quick way to batch fix it even if it means I have to review the code before committing, like a lot of the fixers we have.

[soullivaneuh]

@keradus I could start a WIP with unit test cases. For the fixer, will do it when I will got some time but it could be a good start for developers that want to participate.

[keradus]

Any news on that @Soullivaneuh ? It would be really great to have such a fixer done. It was aked for few times already.

[soullivaneuh]

Not a lot of time for this at the moment.

Maybe @Anahkiasen?

[dkarlovi]

:+1: for this fixer, I'm totally OK with it being marked as risky as it obviously should be.

[hkdobrev]

I have given the fixer a go in #2577. Please check it out and let me know if it fulfils your needs. It does work for my case.