Skip to content

Commit 4a519fe

Browse files
stronk7stronk7
authored
Merge pull request #119 from andrewnicols/MissingDocblockSniff
Add sniff to detect missing docblocks
2 parents 88f207d + 88eb84c commit 4a519fe

18 files changed

+951
-131
lines changed

‎moodle/Sniffs/Commenting/MissingDocblockSniff.php

Lines changed: 190 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,190 @@
1+
<?php
2+
3+
// This file is part of Moodle - https://moodle.org/
4+
//
5+
// Moodle is free software: you can redistribute it and/or modify
6+
// it under the terms of the GNU General Public License as published by
7+
// the Free Software Foundation, either version 3 of the License, or
8+
// (at your option) any later version.
9+
//
10+
// Moodle is distributed in the hope that it will be useful,
11+
// but WITHOUT ANY WARRANdTY; without even the implied warranty of
12+
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
13+
// GNU General Public License for more details.
14+
//
15+
// You should have received a copy of the GNU General Public License
16+
// along with Moodle. If not, see <https://www.gnu.org/licenses/>.
17+
18+
namespace MoodleHQ\MoodleCS\moodle\Sniffs\Commenting;
19+
20+
use MoodleHQ\MoodleCS\moodle\Util\Docblocks;
21+
use MoodleHQ\MoodleCS\moodle\Util\MoodleUtil;
22+
use MoodleHQ\MoodleCS\moodle\Util\TokenUtil;
23+
use PHP_CodeSniffer\Files\File;
24+
use PHP_CodeSniffer\Sniffs\Sniff;
25+
use PHP_CodeSniffer\Util\Tokens;
26+
use PHPCSUtils\Utils\ObjectDeclarations;
27+
28+
/**
29+
* Checks that all files an classes have appropriate docs.
30+
*
31+
* @copyright 2024 Andrew Lyons <[email protected]>
32+
* @license https://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
33+
*/
34+
class MissingDocblockSniff implements Sniff
35+
{
36+
/** @var array A list of standard method names used in unit test files */
37+
protected array $phpunitStandardMethodNames = [
38+
'setUp',
39+
'tearDown',
40+
'setUpBeforeClass',
41+
'tearDownAfterClass',
42+
];
43+
44+
/**
45+
* Register for open tag (only process once per file).
46+
*/
47+
public function register() {
48+
return [
49+
T_OPEN_TAG,
50+
];
51+
}
52+
53+
/**
54+
* Processes php files and perform various checks with file.
55+
*
56+
* @param File $phpcsFile The file being scanned.
57+
* @param int $stackPtr The position in the stack.
58+
*/
59+
public function process(File $phpcsFile, $stackPtr) {
60+
$this->processScopes($phpcsFile, $stackPtr);
61+
$this->processFunctions($phpcsFile, $stackPtr);
62+
}
63+
64+
protected function processScopes(File $phpcsFile, int $stackPtr): void {
65+
$tokens = $phpcsFile->getTokens();
66+
67+
// Each class, interface, trait, and enum must have a docblock.
68+
// If a file has one class, interface, trait, or enum, the file docblock is optional.
69+
// Otherwise, the file docblock is required.
70+
71+
$artifactCount = 0;
72+
$missingDocblocks = [];
73+
$find = Tokens::$ooScopeTokens;
74+
$find[] = T_FUNCTION;
75+
76+
$typePtr = $stackPtr + 1;
77+
while ($typePtr = $phpcsFile->findNext($find, $typePtr + 1)) {
78+
$token = $tokens[$typePtr];
79+
if ($token['code'] === T_FUNCTION && !empty($token['conditions'])) {
80+
// Skip methods of classes, traits and interfaces.
81+
continue;
82+
}
83+
$artifactCount++;
84+
85+
if ($token['code'] === T_FUNCTION) {
86+
// Skip functions. They are handled separately.
87+
continue;
88+
}
89+
90+
if (!Docblocks::getDocBlock($phpcsFile, $typePtr)) {
91+
$missingDocblocks[] = $typePtr;
92+
}
93+
}
94+
95+
if ($artifactCount !== 1) {
96+
// See if there is a file docblock.
97+
$fileblock = Docblocks::getDocBlock($phpcsFile, $stackPtr);
98+
99+
if ($fileblock === null) {
100+
$objectName = TokenUtil::getObjectName($phpcsFile, $stackPtr);
101+
$phpcsFile->addError('Missing docblock for file %s', $stackPtr, 'Missing', [$objectName]);
102+
}
103+
}
104+
105+
foreach ($missingDocblocks as $typePtr) {
106+
$token = $tokens[$typePtr];
107+
$objectName = TokenUtil::getObjectName($phpcsFile, $typePtr);
108+
$objectType = TokenUtil::getObjectType($phpcsFile, $typePtr);
109+
110+
$phpcsFile->addError('Missing docblock for %s %s', $typePtr, 'Missing', [$objectType, $objectName]);
111+
}
112+
113+
if ($artifactCount === 1) {
114+
// Only one artifact.
115+
// No need for file docblock.
116+
return;
117+
}
118+
}
119+
120+
/**
121+
* Process functions.
122+
*
123+
* @param File $phpcsFile The file being scanned.
124+
* @param int $stackPtr The position in the stack.
125+
*/
126+
protected function processFunctions(File $phpcsFile, int $stackPtr): void {
127+
// Missing docblocks for unit tests are treated as warnings.
128+
$isUnitTestFile = MoodleUtil::isUnitTest($phpcsFile);
129+
130+
$tokens = $phpcsFile->getTokens();
131+
132+
$missingDocblocks = [];
133+
$knownClasses = [];
134+
135+
$typePtr = $stackPtr + 1;
136+
while ($typePtr = $phpcsFile->findNext(T_FUNCTION, $typePtr + 1)) {
137+
$token = $tokens[$typePtr];
138+
$extendsOrImplements = false;
139+
140+
if ($isUnitTestFile) {
141+
if (in_array(TokenUtil::getObjectName($phpcsFile, $typePtr), $this->phpunitStandardMethodNames)) {
142+
// Skip standard PHPUnit methods.
143+
continue;
144+
}
145+
}
146+
147+
if (count($token['conditions']) > 0) {
148+
// This method has conditions (a Class, Interface, Trait, etc.).
149+
// Check if that container extends or implements anything.
150+
foreach (array_keys($token['conditions']) as $condition) {
151+
if (!array_key_exists($condition, $knownClasses)) {
152+
$extendsOrImplements = $extendsOrImplements || ObjectDeclarations::findExtendedClassName(
153+
$phpcsFile,
154+
$condition
155+
);
156+
$extendsOrImplements = $extendsOrImplements || ObjectDeclarations::findImplementedInterfaceNames(
157+
$phpcsFile,
158+
$condition
159+
);
160+
$extendsOrImplements = $extendsOrImplements || ObjectDeclarations::findExtendedInterfaceNames(
161+
$phpcsFile,
162+
$condition
163+
);
164+
$knownClasses[$condition] = $extendsOrImplements;
165+
}
166+
$extendsOrImplements = $extendsOrImplements || $knownClasses[$condition];
167+
if ($extendsOrImplements) {
168+
break;
169+
}
170+
}
171+
}
172+
173+
if (!Docblocks::getDocBlock($phpcsFile, $typePtr)) {
174+
$missingDocblocks[$typePtr] = $extendsOrImplements;
175+
}
176+
}
177+
178+
foreach ($missingDocblocks as $typePtr => $extendsOrImplements) {
179+
$token = $tokens[$typePtr];
180+
$objectName = TokenUtil::getObjectName($phpcsFile, $typePtr);
181+
$objectType = TokenUtil::getObjectType($phpcsFile, $typePtr);
182+
183+
if ($extendsOrImplements) {
184+
$phpcsFile->addWarning('Missing docblock for %s %s', $typePtr, 'Missing', [$objectType, $objectName]);
185+
} else {
186+
$phpcsFile->addError('Missing docblock for %s %s', $typePtr, 'Missing', [$objectType, $objectName]);
187+
}
188+
}
189+
}
190+
}

‎moodle/Sniffs/Commenting/PackageSniff.php

Lines changed: 3 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,9 @@
1919

2020
use MoodleHQ\MoodleCS\moodle\Util\MoodleUtil;
2121
use MoodleHQ\MoodleCS\moodle\Util\Docblocks;
22+
use MoodleHQ\MoodleCS\moodle\Util\TokenUtil;
2223
use PHP_CodeSniffer\Sniffs\Sniff;
2324
use PHP_CodeSniffer\Files\File;
24-
use PHPCSUtils\Utils\ObjectDeclarations;
2525

2626
/**
2727
* Checks that all test classes and global functions have appropriate @package tags.
@@ -78,54 +78,13 @@ public function process(File $phpcsFile, $stackPtr) {
7878
$docblock = Docblocks::getDocBlock($phpcsFile, $typePtr);
7979

8080
if ($docblock === null) {
81-
$objectName = $this->getObjectName($phpcsFile, $typePtr);
82-
$objectType = $this->getObjectType($phpcsFile, $typePtr);
83-
$phpcsFile->addError('Missing doc comment for %s %s', $typePtr, 'Missing', [$objectType, $objectName]);
84-
8581
continue;
8682
}
8783

8884
$this->checkDocblock($phpcsFile, $typePtr, $docblock);
8985
}
9086
}
9187

92-
/**
93-
* Get the human-readable object type.
94-
*
95-
* @param File $phpcsFile
96-
* @param int $stackPtr
97-
* @return string
98-
*/
99-
protected function getObjectType(
100-
File $phpcsFile,
101-
int $stackPtr
102-
): string {
103-
$tokens = $phpcsFile->getTokens();
104-
if ($tokens[$stackPtr]['code'] === T_OPEN_TAG) {
105-
return 'file';
106-
}
107-
return $tokens[$stackPtr]['content'];
108-
}
109-
110-
/**
111-
* Get the human readable object name.
112-
*
113-
* @param File $phpcsFile
114-
* @param int $stackPtr
115-
* @return string
116-
*/
117-
protected function getObjectName(
118-
File $phpcsFile,
119-
int $stackPtr
120-
): string {
121-
$tokens = $phpcsFile->getTokens();
122-
if ($tokens[$stackPtr]['code'] === T_OPEN_TAG) {
123-
return basename($phpcsFile->getFilename());
124-
}
125-
126-
return ObjectDeclarations::getName($phpcsFile, $stackPtr);
127-
}
128-
12988
/**
13089
* Check the docblock for a @package tag.
13190
*
@@ -140,8 +99,8 @@ protected function checkDocblock(
14099
array $docblock
141100
): bool {
142101
$tokens = $phpcsFile->getTokens();
143-
$objectName = $this->getObjectName($phpcsFile, $stackPtr);
144-
$objectType = $this->getObjectType($phpcsFile, $stackPtr);
102+
$objectName = TokenUtil::getObjectName($phpcsFile, $stackPtr);
103+
$objectType = TokenUtil::getObjectType($phpcsFile, $stackPtr);
145104
$expectedPackage = MoodleUtil::getMoodleComponent($phpcsFile, true);
146105

147106
// Nothing to do if we have been unable to determine the package

‎moodle/Tests/MoodleCSBaseTestCase.php

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,9 @@ abstract class MoodleCSBaseTestCase extends \PHPUnit\Framework\TestCase
5858
*/
5959
protected ?string $fixture = null;
6060

61+
/** @var string|null Fixture file content */
62+
protected ?string $fixtureContent = null;
63+
6164
/**
6265
* @var array custom config elements to setup before running phpcs. name => value.
6366
*/
@@ -143,12 +146,22 @@ protected function setSniff($sniff) {
143146
* @param string $fixture full path to the file used as input (fixture).
144147
*/
145148
protected function setFixture($fixture) {
149+
if ($this->fixtureContent !== null) {
150+
$this->fail('Fixture file content already set, cannot set it again.');
151+
}
146152
if (!is_readable($fixture)) {
147153
$this->fail('Unreadable fixture passed: ' . $fixture);
148154
}
149155
$this->fixture = $fixture;
150156
}
151157

158+
protected function setFixtureFileContent(string $content): void {
159+
if ($this->fixture !== null) {
160+
$this->fail('Fixture file content already set, cannot set it again.');
161+
}
162+
$this->fixtureContent = $content;
163+
}
164+
152165
/**
153166
* Set the error expectations to ve verified against execution results.
154167
*
@@ -210,7 +223,11 @@ protected function verifyCsResults() {
210223

211224
// Let's process the fixture.
212225
try {
213-
$phpcsfile = new \PHP_CodeSniffer\Files\LocalFile($this->fixture, $ruleset, $config);
226+
if ($this->fixtureContent !== null) {
227+
$phpcsfile = new \PHP_CodeSniffer\Files\DummyFile($this->fixtureContent, $ruleset, $config);
228+
} else {
229+
$phpcsfile = new \PHP_CodeSniffer\Files\LocalFile($this->fixture, $ruleset, $config);
230+
}
214231
$phpcsfile->process();
215232
} catch (\Exception $e) {
216233
$this->fail('An unexpected exception has been caught: ' . $e->getMessage());
@@ -237,8 +254,8 @@ protected function verifyCsResults() {
237254
}
238255

239256
// Now, if there is a file, with the same name than the
240-
// fixture + .fix, use it to verify that the fixed does its job too.
241-
if (is_readable($this->fixture . '.fixed')) {
257+
// fixture + .fix, use it to verify that the fixed does its job too.)
258+
if ($this->fixture !== null && is_readable($this->fixture . '.fixed')) {
242259
$diff = $phpcsfile->fixer->generateDiff($this->fixture . '.fixed');
243260
if (trim($diff) !== '') {
244261
$filename = basename($this->fixture);

0 commit comments

Comments
 (0)