vimeo / psalm

A static analysis tool for finding errors in PHP applications
https://psalm.dev
MIT License
5.55k stars 660 forks source link

DateTimeImmutable::getTimezone can contain false #9244

Open ChristophWurst opened 1 year ago

ChristophWurst commented 1 year ago

https://psalm.dev/r/590531597a

This is very similar to https://github.com/vimeo/psalm/issues/4515. Apparently this still happens, but only for immutable datetime objects.

https://www.php.net/manual/en/datetime.gettimezone.php

psalm-github-bot[bot] commented 1 year ago

I found these snippets:

https://psalm.dev/r/590531597a ```php getTimeZone() === false) { echo "no tz"; } ``` ``` Psalm output (using commit 4eacb2f): ERROR: DocblockTypeContradiction - 5:5 - DateTimeZone does not contain false ```
orklah commented 1 year ago

Callmaps are overriden in stubs in this case: https://github.com/vimeo/psalm/blob/8d36bdc3edaee176ecc25b7e145b9e56fb81d7ea/stubs/CoreImmutableClasses.phpstub#L26

@Ocramius is this an error or was this intended?

Ocramius commented 1 year ago

Not sure/can't remember.

I think I read in php-src about these specific implementation details: would endorse looking at git blame to determine whether it was intentional.

Ocramius commented 1 year ago

Looking at the history, I think the restricted return type is a mistakes, while my intent was to mark the API as pure.

I would suggest checking php-src to verify if that method ever returns false, and no, the PHP documentation is not reliable 😛

othercorey commented 1 year ago

I've looked into this before. It depends on an internal flag that I didn't understand:

if (dateobj->time->is_localtime) {
} else {
    RETURN_FALSE;
}

We ended up ignoring this possibility.