Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecated items do not need a description #140

Merged
merged 2 commits into from
Mar 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions moodle/Sniffs/Commenting/CategorySniff.php
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);
stronk7 marked this conversation as resolved.
Show resolved Hide resolved
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
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
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
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
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
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
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
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
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