HookFormAlterRule for hook_form_alter

.github/workflows/php.yml

@@ -14,8 +14,6 @@ jobs:
     strategy:
       matrix:
         php-version:
-          - "8.1"
-          - "8.2"
           - "8.3"
           - "8.4"
     steps:

bleedingEdge.neon

@@ -9,3 +9,4 @@ parameters:
 			dependencySerializationTraitPropertyRule: true
 			accessResultConditionRule: true
 			cacheableDependencyRule: true
+			hookRules: true

composer.json

@@ -20,7 +20,7 @@
     "require-dev": {
         "behat/mink": "^1.10",
         "composer/installers": "^1.9 || ^2",
-        "drupal/core-recommended": "^10",
+        "drupal/core-recommended": "^11",
         "drush/drush": "^11 || ^12 || ^13",
         "phpstan/extension-installer": "^1.4.3",
         "phpstan/phpstan-strict-rules": "^2.0",

extension.neon

@@ -34,6 +34,7 @@ parameters:
 			classExtendsInternalClassRule: true
 			pluginManagerInspectionRule: false
 			cacheableDependencyRule: false
+			hookRules: false
 		entityMapping:
 			aggregator_feed:
 				class: Drupal\aggregator\Entity\Feed
@@ -269,6 +270,7 @@ parametersSchema:
 			classExtendsInternalClassRule: boolean()
 			pluginManagerInspectionRule: boolean()
 			cacheableDependencyRule: boolean()
+			hookRules: boolean()
 		])
 		entityMapping: arrayOf(anyOf(
 			structure([

rules.neon

@@ -14,6 +14,8 @@ rules:
 	- mglaman\PHPStanDrupal\Rules\Drupal\TestClassesProtectedPropertyModulesRule
 
 conditionalTags:
+	mglaman\PHPStanDrupal\Rules\Drupal\HookFormAlterRule:
+		phpstan.rules.rule: %drupal.rules.hookRules%
 	mglaman\PHPStanDrupal\Rules\Drupal\Tests\TestClassSuffixNameRule:
 		phpstan.rules.rule: %drupal.rules.testClassSuffixNameRule%
 	mglaman\PHPStanDrupal\Rules\Drupal\DependencySerializationTraitPropertyRule:
@@ -28,6 +30,8 @@ conditionalTags:
 		phpstan.rules.rule: %drupal.rules.cacheableDependencyRule%
 
 services:
+	-
+		class: mglaman\PHPStanDrupal\Rules\Drupal\HookFormAlterRule
 	-
 		class: mglaman\PHPStanDrupal\Rules\Drupal\Tests\TestClassSuffixNameRule
 	-

src/Rules/Drupal/HookFormAlterRule.php

@@ -0,0 +1,195 @@
+<?php declare(strict_types=1);
+
+namespace mglaman\PHPStanDrupal\Rules\Drupal;
+
+use Drupal\Core\Form\FormStateInterface;
+use Drupal\Core\Hook\Attribute\Hook;
+use PhpParser\Node;
+use PhpParser\Node\Stmt\ClassMethod;
+use PHPStan\Analyser\Scope;
+use PHPStan\Rules\Rule;
+use PHPStan\Rules\RuleErrorBuilder;
+use PHPStan\Type\ObjectType;
+use PHPStan\Type\VerbosityLevel;
+use function class_exists;
+use function count;
+use function preg_match;
+
+/**
+ * Rule to validate form alter hook implementations using the Hook attribute.
+ *
+ * Validates that OOP hooks implementing form_alter, form_FORM_ID_alter, and
+ * form_BASE_FORM_ID_alter have the correct method signatures.
+ *
+ * @implements Rule<ClassMethod>
+ */
+class HookFormAlterRule implements Rule
+{
+
+    /**
+     * Pattern matching form alter hook names.
+     */
+    private const FORM_ALTER_PATTERNS = [
+        '/^form_alter$/',
+        '/^form_[a-zA-Z0-9_]+_alter$/',
+    ];
+
+
+    public function getNodeType(): string
+    {
+        return ClassMethod::class;
+    }
+
+    public function processNode(Node $node, Scope $scope): array
+    {
+        // Skip if Hook attribute class doesn't exist (older Drupal versions)
+        if (!class_exists(Hook::class)) {
+            return [];
+        }
+
+        return $this->processMethod($node, $scope);
+    }
+
+    /**
+     * Process a method with Hook attribute.
+     *
+     * @return list<\PHPStan\Rules\IdentifierRuleError>
+     */
+    private function processMethod(ClassMethod $method, Scope $scope): array
+    {
+        if (!$scope->isInClass()) {
+            return [];
+        }
+
+        $classReflection = $scope->getClassReflection();
+        $nativeClass = $classReflection->getNativeReflection();
+
+        if (!$nativeClass->hasMethod($method->name->toString())) {
+            return [];
+        }
+
+        $nativeMethod = $nativeClass->getMethod($method->name->toString());
+        $hookAttributes = $nativeMethod->getAttributes(Hook::class);
+
+        if (count($hookAttributes) === 0) {
+            return [];
+        }
+
+        $errors = [];
+        foreach ($hookAttributes as $hookAttribute) {
+            /** @var Hook $hookInstance */
+            $hookInstance = $hookAttribute->newInstance();
+            if ($this->isFormAlterHook($hookInstance->hook)) {
+                $methodErrors = $this->validateMethodSignature($method, $scope, $hookInstance->hook);
+                $errors[] = $methodErrors;
+            }
+        }
+
+        return array_merge(...$errors);
+    }
+
+
+    /**
+     * Check if a hook name matches form alter patterns.
+     */
+    private function isFormAlterHook(string $hookName): bool
+    {
+        foreach (self::FORM_ALTER_PATTERNS as $pattern) {
+            if (preg_match($pattern, $hookName) === 1) {
+                return true;
+            }
+        }
+        return false;
+    }
+
+    /**
+     * Validate method signature for method-targeted hooks.
+     *
+     * @return list<\PHPStan\Rules\IdentifierRuleError>
+     */
+    private function validateMethodSignature(ClassMethod $method, Scope $scope, string $hookName): array
+    {
+        $expectedSignature = 'Expected signature: method(&$form, \Drupal\Core\Form\FormStateInterface $form_state[, $form_id])';
+        $paramCount = count($method->params);
+
+        if ($paramCount < 2 || $paramCount > 3) {
+            return [
+                RuleErrorBuilder::message(
+                    sprintf('Form alter hook "%s" implementation must have 2 or 3 parameters. %s', $hookName, $expectedSignature)
+                )
+                ->line($method->getStartLine())
+                ->identifier('hookFormAlter.invalidParameterCount')
+                ->build()
+            ];
+        }
+
+        $errors = [];
+
+        // Validate first parameter (form array by reference)
+        $formParam = $method->params[0];
+        if (!$formParam->byRef) {
+            $errors[] = RuleErrorBuilder::message(
+                sprintf('Form alter hook "%s" first parameter must be passed by reference (&$form). %s', $hookName, $expectedSignature)
+            )
+            ->line($method->getStartLine())
+            ->identifier('hookFormAlter.formParameterNotByRef')
+            ->build();
+        }
+
+        // Validate parameter types if type hints are present
+        if ($formParam->type !== null) {
+            $formParamType = $scope->getFunctionType($formParam->type, false, false);
+            if (!$formParamType->isArray()->yes()) {
+                $errors[] = RuleErrorBuilder::message(
+                    sprintf('Form alter hook "%s" first parameter should be an array. %s', $hookName, $expectedSignature)
+                )
+                ->line($method->getStartLine())
+                ->identifier('hookFormAlter.invalidFormParameterType')
+                ->build();
+            }
+        }
+
+        // Validate second parameter (FormStateInterface)
+        $formStateParam = $method->params[1];
+        if ($formStateParam->type !== null) {
+            $formStateType = $scope->getFunctionType($formStateParam->type, false, false);
+            $expectedFormStateType = new ObjectType(FormStateInterface::class);
+            if (!$expectedFormStateType->isSuperTypeOf($formStateType)->yes()) {
+                $errors[] = RuleErrorBuilder::message(
+                    sprintf(
+                        'Form alter hook "%s" second parameter should be \Drupal\Core\Form\FormStateInterface, %s given. %s',
+                        $hookName,
+                        $formStateType->describe(VerbosityLevel::typeOnly()),
+                        $expectedSignature
+                    )
+                )
+                ->line($method->getStartLine())
+                ->identifier('hookFormAlter.invalidFormStateParameterType')
+                ->build();
+            }
+        }
+
+        // Validate third parameter (string form_id) - only if present
+        if ($paramCount === 3) {
+            $formIdParam = $method->params[2];
+            if ($formIdParam->type !== null) {
+                $formIdType = $scope->getFunctionType($formIdParam->type, false, false);
+                if (!$formIdType->isString()->yes()) {
+                    $errors[] = RuleErrorBuilder::message(
+                        sprintf(
+                            'Form alter hook "%s" third parameter should be string, %s given. %s',
+                            $hookName,
+                            $formIdType->describe(VerbosityLevel::typeOnly()),
+                            $expectedSignature
+                        )
+                    )
+                    ->line($method->getStartLine())
+                    ->identifier('hookFormAlter.invalidFormIdParameterType')
+                    ->build();
+                }
+            }
+        }
+
+        return $errors;
+    }
+}

tests/src/Rules/HookFormAlterRuleTest.php

@@ -0,0 +1,74 @@
+<?php
+
+namespace mglaman\PHPStanDrupal\Tests\Rules;
+
+use Drupal;
+use mglaman\PHPStanDrupal\Rules\Drupal\HookFormAlterRule;
+use mglaman\PHPStanDrupal\Tests\DrupalRuleTestCase;
+use PHPStan\Rules\Rule;
+
+/**
+ * Test the HookFormAlterRule.
+ */
+class HookFormAlterRuleTest extends DrupalRuleTestCase {
+
+    /**
+     * {@inheritdoc}
+     */
+    protected function getRule(): Rule {
+        return new HookFormAlterRule();
+    }
+
+    /**
+     * Test valid form alter hook implementations.
+     */
+    public function testValidImplementations(): void {
+        if (version_compare(Drupal::VERSION, '11.1', '<')) {
+            self::markTestSkipped();
+        }
+        $this->analyse([
+            __DIR__ . '/data/hook-form-alter-valid.php',
+        ], [
+            // No errors expected for valid implementations
+        ]);
+    }
+
+    /**
+     * Test invalid method-based hook implementations.
+     */
+    public function testInvalidMethodImplementations(): void {
+        if (version_compare(Drupal::VERSION, '11.1', '<')) {
+            self::markTestSkipped();
+        }
+        $this->analyse([
+            __DIR__ . '/data/hook-form-alter-invalid.php',
+        ], [
+            [
+                'Form alter hook "form_alter" implementation must have 2 or 3 parameters. Expected signature: method(&$form, \Drupal\Core\Form\FormStateInterface $form_state[, $form_id])',
+                16,
+            ],
+            [
+                '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])',
+                24,
+            ],
+            [
+                '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])',
+                32,
+            ],
+            [
+                '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])',
+                40,
+            ],
+            [
+                '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,
+            ],
+            [
+                'Form alter hook "form_alter" first parameter should be an array. Expected signature: method(&$form, \Drupal\Core\Form\FormStateInterface $form_state[, $form_id])',
+                56,
+            ],
+        ]);
+    }
+
+
+}

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

@@ -0,0 +1,61 @@
+<?php
+
+namespace mglaman\PHPStanDrupal\Tests\Rules\data;
+
+use Drupal\Core\Form\FormStateInterface;
+use Drupal\Core\Hook\Attribute\Hook;
+
+/**
+ * Invalid form alter hook implementations for testing.
+ */
+class InvalidFormAlterHooks {
+
+  /**
+   * Too few parameters (missing $form_state).
+   */
+  #[Hook('form_alter')]
+  public function tooFewParameters(array &$form): void {
+    // Invalid: missing $form_state parameter
+  }
+
+  /**
+   * First parameter not by reference.
+   */
+  #[Hook('form_alter')]
+  public function formNotByReference(array $form, FormStateInterface $form_state, string $form_id): void {
+    // Invalid: $form not by reference
+  }
+
+  /**
+   * Wrong type for second parameter.
+   */
+  #[Hook('form_alter')]
+  public function wrongFormStateType(array &$form, string $form_state, string $form_id): void {
+    // Invalid: $form_state should be FormStateInterface
+  }
+
+  /**
+   * Wrong type for third parameter.
+   */
+  #[Hook('form_node_form_alter')]
+  public function wrongFormIdType(array &$form, FormStateInterface $form_state, int $form_id): void {
+    // Invalid: $form_id should be string
+  }
+
+  /**
+   * Too many parameters.
+   */
+  #[Hook('form_alter')]
+  public function tooManyParameters(array &$form, FormStateInterface $form_state, string $form_id, string $extra): void {
+    // Invalid: too many parameters
+  }
+
+  /**
+   * Wrong array type hint.
+   */
+  #[Hook('form_alter')]
+  public function wrongFormType(string &$form, FormStateInterface $form_state, string $form_id): void {
+    // Invalid: $form should be array
+  }
+
+}