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-sniffs

Drupal.Commenting.FunctionComment.InvalidReturn
Drupal.Commenting.FunctionComment.VoidReturn

Issue fork drupal-2722621

Command icon 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:

Comments

anoopjohn created an issue. See original summary.

anoopjohn’s picture

Status: Active » Needs review
StatusFileSize
new6.01 KB

Please find attached a patch for the same

Status: Needs review » Needs work
anoopjohn’s picture

Status: Needs work » Needs review

Not sure why the patch failed testing. I just downloaded and tested against latest head and it was working. Marking for review again.

mile23’s picture

Re-running test.

mile23’s picture

Status: Needs review » Needs work
+++ b/core/modules/field/tests/src/Kernel/FieldKernelTestBase.php
@@ -137,10 +137,10 @@ protected function entitySaveReload(EntityInterface $entity) {
+   * Note: Function does not return anything.
+   *
    * @param \Drupal\Core\Entity\EntityInterface $entity
    *   The entity to save.
-   *
-   * @return void

+++ b/core/modules/user/src/UserDataInterface.php
@@ -34,6 +34,8 @@ public function get($module, $uid = NULL, $name = NULL);
+   * Note: Function does not return anything.
+   *

@@ -42,14 +44,14 @@ public function get($module, $uid = NULL, $name = NULL);
+   * Note: Function does not return anything.
+   *

I don't think we need to add these notes. Also, strictly speaking, all methods return NULL, which isn't the same as 'nothing.' :-)

anoopjohn’s picture

Thanks for the review Mile23. I have updated the patch as per your recommendation. Please find attached the revised patch and the interdiff.

anoopjohn’s picture

Status: Needs work » Needs review
mile23’s picture

Status: Needs review » Needs work
Issue tags: +Needs reroll
anoopjohn’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
StatusFileSize
new5.33 KB
new0 bytes

Thanks again for the review Mile23. Please find attached the rerolled patch. I was wondering if it would be alright to give two patches - one for the phpcs change and the other with the remaining? The phpcs one which is the one which fails all the time and it is probably easier to delete the line than go through the re-roll process which does not add any value.

mile23’s picture

Status: Needs review » Needs work

Not sure what you mean about the two patches...

Current patch gives me this:

FILE: ...l/pj2/drupal/core/lib/Drupal/Component/Datetime/DateTimePlus.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 458 | ERROR | Expected "bool" but found "boolean" for function return
     |       | type (Drupal.Commenting.FunctionComment.InvalidReturn)
----------------------------------------------------------------------

Also:

+++ b/core/lib/Drupal/Core/StreamWrapper/PhpStreamWrapperInterface.php
@@ -64,7 +64,7 @@ public function rmdir($path, $options);
   /**
-   * @return void
+   * Closes stream and does not return anything.
    */
   public function stream_close();

We don't need to say that it doesn't return anything.

anoopjohn’s picture

Thanks for the review @Mile23

I meant creating two patches for such fixes related to each coding standards issue. One patch with the change in phpcs.xml.dist and the other patch with changes in the rest of the files. The rerolling is usually only required for the phpcs.xml.dist part of the patch

Please find attached the revised patch and the interdiff.

anoopjohn’s picture

Status: Needs work » Needs review
klausi’s picture

Status: Needs review » Reviewed & tested by the community

Looks good, verified manually that a run of phpcs does not throw any errors with the changed phpcs.xml.dist file.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 5f925ef and pushed to 8.1.x and 8.2.x. Thanks!

FILE: ...sk/dev/drupal/core/modules/field/src/Tests/FieldUnitTestBase.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 146 | ERROR | If there is no return value for a function, there must not
     |       | be a @return tag.
     |       | (Drupal.Commenting.FunctionComment.VoidReturn)
----------------------------------------------------------------------

Fixed on 8.1.x during commit.

alexpott’s picture

  • alexpott committed 700452b on 8.2.x
    Issue #2722621 by anoopjohn: Fix Drupal.Commenting.FunctionComment....

  • alexpott committed ca38d94 on 8.1.x
    Issue #2722621 by anoopjohn: Fix Drupal.Commenting.FunctionComment....

Status: Fixed » Closed (fixed)

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

neclimdul made their first commit to this issue’s fork.