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

fluent interface return type vs return self #7193

Open
jaapio opened this issue Feb 8, 2015 · 13 comments
Open

fluent interface return type vs return self #7193

jaapio opened this issue Feb 8, 2015 · 13 comments

Comments

@jaapio
Copy link
Contributor

jaapio commented Feb 8, 2015

When looking in the api documentation of zf2 I found sometimes @return self is used and in other cases @return {nameOfTheClass} is used. Both situations are resulting in the same documentation. But it would be nice if it would be the same for every fluent interface.

@samsonasik
Copy link
Contributor

as @weierophinney suggestion in my old PRs, it shoud be return self

@marc-mabe
Copy link
Member

@samsonasik @weierophinney @return self is a good thing but in the case for fluent interface it's returning $this so it should be @return $this or @return static see http://www.phpdoc.org/docs/latest/guides/types.html#keywords

@Ocramius
Copy link
Member

Folks, $this is not a type :-\

@tdutrion
Copy link

Not self either... Should we set the actual class name then? That's the most semantically true version...
Although, based on PHPdoc documentation it appears that $this should be used (I was also a bit surprised at first...)

@marc-mabe
Copy link
Member

$this as documented return type is allowed by phpDocumentor but @Ocramius is right - it's not a type and don't know how others (like IDEs) will interpret this. 'static' should be a better keyword here.

@Ocramius
Copy link
Member

self is a valid type (and alias) in php.

On 13 Oct 2016 10:09, "Marc Bennewitz" notifications@github.com wrote:

$this as documented return type is allowed by phpDocumentor but @Ocramius
https://github.com/Ocramius is right - it's not a type and don't know
how others (like IDEs) will interpret this. 'static' should be a better
keyword here.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#7193 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAJakKUNYIkj5U0iyJ-GX-GgQYs9Klsiks5qzeckgaJpZM4DdWKn
.

@marc-mabe
Copy link
Member

@Ocramius static is also a valid type (and alias) in PHP. The only point is that right now PHP doesn't allow static as return type because of issues on implementation site.

The RFC says:

Covariant return types are considered to be type sound and are used in many other languages. This RFC originally proposed covariant return types but was changed to invariant because of a few issues. It is possible to add covariant return types at some point in the future.

But all IDEs I know and phpDocumentator understand @return static so the following should be fine in my opinion (of course the real return type can't be used because we support older PHP versions as well):
https://3v4l.org/VdoHE

@tdutrion
Copy link

So what's what? Should we go with static or self? Given the documentation of PHPDoc, static is maybe more what we're after, but not sure...

@tdutrion
Copy link

ping? What you guys think then? That's a small and boring contribution but could be better for consistency across the codebase.

@marc-mabe
Copy link
Member

marc-mabe commented Nov 5, 2016

@tdutrion @Ocramius I did a bit research for IDE completions and it looks looks like that @return $this was never supported by a lot of IDEs and @return static was supported by more complete IDEs but reverted it because PHP implemented return types but not this feature. E.g. It worked in PHPStorm 7 but not iin PHPStorm 2016 :(

So the most safest is to write the class name.

Thanks

@tdutrion
Copy link

tdutrion commented Nov 8, 2016

I did start an update of my previous PR, will continue over the next weeks :)

@weierophinney
Copy link
Member

The other source to look at for acceptable annotation values is PHPDocumentor. Per the documentation on types:

self, the element to which this type applies is of the same Class, or any of its children, as which the documented element is originally contained.

In the case of fluent interfaces self is almost certainly what we want to use.


Further, I'd argue we should only use the class name as a return type if the method will return a new instance.

@tdutrion
Copy link

@weierophinney Should you discuss this with @marc-mabe, @akrabat and @ezimuel who accepted 3 of the PRs? All arguments so far seem reasonable to me and my take is choose one and stick to it just as for code style, so whatever please you is fine for me :)

I am not against changing again, but really do not want to annoy anyone with such a trivial thing...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants