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

Wrong PossiblyNullReference on if condition #3825

Closed
mrsuh opened this issue Jul 16, 2020 · 5 comments
Closed

Wrong PossiblyNullReference on if condition #3825

mrsuh opened this issue Jul 16, 2020 · 5 comments
Labels

Comments

@mrsuh
Copy link

mrsuh commented Jul 16, 2020

Hi, i wrote a example

<?php

class PropertyClass
{
    public function test(): string
    {
        return 'test';
    }
}

class MainClass
{
    /** @var null|PropertyClass */
    public $property = null;

    public function getProperty(): ?PropertyClass //<-- may be nullable
    {
        return $this->property;
    }
}

$main = new MainClass();
if (random_int(0, 1) === 1) {
    $main->property = new PropertyClass();
}

if ($main->getProperty() !== null && $main->getProperty()->test() === 'test') { //<-- can't be null after first condition
    echo 'ok' . PHP_EOL;
}
ERROR: PossiblyNullReference - src/test.php:27:60 - Cannot call method test on possibly null value (see https://psalm.dev/083)
if ($main->getProperty() !== null && $main->getProperty()->test() === 'test') { //<-- can't be null after first condition

https://psalm.dev/r/1100ba2a85

I think psalm shouldn't warn because $main->getProperty() is already !== null
I found similar issue #3563, but it already fixed

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/1100ba2a85
<?php

class PropertyClass
{
    public function test(): string
    {
        return 'test';
    }
}

class MainClass
{
    /** @var null|PropertyClass */
    public $property = null;

    public function getProperty(): ?PropertyClass //<-- may be nullable
    {
        return $this->property;
    }
}

$main = new MainClass();
if (random_int(0, 1) === 1) {
    $main->property = new PropertyClass();
}

if ($main->getProperty() !== null && $main->getProperty()->test() === 'test') { //<-- can't be null after first condition
    echo 'ok' . PHP_EOL;
}
Psalm output (using commit 8fbc8de):

ERROR: PossiblyNullReference - 27:60 - Cannot call method test on possibly null value

@muglug muglug added bug and removed bug labels Jul 16, 2020
@muglug
Copy link
Collaborator

muglug commented Jul 16, 2020

Simple getters should be assumed to be mutation-free; I need to re-evaluate the circumstances in which that occurs

@muglug muglug closed this as completed in 96bfd14 Jul 16, 2020
@mrsuh
Copy link
Author

mrsuh commented Jul 16, 2020

thx!

@muglug
Copy link
Collaborator

muglug commented Jul 16, 2020

So now this is fixed you have a bunch of possible solutions available:

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/4da4914033
<?php

class PropertyClass
{
    public function test(): string
    {
        return 'test';
    }
}

class MainClass
{
    /** @var null|PropertyClass */
    public $property = null;

    public function getProperty(): ?PropertyClass
    {
        return $this->property;
    }
}

$main = new MainClass();
if (random_int(0, 1) === 1) {
    $main->property = new PropertyClass();
}

if ($main->getProperty() && $main->getProperty()->test() === 'test') {
    echo 'ok' . PHP_EOL;
}
Psalm output (using commit 262bb9f):

No issues!
https://psalm.dev/r/3a3f4ba96e
<?php

class PropertyClass
{
    public function test(): string
    {
        return 'test';
    }
}

class MainClass
{
    /** @var null|PropertyClass */
    public $property = null;

    final public function getProperty(): ?PropertyClass
    {
        return $this->property;
    }
}

$main = new MainClass();
if (random_int(0, 1) === 1) {
    $main->property = new PropertyClass();
}

if ($main->getProperty() !== null && $main->getProperty()->test() === 'test') {
    echo 'ok' . PHP_EOL;
}
Psalm output (using commit 262bb9f):

No issues!
https://psalm.dev/r/3e031e963d
<?php

class PropertyClass
{
    public function test(): string
    {
        return 'test';
    }
}

final class MainClass
{
    /** @var null|PropertyClass */
    public $property = null;

    public function getProperty(): ?PropertyClass
    {
        return $this->property;
    }
}

$main = new MainClass();
if (random_int(0, 1) === 1) {
    $main->property = new PropertyClass();
}

if ($main->getProperty() !== null && $main->getProperty()->test() === 'test') {
    echo 'ok' . PHP_EOL;
}
Psalm output (using commit 262bb9f):

No issues!
https://psalm.dev/r/91a1bb120d
<?php

class PropertyClass
{
    public function test(): string
    {
        return 'test';
    }
}

class MainClass
{
    /** @var null|PropertyClass */
    public $property = null;

    /**
     * @psalm-mutation-free
     */
    public function getProperty(): ?PropertyClass
    {
        return $this->property;
    }
}

$main = new MainClass();
if (random_int(0, 1) === 1) {
    $main->property = new PropertyClass();
}

if ($main->getProperty() !== null && $main->getProperty()->test() === 'test') {
    echo 'ok' . PHP_EOL;
}
Psalm output (using commit 262bb9f):

No issues!

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

2 participants