diff --git a/bleedingEdge.neon b/bleedingEdge.neon index 4690264c..9d0c6cfb 100644 --- a/bleedingEdge.neon +++ b/bleedingEdge.neon @@ -8,3 +8,4 @@ parameters: testClassSuffixNameRule: true dependencySerializationTraitPropertyRule: true accessResultConditionRule: true + cacheableDependencyRule: true diff --git a/extension.neon b/extension.neon index 8cd6318a..6a2e15c4 100644 --- a/extension.neon +++ b/extension.neon @@ -33,6 +33,7 @@ parameters: accessResultConditionRule: false classExtendsInternalClassRule: true pluginManagerInspectionRule: false + cacheableDependencyRule: false entityMapping: aggregator_feed: class: Drupal\aggregator\Entity\Feed @@ -267,6 +268,7 @@ parametersSchema: accessResultConditionRule: boolean() classExtendsInternalClassRule: boolean() pluginManagerInspectionRule: boolean() + cacheableDependencyRule: boolean() ]) entityMapping: arrayOf(anyOf( structure([ diff --git a/rules.neon b/rules.neon index 34f4b3b6..d435567a 100644 --- a/rules.neon +++ b/rules.neon @@ -24,6 +24,8 @@ conditionalTags: phpstan.rules.rule: %drupal.rules.classExtendsInternalClassRule% mglaman\PHPStanDrupal\Rules\Classes\PluginManagerInspectionRule: phpstan.rules.rule: %drupal.rules.pluginManagerInspectionRule% + mglaman\PHPStanDrupal\Rules\Drupal\CacheableDependencyRule: + phpstan.rules.rule: %drupal.rules.cacheableDependencyRule% services: - @@ -38,3 +40,5 @@ services: class: mglaman\PHPStanDrupal\Rules\Classes\ClassExtendsInternalClassRule - class: mglaman\PHPStanDrupal\Rules\Classes\PluginManagerInspectionRule + - + class: mglaman\PHPStanDrupal\Rules\Drupal\CacheableDependencyRule diff --git a/src/Rules/Drupal/CacheableDependencyRule.php b/src/Rules/Drupal/CacheableDependencyRule.php new file mode 100644 index 00000000..74e711bd --- /dev/null +++ b/src/Rules/Drupal/CacheableDependencyRule.php @@ -0,0 +1,79 @@ + + */ +class CacheableDependencyRule implements Rule +{ + + public function getNodeType(): string + { + return Node\Expr\MethodCall::class; + } + + public function processNode(Node $node, Scope $scope): array + { + if (!$node->name instanceof Identifier || $node->name->toString() !== 'addCacheableDependency') { + return []; + } + + $receiverType = $scope->getType($node->var); + + $allowedInterfaces = [ + RefinableCacheableDependencyInterface::class => 0, + CacheableResponseInterface::class => 0, + ContextInterface::class => 0, + RendererInterface::class => 1, + ]; + + $argumentIndex = null; + foreach ($allowedInterfaces as $interfaceName => $argPosition) { + $interfaceType = new ObjectType($interfaceName); + if ($interfaceType->isSuperTypeOf($receiverType)->yes()) { + $argumentIndex = $argPosition; + break; + } + } + + if ($argumentIndex === null) { + return []; + } + + $args = $node->getArgs(); + if (count($args) <= $argumentIndex) { + return []; + } + + $dependencyArg = $args[$argumentIndex]->value; + $object = $scope->getType($dependencyArg); + + $interfaceType = new ObjectType(CacheableDependencyInterface::class); + $implementsInterface = $interfaceType->isSuperTypeOf($object); + + if (!$implementsInterface->no()) { + return []; + } + + return [ + RuleErrorBuilder::message('Calling addCacheableDependency($object) when $object does not implement CacheableDependencyInterface effectively disables caching and should be avoided.') + ->identifier('cacheable.dependency') + ->build(), + ]; + } +} diff --git a/tests/src/Rules/CachableDependencyRuleTest.php b/tests/src/Rules/CachableDependencyRuleTest.php new file mode 100644 index 00000000..d4fef310 --- /dev/null +++ b/tests/src/Rules/CachableDependencyRuleTest.php @@ -0,0 +1,51 @@ + $errorMessages + */ + public function testRule(string $path, array $errorMessages): void + { + $this->analyse([$path], $errorMessages); + } + + public static function resultData(): \Generator + { + yield 'all test cases' => [ + __DIR__ . '/data/cacheable-dependency.php', + [ + [ + 'Calling addCacheableDependency($object) when $object does not implement CacheableDependencyInterface effectively disables caching and should be avoided.', + 13 + ], + [ + 'Calling addCacheableDependency($object) when $object does not implement CacheableDependencyInterface effectively disables caching and should be avoided.', + 36 + ], + [ + 'Calling addCacheableDependency($object) when $object does not implement CacheableDependencyInterface effectively disables caching and should be avoided.', + 39 + ], + [ + 'Calling addCacheableDependency($object) when $object does not implement CacheableDependencyInterface effectively disables caching and should be avoided.', + 55 + ], + ] + ]; + } + + +} diff --git a/tests/src/Rules/data/cacheable-dependency.php b/tests/src/Rules/data/cacheable-dependency.php new file mode 100644 index 00000000..5964dc0b --- /dev/null +++ b/tests/src/Rules/data/cacheable-dependency.php @@ -0,0 +1,59 @@ +addCacheableDependency($object); + } +} + +class UsesCorrectCacheableDependency { + public function test() { + $element = []; + $cacheable_metadata = CacheableMetadata::createFromRenderArray($element); + + $correct_dependency = new CacheableMetadata(); + $cacheable_metadata->addCacheableDependency($correct_dependency); + } +} + +class MultipleCacheableDependencyCalls { + public function test() { + $element = []; + $cacheable_metadata = CacheableMetadata::createFromRenderArray($element); + + $correct_dependency = new CacheableMetadata(); + $cacheable_metadata->addCacheableDependency($correct_dependency); + + $object = new \StdClass; + $cacheable_metadata->addCacheableDependency($object); + + $another_object = new \DateTime(); + $cacheable_metadata->addCacheableDependency($another_object); + } +} + +class RendererInterfaceTestCase { + public function testCorrectUsage(\Drupal\Core\Render\RendererInterface $renderer) { + $elements = []; + + $correct_dependency = new CacheableMetadata(); + $renderer->addCacheableDependency($elements, $correct_dependency); + } + + public function testIncorrectUsage(\Drupal\Core\Render\RendererInterface $renderer) { + $elements = []; + + $object = new \StdClass; + $renderer->addCacheableDependency($elements, $object); + } +} + +