I've run across an issue in drupal 8.x form.inc where we have @code...@endcode within the last @param for a function doc, here's the current code:

 * @param ...
 *   Any additional arguments are passed on to the functions called by
 *   drupal_form_submit(), including the unique form constructor function.
 *   For example, the node_edit form requires that a node object be passed
 *   in here when it is called. Arguments that need to be passed by reference
 *   should not be included here, but rather placed directly in the $form_state
 *   build info array so that the reference can be preserved. For example, a
 *   form builder function with the following signature:
 *   @code
 *   function mymodule_form($form, &$form_state, &$object) {
 *   }
 *   @endcode
 *   would be called via drupal_form_submit() as follows:
 *   @code
 *   $form_state['values'] = $my_form_values;
 *   $form_state['build_info']['args'] = array(&$object);
 *   drupal_form_submit('mymodule_form', $form_state);
 *   @endcode

Here are the errors i get for the code above:

  697 | ERROR   | Last parameter comment requires a blank newline after it
  705 | ERROR   | Expected 1 space between asterisk and tag; 3 found
  708 | ERROR   | Expected 1 space between asterisk and tag; 3 found
  710 | ERROR   | Expected 1 space between asterisk and tag; 3 found
  714 | ERROR   | Expected 1 space between asterisk and tag; 3 found

It seems that when the last @param ends with a @code block the sniff for the newline after the @param block isn't working correctly (in this case there isn't a line space after the last @param, but even when you put on in after the @endcode you still get the same error reported). The sniff seems to expect the newline after "...function with the following signature:" instead of after the @endcode.

And how are we meant to format the @code and @endcode? Indented 3 spaces to match the other content of the @param block as shown above, or indented 1 space to the same level as @param:

 * @param ...
 *   Any additional arguments are passed on to the functions called by
 *   drupal_form_submit(), including the unique form constructor function.
 *   For example, the node_edit form requires that a node object be passed
 *   in here when it is called. Arguments that need to be passed by reference
 *   should not be included here, but rather placed directly in the $form_state
 *   build info array so that the reference can be preserved. For example, a
 *   form builder function with the following signature:
 * @code
 *   function mymodule_form($form, &$form_state, &$object) {
 *   }
 * @endcode
 *   would be called via drupal_form_submit() as follows:
 * @code
 *   $form_state['values'] = $my_form_values;
 *   $form_state['build_info']['args'] = array(&$object);
 *   drupal_form_submit('mymodule_form', $form_state);
 * @endcode

The above code fixes the "Expected 1 space between asterisk and tag; 3 found" errors

I checked the api definition and the code blocks are displayed correctly within the @param definition: https://api.drupal.org/api/drupal/core%21includes%21form.inc/function/drupal_form_submit/8

(As a side note, there is a code block missing at the end of that definition tho, after for example:)

 * For example:
 * @code
 * // register a new user
 * $form_state = array();
 * $form_state['values']['name'] = 'robo-user';
 * $form_state['values']['mail'] = 'robouser@example.com';
 * $form_state['values']['pass']['pass1'] = 'password';
 * $form_state['values']['pass']['pass2'] = 'password';
 * $form_state['values']['op'] = t('Create new account');
 * drupal_form_submit('user_register_form', $form_state);
 * @endcode

Comments

Mile23’s picture

Title: How to format @code block within last @param block » False-positives for @code/@endcode
Component: Review/Rules » Coder Sniffer
Category: Task » Bug report
Priority: Minor » Normal

@code and @endcode blocks are showing some false-positives here: #2707371: Fix several errors in the 'Drupal.Commenting.DocComment' coding standard

The sniffer thinks they are further tags like @param, so it says things like this:

FILE: ...s/drupal8/core/lib/Drupal/Component/Render/FormattableMarkup.php
----------------------------------------------------------------------
FOUND 8 ERRORS AFFECTING 8 LINES
----------------------------------------------------------------------
  42 | ERROR | Tags must be grouped together in a doc comment
  52 | ERROR | Tags must be grouped together in a doc comment
 148 | ERROR | Tags must be grouped together in a doc comment
 152 | ERROR | Tags must be grouped together in a doc comment
 154 | ERROR | Tags must be grouped together in a doc comment
 156 | ERROR | Tags must be grouped together in a doc comment
 164 | ERROR | Tags must be grouped together in a doc comment
 169 | ERROR | Tags must be grouped together in a doc comment
----------------------------------------------------------------------

Here's a link to line 42 of FormattableMarkup, the first error above: http://cgit.drupalcode.org/drupal/tree/core/lib/Drupal/Component/Render/...

Note that it gives this error on the second use of @code, because it thinks @endcode is another type of tag, rather than a presentation markup.

cferthorney’s picture

Could we just do a switch statement along the lines of:

  switch($tagName) {
    case '@code':
    case '@endcode':
    case '@link':
    case '@endlink':
      return FALSE;
  }

It generates a "There must be exactly one blank line before the tags in a doc comment" error but I want to check I'm on the right lines with this before I submit a patch, and try to write tests, etc.

TR’s picture

Same problem as reported in #1, this is still an issue.

/**
 * This is an example of a doc block that has false positives.
 *
 * We want to show some example code:
 * @code
 *   if ($something) {
 *     $x = $y;
 *   } 
 * @endcode
 * Some more example code:
 * @code
 *   if ($something) {
 *     $x = $y;
 *   } 
 * @endcode
 * And one more piece of example code:
 * @code
 *   if ($something) {
 *     $x = $y;
 *   } 
 * @endcode
 * Followed by some summary text.
 */

The first @code/@endcode does not generate coding standards errors.
However, each @code or @endcode after that will result in the error:

Tags must be grouped together in a doc comment

So the above doc block will generate FOUR codings standards errors, when it should generate zero.

RoSk0’s picture

RoSk0’s picture

Title: False-positives for @code/@endcode » False-positives for @code/@endcode by Drupal.Commenting.DocCommentAlignment sniff

Added sniff name to title

TR’s picture

Title: False-positives for @code/@endcode by Drupal.Commenting.DocCommentAlignment sniff » False-positives for @code/@endcode by Drupal.Commenting.DocCommentSniff and Drupal.Commenting.DocCommentAlignment sniff

#1 and #3 errors are generated by Drupal.Commenting.DocCommentSniff, not by Drupal.Commenting.DocCommentAlignment.

Maybe this should be split into two issues, but let's not ignore #1 and #3 by changing the title and narrowing the issue.

jhodgdon’s picture

I would also like to note that the suggested fix in the issue summary

* @param ...
 *   Any additional arguments are passed on to the functions called by
 *   drupal_form_submit(), including the unique form constructor function.
 *   For example, the node_edit form requires that a node object be passed
 *   in here when it is called. Arguments that need to be passed by reference
 *   should not be included here, but rather placed directly in the $form_state
 *   build info array so that the reference can be preserved. For example, a
 *   form builder function with the following signature:
 * @code
 *   function mymodule_form($form, &$form_state, &$object) {
 *   }
 * @endcode

is incorrect from the perspective of working on api.drupal.org. That would result in that @code/@endcode block being considered *not* to be part of the @param documentation, because the API module uses indentation changes (to the left, as in this case) to indicate "that's the end of the preceding doc block section".

So, if there's a sniff that doesn't allow @code/@endcode and apparently @link (see #2937858: Fix 'Drupal.Commenting.DocCommentAlignment' coding standard) to be indented the same as the surrounding text, then that sniff is incorrect.

btully’s picture

Any update on this? Currently there doesn't seem to be a way to reliably run phpcs on a D8 codebase with all of these sniff errors resulting from the usage of @code/@endcode in comments, which is how 99% of all core/contrib code seems to do things.

I've followed the suggestion from @cferthorney above and modified vendor/drupal/coder/coder_sniffer/Drupal/Sniffs/Commenting/DocCommentSniff.php and at https://git.drupalcode.org/project/coder/blob/8.x-3.x/coder_sniffer/Drup... I added the following:

            $tagName = $tokens[$tag]['content'];
            switch($tagName) {
              case '@code':
              case '@endcode':
              case '@link':
              case '@endlink':
                return FALSE;
            }
            if (isset($foundTags[$tagName]) === true) {

And the frequent "Tags must be grouped together in a doc comment" disappeared.

Should we roll a patch?

TR’s picture

@btully: That was proposed by @cferthorney in #2, but in that comment he said:

It generates a "There must be exactly one blank line before the tags in a doc comment" error

So are you saying that doesn't occur for you?

The best thing to do it roll a patch because that will show exactly what you propose to do and will allow others to try it and see if there are any side effects ... but first I suggest you test any patch first to ensure it addresses all the problems listed above ...

klausi’s picture

Version: 8.x-2.x-dev » 8.x-3.x-dev
Status: Active » Needs review
Issue tags: +drupaldevdays

Work in progress at https://github.com/pfrenssen/coder/pull/49

Lot's of annoying exceptions we have to make for @code tags, I have not looked into @return code tags yet.

  • klausi authored 50475f0 on 8.x-3.x
     fix(DocComment): Allow @code tags in parameter and return docs (#...
klausi’s picture

Status: Needs review » Fixed

Added test for return tags and all your examples in this issue as test cases.

This should fix quite a lot of cases, please open new issues when you find more special cases.

klausi’s picture

Status: Fixed » Closed (fixed)

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