WordPress / WordPress-Coding-Standards

PHP_CodeSniffer rules (sniffs) to enforce WordPress coding conventions
MIT License
2.57k stars 488 forks source link

Generic.Commenting.DocComment.MissingShort is too broad #403

Open Rarst opened 9 years ago

Rarst commented 9 years ago

From a stab at making existing plugin adhere to new WordPress-Docs ruleset, the sniff enforcing short descriptions in doc blocks is overly broad.

That would make sense to me if limited to function/property descriptions.

However it catches multiple other kinds of blocks:

GaryJones commented 9 years ago

Summaries in File headers are required per WPCS: https://make.wordpress.org/core/handbook/inline-documentation-standards/php-documentation-standards/#6-file-headers

Summaries in Class properties are required per WPCS: https://make.wordpress.org/core/handbook/inline-documentation-standards/php-documentation-standards/#2-class-properties - nothing about documenting variables though.

A @todo in it's own DocBlock serves little purpose (I know it's the only way for phpDocumentor to see it) - either leave it as // standard comment, or create a GH Issue for it so it's not lost in the code.

(I agree that the CS in the handbook may be too stringent, but WPCS is only codifying what's there.)

Rarst commented 9 years ago

Sniffs should not go above the letter of the standard. Enforcing short descriptions on blocks that aren't intended to have them is sniff issue, not something code should bend to accommodate.

GaryJones commented 9 years ago

Agreed, but equally, code should be used correctly in the first place. I could add a /** DocBlock for every single inline comment, but that is neither standard, not reasonably expected. PSR-5 (for want of better evidence) articulates that @todo has an associated Structural Element. That means @todo should only be added to a standard file/interface/trait/class/property/method/function DocBlock, not added as what would otherwise be an inline comment, mid-function.

Rarst commented 9 years ago

/** @var ... is perfectly correct usage (explicitly used in example in PSR-5 like that). We can argue some of this away, but in the end I don't think the sniff works in reasonable and productive way at the moment.

GaryJones commented 9 years ago

There doesn't appear to be a way to exclude this check for different types of structural elements. The whole sniff gives lots of advantages though, and not everyone uses /** @var ..., so, for the time being, I suggest you just exclude that particular check in your projects.

GaryJones commented 9 years ago

Relevant: https://github.com/squizlabs/PHP_CodeSniffer/issues/258

Also, it looks like this could be solved by creating a custom WP DocCommentSniff and applying https://github.com/squizlabs/PHP_CodeSniffer/pull/276/files

bkdotcom commented 7 years ago

/** @var ... is perfectly correct usage

as is

    /**
     * @var $myVar ...
     */

just to be clear that this isn't single-line syntax specific

larssn commented 4 years ago

I use /** @var ... extensively, and writing // phpcs:ignore kinda defeats the purpose.

Can we get some momentum going on this issue as it's very old at this point.

jrfnl commented 4 years ago

@larssn This is a sniff which comes from PHPCS itself, not one maintained here. You're welcome to send a PR to PHPCS itself to see about fixing it.