diff --git a/extension.neon b/extension.neon index 6d50e3c5..fe294170 100644 --- a/extension.neon +++ b/extension.neon @@ -37,6 +37,7 @@ parameters: hookRules: false loggerFromFactoryPropertyAssignmentRule: false entityStorageDirectInjectionRule: false + constraintValidatorMustNarrowConstraintType: false entityMapping: aggregator_feed: class: Drupal\aggregator\Entity\Feed @@ -275,6 +276,7 @@ parametersSchema: hookRules: boolean() loggerFromFactoryPropertyAssignmentRule: boolean() entityStorageDirectInjectionRule: boolean() + constraintValidatorMustNarrowConstraintType: boolean() ]) entityMapping: arrayOf(anyOf( structure([ diff --git a/rules.neon b/rules.neon index b66d6497..0c927286 100644 --- a/rules.neon +++ b/rules.neon @@ -34,6 +34,8 @@ conditionalTags: phpstan.rules.rule: %drupal.rules.entityStorageDirectInjectionRule% mglaman\PHPStanDrupal\Rules\Drupal\EntityStoragePropertyAssignmentRule: phpstan.rules.rule: %drupal.rules.entityStorageDirectInjectionRule% + mglaman\PHPStanDrupal\Rules\Drupal\ConstraintValidatorValidateNarrowsConstraintTypeRule: + phpstan.rules.rule: %drupal.rules.constraintValidatorMustNarrowConstraintType% services: - @@ -58,3 +60,5 @@ services: class: mglaman\PHPStanDrupal\Rules\Drupal\EntityStorageDirectInjectionRule - class: mglaman\PHPStanDrupal\Rules\Drupal\EntityStoragePropertyAssignmentRule + - + class: mglaman\PHPStanDrupal\Rules\Drupal\ConstraintValidatorValidateNarrowsConstraintTypeRule diff --git a/src/Rules/Drupal/ConstraintValidatorValidateNarrowsConstraintTypeRule.php b/src/Rules/Drupal/ConstraintValidatorValidateNarrowsConstraintTypeRule.php new file mode 100644 index 00000000..e7c54ae5 --- /dev/null +++ b/src/Rules/Drupal/ConstraintValidatorValidateNarrowsConstraintTypeRule.php @@ -0,0 +1,161 @@ + + */ +final class ConstraintValidatorValidateNarrowsConstraintTypeRule implements Rule +{ + + private ReflectionProvider $reflectionProvider; + + public function __construct(ReflectionProvider $reflectionProvider) + { + $this->reflectionProvider = $reflectionProvider; + } + + public function getNodeType(): string + { + return Node\Stmt\ClassMethod::class; + } + + public function processNode(Node $node, Scope $scope): array + { + if ($node->name->toString() !== 'validate') { + return []; + } + if (!$scope->isInClass()) { + return []; + } + + // Guard: symfony/validator may not be installed in every project. + if (!$this->reflectionProvider->hasClass(ConstraintValidator::class) + || !$this->reflectionProvider->hasClass(Constraint::class)) { + return []; + } + + $classReflection = $scope->getClassReflection(); + if (!$classReflection->isSubclassOfClass($this->reflectionProvider->getClass(ConstraintValidator::class))) { + return []; + } + + // Derive the matching constraint class name by stripping 'Validator' suffix. + $fqcn = $classReflection->getName(); + if (!str_ends_with($fqcn, 'Validator')) { + return []; + } + $constraintClass = substr($fqcn, 0, -9); + + if (!$this->reflectionProvider->hasClass($constraintClass)) { + return []; + } + $constraintReflection = $this->reflectionProvider->getClass($constraintClass); + if (!$constraintReflection->isSubclassOfClass($this->reflectionProvider->getClass(Constraint::class))) { + return []; + } + + // Find the $constraint parameter name (second param by Symfony convention). + $constraintParamName = 'constraint'; + if (array_key_exists(1, $node->params)) { + $param = $node->params[1]; + if ($param->var instanceof Node\Expr\Variable && is_string($param->var->name)) { + $constraintParamName = $param->var->name; + } + } + + // Check if the method body asserts the concrete constraint type. + if ($this->hasConstraintAssertion($node->stmts ?? [], $constraintParamName, $constraintClass, $scope)) { + return []; + } + + $validatorFqcn = $classReflection->getName(); + $validatorPos = strrpos($validatorFqcn, '\\'); + $validatorShortName = $validatorPos !== false ? substr($validatorFqcn, $validatorPos + 1) : $validatorFqcn; + $constraintFqcn = $constraintReflection->getName(); + $constraintPos = strrpos($constraintFqcn, '\\'); + $constraintShortName = $constraintPos !== false ? substr($constraintFqcn, $constraintPos + 1) : $constraintFqcn; + + return [ + RuleErrorBuilder::message(sprintf( + 'Method %s::validate() does not narrow the $%s parameter type. Add assert($%s instanceof %s) to allow PHPStan to infer constraint-specific properties.', + $validatorShortName, + $constraintParamName, + $constraintParamName, + $constraintShortName + )) + ->identifier('drupal.constraintValidator.missingNarrow') + ->tip('See https://www.drupal.org/project/drupal/issues/3246287') + ->build(), + ]; + } + + /** + * Returns true if the statement list contains assert($var instanceof ExpectedConstraintClass). + * + * @param Node\Stmt[] $stmts + */ + private function hasConstraintAssertion(array $stmts, string $paramName, string $expectedConstraintFqcn, Scope $scope): bool + { + foreach ($stmts as $stmt) { + if (!$stmt instanceof Node\Stmt\Expression) { + continue; + } + $expr = $stmt->expr; + if (!$expr instanceof Node\Expr\FuncCall) { + continue; + } + if (!$expr->name instanceof Node\Name || $expr->name->toString() !== 'assert') { + continue; + } + $args = $expr->getArgs(); + if (count($args) === 0) { + continue; + } + $assertArg = $args[0]->value; + if (!$assertArg instanceof Node\Expr\Instanceof_) { + continue; + } + if (!$assertArg->expr instanceof Node\Expr\Variable) { + continue; + } + if ($assertArg->expr->name !== $paramName) { + continue; + } + if (!$assertArg->class instanceof Node\Name) { + continue; + } + $assertedClass = $scope->resolveName($assertArg->class); + if ($assertedClass === $expectedConstraintFqcn) { + return true; + } + } + + return false; + } +} diff --git a/tests/src/Rules/ConstraintValidatorValidateNarrowsConstraintTypeRuleTest.php b/tests/src/Rules/ConstraintValidatorValidateNarrowsConstraintTypeRuleTest.php new file mode 100644 index 00000000..dd6a4457 --- /dev/null +++ b/tests/src/Rules/ConstraintValidatorValidateNarrowsConstraintTypeRuleTest.php @@ -0,0 +1,33 @@ +getByType(ConstraintValidatorValidateNarrowsConstraintTypeRule::class); + } + + public function testRule(): void + { + $this->analyse( + [__DIR__ . '/data/constraint-validator.php'], + [ + [ + 'Method UniqueItemValidator::validate() does not narrow the $constraint parameter type. Add assert($constraint instanceof UniqueItem) to allow PHPStan to infer constraint-specific properties.', + 22, + 'See https://www.drupal.org/project/drupal/issues/3246287', + ], + ] + ); + } + +} diff --git a/tests/src/Rules/data/constraint-validator.php b/tests/src/Rules/data/constraint-validator.php new file mode 100644 index 00000000..9381b845 --- /dev/null +++ b/tests/src/Rules/data/constraint-validator.php @@ -0,0 +1,70 @@ +context->addViolation($constraint->alreadyExists); + } + +} + +// Constraint class for the assert-detection test case. +class UniqueItemWithAssert extends Constraint +{ + + public string $alreadyExists = 'The item %value already exists.'; + +} + +// No error: validate() uses assert() to narrow to the expected constraint type. +class UniqueItemWithAssertValidator extends ConstraintValidator +{ + + public function validate(mixed $value, Constraint $constraint): void + { + assert($constraint instanceof UniqueItemWithAssert); + $this->context->addViolation($constraint->alreadyExists); + } + +} + +// No error: 'StandaloneValidator' strips to 'Standalone', but there is no +// corresponding constraint class with that name. +class StandaloneValidator extends ConstraintValidator +{ + + public function validate(mixed $value, Constraint $constraint): void + { + $this->context->addViolation('some message'); + } + +} + +// No error: 'OrphanedValidator' strips to 'Orphaned', which is not a known constraint. +class OrphanedValidator extends ConstraintValidator +{ + + public function validate(mixed $value, Constraint $constraint): void + { + $this->context->addViolation('some message'); + } + +}