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

mb_strtolower|upper does not return false #3287

Closed
vudaltsov opened this issue May 1, 2020 · 6 comments · Fixed by #6036
Closed

mb_strtolower|upper does not return false #3287

vudaltsov opened this issue May 1, 2020 · 6 comments · Fixed by #6036

Comments

@vudaltsov
Copy link
Contributor

https://psalm.dev/r/d499c196d5
https://www.php.net/manual/en/function.mb-strtolower.php
I can fix this tomorrow.

I was also surprised that the second example in the snippet passes. @muglug , is that okay?

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/d499c196d5
<?php

/**
 * @param list<string> $values
 */
function checkStrings(array $values): void {}

checkStrings(array_map('mb_strtolower', ['a']));



function checkString(string $value): void {}

checkString(mb_strtolower('a'));
Psalm output (using commit 07e5250):

INFO: UnusedParam - 6:29 - Param $values is never referenced in this method

ERROR: InvalidScalarArgument - 8:14 - Argument 1 of checkStrings expects list<string>, array{0: false|string} provided

INFO: UnusedParam - 12:29 - Param $value is never referenced in this method

@weirdan
Copy link
Collaborator

weirdan commented May 3, 2020

mb_strtolower|upper does not return false

It does in 7.4.5: https://github.com/php/php-src/blob/php-7.4.5/ext/mbstring/mbstring.c#L3532 (though the exact conditions are not clear to me right away).
And it doesn't in master: https://github.com/php/php-src/blob/master/ext/mbstring/mbstring.c#L2788

@orklah
Copy link
Collaborator

orklah commented Nov 28, 2020

Seems like this was fixed since?

@weirdan
Copy link
Collaborator

weirdan commented Nov 28, 2020

@vudaltsov reopen if necessary.

@weirdan weirdan closed this as completed Nov 28, 2020
@mpesari
Copy link
Contributor

mpesari commented Jun 30, 2021

Hey, these functions have indeed changed since PHP 8 to not return false, but throw instead:

7.4.20: https://github.com/php/php-src/blob/php-7.4.20/ext/mbstring/tests/mb_strtolower_error2.phpt
8.0.0: https://github.com/php/php-src/blob/php-8.0.0/ext/mbstring/tests/mb_strtolower_error2.phpt

Maybe this could be fixed by adding the new signatures to the dictionaries/CallMap_80_delta.php file?

Could be useful to check the whole mbstring family while at it.

@weirdan
Copy link
Collaborator

weirdan commented Jun 30, 2021

Maybe this could be fixed by adding the new signatures to the dictionaries/CallMap_80_delta.php file?

Yes, PR is welcome 😄

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

Successfully merging a pull request may close this issue.

4 participants