From caf2f8313829534febee34e2d6ffe0151894f074 Mon Sep 17 00:00:00 2001 From: "Eirik S. Morland" Date: Tue, 9 Jun 2020 07:24:16 +0200 Subject: [PATCH 1/4] Add a rule for loadIncludes. Fix #124 --- extension.neon | 1 + src/Rules/Drupal/LoadIncludes.php | 77 +++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+) create mode 100644 src/Rules/Drupal/LoadIncludes.php diff --git a/extension.neon b/extension.neon index 94b60a20..18eb3738 100644 --- a/extension.neon +++ b/extension.neon @@ -23,6 +23,7 @@ parametersSchema: entityTypeStorageMapping: arrayOf(string()) ]) rules: + - PHPStan\Rules\Drupal\LoadIncludes - PHPStan\Rules\Classes\PluginManagerInspectionRule - PHPStan\Rules\Drupal\Coder\DiscouragedFunctionsRule - PHPStan\Rules\Drupal\GlobalDrupalDependencyInjectionRule diff --git a/src/Rules/Drupal/LoadIncludes.php b/src/Rules/Drupal/LoadIncludes.php new file mode 100644 index 00000000..f9be90f9 --- /dev/null +++ b/src/Rules/Drupal/LoadIncludes.php @@ -0,0 +1,77 @@ +name; + if ($method_name instanceof Node\Identifier) { + $method_name = $method_name->name; + } + if ($method_name !== 'loadInclude') { + return []; + } + $var_name = $node->var->name; + if ($var_name instanceof Node\Identifier) { + $var_name = $var_name->name; + } + if (!$var_name) { + return []; + } + $type = $scope->getVariableType($var_name); + $reflected = new \ReflectionClass($type->getClassName()); + $implements = $reflected->implementsInterface(ModuleHandlerInterface::class); + if (!$implements) { + return []; + } + // Try to invoke it similarily as the module handler itself. + $finder = new DrupalFinder(); + $finder->locateRoot(dirname($GLOBALS['autoloaderInWorkingDirectory'])); + $drupal_root = $finder->getDrupalRoot(); + $extensionDiscovery = new ExtensionDiscovery($drupal_root); + $modules = $extensionDiscovery->scan('module'); + $module_arg = $node->args[0]->value->value; + $type_arg = $node->args[1]->value->value; + $name_arg = !empty($node->args[2]) ? $node->args[2] : null; + if (!$name_arg) { + $name_arg = $module_arg; + } else { + $name_arg = $name_arg->value->value; + } + if (empty($modules[$module_arg])) { + return []; + } + /** @var \PHPStan\Drupal\Extension $module */ + $module = $modules[$module_arg]; + $file = $drupal_root . '/' . $module->getPath() . "/$name_arg.$type_arg"; + if (is_file($file)) { + require_once $file; + return []; + } + return ['File could not be loaded from ModuleHandler::loadInclude']; + } catch (\Throwable $e) { + } + + return []; + } +} From 3aca30d0b0de624f65e3bfda9b236d38515b9e19 Mon Sep 17 00:00:00 2001 From: "Eirik S. Morland" Date: Thu, 2 Jul 2020 23:22:20 +0200 Subject: [PATCH 2/4] Cache scan results. Make new rule a service --- extension.neon | 6 +++++- src/Drupal/ExtensionDiscovery.php | 12 +++++++++++- src/Rules/Drupal/LoadIncludes.php | 18 +++++++++++++++++- 3 files changed, 33 insertions(+), 3 deletions(-) diff --git a/extension.neon b/extension.neon index 18eb3738..398bc7e1 100644 --- a/extension.neon +++ b/extension.neon @@ -23,7 +23,6 @@ parametersSchema: entityTypeStorageMapping: arrayOf(string()) ]) rules: - - PHPStan\Rules\Drupal\LoadIncludes - PHPStan\Rules\Classes\PluginManagerInspectionRule - PHPStan\Rules\Drupal\Coder\DiscouragedFunctionsRule - PHPStan\Rules\Drupal\GlobalDrupalDependencyInjectionRule @@ -44,3 +43,8 @@ services: - class: PHPStan\Reflection\EntityFieldsViaMagicReflectionExtension tags: [phpstan.broker.propertiesClassReflectionExtension] + - + class: PHPStan\Rules\Drupal\LoadIncludes + tags: [phpstan.rules.rule] + arguments: + - %drupal.drupal_root% diff --git a/src/Drupal/ExtensionDiscovery.php b/src/Drupal/ExtensionDiscovery.php index 104393e5..c58677c1 100644 --- a/src/Drupal/ExtensionDiscovery.php +++ b/src/Drupal/ExtensionDiscovery.php @@ -118,6 +118,15 @@ public function __construct($root) */ public function scan($type) { + static $scanresult; + if (!$scanresult) { + $scanresult = []; + } + + if (isset($scanresult[$type])) { + return $scanresult[$type]; + } + $searchdirs = []; // Search the core directory. $searchdirs[static::ORIGIN_CORE] = 'core'; @@ -152,7 +161,8 @@ public function scan($type) $files = $this->sort($files, $origin_weights); // Process and return the list of extensions keyed by extension name. - return $this->process($files); + $scanresult[$type] = $this->process($files); + return $scanresult[$type]; } /** diff --git a/src/Rules/Drupal/LoadIncludes.php b/src/Rules/Drupal/LoadIncludes.php index f9be90f9..712dd2cd 100644 --- a/src/Rules/Drupal/LoadIncludes.php +++ b/src/Rules/Drupal/LoadIncludes.php @@ -14,6 +14,22 @@ class LoadIncludes implements Rule { + + /** + * The project root. + * + * @var string + */ + protected $projectRoot; + + /** + * LoadIncludes constructor. + */ + public function __construct($project_root) + { + $this->projectRoot = $project_root; + } + public function getNodeType(): string { return Node\Expr\MethodCall::class; @@ -46,7 +62,7 @@ public function processNode(Node $node, Scope $scope): array } // Try to invoke it similarily as the module handler itself. $finder = new DrupalFinder(); - $finder->locateRoot(dirname($GLOBALS['autoloaderInWorkingDirectory'])); + $finder->locateRoot($this->projectRoot); $drupal_root = $finder->getDrupalRoot(); $extensionDiscovery = new ExtensionDiscovery($drupal_root); $modules = $extensionDiscovery->scan('module'); From b01f06ef169a69f1dae7b623c8eb696ef55a39cb Mon Sep 17 00:00:00 2001 From: Matt Glaman Date: Sun, 26 Jul 2020 11:59:41 -0500 Subject: [PATCH 3/4] Add tests, fix PHPStan errors --- src/Rules/Drupal/LoadIncludes.php | 70 +++++++++++-------- .../phpstan_fixtures/phpstan_fixtures.module | 11 +++ tests/src/DrupalIntegrationTest.php | 4 +- 3 files changed, 55 insertions(+), 30 deletions(-) diff --git a/src/Rules/Drupal/LoadIncludes.php b/src/Rules/Drupal/LoadIncludes.php index 712dd2cd..288fcaaa 100644 --- a/src/Rules/Drupal/LoadIncludes.php +++ b/src/Rules/Drupal/LoadIncludes.php @@ -11,6 +11,7 @@ use PHPStan\Reflection\MethodReflection; use PHPStan\Rules\Rule; use PHPStan\ShouldNotHappenException; +use PHPStan\Type\ObjectType; class LoadIncludes implements Rule { @@ -24,8 +25,9 @@ class LoadIncludes implements Rule /** * LoadIncludes constructor. + * @param string $project_root */ - public function __construct($project_root) + public function __construct(string $project_root) { $this->projectRoot = $project_root; } @@ -38,26 +40,30 @@ public function getNodeType(): string public function processNode(Node $node, Scope $scope): array { assert($node instanceof Node\Expr\MethodCall); + if (!$node->name instanceof Node\Identifier) { + return []; + } + $method_name = $node->name->toString(); + if ($method_name !== 'loadInclude') { + return []; + } + $variable = $node->var; + if (!$variable instanceof Node\Expr\Variable) { + return []; + } + $var_name = $variable->name; + if (!is_string($var_name)) { + throw new ShouldNotHappenException(sprintf('Expected string for variable in %s, please open an issue on GitHub https://github.com/mglaman/phpstan-drupal/issues', get_called_class())); + } + $type = $scope->getVariableType($var_name); + assert($type instanceof ObjectType); + if (!class_exists($type->getClassName()) && !interface_exists($type->getClassName())) { + throw new ShouldNotHappenException(sprintf('Could not find class for %s from reflection.', get_called_class())); + } try { - $method_name = $node->name; - if ($method_name instanceof Node\Identifier) { - $method_name = $method_name->name; - } - if ($method_name !== 'loadInclude') { - return []; - } - $var_name = $node->var->name; - if ($var_name instanceof Node\Identifier) { - $var_name = $var_name->name; - } - if (!$var_name) { - return []; - } - $type = $scope->getVariableType($var_name); $reflected = new \ReflectionClass($type->getClassName()); - $implements = $reflected->implementsInterface(ModuleHandlerInterface::class); - if (!$implements) { + if (!$reflected->implementsInterface(ModuleHandlerInterface::class)) { return []; } // Try to invoke it similarily as the module handler itself. @@ -66,26 +72,32 @@ public function processNode(Node $node, Scope $scope): array $drupal_root = $finder->getDrupalRoot(); $extensionDiscovery = new ExtensionDiscovery($drupal_root); $modules = $extensionDiscovery->scan('module'); - $module_arg = $node->args[0]->value->value; - $type_arg = $node->args[1]->value->value; - $name_arg = !empty($node->args[2]) ? $node->args[2] : null; - if (!$name_arg) { + $module_arg = $node->args[0]; + assert($module_arg->value instanceof Node\Scalar\String_); + $type_arg = $node->args[1]; + assert($type_arg->value instanceof Node\Scalar\String_); + $name_arg = $node->args[2] ?? null; + + if ($name_arg === null) { $name_arg = $module_arg; - } else { - $name_arg = $name_arg->value->value; } - if (empty($modules[$module_arg])) { + assert($name_arg->value instanceof Node\Scalar\String_); + + $module_name = $module_arg->value->value; + if (!isset($modules[$module_name])) { return []; } - /** @var \PHPStan\Drupal\Extension $module */ - $module = $modules[$module_arg]; - $file = $drupal_root . '/' . $module->getPath() . "/$name_arg.$type_arg"; + $type_prefix = $name_arg->value->value; + $type_filename = $type_arg->value->value; + $module = $modules[$module_name]; + $file = $drupal_root . '/' . $module->getPath() . "/$type_prefix.$type_filename"; if (is_file($file)) { require_once $file; return []; } - return ['File could not be loaded from ModuleHandler::loadInclude']; + return [sprintf('File %s could not be loaded from %s::loadInclude', $file, $type->getClassName())]; } catch (\Throwable $e) { + return [sprintf('A file could not be loaded from %s::loadInclude', $type->getClassName())]; } return []; diff --git a/tests/fixtures/drupal/modules/phpstan_fixtures/phpstan_fixtures.module b/tests/fixtures/drupal/modules/phpstan_fixtures/phpstan_fixtures.module index 274a0697..95a03641 100644 --- a/tests/fixtures/drupal/modules/phpstan_fixtures/phpstan_fixtures.module +++ b/tests/fixtures/drupal/modules/phpstan_fixtures/phpstan_fixtures.module @@ -16,3 +16,14 @@ function phpstan_fixtures_get_app_root(): string { $app_root = \Drupal::getContainer()->get('app.root'); return $app_root . '/core/includes/install.inc'; } + +function phpstan_fixtures_module_load_includes_test(): array { + $module_handler = \Drupal::moduleHandler(); + $module_handler->loadInclude('locale', 'fetch.inc'); + return _locale_translation_default_update_options(); +} + +function phpstan_fixtures_module_load_includes_negative_test(): void { + $module_handler = \Drupal::moduleHandler(); + $module_handler->loadInclude('phpstan_fixtures', 'fetch.inc'); +} diff --git a/tests/src/DrupalIntegrationTest.php b/tests/src/DrupalIntegrationTest.php index 9f51408b..d58afe52 100644 --- a/tests/src/DrupalIntegrationTest.php +++ b/tests/src/DrupalIntegrationTest.php @@ -38,7 +38,7 @@ public function testDrupalTestInChildSiteContant() { public function testExtensionReportsError() { $errors = $this->runAnalyze(__DIR__ . '/../fixtures/drupal/modules/phpstan_fixtures/phpstan_fixtures.module'); - $this->assertCount(2, $errors->getErrors(), var_export($errors, true)); + $this->assertCount(3, $errors->getErrors(), var_export($errors, true)); $this->assertCount(0, $errors->getInternalErrors(), var_export($errors, true)); $errors = $errors->getErrors(); @@ -46,6 +46,8 @@ public function testExtensionReportsError() { $this->assertEquals('If condition is always false.', $error->getMessage()); $error = array_shift($errors); $this->assertEquals('Function phpstan_fixtures_MissingReturnRule() should return string but return statement is missing.', $error->getMessage()); + $error = array_shift($errors); + $this->assertStringContainsString('phpstan_fixtures/phpstan_fixtures.fetch.inc could not be loaded from Drupal\\Core\\Extension\\ModuleHandlerInterface::loadInclude', $error->getMessage()); } public function testExtensionTestSuiteAutoloading() { From 8b7c1aa983af5abc8f591e71cc52bf4a4ac107be Mon Sep 17 00:00:00 2001 From: Matt Glaman Date: Sun, 26 Jul 2020 12:04:04 -0500 Subject: [PATCH 4/4] Remove unreachable statement --- src/Rules/Drupal/LoadIncludes.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/Rules/Drupal/LoadIncludes.php b/src/Rules/Drupal/LoadIncludes.php index 288fcaaa..ffd13bca 100644 --- a/src/Rules/Drupal/LoadIncludes.php +++ b/src/Rules/Drupal/LoadIncludes.php @@ -99,7 +99,5 @@ public function processNode(Node $node, Scope $scope): array } catch (\Throwable $e) { return [sprintf('A file could not be loaded from %s::loadInclude', $type->getClassName())]; } - - return []; } }