Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

False positives when using isset($array[$key]) === false instead of !isset($array[$key]) #8084

Closed
aboks opened this issue Jun 10, 2022 · 1 comment
Labels

Comments

@aboks
Copy link
Contributor

aboks commented Jun 10, 2022

We have long ago adopted a code style that enforces an explicit === false instead of a ! negation within if conditions. I noticed that this sometimes leads to false positives when using an isset() check for a key within an array, while Psalm does not find issues on the equivalent version with a ! instead of === false.

One that I've been able to reproduce in isolation:

I also see some RedundantPropertyInitializationCheck issues on if (isset($this->array[$key]) === false) patterns on our code base (like Psalm does not understand that we're checking if a specific element is set rather than if the whole array is initialized), which disappear when I switch === false to !. I haven't been able to reproduce these in an isolated case though, and hope these maybe also disappear if the above false positive is solved.

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/6ae508f44e
<?php

interface UuidInterface {
    public function toString(): string;
}

class Foo {}

class FooCollection {
    /** @var Foo[] */
    private array $foos = [];
    
    public function addFoo(UuidInterface $uuid, Foo $foo): void {
        $this->foos[$uuid->toString()] = $foo;
    }
    
    public function getFoo(UuidInterface $uuid): Foo {
        if (isset($this->foos[$uuid->toString()]) === false) {
            throw new \LogicException("No such foo");
        }
        return $this->foos[$uuid->toString()];
    }
}
Psalm output (using commit a9775c6):

INFO: MixedArrayAccess - 21:16 - Cannot access array value on mixed variable $this->foos

INFO: MixedReturnStatement - 21:16 - Could not infer a return type

INFO: MixedInferredReturnType - 17:50 - Could not verify return type 'Foo' for FooCollection::getFoo
https://psalm.dev/r/5681b1b89c
<?php

interface UuidInterface {
    public function toString(): string;
}

class Foo {}

class FooCollection {
    /** @var Foo[] */
    private array $foos = [];
    
    public function addFoo(UuidInterface $uuid, Foo $foo): void {
        $this->foos[$uuid->toString()] = $foo;
    }
    
    public function getFoo(UuidInterface $uuid): Foo {
        if (!isset($this->foos[$uuid->toString()])) {
            throw new \LogicException("No such foo");
        }
        return $this->foos[$uuid->toString()];
    }
}
Psalm output (using commit a9775c6):

No issues!

pvandommelen added a commit to pvandommelen/psalm that referenced this issue Sep 18, 2022
@orklah orklah closed this as completed Sep 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants