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

Add support for integer ranges in strtotime() with constant strings #1045

Merged
merged 2 commits into from Mar 4, 2022

Conversation

Seldaek
Copy link
Contributor

@Seldaek Seldaek commented Mar 3, 2022

I migrated the existing tests from legacy tests, I hope this makes sense.

I am trying to pass strtotime() to a positive-int param and figured this should just work :)

The ranges are always int<$oldestResolvedTime, max> as time flows forward, the result may increase but should not decrease.

@Seldaek Seldaek force-pushed the strtotime branch 2 times, most recently from dac1c88 to a1fb11d Compare March 3, 2022 13:04

$results = array_map('intval', $results);

return IntegerRangeType::createAllGreaterThan(min($results));
Copy link
Contributor Author

@Seldaek Seldaek Mar 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This yields:

57 Parameter #1 $value of static method
PHPStan\Type\IntegerRangeType::createAllGreaterThan() expects
float|int, int|false given.

But I don't get it, because $results is not empty, and so min() cannot return false.

Edit: fixed with a (int) cast for now.. but not pleased :)

Comment on lines 28 to 29
$strtotimeNegativeInt = strtotime('1969-12-31 12:00:00 UTC');
assertType('int<-43199, max>', $strtotimeNegativeInt);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could the min value depend on the systems time-zone?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it does. I think it's mostly ok as the main point is to know if it's positive-int or int IMO. negative-int in dates are pretty rare.. The high precision requires you have a correctly set timezone, which sounds like an OK limitation to me but maybe it isnt?

assertType('int|false', $strtotimeCrash);

$strtotimeWithBase = strtotime('+2 days', time());
assertType('int', $strtotimeWithBase);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe

Suggested change
assertType('int', $strtotimeWithBase);
assertType('positive-int', $strtotimeWithBase);

?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes but that would require parsing the second argument properly, which I am not doing right now. Feel free to send a PR for that, it's beyond my skills here :D

@ondrejmirtes ondrejmirtes force-pushed the strtotime branch 2 times, most recently from 3d0c903 to ca5e927 Compare March 3, 2022 16:45
@Seldaek Seldaek force-pushed the strtotime branch 2 times, most recently from ba41785 to 494a7f8 Compare March 4, 2022 07:47
@ondrejmirtes ondrejmirtes merged commit 98ee715 into phpstan:master Mar 4, 2022
@ondrejmirtes
Copy link
Member

Thank you!

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