Skip to content

Commit 2574aac

Browse files
mglamanmglaman
authored
HookFormAlterRule for hook_form_alter (#921)
* HookFormAlterRule for hook_form_alter fixes #920 * drop some lint versions * Skip HookFormAlterRule on < 11.1
1 parent 1572c47 commit 2574aac

File tree

9 files changed

+383
-3
lines changed

9 files changed

+383
-3
lines changed

‎.github/workflows/php.yml‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,6 @@ jobs:
1414
strategy:
1515
matrix:
1616
php-version:
17-
- "8.1"
18-
- "8.2"
1917
- "8.3"
2018
- "8.4"
2119
steps:

‎bleedingEdge.neon‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,3 +9,4 @@ parameters:
99
dependencySerializationTraitPropertyRule: true
1010
accessResultConditionRule: true
1111
cacheableDependencyRule: true
12+
hookRules: true

‎composer.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
"require-dev": {
2121
"behat/mink": "^1.10",
2222
"composer/installers": "^1.9 || ^2",
23-
"drupal/core-recommended": "^10",
23+
"drupal/core-recommended": "^11",
2424
"drush/drush": "^11 || ^12 || ^13",
2525
"phpstan/extension-installer": "^1.4.3",
2626
"phpstan/phpstan-strict-rules": "^2.0",

‎extension.neon‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ parameters:
3434
classExtendsInternalClassRule: true
3535
pluginManagerInspectionRule: false
3636
cacheableDependencyRule: false
37+
hookRules: false
3738
entityMapping:
3839
aggregator_feed:
3940
class: Drupal\aggregator\Entity\Feed
@@ -269,6 +270,7 @@ parametersSchema:
269270
classExtendsInternalClassRule: boolean()
270271
pluginManagerInspectionRule: boolean()
271272
cacheableDependencyRule: boolean()
273+
hookRules: boolean()
272274
])
273275
entityMapping: arrayOf(anyOf(
274276
structure([

‎rules.neon‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ rules:
1414
- mglaman\PHPStanDrupal\Rules\Drupal\TestClassesProtectedPropertyModulesRule
1515

1616
conditionalTags:
17+
mglaman\PHPStanDrupal\Rules\Drupal\HookFormAlterRule:
18+
phpstan.rules.rule: %drupal.rules.hookRules%
1719
mglaman\PHPStanDrupal\Rules\Drupal\Tests\TestClassSuffixNameRule:
1820
phpstan.rules.rule: %drupal.rules.testClassSuffixNameRule%
1921
mglaman\PHPStanDrupal\Rules\Drupal\DependencySerializationTraitPropertyRule:
@@ -28,6 +30,8 @@ conditionalTags:
2830
phpstan.rules.rule: %drupal.rules.cacheableDependencyRule%
2931

3032
services:
33+
-
34+
class: mglaman\PHPStanDrupal\Rules\Drupal\HookFormAlterRule
3135
-
3236
class: mglaman\PHPStanDrupal\Rules\Drupal\Tests\TestClassSuffixNameRule
3337
-

‎src/Rules/Drupal/HookFormAlterRule.php‎

Lines changed: 195 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,195 @@
1+
<?php declare(strict_types=1);
2+
3+
namespace mglaman\PHPStanDrupal\Rules\Drupal;
4+
5+
use Drupal\Core\Form\FormStateInterface;
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 PHPStan\Type\ObjectType;
13+
use PHPStan\Type\VerbosityLevel;
14+
use function class_exists;
15+
use function count;
16+
use function preg_match;
17+
18+
/**
19+
* Rule to validate form alter hook implementations using the Hook attribute.
20+
*
21+
* Validates that OOP hooks implementing form_alter, form_FORM_ID_alter, and
22+
* form_BASE_FORM_ID_alter have the correct method signatures.
23+
*
24+
* @implements Rule<ClassMethod>
25+
*/
26+
class HookFormAlterRule implements Rule
27+
{
28+
29+
/**
30+
* Pattern matching form alter hook names.
31+
*/
32+
private const FORM_ALTER_PATTERNS = [
33+
'/^form_alter$/',
34+
'/^form_[a-zA-Z0-9_]+_alter$/',
35+
];
36+
37+
38+
public function getNodeType(): string
39+
{
40+
return ClassMethod::class;
41+
}
42+
43+
public function processNode(Node $node, Scope $scope): array
44+
{
45+
// Skip if Hook attribute class doesn't exist (older Drupal versions)
46+
if (!class_exists(Hook::class)) {
47+
return [];
48+
}
49+
50+
return $this->processMethod($node, $scope);
51+
}
52+
53+
/**
54+
* Process a method with Hook attribute.
55+
*
56+
* @return list<\PHPStan\Rules\IdentifierRuleError>
57+
*/
58+
private function processMethod(ClassMethod $method, Scope $scope): array
59+
{
60+
if (!$scope->isInClass()) {
61+
return [];
62+
}
63+
64+
$classReflection = $scope->getClassReflection();
65+
$nativeClass = $classReflection->getNativeReflection();
66+
67+
if (!$nativeClass->hasMethod($method->name->toString())) {
68+
return [];
69+
}
70+
71+
$nativeMethod = $nativeClass->getMethod($method->name->toString());
72+
$hookAttributes = $nativeMethod->getAttributes(Hook::class);
73+
74+
if (count($hookAttributes) === 0) {
75+
return [];
76+
}
77+
78+
$errors = [];
79+
foreach ($hookAttributes as $hookAttribute) {
80+
/** @var Hook $hookInstance */
81+
$hookInstance = $hookAttribute->newInstance();
82+
if ($this->isFormAlterHook($hookInstance->hook)) {
83+
$methodErrors = $this->validateMethodSignature($method, $scope, $hookInstance->hook);
84+
$errors[] = $methodErrors;
85+
}
86+
}
87+
88+
return array_merge(...$errors);
89+
}
90+
91+
92+
/**
93+
* Check if a hook name matches form alter patterns.
94+
*/
95+
private function isFormAlterHook(string $hookName): bool
96+
{
97+
foreach (self::FORM_ALTER_PATTERNS as $pattern) {
98+
if (preg_match($pattern, $hookName) === 1) {
99+
return true;
100+
}
101+
}
102+
return false;
103+
}
104+
105+
/**
106+
* Validate method signature for method-targeted hooks.
107+
*
108+
* @return list<\PHPStan\Rules\IdentifierRuleError>
109+
*/
110+
private function validateMethodSignature(ClassMethod $method, Scope $scope, string $hookName): array
111+
{
112+
$expectedSignature = 'Expected signature: method(&$form, \Drupal\Core\Form\FormStateInterface $form_state[, $form_id])';
113+
$paramCount = count($method->params);
114+
115+
if ($paramCount < 2 || $paramCount > 3) {
116+
return [
117+
RuleErrorBuilder::message(
118+
sprintf('Form alter hook "%s" implementation must have 2 or 3 parameters. %s', $hookName, $expectedSignature)
119+
)
120+
->line($method->getStartLine())
121+
->identifier('hookFormAlter.invalidParameterCount')
122+
->build()
123+
];
124+
}
125+
126+
$errors = [];
127+
128+
// Validate first parameter (form array by reference)
129+
$formParam = $method->params[0];
130+
if (!$formParam->byRef) {
131+
$errors[] = RuleErrorBuilder::message(
132+
sprintf('Form alter hook "%s" first parameter must be passed by reference (&$form). %s', $hookName, $expectedSignature)
133+
)
134+
->line($method->getStartLine())
135+
->identifier('hookFormAlter.formParameterNotByRef')
136+
->build();
137+
}
138+
139+
// Validate parameter types if type hints are present
140+
if ($formParam->type !== null) {
141+
$formParamType = $scope->getFunctionType($formParam->type, false, false);
142+
if (!$formParamType->isArray()->yes()) {
143+
$errors[] = RuleErrorBuilder::message(
144+
sprintf('Form alter hook "%s" first parameter should be an array. %s', $hookName, $expectedSignature)
145+
)
146+
->line($method->getStartLine())
147+
->identifier('hookFormAlter.invalidFormParameterType')
148+
->build();
149+
}
150+
}
151+
152+
// Validate second parameter (FormStateInterface)
153+
$formStateParam = $method->params[1];
154+
if ($formStateParam->type !== null) {
155+
$formStateType = $scope->getFunctionType($formStateParam->type, false, false);
156+
$expectedFormStateType = new ObjectType(FormStateInterface::class);
157+
if (!$expectedFormStateType->isSuperTypeOf($formStateType)->yes()) {
158+
$errors[] = RuleErrorBuilder::message(
159+
sprintf(
160+
'Form alter hook "%s" second parameter should be \Drupal\Core\Form\FormStateInterface, %s given. %s',
161+
$hookName,
162+
$formStateType->describe(VerbosityLevel::typeOnly()),
163+
$expectedSignature
164+
)
165+
)
166+
->line($method->getStartLine())
167+
->identifier('hookFormAlter.invalidFormStateParameterType')
168+
->build();
169+
}
170+
}
171+
172+
// Validate third parameter (string form_id) - only if present
173+
if ($paramCount === 3) {
174+
$formIdParam = $method->params[2];
175+
if ($formIdParam->type !== null) {
176+
$formIdType = $scope->getFunctionType($formIdParam->type, false, false);
177+
if (!$formIdType->isString()->yes()) {
178+
$errors[] = RuleErrorBuilder::message(
179+
sprintf(
180+
'Form alter hook "%s" third parameter should be string, %s given. %s',
181+
$hookName,
182+
$formIdType->describe(VerbosityLevel::typeOnly()),
183+
$expectedSignature
184+
)
185+
)
186+
->line($method->getStartLine())
187+
->identifier('hookFormAlter.invalidFormIdParameterType')
188+
->build();
189+
}
190+
}
191+
}
192+
193+
return $errors;
194+
}
195+
}

‎tests/src/Rules/HookFormAlterRuleTest.php‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
<?php
2+
3+
namespace mglaman\PHPStanDrupal\Tests\Rules;
4+
5+
use Drupal;
6+
use mglaman\PHPStanDrupal\Rules\Drupal\HookFormAlterRule;
7+
use mglaman\PHPStanDrupal\Tests\DrupalRuleTestCase;
8+
use PHPStan\Rules\Rule;
9+
10+
/**
11+
* Test the HookFormAlterRule.
12+
*/
13+
class HookFormAlterRuleTest extends DrupalRuleTestCase {
14+
15+
/**
16+
* {@inheritdoc}
17+
*/
18+
protected function getRule(): Rule {
19+
return new HookFormAlterRule();
20+
}
21+
22+
/**
23+
* Test valid form alter hook implementations.
24+
*/
25+
public function testValidImplementations(): void {
26+
if (version_compare(Drupal::VERSION, '11.1', '<')) {
27+
self::markTestSkipped();
28+
}
29+
$this->analyse([
30+
__DIR__ . '/data/hook-form-alter-valid.php',
31+
], [
32+
// No errors expected for valid implementations
33+
]);
34+
}
35+
36+
/**
37+
* Test invalid method-based hook implementations.
38+
*/
39+
public function testInvalidMethodImplementations(): void {
40+
if (version_compare(Drupal::VERSION, '11.1', '<')) {
41+
self::markTestSkipped();
42+
}
43+
$this->analyse([
44+
__DIR__ . '/data/hook-form-alter-invalid.php',
45+
], [
46+
[
47+
'Form alter hook "form_alter" implementation must have 2 or 3 parameters. Expected signature: method(&$form, \Drupal\Core\Form\FormStateInterface $form_state[, $form_id])',
48+
16,
49+
],
50+
[
51+
'Form alter hook "form_alter" first parameter must be passed by reference (&$form). Expected signature: method(&$form, \Drupal\Core\Form\FormStateInterface $form_state[, $form_id])',
52+
24,
53+
],
54+
[
55+
'Form alter hook "form_alter" second parameter should be \Drupal\Core\Form\FormStateInterface, string given. Expected signature: method(&$form, \Drupal\Core\Form\FormStateInterface $form_state[, $form_id])',
56+
32,
57+
],
58+
[
59+
'Form alter hook "form_node_form_alter" third parameter should be string, int given. Expected signature: method(&$form, \Drupal\Core\Form\FormStateInterface $form_state[, $form_id])',
60+
40,
61+
],
62+
[
63+
'Form alter hook "form_alter" implementation must have 2 or 3 parameters. Expected signature: method(&$form, \Drupal\Core\Form\FormStateInterface $form_state[, $form_id])',
64+
48,
65+
],
66+
[
67+
'Form alter hook "form_alter" first parameter should be an array. Expected signature: method(&$form, \Drupal\Core\Form\FormStateInterface $form_state[, $form_id])',
68+
56,
69+
],
70+
]);
71+
}
72+
73+
74+
}

‎tests/src/Rules/data/hook-form-alter-invalid.php‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
<?php
2+
3+
namespace mglaman\PHPStanDrupal\Tests\Rules\data;
4+
5+
use Drupal\Core\Form\FormStateInterface;
6+
use Drupal\Core\Hook\Attribute\Hook;
7+
8+
/**
9+
* Invalid form alter hook implementations for testing.
10+
*/
11+
class InvalidFormAlterHooks {
12+
13+
/**
14+
* Too few parameters (missing $form_state).
15+
*/
16+
#[Hook('form_alter')]
17+
public function tooFewParameters(array &$form): void {
18+
// Invalid: missing $form_state parameter
19+
}
20+
21+
/**
22+
* First parameter not by reference.
23+
*/
24+
#[Hook('form_alter')]
25+
public function formNotByReference(array $form, FormStateInterface $form_state, string $form_id): void {
26+
// Invalid: $form not by reference
27+
}
28+
29+
/**
30+
* Wrong type for second parameter.
31+
*/
32+
#[Hook('form_alter')]
33+
public function wrongFormStateType(array &$form, string $form_state, string $form_id): void {
34+
// Invalid: $form_state should be FormStateInterface
35+
}
36+
37+
/**
38+
* Wrong type for third parameter.
39+
*/
40+
#[Hook('form_node_form_alter')]
41+
public function wrongFormIdType(array &$form, FormStateInterface $form_state, int $form_id): void {
42+
// Invalid: $form_id should be string
43+
}
44+
45+
/**
46+
* Too many parameters.
47+
*/
48+
#[Hook('form_alter')]
49+
public function tooManyParameters(array &$form, FormStateInterface $form_state, string $form_id, string $extra): void {
50+
// Invalid: too many parameters
51+
}
52+
53+
/**
54+
* Wrong array type hint.
55+
*/
56+
#[Hook('form_alter')]
57+
public function wrongFormType(string &$form, FormStateInterface $form_state, string $form_id): void {
58+
// Invalid: $form should be array
59+
}
60+
61+
}

0 commit comments

Comments
 (0)