From 266ae671e8e13cb994ba11b449b018fabfa0c064 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Wed, 13 Mar 2024 22:21:33 +0800 Subject: [PATCH] Add sniff to detect missing docblocks --- .../Commenting/MissingDocblockSniff.php | 122 +++++++++++++ moodle/Sniffs/Commenting/PackageSniff.php | 4 - .../Commenting/MissingDocblockSniffTest.php | 117 ++++++++++++ .../Sniffs/Commenting/PackageSniffTest.php | 12 +- .../missing_docblock_class_and_file.php | 15 ++ .../fixtures/missing_docblock_class_only.php | 11 ++ ...ng_docblock_class_only_with_attributes.php | 13 ++ ...y_with_attributes_incorrect_whitespace.php | 14 ++ ...k_class_only_with_incorrect_whitespace.php | 12 ++ ...issing_docblock_class_without_docblock.php | 12 ++ .../fixtures/missing_docblock_enum_only.php | 11 ++ .../missing_docblock_interface_only.php | 11 ++ .../missing_docblock_multiple_artefacts.php | 168 ++++++++++++++++++ .../fixtures/missing_docblock_trait_only.php | 11 ++ 14 files changed, 518 insertions(+), 15 deletions(-) create mode 100644 moodle/Sniffs/Commenting/MissingDocblockSniff.php create mode 100644 moodle/Tests/Sniffs/Commenting/MissingDocblockSniffTest.php create mode 100644 moodle/Tests/Sniffs/Commenting/fixtures/missing_docblock_class_and_file.php create mode 100644 moodle/Tests/Sniffs/Commenting/fixtures/missing_docblock_class_only.php create mode 100644 moodle/Tests/Sniffs/Commenting/fixtures/missing_docblock_class_only_with_attributes.php create mode 100644 moodle/Tests/Sniffs/Commenting/fixtures/missing_docblock_class_only_with_attributes_incorrect_whitespace.php create mode 100644 moodle/Tests/Sniffs/Commenting/fixtures/missing_docblock_class_only_with_incorrect_whitespace.php create mode 100644 moodle/Tests/Sniffs/Commenting/fixtures/missing_docblock_class_without_docblock.php create mode 100644 moodle/Tests/Sniffs/Commenting/fixtures/missing_docblock_enum_only.php create mode 100644 moodle/Tests/Sniffs/Commenting/fixtures/missing_docblock_interface_only.php create mode 100644 moodle/Tests/Sniffs/Commenting/fixtures/missing_docblock_multiple_artefacts.php create mode 100644 moodle/Tests/Sniffs/Commenting/fixtures/missing_docblock_trait_only.php diff --git a/moodle/Sniffs/Commenting/MissingDocblockSniff.php b/moodle/Sniffs/Commenting/MissingDocblockSniff.php new file mode 100644 index 0000000..d402d6b --- /dev/null +++ b/moodle/Sniffs/Commenting/MissingDocblockSniff.php @@ -0,0 +1,122 @@ +. + +namespace MoodleHQ\MoodleCS\moodle\Sniffs\Commenting; + +use MoodleHQ\MoodleCS\moodle\Util\Docblocks; +use MoodleHQ\MoodleCS\moodle\Util\Tokens; +use PHP_CodeSniffer\Files\File; +use PHP_CodeSniffer\Sniffs\Sniff; +use PHPCSUtils\Utils\ObjectDeclarations; + +/** + * Checks that all files an classes have appropriate docs. + * + * @copyright 2024 Andrew Lyons + * @license https://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class MissingDocblockSniff implements Sniff +{ + /** + * Register for open tag (only process once per file). + */ + public function register() { + return [ + T_OPEN_TAG, + ]; + } + + /** + * Processes php files and perform various checks with file. + * + * @param File $phpcsFile The file being scanned. + * @param int $stackPtr The position in the stack. + */ + public function process(File $phpcsFile, $stackPtr) { + $tokens = $phpcsFile->getTokens(); + + // Each class, interface, trait, and enum must have a docblock. + // If a file has one class, interface, trait, or enum, the file docblock is optinoal. + // Otherwise, the file docblock is required. + + // First find out how many items there are. + $find = [ + // Classes, enums, interfaces, and traits. + T_CLASS, + T_ENUM, + T_INTERFACE, + T_TRAIT, + + // Functions outside of those. + T_FUNCTION, + ]; + + $artefactCount = 0; + $missingDocblocks = []; + + $typePtr = $stackPtr + 1; + while ($typePtr = $phpcsFile->findNext($find, $typePtr + 1)) { + $token = $tokens[$typePtr]; + if ($token['code'] === T_FUNCTION && !empty($token['conditions'])) { + // Skip methods of classes, traits and interfaces. + continue; + } + + $artefactCount++; + + if ($docblock = Docblocks::getDocBlock($phpcsFile, $typePtr)) { + // There should be no empty lines between the artefact and the docblock. + $lastline = $tokens[$docblock['comment_closer']]['line']; + for ($interimPtr = $docblock['comment_closer'] + 1; $interimPtr < $typePtr; $interimPtr++) { + if ($tokens[$interimPtr]['code'] === T_ATTRIBUTE) { + $interimPtr = $tokens[$interimPtr]['attribute_closer']; + $lastline = $tokens[$interimPtr]['line']; + continue; + } + if ($tokens[$interimPtr]['line'] > $lastline) { + $missingDocblocks[] = $typePtr; + break; + } + } + } else { + $missingDocblocks[] = $typePtr; + } + } + + if ($artefactCount !== 1) { + // See if there is a file docblock. + $fileblock = Docblocks::getDocBlock($phpcsFile, $stackPtr); + + if ($fileblock === null) { + $objectName = Tokens::getObjectName($phpcsFile, $stackPtr); + $phpcsFile->addError('Missing docblock for file %s', $stackPtr, 'Missing', [$objectName]); + } + } + + foreach ($missingDocblocks as $typePtr) { + $objectName = Tokens::getObjectName($phpcsFile, $typePtr); + $objectType = Tokens::getObjectType($phpcsFile, $typePtr); + $phpcsFile->addError('Missing docblock for %s %s', $typePtr, 'Missing', [$objectType, $objectName]); + } + + if ($artefactCount === 1) { + // Only one artefact. + // No need for file docblock. + return; + } + } +} diff --git a/moodle/Sniffs/Commenting/PackageSniff.php b/moodle/Sniffs/Commenting/PackageSniff.php index 3f244ef..c3a2896 100644 --- a/moodle/Sniffs/Commenting/PackageSniff.php +++ b/moodle/Sniffs/Commenting/PackageSniff.php @@ -78,10 +78,6 @@ public function process(File $phpcsFile, $stackPtr) { $docblock = Docblocks::getDocBlock($phpcsFile, $typePtr); if ($docblock === null) { - $objectName = $this->getObjectName($phpcsFile, $typePtr); - $objectType = $this->getObjectType($phpcsFile, $typePtr); - $phpcsFile->addError('Missing doc comment for %s %s', $typePtr, 'Missing', [$objectType, $objectName]); - continue; } diff --git a/moodle/Tests/Sniffs/Commenting/MissingDocblockSniffTest.php b/moodle/Tests/Sniffs/Commenting/MissingDocblockSniffTest.php new file mode 100644 index 0000000..1abc2a8 --- /dev/null +++ b/moodle/Tests/Sniffs/Commenting/MissingDocblockSniffTest.php @@ -0,0 +1,117 @@ +. + +namespace MoodleHQ\MoodleCS\moodle\Tests\Sniffs\Commenting; + +use MoodleHQ\MoodleCS\moodle\Tests\MoodleCSBaseTestCase; + +/** + * Test the MissingDocblockSniff sniff. + * + * @copyright 2024 onwards Andrew Lyons + * @license https://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * + * @covers \MoodleHQ\MoodleCS\moodle\Sniffs\Commenting\MissingDocblockSniff + */ +class MissingDocblockSniffTest extends MoodleCSBaseTestCase +{ + /** + * @dataProvider docblockCorrectnessProvider + */ + public function testMissingDocblockSnfif( + string $fixture, + array $errors, + array $warnings + ): void { + $this->setStandard('moodle'); + $this->setSniff('moodle.Commenting.MissingDocblock'); + $this->setFixture(sprintf("%s/fixtures/%s.php", __DIR__, $fixture)); + $this->setWarnings($warnings); + $this->setErrors($errors); + $this->setComponentMapping([ + 'local_codechecker' => dirname(__DIR__), + ]); + + $this->verifyCsResults(); + } + + public static function docblockCorrectnessProvider(): array { + return [ + 'Multiple artefacts in a file' => [ + 'fixture' => 'missing_docblock_multiple_artefacts', + 'errors' => [ + 1 => 'Missing docblock for file missing_docblock_multiple_artefacts.php', + 34 => 'Missing docblock for function missing_docblock_in_function', + 38 => 'Missing docblock for class missing_docblock_in_class', + 95 => 'Missing docblock for interface missing_docblock_interface', + 118 => 'Missing docblock for trait missing_docblock_trait', ], + 'warnings' => [], + ], + 'File level tag, no class' => [ + 'fixture' => 'missing_docblock_class_without_docblock', + 'errors' => [ + 11 => 'Missing docblock for class class_without_docblock', + ], + 'warnings' => [], + ], + 'Class only (incorrect whitespace)' => [ + 'fixture' => 'missing_docblock_class_only_with_incorrect_whitespace', + 'errors' => [ + 11 => 'Missing docblock for class class_only_with_incorrect_whitespace', + ], + 'warnings' => [], + ], + 'Class only (correct)' => [ + 'fixture' => 'missing_docblock_class_only', + 'errors' => [], + 'warnings' => [], + ], + 'Class only with attributes (correct)' => [ + 'fixture' => 'missing_docblock_class_only_with_attributes', + 'errors' => [], + 'warnings' => [], + ], + 'Class only with attributes and incorrect whitespace' => [ + 'fixture' => 'missing_docblock_class_only_with_attributes_incorrect_whitespace', + 'errors' => [ + 13 => 'Missing docblock for class class_only_with_attributes_incorrect_whitespace', + ], + 'warnings' => [], + ], + 'Class and file (correct)' => [ + 'fixture' => 'missing_docblock_class_and_file', + 'errors' => [], + 'warnings' => [], + ], + 'Enum only (correct)' => [ + 'fixture' => 'missing_docblock_enum_only', + 'errors' => [], + 'warnings' => [], + ], + 'Interface only (correct)' => [ + 'fixture' => 'missing_docblock_interface_only', + 'errors' => [], + 'warnings' => [], + ], + 'Trait only (correct)' => [ + 'fixture' => 'missing_docblock_trait_only', + 'errors' => [], + 'warnings' => [], + ], + ]; + } +} diff --git a/moodle/Tests/Sniffs/Commenting/PackageSniffTest.php b/moodle/Tests/Sniffs/Commenting/PackageSniffTest.php index 3b32196..e075dc5 100644 --- a/moodle/Tests/Sniffs/Commenting/PackageSniffTest.php +++ b/moodle/Tests/Sniffs/Commenting/PackageSniffTest.php @@ -39,13 +39,7 @@ public function testPackageOnMissingComponent(): void { $this->setComponentMapping([]); // No components available. $this->setWarnings([]); - $this->setErrors([ - // These are still checked because this doesn't depend on the - missing - component mapping. - 35 => 'Missing doc comment for class missing_docblock_in_class', - 38 => 'Missing doc comment for interface missing_docblock_in_interface', - 41 => 'Missing doc comment for trait missing_docblock_in_trait', - 44 => 'Missing doc comment for function missing_docblock_in_function', - ]); + $this->setErrors([]); $this->verifyCsResults(); } @@ -77,8 +71,6 @@ public static function packageCorrectnessProvider(): array { 'errors' => [ 18 => 'DocBlock missing a @package tag for function package_missing. Expected @package local_codechecker', 31 => 'DocBlock missing a @package tag for class package_absent. Expected @package local_codechecker', - 34 => 'Missing doc comment for function missing_docblock_in_function', - 38 => 'Missing doc comment for class missing_docblock_in_class', 42 => '@package tag for function package_wrong_in_function. Expected local_codechecker, found wrong_package.', 48 => '@package tag for class package_wrong_in_class. Expected local_codechecker, found wrong_package.', 57 => 'More than one @package tag found in function package_multiple_in_function', @@ -87,10 +79,8 @@ public static function packageCorrectnessProvider(): array { 78 => 'More than one @package tag found in class package_multiple_in_class_all_wrong', 85 => 'More than one @package tag found in interface package_multiple_in_interface_all_wrong', 92 => 'More than one @package tag found in trait package_multiple_in_trait_all_wrong', - 95 => 'Missing doc comment for interface missing_docblock_interface', 101 => 'missing a @package tag for interface missing_package_interface. Expected @package', 106 => '@package tag for interface incorrect_package_interface. Expected local_codechecker, found', - 118 => 'Missing doc comment for trait missing_docblock_trait', 124 => 'DocBlock missing a @package tag for trait missing_package_trait. Expected @package', 129 => 'Incorrect @package tag for trait incorrect_package_trait. Expected local_codechecker, found', ], diff --git a/moodle/Tests/Sniffs/Commenting/fixtures/missing_docblock_class_and_file.php b/moodle/Tests/Sniffs/Commenting/fixtures/missing_docblock_class_and_file.php new file mode 100644 index 0000000..251cee2 --- /dev/null +++ b/moodle/Tests/Sniffs/Commenting/fixtures/missing_docblock_class_and_file.php @@ -0,0 +1,15 @@ +