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 stub for openssl_sign() function #3532

Open
wants to merge 9 commits into
base: 1.12.x
Choose a base branch
from

Conversation

piotrekkr
Copy link

@piotrekkr piotrekkr commented Oct 3, 2024

Adding openssl_sign() stub so PHPStan will correctly deduce $signature variable type which is now shown as mixed instead of string.

Fixes phpstan/phpstan#11791

@phpstan-bot
Copy link
Collaborator

You've opened the pull request against the latest branch 2.0.x. PHPStan 2.0 is not going to be released for months. If your code is relevant on 1.12.x and you want it to be released sooner, please rebase your pull request and change its target to 1.12.x.

@piotrekkr
Copy link
Author

Will rebase properly in a minute

…re` variable type which is now shown as `mixed` instead of `string`.

Fixes phpstan/phpstan#11791
Copy link
Member

@ondrejmirtes ondrejmirtes left a comment

Choose a reason for hiding this comment

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

You have to register this stub file in config/config.neon.

@piotrekkr
Copy link
Author

@ondrejmirtes Added it to conf/config.neon

@piotrekkr piotrekkr requested a review from ondrejmirtes October 3, 2024 12:54
@piotrekkr
Copy link
Author

Do I need to type all arguments or just this @param-out and the rest will be taken from signature? Asking because tests fail https://github.com/phpstan/phpstan-src/actions/runs/11162040542/job/31025901918 and they mention $private_key which is typed in function but seems like it is not enough.

Copy link
Member

@ondrejmirtes ondrejmirtes left a comment

Choose a reason for hiding this comment

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

Please fix proc_open() from phpstan/phpstan#11777 too.

@ondrejmirtes
Copy link
Member

Do I need to type all arguments

Copy the PHPdoc from jetbrains/phpstorm-stubs and leave out unimportant stuff like description.

@piotrekkr
Copy link
Author

@ondrejmirtes I've updated phpdoc. Let me know if I need to do something more. Thanks

@piotrekkr piotrekkr requested a review from ondrejmirtes October 4, 2024 09:53
@piotrekkr
Copy link
Author

@ondrejmirtes I think there is some bug in PHP manual of this openssl_sign() function. They mention about private key that can be one of those

#[\SensitiveParameter] OpenSSLAsymmetricKey|OpenSSLCertificate|array|string $private_key,

In description of input params, there is no mention of array, so my guess is that array is not allowed. I removed it from stub but not sure if it is good way to go. Lent me know what you think.

@piotrekkr
Copy link
Author

Made bug report for php docs php/doc-en#3829

@piotrekkr
Copy link
Author

Also signature changed in PHP 8 so I should probably create two stub files right? Something like openssl.stub and openssl_php8.stub. Then do it similar to this to have both new and older php handled. Is it correct?

@piotrekkr
Copy link
Author

Okay asked about this array in private_key and it seems it is correct but docs are not updated properly so will need to wait for it to be updated and will then update this PR

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.

Incorrect mixed type with openssl_sign and signature variable
3 participants