Fix #3036: make argument $read of internal PHP function stream_socket nullable.

[niconoe-]

According to documentation: https://www.php.net/manual/en/function.stream-select.php

[orklah]

I'm not sure if types in callmap for parameters that are references refers to the @param, @param-out or both.

Your change makes Psalm build fail because it's using the function without checking for null, but I suspect it can never return null (as @param-out) but it can take null (as @param). If I'm right, this would probably be best described in a stub

[niconoe-]

TBH, I don't really know how to use this function, and I though just mapping the CallMap with the PHP doc would be sufficient...

To me, it can take NULL and it can return NULL : https://3v4l.org/4QKQt

My question is "can it take NULL but out as an array of streams?" and "can it take an array of streams but out as NULL ?".
On this, I can't answer.

[orklah]

The fact that it can return null is enough to say Psalm should check against this, so this seem legit.

Can you add checks in Psalm code then?

[niconoe-]

Can you add checks in Psalm code then?

I'm not familiar enough with Psalm internal architecture to know where I can add a test for this. Which location do you want for it? I'll write the test where you advice me.

Thanks

[orklah]

No test needed, but Psalm uses the function internally and your changes makes it fail on CI

Can you check error in Psalm in the "Run Shepherd" job? It should point you to specific pieces of codes where "null" case should be handled

[niconoe-]

Ok, thanks.

I casted the usage of it in a foreach to an array ((array)null === [] so it will just don't go into the foreach block).

Also, I upated the historical CallMap as it provoked an error in the unit tests executed.

The error on the job "Pull Request Labels / label", on the other hand, looks like out of my hands, unless you can help me figure out what I've done wrong?

[AndrolGenhald]

@orklah It's probably not possible to have an @param-out dependent on the @returned value is it? The PHPUnit error is a bit unfortunate since all of the checks are there to guarantee that $r should not be empty or null.

I'm pretty sure for each of the 3 ?arrays if you pass an array in you get an array and if you pass null in you get null, maybe just correcting that would be good enough. I thought this might work but it doesn't seem to be supported for some reason.

Edit: Actually it does work! You just have to ignore the ReferenceConstraintViolation. I think using a stub like this is a better way to go, it should prevent a lot more false positives.

[orklah]

That's weird that this error pops up...

[muglug]

@niconoe- could you add the stub for stream_select suggested by @AndrolGenhald instead?

[niconoe-]

@niconoe- could you add the stub for stream_select suggested by @AndrolGenhald instead?

Sorry, I don't think I have the capabilities of adding such stub. Maybe @AndrolGenhald can give it a try?

[AndrolGenhald]

@niconoe- You can basically just copy/paste the function from this into CoreGenericFunctions.phpstub somewhere and change the name and whatever other tweaks are needed so it's correct.

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/50ec07381c

<?php

/**
 * @template TRead of null|array<array-key, resource>
 * @template TWrite of null|array<array-key, resource>
 * @template TExcept of null|array<array-key, resource>
 *
 * @param TRead $read
 * @param TWrite $write
 * @param TExcept $except
 *
 * @return false|int<0, max>
 * @param-out (TRead is null ? null : array<array-key, resource>) $read
 * @param-out (TWrite is null ? null : array<array-key, resource>) $write
 * @param-out (TExcept is null ? null : array<array-key, resource>) $except
 *
 * @psalm-suppress ReferenceConstraintViolation
 */
function _stream_select(
    ?array &$read,
    ?array &$write,
    ?array &$except,
    ?int $seconds,
    ?int $microseconds = null
) {
    return false;
}

/** @var list<resource> */
$r = [];
$w = null;
/** @var list<resource>|null */
$e = [];

_stream_select($r, $w, $e, 1);

/** @psalm-trace $r, $w, $e */;

Psalm output (using commit f0b2142):

INFO: Trace - 37:31 - $r: array<array-key, resource>

INFO: Trace - 37:31 - $w: null

INFO: Trace - 37:31 - $e: array<array-key, resource>|null

[niconoe-]

@niconoe- You can basically just copy/paste the function from this into CoreGenericFunctions.phpstub somewhere and change the name and whatever other tweaks are needed so it's correct.

Done.

Thank you for the help 👍

EDIT: As the CallMap is reset, the array casting on src/Psalm/Internal/Fork/Pool.php is no longer necessary. I removed it.

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/50ec07381c

<?php

/**
 * @template TRead of null|array<array-key, resource>
 * @template TWrite of null|array<array-key, resource>
 * @template TExcept of null|array<array-key, resource>
 *
 * @param TRead $read
 * @param TWrite $write
 * @param TExcept $except
 *
 * @return false|int<0, max>
 * @param-out (TRead is null ? null : array<array-key, resource>) $read
 * @param-out (TWrite is null ? null : array<array-key, resource>) $write
 * @param-out (TExcept is null ? null : array<array-key, resource>) $except
 *
 * @psalm-suppress ReferenceConstraintViolation
 */
function _stream_select(
    ?array &$read,
    ?array &$write,
    ?array &$except,
    ?int $seconds,
    ?int $microseconds = null
) {
    return false;
}

/** @var list<resource> */
$r = [];
$w = null;
/** @var list<resource>|null */
$e = [];

_stream_select($r, $w, $e, 1);

/** @psalm-trace $r, $w, $e */;

Psalm output (using commit f0b2142):

INFO: Trace - 37:31 - $r: array<array-key, resource>

INFO: Trace - 37:31 - $w: null

INFO: Trace - 37:31 - $e: array<array-key, resource>|null

[orklah]

Thanks!