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

Refine *strlen() return type to exclude negative integers #945

Merged
merged 4 commits into from Jan 27, 2022

Conversation

staabm
Copy link
Contributor

@staabm staabm commented Jan 26, 2022

@@ -9395,7 +9395,7 @@
'Redis::sRem' => ['int', 'key'=>'string', 'member1'=>'string', '...other_members='=>'string'],
'Redis::sRemove' => ['int', 'key'=>'string', 'member1'=>'string', '...other_members='=>'string'],
'Redis::sScan' => ['array|bool', 'key'=>'string', '&iterator'=>'int', 'pattern='=>'string', 'count='=>'int'],
'Redis::strLen' => ['int', 'key'=>'string'],
'Redis::strLen' => ['0|positive-int', 'key'=>'string'],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ondrejmirtes
Copy link
Member

The tests here need fixing.

@staabm
Copy link
Contributor Author

staabm commented Jan 26, 2022

The tests here need fixing.

sorry, locally on php8 it was green.

mb_strlen seems to have some special cases I don't want to dig into. reverted these parts for now:

on the initial commit these mbStrlen* cases failed on php 7.3 while were fine on php8.
https://github.com/phpstan/phpstan-src/runs/4951628678?check_suite_focus=true

fixing these for php7, made them fail on php8.
https://github.com/phpstan/phpstan-src/runs/4954732688?check_suite_focus=true

this should be good to go as is.

@ondrejmirtes
Copy link
Member

mb_strlen has a dynamic return type extension: MbFunctionsReturnTypeExtension

@ondrejmirtes ondrejmirtes merged commit fa56531 into phpstan:master Jan 27, 2022
@ondrejmirtes
Copy link
Member

Thank you!

@staabm staabm deleted the patch-3 branch January 27, 2022 11:11
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