PHPCSStandards / PHP_CodeSniffer

PHP_CodeSniffer tokenizes PHP files and detects violations of a defined set of coding standards.
BSD 3-Clause "New" or "Revised" License
880 stars 54 forks source link

`Squiz.Commenting.PostStatementComment` should flag all types of trailing comments #562

Open rodrigoprimo opened 1 month ago

rodrigoprimo commented 1 month ago

Describe the bug

Squiz.Commenting.PostStatementComment flags only single-line trailing comments that start with //. But, as @jrfnl mentioned in https://github.com/PHPCSStandards/PHP_CodeSniffer/issues/560#issuecomment-2237930952, # is also a valid single-line trailing comment in PHP and should be flagged by the sniff as well.

I wonder if single-line trailing comments using /* ... */ should be flagged as well or not. I think they should, as this is yet another valid type of trailing comment, but I'm not sure.

Code sample

$a = 1; // This comment is already flagged by the sniff.
$b = 2; # This comment is not currently flagged by the sniff, but it should be.
$c = 3; /* I believe this comment should be flagged as well, but this is an open discussion */

To reproduce

Steps to reproduce the behavior:

  1. Create a file called test.php with the code sample above.
  2. Run phpcs -s --standard=Squiz --sniffs=Squiz.Commenting.PostStatementComment test.php
  3. See error message displayed
    FILE: test.php
    --------------------------------------------------------------------------------------------------------
    FOUND 1 ERROR AFFECTING 1 LINE
    --------------------------------------------------------------------------------------------------------
    9 | ERROR | [x] Comments may not appear after statements (Squiz.Commenting.PostStatementComment.Found)
    --------------------------------------------------------------------------------------------------------
    PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
    --------------------------------------------------------------------------------------------------------

Expected behavior

Besides displaying an error message for line 1, which already happens, I expect PHPCS to display an error for line 2 and potentially line 3 as well.

Versions (please complete the following information)

Operating System not relevant (Ubuntu 24.04)
PHP version not relevant (8.3)
PHP_CodeSniffer version master
Standard Squiz
Install type not relevant (git clone)

Please confirm

jrfnl commented 1 month ago

I wonder if single-line trailing comments using /* ... */ should be flagged as well or not. I think they should, as this is yet another valid type of trailing comment, but I'm not sure.

I'm fine with flagging all three variants - with the explicit caveat that the /* */ style comment should only be flagged when single-line.

We might also want to have a think about whether all three should be flagged with the existing error code or that each type of trailing comment should have its own error code. In principle, I think the same error code should be used as they are all trailing comments, but opinions may differ on this.