Generic.Commenting.DocComment not finding errors when long description is omitted

[bkdotcom]

I'm just coming from v1.5.5 (i know, I know...)

In 1.5.5 there was a sniff to enforce a space between descriptions and params/return/tags

Squiz.Commenting.FunctionComment.SpacingBeforeTags

Somewhere between 1.5.5 and 2.5.0 any such sniff has been removed and the following is allowed:

/**
 * I'm a function short-description
 * @return boolean
 */

(There's no longer a sniff to enforce a blank line before @return)

Did the Squiz standard intentionally change? (I don't see any mention in changelog)

[aik099]

All DocBlock processing sniffs were rewritten from scratch/removed and now there is DocComment sniff (see https://github.com/squizlabs/PHP_CodeSniffer/blob/master/CodeSniffer/Standards/Generic/Sniffs/Commenting/DocCommentSniff.php) that checks all that.

[bkdotcom]

my ruleset

<?xml version="1.0"?>
<ruleset name="PSR2Plus">
  <description>PSR2 plus some comment formatting rules</description>

  <rule ref="PSR2" />

  <rule ref="Generic">
    <exclude name="Generic.Arrays.DisallowLongArraySyntax.Found" />
    <exclude name="Generic.Files.EndFileNewline.NotFound" /> <!-- using PSR2.file -->
    <exclude name="Generic.Files.EndFileNoNewline.Found" />
    <exclude name="Generic.Files.LowercasedFilename.NotFound" />
    <exclude name="Generic.Functions.OpeningFunctionBraceKernighanRitchie.BraceOnNewLine" />
    <exclude name="Generic.PHP.ClosingPHPTag.NotFound" />
    <exclude name="Generic.PHP.UpperCaseConstant.Found" />
    <exclude name="Generic.WhiteSpace.DisallowSpaceIndent.SpacesUsed" />
    <exclude name="Generic.WhiteSpace.DisallowTabIndent.NonIndentTabsUsed" />
    <exclude name="Generic.WhiteSpace.DisallowTabIndent.TabsUsed" />
  </rule>

  <rule ref="Squiz.Commenting" />

  <rule ref="Zend.Debug.CodeAnalyzer"/>

</ruleset>

PHP

/**
 * this is a test
 * @author bkdotcom
 * @param boolean $foo blah
 * @return boolean
 * @param boolean $bar Blah.
 */
function testaroo($foo, $bar)
{
    return $foo && $bar;
}

only finds

 <error line="6" column="4" severity="error" message="Doc comment short description must start with a capital letter" source="Generic.Commenting.DocComment.ShortNotCapital"/>
 <error line="8" column="4" severity="error" message="Parameter comment must start with a capital letter" source="Squiz.Commenting.FunctionComment.ParamCommentNotCapital"/>
 <error line="8" column="4" severity="error" message="Parameter comment must end with a full stop" source="Squiz.Commenting.FunctionComment.ParamCommentFullStop"/>
 <error line="15" column="1" severity="error" message="Expected //end testaroo()" source="Squiz.Commenting.ClosingDeclarationComment.Missing"/>

There are many "assertions" in Generic.Commenting.DocComment that aren't being caught here

• @​param isn't first
• @​param tags aren't grouped together
• no blank line between "tag groups"
• no blank line before tags

It caught Generic.Commenting.DocComment.ShortNotCapital so the tests are being run...

[aik099]

If other words here is what your finding can be called:

@​param isn't first

Yes, no way to specify relative order for tags.

@​param tags aren't grouped together

No way to define tag groups, that consist of one-type or different-type tags

no blank line between "tag groups"
no blank line before tags

No blank lines between tag groups. Actually there is blank line between short/long description and @param tags.

If you know how to implement even part of that, then it would be really great. I've tried myself, but without much success.

[bkdotcom]

https://github.com/squizlabs/PHP_CodeSniffer/blob/master/CodeSniffer/Standards/Generic/Sniffs/Commenting/DocCommentSniff.php

@​param isn't first

Yes, no way to specify relative order for tags.

See DocCommentSniff line 329

        // If there is a param group, it needs to be first.
        if ($paramGroupid !== null && $paramGroupid !== 0) {
            $error = 'Parameter tags must be defined first in a doc comment';
            $phpcsFile->addError($error, $tagGroups[$paramGroupid][0], 'ParamNotFirst');
        }

for a better example of requiring relative order of tags see Squiz/Sniffs/Commenting/FileCommentSniff.php line 154 which specifies that @​package, @​subpackage, @​author, and @​copyright are present and in that order

@​param tags aren't grouped together

No way to define tag groups, that consist of one-type or different-type tags

See DocCommentSniff line 335

                    $error = 'Tags must be grouped together in a doc comment';
                    $phpcsFile->addError($error, $tag, 'TagsNotGrouped');

no blank line between "tag groups"
no blank line before tags

No blank lines between tag groups. Actually there is blank line between short/long description and @param tags.

Are you saying my PHP example has a blank line (it doesn't) or that the sniff contains that test (it does.. and that's the issue I'm having)

The 4 tests I listed are already defined in DocCommentSniff.php.. but don't seem to be working for me / issue with my ruleset / something

[aik099]

Are you sure, that mentioned sniffs are ever executed? Maybe they are somehow excluded from the ruleset.

[gsherwood]

The doc block sniffs were completely rewritten in 2.0, and the Squiz sniffs were significantly relaxed. Most of those changes were made in the 2.0.0a1 release: https://github.com/squizlabs/PHP_CodeSniffer/releases/tag/2.0.0a1

I'll have a look at the @param grouping issue.

[gsherwood]

I found the problem. All of these issue were caused by the comment not having a long description. For some reason, I made the sniff stop processing if there was no long description, which makes no sense.

Thanks for finding and reporting this.

[gsherwood]

Forgot to post the "after" result. For this code:

<?php
/**
 * this is a test
 * @author bkdotcom
 * @param boolean $foo blah
 * @return boolean
 * @param boolean $bar Blah.
 */

Running this command: phpcs temp.php --standard=Squiz --sniffs=Generic.Commenting.DocComment -s

Now gives this result:

FILE: temp.php
-----------------------------------------------------------------------------------------------------------------------------------------
FOUND 7 ERRORS AFFECTING 5 LINES
-----------------------------------------------------------------------------------------------------------------------------------------
 3 | ERROR | [ ] Doc comment short description must start with a capital letter (Generic.Commenting.DocComment.ShortNotCapital)
 4 | ERROR | [x] There must be exactly one blank line before the tags in a doc comment (Generic.Commenting.DocComment.SpacingBeforeTags)
 5 | ERROR | [ ] Parameter tags must be grouped together in a doc comment (Generic.Commenting.DocComment.ParamGroup)
 5 | ERROR | [x] Tag value indented incorrectly; expected 2 spaces but found 1 (Generic.Commenting.DocComment.TagValueIndent)
 6 | ERROR | [ ] Tag cannot be grouped with parameter tags in a doc comment (Generic.Commenting.DocComment.NonParamGroup)
 7 | ERROR | [ ] Tags must be grouped together in a doc comment (Generic.Commenting.DocComment.TagsNotGrouped)
 7 | ERROR | [x] Tag value indented incorrectly; expected 2 spaces but found 1 (Generic.Commenting.DocComment.TagValueIndent)
-----------------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 3 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-----------------------------------------------------------------------------------------------------------------------------------------

[bkdotcom]

Awesome thanks.
Sorry I couldn't be more help / haven't upgraded in nearly 2 years.

[gsherwood]

Sorry I couldn't be more help

You helped plenty. That's been around for nearly 2 years and nobody else found it.

[aik099]

Now, when you mention it I understand why

/*
 * Short description
 *
 * @param string $test Description.
 * @return void
 */

was auto-fixed into

/*
 * Short description
 *
 * @param  string $test Description.
 * @return void
 */

rather than into

/*
 * Short description
 *
 * @param string $test Description.
 *
 * @return void
 */

in my own code.

[gsherwood]

^ Ignore that commit for Potential issue with CommentedOutCode. It was for bug #862.