diff --git a/composer.json b/composer.json index 2717231..710dd71 100644 --- a/composer.json +++ b/composer.json @@ -52,7 +52,7 @@ "psalm --find-unused-psalm-suppress", "phpstan analyse", "@style-check", - "phpmd src ansi codesize,controversial,design,naming,unusedcode,design", + "phpmd src ansi codesize,controversial,naming,unusedcode", "phpmd tests ansi codesize,controversial,design" ], "style-check": "php-cs-fixer fix -v --config=.php-cs-fixer.dist.php --using-cache=no --dry-run", diff --git a/phpstan.neon b/phpstan.neon index 34be61c..706d54b 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -7,6 +7,7 @@ parameters: excludePaths: analyseAndScan: - tests/data/* + - tests/dataAttrib/* rules: - \Ebln\PHPStan\EnforceFactory\ForceFactoryRule diff --git a/src/ForceFactoryRule.php b/src/ForceFactoryRule.php index 6cfa650..620c8cf 100644 --- a/src/ForceFactoryRule.php +++ b/src/ForceFactoryRule.php @@ -75,7 +75,6 @@ public function processNode(Node $node, Scope $scope): array private function getAllowedFactories(string $className): ?array { $allowedFactories = $this->getFactoriesFromAttribute($className); - if (is_a($className, ForceFactoryInterface::class, true)) { /* phpstan-var class-string $className */ $interfaceFactories = $className::getFactories(); @@ -100,21 +99,45 @@ private function getFactoriesFromAttribute(string $className): ?array if (\PHP_VERSION_ID < 80000 && $this->reflectionProvider->hasClass($className)) { return null; } - /** @psalm-suppress UndefinedDocblockClass */ - $reflection = $this->reflectionProvider->getClass($className)->getNativeReflection(); + + $reflection = $this->reflectionProvider->getClass($className); + /* psalm-suppress UndefinedClass */ + $allowedFactories = []; + do { + /** @psalm-suppress UndefinedClass */ + $allowedFactories = [...$allowedFactories, ...$this->getFactoriesFromAttributeByClass($reflection->getNativeReflection())]; + } while ($reflection = $reflection->getParentClass()); + + if (empty($allowedFactories)) { + return null; + } + $allowedFactories = array_filter($allowedFactories); + sort($allowedFactories); + + return $allowedFactories; + } + + /** + * @psalm-suppress UndefinedDocblockClass,MismatchingDocblockParamType + * + * @psalm-param \PHPStan\BetterReflection\Reflection\Adapter\ReflectionClass|\PHPStan\BetterReflection\Reflection\Adapter\ReflectionEnum $reflection + * + * @return array + */ + private function getFactoriesFromAttributeByClass(\ReflectionClass $reflection): array + { /** @psalm-suppress UndefinedClass */ foreach ($reflection->getAttributes() as $attribute) { if (ForceFactory::class === $attribute->getName()) { /** @var ForceFactory $forceFactory */ $forceFactory = $attribute->newInstance(); $allowedFactories = $forceFactory->getAllowedFactories(); - sort($allowedFactories); - return $allowedFactories; + return empty($allowedFactories) ? [null] : $allowedFactories; } } - return null; + return []; } /** diff --git a/tests/AttribForceFactoryRuleTest.php b/tests/AttribForceFactoryRuleTest.php new file mode 100644 index 0000000..018a56e --- /dev/null +++ b/tests/AttribForceFactoryRuleTest.php @@ -0,0 +1,73 @@ += 8.0 + * @extends RuleTestCase + */ +class AttribForceFactoryRuleTest extends RuleTestCase +{ + private const ERROR_MESSAGE = 'Test\Ebln\PHPStan\EnforceFactory\dataAttrib\code\ForcedFactoryProduct must be instantiated by Test\Ebln\PHPStan\EnforceFactory\dataAttrib\ForcedFactory or Test\Ebln\PHPStan\EnforceFactory\dataAttrib\TraitFactory!'; + + // Sadly this remains a vector, as phpstan fails to infer the created class name + public function testLoopholeFactory(): void + { + $this->analyse([__DIR__ . '/dataAttrib/LoopholeFactory.php'], []); + } + + public function testEmptyAllowedClasses(): void + { + $this->analyse([__DIR__ . '/dataAttrib/EmptyFactory.php'], [ + [ + 'Test\Ebln\PHPStan\EnforceFactory\dataAttrib\code\EmptyProduct has either no factories defined or a conflict between interface and attribute!', + 13, + ], + ]); + } + + public function testRogueFactory(): void + { + $this->analyse([__DIR__ . '/dataAttrib/RogueFactory.php'], [ + [self::ERROR_MESSAGE, 15], + [self::ERROR_MESSAGE, 22], + [self::ERROR_MESSAGE, 29], + [self::ERROR_MESSAGE, 40], + [self::ERROR_MESSAGE, 40], + [self::ERROR_MESSAGE, 51], + [self::ERROR_MESSAGE, 56], + ['Test\Ebln\PHPStan\EnforceFactory\dataAttrib\code\ExtendedProduct must be instantiated by Test\Ebln\PHPStan\EnforceFactory\dataAttrib\ForcedFactory or Test\Ebln\PHPStan\EnforceFactory\dataAttrib\TraitFactory!', 69], + [self::ERROR_MESSAGE, 95], + ]); + } + + public function testRogueFactoryAndTrait(): void + { + $this->analyse([__DIR__ . '/dataAttrib/RogueTraitFactory.php', __DIR__ . '/dataAttrib/FactoryTrait.php'], [ + [self::ERROR_MESSAGE, 13], + [self::ERROR_MESSAGE, 20], + [self::ERROR_MESSAGE, 27], + ]); + } + + public function testTraitedFactory(): void + { + $this->analyse([__DIR__ . '/dataAttrib/TraitFactory.php', __DIR__ . '/dataAttrib/FactoryTrait.php'], []); + } + + public function testAllowedFactory(): void + { + $this->analyse([__DIR__ . '/dataAttrib/ForcedFactory.php'], []); + } + + protected function getRule(): Rule + { + return new ForceFactoryRule($this->createReflectionProvider()); + } +} diff --git a/tests/dataAttrib/EmptyFactory.php b/tests/dataAttrib/EmptyFactory.php new file mode 100644 index 0000000..1ccd65b --- /dev/null +++ b/tests/dataAttrib/EmptyFactory.php @@ -0,0 +1,15 @@ +foo(); + } + + public function anonymousExtendingSquare(): void + { + $x = new class() extends ExtendedProduct + { + public function foo(): string + { + return 'bar'; + } + }; + + $bar = $x->foo(); + } + + public function anonymousPassing(): void + { + $x = new class() + { + public function foo(): string + { + return 'bar'; + } + }; + + $bar = $x->foo(); + } + + public static function staticClass(): ForcedFactoryProduct + { + return new ForcedFactoryProduct(); + } +} diff --git a/tests/dataAttrib/LoopholeFactory.php b/tests/dataAttrib/LoopholeFactory.php new file mode 100644 index 0000000..13b707f --- /dev/null +++ b/tests/dataAttrib/LoopholeFactory.php @@ -0,0 +1,19 @@ +foo(); + } + + public function anonymousExtendingSquare(): void + { + $x = new class() extends ExtendedProduct + { + public function foo(): string + { + return 'bar'; + } + }; + + $bar = $x->foo(); + } + + public function anonymousPassing(): void + { + $x = new class() + { + public function foo(): string + { + return 'bar'; + } + }; + + $bar = $x->foo(); + } + + public static function staticClass(): ForcedFactoryProduct + { + return new ForcedFactoryProduct(); + } +} diff --git a/tests/dataAttrib/RogueTraitFactory.php b/tests/dataAttrib/RogueTraitFactory.php new file mode 100644 index 0000000..083ea54 --- /dev/null +++ b/tests/dataAttrib/RogueTraitFactory.php @@ -0,0 +1,10 @@ +