From ced63d2c4ab5d0a4b25388437ee00a1d40ac77f9 Mon Sep 17 00:00:00 2001 From: Ruslan Kabalin Date: Thu, 10 Oct 2024 12:44:03 +0100 Subject: [PATCH] Implement getRequiredFunctionCalls check and use it in filter plugin validation. This address scenario where file is supposed to contain certain function call, such as `class_alias` in Filter plugin type backward compatibility support per https://moodledev.io/docs/4.5/devupdate#filter-plugins The patch makes possible for deleveloper to specify: * getRequiredFunctionCalls to make sure file contains function call as name suggests. * FileTokens::notFoundHint to give some context for requirement to improve developer experience. This works with FileTokens in any other validation methods. --- docs/CHANGELOG.md | 3 ++ src/Parser/StatementFilter.php | 22 ++++++++++ src/PluginValidate/Finder/FileTokens.php | 31 +++++++++++++ .../Finder/FunctionCallFinder.php | 37 ++++++++++++++++ src/PluginValidate/PluginValidate.php | 5 +++ .../Requirements/AbstractRequirements.php | 7 +++ .../Requirements/FilterRequirements.php | 11 +++++ .../Requirements/GenericRequirements.php | 5 +++ tests/Fixture/moodle-local_ci/lib.php | 5 +++ .../Finder/FunctionCallFinderTest.php | 44 +++++++++++++++++++ .../Requirements/FilterRequirementsTest.php | 33 ++++++++++++++ 11 files changed, 203 insertions(+) create mode 100644 src/PluginValidate/Finder/FunctionCallFinder.php create mode 100644 tests/PluginValidate/Finder/FunctionCallFinderTest.php diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 7d5fc8fe..8e58506c 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -9,6 +9,9 @@ This project adheres to [Semantic Versioning](https://semver.org/). The format of this change log follows the advice given at [Keep a CHANGELOG](https://keepachangelog.com). ## [Unreleased] +### Fixed +- Adjust filter plugin validation requirements to comply with Moodle 4.5 + ## [4.5.4] - 2024-08-23 ### Changed - Fixed nvm loading issue caused by upstream regression. diff --git a/src/Parser/StatementFilter.php b/src/Parser/StatementFilter.php index 04649de6..4d5bb4e2 100644 --- a/src/Parser/StatementFilter.php +++ b/src/Parser/StatementFilter.php @@ -14,9 +14,11 @@ use PhpParser\Node\Expr\Array_; use PhpParser\Node\Expr\Assign; +use PhpParser\Node\Expr\FuncCall; use PhpParser\Node\Expr\PropertyFetch; use PhpParser\Node\Expr\Variable; use PhpParser\Node\Identifier; +use PhpParser\Node\Name; use PhpParser\Node\Scalar\String_; use PhpParser\Node\Stmt; use PhpParser\Node\Stmt\Class_; @@ -116,6 +118,26 @@ public function filterAssignments(array $statements): array return $assigns; } + /** + * Extract all the function call expressions from the statements. + * + * @param Stmt[] $statements + * + * @return FuncCall[] + */ + public function filterFunctionCalls(array $statements): array + { + $calls = []; + foreach ($statements as $statement) { + // Only expressions that are function calls. + if ($statement instanceof Expression && $statement->expr instanceof FuncCall) { + $calls[] = $statement->expr; + } + } + + return $calls; + } + /** * Find first variable assignment with a given name. * diff --git a/src/PluginValidate/Finder/FileTokens.php b/src/PluginValidate/Finder/FileTokens.php index ca0014e3..4f5b08f4 100644 --- a/src/PluginValidate/Finder/FileTokens.php +++ b/src/PluginValidate/Finder/FileTokens.php @@ -29,6 +29,13 @@ class FileTokens */ public string $file; + /** + * Not found error hint. + * + * @var string + */ + public string $hint = ''; + /** * @param string $file */ @@ -59,6 +66,16 @@ public function hasTokens(): bool return !empty($this->tokens); } + /** + * Do we have any hint? + * + * @return bool + */ + public function hasHint(): bool + { + return !empty($this->hint); + } + /** * @param Token $token * @@ -166,4 +183,18 @@ public function resetTokens(): void $token->reset(); } } + + /** + * Not found error additional information guiding user how to fix it (optional). + * + * @param string $hint + * + * @return self + */ + public function notFoundHint(string $hint): self + { + $this->hint = $hint; + + return $this; + } } diff --git a/src/PluginValidate/Finder/FunctionCallFinder.php b/src/PluginValidate/Finder/FunctionCallFinder.php new file mode 100644 index 00000000..609ec1e3 --- /dev/null +++ b/src/PluginValidate/Finder/FunctionCallFinder.php @@ -0,0 +1,37 @@ + + * License http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +namespace MoodlePluginCI\PluginValidate\Finder; + +use PhpParser\Node\Name; + +/** + * Finds function call. + */ +class FunctionCallFinder extends AbstractParserFinder +{ + public function getType(): string + { + return 'function call'; + } + + public function findTokens($file, FileTokens $fileTokens): void + { + $statements = $this->parser->parseFile($file); + + foreach ($this->filter->filterFunctionCalls($statements) as $funccall) { + if ($funccall->name instanceof Name) { + $fileTokens->compare((string) $funccall->name); + } + } + } +} diff --git a/src/PluginValidate/PluginValidate.php b/src/PluginValidate/PluginValidate.php index 93c03b82..be9c5ee9 100644 --- a/src/PluginValidate/PluginValidate.php +++ b/src/PluginValidate/PluginValidate.php @@ -17,6 +17,7 @@ use MoodlePluginCI\PluginValidate\Finder\ClassFinder; use MoodlePluginCI\PluginValidate\Finder\FileTokens; use MoodlePluginCI\PluginValidate\Finder\FinderInterface; +use MoodlePluginCI\PluginValidate\Finder\FunctionCallFinder; use MoodlePluginCI\PluginValidate\Finder\FunctionFinder; use MoodlePluginCI\PluginValidate\Finder\LangFinder; use MoodlePluginCI\PluginValidate\Finder\TableFinder; @@ -98,6 +99,9 @@ public function addMessagesFromTokens(string $type, FileTokens $fileTokens): voi $this->addSuccess(sprintf('In %s, found %s %s', $fileTokens->file, $type, implode(' OR ', $token->tokens))); } else { $this->addError(sprintf('In %s, failed to find %s %s', $fileTokens->file, $type, implode(' OR ', $token->tokens))); + if ($fileTokens->hasHint()) { + $this->addError(sprintf('Hint: %s', $fileTokens->hint)); + } } } } @@ -115,6 +119,7 @@ public function verifyRequirements(): void $this->findRequiredTokens(new TableFinder(), [$this->requirements->getRequiredTables()]); $this->findRequiredTokens(new TablePrefixFinder(), [$this->requirements->getRequiredTablePrefix()]); $this->findRequiredTokens(new BehatTagFinder(), $this->requirements->getRequiredBehatTags()); + $this->findRequiredTokens(new FunctionCallFinder(), $this->requirements->getRequiredFunctionCalls()); } /** diff --git a/src/PluginValidate/Requirements/AbstractRequirements.php b/src/PluginValidate/Requirements/AbstractRequirements.php index 32c1e154..5ae9d70e 100644 --- a/src/PluginValidate/Requirements/AbstractRequirements.php +++ b/src/PluginValidate/Requirements/AbstractRequirements.php @@ -76,6 +76,13 @@ protected function fileExists(string $file): bool return file_exists($this->plugin->directory . '/' . $file); } + /** + * Required function calls. + * + * @return FileTokens[] + */ + abstract public function getRequiredFunctionCalls(): array; + /** * An array of required files, paths are relative to the plugin directory. * diff --git a/src/PluginValidate/Requirements/FilterRequirements.php b/src/PluginValidate/Requirements/FilterRequirements.php index c6c2ed90..caa8fd94 100644 --- a/src/PluginValidate/Requirements/FilterRequirements.php +++ b/src/PluginValidate/Requirements/FilterRequirements.php @@ -50,4 +50,15 @@ public function getRequiredStrings(): FileTokens { return FileTokens::create($this->getLangFile())->mustHave('filtername'); } + + public function getRequiredFunctionCalls(): array + { + if ($this->moodleVersion <= 404 && !$this->fileExists('classes/text_filter.php')) { + return []; + } + + return [ + FileTokens::create('filter.php')->mustHave('class_alias')->notFoundHint('https://moodledev.io/docs/4.5/devupdate#filter-plugins'), + ]; + } } diff --git a/src/PluginValidate/Requirements/GenericRequirements.php b/src/PluginValidate/Requirements/GenericRequirements.php index d1dccf4f..d6cd412b 100644 --- a/src/PluginValidate/Requirements/GenericRequirements.php +++ b/src/PluginValidate/Requirements/GenericRequirements.php @@ -46,6 +46,11 @@ public function getRequiredClasses(): array return []; } + public function getRequiredFunctionCalls(): array + { + return []; + } + public function getRequiredStrings(): FileTokens { return FileTokens::create($this->getLangFile())->mustHave('pluginname'); diff --git a/tests/Fixture/moodle-local_ci/lib.php b/tests/Fixture/moodle-local_ci/lib.php index 75451d04..2a000d7d 100644 --- a/tests/Fixture/moodle-local_ci/lib.php +++ b/tests/Fixture/moodle-local_ci/lib.php @@ -28,6 +28,8 @@ // set then can check for anything, like CUSTOM-123 or https://github.com // or whatever. +defined('MOODLE_INTERNAL') || die(); + /** * Add * @@ -72,3 +74,6 @@ public function add($a, $b) { return $a + $b; } } + +// Call function. +local_ci_subtract(1, 2); diff --git a/tests/PluginValidate/Finder/FunctionCallFinderTest.php b/tests/PluginValidate/Finder/FunctionCallFinderTest.php new file mode 100644 index 00000000..69f44e2b --- /dev/null +++ b/tests/PluginValidate/Finder/FunctionCallFinderTest.php @@ -0,0 +1,44 @@ + + * License http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +namespace MoodlePluginCI\Tests\PluginValidate\Finder; + +use MoodlePluginCI\PluginValidate\Finder\FileTokens; +use MoodlePluginCI\PluginValidate\Finder\FunctionCallFinder; + +class FunctionCallFinderTest extends \PHPUnit\Framework\TestCase +{ + public function testFindTokens() + { + $file = __DIR__ . '/../../Fixture/moodle-local_ci/lib.php'; + $fileTokens = FileTokens::create('lib.php')->mustHave('local_ci_subtract'); + + $finder = new FunctionCallFinder(); + $finder->findTokens($file, $fileTokens); + + $this->assertTrue($fileTokens->hasFoundAllTokens()); + $this->assertFalse($fileTokens->hasHint()); + } + + public function testFindTokensNotFound() + { + $file = __DIR__ . '/../../Fixture/moodle-local_ci/lib.php'; + $fileTokens = FileTokens::create('lib.php')->mustHave('exit')->notFoundHint('Exit not found'); + + $finder = new FunctionCallFinder(); + $finder->findTokens($file, $fileTokens); + + $this->assertFalse($fileTokens->hasFoundAllTokens()); + $this->assertTrue($fileTokens->hasHint()); + $this->assertSame('Exit not found', $fileTokens->hint); + } +} diff --git a/tests/PluginValidate/Requirements/FilterRequirementsTest.php b/tests/PluginValidate/Requirements/FilterRequirementsTest.php index 7f44ed44..13dd7765 100644 --- a/tests/PluginValidate/Requirements/FilterRequirementsTest.php +++ b/tests/PluginValidate/Requirements/FilterRequirementsTest.php @@ -114,4 +114,37 @@ public function testGetRequiredStrings() $this->assertInstanceOf('MoodlePluginCI\PluginValidate\Finder\FileTokens', $fileToken); $this->assertSame('lang/en/filter_activitynames.php', $fileToken->file); } + + public function testGetRequiredFunctionCalls404() + { + $requirements = $this->getMockBuilder('MoodlePluginCI\PluginValidate\Requirements\FilterRequirements') + ->setConstructorArgs([new Plugin('filter_activitynames', 'filter', 'activitynames', ''), 404]) + ->onlyMethods(['fileExists']) + ->getMock(); + // On first call fileExists return false, on second call return true. + $requirements->method('fileExists') + ->with($this->identicalTo('classes/text_filter.php')) + ->willReturn(false, true); + + // If classes/text_filter.php does not exist, expect class alias is not needed in filter.php. + $calls = $requirements->getRequiredFunctionCalls(); + $this->assertCount(0, $calls); + + // If classes/text_filter.php exists, expect class alias in filter.php (4.5 plugin backward compatibility). + $calls = $requirements->getRequiredFunctionCalls(); + $this->assertCount(1, $calls); + $call = reset($calls); + $this->assertInstanceOf('MoodlePluginCI\PluginValidate\Finder\FileTokens', $call); + $this->assertSame('filter.php', $call->file); + } + + public function testGetRequiredFunctionCalls() + { + $calls = $this->requirements->getRequiredFunctionCalls(); + + $this->assertCount(1, $calls); + $call = reset($calls); + $this->assertInstanceOf('MoodlePluginCI\PluginValidate\Finder\FileTokens', $call); + $this->assertSame('filter.php', $call->file); + } }