Skip to content

Commit

Permalink
Merge pull request moodlehq#140 from andrewnicols/missingDocblockDepr…
Browse files Browse the repository at this point in the history 
…ecated

Deprecated items do not need a description
  • Loading branch information
@stronk7
stronk7 authored Mar 29, 2024
2 parents 66d0f48 + b425006 commit 1d63691
Show file tree
Hide file tree
Showing 9 changed files with 86 additions and 80 deletions.
10 changes: 8 additions & 2 deletions moodle/Sniffs/Commenting/CategorySniff.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
6 changes: 6 additions & 0 deletions moodle/Sniffs/Commenting/DocblockDescriptionSniff.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down
8 changes: 4 additions & 4 deletions moodle/Sniffs/Commenting/FileExpectedTagsSniff.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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',
Expand Down
8 changes: 4 additions & 4 deletions moodle/Sniffs/Commenting/MissingDocblockSniff.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}
}
Expand Down Expand Up @@ -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;
}
Expand Down
24 changes: 13 additions & 11 deletions moodle/Sniffs/Commenting/PackageSniff.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
}
}

Expand All @@ -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
Expand All @@ -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',
Expand Down
2 changes: 1 addition & 1 deletion moodle/Sniffs/Commenting/ValidTagsSniff.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down
5 changes: 5 additions & 0 deletions moodle/Tests/Sniffs/Commenting/fixtures/DocblockDescription/standard.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {}
68 changes: 34 additions & 34 deletions moodle/Tests/Util/DocblocksTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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);
}

Expand All @@ -62,38 +62,39 @@ 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();
$startDocPointer = $phpcsFile->findNext(T_DOC_COMMENT_OPEN_TAG, 0);
$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 {
Expand All @@ -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));
}

/**
Expand All @@ -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);
}

Expand All @@ -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);
}

Expand Down
35 changes: 11 additions & 24 deletions moodle/Util/Docblocks.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit 1d63691

Please sign in to comment.