Proposal: flag regular PHP comments with PHPDoc tags and convert them to DocBlocks

[anton-vlasenko]

Current situation

PHP codebases often contain inline (//, #) and/or multiline (/* ... */) comments that include PHPDoc-style tags such as @param, @return, or @todo.
However, these comments are not recognized as valid PHPDoc blocks because they lack the correct DocBlock delimiters (/** ... */).

As a result, the PHP tokenizer may fail to parse these tags, leading to incomplete or inaccurate code analysis and documentation generation.

Solution

Develop a PHPCS sniff to detect inline/multiline comments that contain PHPDoc-style tags (e.g., @param, @return, @todo).

Sniff behavior:

• identify comments (T_COMMENT) that include any PHPDoc-style tags;
• flag these comments with an error or warning;
• automatically fix these comments by converting them to proper PHPDoc format (/** ... */);
• the sniff should flag only comments where PHPDoc tags appear at the beginning of a new line; this ensures false positives are avoided when a PHPDoc tag is used merely as part of a description in the context, rather than as an actual PHPDoc tag.

Impact:

• enhance compatibility with the PHP tokenizer and related tools;
• promote consistent code quality by enforcing proper DocBlock syntax;
• support developers in maintaining well-documented and easily parsable codebases.

[jrfnl]

@anton-vlasenko DocBlocks are special type of comments and should only be used for structural elements.

Inline comments should be used for everything else.
While it may be less common, inline comments can contain tags if this helps developers convey the relevant information more effectively.

Refs:

• https://docs.phpdoc.org/guide/getting-started/what-is-a-docblock.html
• https://github.com/php-fig/fig-standards/blob/master/proposed/phpdoc.md

As a result, the PHP tokenizer may fail to parse these tags, leading to incomplete or inaccurate code analysis and documentation generation.

The PHP tokenizer does nothing with tags in the first place and at compile-time all comments will be removed anyway, so I don't understand what you are trying to say here.

Turning inline comments, which are not related to structural elements, into DocBlocks sounds like an oxymoron to me.

For structural elements there are already numerous sniffs - in the PEAR and Squiz standards - which check those elements have a docblock and these sniffs will already flag if a non-DocBlock style comment is found instead.

All in all, I can't think of a single reason why a sniff like this would be a good idea, though I'm open to hearing valid arguments to the contrary.
Marking this feature request as a close candidate in the mean time.

[anton-vlasenko]

Thank you for the feedback, @jrfnl.

DocBlocks are special type of comments and should only be used for structural elements.
Inline comments should be used for everything else.
While it may be less common, inline comments can contain tags if this helps developers convey the relevant information more effectively.

Yes, I know.

The PHP tokenizer does nothing with tags in the first place and at compile-time all comments will be removed anyway, so I don't understand what you are trying to say here.

While I agree that the PHP tokenizer doesn't parse tags specifically, it does differentiate between "regular" comments and DocBlocks, as there are distinct tokens for comments (T_COMMENT) and DocBlocks (T_DOC_COMMENT). This means that a comment containing valid PHPDoc tags but lacking proper DocBlock syntax will not be processed by sniffs listening for the T_DOC_COMMENT token.

For example, @todo is a valid PHPDoc tag that can be used on virtually any element, as noted here:
https://manual.phpdoc.org/HTMLSmartyConverter/HandS/phpDocumentor/tutorial_tags.todo.pkg.html
It is not limited to "structural" elements. However, many comments using @todo are not written as DocBlocks and instead appear as regular comments in the // or /* */ style.

Consider another scenario:

/* @var Foo $foo */
$foo = 'bar';

Will PHPCS catch this issue? This comment should be written as a DocBlock, but since it is formatted as a regular comment, sniffs listening for the T_DOC_COMMENT token will not process it.

Turning inline comments, which are not related to structural elements, into DocBlocks sounds like an oxymoron to me.

I'm not entirely sure what your definition of a structural element is, but there are cases, as shown above, where DocBlocks can be used for regular variables or virtually any part of the code that is not a class, trait, property, method, function, or similar (i.e., structural elements).

[jrfnl]

I'm not entirely sure what your definition of a structural element is

It's not about my definition, but about the definition of the concept. See the reference links I posted above.

Consider another scenario:

/* @var Foo $foo */
$foo = 'bar';

Will PHPCS catch this issue?

Whether this is caught depends on the sniff and is the responsibility of the sniff.

This comment should be written as a DocBlock, but since it is formatted as a regular comment, sniffs listening for the T_DOC_COMMENT token will not process it.

Correct, in which case, the sniff checking those comments should be improved.
In most cases, this means the sniff should not be checking for a T_DOC_COMMENT token in the first place, but for a structural element and should verify the comment type used for the structural element and flag it if the wrong type is used - which is exactly what the sniffs in PHPCS itself do.

Blindly turning any comment which contains a tag into a DocBlock is not the correct way to go about this.

[anton-vlasenko]

It's not about my definition, but about the definition of the concept. See the reference links I posted above.

Sorry, I didn't mean to emphasize personal perception.

Whether this is caught depends on the sniff and is the responsibility of the sniff.

Yes, the sniffs could be improved. However, instead of making it solely the sniff's responsibility to handle incorrectly formatted DocBlocks, what if the sniff doesn't autofix the DocBlocks but simply generates a warning?
This would leave it up to the developer to decide whether such comments (those containing only PHPDoc tags) should be converted to proper DocBlocks.
This approach could also help avoid code duplication, as each sniff needing to account for incorrectly formatted DocBlocks wouldn't have to include the same logic to check whether regular comments contain PHPDoc tags.

[jrfnl]

what if the sniff doesn't autofix the DocBlocks but simply generates a warning?

That would mean the sniff would throws unfixable warnings for those cases where the comment should remain a comment and not be turned into a docblock.

Sniffs in PHPCS should only flag things which are solvable, so that is not a sniff which would be acceptable for PHPCS itself.

[anton-vlasenko]

That would mean the sniff would throw unfixable warnings in cases where a comment should remain a comment instead of being turned into a docblock.

Warnings indicate a potential error, but not all warnings are fixable.
Warning by definition is a statement or event telling somebody that something bad or unpleasant may happen in the future.
This is confirmed by the PHPCS code itself.
For example, in the ClassDeclarationSniff::process() method, we see:

if ( isset($tokens[$stackPtr]['scope_opener'] ) === false ) {
    $error = 'Possible parse error: %s missing opening or closing brace';
    $phpcsFile->addWarning( $error, $stackPtr, 'MissingBrace', $errorData );
    return;
}

Here, the sniff states there is a “possible parse error,” which may not be fixable.

Sniffs in PHPCS should only flag things which are solvable, so that is not a sniff which would be acceptable for PHPCS itself.

You mentioned that sniffs in PHPCS should only flag things that are solvable. However, as demonstrated above, there are sniffs that flag potential issues that might not be solvable at all.

[jrfnl]

Warnings indicate a potential error, but not all warnings are fixable. Warning by definition is a statement or event telling somebody that something bad or unpleasant may happen in the future. This is confirmed by the PHPCS code itself. For example, in the ClassDeclarationSniff::process() method, we see:

if ( isset($tokens[$stackPtr]['scope_opener'] ) === false ) {
    $error = 'Possible parse error: %s missing opening or closing brace';
    $phpcsFile->addWarning( $error, $stackPtr, 'MissingBrace', $errorData );
    return;
}

Here, the sniff states there is a “possible parse error,” which may not be fixable.

And that type of potential parse error warning is being removed in the 4.0 release as per squizlabs/PHP_CodeSniffer#2455, so no, this is not a valid example and not a reason to introduce non-solvable warnings.