diff --git a/rules-tests/CodeQuality/Rector/Class_/GetRepositoryServiceLocatorToRepositoryServiceInjectionRector/Fixture/get_odm_repository_on_service.php.inc b/rules-tests/CodeQuality/Rector/Class_/GetRepositoryServiceLocatorToRepositoryServiceInjectionRector/Fixture/get_odm_repository_on_service.php.inc new file mode 100644 index 00000000..a51b43d6 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/Class_/GetRepositoryServiceLocatorToRepositoryServiceInjectionRector/Fixture/get_odm_repository_on_service.php.inc @@ -0,0 +1,40 @@ +getRepository(SomeEntityUsingService::class); + + return $someRepository->find(1); + } +} + +?> +----- +someEntityUsingServiceRepository; + + return $someRepository->find(1); + } +} + +?> diff --git a/rules-tests/CodeQuality/Rector/Class_/GetRepositoryServiceLocatorToRepositoryServiceInjectionRector/Fixture/get_repository_on_service.php.inc b/rules-tests/CodeQuality/Rector/Class_/GetRepositoryServiceLocatorToRepositoryServiceInjectionRector/Fixture/get_repository_on_service.php.inc new file mode 100644 index 00000000..5b01d27b --- /dev/null +++ b/rules-tests/CodeQuality/Rector/Class_/GetRepositoryServiceLocatorToRepositoryServiceInjectionRector/Fixture/get_repository_on_service.php.inc @@ -0,0 +1,40 @@ +getRepository(SomeEntityUsingORMService::class); + + return $someRepository->find(1); + } +} + +?> +----- +someEntityUsingORMServiceRepository; + + return $someRepository->find(1); + } +} + +?> diff --git a/rules-tests/CodeQuality/Rector/Class_/GetRepositoryServiceLocatorToRepositoryServiceInjectionRector/Fixture/skip_callable.php.inc b/rules-tests/CodeQuality/Rector/Class_/GetRepositoryServiceLocatorToRepositoryServiceInjectionRector/Fixture/skip_callable.php.inc new file mode 100644 index 00000000..b9145a7c --- /dev/null +++ b/rules-tests/CodeQuality/Rector/Class_/GetRepositoryServiceLocatorToRepositoryServiceInjectionRector/Fixture/skip_callable.php.inc @@ -0,0 +1,13 @@ +getRepository(...); + } +} diff --git a/rules-tests/CodeQuality/Rector/Class_/GetRepositoryServiceLocatorToRepositoryServiceInjectionRector/Fixture/skip_inside_repository.php.inc b/rules-tests/CodeQuality/Rector/Class_/GetRepositoryServiceLocatorToRepositoryServiceInjectionRector/Fixture/skip_inside_repository.php.inc new file mode 100644 index 00000000..7b89c3ae --- /dev/null +++ b/rules-tests/CodeQuality/Rector/Class_/GetRepositoryServiceLocatorToRepositoryServiceInjectionRector/Fixture/skip_inside_repository.php.inc @@ -0,0 +1,15 @@ +getRepository(SomeEntityUsingService::class); + } +} diff --git a/rules-tests/CodeQuality/Rector/Class_/GetRepositoryServiceLocatorToRepositoryServiceInjectionRector/GetRepositoryServiceLocatorToRepositoryServiceInjectionRectorTest.php b/rules-tests/CodeQuality/Rector/Class_/GetRepositoryServiceLocatorToRepositoryServiceInjectionRector/GetRepositoryServiceLocatorToRepositoryServiceInjectionRectorTest.php new file mode 100644 index 00000000..3246bcd4 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/Class_/GetRepositoryServiceLocatorToRepositoryServiceInjectionRector/GetRepositoryServiceLocatorToRepositoryServiceInjectionRectorTest.php @@ -0,0 +1,28 @@ +doTestFile($filePath); + } + + public static function provideData(): Iterator + { + return self::yieldFilesFromDirectory(__DIR__ . '/Fixture'); + } + + public function provideConfigFilePath(): string + { + return __DIR__ . '/config/configured_rule.php'; + } +} diff --git a/rules-tests/CodeQuality/Rector/Class_/GetRepositoryServiceLocatorToRepositoryServiceInjectionRector/Source/Entity/SomeEntityUsingORMService.php b/rules-tests/CodeQuality/Rector/Class_/GetRepositoryServiceLocatorToRepositoryServiceInjectionRector/Source/Entity/SomeEntityUsingORMService.php new file mode 100644 index 00000000..088c0aa6 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/Class_/GetRepositoryServiceLocatorToRepositoryServiceInjectionRector/Source/Entity/SomeEntityUsingORMService.php @@ -0,0 +1,11 @@ +rule(GetRepositoryServiceLocatorToRepositoryServiceInjectionRector::class); +}; diff --git a/rules/CodeQuality/Enum/DoctrineClass.php b/rules/CodeQuality/Enum/DoctrineClass.php index 5e746987..0132802e 100644 --- a/rules/CodeQuality/Enum/DoctrineClass.php +++ b/rules/CodeQuality/Enum/DoctrineClass.php @@ -10,4 +10,24 @@ * @var string */ public const COLLECTION = 'Doctrine\Common\Collections\Collection'; + + /** + * @var string + */ + public const SERVICE_DOCUMENT_REPOSITORY = 'Doctrine\Bundle\MongoDBBundle\Repository\ServiceDocumentRepository'; + + /** + * @var string + */ + public const SERVICE_ENTITY_REPOSITORY = 'Doctrine\Bundle\DoctrineBundle\Repository\ServiceEntityRepository'; + + /** + * @var string + */ + public const ENTITY_REPOSITORY = 'Doctrine\ORM\EntityRepository'; + + /** + * @var string + */ + public const OBJECT_REPOSITORY = 'Doctrine\Persistence\ObjectRepository'; } diff --git a/rules/CodeQuality/Rector/Class_/AddReturnDocBlockToCollectionPropertyGetterByToManyAnnotationRector.php b/rules/CodeQuality/Rector/Class_/AddReturnDocBlockToCollectionPropertyGetterByToManyAnnotationRector.php index 12006f35..02c5781e 100644 --- a/rules/CodeQuality/Rector/Class_/AddReturnDocBlockToCollectionPropertyGetterByToManyAnnotationRector.php +++ b/rules/CodeQuality/Rector/Class_/AddReturnDocBlockToCollectionPropertyGetterByToManyAnnotationRector.php @@ -44,6 +44,7 @@ public function getRuleDefinition(): RuleDefinition new CodeSample( <<<'CODE_SAMPLE' use App\Entity\Training; +use Doctrine\ORM\Mapping as ORM; /** * @ORM\Entity @@ -63,6 +64,8 @@ public function getTrainings() CODE_SAMPLE , <<<'CODE_SAMPLE' +use Doctrine\ORM\Mapping as ORM; + /** * @ORM\Entity */ diff --git a/rules/CodeQuality/Rector/Class_/GetRepositoryServiceLocatorToRepositoryServiceInjectionRector.php b/rules/CodeQuality/Rector/Class_/GetRepositoryServiceLocatorToRepositoryServiceInjectionRector.php new file mode 100644 index 00000000..a78ef7ab --- /dev/null +++ b/rules/CodeQuality/Rector/Class_/GetRepositoryServiceLocatorToRepositoryServiceInjectionRector.php @@ -0,0 +1,211 @@ +entityManager->getRepository(...) on entity that suppors service repository, to constructor injection', + [ + new CodeSample( + <<<'CODE_SAMPLE' +use Doctrine\ORM\Mapping as ORM; + +/** + * @ORM\Entity(repositoryClass="SomeRepository") + */ +class SomeEntity +{ +} + +use Doctrine\Bundle\MongoDBBundle\Repository\ServiceDocumentRepository; + +final class SomeRepository extends ServiceDocumentRepository +{ +} + +final class SomeClass +{ + public function run() + { + return $this->getRepository(SomeEntity::class)->find(1); + } +} + +CODE_SAMPLE + , + <<<'CODE_SAMPLE' +final class SomeClass +{ + public function __construct( + private ServiceDocumentRepository $someEntityRepository + ) { + } + + public function run() + { + return $this->someEntityRepository->find(1); + } +} +CODE_SAMPLE + ), + + ] + ); + } + + public function getNodeTypes(): array + { + return [Class_::class]; + } + + /** + * @param Class_ $node + */ + public function refactor(Node $node): ?Class_ + { + if ($this->shouldSkipClass($node)) { + return null; + } + + $repositoryPropertyMetadatas = []; + + $this->traverseNodesWithCallable($node->stmts, function (Node $node) use ( + &$repositoryPropertyMetadatas + ): PropertyFetch|int|null { + if ($node instanceof Class_ || $node instanceof Function_) { + // avoid nested anonymous class or function + return NodeVisitor::DONT_TRAVERSE_CURRENT_AND_CHILDREN; + } + + if (! $node instanceof MethodCall) { + return null; + } + + if (! $this->isName($node->name, self::GET_REPOSITORY_METHOD)) { + return null; + } + + if ($node->isFirstClassCallable()) { + return null; + } + + if (count($node->getArgs()) !== 1) { + return null; + } + + $fetchedEntityValue = $node->getArgs()[0] + ->value; + + // must be constant value, not dynamic method call or variable + $entityClassName = $this->valueResolver->getValue($fetchedEntityValue); + if ($entityClassName === null) { + return null; + } + + $repositoryVariableName = $this->propertyNaming->fqnToVariableName($entityClassName) . 'Repository'; + + $repositoryClass = $this->repositoryClassResolver->resolveFromEntityClass($entityClassName); + + // unable to resolve + if (! is_string($repositoryClass)) { + return null; + } + + $repositoryClassReflection = $this->reflectionProvider->getClass($repositoryClass); + + if (! $repositoryClassReflection->isSubclassOf( + DoctrineClass::SERVICE_DOCUMENT_REPOSITORY + ) && ! $repositoryClassReflection->isSubclassOf(DoctrineClass::SERVICE_ENTITY_REPOSITORY)) { + return null; + } + + $repositoryPropertyMetadatas[] = new PropertyMetadata( + $repositoryVariableName, + new FullyQualifiedObjectType($repositoryClass) + ); + + return new PropertyFetch(new Variable('this'), new Identifier($repositoryVariableName)); + }); + + if ($repositoryPropertyMetadatas === []) { + return null; + } + + foreach ($repositoryPropertyMetadatas as $repositoryPropertyMetadata) { + $this->classDependencyManipulator->addConstructorDependency($node, $repositoryPropertyMetadata); + } + + return $node; + } + + private function shouldSkipClass(Class_ $class): bool + { + // keep it safe + if (! $class->isFinal() || $class->isAnonymous()) { + return true; + } + + $classScope = ScopeFetcher::fetch($class); + $classReflection = $classScope->getClassReflection(); + + if (! $classReflection instanceof ClassReflection) { + return true; + } + + // skip repositories themselves to avoid circular dependencies + if ($classReflection->isSubclassOf(DoctrineClass::OBJECT_REPOSITORY)) { + return true; + } + + if ($classReflection->isSubclassOf(DoctrineClass::ENTITY_REPOSITORY)) { + return true; + } + + return $classReflection->isSubclassOf(TestClass::BEHAT_CONTEXT); + } +} diff --git a/src/Enum/TestClass.php b/src/Enum/TestClass.php new file mode 100644 index 00000000..6b9230cf --- /dev/null +++ b/src/Enum/TestClass.php @@ -0,0 +1,13 @@ +findAttributeByClasses($node, $attributeClasses) instanceof Attribute; } + /** + * @param string[] $names + * @return Attribute[] + */ + public function findManyByClasses(ClassMethod|Property|Class_|Param $node, array $names): array + { + $attributes = []; + + foreach ($names as $name) { + $justFoundAttributes = $this->findManyByClass($node, $name); + $attributes = [...$attributes, ...$justFoundAttributes]; + } + + return $attributes; + } + private function findArgByName(Attribute $attribute, string $argName): Expr|null { foreach ($attribute->args as $arg) { @@ -166,20 +182,4 @@ private function findArgByName(Attribute $attribute, string $argName): Expr|null return null; } - - /** - * @param string[] $names - * @return Attribute[] - */ - public function findManyByClasses(ClassMethod|Property|Class_|Param $node, array $names): array - { - $attributes = []; - - foreach ($names as $name) { - $justFoundAttributes = $this->findManyByClass($node, $name); - $attributes = [...$attributes, ...$justFoundAttributes]; - } - - return $attributes; - } } diff --git a/src/NodeAnalyzer/RepositoryClassResolver.php b/src/NodeAnalyzer/RepositoryClassResolver.php new file mode 100644 index 00000000..fe3094d5 --- /dev/null +++ b/src/NodeAnalyzer/RepositoryClassResolver.php @@ -0,0 +1,67 @@ +.*?)\"#'; + + /** + * @var string + */ + private const USE_REPOSITORY_REGEX = '#use (?.*?Repository);#'; + + public function __construct( + private ReflectionProvider $reflectionProvider + ) { + } + + public function resolveFromEntityClass(string $entityClassName): ?string + { + if (! $this->reflectionProvider->hasClass($entityClassName)) { + throw new ShouldNotHappenException(); + } + + $classReflection = $this->reflectionProvider->getClass($entityClassName); + + $entityClassFileName = $classReflection->getFileName(); + if ($entityClassFileName === null) { + return null; + } + + $entityFileContents = FileSystem::read($entityClassFileName); + $repositoryClass = null; + + // match repositoryClass="..." in entity + $match = Strings::match($entityFileContents, self::QUOTED_REPOSITORY_CLASS_REGEX); + + if (! isset($match['repositoryClass'])) { + // try fallback to repository ::class + use import + + $repositoryUseMatch = Strings::match($entityFileContents, self::USE_REPOSITORY_REGEX); + if (isset($repositoryUseMatch['repositoryClass'])) { + $repositoryClass = $repositoryUseMatch['repositoryClass']; + } + } else { + $repositoryClass = $match['repositoryClass']; + } + + if (! $this->reflectionProvider->hasClass($repositoryClass)) { + throw new ShouldNotHappenException( + sprintf('Repository class "%s" for entity "%s" does not exist', $repositoryClass, $entityClassName) + ); + } + + return $repositoryClass; + } +} diff --git a/stubs/Common/Persistence/ObjectManager.php b/stubs/Common/Persistence/ObjectManager.php index 08b17e57..e323c8df 100644 --- a/stubs/Common/Persistence/ObjectManager.php +++ b/stubs/Common/Persistence/ObjectManager.php @@ -10,5 +10,5 @@ interface ObjectManager { - public function getRepository(): \Doctrine\ORM\EntityRepository; + public function getRepository(string $entityClassName): \Doctrine\ORM\EntityRepository; } diff --git a/stubs/Doctrine/Bundle/MongoDBBundle/Repository/ServiceDocumentRepository.php b/stubs/Doctrine/Bundle/MongoDBBundle/Repository/ServiceDocumentRepository.php new file mode 100644 index 00000000..40a43a71 --- /dev/null +++ b/stubs/Doctrine/Bundle/MongoDBBundle/Repository/ServiceDocumentRepository.php @@ -0,0 +1,11 @@ +