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

Psalm thinks explode() can return false #5347

Closed
simPod opened this issue Mar 8, 2021 · 4 comments · Fixed by #5350
Closed

Psalm thinks explode() can return false #5347

simPod opened this issue Mar 8, 2021 · 4 comments · Fixed by #5350
Labels

Comments

@simPod
Copy link
Contributor

simPod commented Mar 8, 2021

I have enabled ignoreInternalFunctionFalseReturn="false"

(I cannot do it in playground) https://psalm.dev/r/96ef362b30

explode() returns false when value is empty string.

I asserted the value is not empty string but psalm still reports it can return false

Cannot iterate over falsable var false|non-empty-list

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/96ef362b30
<?php

/**
 * @return non-empty-list<string>
 */
function takesAString(string $value) : array {
    assert($value !== '');
    return explode(',', $value);
}

foreach(takesAString('h,i') as $entry) {
    echo $entry;
}
Psalm output (using commit 3c66b75):

No issues!

@weirdan
Copy link
Collaborator

weirdan commented Mar 8, 2021

explode() returns false when value is empty string.

Actually it returns false when separator is empty.
Reproduced in https://psalm.dev/r/6b3a3e88a6

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/6b3a3e88a6
<?php
/** @param non-empty-string $sep */
function f(string $sep): void {
    $list = explode($sep, "asd");
    if (false === $list) {} // expected: TypeDoesNotContainType: non-empty-list<string> does not contain false
}
Psalm output (using commit 3c66b75):

No issues!

@weirdan weirdan added the bug label Mar 8, 2021
weirdan added a commit to weirdan/psalm that referenced this issue Mar 8, 2021
Fixes vimeo#5347

`explode()` now omits `false` from the return type  when separator is a
definitely non-empty string.
@simPod
Copy link
Contributor Author

simPod commented Mar 8, 2021

Ah, u're right. I had delimiter in constant as const D = ',';. My brain somehow ruled it out and rather focused on exploded value.

muglug pushed a commit that referenced this issue Mar 11, 2021
Fixes #5347

`explode()` now omits `false` from the return type  when separator is a
definitely non-empty string.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants