As a follow-up for #2222435: Add a sniff to ensure @todo comments follow the coding standards, we want to all check trailing line indentation.

We could also open a follow-up to check the indentation of lines that follow an @todo comment.

CommentFileSizeAuthor
#6 todo2.txt1.69 KBjonathan1055

Comments

adamzimmermann created an issue. See original summary.

jonathan1055’s picture

jonathan1055 commented

Would this be a separate sniff? or just a new error/warning within the existing sniff file?

adamzimmermann’s picture

adamzimmermann commented

Would this be a separate sniff? or just a new error/warning within the existing sniff file?

That's a great question. I'm not sure.

I also wonder if the existing sniffs that check the indentation of others tags such as @param will just automagically work for @todo as well. I seem to remember thinking about this during my work on the initial sniff. I don't remember where I landed with the idea or if I tested it though.

So whoever starts on this first should check that idea and determine if any action is needed here at all. This might already be handled.

kingdutch’s picture

kingdutch
he/him
Primary language English
commented

This is not currently flagged as incorrect in a function docblock in a class.

  /**
   * This is the short comment.
   *
   * @todo This is a multiline todo.
   * this line is incorrectly formatted for the multiline.
   */
  public function test() {}

The following correctly formatted multiline todo is also not flagged, which is a good thing :)

  /**
   * This is the short comment.
   *
   * @todo This is a multiline todo.
   *   This line is correctly formatted for the multiline.
   */
  public function test() {}
adamzimmermann’s picture

adamzimmermann commented

@Kingdutch thanks for testing this.

@jonathan1055 it seems that this should already be flagged, right? If this was @param instead of @todo this would still be incorrect. I'm guessing that the @param sniff is pretty targeted though. I'm wondering if we could make it more generic and repurpose it's logic. I wanted to get thoughts from you or anyone who has a better big picture understanding of what sniffs do what before any code was started though.

  /**
   * This is the short comment.
   *
   * @param string $my_string
   * this line is incorrectly formatted for the multiline.
   */
  public function test() {}
jonathan1055’s picture

jonathan1055 commented
StatusFileSize
new todo2.txt1.69 KB

I thought I would look to see what happens with indenting for @param and @var in docblock, and also regular comments and @todo in code. Attached is a test file, shown below with the output. 

 * @param string $p1
 *   Multiple line param description with second line not indented enough.
 * Fails Commenting.FunctionComment.ParamCommentIndentation and IS fixable. (13)
 * @param string $p2
 *   Multiple lines with second line indented too much.
 *        This error is NOT reported.
 *
 * @var $v1 with multiple lines and the second one is not indented enough,
 * This is not reported, unlike @param which is reported.
 * @var $v2 with multiple lines and the second one is indented too much,
 *           This is not reported either.
 *
 * @todo with multiple lines d8testing_devel_generate_drush_content and the
 * second line indented not enough. Not reported.
 * @todo with multiple lines d8testing_devel_generate_drush_content and the
 *           second line indented too much. Not reported.
 * @todo with multiple lines d8testing_devel_generate_drush_content and the
 *   second line indented just the right amount.
 */
function a($p1, $p2) {
  // Here is a regular comment in code with the second line indented
  //     too much. Fails Commenting.InlineComment.SpacingBefore, NOT fixable. (32)
  // @todo This is a todo comment in regular code with multiple lines and the
  // next line is not indented enough. Not reported.
  // @todo This is a comment in regular code with second line indented too much,
  //     Fails Drupal.Commenting.InlineComment.SpacingBefore and IS fixable. (36)
  // and the third line not enough. This is not reported.
  // @todo This is a comment in regular code with multiple lines and the next
  //   is indented just right but third line is indented too much.
  //     Fails Commenting.InlineComment.SpacingBefore but is NOT fixable. (40)
}

The current phpcs results are:

------------------------------------------------------------------------------------
FOUND 4 ERRORS AFFECTING 4 LINES
------------------------------------------------------------------------------------
 13 | ERROR | [x] Parameter comment indentation must be 3 spaces, found 1 spaces
    |       |     (Drupal.Commenting.FunctionComment.ParamCommentIndentation)
 32 | ERROR | [ ] Comment indentation error, expected only 1 spaces
    |       |     (Drupal.Commenting.InlineComment.SpacingBefore)
 36 | ERROR | [x] Comment indentation error after @todo element, expected 3
    |       |     spaces (Drupal.Commenting.InlineComment.SpacingBefore)
 40 | ERROR | [ ] Comment indentation error, expected only 3 spaces
    |       |     (Drupal.Commenting.InlineComment.SpacingBefore)
------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
------------------------------------------------------------------------------------

So it seems that the indentation errors in a function doc block are reported by a sniff of the type of tag e.g @param is covered by Drupal.Commenting.FunctionComment.ParamCommentIndentation but @var does not have the equivalent. I did not look at @throws or @ingroup.

In regular code comments, having too much indentation is already covered for @todo as well as plain comments. But having not enough indentation is not reported. Also some are fixable and others are not, but I guess that's a subsequent question to look at.

jonathan1055’s picture

jonathan1055 commented
Related issues: +#3444003: The text of an @todo should start with a capitalized word and sniff non-php files

There is a coding standards issue for discussing potential changes to the rules
#3444003: The text of an @todo should start with a capitalized word and sniff non-php files