symfony: Reconsider the CS rule for returning null in functions
currently, our CS mandate to use return; to return null from a function.
But there is 2 different cases actually:
- a function without any return value (returning void), for which
return;makes sense - a function returning something or
null, for whichreturn null;would be more meaningful
The original discussion said that PHP had no concept of void and so that our CS should not distinguish these 2 cases (and so should have a single rule). But PHP 7.1 now has such concept: https://wiki.php.net/rfc/void_return_type
I think we should revisit this rule in our coding standards, to distinguish functions returning void and functions returning something or null.
what do you think ?
About this issue
- Original URL
- State: closed
- Created 9 years ago
- Reactions: 13
- Comments: 29 (28 by maintainers)
Commits related to this issue
- minor #6614 Updated the CS rule about return null; and return; (javiereguiluz) This PR was submitted for the master branch but it was merged into the 2.7 branch instead (closes #6614). Discussion --... — committed to symfony/symfony-docs by xabbuh 8 years ago
- minor #32144 add missing return type in User class (Tobion) This PR was merged into the 5.0-dev branch. Discussion ---------- add missing return type in User class | Q | A | ----------... — committed to symfony/symfony by deleted user 5 years ago
- minor #32143 use proper return types in ErrorHandler and ArgumentResolver (Tobion) This PR was merged into the 4.4 branch. Discussion ---------- use proper return types in ErrorHandler and Argument... — committed to symfony/symfony by fabpot 5 years ago
- minor #33009 Fix inconsistent return points (derrabus) This PR was merged into the 3.4 branch. Discussion ---------- Fix inconsistent return points | Q | A | ------------- | --- | Bran... — committed to symfony/symfony by nicolas-grekas 5 years ago
- minor #33252 Fix inconsistent return points (derrabus) This PR was merged into the 3.4 branch. Discussion ---------- Fix inconsistent return points | Q | A | ------------- | --- | Bran... — committed to symfony/symfony by nicolas-grekas 5 years ago
- minor #33254 Fix inconsistent return points (derrabus) This PR was merged into the 4.3 branch. Discussion ---------- Fix inconsistent return points | Q | A | ------------- | --- | Bran... — committed to symfony/symfony by nicolas-grekas 5 years ago
@nicolas-grekas:
It helps prevent mistakes, and prevent calling code rom depending on a return value.
https://github.com/symfony/symfony/blob/41234653d5f67fb512be1d19029b78eb21f526f1/src/Symfony/Component/Console/EventListener/ErrorListener.php#L43
is really bad style anyway…
It should be rewritten as:
Technically, we could totally update the existing code in an automated way once we update PHP-CS-Fixer to cover the new standard. And I would even say we should do it, otherwise people will receive warnings from fabbot.io all the time once the automated tool is aware of the new rule
Or, we can decide it’s not worth adding
void. I think it provides nothing personally.Not a Symfony decider, but I have always hated how Symfony blurred
nullreturns andvoidreturns, even if separating such was only a “convention for clarity” prior to PHP 7.1. 👍 To the change. Wasn’t this already accepted, though?since php 7.1 will force us to do so in master, this should help merges into master. So 👍, even if it’s a boring task (it’s not only having a command to help, but also doing to job from 2.7 up to master, potentially handle cross version test failures and dealing with conflicts, and forcing everyone to rebase their PRs meanwhile…)
@symfony/deciders I’m reopening the issue for this discussion, now that
return;andreturn null;are considered different in PHP 7.1+