Skip to content

Commit

Permalink
Implement getRequiredFunctionCalls check and use it in filter plugin …
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
kabalin committed Oct 24, 2024
1 parent 8b2d4c3 commit 2b36993
Show file tree
Hide file tree
Showing 13 changed files with 208 additions and 0 deletions.
1 change: 1 addition & 0 deletions .php-cs-fixer.cache

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions .phpunit.result.cache

Large diffs are not rendered by default.

6 changes: 6 additions & 0 deletions docs/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,14 @@ 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]
### Added
- Improvments to plugin validation implementation:
`getRequiredFunctionCalls` in plugin type specific `Requirements` class can be used to validate that file contains function call.
`FileTokens::notFoundHint` can be used to give some context for validation error to improve DX.

### Fixed
- Fixed stylelinting error in non-theme plugins containing scss.
- Updated filter plugin validation requirements to comply with Moodle 4.5

### Removed
- Stylelint less component task (`grunt stylelint:less`) has been deprecated in
Expand Down
22 changes: 22 additions & 0 deletions src/Parser/StatementFilter.php
Original file line number Diff line number Diff line change
Expand Up @@ -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_;
Expand Down Expand Up @@ -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.
*
Expand Down
31 changes: 31 additions & 0 deletions src/PluginValidate/Finder/FileTokens.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,13 @@ class FileTokens
*/
public string $file;

/**
* Not found error hint.
*
* @var string
*/
public string $hint = '';

/**
* @param string $file
*/
Expand Down Expand Up @@ -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
*
Expand Down Expand Up @@ -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;
}
}
37 changes: 37 additions & 0 deletions src/PluginValidate/Finder/FunctionCallFinder.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
<?php

/*
* This file is part of the Moodle Plugin CI package.
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*
* Copyright (c) 2024 Moodle Pty Ltd <support@moodle.com>
* 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);
}
}
}
}
5 changes: 5 additions & 0 deletions src/PluginValidate/PluginValidate.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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));
}
}
}
}
Expand All @@ -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());
}

/**
Expand Down
7 changes: 7 additions & 0 deletions src/PluginValidate/Requirements/AbstractRequirements.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down
11 changes: 11 additions & 0 deletions src/PluginValidate/Requirements/FilterRequirements.php
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
];
}
}
5 changes: 5 additions & 0 deletions src/PluginValidate/Requirements/GenericRequirements.php
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
5 changes: 5 additions & 0 deletions tests/Fixture/moodle-local_ci/lib.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
// set then can check for anything, like CUSTOM-123 or https://github.com
// or whatever.

defined('MOODLE_INTERNAL') || die();

/**
* Add
*
Expand Down Expand Up @@ -72,3 +74,6 @@ public function add($a, $b) {
return $a + $b;
}
}

// Call function.
local_ci_subtract(1, 2);
44 changes: 44 additions & 0 deletions tests/PluginValidate/Finder/FunctionCallFinderTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
<?php

/*
* This file is part of the Moodle Plugin CI package.
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*
* Copyright (c) 2024 Moodle Pty Ltd <support@moodle.com>
* 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);
}
}
33 changes: 33 additions & 0 deletions tests/PluginValidate/Requirements/FilterRequirementsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

0 comments on commit 2b36993

Please sign in to comment.