Suddenly duplicate RedundantCondition for constants

[kkmuffme]

https://psalm.dev/r/3c04ed05dc

Until recently it only reported 1 error:
ERROR: TypeDoesNotContainType - 5:28 - Operand of type false is always false

Since 4.16 (I think) it suddenly reports 2 errors for the same thing, as it now added:
ERROR: TypeDoesNotContainType - 5:6 - Type false for HELLO is always falsy

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/3c04ed05dc

<?php

define( 'HELLO', false );

if ( defined( 'HELLO' ) && HELLO ) {
    echo "world";
}

Psalm output (using commit 03b7e94):

ERROR: TypeDoesNotContainType - 5:28 - Operand of type false is always false

ERROR: TypeDoesNotContainType - 5:6 - Type false for HELLO is always falsy

[orklah]

This is more or less expected. I'll explain.

The issue was revealed by #7054. In this PR, I allowed global constants to have variable identifiers. Which means, they can now be the target of assertions

Assertions is the module Psalm use to refine the type of a variable depending on the checks that are made on it in a specific context. For example, in https://psalm.dev/r/e4129d69f0, the first value of HELLO is empty because we know it's false and we're in a context where HELLO can only be truthy, so there's no type that allows this.

Before my #7054, both branches of the conditional for this snippet showed false, which didn't make sense.

The assertion module emits an issue whenever it sees an impossible or redundant check on a variable and it works on the whole condition (that's why it reports an error pretty wide with 5:28). One downside of this module is that it only works on variables (and now constants)

On the other hand, we have an other check that just report any operand that is always truthy or always falsy, no matter what the source of the type is. For example, functions calls or literals: https://psalm.dev/r/d4a047a858

Your snippet match boths modules. It's a constant that happens to be always false so it triggers 2 errors.

I'm not perfectly sure how to fix this to prevent overlaps while still covering every cases. I have two ideas:

• make the falsy operand checks reports the error on the full condition to make it overlap in position with the other error and find a way to deduplicate those two (I believe there is already a system for that). It's too bad though, because it's the error that is the more targeted to the actual error. I can't change the location of the other one though.
• maybe prevent emitting the falsy operand one if the statement has a var id and assume the assertion module took care of it. The risk here would be to still have some overlap and possibly missing cases.

It's pretty low priority for me though as both error can be suppressed in a single annotation and it's not a big bother from my point of view

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/e4129d69f0

<?php

define( 'HELLO', false );

if (HELLO) {
    /** @psalm-trace $a */
    $a = HELLO;
}
else {
    /** @psalm-trace $b */
    $b = HELLO;
}

Psalm output (using commit 0ec8dd2):

ERROR: TypeDoesNotContainType - 5:5 - Operand of type false is always false

INFO: Trace - 7:5 - $a: empty

INFO: Trace - 11:5 - $b: false

INFO: UnusedVariable - 7:5 - $a is never referenced or the value is not used

INFO: UnusedVariable - 11:5 - $b is never referenced or the value is not used

https://psalm.dev/r/d4a047a858

<?php

if(0){
    
}

if(returnzero()){
    
}

/** @return 0 */
function returnzero(){
  return 0;   
}

Psalm output (using commit 0ec8dd2):

ERROR: TypeDoesNotContainType - 3:4 - Operand of type 0 is always false

ERROR: DocblockTypeContradiction - 7:4 - Operand of type 0 is always false

[kkmuffme]

Thanks for clarifying this is "on purpose". Since the errors are correct, I will just mark it as closed as it's documented now why this behavior exists.

[weirdan]

#7054 has been reverted