This is a part of the attempt to fix #2572645: [Meta] Fix 'Drupal.Commenting.FunctionComment' coding standard

This issue is created to tackle the sub-sniff

Drupal.Commenting.FunctionComment.InvalidReturnNotVoid

Some of the issues reported by this sub-sniff is because the specific test in the FunctionComment sniff was not checking for the presence of NULL as a possible return value in the @return comment. This issue has been reported under the coder queue and a patch submitted for the same #2722695: Drupal.Commenting.FunctionComment.InvalidReturnNotVoid raised even when null is a possible return value. Such issues have not been fixed in this patch.

To review:

$ composer require drupal/coder
$ ./vendor/bin/phpcs --config-set installed_paths /PATH/TO/drupal/vendor/drupal/coder/coder_sniffer/
$ cd core
$ ../vendor/bin/phpcs -p -s

Should result in no errors found.

CommentFileSizeAuthor
#17 drupal-coding-standards-2722699-17.patch6.56 KBmfernea
#7 diff-2722699-5-7.txt739 bytesanoopjohn
#7 drupal-coding-standards-function-comment-InvalidReturnNotVoid-2722699-7.patch5.22 KBanoopjohn
#5 interdiff-2722699-2-5.txt3.57 KBanoopjohn
#5 drupal-coding-standards-function-comment-InvalidReturnNotVoid-2722699-5.patch5.24 KBanoopjohn
#2 drupal-coding-standards-function-comment-InvalidReturnNotVoid-2722699-2.patch5.18 KBanoopjohn

Comments

anoopjohn created an issue. See original summary.

anoopjohn’s picture

anoopjohn commented
Status: Active » Needs review
StatusFileSize
new drupal-coding-standards-function-comment-InvalidReturnNotVoid-2722699-2.patch5.18 KB

Please find attached the patch for the same.

anoopjohn’s picture

anoopjohn commented
Issue summary: View changes
mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Status: Needs review » Needs work 
+++ b/core/modules/dblog/src/Controller/DbLogController.php
@@ -298,8 +298,9 @@ public function eventDetails($event_id) {
+   * @return array|null
+   *   An associative array with keys 'where' and 'args'
+   *   or NULL if there were no filters set.

The description line should extend all the way out to 80 characters before wrapping.

Same for many others.

anoopjohn’s picture

anoopjohn commented
Status: Needs work » Needs review
StatusFileSize
new drupal-coding-standards-function-comment-InvalidReturnNotVoid-2722699-5.patch5.24 KB
new interdiff-2722699-2-5.txt3.57 KB

Thanks for the review Mile23. I have made the changes as recommended and have attached the patch.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Status: Needs review » Needs work 
+++ b/core/modules/system/src/Tests/Module/ModuleTestBase.php
@@ -87,8 +87,9 @@ function assertModuleTablesDoNotExist($module) {
+   * @return bool|NULL

Lower-case null.

anoopjohn’s picture

anoopjohn commented
StatusFileSize
new drupal-coding-standards-function-comment-InvalidReturnNotVoid-2722699-7.patch5.22 KB
new diff-2722699-5-7.txt739 bytes

Thanks again for the review Mile23. I have made the corrections as suggested. Sorry I had missed this in the earlier fix. Please find attached the re-rolled patch. Had to fix conflicts in phpcs.xml.dist as well. Interdiff was failing so attaching diff.

anoopjohn’s picture

anoopjohn commented
Status: Needs work » Needs review
mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Status: Needs review » Needs work

Looks like a bunch of errors still in there, along with unrelated ThrowsComment errors:

$ ../vendor/bin/phpcs -p -s

[...]


FILE: ...sers/paulmitchum/projects/drupal8/core/includes/install.core.inc
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 1825 | ERROR | Function return type is not void, but function is
      |       | returning void here
      |       | (Drupal.Commenting.FunctionComment.InvalidReturnNotVoid)
----------------------------------------------------------------------


FILE: ...ects/drupal8/core/lib/Drupal/Component/Datetime/DateTimePlus.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 635 | ERROR | Function return type is not void, but function is
     |       | returning void here
     |       | (Drupal.Commenting.FunctionComment.InvalidReturnNotVoid)
----------------------------------------------------------------------


FILE: ...ts/drupal8/core/modules/dblog/src/Controller/DbLogController.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 308 | ERROR | Function return type is not void, but function is
     |       | returning void here
     |       | (Drupal.Commenting.FunctionComment.InvalidReturnNotVoid)
----------------------------------------------------------------------


FILE: ...chum/projects/drupal8/core/modules/forum/src/Tests/ForumTest.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 545 | ERROR | Function return type is not void, but function is
     |       | returning void here
     |       | (Drupal.Commenting.FunctionComment.InvalidReturnNotVoid)
----------------------------------------------------------------------


FILE: ...core/modules/options/src/Plugin/Field/FieldType/ListItemBase.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 202 | ERROR | Function return type is not void, but function is
     |       | returning void here
     |       | (Drupal.Commenting.FunctionComment.InvalidReturnNotVoid)
----------------------------------------------------------------------


FILE: ...tchum/projects/drupal8/core/modules/simpletest/simpletest.module
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 700 | ERROR | Function return type is not void, but function is
     |       | returning void here
     |       | (Drupal.Commenting.FunctionComment.InvalidReturnNotVoid)
----------------------------------------------------------------------


FILE: .../drupal8/core/modules/system/src/Tests/Module/ModuleTestBase.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 97 | ERROR | Function return type is not void, but function is returning
    |       | void here
    |       | (Drupal.Commenting.FunctionComment.InvalidReturnNotVoid)
----------------------------------------------------------------------


FILE: .../core/modules/views/src/Plugin/views/filter/FilterPluginBase.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 1211 | ERROR | Function return type is not void, but function is
      |       | returning void here
      |       | (Drupal.Commenting.FunctionComment.InvalidReturnNotVoid)
----------------------------------------------------------------------


FILE: ...tchum/projects/drupal8/core/modules/views/src/ViewExecutable.php
----------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
----------------------------------------------------------------------
 1185 | ERROR | Function return type is not void, but function is
      |       | returning void here
      |       | (Drupal.Commenting.FunctionComment.InvalidReturnNotVoid)
 1445 | ERROR | Function return type is not void, but function is
      |       | returning void here
      |       | (Drupal.Commenting.FunctionComment.InvalidReturnNotVoid)
----------------------------------------------------------------------


FILE: ...ers/paulmitchum/projects/drupal8/core/modules/views/views.module
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 742 | ERROR | Function return type is not void, but function is
     |       | returning void here
     |       | (Drupal.Commenting.FunctionComment.InvalidReturnNotVoid)
----------------------------------------------------------------------


Time: 6 mins, 30.92 secs; Memory: 126.5Mb
anoopjohn’s picture

anoopjohn commented
Status: Needs work » Needs review

Thanks for the Review again Mile23. There are a few false positives for InvalidReturnNotVoid. This has been raised in the coder issue queue and a patch has been submitted there #2722695: Drupal.Commenting.FunctionComment.InvalidReturnNotVoid raised even when null is a possible return value. Once that patch goes in then there would not be any more InvalidReturnNotVoid errors in core. @throws errors are not addressed in this patch as we are trying to fix errors by each sniff or sub-sniff in separate issues as per the plan.

dawehner’s picture

dawehner
Primary language German
commented

This false positives are fixed as part of 8.2.8, but this has introduced other exceptions, so we basically need to wait until we fix those regressions and then continue with this issue?

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Status: Needs review » Postponed

So I'd say this should be postponed on #2744463: Add phpcs, coder 8.2.8 as --dev requirements for drupal/core where we'll decide which version of Coder to use.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

mfernea’s picture

mfernea commented
Status: Postponed » Needs work
mfernea’s picture

mfernea commented
Status: Needs work » Needs review
StatusFileSize
new drupal-coding-standards-2722699-17.patch6.56 KB

Here is the updated patch.

martin107’s picture

martin107 commented
Status: Needs review » Reviewed & tested by the community

I am going to sit on this issue until committed - and provide reviews on a quick turn around.

Here is a checklist ... Here is what I can say after a careful visual scan of the patch

1) The patch applies :)
2) All the text description of the return type has been updated with appropriate words.
3) The phpcs.xml.dist has been correctly modified.
4) There are no coding standard warnings issue by the last testbot run

This looks good to me.

  • catch committed 73347e1 on 8.5.x 
    Issue #2722699 by anoopjohn, mfernea, Mile23, martin107: Fix Drupal....

  • catch committed 040649d on 8.4.x 
    Issue #2722699 by anoopjohn, mfernea, Mile23, martin107: Fix Drupal....

catch credited catch.

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.