#27098 closed defect (bug) (fixed)
Bundled Themes: ditch all uses of `@return void`
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | 3.9 | Priority: | normal |
Severity: | normal | Version: | 3.9 |
Component: | Bundled Theme | Keywords: | has-patch |
Focuses: | docs | Cc: |
Description
It appears that the use of @return void
should not be used for method or function DocBlocks:
- http://stackoverflow.com/a/2061559 (See: "The datatype should be a valid PHP type (int, string, bool, etc), a class name for the type of object returned, or simply "mixed"."
- http://en.wikipedia.org/wiki/PHPDoc (See: "This tag should not be used for constructors or methods defined with a void return type.")
What are your thoughts about removing this altogether?
Attachments (1)
Change History (25)
#4
follow-up:
↓ 5
@
11 years ago
- Milestone set to Awaiting Review
- Resolution invalid deleted
- Status changed from closed to reopened
Re-opening for more discussion.
The core inline docs standard -- which is our first-and-foremost reference here, doesn't recommend using @return void
anywhere outside of the bundled themes.
When we developed the standard last fall, the decision was made to ignore the use of @return void
in the bundled themes, as they had been utilizing that style since Twenty Ten. The @return void
tags aren't used in any consistency anywhere else in Core.
I think there's a good argument in favor of removing them altogether from the bundled themes -- this would bring that code in-line with the core standard.
Side note: It's important to remember that while the core inline documentation standard is largely based on and inspired by the phpDocumenter spec, it doesn't adhere to it 100 percent. Our standard is largely based on existing practices -- regardless of whether they might be considered "good" or "bad".
#5
in reply to:
↑ 4
@
11 years ago
Replying to DrewAPicture:
When we developed the standard last fall, the decision was made to ignore the use of
@return void
in the bundled themes, as they had been utilizing that style since Twenty Ten.
The decision reference: http://irclogs.wordpress.org/chanlog.php?channel=wordpress-sfd&day=2013-09-25&sort=asc#m48668
I think there's a good argument in favor of removing them altogether from the bundled themes -- this would bring that code in-line with the core standard.
+1.
Side note: It's important to remember that while the core inline documentation standard is largely based on and inspired by the phpDocumenter spec, it doesn't adhere to it 100 percent.
Also to add, that some of the recent changes are based on the draft PSR-5, of which many code editors and phpDocumentor itself doesn't follow fully yet.
#6
@
11 years ago
I don't know if it's worth patching older themes for it, but in general I'd follow core.
The basic argument for omitting @return void, for what it's worth, is rooted in the fact that we're always backwards compatible. For us, changing the return value of a function is pretty much a no-no. However, I have no qualms with adding a return value to a function that didn't have one previously, and we've done that a number of times. To me, @return void "bakes in" that the function returns no value, making it explicit. It's also possible to have the opposite feeling: that @return void specifically tells developers to not rely on a return value, at least until further notice. You could see it both ways.
#9
follow-up:
↓ 11
@
11 years ago
- Keywords 2nd-opinion added
Any more opinions on this? If we remove from Twenty Fourteen we should remove from Ten to Thirteen, also.
#10
@
11 years ago
I'm still in favor of keeping as is. At worst, it would be "overdocumented". We can omit it in new default themes.
#11
in reply to:
↑ 9
@
11 years ago
- Keywords 2nd-opinion removed
Replying to lancewillett:
Any more opinions on this? If we remove from Twenty Fourteen we should remove from Ten to Thirteen, also.
We made the exception in the inline docs standard for the whole of the default themes library. So if we remove them from one theme, we need to do them all.
Personally, I'd prefer we remove them from the entire library. Fewer exceptions make for less confusion down the road. It also solidifies the standard as the standard through and through.
This ticket was mentioned in IRC in #wordpress-themes by lancewillett. View the logs.
11 years ago
#13
@
11 years ago
- Keywords needs-refresh added
- Summary changed from Twenty Fourteen: Ditch all uses of `@return void`. to Bundled Themes: ditch all uses of `@return void`
Let's remove from all default themes.
#18
@
11 years ago
- Keywords needs-refresh removed
- Resolution set to fixed
- Status changed from reopened to closed
#19
follow-ups:
↓ 20
↓ 21
@
11 years ago
A possibly warranted use of @return void
is in cases where a function may return multiple types (which is all too common).
An example that I struggled with myself and which actually got committed in [25697] (for the function _fix_attachment_links()
) uses
@return void|int|WP_Error
In this case void
is returned if nothing is fixed, 0 or WP_Error on update failure, and the post ID on update success. (One could argue that this is poor design, but thankfully the function is not meant for public access).
I we cannot use void
in this example, we are left with @return mixed
, which is much less descriptive.
When the only return is void
(i.e. "nothing") there is not much hurt in omitting @return void
, but maybe we can officially allow it in "mixed" situations?
#20
in reply to:
↑ 19
@
11 years ago
Replying to jond3r:
When the only return is
void
(i.e. "nothing") there is not much hurt in omitting@return void
, but maybe we can officially allow it in "mixed" situations?
A recent example where void
was used in a @return
, specifically as
* @return bool|void
was in [27575]. So I think we can say that void
is allowed in mixed cases.
#21
in reply to:
↑ 19
;
follow-up:
↓ 22
@
11 years ago
Replying to jond3r:
A possibly warranted use of
@return void
is in cases where a function may return multiple types (which is all too common).
If a return
tag is present, then it's never void. It's either a standard type, or null
, even when the null
is not explicit e.g. return;
.
An example that I struggled with myself and which actually got committed in [25697]
That was committed in June 2013, while the discussion and decision to tidy up on the voids was not done until September, so it's not a sound example of agreement. Likewise, it doesn't follow the now-agreed coding standards of using (previously optional) braces.
@return void|int|WP_ErrorIn this case
void
is returned if nothing is fixed.
No, null
is returned.
"If no parameter is supplied, then the parentheses must be omitted and NULL will be returned." - http://www.php.net/manual/en/function.return.php
Null is also returned when there is no return
control structure, but that's a different debate.
A recent example where void was used in a @return, specifically as
- @return bool|void
was in [27575]. So I think we can say that void is allowed in mixed cases.
It's not 100% clearcut, but I'd say that documentation is wrong. There is at least one return
(so it can't be void) in the function, just not for a particular path - so in that case, it returns bool|null
.
#22
in reply to:
↑ 21
;
follow-up:
↓ 23
@
11 years ago
Replying to GaryJ:
This is interesting.
When I read the documentation about the return statement on php.net (your link above), it does say that NULL
is returned: both if no return statement is given, and for a return
without an argument.
I have a background in C/C++, where this is not true: void
is returned in those cases. In C/C++ void
is very distinct from NULL
(literally 'nothing' versus the pointer address NULL (often defined as the integer 0)). In PHP, NULL
apparently is a special value distinct from the integer 0. I had somewhat casually assumed that PHP followed C/C++ in this regard. Apparently not.
On the other hand, I might be correct anyway about the documentation of the return
value. Reading the documentation of phpDoc http://www.phpdoc.org/docs/latest/references/phpdoc/types.html , under the section 'Keyword': '9. void' and '10. null', which I actually had read and which I based my comments above on, it seems that @return void
should be used either if no return statement is given or if return
without an argument is used.
So the interesting point here is that phpDocumentor sets itself apart from php.net and more adheres to the standard of C/C++ (and Java for that matter).
#23
in reply to:
↑ 22
;
follow-up:
↓ 24
@
11 years ago
The key document to read is https://github.com/php-fig/fig-standards/pull/169 (the in-progress pull request for PSR-5). It too RECOMMENDS that @return void
should be used when no return
is present, but I have my own view on that: http://code.garyjones.co.uk/why-return-void-is-wrong .
The overridding factor here is historical precedent - while the rest of PHP might be using @return void
(perhaps originating from those with a similar background as yours), WP was only doing so in limited circumstances and doing it inconsistently so the cleanest and semantically correct option was to not use it at all.
#24
in reply to:
↑ 23
@
11 years ago
Replying to GaryJ:
The overridding factor here is historical precedent - while the rest of PHP might be using
@return void
(perhaps originating from those with a similar background as yours), WP was only doing so in limited circumstances and doing it inconsistently so the cleanest and semantically correct option was to not use it at all.
Yep. And with that, I'm going to suggest that anyone interested continue this conversation at the weekly inline docs chat, held on Wednesdays at 19:00 UTC in #wordpress-sfd. I'll be happy to add it to the agenda.
We're also open to discussions on the make/docs P2 from interested community members.
In phpDocumentor:
So we're just being explicit here.