Skip to content

More precise md5/sha1 return type #3541

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

Merged
merged 6 commits into from
Oct 8, 2024
Merged

More precise md5/sha1 return type #3541

merged 6 commits into from
Oct 8, 2024

Conversation

staabm
Copy link
Contributor

@staabm staabm commented Oct 6, 2024

utilizing lowercase-string for this old school hashing functions

//cc @VincentLanglet

@staabm staabm marked this pull request as ready for review October 6, 2024 18:48
@phpstan-bot
Copy link
Collaborator

This pull request has been marked as ready for review.

@VincentLanglet
Copy link
Contributor

If I understand correctly

You are using numeric-string in case only numbers are used ?
But numbers like 1234 is considered as a numeric string lowercased. (Only things like 10E2 are not but I dont think there is uppercase letter in sha or md5)

So it's non-falsy&numeric&lowercase|non-falsy&lowercase
Which can be simplified to non-falsy&lowercase.

I dont think there is a benefit to specify that sometimes it's a fully numeric string

@staabm
Copy link
Contributor Author

staabm commented Oct 6, 2024

great feedback. I like the simplified version a lot more.

I am not yet used to thinking about numbers to be lowercase strings ;).

this also means we can do similar stuff in the hash() dynamic return type extension when $algo is a constant string known

@@ -6403,8 +6403,8 @@
'mcrypt_module_open' => ['resource|false', 'cipher'=>'string', 'cipher_directory'=>'string', 'mode'=>'string', 'mode_directory'=>'string'],
'mcrypt_module_self_test' => ['bool', 'algorithm'=>'string', 'lib_dir='=>'string'],
'mcrypt_ofb' => ['string', 'cipher'=>'string', 'key'=>'string', 'data'=>'string', 'mode'=>'int', 'iv='=>'string'],
'md5' => ['non-falsy-string', 'str'=>'string', 'raw_output='=>'bool'],
'md5_file' => ['non-falsy-string|false', 'filename'=>'string', 'raw_output='=>'bool'],
'md5' => ['(non-falsy-string&lowercase-string)', 'str'=>'string', 'raw_output='=>'bool'],
Copy link
Member

Choose a reason for hiding this comment

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

No need for the parentheses here.

@ondrejmirtes ondrejmirtes merged commit 98e2b6e into phpstan:1.12.x Oct 8, 2024
474 of 500 checks passed
@ondrejmirtes
Copy link
Member

Thank you!

@staabm staabm deleted the hash branch October 8, 2024 16:18
@VincentLanglet
Copy link
Contributor

@staabm I'm not familiar with sha256 ; should the return type be updated too ?

@staabm
Copy link
Contributor Author

staabm commented Nov 24, 2024

I am not familiar either.

Tested a few inputs on https://emn178.github.io/online-tools/sha256.html and it seems you are right

@VincentLanglet
Copy link
Contributor

VincentLanglet commented Nov 24, 2024

I am not familiar either.

Tested a few inputs on emn178.github.io/online-tools/sha256.html and it seems you are right

I'll do the PR as soon as I have confirmation this function really exists (cf my interrogation phpstan/phpstan#12048 (comment))

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 this pull request may close these issues.

None yet

4 participants