AssignmentInCondition: also check ternary conditions + warning for non variable assignment

[jrfnl]

In the original PR, the conditions for ternaries were explicitly not examined by this sniff.

This PR changes that. As long as either the condition, i.e. the part before the ? is within parentheses, òr the complete ternary is within parentheses, they will now be examined and throw the appropriate errors.

I suspect this may still be a little buggy with parentheses not belonging to the ternary, but with a ternary somewhere within them allowing this sniff to trigger.

More unit test cases very welcome!

For now, I'm only pulling this in WPCS. Once the worst bugs have been removed, I will upstream this improvement to the PHPCS version as well.

This PR also enables warnings to be thrown for non-variable assignments as those are even more likely to be typos: if ( CONSTANT = 3 ){}

[GaryJones]

Taking one of the test cases:

$mode = ( $a = 'on' ) ? 'on' : 'off';

Wouldn't that comparison usually be $a == 'on' or 'on' == $a instead of the single =? If so, then my usual coding style would be to not have the parentheses:

$mode = $a == 'on' ? 'on' : 'off';

(a case not checked for yet).

Also, does ?? operator count as being a ternary?

[jrfnl]

Wouldn't that comparison usually be $a == 'on' or 'on' == $a instead of the single =?

Correct, it should be == or ===, and if it is , this sniff will not report anything anyway as it only looks for assignment operators, not comparison operators. That's the whole point of this sniff 😎

The only way for this part of the check to work with any form of reliability for ternaries is to look for the parentheses. Without them there would be no distinguishing where the condition of the ternary begins.

If so, then my usual coding style would be to not have the parentheses:

With simple comparison ternaries, you should be fine, but using parenthesis is considered advisable as you may otherwise get quite unexpected results. See:

$a = 'Hello, ' . isset( $i ) ? 'my friend ' . $i : 'unknown world'

(a case not checked for yet).

And which shouldn't be checked for by this sniff either as what it would need to detect would be:

$mode = $a = 'on' ? 'on' : 'off';
//      ^^^^^^^^

and the sniff would never be able to determine whether it is an intentional two variable assignment with 'on' ? 'on' : 'off' as the ternary or that $a = 'on' is intended as a condition.

Also, does ?? operator count as being a ternary?

It does, but the "conditional" part before the ?? is always just a variable, never a comparison, so it would be useless for this sniff to check it.

$a = $b ?? $c; // If  $b isset, $b, otherwise $c.

[JDGrimes]

$mode = $a = 'on' ? 'on' : 'off';

Worth noting that this will be picked up by Squiz.PHP.DisallowMultipleAssignments, which flags multiple assignments at the start of a line.