Problem/Motivation
Following on from #3483037: [META] Add return types to hook implementations there are thousands of functions without return types. The next bunch of low hanging fruit here is procedural functions in the global namespace. These should be simple because there is no BC concern, i.e. they don't belong to an interface or class and therefore can't be extended.
Steps to reproduce
grep -oP 'Function \K[^D].*(?=\\\\\(\\\\\) has no return type specified)' core/.phpstan-baseline.php
That lists 255 58 45 functions with no return type.
Proposed resolution
- Address low hanging fruit with
AddFunctionVoidReturnTypeWhereNoReturnRectorandReturnTypeFromStrictNewArrayRector - Manually fix the rest
- Skip any where there might not be anything returned or it could be
mixed
Remaining tasks
Review current MR and agree on proposed resolution based on what's changed in the MR.
User interface changes
Introduced terminology
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|---|---|---|
| #9 | 3513326-nr-bot.txt | 91 bytes | needs-review-queue-bot |
Issue fork drupal-3513326
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- 3513326-global-return
changes, plain diff MR !11501
- 3513326-add-return-type
compare
Comments
Comment #2
lavanyatalwar commentedWorking on it.
Comment #3
mstrelan commentedComment #5
mstrelan commentedPushed my work so far using the rector rules in the IS to MR !11501. There are 67 functions remaining, using the steps to reproduce.
Comment #6
mstrelan commentedPushed some manual updates, this is down to 39 now. Definitely leaning towards getting void returns in first then splitting the rest.
Comment #7
quietone commentedI agree, voids are the easiest to review.
Comment #8
mstrelan commentedNow that the low-hanging fruit is committed I've rebased the MR and commented on some of the less obvious changes. I've updated the issue summary with a proposed resolution on how to split the remaining work. Setting NR for agreement on that approach, and for eyes on the MR.
Comment #9
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #10
mstrelan commentedRebased and updated the IS a little
Comment #12
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #14
mstrelan commentedThere are not as many functions left in the global namespace, so I think we should close this.
The grep in the issue summary is down to 45. Of those, 18 are deprecated, leaving 27. Most of those will be deprecated as part of #3566536: [meta] eliminate core .module files and #3097045: [META] Provide modern replacements for and deprecate the legacy include files, so this issue no longer makes sense.