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

Return type added as string|bool instead of string|false #2588

Closed
calebdw opened this issue Mar 7, 2024 · 5 comments
Closed

Return type added as string|bool instead of string|false #2588

calebdw opened this issue Mar 7, 2024 · 5 comments

Comments

@calebdw
Copy link

calebdw commented Mar 7, 2024

Hello!

There are several php functions that return false if an error occurs (e.g., date()). Given that false is now a first class type I would expect phpactor to automatically set the return type for a function to be string|false instead of string|bool:

function test()
{
    return date('Y-m-d', 1);
}

Thanks!

@dantleech
Copy link
Collaborator

dantleech commented Mar 7, 2024

This is due to the phpstorm stubs, it would be good to replace/augment those with better stubs (e.g. the ones Psalm/PHPStan uses) for better types all round.

@calebdw
Copy link
Author

calebdw commented Mar 7, 2024

So the vendor/jetbrains/phpstorm-stubs/date/date.php stub (the one that opens when I jump to definition) has:

/**
 * ...
 * @param int|null $timestamp [optional] Default value: time(). The optional timestamp parameter is an integer Unix timestamp
 * that defaults to the current local time if a timestamp is not given.
 * @return string|false a formatted date string. If a non-numeric value is used for
 * timestamp, false is returned and an
 * E_WARNING level error is emitted.
 */
#[Pure(true)]
#[LanguageLevelTypeAware(["8.0" => "string"], default: "string|false")]
function date(string $format, ?int $timestamp) {}

The types appear to be correct?

@dantleech
Copy link
Collaborator

Hrmph, then it's a "bug"

dantleech added a commit that referenced this issue Mar 9, 2024
… status (#2591)

* Add passing test for false type

* Show PHP version and source in status

* Fix test case name

* GH-2588: Do not generalize types for return and PHP version in status

* Update CL
@dantleech
Copy link
Collaborator

For some reason we were downgrading the return types and not downgrading them didn't break any tests, so, let's see if anything breaks... :) note that this will only work if the Phpactor thinks the project is on PHP 8.2

@calebdw
Copy link
Author

calebdw commented Mar 9, 2024

Awesome thanks!

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

No branches or pull requests

2 participants