mysqli_field_seek return type changed to true

[kamil-tekiela]

mysqli_field_seek should have been marked as returning true in PHP 8.2.
@kocsismate Is there a reason why the methods cannot have true type? I can't remember. I will drop the second commit if that's not possible.

[Girgias]

Is mysqli a class that can be extended? That is probably the reason then as to why true types cannot be used.

[kamil-tekiela]

Is mysqli a class that can be extended? That is probably the reason then as to why true types cannot be used.

Can you explain a little more? Is this true even with @tentative-return-type?

[Girgias]

Is mysqli a class that can be extended? That is probably the reason then as to why true types cannot be used.

Can you explain a little more? Is this true even with @tentative-return-type?

So using tentative return types would force implementations to return true, however if we possibly want to change those to void that might be an issue? As currently, an overriding class could just return void. But I'm just speculating here, I don't know why actual types were not used :/

[kocsismate]

Yes, George is right: I didn't add any kind of type declaration because of the possible migration to void as I wasn't sure what the best way to achieve this was.

[kamil-tekiela]

Thanks for explanation. The second commit is dropped.

[kocsismate]

its alias should also be adjusted, otherwise ./build/gen_stub.php --verify will emit an error:

mysqli_result::field_seek() and mysqli_field_seek() must have the same return type

[kamil-tekiela]

And how do I fix it without adding true return type to the method? Would @return true work?

[kamil-tekiela]

I changed the return type to true. Maybe we should change it straight away to void?

[Girgias]

The issue is true to void is a BC break as a void function returns null so if the return value of the function is used and compared against, a different code path will then be taken.

[ondrejmirtes]

Would be nice to have this correct in PHP-8.0 and newer branches, not just for PHP 8.3. For example mysqli_close() returns true according to the stubs in PHP 8.2+, not sure if that reflects reality.

[kamil-tekiela]

We did not have return type true before PHP 8.2. So while it's true that the function only returns true since PHP 8.0, there was no way to specify that in the stubs.
In regards to field_seek(), I think it would not be wise to change the return type in a minor version. It should have been done in 8.2, but was missed, so it will be changed in 8.3 hopefully.

[Girgias]

Would be nice to have this correct in PHP-8.0 and newer branches, not just for PHP 8.3. For example mysqli_close() returns true according to the stubs in PHP 8.2+, not sure if that reflects reality.

8.0 is out of support, and the true type only exists as of PHP 8.2, so it is impossible to backport this to 8.1.

And yes, the stubs reflect reality that's the whole point of them. If the stubs are wrong by having a return type that is too narrow or a parameter type that is too wide we have some major issues.

So for PHPStan for version 8.0 and 8.1 for the most part you will need to have a sort of registry to inform about the "effective" return type of those functions, which then can include minor fixes for 8.2 too.

[ondrejmirtes]

Alright, nice, thanks.

there was no way to specify that in the stubs.

In phpstan/php-8-stubs where I extract the stubs, I check out each branch separately and then merge the definitions, like this: https://github.com/phpstan/php-8-stubs/blob/main/stubs/ext/mysqli/mysqli_close.php

So as long as each php-src branch has the correct data, it's fine for me as well 👍

[Girgias]

Add an UPGRADING note and LGTM