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

Incorrect return types for stubbed core functions depending on PHP version #9888

Closed
kkmuffme opened this issue Jun 9, 2023 · 3 comments · Fixed by #9943
Closed

Incorrect return types for stubbed core functions depending on PHP version #9888

kkmuffme opened this issue Jun 9, 2023 · 3 comments · Fixed by #9943

Comments

@kkmuffme
Copy link
Contributor

kkmuffme commented Jun 9, 2023

https://psalm.dev/r/7106f13383

in PHP 8 vsprintf will never return false, but in PHP 7 it will.
As discussed in #9877 (comment) the stubs will never fall back to the callmap - this makes declaring different return/param types between PHP versions impossible in psalm.

I suggest to change it to fall back from the stubs to the callmap if the param/return type is not declared in the stubs (but is in the callmap)

Also somehow related to #9843 since atm no consistency checking happens in-between the different versions of the callmap and the stubs either

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/7106f13383
<?php

$x = vsprintf( 'a', array() );
/** @psalm-trace $x */;
Psalm output (using commit 99a54fb):

INFO: Trace - 4:23 - $x: false|string

INFO: UnusedVariable - 3:1 - $x is never referenced or the value is not used

@ygottschalk
Copy link
Contributor

I thought that we have the files stubs/Php8X.phpstub for that...

@orklah
Copy link
Collaborator

orklah commented Jun 12, 2023

I thought that we have the files stubs/Php8X.phpstub for that...

Yeah, that is a first option. We also have this:

* PHP_MAJOR_VERSION is 8 ?

For anything that those two tools can't answer, TypeProviders are the go to.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants