False positive: PossiblyNullArgument

[zmitic]

https://psalm.dev/r/8c2110b83b

Problem 1:

Psalm-trace correctly shows that $id is string, but in the next line, it thinks it is null|string. Reproducer: https://3v4l.org/k51io#v8.1.4

Problem 2:

If the line 4 (the one with throw) is removed, psalm correctly reports PossiblyNullArgument, but trace is wrong: https://psalm.dev/r/f0585c4a94

[psalm-github-bot[bot]]

I found these snippets:

https://psalm.dev/r/8c2110b83b

<?php

function start(?string $id): void {
    $id ?? throw new RuntimeException();
    /** @psalm-trace $id */
    
    process($id);
}

/** @psalm-suppress UnusedParam */
function process(string $id): void {}

Psalm output (using commit c8cc3f4):

ERROR: PossiblyNullArgument - 7:13 - Argument 1 of process cannot be null, possibly null value provided

INFO: Trace - 7:5 - $id: string

https://psalm.dev/r/f0585c4a94

<?php

function start(?string $id): void {
    /** @psalm-trace $id */
    
    process($id);
}

/** @psalm-suppress UnusedParam */
function process(string $id): void {}

Psalm output (using commit c8cc3f4):

ERROR: PossiblyNullArgument - 6:13 - Argument 1 of process cannot be null, possibly null value provided

INFO: Trace - 6:5 - $id: string

[AndrolGenhald]

I don't think the trace is actually wrong, it appears Psalm is narrowing it based on the argument type: https://psalm.dev/r/ea3bdf8ed7

Since @psalm-trace is applied to the next statement the trace happens after the function call (which is also evident in the output, the trace appears after PossiblyNullArgument). Add an empty statement by sticking a ; after it and it's correct: https://psalm.dev/r/aeafb9ffa7 https://psalm.dev/r/4e937b1cee

[psalm-github-bot[bot]]

I found these snippets:

https://psalm.dev/r/ea3bdf8ed7

<?php

/** @var mixed */
$foo = "bar";

function foobar(string $_): void {}

/** @psalm-trace $foo */;
foobar($foo);
/** @psalm-trace $foo */;

Psalm output (using commit c8cc3f4):

INFO: Trace - 8:25 - $foo: mixed

INFO: MixedArgument - 9:8 - Argument 1 of foobar cannot be mixed, expecting string

INFO: Trace - 10:25 - $foo: string

https://psalm.dev/r/aeafb9ffa7

<?php

function start(?string $id): void {
    $id ?? throw new RuntimeException();
    /** @psalm-trace $id */;
    
    process($id);
}

/** @psalm-suppress UnusedParam */
function process(string $id): void {}

Psalm output (using commit c8cc3f4):

INFO: Trace - 5:28 - $id: null|string

ERROR: PossiblyNullArgument - 7:13 - Argument 1 of process cannot be null, possibly null value provided

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

<?php

function start(?string $id): void {
    /** @psalm-trace $id */;
    
    process($id);
}

/** @psalm-suppress UnusedParam */
function process(string $id): void {}

Psalm output (using commit c8cc3f4):

INFO: Trace - 4:28 - $id: null|string

ERROR: PossiblyNullArgument - 6:13 - Argument 1 of process cannot be null, possibly null value provided

[weirdan]

or throw ... works as expected: https://psalm.dev/r/76278468d2

[psalm-github-bot[bot]]

I found these snippets:

https://psalm.dev/r/76278468d2

<?php

function start(?string $id): void {
    $id !== null or throw new RuntimeException();
    /** @psalm-trace $id */;
    
    process($id);
}

function process(string $_id): void {}

Psalm output (using commit 8f39de9):

INFO: Trace - 5:28 - $id: string

[zmitic]

Solved it in this way: https://psalm.dev/r/c2e95cddfb

It is minor bug for a rare case so I suggest closing the issue.

[psalm-github-bot[bot]]

I found these snippets:

https://psalm.dev/r/c2e95cddfb

<?php

function start(?string $id): void {
    process($id ?? throw new RuntimeException());
}

/** @psalm-suppress UnusedParam */
function process(string $id): void {}

Psalm output (using commit 8f39de9):

No issues!

[orklah]

I'd leave this open. Our nullsafe operator and null coalesce operator handling are clearly broken (this is far from the only issue here, we have a dedicated tag for that: https://github.com/vimeo/psalm/labels/nullsafe%20%2F%20null%20coalesce). This issue will be important the day where we'll want to tackle this

[mgiuffrida]

Is there a metabug for all these nullsafe / null coalesce issues, or are there plans to tackle them? (Would be helpful to know either way.)