From e44a99c6fc5654a3cdae2b4d5c4db39cb57a71cd Mon Sep 17 00:00:00 2001 From: jrfnl Date: Fri, 13 Oct 2023 06:27:02 +0200 Subject: [PATCH 1/3] Commenting/CodeCoverageIgnoreDeprecated: minor test improvements Add some more variation of method prefixes to the tests to safeguard the handling of those better. --- .../Commenting/CodeCoverageIgnoreDeprecatedUnitTest.inc | 6 +++--- .../CodeCoverageIgnoreDeprecatedUnitTest.inc.fixed | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Yoast/Tests/Commenting/CodeCoverageIgnoreDeprecatedUnitTest.inc b/Yoast/Tests/Commenting/CodeCoverageIgnoreDeprecatedUnitTest.inc index d9165e9e..bf6cde15 100644 --- a/Yoast/Tests/Commenting/CodeCoverageIgnoreDeprecatedUnitTest.inc +++ b/Yoast/Tests/Commenting/CodeCoverageIgnoreDeprecatedUnitTest.inc @@ -1,6 +1,6 @@ Date: Fri, 13 Oct 2023 06:29:44 +0200 Subject: [PATCH 2/3] Commenting/CodeCoverageIgnoreDeprecated: minor code simplifications Remove two redundant conditions. The first is not needed as it's not actually important what token type we encounter, the only thing important is that there is a known attribute opener to skip to. The second is not needed as the `findPrevious()` call will always find something, if nothing else, it will find the docblock opener, so checking for `false` is unnecessary. --- .../Sniffs/Commenting/CodeCoverageIgnoreDeprecatedSniff.php | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/Yoast/Sniffs/Commenting/CodeCoverageIgnoreDeprecatedSniff.php b/Yoast/Sniffs/Commenting/CodeCoverageIgnoreDeprecatedSniff.php index 3e28cc82..9e3df3cb 100644 --- a/Yoast/Sniffs/Commenting/CodeCoverageIgnoreDeprecatedSniff.php +++ b/Yoast/Sniffs/Commenting/CodeCoverageIgnoreDeprecatedSniff.php @@ -45,9 +45,7 @@ public function process( File $phpcsFile, $stackPtr ) { continue; } - if ( $tokens[ $commentEnd ]['code'] === \T_ATTRIBUTE_END - && isset( $tokens[ $commentEnd ]['attribute_opener'] ) === true - ) { + if ( isset( $tokens[ $commentEnd ]['attribute_opener'] ) === true ) { $commentEnd = $tokens[ $commentEnd ]['attribute_opener']; continue; } @@ -91,7 +89,7 @@ public function process( File $phpcsFile, $stackPtr ) { $hasTagAsString = $phpcsFile->findNext( \T_DOC_COMMENT_STRING, ( $commentStart + 1 ), $commentEnd, false, 'codeCoverageIgnore' ); if ( $hasTagAsString !== false ) { $prev = $phpcsFile->findPrevious( \T_DOC_COMMENT_WHITESPACE, ( $hasTagAsString - 1 ), $commentStart, true ); - if ( $prev !== false && $tokens[ $prev ]['code'] === \T_DOC_COMMENT_STAR ) { + if ( $tokens[ $prev ]['code'] === \T_DOC_COMMENT_STAR ) { $fix = $phpcsFile->addFixableError( 'The `codeCoverageIgnore` annotation in the function docblock needs to be prefixed with an `@`.', $hasTagAsString, From 66ee5cbee43cb0900b4e6e9de4e6318b44d824f7 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Fri, 13 Oct 2023 06:36:44 +0200 Subject: [PATCH 3/3] Commenting/CodeCoverageIgnoreDeprecated: check OO docblocks too As things were, the `Yoast.Commenting.CodeCoverageIgnoreDeprecated` sniff only checked function docblocks to find a `@deprecated` tag and verify this was accompanied by a `@codeCoverageIgnore` tag. However, if a complete class (or other OO structure) is deprecated, we can also safely ignore it for code coverage checking. This updates the sniff to also check the docblocks of OO structures for `@deprecated` tags and verifies that, if those are found, they are accompanied by a `@codeCoverageIgnore` tag. Two exceptions are made: * The rule does not apply to interfaces as those can't contain code, so code coverage is not relevant. * The rule does not apply to anonymous classes. Those are treated the same as closures/arrow functions: those are normally nested within another construct and code coverage should be based on the wrapping construct. Includes tests. --- .../CodeCoverageIgnoreDeprecatedStandard.xml | 2 +- .../CodeCoverageIgnoreDeprecatedSniff.php | 23 +++-- .../CodeCoverageIgnoreDeprecatedUnitTest.inc | 93 +++++++++++++++++++ ...CoverageIgnoreDeprecatedUnitTest.inc.fixed | 93 +++++++++++++++++++ .../CodeCoverageIgnoreDeprecatedUnitTest.php | 18 +++- 5 files changed, 217 insertions(+), 12 deletions(-) diff --git a/Yoast/Docs/Commenting/CodeCoverageIgnoreDeprecatedStandard.xml b/Yoast/Docs/Commenting/CodeCoverageIgnoreDeprecatedStandard.xml index 83ffbe39..75ff7494 100644 --- a/Yoast/Docs/Commenting/CodeCoverageIgnoreDeprecatedStandard.xml +++ b/Yoast/Docs/Commenting/CodeCoverageIgnoreDeprecatedStandard.xml @@ -5,7 +5,7 @@ > diff --git a/Yoast/Sniffs/Commenting/CodeCoverageIgnoreDeprecatedSniff.php b/Yoast/Sniffs/Commenting/CodeCoverageIgnoreDeprecatedSniff.php index 9e3df3cb..ab2ef68f 100644 --- a/Yoast/Sniffs/Commenting/CodeCoverageIgnoreDeprecatedSniff.php +++ b/Yoast/Sniffs/Commenting/CodeCoverageIgnoreDeprecatedSniff.php @@ -5,12 +5,14 @@ use PHP_CodeSniffer\Files\File; use PHP_CodeSniffer\Sniffs\Sniff; use PHP_CodeSniffer\Util\Tokens; +use PHPCSUtils\Tokens\Collections; /** - * Verifies functions which are marked as `deprecated` have a `codeCoverageIgnore` tag + * Verifies functions/OO structures which are marked as `deprecated` have a `codeCoverageIgnore` tag * in their docblock. * * @since 1.1.0 + * @since 3.0.0 Not just checks function docblocks, but also class/OO docblocks. */ final class CodeCoverageIgnoreDeprecatedSniff implements Sniff { @@ -20,9 +22,13 @@ final class CodeCoverageIgnoreDeprecatedSniff implements Sniff { * @return array */ public function register() { - return [ - \T_FUNCTION, - ]; + $targets = Tokens::$ooScopeTokens; + // Ignore interfaces as they can't contain code. Ignore anon classes as they are normally nested in another construct. + unset( $targets[ \T_ANON_CLASS ], $targets[ \T_INTERFACE ] ); + + $targets[ \T_FUNCTION ] = \T_FUNCTION; + + return $targets; } /** @@ -37,7 +43,12 @@ public function process( File $phpcsFile, $stackPtr ) { $tokens = $phpcsFile->getTokens(); - $ignore = Tokens::$methodPrefixes; + if ( $tokens[ $stackPtr ]['code'] === \T_FUNCTION ) { + $ignore = Tokens::$methodPrefixes; + } + else { + $ignore = Collections::classModifierKeywords(); + } $ignore[ \T_WHITESPACE ] = \T_WHITESPACE; for ( $commentEnd = ( $stackPtr - 1 ); $commentEnd >= 0; $commentEnd-- ) { @@ -69,7 +80,7 @@ public function process( File $phpcsFile, $stackPtr ) { } if ( $deprecated === false ) { - // Not a deprecated function. + // Not a deprecated function/OO structure. return; } diff --git a/Yoast/Tests/Commenting/CodeCoverageIgnoreDeprecatedUnitTest.inc b/Yoast/Tests/Commenting/CodeCoverageIgnoreDeprecatedUnitTest.inc index bf6cde15..3e39aeac 100644 --- a/Yoast/Tests/Commenting/CodeCoverageIgnoreDeprecatedUnitTest.inc +++ b/Yoast/Tests/Commenting/CodeCoverageIgnoreDeprecatedUnitTest.inc @@ -89,3 +89,96 @@ abstract class SomeThing { #[fietsbel /*comment*/] public function inlineCommentWithinAttribute() {} } + +/** + * Description. + * + * @deprecated + */ +class C_IsDeprecatedNotIgnored {} + +/** + * Description. + * + * @deprecated + */ +abstract class C_IsDeprecatedNotIgnoredAbstract {} + +/** + * Description. + * + * @deprecated + */ +#[AllowDynamicProperties] +final class C_IsDeprecatedNotIgnoredFinal {} + +/** + * Description. + * + * @deprecated + */ +readonly class C_IsDeprecatedNotIgnoredReadonly {} + +/** + * Description. + * + * @deprecated + */ +final readonly class C_IsDeprecatedNotIgnoredFinalReadonly {} + +/** + * Description. + * + * @codeCoverageIgnore + * @deprecated + */ +readonly final class C_IsDeprecatedAndIgnored {} + +/** + * @deprecated + */ +#[MyAttribute, AnotherAttribute] +interface I_IsDeprecatedNotIgnored {} + +/** + * @codeCoverageIgnore + * @deprecated + */ +interface I_IsDeprecatedAndIgnored {} + +/** + * Description. + * + * @deprecated + */ +trait T_IsDeprecatedNotIgnored {} + +/** + * @deprecated + * codeCoverageIgnore + */ +trait T_IsDeprecatedAndIgnored {} + +/** + * @deprecated + */ +enum E_IsDeprecatedNotIgnored {} + +/** + * Description. + * + * @deprecated + * @codeCoverageIgnore + */ +enum E_IsDeprecatedAndIgnored {} + +/** + * Not necessarily correct, but as this is an anomymous class, this won't be flagged + * same as closures/arrow functions won't be flagged. + * + * @deprecated + */ +$anon = new + #[AllowDynamicProperties] + #[AnotherAttribute([1, 2, 3])] + class {}; diff --git a/Yoast/Tests/Commenting/CodeCoverageIgnoreDeprecatedUnitTest.inc.fixed b/Yoast/Tests/Commenting/CodeCoverageIgnoreDeprecatedUnitTest.inc.fixed index 0b093551..98b87df3 100644 --- a/Yoast/Tests/Commenting/CodeCoverageIgnoreDeprecatedUnitTest.inc.fixed +++ b/Yoast/Tests/Commenting/CodeCoverageIgnoreDeprecatedUnitTest.inc.fixed @@ -89,3 +89,96 @@ abstract class SomeThing { #[fietsbel /*comment*/] public function inlineCommentWithinAttribute() {} } + +/** + * Description. + * + * @deprecated + */ +class C_IsDeprecatedNotIgnored {} + +/** + * Description. + * + * @deprecated + */ +abstract class C_IsDeprecatedNotIgnoredAbstract {} + +/** + * Description. + * + * @deprecated + */ +#[AllowDynamicProperties] +final class C_IsDeprecatedNotIgnoredFinal {} + +/** + * Description. + * + * @deprecated + */ +readonly class C_IsDeprecatedNotIgnoredReadonly {} + +/** + * Description. + * + * @deprecated + */ +final readonly class C_IsDeprecatedNotIgnoredFinalReadonly {} + +/** + * Description. + * + * @codeCoverageIgnore + * @deprecated + */ +readonly final class C_IsDeprecatedAndIgnored {} + +/** + * @deprecated + */ +#[MyAttribute, AnotherAttribute] +interface I_IsDeprecatedNotIgnored {} + +/** + * @codeCoverageIgnore + * @deprecated + */ +interface I_IsDeprecatedAndIgnored {} + +/** + * Description. + * + * @deprecated + */ +trait T_IsDeprecatedNotIgnored {} + +/** + * @deprecated + * @codeCoverageIgnore + */ +trait T_IsDeprecatedAndIgnored {} + +/** + * @deprecated + */ +enum E_IsDeprecatedNotIgnored {} + +/** + * Description. + * + * @deprecated + * @codeCoverageIgnore + */ +enum E_IsDeprecatedAndIgnored {} + +/** + * Not necessarily correct, but as this is an anomymous class, this won't be flagged + * same as closures/arrow functions won't be flagged. + * + * @deprecated + */ +$anon = new + #[AllowDynamicProperties] + #[AnotherAttribute([1, 2, 3])] + class {}; diff --git a/Yoast/Tests/Commenting/CodeCoverageIgnoreDeprecatedUnitTest.php b/Yoast/Tests/Commenting/CodeCoverageIgnoreDeprecatedUnitTest.php index 13214ab4..c3858aa9 100644 --- a/Yoast/Tests/Commenting/CodeCoverageIgnoreDeprecatedUnitTest.php +++ b/Yoast/Tests/Commenting/CodeCoverageIgnoreDeprecatedUnitTest.php @@ -20,11 +20,19 @@ final class CodeCoverageIgnoreDeprecatedUnitTest extends AbstractSniffUnitTest { */ public function getErrorList(): array { return [ - 41 => 1, - 50 => 1, - 55 => 1, - 69 => 1, - 90 => 1, + 41 => 1, + 50 => 1, + 55 => 1, + 69 => 1, + 90 => 1, + 98 => 1, + 105 => 1, + 113 => 1, + 120 => 1, + 127 => 1, + 154 => 1, + 158 => 1, + 165 => 1, ]; }