Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create a fixer fixing the return null vs void return statements #1916

Closed
stof opened this issue May 26, 2016 · 8 comments
Closed

Create a fixer fixing the return null vs void return statements #1916

stof opened this issue May 26, 2016 · 8 comments
Labels
kind/feature request status/stale topic/types FQCNs and types used in signatures (class-like, function, property)

Comments

@stof
Copy link
Contributor

stof commented May 26, 2016

This fixer should handle 2 things (may be 2 different fixers though):

  • converting return; to return null; when the function is not a void one (i.e. it has another return statement returning something explicitly. This may also read the phpdoc to detect more case but this is optional)
  • converting return null; to return; in void-returning functions (detecting through the vod return typehint or through @return void. Detecting cases returning only null in the function would detect false positives when writing a null object).

such fixer could be in the Symfony ruleset as we changed our CS

@SpacePossum
Copy link
Contributor

If we check the code and the PHPDoc of the function/method and these are in conflict than how should we proceed, fix the return value or the PHPDoc?

@stof
Copy link
Contributor Author

stof commented May 26, 2016

@SpacePossum you mean if the phpdoc says @return void and the code contains return statements returning something else than null ? The method should be considered as a non-void-returning function, to keep the existing behavior of the code. Making the phpdoc win over the existing code would be risky.

@SpacePossum
Copy link
Contributor

related discussion @ #1638

@julienfalque
Copy link
Member

IMO implementing a new fixer for this is not worth it: returning something in a void function or returning nothing in a non-void function is something that tools like PHPStan already report anyway.

@Wirone Wirone added the status/to verify issue needs to be confirmed or analysed to continue label May 16, 2023
@Wirone
Copy link
Member

Wirone commented Jun 3, 2023

@julienfalque they report it, but they don't fix it 😉.

In terms of conflicts between code, native return type and phpDoc I believe fixer should use only 2 latter to determine return type, with priorities:

  • native return type
  • phpDoc

If void is used, empty return; should be enforced, if nullable type is used then all empty returns should be converted to return null;. Fixer does not have full static analysis context and possible returned values can't be inferred from the code, so we can't find out if null is returned (not counting explicit return null;).

This fixer should be risky IMHO.

@Wirone Wirone added topic/types FQCNs and types used in signatures (class-like, function, property) and removed status/to verify issue needs to be confirmed or analysed to continue labels Jun 3, 2023
@github-actions

This comment was marked as outdated.

Copy link

Since this issue has not had any activity within the last 90 days, I have marked it as stale.

The purpose of this action is to enforce backlog review once in a while. This is mostly for maintainers and helps with keeping repository in good condition, because stale issues and PRs can accumulate over time and make it harder for others to find relevant information. It is also possible that some changes has been made to the repo already, and issue or PR became outdated, but wasn't closed for some reason. This action helps with periodic review and closing of such stale items in automated way.

You may let maintainers handle this or verify current relevancy by yourself, to help with re-triage. Any activity will remove stale label so it won't be automatically closed at this point.

I will close it if no further activity occurs within the next 30 days.

Copy link

The fact this was automatically closed doesn't mean that the idea got rejected - it simply didn't get any priority for way too long to keep it open. If you're still interested in this, please let as know, we can consider re-opening it.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Feb 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature request status/stale topic/types FQCNs and types used in signatures (class-like, function, property)
Projects
None yet
Development

No branches or pull requests

4 participants