Skip to content

Commit a2ef88b

Browse files
mglamanclaude
mglaman
and
claude
authored
Add rules to detect missing CacheableMetadata parameter in entity operation methods and hooks (#979)
* Add rule to detect missing CacheableMetadata parameter in EntityListBuilder subclasses Closes #930 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Use version_compare for Drupal version gate Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Add rules for hook_entity_operation and hook_entity_operation_alter cacheability parameter Covers both OOP (#[Hook] attribute) and procedural hook implementations. Procedural rule uses version_compare rather than ReflectionProvider since api.php files are not loaded by default. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Use entityListBuilderOperationsCacheabilityRule flag for all three rules Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Fix tests to handle Drupal version matrix in CI The CI tests against Drupal ~11.2.0 where the version gate causes rules to return no errors. Make expected errors conditional on the running Drupal version so tests pass across the full version matrix. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Rename entityListBuilderOperationsCacheabilityRule to entityOperationsCacheabilityRule * Validate CacheableMetadata parameter type and nullability in entity operation rules * Include = NULL in suggested signatures for entity operation cacheability rules * phpcs * Refactor cacheability parameter validation to use PHPStan isNull() and simplify helper signature * phpcs * Extract shared ParamHelper for reusable param type validation Replace duplicated isValidCacheabilityParam() in three rules with a general-purpose ParamHelper::isValidParam() that accepts any expected FQCN and nullable constraint. Callers now own bounds checking via count(). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Remove Drupal 12 upper version gate from hook cacheability rules Hooks have no shared interface contract, so PHPStan native method-signature checking cannot catch missing cacheability params in Drupal 12+. Keep the rule firing for any Drupal >= 11.3. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 25e0bea commit a2ef88b

13 files changed

Lines changed: 603 additions & 0 deletions

‎bleedingEdge.neon‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,3 +13,4 @@ parameters:
1313
loggerFromFactoryPropertyAssignmentRule: true
1414
entityStorageDirectInjectionRule: true
1515
symfonyYamlParseRule: true
16+
entityOperationsCacheabilityRule: true

‎extension.neon‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ parameters:
3838
loggerFromFactoryPropertyAssignmentRule: false
3939
entityStorageDirectInjectionRule: false
4040
symfonyYamlParseRule: false
41+
entityOperationsCacheabilityRule: false
4142
entityMapping:
4243
aggregator_feed:
4344
class: Drupal\aggregator\Entity\Feed
@@ -277,6 +278,7 @@ parametersSchema:
277278
loggerFromFactoryPropertyAssignmentRule: boolean()
278279
entityStorageDirectInjectionRule: boolean()
279280
symfonyYamlParseRule: boolean()
281+
entityOperationsCacheabilityRule: boolean()
280282
])
281283
entityMapping: arrayOf(anyOf(
282284
structure([

‎rules.neon‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,12 @@ conditionalTags:
3636
phpstan.rules.rule: %drupal.rules.entityStorageDirectInjectionRule%
3737
mglaman\PHPStanDrupal\Rules\Drupal\SymfonyYamlParseRule:
3838
phpstan.rules.rule: %drupal.rules.symfonyYamlParseRule%
39+
mglaman\PHPStanDrupal\Rules\Drupal\EntityListBuilderOperationsCacheabilityRule:
40+
phpstan.rules.rule: %drupal.rules.entityOperationsCacheabilityRule%
41+
mglaman\PHPStanDrupal\Rules\Drupal\HookEntityOperationCacheabilityRule:
42+
phpstan.rules.rule: %drupal.rules.entityOperationsCacheabilityRule%
43+
mglaman\PHPStanDrupal\Rules\Drupal\ProceduralHookEntityOperationCacheabilityRule:
44+
phpstan.rules.rule: %drupal.rules.entityOperationsCacheabilityRule%
3945

4046
services:
4147
-
@@ -62,3 +68,9 @@ services:
6268
class: mglaman\PHPStanDrupal\Rules\Drupal\EntityStoragePropertyAssignmentRule
6369
-
6470
class: mglaman\PHPStanDrupal\Rules\Drupal\SymfonyYamlParseRule
71+
-
72+
class: mglaman\PHPStanDrupal\Rules\Drupal\EntityListBuilderOperationsCacheabilityRule
73+
-
74+
class: mglaman\PHPStanDrupal\Rules\Drupal\HookEntityOperationCacheabilityRule
75+
-
76+
class: mglaman\PHPStanDrupal\Rules\Drupal\ProceduralHookEntityOperationCacheabilityRule

‎src/Rules/Drupal/EntityListBuilderOperationsCacheabilityRule.php‎

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
<?php declare(strict_types=1);
2+
3+
namespace mglaman\PHPStanDrupal\Rules\Drupal;
4+
5+
use Drupal;
6+
use Drupal\Core\Entity\EntityListBuilder;
7+
use Drupal\Core\Entity\EntityListBuilderInterface;
8+
use PhpParser\Node;
9+
use PhpParser\Node\Stmt\ClassMethod;
10+
use PHPStan\Analyser\Scope;
11+
use PHPStan\Rules\Rule;
12+
use PHPStan\Rules\RuleErrorBuilder;
13+
use PHPStan\Type\ObjectType;
14+
use function count;
15+
use function in_array;
16+
use function version_compare;
17+
18+
/**
19+
* Detects EntityListBuilder subclasses overriding getOperations() or
20+
* getDefaultOperations() without the CacheableMetadata parameter added in
21+
* Drupal 11.3.
22+
*
23+
* The parameter was introduced as a deprecated workaround (via func_get_args)
24+
* in 11.3.0 and becomes a formal required-compatible parameter in 12.0.0.
25+
* This rule fires for Drupal 11.3.x only; for 12+ PHPStan's native
26+
* method-signature checking handles the incompatibility.
27+
*
28+
* @implements Rule<ClassMethod>
29+
*/
30+
class EntityListBuilderOperationsCacheabilityRule implements Rule
31+
{
32+
33+
private const METHODS = ['getOperations', 'getDefaultOperations'];
34+
35+
public function getNodeType(): string
36+
{
37+
return ClassMethod::class;
38+
}
39+
40+
public function processNode(Node $node, Scope $scope): array
41+
{
42+
// Version gate: only applies for Drupal 11.3.x.
43+
if (version_compare(Drupal::VERSION, '11.3', '<') || version_compare(Drupal::VERSION, '12.0', '>=')) {
44+
return [];
45+
}
46+
47+
if (!$scope->isInClass()) {
48+
return [];
49+
}
50+
51+
$methodName = $node->name->toString();
52+
if (!in_array($methodName, self::METHODS, true)) {
53+
return [];
54+
}
55+
56+
$classReflection = $scope->getClassReflection();
57+
58+
// Skip the base class itself — it uses func_get_args() intentionally.
59+
if ($classReflection->getName() === EntityListBuilder::class) {
60+
return [];
61+
}
62+
63+
$classType = new ObjectType($classReflection->getName());
64+
65+
// getDefaultOperations is protected and not part of the interface;
66+
// only flag it for subclasses of EntityListBuilder.
67+
$parentType = $methodName === 'getDefaultOperations'
68+
? new ObjectType(EntityListBuilder::class)
69+
: new ObjectType(EntityListBuilderInterface::class);
70+
71+
if (!$parentType->isSuperTypeOf($classType)->yes()) {
72+
return [];
73+
}
74+
75+
if (count($node->params) > 1 && ParamHelper::isValidParam($node->params[1], 'Drupal\Core\Cache\CacheableMetadata', true, $scope)) {
76+
return [];
77+
}
78+
79+
return [
80+
RuleErrorBuilder::message(
81+
sprintf(
82+
'Method %s::%s() is missing the CacheableMetadata parameter added in Drupal 11.3. Update the signature to: %s(\Drupal\Core\Entity\EntityInterface $entity, ?\Drupal\Core\Cache\CacheableMetadata $cacheability = NULL).',
83+
$classReflection->getName(),
84+
$methodName,
85+
$methodName,
86+
)
87+
)
88+
->tip('See https://www.drupal.org/node/3533080')
89+
->identifier('drupal.entityListBuilderMissingCacheabilityParameter')
90+
->build(),
91+
];
92+
}
93+
}

‎src/Rules/Drupal/HookEntityOperationCacheabilityRule.php‎

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
<?php declare(strict_types=1);
2+
3+
namespace mglaman\PHPStanDrupal\Rules\Drupal;
4+
5+
use Drupal;
6+
use Drupal\Core\Hook\Attribute\Hook;
7+
use PhpParser\Node;
8+
use PhpParser\Node\Stmt\ClassMethod;
9+
use PHPStan\Analyser\Scope;
10+
use PHPStan\Rules\Rule;
11+
use PHPStan\Rules\RuleErrorBuilder;
12+
use function class_exists;
13+
use function count;
14+
use function version_compare;
15+
16+
/**
17+
* Detects OOP hook implementations of hook_entity_operation and
18+
* hook_entity_operation_alter that are missing the CacheableMetadata
19+
* parameter added in Drupal 11.3.
20+
*
21+
* @implements Rule<ClassMethod>
22+
*/
23+
class HookEntityOperationCacheabilityRule implements Rule
24+
{
25+
26+
public function getNodeType(): string
27+
{
28+
return ClassMethod::class;
29+
}
30+
31+
public function processNode(Node $node, Scope $scope): array
32+
{
33+
if (!class_exists(Hook::class)) {
34+
return [];
35+
}
36+
37+
if (version_compare(Drupal::VERSION, '11.3', '<')) {
38+
return [];
39+
}
40+
41+
if (!$scope->isInClass()) {
42+
return [];
43+
}
44+
45+
$classReflection = $scope->getClassReflection();
46+
$nativeClass = $classReflection->getNativeReflection();
47+
$methodName = $node->name->toString();
48+
49+
if (!$nativeClass->hasMethod($methodName)) {
50+
return [];
51+
}
52+
53+
$nativeMethod = $nativeClass->getMethod($methodName);
54+
$hookAttributes = $nativeMethod->getAttributes(Hook::class);
55+
56+
if (count($hookAttributes) === 0) {
57+
return [];
58+
}
59+
60+
$errors = [];
61+
foreach ($hookAttributes as $hookAttribute) {
62+
/** @var Hook $hookInstance */
63+
$hookInstance = $hookAttribute->newInstance();
64+
65+
if ($hookInstance->hook === 'entity_operation') {
66+
if (count($node->params) < 2 || !ParamHelper::isValidParam($node->params[1], 'Drupal\Core\Cache\CacheableMetadata', true, $scope)) {
67+
$errors[] = RuleErrorBuilder::message(
68+
sprintf(
69+
'Method %s::%s() implements hook_entity_operation but is missing the CacheableMetadata parameter added in Drupal 11.3. Update the signature to include ?\Drupal\Core\Cache\CacheableMetadata $cacheability = NULL as the second parameter.',
70+
$classReflection->getName(),
71+
$methodName,
72+
)
73+
)
74+
->tip('See https://www.drupal.org/node/3533080')
75+
->identifier('drupal.hookEntityOperationMissingCacheabilityParameter')
76+
->build();
77+
}
78+
}
79+
80+
if ($hookInstance->hook === 'entity_operation_alter') {
81+
if (count($node->params) < 3 || !ParamHelper::isValidParam($node->params[2], 'Drupal\Core\Cache\CacheableMetadata', true, $scope)) {
82+
$errors[] = RuleErrorBuilder::message(
83+
sprintf(
84+
'Method %s::%s() implements hook_entity_operation_alter but is missing the CacheableMetadata parameter added in Drupal 11.3. Update the signature to include ?\Drupal\Core\Cache\CacheableMetadata $cacheability = NULL as the third parameter.',
85+
$classReflection->getName(),
86+
$methodName,
87+
)
88+
)
89+
->tip('See https://www.drupal.org/node/3533080')
90+
->identifier('drupal.hookEntityOperationAlterMissingCacheabilityParameter')
91+
->build();
92+
}
93+
}
94+
}
95+
96+
return $errors;
97+
}
98+
}

‎src/Rules/Drupal/ParamHelper.php‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
<?php declare(strict_types=1);
2+
3+
namespace mglaman\PHPStanDrupal\Rules\Drupal;
4+
5+
use PhpParser\Node;
6+
use PHPStan\Analyser\Scope;
7+
8+
final class ParamHelper
9+
{
10+
11+
public static function isValidParam(Node\Param $param, string $expectedFqcn, bool $isNullable, Scope $scope): bool
12+
{
13+
if ($param->type === null) {
14+
return false;
15+
}
16+
17+
$type = $param->type;
18+
$paramIsNullable = $type instanceof Node\NullableType;
19+
if ($paramIsNullable) {
20+
$type = $type->type;
21+
}
22+
23+
if (!($type instanceof Node\Name) || $scope->resolveName($type) !== $expectedFqcn) {
24+
return false;
25+
}
26+
27+
if (!$paramIsNullable && $param->default !== null) {
28+
$paramIsNullable = $scope->getType($param->default)->isNull()->yes();
29+
}
30+
31+
return !$isNullable || $paramIsNullable;
32+
}
33+
}

‎src/Rules/Drupal/ProceduralHookEntityOperationCacheabilityRule.php‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
<?php declare(strict_types=1);
2+
3+
namespace mglaman\PHPStanDrupal\Rules\Drupal;
4+
5+
use Drupal;
6+
use PhpParser\Node;
7+
use PhpParser\Node\Stmt\Function_;
8+
use PHPStan\Analyser\Scope;
9+
use PHPStan\Rules\Rule;
10+
use PHPStan\Rules\RuleErrorBuilder;
11+
use function basename;
12+
use function count;
13+
use function explode;
14+
use function str_ends_with;
15+
use function str_starts_with;
16+
use function strlen;
17+
use function substr_replace;
18+
use function version_compare;
19+
20+
/**
21+
* Detects procedural implementations of hook_entity_operation and
22+
* hook_entity_operation_alter that are missing the CacheableMetadata
23+
* parameter added in Drupal 11.3.
24+
*
25+
* @implements Rule<Function_>
26+
*/
27+
class ProceduralHookEntityOperationCacheabilityRule implements Rule
28+
{
29+
30+
public function getNodeType(): string
31+
{
32+
return Function_::class;
33+
}
34+
35+
public function processNode(Node $node, Scope $scope): array
36+
{
37+
if (!str_ends_with($scope->getFile(), '.module') && !str_ends_with($scope->getFile(), '.inc')) {
38+
return [];
39+
}
40+
41+
if (version_compare(Drupal::VERSION, '11.3', '<')) {
42+
return [];
43+
}
44+
45+
$moduleName = explode('.', basename($scope->getFile()))[0];
46+
$functionName = $node->name->toString();
47+
48+
if (!str_starts_with($functionName, "{$moduleName}_")) {
49+
return [];
50+
}
51+
52+
$hookName = substr_replace($functionName, 'hook', 0, strlen($moduleName));
53+
54+
if ($hookName === 'hook_entity_operation') {
55+
if (count($node->params) < 2 || !ParamHelper::isValidParam($node->params[1], 'Drupal\Core\Cache\CacheableMetadata', true, $scope)) {
56+
return [
57+
RuleErrorBuilder::message(
58+
sprintf(
59+
'Function %s() implements hook_entity_operation but is missing the CacheableMetadata parameter added in Drupal 11.3. Update the signature to: %s(\Drupal\Core\Entity\EntityInterface $entity, ?\Drupal\Core\Cache\CacheableMetadata $cacheability = NULL).',
60+
$functionName,
61+
$functionName,
62+
)
63+
)
64+
->tip('See https://www.drupal.org/node/3533080')
65+
->identifier('drupal.proceduralHookEntityOperationMissingCacheabilityParameter')
66+
->build(),
67+
];
68+
}
69+
}
70+
71+
if ($hookName === 'hook_entity_operation_alter') {
72+
if (count($node->params) < 3 || !ParamHelper::isValidParam($node->params[2], 'Drupal\Core\Cache\CacheableMetadata', true, $scope)) {
73+
return [
74+
RuleErrorBuilder::message(
75+
sprintf(
76+
'Function %s() implements hook_entity_operation_alter but is missing the CacheableMetadata parameter added in Drupal 11.3. Update the signature to: %s(array &$operations, \Drupal\Core\Entity\EntityInterface $entity, ?\Drupal\Core\Cache\CacheableMetadata $cacheability = NULL).',
77+
$functionName,
78+
$functionName,
79+
)
80+
)
81+
->tip('See https://www.drupal.org/node/3533080')
82+
->identifier('drupal.proceduralHookEntityOperationAlterMissingCacheabilityParameter')
83+
->build(),
84+
];
85+
}
86+
}
87+
88+
return [];
89+
}
90+
}

0 commit comments

Comments
 (0)