diff --git a/moodle/Sniffs/Commenting/CategorySniff.php b/moodle/Sniffs/Commenting/CategorySniff.php index 675078e..f8dbfd1 100644 --- a/moodle/Sniffs/Commenting/CategorySniff.php +++ b/moodle/Sniffs/Commenting/CategorySniff.php @@ -46,15 +46,21 @@ public function register() { * @param int $stackPtr The position in the stack. */ public function process(File $phpcsFile, $stackPtr) { - $categoryTokens = Docblocks::getMatchingDocTags($phpcsFile, $stackPtr, '@category'); + $docPtr = Docblocks::getDocBlockPointer($phpcsFile, $stackPtr); + if (empty($docPtr)) { + // It should not be possible to reach this line. It is a safety check. + return; // @codeCoverageIgnore + } + + $categoryTokens = Docblocks::getMatchingDocTags($phpcsFile, $docPtr, '@category'); if (empty($categoryTokens)) { return; } $tokens = $phpcsFile->getTokens(); + $docblock = $tokens[$docPtr]; $apis = MoodleUtil::getMoodleApis($phpcsFile); - $docblock = Docblocks::getDocBlock($phpcsFile, $stackPtr); foreach ($categoryTokens as $tokenPtr) { $categoryValuePtr = $phpcsFile->findNext( T_DOC_COMMENT_STRING, diff --git a/moodle/Sniffs/Commenting/DocblockDescriptionSniff.php b/moodle/Sniffs/Commenting/DocblockDescriptionSniff.php index 330a87c..74028ea 100644 --- a/moodle/Sniffs/Commenting/DocblockDescriptionSniff.php +++ b/moodle/Sniffs/Commenting/DocblockDescriptionSniff.php @@ -79,6 +79,12 @@ public function process(File $phpcsFile, $stackPtr) { } $faultAtLine = $tokens[$stopAt]['line']; + $deprecatedTagPtrs = Docblocks::getMatchingDocTags($phpcsFile, $docblockPtr, '@deprecated'); + if (count($deprecatedTagPtrs) > 0) { + // Skip if the docblock contains a @deprecated tag. + continue; + } + // Skip to the next T_DOC_COMMENT_STAR line. We do not accept single line docblocks. $docblockLinePtr = $docblockPtr; while ($docblockLinePtr = $phpcsFile->findNext(T_DOC_COMMENT_STAR, $docblockLinePtr + 1, $stopAt)) { diff --git a/moodle/Sniffs/Commenting/FileExpectedTagsSniff.php b/moodle/Sniffs/Commenting/FileExpectedTagsSniff.php index 621ebe9..9a09241 100644 --- a/moodle/Sniffs/Commenting/FileExpectedTagsSniff.php +++ b/moodle/Sniffs/Commenting/FileExpectedTagsSniff.php @@ -93,9 +93,9 @@ public function process(File $phpcsFile, $stackPtr) { * @param int $stackPtr The position in the stack. */ private function processFileCopyright(File $phpcsFile, $stackPtr): void { - $copyrightTokens = Docblocks::getMatchingDocTags($phpcsFile, $stackPtr, '@copyright'); + $docPtr = Docblocks::getDocBlockPointer($phpcsFile, $stackPtr); + $copyrightTokens = Docblocks::getMatchingDocTags($phpcsFile, $docPtr, '@copyright'); if (empty($copyrightTokens)) { - $docPtr = Docblocks::getDocBlockPointer($phpcsFile, $stackPtr); if (empty($docPtr)) { $docPtr = $stackPtr; } @@ -117,9 +117,9 @@ private function processFileCopyright(File $phpcsFile, $stackPtr): void { */ private function processFileLicense(File $phpcsFile, $stackPtr): void { $tokens = $phpcsFile->getTokens(); - $foundTokens = Docblocks::getMatchingDocTags($phpcsFile, $stackPtr, '@license'); + $docPtr = Docblocks::getDocBlockPointer($phpcsFile, $stackPtr); + $foundTokens = Docblocks::getMatchingDocTags($phpcsFile, $docPtr, '@license'); if (empty($foundTokens)) { - $docPtr = Docblocks::getDocBlockPointer($phpcsFile, $stackPtr); if ($docPtr) { $phpcsFile->addError( 'Missing @license tag', diff --git a/moodle/Sniffs/Commenting/MissingDocblockSniff.php b/moodle/Sniffs/Commenting/MissingDocblockSniff.php index 06b419c..4467a06 100644 --- a/moodle/Sniffs/Commenting/MissingDocblockSniff.php +++ b/moodle/Sniffs/Commenting/MissingDocblockSniff.php @@ -89,14 +89,14 @@ protected function processScopes(File $phpcsFile, int $stackPtr): void { continue; } - if (!Docblocks::getDocBlock($phpcsFile, $typePtr)) { + if (!Docblocks::getDocBlockPointer($phpcsFile, $typePtr)) { $missingDocblocks[] = $typePtr; } } if ($artifactCount !== 1) { // See if there is a file docblock. - $fileblock = Docblocks::getDocBlock($phpcsFile, $stackPtr); + $fileblock = Docblocks::getDocBlockPointer($phpcsFile, $stackPtr); if ($fileblock === null) { $objectName = TokenUtil::getObjectName($phpcsFile, $stackPtr); @@ -176,7 +176,7 @@ protected function processFunctions(File $phpcsFile, int $stackPtr): void { } } - if (!Docblocks::getDocBlock($phpcsFile, $typePtr)) { + if (!Docblocks::getDocBlockPointer($phpcsFile, $typePtr)) { $missingDocblocks[$typePtr] = $extendsOrImplements; } } @@ -229,7 +229,7 @@ protected function processConstants(File $phpcsFile, int $stackPtr): void { } } - if (Docblocks::getDocBlock($phpcsFile, $typePtr)) { + if (Docblocks::getDocBlockPointer($phpcsFile, $typePtr)) { // This is documented. continue; } diff --git a/moodle/Sniffs/Commenting/PackageSniff.php b/moodle/Sniffs/Commenting/PackageSniff.php index 50e472e..ab15b57 100644 --- a/moodle/Sniffs/Commenting/PackageSniff.php +++ b/moodle/Sniffs/Commenting/PackageSniff.php @@ -49,12 +49,12 @@ public function register() { public function process(File $phpcsFile, $stackPtr) { $tokens = $phpcsFile->getTokens(); - $docblock = Docblocks::getDocBlock($phpcsFile, $stackPtr); - if ($docblock) { + $docPtr = Docblocks::getDocBlockPointer($phpcsFile, $stackPtr); + if ($docPtr) { $filePackageFound = $this->checkDocblock( $phpcsFile, $stackPtr, - $docblock + $docPtr ); if ($filePackageFound) { return; @@ -75,13 +75,13 @@ public function process(File $phpcsFile, $stackPtr) { continue; } - $docblock = Docblocks::getDocBlock($phpcsFile, $typePtr); + $docPtr = Docblocks::getDocBlockPointer($phpcsFile, $typePtr); - if ($docblock === null) { + if ($docPtr === null) { continue; } - $this->checkDocblock($phpcsFile, $typePtr, $docblock); + $this->checkDocblock($phpcsFile, $typePtr, $docPtr); } } @@ -96,11 +96,8 @@ public function process(File $phpcsFile, $stackPtr) { protected function checkDocblock( File $phpcsFile, int $stackPtr, - array $docblock + int $docPtr ): bool { - $tokens = $phpcsFile->getTokens(); - $objectName = TokenUtil::getObjectName($phpcsFile, $stackPtr); - $objectType = TokenUtil::getObjectType($phpcsFile, $stackPtr); $expectedPackage = MoodleUtil::getMoodleComponent($phpcsFile, true); // Nothing to do if we have been unable to determine the package @@ -109,7 +106,12 @@ protected function checkDocblock( return false; } - $packageTokens = Docblocks::getMatchingDocTags($phpcsFile, $stackPtr, '@package'); + $tokens = $phpcsFile->getTokens(); + $objectName = TokenUtil::getObjectName($phpcsFile, $stackPtr); + $objectType = TokenUtil::getObjectType($phpcsFile, $stackPtr); + $docblock = $tokens[$docPtr]; + + $packageTokens = Docblocks::getMatchingDocTags($phpcsFile, $docPtr, '@package'); if (empty($packageTokens)) { $fix = $phpcsFile->addFixableError( 'DocBlock missing a @package tag for %s %s. Expected @package %s', diff --git a/moodle/Sniffs/Commenting/ValidTagsSniff.php b/moodle/Sniffs/Commenting/ValidTagsSniff.php index 88fbbf9..dc64cfd 100644 --- a/moodle/Sniffs/Commenting/ValidTagsSniff.php +++ b/moodle/Sniffs/Commenting/ValidTagsSniff.php @@ -49,7 +49,7 @@ public function process(File $phpcsFile, $stackPtr) { $tokens = $phpcsFile->getTokens(); while ($docPtr = $phpcsFile->findNext(T_DOC_COMMENT_OPEN_TAG, $stackPtr)) { - $docblock = Docblocks::getDocBlock($phpcsFile, $docPtr); + $docblock = $tokens[$docPtr]; foreach ($docblock['comment_tags'] as $tagPtr) { $tagName = ltrim($tokens[$tagPtr]['content'], '@'); if (!Docblocks::isValidTag($phpcsFile, $tagPtr)) { diff --git a/moodle/Tests/Sniffs/Commenting/fixtures/DocblockDescription/standard.php b/moodle/Tests/Sniffs/Commenting/fixtures/DocblockDescription/standard.php index 3400894..e0d3ac9 100644 --- a/moodle/Tests/Sniffs/Commenting/fixtures/DocblockDescription/standard.php +++ b/moodle/Tests/Sniffs/Commenting/fixtures/DocblockDescription/standard.php @@ -58,3 +58,8 @@ trait trait_with_docblock_but_no_description {} * @license */ function function_with_docblock_but_no_description() {} + +/** + * @deprecated + */ +function function_with_deprecated_tag() {} diff --git a/moodle/Tests/Util/DocblocksTest.php b/moodle/Tests/Util/DocblocksTest.php index 70da6a5..b0362ad 100644 --- a/moodle/Tests/Util/DocblocksTest.php +++ b/moodle/Tests/Util/DocblocksTest.php @@ -33,7 +33,7 @@ */ class DocblocksTest extends MoodleCSBaseTestCase { - public function testGetDocBlock(): void { + public function testgetDocBlockPointer(): void { $phpcsConfig = new Config(); $phpcsRuleset = new Ruleset($phpcsConfig); $phpcsFile = new \PHP_CodeSniffer\Files\LocalFile( @@ -45,7 +45,7 @@ public function testGetDocBlock(): void { $phpcsFile->process(); $filePointer = $phpcsFile->findNext(T_OPEN_TAG, 0); - $docBlock = Docblocks::getDocBlock($phpcsFile, $filePointer); + $docBlock = Docblocks::getDocBlockPointer($phpcsFile, $filePointer); $this->assertNull($docBlock); } @@ -62,20 +62,21 @@ public function testGetDocBlockTags(): void { $filePointer = $phpcsFile->findNext(T_OPEN_TAG, 0); $classPointer = $phpcsFile->findNext(T_CLASS, 0); - $fileDocBlock = Docblocks::getDocBlock($phpcsFile, $filePointer); - $this->assertNotNull($fileDocBlock); - $this->assertCount(1, Docblocks::getMatchingDocTags($phpcsFile, $filePointer, '@copyright')); - $this->assertCount(0, Docblocks::getMatchingDocTags($phpcsFile, $filePointer, '@property')); + $this->assertCount(0, Docblocks::getMatchingDocTags($phpcsFile, null, '@copyright')); + + $fileDocBlockPtr = Docblocks::getDocBlockPointer($phpcsFile, $filePointer); + $this->assertNotNull($fileDocBlockPtr); + $this->assertCount(1, Docblocks::getMatchingDocTags($phpcsFile, $fileDocBlockPtr, '@copyright')); + $this->assertCount(0, Docblocks::getMatchingDocTags($phpcsFile, $fileDocBlockPtr, '@property')); - $classDocBlock = Docblocks::getDocBlock($phpcsFile, $classPointer); - $this->assertNotNull($classDocBlock); - $this->assertNotEquals($fileDocBlock, $classDocBlock); - $this->assertCount(1, Docblocks::getMatchingDocTags($phpcsFile, $classPointer, '@copyright')); - $this->assertCount(2, Docblocks::getMatchingDocTags($phpcsFile, $classPointer, '@property')); + $classDocBlockPtr = Docblocks::getDocBlockPointer($phpcsFile, $classPointer); + $this->assertNotNull($classDocBlockPtr); + $this->assertNotEquals($fileDocBlockPtr, $classDocBlockPtr); + $this->assertCount(1, Docblocks::getMatchingDocTags($phpcsFile, $classDocBlockPtr, '@copyright')); + $this->assertCount(2, Docblocks::getMatchingDocTags($phpcsFile, $classDocBlockPtr, '@property')); $methodPointer = $phpcsFile->findNext(T_FUNCTION, $classPointer); - $this->assertNull(Docblocks::getDocBlock($phpcsFile, $methodPointer)); - $this->assertCount(0, Docblocks::getMatchingDocTags($phpcsFile, $methodPointer, '@property')); + $this->assertNull(Docblocks::getDocBlockPointer($phpcsFile, $methodPointer)); // Get the docblock from pointers at the start, middle, and end, of a docblock. $tokens = $phpcsFile->getTokens(); @@ -83,17 +84,17 @@ public function testGetDocBlockTags(): void { $endDocPointer = $phpcsFile->findNext(T_DOC_COMMENT_CLOSE_TAG, $startDocPointer); $middleDocPointer = $phpcsFile->findNext(T_DOC_COMMENT_STRING, $startDocPointer, $endDocPointer); - $docblock = Docblocks::getDocBlock($phpcsFile, $startDocPointer); - $this->assertIsArray($docblock); - $this->assertEquals($tokens[$startDocPointer], $docblock); + $docblock = Docblocks::getDocBlockPointer($phpcsFile, $startDocPointer); + $this->assertIsInt($docblock); + $this->assertEquals($startDocPointer, $docblock); - $docblock = Docblocks::getDocBlock($phpcsFile, $middleDocPointer); - $this->assertIsArray($docblock); - $this->assertEquals($tokens[$startDocPointer], $docblock); + $docblock = Docblocks::getDocBlockPointer($phpcsFile, $middleDocPointer); + $this->assertIsInt($docblock); + $this->assertEquals($startDocPointer, $docblock); - $docblock = Docblocks::getDocBlock($phpcsFile, $endDocPointer); - $this->assertIsArray($docblock); - $this->assertEquals($tokens[$startDocPointer], $docblock); + $docblock = Docblocks::getDocBlockPointer($phpcsFile, $endDocPointer); + $this->assertIsInt($docblock); + $this->assertEquals($startDocPointer, $docblock); } public function testGetDocBlockClassOnly(): void { @@ -109,18 +110,17 @@ public function testGetDocBlockClassOnly(): void { $filePointer = $phpcsFile->findNext(T_OPEN_TAG, 0); $classPointer = $phpcsFile->findNext(T_CLASS, 0); - $fileDocBlock = Docblocks::getDocBlock($phpcsFile, $filePointer); + $fileDocBlock = Docblocks::getDocBlockPointer($phpcsFile, $filePointer); $this->assertNull($fileDocBlock); - $classDocBlock = Docblocks::getDocBlock($phpcsFile, $classPointer); - $this->assertNotNull($classDocBlock); - $this->assertNotEquals($fileDocBlock, $classDocBlock); - $this->assertCount(1, Docblocks::getMatchingDocTags($phpcsFile, $classPointer, '@copyright')); - $this->assertCount(2, Docblocks::getMatchingDocTags($phpcsFile, $classPointer, '@property')); + $classDocBlockPtr = Docblocks::getDocBlockPointer($phpcsFile, $classPointer); + $this->assertNotNull($classDocBlockPtr); + $this->assertNotEquals($fileDocBlock, $classDocBlockPtr); + $this->assertCount(1, Docblocks::getMatchingDocTags($phpcsFile, $classDocBlockPtr, '@copyright')); + $this->assertCount(2, Docblocks::getMatchingDocTags($phpcsFile, $classDocBlockPtr, '@property')); $methodPointer = $phpcsFile->findNext(T_FUNCTION, $classPointer); - $this->assertNull(Docblocks::getDocBlock($phpcsFile, $methodPointer)); - $this->assertCount(0, Docblocks::getMatchingDocTags($phpcsFile, $methodPointer, '@property')); + $this->assertNull(Docblocks::getDocBlockPointer($phpcsFile, $methodPointer)); } /** @@ -140,10 +140,10 @@ public function testGetDocBlockClassWithoutDocblock(): void { $filePointer = $phpcsFile->findNext(T_OPEN_TAG, 0); $classPointer = $phpcsFile->findNext(T_CLASS, 0); - $fileDocBlock = Docblocks::getDocBlock($phpcsFile, $filePointer); + $fileDocBlock = Docblocks::getDocBlockPointer($phpcsFile, $filePointer); $this->assertNotNull($fileDocBlock); - $classDocBlock = Docblocks::getDocBlock($phpcsFile, $classPointer); + $classDocBlock = Docblocks::getDocBlockPointer($phpcsFile, $classPointer); $this->assertNull($classDocBlock); } @@ -164,10 +164,10 @@ public function testGetDocBlockClassWithAttribute(): void { $filePointer = $phpcsFile->findNext(T_OPEN_TAG, 0); $classPointer = $phpcsFile->findNext(T_CLASS, 0); - $fileDocBlock = Docblocks::getDocBlock($phpcsFile, $filePointer); + $fileDocBlock = Docblocks::getDocBlockPointer($phpcsFile, $filePointer); $this->assertNotNull($fileDocBlock); - $classDocBlock = Docblocks::getDocBlock($phpcsFile, $classPointer); + $classDocBlock = Docblocks::getDocBlockPointer($phpcsFile, $classPointer); $this->assertNull($classDocBlock); } diff --git a/moodle/Util/Docblocks.php b/moodle/Util/Docblocks.php index 6631daa..15c4f57 100644 --- a/moodle/Util/Docblocks.php +++ b/moodle/Util/Docblocks.php @@ -144,25 +144,6 @@ abstract class Docblocks // Commented out: 'uses' => ['#.*/tests/.*_test.php#'], can also be out from tests (Coding style dixit). ]; - /** - * Get the docblock for a file, class, interface, trait, or method. - * - * @param File $phpcsFile - * @param int $stackPtr - * @return null|array - */ - public static function getDocBlock( - File $phpcsFile, - int $stackPtr - ): ?array { - $docPtr = self::getDocBlockPointer($phpcsFile, $stackPtr); - if ($docPtr !== null) { - return $phpcsFile->getTokens()[$docPtr]; - } - - return null; - } - /** * Get the docblock pointer for a file, class, interface, trait, or method. * @@ -301,17 +282,23 @@ protected static function getDocTagFromOpenTag( return null; } + /** + * Get the tags that match the given tag name. + * + * @param File $phpcsFile + * @param int|null $stackPtr The pointer of the docblock + * @param string $tagName + */ public static function getMatchingDocTags( File $phpcsFile, - int $stackPtr, + ?int $stackPtr, string $tagName ): array { - $tokens = $phpcsFile->getTokens(); - $docblock = self::getDocBlock($phpcsFile, $stackPtr); - if ($docblock === null) { + if ($stackPtr === null) { return []; } - + $tokens = $phpcsFile->getTokens(); + $docblock = $tokens[$stackPtr]; $matchingTags = []; foreach ($docblock['comment_tags'] as $tag) { if ($tokens[$tag]['content'] === $tagName) {