false is not eliminated when comparing against != ""

[bwoebi]

A comparison if ($key != "") { ... } does still identify $key as being string|false instead of string within the if.

https://psalm.dev/r/4fb572a4fe

The goal is being able to simply compare is not empty and not false at once.

Related to #6965.

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/4fb572a4fe

<?php

class A {
    private ?string $prop = null;
    
    public function __construct() {
        if ("" != $env = getenv("TEST")) {
        	$this->prop = $env;
        }
    }
}

Psalm output (using commit a75d26a):

ERROR: PossiblyFalsePropertyAssignmentValue - 8:24 - $this->prop with non-falsable declared type 'null|string' cannot be assigned possibly false type 'false|string'

[danog]

Just using a direct check fixes: https://psalm.dev/r/fb503c39ce

Now, a new issue is emitted due to the recently merged #10502, and both in case of the != '' comparison and in this new issue, the answer I'm inclined to give is the same: loose comparisons are generally a bad idea, and should be avoided whenever possible.

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/fb503c39ce

<?php

class A {
    private ?string $prop = null;
    
    public function __construct() {
        if ($env = getenv("TEST")) {
        	$this->prop = $env;
        }
    }
}

Psalm output (using commit a2140cc):

ERROR: RiskyTruthyFalsyComparison - 7:13 - Operand of type false|string contains type string, which can be falsy and truthy. This can cause possibly unexpected behavior. Use strict comparison instead.

[bwoebi]

A direct check would also compare against "0" which would not be intended here. I want just "" and null/false basically.

Obviously, writing it out would work, but it's extra verbose and also less efficient code.

[kkmuffme]

You could just use !empty()

If you really need this, give a PR a try
(since I doubt anybody of the existing contributors will fix this, since most people use/require strict comparisons bc the loose comparisons are a pain and huge source of bugs)

[bwoebi]

empty() sadly also compares against "0" :-/

[danog]

loose comparisons are a pain and huge source of bugs

+++, it along with empty and isset are banned in my codebase (and I'm very thankful for that)

I consider this issue as super low priority, I'd rather finish working on #10577 first :)