[PSR-7] Opinions requested on potential patch

[Matthew Weier O'Phinney]

Hi!

I've been reviewing pull requests against the http-message repository, and had
given my thumbs-up to the following:

- https://github.com/php-fig/http-message/pull/50

That pull request update all `@return self` annotations to `@return static`. To
summarize the discussion, `@return static` is interpreted by both phpDoc and
most popular IDEs in such a way as to resolve to the class in which invocation
occurs, vs `@return self`, which resolves to the class in which it was defined.
Essentially, `@return self` is problematic when used in interfaces with
inheritance (such as `RequestInterface` inheriting from `MessageInterface`, or
`ServerRequestInterface` inheriting from `RequestInterface`, as the return value
might be of a looser type.

Additionally, `@return static` has a *connotation* that a new instance, not the
same instance, is being returned, which is the specific semantic we already
define within the PSR-7 interfaces.

However, Michael Cullum posed the following question: is such a change
considered a *fix* (because it updates the syntax to match intent) or a
*clarification* (which would require a vote)?

Opinions?

--
Matthew Weier O'Phinney
mweiero...@gmail.com
https://mwop.net/

[Woody Gilk]

I've always considered the `@return self` to be a bug in the PSR-7 and
as such would be a *fix*.
--
Woody Gilk
http://about.me/shadowhand

> --
> You received this message because you are subscribed to the Google Groups "PHP Framework Interoperability Group" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to php-fig+u...@googlegroups.com.
> To post to this group, send email to php...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/CAJp_myVMXBh24D5u6C54CuFUqqtymStcW6rW3MopTrJ7U0fcFw%40mail.gmail.com.
> For more options, visit https://groups.google.com/d/optout.

[Colin O'Dell]

What about `@return $this` instead?  http://stackoverflow.com/a/33437907/158766

Colin O'Dell

To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/CAGOJM6KCWbKmDfZF%3DWX25A9Qy3Mk5qHUbsWSr0GdY8%3D7jKZobA%40mail.gmail.com.

[Woody Gilk]

Colin, because it doesn't.

To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/CAJaRsPv%2BUZ%3DUdo2piHC6UBrHx44NVrmQYRt2xN9p68CgoTuQ2g%40mail.gmail.com.

For more options, visit https://groups.google.com/d/optout.

--

Woody Gilk

http://about.me/shadowhand

[Colin O'Dell]

Right, I wasn't thinking clearly.  Please disregard! :)

To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/CAGOJM6%2B2gv0c9jm44Aa1Cp8BZfWXS-SyaC7CE9TcNK4xvuxRww%40mail.gmail.com.

[Paul Jones]

> On Aug 3, 2016, at 13:54, Matthew Weier O'Phinney <mweiero...@gmail.com> wrote:
>
> is such a change
> considered a *fix* (because it updates the syntax to match intent) or a
> *clarification* (which would require a vote)?

IMO that's a fix; no vote needed.


--

Paul M. Jones
http://paul-m-jones.com

[guilher...@gmail.com]

Hi Matthew,

In this specific case (when considering an interface), @return self is wrong, as there is no way to return purely an interface instance. You just can't instantiate an interface.

However, we should never assume that @return self is wrong in all scenarios, and IDEs should make sure that overriding it is not wrong. Conceptually, I can have a @return static in an interface, but the implementation overrides it to be @return self. This is needed in the case we're doing something like this:

interface Factory {

    /** 

     *@return static 

     */

    function create();

}

class BarFactory implements Factory {

    /** 

     *@return self 

     */

    public function create() { 

        return new self(); // Yes, create the Factory as an instance for demonstration purposes... =)

    }

}

Cheers,

--
You received this message because you are subscribed to the Google Groups "PHP Framework Interoperability Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email to php-fig+u...@googlegroups.com.
To post to this group, send email to php...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/F92B068D-C3BD-4FC6-88F9-5AA9C38D2821%40gmail.com.

For more options, visit https://groups.google.com/d/optout.

--

Guilherme Blanco

Lead Architect at E-Block

[Alexander Makarov]

Looks like a fix to me.

[Krzysiek Piasecki]

In my opinion both 'self' and 'static' are for the implementors. These interfaces should be describe as always returning concrete interface.

 

-- 

Krzysiek Piasecki

[Théo FIDRY]

>Additionally, `@return static` has a *connotation* that a new instance, not the 
same instance, is being returned, which is the specific semantic we already 
define within the PSR-7 interfaces. 

From PSR-5:

>`self`, the element to which this type applies is of the same class as which the documented element is originally contained.
>`static`, the element to which this type applies is of the same class as which the documented element is contained, or when encountered in a subclass is of type of that subclass instead of the original class. This keyword behaves the same way as the keyword for late static binding (not the static method, property, nor variable modifier) as defined by PHP.

So not sure if this connotation is correct. At least from what is said in PSR-5, nothing is said about the return value being the same or a new instance. That said if it was the same instance, `$this` would be more appropriate right?

>In this specific case (when considering an interface), @return self is wrong, as there is no way to return purely an interface instance. You just can't instantiate an interface.

>However, we should never assume that @return self is wrong in all scenarios, and IDEs should make sure that overriding it is not wrong. Conceptually, I can have a @return static in an interface, but the implementation overrides it to be @return self. This is needed in the case we're doing something like this:

if you have

interface DummyInterface {

  /** @return self */

  public function create();

}

class Foo implements DummyInterface {

  public function create();

}

It would just mean that `Foo::create()` may return another implementation of `DummyInterface` than `Foo`, which is perfectly correct. However, if you specified `@return static` in `DummyInterface::create()`, then this case would become incorrect: you are forcing `Foo::create()` to return a `Foo` instance.

Well, PSR-5 is not finished, but that's still a reference.

[Ciaran McNulty]

On Wednesday, 3 August 2016 19:54:13 UTC+1, Matthew Weier O'Phinney wrote:

That pull request update all `@return self` annotations to `@return static`. To
summarize the discussion, `@return static` is interpreted by both phpDoc and
most popular IDEs in such a way as to resolve to the class in which invocation
occurs, vs `@return self`, which resolves to the class in which it was defined.

Is it a requirement that an instance of the same concreted is returned, or is anything implementing the interface acceptable?

 

Essentially, `@return self` is problematic when used in interfaces with
inheritance (such as `RequestInterface` inheriting from `MessageInterface`, or
`ServerRequestInterface` inheriting from `RequestInterface`, as the return value
might be of a looser type.

I don't understand this argument. Returning a subtype fits the return contract, doesn't it?

-C

[Matthew Weier O'Phinney]

On Sat, Aug 6, 2016 at 2:16 PM, Ciaran McNulty <ma...@ciaranmcnulty.com> wrote:
>
> On Wednesday, 3 August 2016 19:54:13 UTC+1, Matthew Weier O'Phinney wrote:
>>
>> That pull request update all `@return self` annotations to `@return
>> static`. To
>> summarize the discussion, `@return static` is interpreted by both phpDoc
>> and
>> most popular IDEs in such a way as to resolve to the class in which
>> invocation
>> occurs, vs `@return self`, which resolves to the class in which it was
>> defined.
>
> Is it a requirement that an instance of the same concreted is returned, or
> is anything implementing the interface acceptable?

The idea with PSR-7 is that a new instance of the same type is
returned. So, in the case of ServerRequestInterface::withHeader(), you
would expect another ServerRequestInterface instance be returned.
withHeader(), however, is defined as part of MessageInterface (and
ServerRequestInterface inherits from that).

>> Essentially, `@return self` is problematic when used in interfaces with
>> inheritance (such as `RequestInterface` inheriting from
>> `MessageInterface`, or
>> `ServerRequestInterface` inheriting from `RequestInterface`, as the return
>> value
>> might be of a looser type.
>
>
> I don't understand this argument. Returning a subtype fits the return
> contract, doesn't it?

See above. I'd expect ServerRequestInterface::withHeader() to return a
ServerRequestInterface instance, and not just *any* MessageInterface.
Since MessageInterface defines withHeader(), using `@return self`
there implies any MessageInterface implementation may be returned —
which means that a `ResponseInterface` return value would be valid! We
want to limit to the more specific subtype — in this case, the
ServerRequestInterface.

`@return static` properly denotes that we should resolve to the subtype.