Skip to content
This repository has been archived by the owner on Jan 29, 2020. It is now read-only.

Fix: Return static instead of self #37

Merged
merged 1 commit into from
Jun 12, 2015
Merged

Fix: Return static instead of self #37

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions src/MessageTrait.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public function getProtocolVersion()
* new protocol version.
*
* @param string $version HTTP protocol version
* @return self
* @return static
Copy link
Member

Choose a reason for hiding this comment

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

Just joining the conversation: since the interface defines that a MessageInterface is returned, so should also the trait providing the concrete impl :-)

Copy link
Member

Choose a reason for hiding this comment

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

@Ocramius No, it does not. The interface indicates uses @return self, and indicates a new instance is returned. Considering that MessageInterface is not expected to be directly implemented, but rather implemented via one of RequestInterface or ResponseInterface, the question is whether self or static should be specified in the return annotation to ensure that IDEs, PHPDocumentor, and implementors are following the spec properly.

Copy link
Member

Choose a reason for hiding this comment

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

Considering that MessageInterface is not expected to be directly implemented

It's an interface. It will be implemented, and nobody says that it will necessarily be a RequestInterface or a ResponseInterface :-)

Copy link
Member

Choose a reason for hiding this comment

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

@Ocramius The issue here is the inheritance tree, which includes interfaces, and, more specifically, traits. Per this comment from @mvriel, self indicates that it's an instance of the class where the docblock is defined, while static is defined to return an instance of the current class, following inheritance.

So, a change is required here for sure. The question is whether changes are needed on the PSR-7 interfaces. I'd argue they are for MessageInterface and RequestInterface, as those two both are inherited by other interfaces in the specification.

*/
public function withProtocolVersion($version)
{
Expand Down Expand Up @@ -184,7 +184,7 @@ public function getHeaderLine($header)
*
* @param string $header Case-insensitive header field name.
* @param string|string[] $value Header value(s).
* @return self
* @return static
* @throws \InvalidArgumentException for invalid header names or values.
*/
public function withHeader($header, $value)
Expand Down Expand Up @@ -225,7 +225,7 @@ public function withHeader($header, $value)
*
* @param string $header Case-insensitive header field name to add.
* @param string|string[] $value Header value(s).
* @return self
* @return static
* @throws \InvalidArgumentException for invalid header names or values.
*/
public function withAddedHeader($header, $value)
Expand Down Expand Up @@ -265,7 +265,7 @@ public function withAddedHeader($header, $value)
* the named header.
*
* @param string $header Case-insensitive header field name to remove.
* @return self
* @return static
*/
public function withoutHeader($header)
{
Expand Down Expand Up @@ -301,7 +301,7 @@ public function getBody()
* new body stream.
*
* @param StreamInterface $body Body.
* @return self
* @return static
* @throws \InvalidArgumentException When the body is not valid.
*/
public function withBody(StreamInterface $body)
Expand Down
6 changes: 3 additions & 3 deletions src/RequestTrait.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ public function getRequestTarget()
* @link http://tools.ietf.org/html/rfc7230#section-2.7 (for the various
* request-target forms allowed in request messages)
* @param mixed $requestTarget
* @return self
* @return static
* @throws InvalidArgumentException if the request target is invalid.
*/
public function withRequestTarget($requestTarget)
Expand Down Expand Up @@ -193,7 +193,7 @@ public function getMethod()
* changed request method.
*
* @param string $method Case-insensitive method.
* @return self
* @return static
* @throws InvalidArgumentException for invalid HTTP methods.
*/
public function withMethod($method)
Expand Down Expand Up @@ -241,7 +241,7 @@ public function getUri()
* @link http://tools.ietf.org/html/rfc3986#section-4.3
* @param UriInterface $uri New request URI to use.
* @param bool $preserveHost Preserve the original state of the Host header.
* @return self
* @return static
*/
public function withUri(UriInterface $uri, $preserveHost = false)
{
Expand Down