From 13b8e00d27a5b6a03f44a9c0471fc81c85ae32e8 Mon Sep 17 00:00:00 2001 From: "Eloy Lafuente (stronk7)" Date: Thu, 27 Jun 2024 18:26:47 +0200 Subject: [PATCH] Ensure we stop looking for file phpdoc block asap 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 #172 --- CHANGELOG.md | 3 +++ moodle/Tests/Util/DocblocksTest.php | 14 ++++++++++++-- moodle/Tests/Util/fixtures/docblocks/none.php | 5 +++++ .../fixtures/docblocks/none_global_scope.php | 18 ++++++++++++++++++ moodle/Util/Docblocks.php | 5 +++++ 5 files changed, 43 insertions(+), 2 deletions(-) create mode 100644 moodle/Tests/Util/fixtures/docblocks/none_global_scope.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b013d15..37ea2f0b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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. diff --git a/moodle/Tests/Util/DocblocksTest.php b/moodle/Tests/Util/DocblocksTest.php index b0362ad8..5bcc5862 100644 --- a/moodle/Tests/Util/DocblocksTest.php +++ b/moodle/Tests/Util/DocblocksTest.php @@ -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 ); diff --git a/moodle/Tests/Util/fixtures/docblocks/none.php b/moodle/Tests/Util/fixtures/docblocks/none.php index 3b6c5832..ab8313a1 100644 --- a/moodle/Tests/Util/fixtures/docblocks/none.php +++ b/moodle/Tests/Util/fixtures/docblocks/none.php @@ -17,4 +17,9 @@ namespace MoodleHQ\MoodleCS\moodle\Tests; class example { + public function get_something() { + /** @var int $variable */ + $variable = 1; + return $variable; + } } diff --git a/moodle/Tests/Util/fixtures/docblocks/none_global_scope.php b/moodle/Tests/Util/fixtures/docblocks/none_global_scope.php new file mode 100644 index 00000000..98f557dd --- /dev/null +++ b/moodle/Tests/Util/fixtures/docblocks/none_global_scope.php @@ -0,0 +1,18 @@ +. + +// Just an extreme case, that makes no sense within Moodle, but it's a valid PHP file. +$variable = 1; diff --git a/moodle/Util/Docblocks.php b/moodle/Util/Docblocks.php index 6b86cb7e..a67b5d20 100644 --- a/moodle/Util/Docblocks.php +++ b/moodle/Util/Docblocks.php @@ -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;