From 1cb8e83f74aa6582905900dd240679c5ec439b82 Mon Sep 17 00:00:00 2001 From: Mariano D'Agostino Date: Fri, 29 Aug 2025 16:45:36 -0300 Subject: [PATCH 1/4] New rule to detect incorrect use of addCacheableDependency --- rules.neon | 1 + src/Rules/Drupal/CacheableDependencyRule.php | 49 +++++++++++++++++++ .../src/UsesIncorrectCacheableDependency.php | 15 ++++++ .../src/Rules/CachableDependencyRuleTest.php | 40 +++++++++++++++ 4 files changed, 105 insertions(+) create mode 100644 src/Rules/Drupal/CacheableDependencyRule.php create mode 100644 tests/fixtures/drupal/modules/phpstan_fixtures/src/UsesIncorrectCacheableDependency.php create mode 100644 tests/src/Rules/CachableDependencyRuleTest.php diff --git a/rules.neon b/rules.neon index 34f4b3b6..19d56a96 100644 --- a/rules.neon +++ b/rules.neon @@ -12,6 +12,7 @@ rules: - mglaman\PHPStanDrupal\Rules\Drupal\LoadIncludes - mglaman\PHPStanDrupal\Rules\Drupal\EntityQuery\EntityQueryHasAccessCheckRule - mglaman\PHPStanDrupal\Rules\Drupal\TestClassesProtectedPropertyModulesRule + - mglaman\PHPStanDrupal\Rules\Drupal\CacheableDependencyRule conditionalTags: mglaman\PHPStanDrupal\Rules\Drupal\Tests\TestClassSuffixNameRule: diff --git a/src/Rules/Drupal/CacheableDependencyRule.php b/src/Rules/Drupal/CacheableDependencyRule.php new file mode 100644 index 00000000..d0cbe9ee --- /dev/null +++ b/src/Rules/Drupal/CacheableDependencyRule.php @@ -0,0 +1,49 @@ + + */ +class CacheableDependencyRule implements Rule { + + /** + * {@inheritdoc} + */ + public function getNodeType(): string { + return MethodCall::class; + } + + /** + * {@inheritdoc} + */ + public function processNode(Node $node, Scope $scope): array { + if (!$node instanceof MethodCall || + !$node->name instanceof Identifier || + $node->name->toString() !== 'addCacheableDependency' + ) { + return []; + } + + $object = $scope->getType($node->args[0]->value); + + // We need to check if isInstanceOf method exists as phpstan returns + // MixedType for unknown objects. + if (method_exists($object, 'isInstanceOf') && $object->isInstanceOf('Drupal\Core\Cache\CacheableDependencyInterface')) { + return []; + } + return [ + 'Calling addCacheableDependency($object) when $object does not implement CacheableDependencyInterface effectively disables caching and should be avoided.', + ]; + } + +} + diff --git a/tests/fixtures/drupal/modules/phpstan_fixtures/src/UsesIncorrectCacheableDependency.php b/tests/fixtures/drupal/modules/phpstan_fixtures/src/UsesIncorrectCacheableDependency.php new file mode 100644 index 00000000..ce6d68e7 --- /dev/null +++ b/tests/fixtures/drupal/modules/phpstan_fixtures/src/UsesIncorrectCacheableDependency.php @@ -0,0 +1,15 @@ +addCacheableDependency($object); + } +} diff --git a/tests/src/Rules/CachableDependencyRuleTest.php b/tests/src/Rules/CachableDependencyRuleTest.php new file mode 100644 index 00000000..9e22fe71 --- /dev/null +++ b/tests/src/Rules/CachableDependencyRuleTest.php @@ -0,0 +1,40 @@ + $errorMessages + */ + public function testRule(string $path, array $errorMessages): void + { + $this->analyse([$path], $errorMessages); + } + + public static function resultData(): \Generator + { + yield [ + __DIR__ . '/../../fixtures/drupal/modules/phpstan_fixtures/src/UsesIncorrectCacheableDependency.php', + [ + [ + 'Calling addCacheableDependency($object) when $object does not implement CacheableDependencyInterface effectively disables caching and should be avoided.', + 07 + ], + ] + ]; + + } + + +} From 383c16deff7f14495b6bc037d677e9fdb5281089 Mon Sep 17 00:00:00 2001 From: Mariano D'Agostino Date: Mon, 1 Sep 2025 10:46:37 -0300 Subject: [PATCH 2/4] Fix coding standards --- src/Rules/Drupal/CacheableDependencyRule.php | 58 +++++++++----------- 1 file changed, 26 insertions(+), 32 deletions(-) diff --git a/src/Rules/Drupal/CacheableDependencyRule.php b/src/Rules/Drupal/CacheableDependencyRule.php index d0cbe9ee..c6c215aa 100644 --- a/src/Rules/Drupal/CacheableDependencyRule.php +++ b/src/Rules/Drupal/CacheableDependencyRule.php @@ -4,46 +4,40 @@ namespace mglaman\PHPStanDrupal\Rules\Drupal; -use PHPStan\Analyser\Scope; -use PHPStan\Rules\Rule; use PhpParser\Node; use PhpParser\Node\Expr\MethodCall; use PhpParser\Node\Identifier; +use PHPStan\Analyser\Scope; +use PHPStan\Rules\Rule; /** * @implements Rule */ -class CacheableDependencyRule implements Rule { - - /** - * {@inheritdoc} - */ - public function getNodeType(): string { - return MethodCall::class; - } - - /** - * {@inheritdoc} - */ - public function processNode(Node $node, Scope $scope): array { - if (!$node instanceof MethodCall || - !$node->name instanceof Identifier || - $node->name->toString() !== 'addCacheableDependency' - ) { - return []; - } - - $object = $scope->getType($node->args[0]->value); +class CacheableDependencyRule implements Rule +{ - // We need to check if isInstanceOf method exists as phpstan returns - // MixedType for unknown objects. - if (method_exists($object, 'isInstanceOf') && $object->isInstanceOf('Drupal\Core\Cache\CacheableDependencyInterface')) { - return []; + public function getNodeType(): string { + return MethodCall::class; } - return [ - 'Calling addCacheableDependency($object) when $object does not implement CacheableDependencyInterface effectively disables caching and should be avoided.', - ]; - } + public function processNode(Node $node, Scope $scope): array + { + if (!$node instanceof MethodCall || + !$node->name instanceof Identifier || + $node->name->toString() !== 'addCacheableDependency' + ) { + return []; + } + + $object = $scope->getType($node->args[0]->value); + + // We need to check if isInstanceOf method exists as phpstan returns + // MixedType for unknown objects. + if (method_exists($object, 'isInstanceOf') && $object->isInstanceOf('Drupal\Core\Cache\CacheableDependencyInterface')) { + return []; + } + return [ + 'Calling addCacheableDependency($object) when $object does not implement CacheableDependencyInterface effectively disables caching and should be avoided.', + ]; + } } - From a70ef94f20bae3d920ee3a8fb57fdb7c957cca2a Mon Sep 17 00:00:00 2001 From: Mariano D'Agostino Date: Mon, 1 Sep 2025 10:57:22 -0300 Subject: [PATCH 3/4] Fix coding standards --- src/Rules/Drupal/CacheableDependencyRule.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Rules/Drupal/CacheableDependencyRule.php b/src/Rules/Drupal/CacheableDependencyRule.php index c6c215aa..036c00c2 100644 --- a/src/Rules/Drupal/CacheableDependencyRule.php +++ b/src/Rules/Drupal/CacheableDependencyRule.php @@ -16,7 +16,8 @@ class CacheableDependencyRule implements Rule { - public function getNodeType(): string { + public function getNodeType(): string + { return MethodCall::class; } From 769af3a3d0e1bba3248f65cd0f1c279b0b64132f Mon Sep 17 00:00:00 2001 From: Mariano D'Agostino Date: Mon, 1 Sep 2025 11:27:05 -0300 Subject: [PATCH 4/4] Use nodeFinder --- src/Rules/Drupal/CacheableDependencyRule.php | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/Rules/Drupal/CacheableDependencyRule.php b/src/Rules/Drupal/CacheableDependencyRule.php index 036c00c2..0e968319 100644 --- a/src/Rules/Drupal/CacheableDependencyRule.php +++ b/src/Rules/Drupal/CacheableDependencyRule.php @@ -5,6 +5,7 @@ namespace mglaman\PHPStanDrupal\Rules\Drupal; use PhpParser\Node; +use PhpParser\NodeFinder; use PhpParser\Node\Expr\MethodCall; use PhpParser\Node\Identifier; use PHPStan\Analyser\Scope; @@ -18,19 +19,22 @@ class CacheableDependencyRule implements Rule public function getNodeType(): string { - return MethodCall::class; + return Node\Expr\MethodCall::class; } public function processNode(Node $node, Scope $scope): array { - if (!$node instanceof MethodCall || - !$node->name instanceof Identifier || - $node->name->toString() !== 'addCacheableDependency' - ) { + $nodeFinder = new NodeFinder(); + $method = $nodeFinder->findFirst($class->stmts, static function (Node $node) { + return $node instanceof Node\Stmt\ClassMethod && $node->name->toString() === 'addCacheableDependency'; + }); + if (!$method instanceof Node\Stmt\ClassMethod) { return []; } - $object = $scope->getType($node->args[0]->value); + $args = $node->getArgs(); + $traversableArg = $args[0]->value; + $object = $scope->getType($traversableArg); // We need to check if isInstanceOf method exists as phpstan returns // MixedType for unknown objects.