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

Allow usage with PSR-7 v1.1 #133

Merged
merged 2 commits into from
Apr 6, 2023

Conversation

weierophinney
Copy link
Member

@weierophinney weierophinney commented Apr 4, 2023

Q A
Documentation no
Bugfix no
BC Break no
New Feature yes
RFC no
QA yes

Description

This patch updates the PSR-7 minimum supported version to 1.1. PSR-7 1.1 adds parameter type declarations, but not return type declarations. Since parameters can be widened in implementations, and return types narrowed, our existing implementation (which already features return types) already complies.

For testing, I've pinned this to a branch of the PSR-7 integration test suite submitted as php-http/psr7-integration-tests#68. This ensures that we fulfill the changed expectations (raising either InvalidArgumentException OR TypeError for invalid parameters), and will not need to update again once we prepare a v3 release of this library that explicitly adds parameter type declarations.

After updating dependencies, Psalm flagged a number of issues. I corrected one that was easily corrected; the remainder are due to Psalm detecting the parent parameter type declarations and deciding we don't need to do some of our validations. Since at runtime, invalid data could still be passed to the implementation, I've pushed these into the Psalm baseline for now, and for removal once we prepare for v3 targeting PSR-7 v2.

@weierophinney weierophinney added this to the 2.25.0 milestone Apr 4, 2023
Copy link
Member

@gsteel gsteel left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member

@boesing boesing left a comment

Choose a reason for hiding this comment

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

LGTM

Updates dependencies to pin to PSR-7 1.1 or later.
No changes required for compatibility.

Signed-off-by: Matthew Weier O'Phinney <[email protected]>
Fixed one type issue in a test where it was possible to expand types expected.
All other reported issues are now due to type inference being less strict than the engine, leading us to perform validation to be correct, but Psalm complaining that the expected types would not require validation.

Signed-off-by: Matthew Weier O'Phinney <[email protected]>
@weierophinney weierophinney merged commit 0e9dd25 into laminas:2.25.x Apr 6, 2023
@weierophinney weierophinney deleted the feature/psr-7v1.1 branch April 6, 2023 19:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants