From a1b4f9eccf3e06c2b286fc06564a3d738f80f4cc Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Mon, 21 Sep 2026 09:46:51 +0200 Subject: [PATCH 1/4] Add 6 generic rules: command/constraint attributes, interface-in-constructor, duplicate attribute, property-to-property assign, array-shape return --- README.md | 186 +++++++++++++++ config/code-complexity-rules.neon | 1 + config/static-rules.neon | 4 + config/symfony-rules.neon | 5 + src/Enum/RuleIdentifier.php | 12 + .../NoPropertyToPropertyAssignRule.php | 103 ++++++++ .../NoDuplicateNonRepeatableAttributeRule.php | 116 +++++++++ src/Rules/RequireArrayShapeReturnRule.php | 221 ++++++++++++++++++ .../CommandMustHaveAsCommandAttributeRule.php | 79 +++++++ .../ConstraintMustHaveAttributeRule.php | 82 +++++++ .../PreferInterfaceInConstructorRule.php | 131 +++++++++++ .../Component/Console/Attribute/AsCommand.php | 15 ++ .../Component/Validator/Constraint.php | 11 + .../Fixture/ReportPropertyAssign.php | 19 ++ .../Fixture/SkipScalarAssign.php | 17 ++ .../NoPropertyToPropertyAssignRuleTest.php | 42 ++++ .../Source/SomeService.php | 9 + .../Fixture/ReportDuplicateAttribute.php | 14 ++ .../Fixture/SkipRepeatableAttribute.php | 14 ++ .../Fixture/SkipSingleAttribute.php | 13 ++ ...uplicateNonRepeatableAttributeRuleTest.php | 59 +++++ .../Source/RepeatableAttribute.php | 10 + .../Source/SingleAttribute.php | 10 + .../config/configured_rule.neon | 5 + .../Fixture/ReportKeyedArrayReturn.php | 16 ++ .../Fixture/SkipPositionalArrayReturn.php | 13 ++ .../Fixture/SkipShapedReturn.php | 19 ++ .../Fixture/SkipSingleValueReturn.php | 13 ++ .../RequireArrayShapeReturnRuleTest.php | 44 ++++ ...mandMustHaveAsCommandAttributeRuleTest.php | 58 +++++ .../Fixture/ReportCommand.php | 11 + .../Fixture/SkipAbstractCommand.php | 11 + .../Fixture/SkipCommandWithAttribute.php | 13 ++ .../Fixture/SkipNonCommand.php | 9 + .../config/configured_rule.neon | 5 + .../ConstraintMustHaveAttributeRuleTest.php | 58 +++++ .../Fixture/ReportConstraint.php | 11 + .../Fixture/SkipAbstractConstraint.php | 11 + .../Fixture/SkipConstraintWithAttribute.php | 13 ++ .../Fixture/SkipNonConstraint.php | 9 + .../config/configured_rule.neon | 5 + .../Fixture/ReportConcreteRouter.php | 15 ++ .../Fixture/SkipProjectClass.php | 13 ++ .../Fixture/SkipRouterInterface.php | 15 ++ .../PreferInterfaceInConstructorRuleTest.php | 60 +++++ .../config/configured_rule.neon | 5 + 46 files changed, 1605 insertions(+) create mode 100644 src/Rules/Complexity/NoPropertyToPropertyAssignRule.php create mode 100644 src/Rules/NoDuplicateNonRepeatableAttributeRule.php create mode 100644 src/Rules/RequireArrayShapeReturnRule.php create mode 100644 src/Rules/Symfony/CommandMustHaveAsCommandAttributeRule.php create mode 100644 src/Rules/Symfony/ConstraintMustHaveAttributeRule.php create mode 100644 src/Rules/Symfony/PreferInterfaceInConstructorRule.php create mode 100644 stubs/Symfony/Component/Console/Attribute/AsCommand.php create mode 100644 stubs/Symfony/Component/Validator/Constraint.php create mode 100644 tests/Rules/Complexity/NoPropertyToPropertyAssignRule/Fixture/ReportPropertyAssign.php create mode 100644 tests/Rules/Complexity/NoPropertyToPropertyAssignRule/Fixture/SkipScalarAssign.php create mode 100644 tests/Rules/Complexity/NoPropertyToPropertyAssignRule/NoPropertyToPropertyAssignRuleTest.php create mode 100644 tests/Rules/Complexity/NoPropertyToPropertyAssignRule/Source/SomeService.php create mode 100644 tests/Rules/NoDuplicateNonRepeatableAttributeRule/Fixture/ReportDuplicateAttribute.php create mode 100644 tests/Rules/NoDuplicateNonRepeatableAttributeRule/Fixture/SkipRepeatableAttribute.php create mode 100644 tests/Rules/NoDuplicateNonRepeatableAttributeRule/Fixture/SkipSingleAttribute.php create mode 100644 tests/Rules/NoDuplicateNonRepeatableAttributeRule/NoDuplicateNonRepeatableAttributeRuleTest.php create mode 100644 tests/Rules/NoDuplicateNonRepeatableAttributeRule/Source/RepeatableAttribute.php create mode 100644 tests/Rules/NoDuplicateNonRepeatableAttributeRule/Source/SingleAttribute.php create mode 100644 tests/Rules/NoDuplicateNonRepeatableAttributeRule/config/configured_rule.neon create mode 100644 tests/Rules/RequireArrayShapeReturnRule/Fixture/ReportKeyedArrayReturn.php create mode 100644 tests/Rules/RequireArrayShapeReturnRule/Fixture/SkipPositionalArrayReturn.php create mode 100644 tests/Rules/RequireArrayShapeReturnRule/Fixture/SkipShapedReturn.php create mode 100644 tests/Rules/RequireArrayShapeReturnRule/Fixture/SkipSingleValueReturn.php create mode 100644 tests/Rules/RequireArrayShapeReturnRule/RequireArrayShapeReturnRuleTest.php create mode 100644 tests/Rules/Symfony/CommandMustHaveAsCommandAttributeRule/CommandMustHaveAsCommandAttributeRuleTest.php create mode 100644 tests/Rules/Symfony/CommandMustHaveAsCommandAttributeRule/Fixture/ReportCommand.php create mode 100644 tests/Rules/Symfony/CommandMustHaveAsCommandAttributeRule/Fixture/SkipAbstractCommand.php create mode 100644 tests/Rules/Symfony/CommandMustHaveAsCommandAttributeRule/Fixture/SkipCommandWithAttribute.php create mode 100644 tests/Rules/Symfony/CommandMustHaveAsCommandAttributeRule/Fixture/SkipNonCommand.php create mode 100644 tests/Rules/Symfony/CommandMustHaveAsCommandAttributeRule/config/configured_rule.neon create mode 100644 tests/Rules/Symfony/ConstraintMustHaveAttributeRule/ConstraintMustHaveAttributeRuleTest.php create mode 100644 tests/Rules/Symfony/ConstraintMustHaveAttributeRule/Fixture/ReportConstraint.php create mode 100644 tests/Rules/Symfony/ConstraintMustHaveAttributeRule/Fixture/SkipAbstractConstraint.php create mode 100644 tests/Rules/Symfony/ConstraintMustHaveAttributeRule/Fixture/SkipConstraintWithAttribute.php create mode 100644 tests/Rules/Symfony/ConstraintMustHaveAttributeRule/Fixture/SkipNonConstraint.php create mode 100644 tests/Rules/Symfony/ConstraintMustHaveAttributeRule/config/configured_rule.neon create mode 100644 tests/Rules/Symfony/PreferInterfaceInConstructorRule/Fixture/ReportConcreteRouter.php create mode 100644 tests/Rules/Symfony/PreferInterfaceInConstructorRule/Fixture/SkipProjectClass.php create mode 100644 tests/Rules/Symfony/PreferInterfaceInConstructorRule/Fixture/SkipRouterInterface.php create mode 100644 tests/Rules/Symfony/PreferInterfaceInConstructorRule/PreferInterfaceInConstructorRuleTest.php create mode 100644 tests/Rules/Symfony/PreferInterfaceInConstructorRule/config/configured_rule.neon diff --git a/README.md b/README.md index 1d674f566..7106dfe7f 100644 --- a/README.md +++ b/README.md @@ -1420,6 +1420,97 @@ final class SomeService
+### NoPropertyToPropertyAssignRule + +An object property must not be assigned from another object property of the same object - it keeps the same service under 2 names. Use the original property directly instead. + +```yaml +rules: + - Symplify\PHPStanRules\Rules\Complexity\NoPropertyToPropertyAssignRule +``` + +```php +$this->repository = $this->someRepository; +``` + +:x: + +
+ +```php +// use $this->someRepository directly +``` + +:+1: + +
+ +### NoDuplicateNonRepeatableAttributeRule + +An attribute can only be repeated on the same class, method or property when it is declared with the `\Attribute::IS_REPEATABLE` flag. + +```yaml +rules: + - Symplify\PHPStanRules\Rules\NoDuplicateNonRepeatableAttributeRule +``` + +```php +#[SomeAttribute] +#[SomeAttribute] +private string $name; +``` + +:x: + +
+ +```php +#[SomeAttribute] +private string $name; +``` + +:+1: + +
+ +### RequireArrayShapeReturnRule + +A method that returns a packed keyed array of 2-3 named values should declare that shape in its `@return`, so the caller knows each key and its type. + +```yaml +rules: + - Symplify\PHPStanRules\Rules\RequireArrayShapeReturnRule +``` + +```php +public function run(): array +{ + return ['name' => $name, 'age' => $age]; +} +``` + +:x: + +
+ +```php +/** + * @return array{name: string, age: int} + */ +public function run(): array +{ + return ['name' => $name, 'age' => $age]; +} +``` + +:+1: + +
+ +--- + +
+ ## 2. Doctrine-specific Rules ### RequireQueryBuilderOnRepositoryRule @@ -2615,6 +2706,101 @@ return function (ContainerConfigurator $container) {
+### CommandMustHaveAsCommandAttributeRule + +Every class that extends Symfony `Command` must declare the `#[AsCommand]` attribute. + +```yaml +rules: + - Symplify\PHPStanRules\Rules\Symfony\CommandMustHaveAsCommandAttributeRule +``` + +```php +final class ReportCommand extends Command +{ +} +``` + +:x: + +
+ +```php +#[AsCommand('app:report')] +final class ReportCommand extends Command +{ +} +``` + +:+1: + +
+ +### ConstraintMustHaveAttributeRule + +Every class that extends Symfony `Constraint` must declare the `#[\Attribute]` attribute, so it can be used as an attribute on properties. + +```yaml +rules: + - Symplify\PHPStanRules\Rules\Symfony\ConstraintMustHaveAttributeRule +``` + +```php +final class UniqueEmail extends Constraint +{ +} +``` + +:x: + +
+ +```php +#[\Attribute] +final class UniqueEmail extends Constraint +{ +} +``` + +:+1: + +
+ +### PreferInterfaceInConstructorRule + +A constructor dependency typed as a concrete Symfony/Doctrine class that has a same-named `*Interface` should use that interface instead - it blocks decoration otherwise. + +```yaml +rules: + - Symplify\PHPStanRules\Rules\Symfony\PreferInterfaceInConstructorRule +``` + +```php +public function __construct( + private Router $router, +) { +} +``` + +:x: + +
+ +```php +public function __construct( + private RouterInterface $router, +) { +} +``` + +:+1: + +
+ +--- + +
+ ## 4. PHPUnit-specific Rules ### NoAssertFuncCallInTestsRule diff --git a/config/code-complexity-rules.neon b/config/code-complexity-rules.neon index 960c080a6..88472a28b 100644 --- a/config/code-complexity-rules.neon +++ b/config/code-complexity-rules.neon @@ -1,6 +1,7 @@ rules: - Symplify\PHPStanRules\Rules\NoDynamicNameRule - Symplify\PHPStanRules\Rules\Complexity\NoJustPropertyAssignRule + - Symplify\PHPStanRules\Rules\Complexity\NoPropertyToPropertyAssignRule - Symplify\PHPStanRules\Rules\Complexity\NoArrayMapWithArrayCallableRule - Symplify\PHPStanRules\Rules\Complexity\NoConstructorOverrideRule - Symplify\PHPStanRules\Rules\Complexity\ForeachCeptionRule diff --git a/config/static-rules.neon b/config/static-rules.neon index d76909435..8228ec45f 100644 --- a/config/static-rules.neon +++ b/config/static-rules.neon @@ -18,3 +18,7 @@ rules: # docblock - Symplify\PHPStanRules\Rules\NoMissnamedDocTagRule + - Symplify\PHPStanRules\Rules\RequireArrayShapeReturnRule + + # attributes + - Symplify\PHPStanRules\Rules\NoDuplicateNonRepeatableAttributeRule diff --git a/config/symfony-rules.neon b/config/symfony-rules.neon index 1b0fb5013..76b6fdf1a 100644 --- a/config/symfony-rules.neon +++ b/config/symfony-rules.neon @@ -30,3 +30,8 @@ rules: # attributes - Symplify\PHPStanRules\Rules\Symfony\RequireIsGrantedEnumRule - Symplify\PHPStanRules\Rules\Symfony\NoBareAndSecurityIsGrantedContentsRule + - Symplify\PHPStanRules\Rules\Symfony\CommandMustHaveAsCommandAttributeRule + - Symplify\PHPStanRules\Rules\Symfony\ConstraintMustHaveAttributeRule + + # constructor injection + - Symplify\PHPStanRules\Rules\Symfony\PreferInterfaceInConstructorRule diff --git a/src/Enum/RuleIdentifier.php b/src/Enum/RuleIdentifier.php index aedcf4285..41cdf2fb0 100644 --- a/src/Enum/RuleIdentifier.php +++ b/src/Enum/RuleIdentifier.php @@ -81,4 +81,16 @@ final class RuleIdentifier public const string NO_MISSNAMED_DOC_TAG = 'symplify.noMissnamedDocTag'; public const string NEW_OVER_SETTERS = 'symplify.newOverSetters'; + + public const string COMMAND_HAS_AS_COMMAND_ATTRIBUTE = 'symplify.commandHasAsCommandAttribute'; + + public const string CONSTRAINT_HAS_ATTRIBUTE = 'symplify.constraintHasAttribute'; + + public const string PREFER_INTERFACE_IN_CONSTRUCTOR = 'symplify.preferInterfaceInConstructor'; + + public const string NO_DUPLICATE_NON_REPEATABLE_ATTRIBUTE = 'symplify.noDuplicateNonRepeatableAttribute'; + + public const string NO_PROPERTY_TO_PROPERTY_ASSIGN = 'symplify.noPropertyToPropertyAssign'; + + public const string REQUIRE_ARRAY_SHAPE_RETURN = 'symplify.requireArrayShapeReturn'; } diff --git a/src/Rules/Complexity/NoPropertyToPropertyAssignRule.php b/src/Rules/Complexity/NoPropertyToPropertyAssignRule.php new file mode 100644 index 000000000..102f0f7c2 --- /dev/null +++ b/src/Rules/Complexity/NoPropertyToPropertyAssignRule.php @@ -0,0 +1,103 @@ +repository = $this->someRepository;" keeps the very same service under 2 names, so both properties have to be + * kept in sync forever. Use the original property directly instead and drop the duplicate one. + * + * Only object properties are reported - a scalar or array property is often a deliberate snapshot of a previous state, + * e.g. "$this->bodyInitial = $this->body;". Anonymous classes are skipped, as they are local one-offs. + * + * @see \Symplify\PHPStanRules\Tests\Rules\Complexity\NoPropertyToPropertyAssignRule\NoPropertyToPropertyAssignRuleTest + * + * @implements Rule + */ +final class NoPropertyToPropertyAssignRule implements Rule +{ + public const string ERROR_MESSAGE = 'Property "$this->%s" must not be assigned from property "$this->%s". Use the original property directly instead'; + + public function getNodeType(): string + { + return Assign::class; + } + + /** + * @param Assign $node + * + * @return list + */ + public function processNode(Node $node, Scope $scope): array + { + // an anonymous class is a local one-off, e.g. a test double filling a parent property + $classReflection = $scope->getClassReflection(); + if (! $classReflection instanceof ClassReflection || $classReflection->isAnonymous()) { + return []; + } + + $assignedPropertyName = $this->matchThisPropertyName($node->var); + if ($assignedPropertyName === null) { + return []; + } + + $sourcePropertyName = $this->matchThisPropertyName($node->expr); + if ($sourcePropertyName === null) { + return []; + } + + // "$this->items = $this->items" is a different smell, not a duplicated property + if ($assignedPropertyName === $sourcePropertyName) { + return []; + } + + // a scalar or array property is often a deliberate snapshot of a previous state + if ($scope->getType($node->expr)->getObjectClassNames() === []) { + return []; + } + + $identifierRuleError = RuleErrorBuilder::message( + sprintf(self::ERROR_MESSAGE, $assignedPropertyName, $sourcePropertyName) + ) + ->identifier(RuleIdentifier::NO_PROPERTY_TO_PROPERTY_ASSIGN) + ->build(); + + return [$identifierRuleError]; + } + + /** + * Returns the property name of a "$this->someProperty" fetch, null for anything else. + */ + private function matchThisPropertyName(Expr $expr): ?string + { + if (! $expr instanceof PropertyFetch) { + return null; + } + + if (! $expr->var instanceof Variable || $expr->var->name !== 'this') { + return null; + } + + if (! $expr->name instanceof Identifier) { + return null; + } + + return $expr->name->toString(); + } +} diff --git a/src/Rules/NoDuplicateNonRepeatableAttributeRule.php b/src/Rules/NoDuplicateNonRepeatableAttributeRule.php new file mode 100644 index 000000000..7f64211c7 --- /dev/null +++ b/src/Rules/NoDuplicateNonRepeatableAttributeRule.php @@ -0,0 +1,116 @@ + + */ +final readonly class NoDuplicateNonRepeatableAttributeRule implements Rule +{ + public const string ERROR_MESSAGE = 'Attribute "#[%s]" is used %d times on the same %s, but is not repeatable. Add the ' . Attribute::class . '::IS_REPEATABLE flag to its #[' . Attribute::class . '] declaration, or remove the duplicate'; + + public function __construct( + private ReflectionProvider $reflectionProvider, + ) { + } + + public function getNodeType(): string + { + return Stmt::class; + } + + /** + * @return list + */ + public function processNode(Node $node, Scope $scope): array + { + $elementType = $this->resolveElementType($node); + if ($elementType === null) { + return []; + } + + /** @var Class_|ClassMethod|Property $node */ + $countByAttribute = []; + foreach ($node->attrGroups as $attrGroup) { + foreach ($attrGroup->attrs as $attr) { + $attributeName = $attr->name->toString(); + $countByAttribute[$attributeName] = ($countByAttribute[$attributeName] ?? 0) + 1; + } + } + + $ruleErrors = []; + foreach ($countByAttribute as $attributeName => $count) { + if ($count < 2) { + continue; + } + + if ($this->isRepeatable($attributeName)) { + continue; + } + + $ruleErrors[] = RuleErrorBuilder::message( + sprintf(self::ERROR_MESSAGE, $attributeName, $count, $elementType) + ) + ->identifier(RuleIdentifier::NO_DUPLICATE_NON_REPEATABLE_ATTRIBUTE) + ->build(); + } + + return $ruleErrors; + } + + private function resolveElementType(Node $node): ?string + { + if ($node instanceof Class_) { + return 'class'; + } + + if ($node instanceof ClassMethod) { + return 'method'; + } + + if ($node instanceof Property) { + return 'property'; + } + + return null; + } + + private function isRepeatable(string $attributeName): bool + { + if (! $this->reflectionProvider->hasClass($attributeName)) { + // cannot confirm it is non-repeatable, so stay silent to avoid a false positive + return true; + } + + $nativeReflection = $this->reflectionProvider->getClass($attributeName) + ->getNativeReflection(); + foreach ($nativeReflection->getAttributes(Attribute::class) as $reflectionAttribute) { + $flags = $reflectionAttribute->getArguments()[0] ?? 0; + + return (bool) ($flags & Attribute::IS_REPEATABLE); + } + + // the class has no #[\Attribute] declaration, treat as non-repeatable + return false; + } +} diff --git a/src/Rules/RequireArrayShapeReturnRule.php b/src/Rules/RequireArrayShapeReturnRule.php new file mode 100644 index 000000000..c11036257 --- /dev/null +++ b/src/Rules/RequireArrayShapeReturnRule.php @@ -0,0 +1,221 @@ + + */ +final readonly class RequireArrayShapeReturnRule implements Rule +{ + public const string ERROR_MESSAGE = 'Method "%s()" returns a keyed array of %d values; declare its shape in @return, e.g. array{key: type}'; + + private const int MIN_VALUE_COUNT = 2; + + private const int MAX_VALUE_COUNT = 3; + + public function getNodeType(): string + { + return ClassMethod::class; + } + + /** + * @param ClassMethod $node + * + * @return list + */ + public function processNode(Node $node, Scope $scope): array + { + if ($node->stmts === null) { + return []; + } + + if (! $scope->isInClass()) { + return []; + } + + $classReflection = $scope->getClassReflection(); + + // anonymous classes are local one-off implementations, skip them + if ($classReflection->isAnonymous()) { + return []; + } + + $methodName = $node->name->toString(); + + // a method overriding a parent one is bound to that contract's shape, skip it + if ($this->isDeclaredInParent($classReflection, $methodName)) { + return []; + } + + $returnType = $classReflection->getNativeMethod($methodName) + ->getVariants()[0] + ->getReturnType(); + if ($returnType->isVoid()->yes()) { + return []; + } + + // a mixed return cannot be pinned to an array shape, skip it + if ($returnType instanceof MixedType) { + return []; + } + + // @return already declares an array shape, the keys and types are documented + if ($this->declaresArrayShape($returnType)) { + return []; + } + + $valueReturns = array_filter( + $this->collectReturns($node->stmts), + static fn (Return_ $return): bool => $return->expr instanceof Node + ); + if ($valueReturns === []) { + return []; + } + + $firstKeyedArray = null; + foreach ($valueReturns as $valueReturn) { + $expr = $valueReturn->expr; + + // a return that is not a packed keyed array means no single shape fits, skip the method + if (! $expr instanceof Array_ || ! $this->isPackedKeyedArray($expr)) { + return []; + } + + $firstKeyedArray ??= $expr; + } + + $ruleError = RuleErrorBuilder::message( + sprintf(self::ERROR_MESSAGE, $methodName, count($firstKeyedArray->items)) + ) + ->identifier(RuleIdentifier::REQUIRE_ARRAY_SHAPE_RETURN) + ->line($firstKeyedArray->getStartLine()) + ->build(); + + return [$ruleError]; + } + + private function isPackedKeyedArray(Array_ $array): bool + { + $valueCount = count($array->items); + if ($valueCount < self::MIN_VALUE_COUNT || $valueCount > self::MAX_VALUE_COUNT) { + return false; + } + + if (! $this->hasStringKeyOnEveryItem($array)) { + return false; + } + + return ! $this->hasOnlyStaticDataValues($array); + } + + /** + * Collect return statements in the given nodes, without descending into nested functions or classes. + * + * @param Node[] $nodes + * + * @return list + */ + private function collectReturns(array $nodes): array + { + $returns = []; + foreach ($nodes as $node) { + if ($node instanceof Return_) { + $returns[] = $node; + continue; + } + + // nested closures, functions and anonymous classes have their own return context + if ($node instanceof FunctionLike || $node instanceof Class_) { + continue; + } + + foreach ($node->getSubNodeNames() as $subNodeName) { + $child = $node->{$subNodeName}; + if ($child instanceof Node) { + $returns = [...$returns, ...$this->collectReturns([$child])]; + } elseif (is_array($child)) { + $returns = [...$returns, ...$this->collectReturns( + array_filter($child, static fn ($item): bool => $item instanceof Node) + )]; + } + } + } + + return $returns; + } + + // an array shape may be one member of a union (e.g. array{...}|null), so check each member + private function declaresArrayShape(Type $type): bool + { + $types = $type instanceof UnionType ? $type->getTypes() : [$type]; + return array_any($types, fn (Type $innerType): bool => $innerType->isConstantArray()->yes()); + } + + private function isDeclaredInParent(ClassReflection $classReflection, string $methodName): bool + { + $parentClass = $classReflection->getParentClass(); + while ($parentClass instanceof ClassReflection) { + if ($parentClass->hasMethod($methodName)) { + return true; + } + + $parentClass = $parentClass->getParentClass(); + } + + return false; + } + + private function hasStringKeyOnEveryItem(Array_ $array): bool + { + return array_all($array->items, fn (ArrayItem $arrayItem): bool => $arrayItem->key instanceof String_); + } + + /** + * Static data maps - nested arrays or constant lookups - are config/definition tables, not packed results. + */ + private function hasOnlyStaticDataValues(Array_ $array): bool + { + foreach ($array->items as $arrayItem) { + $value = $arrayItem->value; + if (! $value instanceof Array_ && ! $value instanceof ClassConstFetch && ! $value instanceof ConstFetch) { + return false; + } + } + + return true; + } +} diff --git a/src/Rules/Symfony/CommandMustHaveAsCommandAttributeRule.php b/src/Rules/Symfony/CommandMustHaveAsCommandAttributeRule.php new file mode 100644 index 000000000..62de33446 --- /dev/null +++ b/src/Rules/Symfony/CommandMustHaveAsCommandAttributeRule.php @@ -0,0 +1,79 @@ + + */ +final readonly class CommandMustHaveAsCommandAttributeRule implements Rule +{ + public const string ERROR_MESSAGE = 'Class "%s" extends Command but is missing the #[AsCommand] attribute'; + + private const string AS_COMMAND_ATTRIBUTE = 'Symfony\Component\Console\Attribute\AsCommand'; + + public function __construct( + private ReflectionProvider $reflectionProvider, + ) { + } + + public function getNodeType(): string + { + return Class_::class; + } + + /** + * @param Class_ $node + * + * @return list + */ + public function processNode(Node $node, Scope $scope): array + { + // an anonymous class carries no name to report, and is no registered command + if (! $node->name instanceof Identifier) { + return []; + } + + // abstract base commands do not need the attribute + if ($node->isAbstract()) { + return []; + } + + $className = (string) $node->namespacedName; + if (! $this->reflectionProvider->hasClass($className)) { + return []; + } + + if (! $this->reflectionProvider->getClass($className)->is(Command::class)) { + return []; + } + + foreach ($node->attrGroups as $attrGroup) { + foreach ($attrGroup->attrs as $attr) { + if ($attr->name->toString() === self::AS_COMMAND_ATTRIBUTE) { + return []; + } + } + } + + $identifierRuleError = RuleErrorBuilder::message(sprintf(self::ERROR_MESSAGE, $className)) + ->identifier(RuleIdentifier::COMMAND_HAS_AS_COMMAND_ATTRIBUTE) + ->build(); + + return [$identifierRuleError]; + } +} diff --git a/src/Rules/Symfony/ConstraintMustHaveAttributeRule.php b/src/Rules/Symfony/ConstraintMustHaveAttributeRule.php new file mode 100644 index 000000000..64b834342 --- /dev/null +++ b/src/Rules/Symfony/ConstraintMustHaveAttributeRule.php @@ -0,0 +1,82 @@ + + */ +final readonly class ConstraintMustHaveAttributeRule implements Rule +{ + public const string ERROR_MESSAGE = 'Class "%s" extends Constraint but is missing the #[\Attribute] attribute. Add it, so the constraint can be used as an attribute on properties, as Symfony convention'; + + private const string CONSTRAINT_CLASS = 'Symfony\Component\Validator\Constraint'; + + public function __construct( + private ReflectionProvider $reflectionProvider, + ) { + } + + public function getNodeType(): string + { + return Class_::class; + } + + /** + * @param Class_ $node + * + * @return list + */ + public function processNode(Node $node, Scope $scope): array + { + // an anonymous class carries no name to report, and cannot be used as an attribute + if (! $node->name instanceof Identifier) { + return []; + } + + // abstract base constraints are never used directly + if ($node->isAbstract()) { + return []; + } + + $className = (string) $node->namespacedName; + if (! $this->reflectionProvider->hasClass($className)) { + return []; + } + + if (! $this->reflectionProvider->getClass($className)->is(self::CONSTRAINT_CLASS)) { + return []; + } + + foreach ($node->attrGroups as $attrGroup) { + foreach ($attrGroup->attrs as $attr) { + if ($attr->name->toString() === Attribute::class) { + return []; + } + } + } + + $identifierRuleError = RuleErrorBuilder::message(sprintf(self::ERROR_MESSAGE, $className)) + ->identifier(RuleIdentifier::CONSTRAINT_HAS_ATTRIBUTE) + ->build(); + + return [$identifierRuleError]; + } +} diff --git a/src/Rules/Symfony/PreferInterfaceInConstructorRule.php b/src/Rules/Symfony/PreferInterfaceInConstructorRule.php new file mode 100644 index 000000000..4e4c05c41 --- /dev/null +++ b/src/Rules/Symfony/PreferInterfaceInConstructorRule.php @@ -0,0 +1,131 @@ + + */ +final readonly class PreferInterfaceInConstructorRule implements Rule +{ + public const string ERROR_MESSAGE = 'Constructor dependency "%s" is typed as concrete "%s". Use the "%s" interface instead'; + + /** + * Only 3rd-party contracts are enforced - project classes are free to be typed directly. + * + * @var string[] + */ + private const array HANDLED_NAMESPACE_PREFIXES = ['Symfony\\', 'Doctrine\\']; + + public function __construct( + private ReflectionProvider $reflectionProvider, + ) { + } + + public function getNodeType(): string + { + return ClassMethod::class; + } + + /** + * @param ClassMethod $node + * + * @return list + */ + public function processNode(Node $node, Scope $scope): array + { + if ($node->name->toLowerString() !== '__construct') { + return []; + } + + $ruleErrors = []; + + foreach ($node->params as $param) { + if (! $param->type instanceof Name) { + continue; + } + + $className = $scope->resolveName($param->type); + $interfaceName = $this->matchImplementedSameNamedInterface($className); + if ($interfaceName === null) { + continue; + } + + $parameterName = $param->var instanceof Variable && is_string($param->var->name) + ? '$' . $param->var->name + : ''; + + $ruleErrors[] = RuleErrorBuilder::message( + sprintf(self::ERROR_MESSAGE, $parameterName, $className, $interfaceName) + ) + ->identifier(RuleIdentifier::PREFER_INTERFACE_IN_CONSTRUCTOR) + ->line($param->getStartLine()) + ->build(); + } + + return $ruleErrors; + } + + /** + * Returns the "Interface" name when the class implements it, null otherwise. + */ + private function matchImplementedSameNamedInterface(string $className): ?string + { + if (! $this->isHandledNamespace($className)) { + return null; + } + + if (! $this->reflectionProvider->hasClass($className)) { + return null; + } + + $classReflection = $this->reflectionProvider->getClass($className); + if ($classReflection->isInterface()) { + return null; + } + + $interfaceName = $className . 'Interface'; + if (! $this->reflectionProvider->hasClass($interfaceName)) { + return null; + } + + if (! $this->reflectionProvider->getClass($interfaceName)->isInterface()) { + return null; + } + + if (! $classReflection->implementsInterface($interfaceName)) { + return null; + } + + return $interfaceName; + } + + private function isHandledNamespace(string $className): bool + { + return array_any( + self::HANDLED_NAMESPACE_PREFIXES, + fn (string $handledNamespacePrefix): bool => str_starts_with($className, $handledNamespacePrefix) + ); + } +} diff --git a/stubs/Symfony/Component/Console/Attribute/AsCommand.php b/stubs/Symfony/Component/Console/Attribute/AsCommand.php new file mode 100644 index 000000000..a81210022 --- /dev/null +++ b/stubs/Symfony/Component/Console/Attribute/AsCommand.php @@ -0,0 +1,15 @@ +primary = $this->secondary; + } +} diff --git a/tests/Rules/Complexity/NoPropertyToPropertyAssignRule/Fixture/SkipScalarAssign.php b/tests/Rules/Complexity/NoPropertyToPropertyAssignRule/Fixture/SkipScalarAssign.php new file mode 100644 index 000000000..e7ead650c --- /dev/null +++ b/tests/Rules/Complexity/NoPropertyToPropertyAssignRule/Fixture/SkipScalarAssign.php @@ -0,0 +1,17 @@ +bodyInitial = $this->body; + } +} diff --git a/tests/Rules/Complexity/NoPropertyToPropertyAssignRule/NoPropertyToPropertyAssignRuleTest.php b/tests/Rules/Complexity/NoPropertyToPropertyAssignRule/NoPropertyToPropertyAssignRuleTest.php new file mode 100644 index 000000000..dd45b8580 --- /dev/null +++ b/tests/Rules/Complexity/NoPropertyToPropertyAssignRule/NoPropertyToPropertyAssignRuleTest.php @@ -0,0 +1,42 @@ + + */ +final class NoPropertyToPropertyAssignRuleTest extends RuleTestCase +{ + /** + * @param list $expectedErrorsWithLines + */ + #[DataProvider('provideData')] + public function testRule(string $filePath, array $expectedErrorsWithLines): void + { + $this->analyse([$filePath], $expectedErrorsWithLines); + } + + /** + * @return Iterator, mixed>> + */ + public static function provideData(): Iterator + { + $errorMessage = sprintf(NoPropertyToPropertyAssignRule::ERROR_MESSAGE, 'primary', 'secondary'); + yield [__DIR__ . '/Fixture/ReportPropertyAssign.php', [[$errorMessage, 17]]]; + + yield [__DIR__ . '/Fixture/SkipScalarAssign.php', []]; + } + + protected function getRule(): Rule + { + return new NoPropertyToPropertyAssignRule(); + } +} diff --git a/tests/Rules/Complexity/NoPropertyToPropertyAssignRule/Source/SomeService.php b/tests/Rules/Complexity/NoPropertyToPropertyAssignRule/Source/SomeService.php new file mode 100644 index 000000000..3a69f1979 --- /dev/null +++ b/tests/Rules/Complexity/NoPropertyToPropertyAssignRule/Source/SomeService.php @@ -0,0 +1,9 @@ + + */ +final class NoDuplicateNonRepeatableAttributeRuleTest extends RuleTestCase +{ + /** + * @param list $expectedErrorsWithLines + */ + #[DataProvider('provideData')] + public function testRule(string $filePath, array $expectedErrorsWithLines): void + { + $this->analyse([$filePath], $expectedErrorsWithLines); + } + + /** + * @return Iterator, mixed>> + */ + public static function provideData(): Iterator + { + $errorMessage = sprintf( + NoDuplicateNonRepeatableAttributeRule::ERROR_MESSAGE, + SingleAttribute::class, + 2, + 'property' + ); + yield [__DIR__ . '/Fixture/ReportDuplicateAttribute.php', [[$errorMessage, 11]]]; + + yield [__DIR__ . '/Fixture/SkipRepeatableAttribute.php', []]; + yield [__DIR__ . '/Fixture/SkipSingleAttribute.php', []]; + } + + /** + * @return string[] + */ + #[Override] + public static function getAdditionalConfigFiles(): array + { + return [__DIR__ . '/config/configured_rule.neon']; + } + + protected function getRule(): Rule + { + return self::getContainer()->getByType(NoDuplicateNonRepeatableAttributeRule::class); + } +} diff --git a/tests/Rules/NoDuplicateNonRepeatableAttributeRule/Source/RepeatableAttribute.php b/tests/Rules/NoDuplicateNonRepeatableAttributeRule/Source/RepeatableAttribute.php new file mode 100644 index 000000000..21b8e0ba8 --- /dev/null +++ b/tests/Rules/NoDuplicateNonRepeatableAttributeRule/Source/RepeatableAttribute.php @@ -0,0 +1,10 @@ + 'Tom', + 'age' => 30, + ]; + } +} diff --git a/tests/Rules/RequireArrayShapeReturnRule/Fixture/SkipPositionalArrayReturn.php b/tests/Rules/RequireArrayShapeReturnRule/Fixture/SkipPositionalArrayReturn.php new file mode 100644 index 000000000..eed9f4fc1 --- /dev/null +++ b/tests/Rules/RequireArrayShapeReturnRule/Fixture/SkipPositionalArrayReturn.php @@ -0,0 +1,13 @@ + 'Tom', + 'age' => 30, + ]; + } +} diff --git a/tests/Rules/RequireArrayShapeReturnRule/Fixture/SkipSingleValueReturn.php b/tests/Rules/RequireArrayShapeReturnRule/Fixture/SkipSingleValueReturn.php new file mode 100644 index 000000000..1c67f566b --- /dev/null +++ b/tests/Rules/RequireArrayShapeReturnRule/Fixture/SkipSingleValueReturn.php @@ -0,0 +1,13 @@ + + */ +final class RequireArrayShapeReturnRuleTest extends RuleTestCase +{ + /** + * @param list $expectedErrorsWithLines + */ + #[DataProvider('provideData')] + public function testRule(string $filePath, array $expectedErrorsWithLines): void + { + $this->analyse([$filePath], $expectedErrorsWithLines); + } + + /** + * @return Iterator, mixed>> + */ + public static function provideData(): Iterator + { + $errorMessage = sprintf(RequireArrayShapeReturnRule::ERROR_MESSAGE, 'run', 2); + yield [__DIR__ . '/Fixture/ReportKeyedArrayReturn.php', [[$errorMessage, 11]]]; + + yield [__DIR__ . '/Fixture/SkipShapedReturn.php', []]; + yield [__DIR__ . '/Fixture/SkipSingleValueReturn.php', []]; + yield [__DIR__ . '/Fixture/SkipPositionalArrayReturn.php', []]; + } + + protected function getRule(): Rule + { + return new RequireArrayShapeReturnRule(); + } +} diff --git a/tests/Rules/Symfony/CommandMustHaveAsCommandAttributeRule/CommandMustHaveAsCommandAttributeRuleTest.php b/tests/Rules/Symfony/CommandMustHaveAsCommandAttributeRule/CommandMustHaveAsCommandAttributeRuleTest.php new file mode 100644 index 000000000..1379ce3f5 --- /dev/null +++ b/tests/Rules/Symfony/CommandMustHaveAsCommandAttributeRule/CommandMustHaveAsCommandAttributeRuleTest.php @@ -0,0 +1,58 @@ + + */ +final class CommandMustHaveAsCommandAttributeRuleTest extends RuleTestCase +{ + /** + * @param list $expectedErrorsWithLines + */ + #[DataProvider('provideData')] + public function testRule(string $filePath, array $expectedErrorsWithLines): void + { + $this->analyse([$filePath], $expectedErrorsWithLines); + } + + /** + * @return Iterator, mixed>> + */ + public static function provideData(): Iterator + { + $errorMessage = sprintf( + CommandMustHaveAsCommandAttributeRule::ERROR_MESSAGE, + ReportCommand::class + ); + yield [__DIR__ . '/Fixture/ReportCommand.php', [[$errorMessage, 9]]]; + + yield [__DIR__ . '/Fixture/SkipCommandWithAttribute.php', []]; + yield [__DIR__ . '/Fixture/SkipAbstractCommand.php', []]; + yield [__DIR__ . '/Fixture/SkipNonCommand.php', []]; + } + + /** + * @return string[] + */ + #[Override] + public static function getAdditionalConfigFiles(): array + { + return [__DIR__ . '/config/configured_rule.neon']; + } + + protected function getRule(): Rule + { + return self::getContainer()->getByType(CommandMustHaveAsCommandAttributeRule::class); + } +} diff --git a/tests/Rules/Symfony/CommandMustHaveAsCommandAttributeRule/Fixture/ReportCommand.php b/tests/Rules/Symfony/CommandMustHaveAsCommandAttributeRule/Fixture/ReportCommand.php new file mode 100644 index 000000000..6e3d87596 --- /dev/null +++ b/tests/Rules/Symfony/CommandMustHaveAsCommandAttributeRule/Fixture/ReportCommand.php @@ -0,0 +1,11 @@ + + */ +final class ConstraintMustHaveAttributeRuleTest extends RuleTestCase +{ + /** + * @param list $expectedErrorsWithLines + */ + #[DataProvider('provideData')] + public function testRule(string $filePath, array $expectedErrorsWithLines): void + { + $this->analyse([$filePath], $expectedErrorsWithLines); + } + + /** + * @return Iterator, mixed>> + */ + public static function provideData(): Iterator + { + $errorMessage = sprintf( + ConstraintMustHaveAttributeRule::ERROR_MESSAGE, + ReportConstraint::class + ); + yield [__DIR__ . '/Fixture/ReportConstraint.php', [[$errorMessage, 9]]]; + + yield [__DIR__ . '/Fixture/SkipConstraintWithAttribute.php', []]; + yield [__DIR__ . '/Fixture/SkipAbstractConstraint.php', []]; + yield [__DIR__ . '/Fixture/SkipNonConstraint.php', []]; + } + + /** + * @return string[] + */ + #[Override] + public static function getAdditionalConfigFiles(): array + { + return [__DIR__ . '/config/configured_rule.neon']; + } + + protected function getRule(): Rule + { + return self::getContainer()->getByType(ConstraintMustHaveAttributeRule::class); + } +} diff --git a/tests/Rules/Symfony/ConstraintMustHaveAttributeRule/Fixture/ReportConstraint.php b/tests/Rules/Symfony/ConstraintMustHaveAttributeRule/Fixture/ReportConstraint.php new file mode 100644 index 000000000..3e71a4f05 --- /dev/null +++ b/tests/Rules/Symfony/ConstraintMustHaveAttributeRule/Fixture/ReportConstraint.php @@ -0,0 +1,11 @@ + + */ +final class PreferInterfaceInConstructorRuleTest extends RuleTestCase +{ + /** + * @param list $expectedErrorsWithLines + */ + #[DataProvider('provideData')] + public function testRule(string $filePath, array $expectedErrorsWithLines): void + { + $this->analyse([$filePath], $expectedErrorsWithLines); + } + + /** + * @return Iterator, mixed>> + */ + public static function provideData(): Iterator + { + $errorMessage = sprintf( + PreferInterfaceInConstructorRule::ERROR_MESSAGE, + '$router', + Router::class, + RouterInterface::class + ); + yield [__DIR__ . '/Fixture/ReportConcreteRouter.php', [[$errorMessage, 12]]]; + + yield [__DIR__ . '/Fixture/SkipRouterInterface.php', []]; + yield [__DIR__ . '/Fixture/SkipProjectClass.php', []]; + } + + /** + * @return string[] + */ + #[Override] + public static function getAdditionalConfigFiles(): array + { + return [__DIR__ . '/config/configured_rule.neon']; + } + + protected function getRule(): Rule + { + return self::getContainer()->getByType(PreferInterfaceInConstructorRule::class); + } +} diff --git a/tests/Rules/Symfony/PreferInterfaceInConstructorRule/config/configured_rule.neon b/tests/Rules/Symfony/PreferInterfaceInConstructorRule/config/configured_rule.neon new file mode 100644 index 000000000..1021b9af3 --- /dev/null +++ b/tests/Rules/Symfony/PreferInterfaceInConstructorRule/config/configured_rule.neon @@ -0,0 +1,5 @@ +includes: + - ../../../../config/included_services.neon + +rules: + - Symplify\PHPStanRules\Rules\Symfony\PreferInterfaceInConstructorRule From ec9bd794d5cfb48dcbe5ceaa86ca6b3443b7abc6 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Mon, 21 Sep 2026 09:56:23 +0200 Subject: [PATCH 2/4] bump deps --- composer.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/composer.json b/composer.json index 9157cbb31..b3c109268 100644 --- a/composer.json +++ b/composer.json @@ -17,12 +17,12 @@ "illuminate/container": "^11.51", "symplify/easy-coding-standard": "^13.3", "tomasvotruba/class-leak": "^2.2", - "rector/rector": "^2.6", + "rector/rector": "^2.6.7", "phpstan/extension-installer": "^1.4", "tomasvotruba/unused-public": "^2.2", "tomasvotruba/type-coverage": "^2.3", "shipmonk/composer-dependency-analyser": "^1.8", - "rector/jack": "^1.0", + "rector/jack": "^1.1", "nette/neon": "^3.4" }, "autoload": { From 28bf95cf9f763276e1a27e946654c7af2dd13abb Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 21 Sep 2026 07:58:25 +0000 Subject: [PATCH 3/4] Use class constants in Symfony rules Co-authored-by: TomasVotruba <924196+TomasVotruba@users.noreply.github.com> --- src/Rules/Symfony/CommandMustHaveAsCommandAttributeRule.php | 3 ++- src/Rules/Symfony/ConstraintMustHaveAttributeRule.php | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Rules/Symfony/CommandMustHaveAsCommandAttributeRule.php b/src/Rules/Symfony/CommandMustHaveAsCommandAttributeRule.php index 62de33446..ef044e11f 100644 --- a/src/Rules/Symfony/CommandMustHaveAsCommandAttributeRule.php +++ b/src/Rules/Symfony/CommandMustHaveAsCommandAttributeRule.php @@ -12,6 +12,7 @@ use PHPStan\Rules\IdentifierRuleError; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; +use Symfony\Component\Console\Attribute\AsCommand; use Symfony\Component\Console\Command\Command; use Symplify\PHPStanRules\Enum\RuleIdentifier; @@ -24,7 +25,7 @@ { public const string ERROR_MESSAGE = 'Class "%s" extends Command but is missing the #[AsCommand] attribute'; - private const string AS_COMMAND_ATTRIBUTE = 'Symfony\Component\Console\Attribute\AsCommand'; + private const string AS_COMMAND_ATTRIBUTE = AsCommand::class; public function __construct( private ReflectionProvider $reflectionProvider, diff --git a/src/Rules/Symfony/ConstraintMustHaveAttributeRule.php b/src/Rules/Symfony/ConstraintMustHaveAttributeRule.php index 64b834342..f2211c4f1 100644 --- a/src/Rules/Symfony/ConstraintMustHaveAttributeRule.php +++ b/src/Rules/Symfony/ConstraintMustHaveAttributeRule.php @@ -13,6 +13,7 @@ use PHPStan\Rules\IdentifierRuleError; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; +use Symfony\Component\Validator\Constraint; use Symplify\PHPStanRules\Enum\RuleIdentifier; /** @@ -27,7 +28,7 @@ { public const string ERROR_MESSAGE = 'Class "%s" extends Constraint but is missing the #[\Attribute] attribute. Add it, so the constraint can be used as an attribute on properties, as Symfony convention'; - private const string CONSTRAINT_CLASS = 'Symfony\Component\Validator\Constraint'; + private const string CONSTRAINT_CLASS = Constraint::class; public function __construct( private ReflectionProvider $reflectionProvider, From 7b8d17ce28eee94d4cee890e91e5149070efe19e Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Mon, 21 Sep 2026 10:00:02 +0200 Subject: [PATCH 4/4] smisc --- rector.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rector.php b/rector.php index e2d96e3c1..cb3e8c8df 100644 --- a/rector.php +++ b/rector.php @@ -18,6 +18,8 @@ __DIR__ . '/src/Enum', __DIR__ . '/src/Testing/PHPUnitTestAnalyser.php', __DIR__ . '/src/Rules/NoEntityOutsideEntityNamespaceRule.php', + __DIR__ . '/src/Rules/Symfony/CommandMustHaveAsCommandAttributeRule.php', + __DIR__ . '/src/Rules/Symfony/ConstraintMustHaveAttributeRule.php', __DIR__ . '/tests/Naming/ClassToSuffixResolverTest.php', __DIR__ . '/src/Doctrine/DoctrineEntityDocumentAnalyser.php', ],