Php_codesniffer: DocCommentSniff reporting single-line block comments as error

[aik099]

There are 2 way how you can write a block comment:

class Something
{
    /**
     * Description.
     *
     * @var ClassName
     */
    protected $somethingMultiLine;

    /** @var ClassName Description */
    protected $somethingElse;

}

Same goes for method DocBlocks and DocBlocks for individual variables in the code.

However the DocCommentSniff reports 1st line in following code as error:

/** @var NodeElement $content_box_title */
$content_box_title = array_shift($content_box_titles);

Error text: The open comment tag must be the only content on the line
Error code: ContentAfterOpen

Reference: http://www.phpdoc.org/docs/latest/references/phpdoc/tags/var.html

[aik099]

What I believe should be done is to apply that ContentAfterOpen error validation only when T_DOC_COMMENT_OPEN_TAG and T_DOC_COMMENT_CLOSE_TAG of a DocBlock are on different lines only.

Currently only a line number of DocComment start and short description start are compared: https://github.com/squizlabs/PHP_CodeSniffer/blob/phpcs-fixer/CodeSniffer/Standards/Generic/Sniffs/Commenting/DocCommentSniff.php#L81

[gsherwood]

This actually needs more thought.

Even removing this specific error, you're still going to get other errors due to a missing short description. This sniff, and the other commenting sniffs, just don't support the @var shorthand comment style because it is not used by the included standards.

[aik099]

I know, that @var isn't supported by any Squiz standards and I had to write my own sniffs because of that. Now with 2.x version this new check in the commenting sniff it made things worse for me.

Please consider doing something about this before final 2.0 release.

Feel free to post your thoughts on that here. IMO easiest fix would be to ignore specific check if @var tag is present inside a block comment.

[gsherwood]

MO easiest fix would be to ignore specific check if @var tag is present inside a block comment.

I can't do that because it will then not be enforcing the checks it was written for. I can't relax the checks in any way without adversely affecting the included standards, or custom standards that use the sniffs. This is the real problem.

I suspect you just wont be able to use the included sniffs unless you are okay with completely muting that error. But as I said, you'll still get others from the same sniff because of the missing short description, so you may not be able to use that sniff at all.

You'll probably need a custom sniff that checks for the @var short syntax and then does custom checks if it is found or includes the DocCommentSniff and runs process() on the token if it is not found.

[aik099]

I can't do that because it will then not be enforcing the checks it was written for. I can't relax the checks in any way without adversely affecting the included standards, or custom standards that use the sniffs. This is the real problem.

From what I understand the check in question was added in 2.x version and there was no similar check in 1.x version of commenting sniffs. Why technically there can be a problem altering that specific check if a final release of 2.0 version wasn't made yet?

But as I said, you'll still get others from the same sniff because of the missing short description, so you may not be able to use that sniff at all.

That's perfectly ok. It's that specific check that concerns me, not others. I'm just worried that if somebody using @var (and I bet somebody is using them) will hit this new check and need to disable that specific error manually as well.

[Mairu]

That's perfectly ok. It's that specific check that concerns me, not others. I'm just worried that if somebody using @var (and I bet somebody is using them) will hit this new check and need to disable that specific error manually as well.

:+1:
Just want to point out that single line DocBlocks are part of the proposed PSR-5 with @type (and @var).

[aik099]

Thanks for the info @Mairu , that's one more point of revising how that ContentAfterOpen error is being checked. So ignoring ContentAfterOpen error if start and end line of DocBlock matches seems right solution.

[gsherwood]

Just want to point out that single line DocBlocks are part of the proposed PSR-5 with @type (and @var).

And I intend to support PSR-5 when it comes out. I'd love a good solid commenting standard, so I very much welcome this effort.

So ignoring ContentAfterOpen error if start and end line of DocBlock matches seems right solution.

This isn't the correct solution. If I mute this error, it will cause comments like /* Comment here. */ to pass validation in the Squiz standard, which I do not want (not yet anyway).

From what I understand the check in question was added in 2.x version and there was no similar check in 1.x version of commenting sniffs.

This was a missing check that I uncovered when doing the auto-fixing. That process made me write a lot more code for testing, so I found quite a few places where sniffs were not enforcing the rules they were supposed to. To fix an issue, the sniff has to first enforce it the check, which is why more were added. Given the complete rewrite of the commenting sniffs (mostly so that I could auto-fix comments) these are where the majority of checks were added and removed.

[aik099]

This isn't the correct solution. If I mute this error, it will cause comments like /* Comment here. */ to pass validation in the Squiz standard, which I do not want (not yet anyway).

I've totally forgot that Squiz standard doesn't allow any @var and since all commenting sniffs (from all standards) were merged back to new DocCommentSniff that rule is now enforced in General standard as well.

Given the complete rewrite of the commenting sniffs (mostly so that I could auto-fix comments) these are where the majority of checks were added and removed.

I doubt thought that the ContentAfterOpen error would suffer much if it won't be replacing

/** Something */

to

/**
 * Something */

as it will do right now.

Thinking about that again line matching solution, that I proposed, might not be quite right because people really can try to write whole DocBlock in one line and PHPCS should report that. So looking for @var and @type and if found disable that check will be more safer way to fix that.

[aik099]

@gsherwood , were you able to think more about this? If so, then what are pros/cons of change made to specific inspection within that sniff?

[aik099]

ping

[TomasVotruba]

What's the current status of this? I think I didn't came across this using single line docs.

[aik099]

The variable single line docs is common use case, when you want to have IDE method/property autocomplete, but object in a variable is created by service container method, that have same return type for all objects.

[TomasVotruba]

Ah, I probably don't use this Sniff then. Never came across this issue.

[vedmant]

Just came across this issue as well, it looks like I need either to disable doc comments sniff or avoid using shorthand @var declarations, both variants are not appealing to me.

[miken32]

So, what is the argument for not including this in the generic checks? Given that PHPDoc itself is a mature standard, I don't see any reason to wait for PSR-5, if it's even completed; it will just refine the existing PHPDoc standard.

[oojacoboo]

Bump 👍

[mikaelz]

Valid

/**
 * Import count
 *
 * @var int
 */
private $import_count = 0;

Invalid :(

/** @var int $import_count Import count */
private $import_count = 0;

[LastRide]

Ping.

[glen-84]

Is the fact that:

/* @var $matches array */

(note single asterisk)

... results in:

Single line block comment not allowed; use inline ("// text") comment instead

... related to this issue?

It would be nice to allow the above while still disallowing single-line block comments that don't include the @var.

[gsherwood]

... related to this issue?

I don't think so. Only comments that start with /** are tokenized as doc blocks. Any comments that just start with /* are considered inline comments and are picked up by a different sniff banning that syntax for single-line inline comments.

I'm assuming you are documenting variables inside code and not class properties? If so, there are no sniffs or standards included with PHPCS to enforce any rules in this area, so you're probably asking for a different new feature.

[glen-84]

I'm assuming you are documenting variables inside code and not class properties?

Correct. I've opened #1977.

[VincentLanglet]

What is the status of this ?

[larssn]

@VincentLanglet It's still not supported, and I assume this answer https://github.com/squizlabs/PHP_CodeSniffer/pull/276#issuecomment-58981916 is the official stance on the matter.

I'm disappoint

[bkdotcom]

Started using static analysis tool psalm (https://psalm.dev/docs/annotating_code/typing_in_psalm/) which uses inline /** @var */
trying to figure out how to allow this use-case...

[glen-84]

@bkdotcom You can do something like this for now.

[bkdotcom]

@glen-84 thanks.. I also had to exclude
Squiz.Commenting.BlockComment.WrongStart
and
Squiz.Commenting.InlineComment.DocBlock

I believe the only relevant sniffs left in play are the PSR12 sniffs