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

Cannot detect check for null when assigning to a variable #3631

Closed
Nyholm opened this issue Jun 22, 2020 · 4 comments
Closed

Cannot detect check for null when assigning to a variable #3631

Nyholm opened this issue Jun 22, 2020 · 4 comments
Labels
bug hard problems Problems without an obvious easy solution

Comments

@Nyholm
Copy link
Contributor

Nyholm commented Jun 22, 2020

I found this bug. Not sure if it is "too complex" or not. I just thought to report it.

function takesAString(string $name): void {
    echo "hello: ".$name;
}

function randomReturn(): ?string {
    return rand(1,2) === 1 ? 'foo' : null;
}

$name = randomReturn();

// Assignment in if does not work
if ($hasStringValue = $name !== null) {
    takesAString($name);
}

https://psalm.dev/r/c9ebe1f843

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/c9ebe1f843
<?php

function takesAString(string $name): void {
    echo "hello: ".$name;
}

function randomReturn(): ?string {
    return rand(1,2) === 1 ? 'foo' : null;
}

$name = randomReturn();

// This comparison works file
if ($name !== null) {
    takesAString($name);
}

// Adding comparison to variable does not work
$hasStringValue = $name !== null;
if ($hasStringValue) {
    takesAString($name);
}


// Assignment in if does not work
if ($hasStringValue = $name !== null) {
    takesAString($name);
}
Psalm output (using commit 29eb830):

ERROR: PossiblyNullArgument - 21:18 - Argument 1 of takesAString cannot be null, possibly null value provided

ERROR: PossiblyNullArgument - 27:18 - Argument 1 of takesAString cannot be null, possibly null value provided

INFO: UnusedVariable - 26:5 - Variable $hasStringValue is never referenced

@muglug muglug added the bug label Jun 22, 2020
@muglug muglug closed this as completed in 9c17795 Jun 22, 2020
@muglug
Copy link
Collaborator

muglug commented Jun 22, 2020

Generally if it works in TypeScript it should work in Psalm, and the example in this ticket works in TypeScript (so it now works in Psalm too).

The other example in the link doesn't work in Psalm, as it would require some pretty complex analysis:

$hasStringValue = $name !== null;
if ($hasStringValue) {
    takesAString($name);
}

@Nyholm
Copy link
Contributor Author

Nyholm commented Jun 22, 2020

Lovely. Thank you for a quick fix.

muglug added a commit that referenced this issue Jun 23, 2020
@muglug
Copy link
Collaborator

muglug commented Jun 23, 2020

The fix caused a regression so I'm unfixing for now

@muglug muglug reopened this Jun 23, 2020
@muglug muglug added the hard problems Problems without an obvious easy solution label Jun 23, 2020
@muglug muglug closed this as completed in 7a7cd91 Jun 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug hard problems Problems without an obvious easy solution
Projects
None yet
Development

No branches or pull requests

2 participants