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

PHP8 "static" return type #107

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

codemasher
Copy link

@codemasher codemasher commented Jul 19, 2023

Hi! I just upgraded psr/http-message to ^2.0 and my IDE started yelling at me because my library already requires a minimum of PHP 8.1 and i was already using the new static return type.

src\Psr7\Message.php:45 PhanParamSignatureRealMismatchReturnType Declaration of function withProtocolVersion(string $version) : static should be compatible with function withProtocolVersion(string $version) : \Psr\Http\Message\MessageInterface (method where the return type in the real signature is 'static' cannot override method where the return type in the real signature is '\Psr\Http\Message\MessageInterface') defined in vendor\psr\http-message\src\MessageInterface.php:41

Here, I'm proposing a v3.0 that requires a minimum of PHP 8.0:

  • return types have changed from <interface> to static
  • mixed types have been added
  • several union types (such as array|string) have been added
  • additionally I've fixed some grammar and typos in a different branch - you can have these too as separate PR

This update would make the PSR-7 interfaces finally fully typed and I am asking (no I am begging) for a swift review and release ideally before PHP 9 arrives. Thanks!

Edit: I am very well aware that this would break compatibility with v2.0. However, the issue is not the BC break of this PR but the fact that v2.0 was released 3 months ago (2023-04-17) and PHP 8.0 was released almost 3 years ago (2020-11-06), so missing static was not just an oversight, but a collective failure.
So how to proceed? The easiest "fix" for damage control would be to just remove the <interface> return types from the current v2.0 and re-tag it, so that future installs just ignore the return type while maintaining compatibility in both directions. As an alternative, limit v2,0 to PHP ^7.2.

@@ -129,7 +129,7 @@ public function getHeaderLine(string $name): string;
* @return static
* @throws \InvalidArgumentException for invalid header names or values.
*/
public function withHeader(string $name, $value): static;
public function withHeader(string $name, array|string $value): static;
Copy link
Author

Choose a reason for hiding this comment

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

Thinking about it, this should rather accept mixed to include scalar types, as some implementations are already implemented this way, ignoring the defined type in the docblock.

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