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

date_get_last_errors(), DateTime::getLastErrors() may return false #8478

Merged

Conversation

gsteel
Copy link
Contributor

@gsteel gsteel commented Sep 12, 2022

Up to PHP 8.2, date_get_last_errors(), DateTime::getLastErrors() and DateTimeImmutable::getLastErrors() return false if no previous date operations have been performed. Currently, In PHP 8.2-RC, false is returned after a date operation that yields neither warnings nor errors:

https://3v4l.org/HBq0q
https://3v4l.org/3QsKY

@boesing
Copy link
Contributor

boesing commented Sep 12, 2022

#7758 (comment)

I guess thats related and thus this can be closed.

@gsteel
Copy link
Contributor Author

gsteel commented Sep 12, 2022

@boesing just educated me on ignoreInternalFunctionFalseReturn - closing!

@gsteel gsteel closed this Sep 12, 2022
@AndrolGenhald
Copy link
Collaborator

Actually I think maybe you should reopen this. The example with my comment there showed that @psalm-traceing the type shows the false, but errors involving that false type are ignored by default. If ignoreInternalFunctionFalseReturn is disabled though it should still give an error.

If we try the same thing with date_get_last_errors, it doesn't show the false type, and if you try it locally with ignoreInternalFunctionFalseReturn disabled I bet it's not going to give an error like it should.

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/393ea844d9
<?php

function foo(): array
{
    $foo = date_get_last_errors();
    /** @psalm-trace $foo */
    return $foo;
}
Psalm output (using commit afe85fa):

INFO: Trace - 7:5 - $foo: array{error_count: int, errors: array<int, string>, warning_count: int, warnings: array<int, string>}

@gsteel gsteel reopened this Sep 12, 2022
@gsteel
Copy link
Contributor Author

gsteel commented Sep 12, 2022

@AndrolGenhald - Re-opened. Happy to add some tests, but I could do with a little bit of guidance where these should go

@AndrolGenhald AndrolGenhald added the release:fix The PR will be included in 'Fixes' section of the release notes label Sep 12, 2022
@orklah
Copy link
Collaborator

orklah commented Sep 13, 2022

you're missing a change for date_get_last_errors is Callmap_historical it seems

@gsteel gsteel force-pushed the correct_date_get_last_errors_return_type branch from b3d9225 to 0f1e416 Compare September 13, 2022 21:59
@gsteel
Copy link
Contributor Author

gsteel commented Sep 13, 2022

Thanks @orklah

Up to PHP 8.2, these functions return false if no previous date operations have been performed. In PHP 8.2, false is returned after a date operation that yields neither warnings nor errors:

https://3v4l.org/HBq0q
https://3v4l.org/3QsKY
Signed-off-by: George Steel <george@net-glue.co.uk>
@gsteel gsteel force-pushed the correct_date_get_last_errors_return_type branch from 0f1e416 to 17ca8ef Compare September 13, 2022 22:51
@gsteel
Copy link
Contributor Author

gsteel commented Sep 13, 2022

Rebased so cs tests pass 🤞

@AndrolGenhald AndrolGenhald merged commit be3a88d into vimeo:4.x Sep 14, 2022
@AndrolGenhald
Copy link
Collaborator

Thanks!

@gsteel gsteel deleted the correct_date_get_last_errors_return_type branch September 14, 2022 06:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:fix The PR will be included in 'Fixes' section of the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants