Note that if this is accepted, the existing return fixer in PHP-CS-Fixer will need to be disabled (see PHP-CS-Fixer/PHP-CS-Fixer#1638 for a discussion about it), and a new fixer might be created to automate the new rule (allowing to update our whole codebase in an automated way).

IIUC, return; is only used when the function must return an early void, but if the function doesn't return anything, return must be omitted: https://symfony.com/doc/current/contributing/code/standards.html#structure

Use just return; instead of return null; when a function must return void early;

So, just as a recap for your 2 points, this should be the current CS:

• a function without any return value (returning void)

return statements are fully absent.

• a function returning something or null

return; statement only present if the return is required before the end of function's flow (early return inside a condition by instance).

As far as I can see, both cases are taken into account with the current spec.

IIUC, return; is only used when the function must return an early void, but if the function doesn't return anything, return must be omitted

anything or void is the same. It is just that PHP calls this void (because other languages call it void too)

As far as I can see, both cases are taken into account with the current spec.

you are confusing the case of returning null early and returning void early. Your comment assumes they are the same. They are not (and this is precisely my point).

You're right @stof, I misunderstood your point. Thank you for the clarification.

I never liked this specialty of Symfony's coding standard, so if you want to change it: 👍

There's no direct need to change it, though. Code with the current CS will run fine under php 7.1 as long as a function is not declared as void. And it will take some time untill Symfony can set php 7.1 as a minimum requirement in order to make use of the void return type – we're probably talking about Symfony 5 here.

But you're probably right. If php introduces the concept of void type functions, Symfony should probably embrace this concept even before making use of the actual feature.

But you're probably right. If php introduces the concept of void type functions, Symfony should probably embrace this concept even before making use of the actual feature.

PHP 7.1 has already done this.

👍

👍 Sounds reasonable to me.

👍

For what it's worth, 👎 from me because of the fact that any assignment of the result of the function will still be null.

👍 for clarity.

👍

We should also reconsider phpdoc_no_empty_return, which will remove from phpdocs @return void and @return null annotations.

Indeed. I've always not liked that fixer.

👍 i like it

I'm closing this as "fixed" because the community has accepted it and there is nothing else to do:

1. The docs have been updated: Updated the CS rule about return null; and return; symfony-docs#6614
2. We (probably) won't update the existing code to avoid merging conflicts.
3. We'll use this new rule for the new code added from now on.

We (probably) won't update the existing code to avoid merging conflicts.

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

Please extend needed actions if needed:
PHP-CS-Fixer/PHP-CS-Fixer#1638 (comment)

@symfony/deciders I'm reopening the issue for this discussion, now that return; and return null; are considered different in PHP 7.1+

Not a Symfony decider, but I have always hated how Symfony blurred null returns and void returns, 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...)

Inconsistent return points in functions are going to bite us if we want to add return types to those functions in Symfony 5.0:

• As soon as we add a return type, we cannot mix return; and return $someValue; within the same function anymore. https://3v4l.org/RBoMM
• If we add a a return type other than void, php will not implicitly return null anymore when reaching the end of a function, but emit a TypeError instead. https://3v4l.org/5GrdN
• If we add void as return type, returning with a value will result in a fatal error, even if we use the result of another void method. https://3v4l.org/B5jgp

I have seen all three cases in Symfony's codebase. I don't know if we can fix all inconsistent return points automatically. The third case needs a different strategy than the other two and an automated tool would have a hard time detecting that case.

But at least PhpStorm is pretty good at finding them. On the current 3.4 branch, PhpStorm detects 353 issues. I've browsed them and I've only found a handful of false positives (mostly fixtures and, let's say, interesting usages of for loops).

I'd say, it's worth fixing those issues in 3.4 and 4.3 now to make adding return types to Symfony 5 easier.

Or, we can decide it's not worth adding void. I think it provides nothing personally.

Or, we can decide it's not worth adding void. I think it provides nothing personally.

Fair enough, in that case we could ignore item 3 on my list and would treat PhpStorm warnings on lines like this one like a false positive.

That wouldn't make fixing the other two cases easier, though.

@nicolas-grekas:

it's not worth adding void. I think it provides nothing personally.

It helps prevent mistakes, and prevent calling code rom depending on a return value.

is really bad style anyway...

It should be rewritten as:

             $this->logger->error('An error occurred while using the console. Message: "{message}"', ['exception' => $error, 'message' => $error->getMessage()]);

             return;