From f6053a399514983ec712cdbecb892480216ac797 Mon Sep 17 00:00:00 2001 From: Rodrigo Primo Date: Fri, 11 Oct 2024 15:56:31 -0300 Subject: [PATCH 1/3] Squiz/PostStatementComment: use a different error code for annotations This commit changes `Squiz.Commenting.PostStatementComment` to use a different error code when an annotation is found. This will allow ruleset maintainers to selectively exclude the flagging of trailing annotations from their ruleset. For example, for PHPCS itself, this will make it possible to use the `// @codeCoverageIgnore` annotation where needed. An annotation is defined as any comment that starts with an optional space, followed by a `@` and any character, as long as it is not a whitespace. For now, only `//` comments are supported as this is the only type of comment supported by this sniff. This change only applies to PHP code as support for JS will be dropped soon, and JS doesn't seem to follow the same convention of annotations starting with `@`. --- .../Squiz/Sniffs/Commenting/PostStatementCommentSniff.php | 8 ++++++++ .../Tests/Commenting/PostStatementCommentUnitTest.inc | 4 ++++ .../Commenting/PostStatementCommentUnitTest.inc.fixed | 5 +++++ .../Tests/Commenting/PostStatementCommentUnitTest.php | 3 +++ 4 files changed, 20 insertions(+) diff --git a/src/Standards/Squiz/Sniffs/Commenting/PostStatementCommentSniff.php b/src/Standards/Squiz/Sniffs/Commenting/PostStatementCommentSniff.php index 9b36abe76f..a0d91d3445 100644 --- a/src/Standards/Squiz/Sniffs/Commenting/PostStatementCommentSniff.php +++ b/src/Standards/Squiz/Sniffs/Commenting/PostStatementCommentSniff.php @@ -109,6 +109,14 @@ public function process(File $phpcsFile, $stackPtr) } } + if ($phpcsFile->tokenizerType === 'PHP' + && preg_match('|//\s?@[^\s]+|', $tokens[$stackPtr]['content']) === 1 + ) { + $error = 'Annotations may not appear after statements'; + $phpcsFile->addError($error, $stackPtr, 'AnnotationFound'); + return; + } + $error = 'Comments may not appear after statements'; $fix = $phpcsFile->addFixableError($error, $stackPtr, 'Found'); if ($fix === true) { diff --git a/src/Standards/Squiz/Tests/Commenting/PostStatementCommentUnitTest.inc b/src/Standards/Squiz/Tests/Commenting/PostStatementCommentUnitTest.inc index b83469613f..d4ee337459 100644 --- a/src/Standards/Squiz/Tests/Commenting/PostStatementCommentUnitTest.inc +++ b/src/Standards/Squiz/Tests/Commenting/PostStatementCommentUnitTest.inc @@ -52,3 +52,7 @@ $match = match($foo // comment ) { 1 => 1, // comment }; + +$a = 1; // @codeCoverageIgnore +$b = 2; //@phpstan-ignore variable.undefined +$c = 3; // @thisIsNotAnAnnotation - more than one space before the @ symbol. diff --git a/src/Standards/Squiz/Tests/Commenting/PostStatementCommentUnitTest.inc.fixed b/src/Standards/Squiz/Tests/Commenting/PostStatementCommentUnitTest.inc.fixed index 21a4bbe032..4cbd871617 100644 --- a/src/Standards/Squiz/Tests/Commenting/PostStatementCommentUnitTest.inc.fixed +++ b/src/Standards/Squiz/Tests/Commenting/PostStatementCommentUnitTest.inc.fixed @@ -57,3 +57,8 @@ $match = match($foo // comment 1 => 1, // comment }; + +$a = 1; // @codeCoverageIgnore +$b = 2; //@phpstan-ignore variable.undefined +$c = 3; +// @thisIsNotAnAnnotation - more than one space before the @ symbol. diff --git a/src/Standards/Squiz/Tests/Commenting/PostStatementCommentUnitTest.php b/src/Standards/Squiz/Tests/Commenting/PostStatementCommentUnitTest.php index b9751f6147..de09d0c773 100644 --- a/src/Standards/Squiz/Tests/Commenting/PostStatementCommentUnitTest.php +++ b/src/Standards/Squiz/Tests/Commenting/PostStatementCommentUnitTest.php @@ -40,6 +40,9 @@ public function getErrorList($testFile='') 18 => 1, 35 => 1, 53 => 1, + 56 => 1, + 57 => 1, + 58 => 1, ]; case 'PostStatementCommentUnitTest.1.js': From 009c701d503c3b9ddbb3735063b4a554fae69194 Mon Sep 17 00:00:00 2001 From: Rodrigo Primo Date: Mon, 28 Oct 2024 17:00:51 -0300 Subject: [PATCH 2/3] Modify regex and add more tests based on PR review feedback --- .../Commenting/PostStatementCommentSniff.php | 2 +- .../Commenting/PostStatementCommentUnitTest.inc | 12 +++++++++--- .../PostStatementCommentUnitTest.inc.fixed | 15 +++++++++++---- .../Commenting/PostStatementCommentUnitTest.php | 5 ++++- 4 files changed, 25 insertions(+), 9 deletions(-) diff --git a/src/Standards/Squiz/Sniffs/Commenting/PostStatementCommentSniff.php b/src/Standards/Squiz/Sniffs/Commenting/PostStatementCommentSniff.php index a0d91d3445..82b934a792 100644 --- a/src/Standards/Squiz/Sniffs/Commenting/PostStatementCommentSniff.php +++ b/src/Standards/Squiz/Sniffs/Commenting/PostStatementCommentSniff.php @@ -110,7 +110,7 @@ public function process(File $phpcsFile, $stackPtr) } if ($phpcsFile->tokenizerType === 'PHP' - && preg_match('|//\s?@[^\s]+|', $tokens[$stackPtr]['content']) === 1 + && preg_match('|^//[ \t]*@[^\s]+|', $tokens[$stackPtr]['content']) === 1 ) { $error = 'Annotations may not appear after statements'; $phpcsFile->addError($error, $stackPtr, 'AnnotationFound'); diff --git a/src/Standards/Squiz/Tests/Commenting/PostStatementCommentUnitTest.inc b/src/Standards/Squiz/Tests/Commenting/PostStatementCommentUnitTest.inc index d4ee337459..0eddeea646 100644 --- a/src/Standards/Squiz/Tests/Commenting/PostStatementCommentUnitTest.inc +++ b/src/Standards/Squiz/Tests/Commenting/PostStatementCommentUnitTest.inc @@ -53,6 +53,12 @@ $match = match($foo // comment 1 => 1, // comment }; -$a = 1; // @codeCoverageIgnore -$b = 2; //@phpstan-ignore variable.undefined -$c = 3; // @thisIsNotAnAnnotation - more than one space before the @ symbol. +// Comments recognized as annotations by PHPCS. +$a = 1; //@codeCoverageIgnore +$b = 2; // @phpstan-ignore variable.undefined +$c = 3; // @phpstan-ignore variable.undefined +$d = 4; // @tabInsteadOfSpace + +// Comments that include `@`, but are not recognized as annotations by PHPCS. +$a = 1; // @ = add tag. +$b = 2; // Some comment. // @username diff --git a/src/Standards/Squiz/Tests/Commenting/PostStatementCommentUnitTest.inc.fixed b/src/Standards/Squiz/Tests/Commenting/PostStatementCommentUnitTest.inc.fixed index 4cbd871617..668346e60a 100644 --- a/src/Standards/Squiz/Tests/Commenting/PostStatementCommentUnitTest.inc.fixed +++ b/src/Standards/Squiz/Tests/Commenting/PostStatementCommentUnitTest.inc.fixed @@ -58,7 +58,14 @@ $match = match($foo // comment // comment }; -$a = 1; // @codeCoverageIgnore -$b = 2; //@phpstan-ignore variable.undefined -$c = 3; -// @thisIsNotAnAnnotation - more than one space before the @ symbol. +// Comments recognized as annotations by PHPCS. +$a = 1; //@codeCoverageIgnore +$b = 2; // @phpstan-ignore variable.undefined +$c = 3; // @phpstan-ignore variable.undefined +$d = 4; // @tabInsteadOfSpace + +// Comments that include `@`, but are not recognized as annotations by PHPCS. +$a = 1; +// @ = add tag. +$b = 2; +// Some comment. // @username diff --git a/src/Standards/Squiz/Tests/Commenting/PostStatementCommentUnitTest.php b/src/Standards/Squiz/Tests/Commenting/PostStatementCommentUnitTest.php index de09d0c773..ba3b1c7222 100644 --- a/src/Standards/Squiz/Tests/Commenting/PostStatementCommentUnitTest.php +++ b/src/Standards/Squiz/Tests/Commenting/PostStatementCommentUnitTest.php @@ -40,9 +40,12 @@ public function getErrorList($testFile='') 18 => 1, 35 => 1, 53 => 1, - 56 => 1, 57 => 1, 58 => 1, + 59 => 1, + 60 => 1, + 63 => 1, + 64 => 1, ]; case 'PostStatementCommentUnitTest.1.js': From e28bf0616b47e4d0f8b9f4c7ca6e2f1aa3173417 Mon Sep 17 00:00:00 2001 From: Rodrigo Primo Date: Tue, 29 Oct 2024 09:07:16 -0300 Subject: [PATCH 3/3] Apply suggestions from code review Co-authored-by: Juliette <663378+jrfnl@users.noreply.github.com> --- .../Squiz/Tests/Commenting/PostStatementCommentUnitTest.inc | 4 ++-- .../Tests/Commenting/PostStatementCommentUnitTest.inc.fixed | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Standards/Squiz/Tests/Commenting/PostStatementCommentUnitTest.inc b/src/Standards/Squiz/Tests/Commenting/PostStatementCommentUnitTest.inc index 0eddeea646..3374c4760a 100644 --- a/src/Standards/Squiz/Tests/Commenting/PostStatementCommentUnitTest.inc +++ b/src/Standards/Squiz/Tests/Commenting/PostStatementCommentUnitTest.inc @@ -53,12 +53,12 @@ $match = match($foo // comment 1 => 1, // comment }; -// Comments recognized as annotations by PHPCS. +// Issue #560: Annotations should be reported separately and be non-auto-fixable as their meaning may change when moved. $a = 1; //@codeCoverageIgnore $b = 2; // @phpstan-ignore variable.undefined $c = 3; // @phpstan-ignore variable.undefined $d = 4; // @tabInsteadOfSpace -// Comments that include `@`, but are not recognized as annotations by PHPCS. +// Comments that include `@`, but are not recognized as annotations by this sniff. $a = 1; // @ = add tag. $b = 2; // Some comment. // @username diff --git a/src/Standards/Squiz/Tests/Commenting/PostStatementCommentUnitTest.inc.fixed b/src/Standards/Squiz/Tests/Commenting/PostStatementCommentUnitTest.inc.fixed index 668346e60a..bd6b171b04 100644 --- a/src/Standards/Squiz/Tests/Commenting/PostStatementCommentUnitTest.inc.fixed +++ b/src/Standards/Squiz/Tests/Commenting/PostStatementCommentUnitTest.inc.fixed @@ -58,13 +58,13 @@ $match = match($foo // comment // comment }; -// Comments recognized as annotations by PHPCS. +// Issue #560: Annotations should be reported separately and be non-auto-fixable as their meaning may change when moved. $a = 1; //@codeCoverageIgnore $b = 2; // @phpstan-ignore variable.undefined $c = 3; // @phpstan-ignore variable.undefined $d = 4; // @tabInsteadOfSpace -// Comments that include `@`, but are not recognized as annotations by PHPCS. +// Comments that include `@`, but are not recognized as annotations by this sniff. $a = 1; // @ = add tag. $b = 2;