Skip to content

Commit

Permalink
Ensure we stop looking for file phpdoc block asap
Browse files Browse the repository at this point in the history
Previously, we were allowing the getDocTagFromOpenTag()
method to advance too much when looking for file phpdoc
blocks. Now we stop as soon as any of the stop tokens is found.

Note that this case is very edge one, only reproducible when
there are lots of missing class/function phpdoc block, causing
some other phpdoc block to be picked.

In any case, I think that it's perfectly ok to shortcut the
search that way, it can save us some precious iterations.

Fixes moodlehq#172
  • Loading branch information
stronk7 committed Jun 29, 2024
1 parent 85e9e0a commit 13b8e00
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 2 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ The format of this change log follows the advice given at [Keep a CHANGELOG](htt
- The `moodle.NamingConventions.ValidFunctionName` sniff will now ignore errors on methods employing the `#[\Override]` attribute.
- The `moodle.Commenting.MissingDocblock` sniff no longer warns about missing docs on non-global anonymous classes, for example those written as an instance class in a unit test.

### Fixed
- Fixed an edge case leading to the file phpdoc block being incorrectly detected by various sniffs.

## [v3.4.9] - 2024-06-19
### Fixed
- Fixed a recent regression by allowing to the `moodle.Files.BoilerplateComment` sniff to contain "extra" consecutive comment lines immediately after the official boilerplate ends.
Expand Down
14 changes: 12 additions & 2 deletions moodle/Tests/Util/DocblocksTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,21 @@
*/
class DocblocksTest extends MoodleCSBaseTestCase
{
public function testgetDocBlockPointer(): void {
public static function getNullDocBlockPointerProvider(): array {
return [
'global_scope_code' => ['none_global_scope.php'],
'oop_scope_code' => ['none.php'],
];
}

/**
* @dataProvider getNullDocBlockPointerProvider
*/
public function testGetNullDocBlockPointer(string $fixture): void {
$phpcsConfig = new Config();
$phpcsRuleset = new Ruleset($phpcsConfig);
$phpcsFile = new \PHP_CodeSniffer\Files\LocalFile(
__DIR__ . '/fixtures/docblocks/none.php',
__DIR__ . '/fixtures/docblocks/' . $fixture,
$phpcsRuleset,
$phpcsConfig
);
Expand Down
5 changes: 5 additions & 0 deletions moodle/Tests/Util/fixtures/docblocks/none.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,9 @@
namespace MoodleHQ\MoodleCS\moodle\Tests;

class example {
public function get_something() {
/** @var int $variable */
$variable = 1;
return $variable;
}
}
18 changes: 18 additions & 0 deletions moodle/Tests/Util/fixtures/docblocks/none_global_scope.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<?php
// This file is part of Moodle - http://moodle.org/
//
// Moodle is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// Moodle is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU General Public License
// along with Moodle. If not, see <http://www.gnu.org/licenses/>.

// Just an extreme case, that makes no sense within Moodle, but it's a valid PHP file.
$variable = 1;
5 changes: 5 additions & 0 deletions moodle/Util/Docblocks.php
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,11 @@ protected static function getDocTagFromOpenTag(
];

while ($stackPtr = $phpcsFile->findNext($ignore, ($stackPtr + 1), null, true)) {
// If we have arrived to a stop token, and haven't found the file docblock yet, then there isn't one.
if (in_array($tokens[$stackPtr]['code'], $stopAtTypes)) {
return null;
}

if ($tokens[$stackPtr]['code'] === T_NAMESPACE || $tokens[$stackPtr]['code'] === T_USE) {
$stackPtr = $phpcsFile->findNext(T_SEMICOLON, $stackPtr + 1);
continue;
Expand Down

0 comments on commit 13b8e00

Please sign in to comment.