Skip to content

Commit

Permalink
Ignore LoadIncludes rule if module and file name unknown (#524)
Browse files Browse the repository at this point in the history
  • Loading branch information
Boegie authored Apr 7, 2023
1 parent 46786f8 commit 21b6249
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 14 deletions.
4 changes: 2 additions & 2 deletions src/Rules/Drupal/LoadIncludeBase.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,11 @@ protected function parseLoadIncludeArgs(Node\Arg $module, Node\Arg $type, ?Node\
{
$moduleName = $this->getStringArgValue($module->value, $scope);
if ($moduleName === null) {
return [];
return [false, false];
}
$fileType = $this->getStringArgValue($type->value, $scope);
if ($fileType === null) {
return [];
return [false, false];
}
$baseName = null;
if ($name !== null) {
Expand Down
5 changes: 5 additions & 0 deletions src/Rules/Drupal/LoadIncludes.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,11 @@ public function processNode(Node $node, Scope $scope): array
try {
// Try to invoke it similarly as the module handler itself.
[$moduleName, $filename] = $this->parseLoadIncludeArgs($args[0], $args[1], $args[2] ?? null, $scope);
if (!$moduleName && !$filename) {
// Couldn't determine module- nor file-name, most probably
// because it's a variable. Nothing to load, bail now.
return [];
}
$module = $this->extensionMap->getModule($moduleName);
if ($module === null) {
return [
Expand Down
37 changes: 25 additions & 12 deletions tests/src/Rules/LoadIncludesRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,26 +12,39 @@ protected function getRule(): \PHPStan\Rules\Rule
return self::getContainer()->getByType(LoadIncludes::class);
}

public function testRule(): void
/**
* @dataProvider cases
*/
public function test(array $files, array $errors): void
{
$this->analyse([
__DIR__ . '/../../fixtures/drupal/modules/phpstan_fixtures/phpstan_fixtures.module'
],
$this->analyse($files, $errors);
}

public function cases(): \Generator
{
yield [
[
__DIR__ . '/../../fixtures/drupal/modules/phpstan_fixtures/phpstan_fixtures.module'
],
[
[
'File modules/phpstan_fixtures/phpstan_fixtures.fetch.inc could not be loaded from Drupal\Core\Extension\ModuleHandlerInterface::loadInclude',
30
]
]);
}
],
];

public function testFormStateLoadInclude(): void
{
$this->analyse([
__DIR__ . '/../../fixtures/drupal/core/tests/Drupal/Tests/Core/Form/FormStateTest.php'
],
yield [
[
]);
__DIR__ . '/../../fixtures/drupal/core/tests/Drupal/Tests/Core/Form/FormStateTest.php'
],
[],
];

yield 'bug-516.php' => [
[__DIR__.'/data/bug-516.php'],
[]
];
}

}
36 changes: 36 additions & 0 deletions tests/src/Rules/data/bug-516.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
<?php

namespace Bug516;

use Drupal\Core\Extension\ModuleHandlerInterface;

/**
* Tests moduleHandler::loadInclude where first argument is a variable.
*/
class TestClass {

/**
* Code snippet from \Drupal\TestTools\Extension\SchemaInspector.
*
* @param \Drupal\Core\Extension\ModuleHandlerInterface $handler
* @param string $module
*/
function bug516(ModuleHandlerInterface $handler, string $module): void
{
$handler->loadInclude($module, 'install');
}

/**
* Code snippet from \_update_fix_missing_schema.
*/
function bug516_2(): void
{
$module_handler = \Drupal::moduleHandler();
$enabled_modules = $module_handler->getModuleList();

foreach (array_keys($enabled_modules) as $module) {
$module_handler->loadInclude($module, 'install');
}
}

}

0 comments on commit 21b6249

Please sign in to comment.