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

Make last parameter of openssl_seal optional #329

Merged
merged 1 commit into from Sep 30, 2020

Conversation

phil-davis
Copy link
Contributor

@phil-davis phil-davis commented Sep 30, 2020

Fixes issue phpstan/phpstan#3896

Which seemed to have been broken by #327

Where are the tests for this? It seems to be untested, so we won't know if there is a regression in this file of function signature definitions.

@ondrejmirtes
Copy link
Member

Where are the tests for this? It seems to be untested

How would you test function signatures? It'd just duplicate information that's already in functionMap.php, and it wouldn't actually be the guarantee the signatures are correct :)

@ondrejmirtes
Copy link
Member

You can add a test case in CallToFunctionParametersRuleTest, but I definitely don't want that for all the described functions in that file :)

@ondrejmirtes ondrejmirtes merged commit 078ebaf into phpstan:master Sep 30, 2020
@ondrejmirtes
Copy link
Member

Thank you!

@ondrejmirtes
Copy link
Member

Alright, I just tested this exact situation and found a couple more violations: d2f443c

@phil-davis phil-davis deleted the issue-3896 branch September 30, 2020 13:50
@phil-davis
Copy link
Contributor Author

Where are the tests for this? It seems to be untested

How would you test function signatures? It'd just duplicate information that's already in functionMap.php, and it wouldn't actually be the guarantee the signatures are correct :)

Yes, I agree and realise that - what to do for tests of things that are just "constant input data". The test criteria is that "the function signatures match correctly the signatures defined by the PHP documentation/spec". But the real implementation will probably be a nonsense comparison of the content of functionMap with an "expected map", and the "expected map" will be just as likely to have errors in its definition!

If there is somewhere that has an easily machine-readable-parseable "official specification" of all those functions, then a test could parse all that and check against functionMap - but that's a job for for when someone is stuck for 2 weeks in quarantine...

@ondrejmirtes
Copy link
Member

easily machine-readable-parseable "official specification" of all those functions

This is notoriously messy. If it wasn't messy then we wouldn't need to manually maintain this functionMap at all :)

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