From fa1c22b873eee09f751ed557885863a01da9fb18 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Wed, 17 Apr 2024 10:31:26 +0800 Subject: [PATCH] Do not warn about missing blocks if a method has the Override attribute Fixes #155 --- CHANGELOG.md | 1 + .../Commenting/MissingDocblockSniff.php | 14 +- .../Commenting/MissingDocblockSniffTest.php | 23 ++- .../with_and_without_overrides.php | 54 +++++ moodle/Tests/Util/AttributesTest.php | 187 ++++++++++++++++++ moodle/Util/Attributes.php | 111 +++++++++++ 6 files changed, 383 insertions(+), 7 deletions(-) create mode 100644 moodle/Tests/Sniffs/Commenting/fixtures/MissingDocblock/with_and_without_overrides.php create mode 100644 moodle/Tests/Util/AttributesTest.php create mode 100644 moodle/Util/Attributes.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 87e1cef..e578c62 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ The format of this change log follows the advice given at [Keep a CHANGELOG](htt ## [Unreleased] ### Changed - Update composer dependencies to current versions, notably `PHP_CodeSniffer` (3.10.1) and `PHPCompatibility` (96072c30). +- The `moodle.Commenting.MissingDocblock` sniff will now detect use of the Override attribute (Fixes #155). ## [v3.4.6] - 2024-04-03 ### Fixed diff --git a/moodle/Sniffs/Commenting/MissingDocblockSniff.php b/moodle/Sniffs/Commenting/MissingDocblockSniff.php index 4467a06..8999a59 100644 --- a/moodle/Sniffs/Commenting/MissingDocblockSniff.php +++ b/moodle/Sniffs/Commenting/MissingDocblockSniff.php @@ -17,6 +17,7 @@ namespace MoodleHQ\MoodleCS\moodle\Sniffs\Commenting; +use MoodleHQ\MoodleCS\moodle\Util\Attributes; use MoodleHQ\MoodleCS\moodle\Util\Docblocks; use MoodleHQ\MoodleCS\moodle\Util\MoodleUtil; use MoodleHQ\MoodleCS\moodle\Util\TokenUtil; @@ -183,6 +184,17 @@ protected function processFunctions(File $phpcsFile, int $stackPtr): void { foreach ($missingDocblocks as $typePtr => $extendsOrImplements) { $token = $tokens[$typePtr]; + if ($extendsOrImplements) { + $attributes = Attributes::getAttributePointers($phpcsFile, $typePtr); + foreach ($attributes as $attributePtr) { + $attribute = Attributes::getAttributeProperties($phpcsFile, $attributePtr); + if ($attribute['attribute_name'] === '\Override') { + // Skip methods that are marked as overrides. + continue 2; + } + } + } + $objectName = TokenUtil::getObjectName($phpcsFile, $typePtr); $objectType = TokenUtil::getObjectType($phpcsFile, $typePtr); @@ -195,8 +207,6 @@ protected function processFunctions(File $phpcsFile, int $stackPtr): void { [$objectType, $objectName] ); } - } elseif ($extendsOrImplements) { - $phpcsFile->addWarning('Missing docblock for %s %s', $typePtr, 'Missing', [$objectType, $objectName]); } else { $phpcsFile->addError('Missing docblock for %s %s', $typePtr, 'Missing', [$objectType, $objectName]); } diff --git a/moodle/Tests/Sniffs/Commenting/MissingDocblockSniffTest.php b/moodle/Tests/Sniffs/Commenting/MissingDocblockSniffTest.php index 39f5d67..25da953 100644 --- a/moodle/Tests/Sniffs/Commenting/MissingDocblockSniffTest.php +++ b/moodle/Tests/Sniffs/Commenting/MissingDocblockSniffTest.php @@ -18,9 +18,6 @@ namespace MoodleHQ\MoodleCS\moodle\Tests\Sniffs\Commenting; use MoodleHQ\MoodleCS\moodle\Tests\MoodleCSBaseTestCase; -use PHP_CodeSniffer\Config; -use PHP_CodeSniffer\Files\DummyFile; -use PHP_CodeSniffer\Ruleset; /** * Test the MissingDocblockSniff sniff. @@ -68,11 +65,11 @@ public static function docblockCorrectnessProvider(): array { 159 => 'Missing docblock for function test_method', 166 => 'Missing docblock for function test_method', 170 => 'Missing docblock for class example_extends', + 171 => 'Missing docblock for function test_method', 175 => 'Missing docblock for class example_implements', + 176 => 'Missing docblock for function test_method', ], 'warnings' => [ - 171 => 'Missing docblock for function test_method', - 176 => 'Missing docblock for function test_method', ], ], 'File level tag, no class' => [ @@ -166,6 +163,22 @@ public static function docblockCorrectnessProvider(): array { 18 => 'Missing docblock for function this_is_a_dataprovider in testcase', ], ], + 'With and without Overrides attributes' => [ + 'fixture' => 'with_and_without_overrides', + 'fixtureFilename' => null, + 'errors' => [ + 1 => 'Missing docblock for file with_and_without_overrides.php', + 11 => 'Missing docblock for function has_override', + 13 => 'Missing docblock for function no_override', + 21 => 'Missing docblock for function has_override', + 23 => 'Missing docblock for function no_override', + 33 => 'Missing docblock for function no_override', + 43 => 'Missing docblock for function no_override', + 53 => 'Missing docblock for function no_override', + ], + 'warnings' => [ + ], + ], ]; if (version_compare(PHP_VERSION, '8.0.0') >= 0) { diff --git a/moodle/Tests/Sniffs/Commenting/fixtures/MissingDocblock/with_and_without_overrides.php b/moodle/Tests/Sniffs/Commenting/fixtures/MissingDocblock/with_and_without_overrides.php new file mode 100644 index 0000000..fbc5776 --- /dev/null +++ b/moodle/Tests/Sniffs/Commenting/fixtures/MissingDocblock/with_and_without_overrides.php @@ -0,0 +1,54 @@ +. + +namespace MoodleHQ\MoodleCS\moodle\Tests\Util; + +use MoodleHQ\MoodleCS\moodle\Tests\MoodleCSBaseTestCase; +use MoodleHQ\MoodleCS\moodle\Util\Attributes; +use PHP_CodeSniffer\Config; +use PHP_CodeSniffer\Files\DummyFile; +use PHP_CodeSniffer\Ruleset; + +/** + * Test the Attributes specific moodle utilities class + * + * @copyright 2024 onwards Andrew Lyons + * @license https://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * + * @covers \MoodleHQ\MoodleCS\moodle\Util\Attributes + */ +class AttributesTest extends MoodleCSBaseTestCase +{ + /** + * @dataProvider validTagsProvider + */ + public function testGetAttributePointers( + string $content, + $stackPtrSearch, + ?array $expectations + ): void { + $config = new Config([]); + $ruleset = new Ruleset($config); + + $phpcsFile = new DummyFile($content, $ruleset, $config); + $phpcsFile->process(); + + $searchPtr = $phpcsFile->findNext($stackPtrSearch, 0); + + $pointers = Attributes::getAttributePointers($phpcsFile, $searchPtr); + if (count($expectations)) { + foreach ($expectations as $expectation) { + $this->assertCount( + $expectation['count'], + array_filter($pointers, function ($pointer) use ($expectation, $phpcsFile) { + $properties = Attributes::getAttributeProperties($phpcsFile, $pointer); + + return $properties['attribute_name'] === $expectation['name']; + }) + ); + } + } else { + $this->assertEmpty($pointers); + } + } + + public static function validTagsProvider(): array { + return [ + 'No attributes' => [ + ' [ + ' '\\Override', 'count' => 1], + ], + ], + 'Multiple Attribute matches (same)' => [ + ' '\\Override', 'count' => 2], + ], + ], + 'Multiple Attribute matches (different)' => [ + ' '\\Override', 'count' => 2], + ['name' => '\\Other', 'count' => 1], + ], + ], + 'Attribute on other funciton' => [ + ' [ + ' '\\Override', 'count' => 1], + ['name' => '\\Route', 'count' => 2], + ['name' => '\\core\\attribute\\deprecated', 'count' => 1], + ], + ], + 'Attribute start will only get that attribute' => [ + ' '\\Route', 'count' => 1], + ], + ], + 'Attribute End will only get that attribute' => [ + ' '\\Route', 'count' => 1], + ], + ], + ]; + } + + public function testGetAttributePropertiesNotAnAttribute(): void { + $config = new Config([]); + $ruleset = new Ruleset($config); + + $content = <<process(); + + $searchPtr = $phpcsFile->findNext(T_FUNCTION, 0); + + $this->assertNull(Attributes::getAttributeProperties($phpcsFile, $searchPtr)); + } +} diff --git a/moodle/Util/Attributes.php b/moodle/Util/Attributes.php new file mode 100644 index 0000000..ef6ad71 --- /dev/null +++ b/moodle/Util/Attributes.php @@ -0,0 +1,111 @@ +. + +namespace MoodleHQ\MoodleCS\moodle\Util; + +use PHP_CodeSniffer\Files\File; +use PHPCSUtils\Utils\Context; +use PHPCSUtils\Utils\Namespaces; + +/** + * Utilities related to PHP Attributes. + * + * @copyright 2024 Andrew Lyons + * @license https://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +abstract class Attributes +{ + /** + * Get the pointer for an Attribute on an Attributable object. + * + * @param File $phpcsFile + * @param int $stackPtr + * @return array + */ + public static function getAttributePointers( + File $phpcsFile, + int $stackPtr + ): array { + $tokens = $phpcsFile->getTokens(); + $attributes = []; + + $stopAt = [ + T_DOC_COMMENT_CLOSE_TAG, + T_CLOSE_CURLY_BRACKET, + T_OPEN_CURLY_BRACKET, + T_SEMICOLON, + ]; + + for ($attributePtr = $stackPtr; $attributePtr > 0; $attributePtr--) { + $token = $tokens[$attributePtr]; + // The phpcs parser places an attribute_opener and attribute_closer on every part of an attribute. + if (isset($token['attribute_opener'])) { + $attributePtr = $token['attribute_opener']; + $attributes[] = $attributePtr; + } + + if (in_array($token['code'], $stopAt)) { + break; + } + } + + return $attributes; + } + + /** + * Get the properties of an Attribute. + * + * Note: The attribute name is not currently qualified relative to the current namespace or any imported classes. + * + * @param File $phpcsFile + * @param int $stackPtr + * @return array|null + */ + public static function getAttributeProperties( + File $phpcsFile, + int $stackPtr + ): ?array { + $tokens = $phpcsFile->getTokens(); + if (!isset($tokens[$stackPtr]['attribute_opener'])) { + return null; + } + + $opener = $tokens[$stackPtr]['attribute_opener']; + $closer = $tokens[$stackPtr]['attribute_closer']; + + $properties = [ + 'attribute_opener' => $opener, + 'attribute_closer' => $closer, + 'attribute_name' => null, + ]; + + $stopAt = [ + T_OPEN_PARENTHESIS, + ]; + + for ($i = $opener + 1; $i < $closer; $i++) { + if (in_array($tokens[$i]['code'], $stopAt)) { + break; + } + $properties['attribute_name'] .= $tokens[$i]['content']; + } + + // TODO Get the qualified name. + + return $properties; + } +}