From d12c2f9eb227ad7e34b43d0dbec33c97bddf9d71 Mon Sep 17 00:00:00 2001 From: Simon Marx Date: Sun, 4 Jul 2021 10:33:57 +0200 Subject: [PATCH 1/9] add attribute support --- .../SetDoctrineAnnotatedPrefixesPass.php | 7 +- src/Doctrine/DoctrineHelper.php | 20 +++ src/Doctrine/EntityClassGenerator.php | 3 +- src/Maker/MakeEntity.php | 26 ++-- .../skeleton/doctrine/Entity.tpl.php | 16 ++- src/Util/ClassSourceManipulator.php | 128 ++++++++++++++++-- 6 files changed, 172 insertions(+), 28 deletions(-) diff --git a/src/DependencyInjection/CompilerPass/SetDoctrineAnnotatedPrefixesPass.php b/src/DependencyInjection/CompilerPass/SetDoctrineAnnotatedPrefixesPass.php index f51fc944f..b3133a84c 100644 --- a/src/DependencyInjection/CompilerPass/SetDoctrineAnnotatedPrefixesPass.php +++ b/src/DependencyInjection/CompilerPass/SetDoctrineAnnotatedPrefixesPass.php @@ -41,10 +41,6 @@ public function process(ContainerBuilder $container) $methodCalls = $metadataDriverImpl->getMethodCalls(); foreach ($methodCalls as $i => [$method, $arguments]) { - if ('addDriver' !== $method) { - continue; - } - if ($arguments[0] instanceof Definition) { $class = $arguments[0]->getClass(); $namespace = substr($class, 0, strrpos($class, '\\')); @@ -59,10 +55,9 @@ public function process(ContainerBuilder $container) $methodCalls[$i] = $arguments; } - $isAnnotated = false !== strpos($arguments[0], '_annotation_metadata_driver'); $annotatedPrefixes[$managerName][] = [ $arguments[1], - $isAnnotated ? new Reference($arguments[0]) : null, + new Reference($arguments[0]), ]; } diff --git a/src/Doctrine/DoctrineHelper.php b/src/Doctrine/DoctrineHelper.php index 4e0aab143..51344a29c 100644 --- a/src/Doctrine/DoctrineHelper.php +++ b/src/Doctrine/DoctrineHelper.php @@ -23,6 +23,7 @@ use Doctrine\Persistence\Mapping\AbstractClassMetadataFactory; use Doctrine\Persistence\Mapping\ClassMetadata; use Doctrine\Persistence\Mapping\Driver\AnnotationDriver; +use Doctrine\Persistence\Mapping\Driver\MappingDriver; use Doctrine\Persistence\Mapping\Driver\MappingDriverChain; use Doctrine\Persistence\Mapping\MappingException as PersistenceMappingException; use Symfony\Bundle\MakerBundle\Util\ClassNameDetails; @@ -85,6 +86,7 @@ public function getEntityNamespace(): string return $this->entityNamespace; } + // check is not needed anymore public function isClassAnnotated(string $className): bool { /** @var EntityManagerInterface $em */ @@ -268,4 +270,22 @@ public function isKeyword(string $name): bool return $connection->getDatabasePlatform()->getReservedKeywordsList()->isKeyword($name); } + + public function getMappingDriverForNamespace(string $namespace): ?MappingDriver + { + $lowestCharacterDiff = null; + $foundDriver = null; + + foreach ($this->annotatedPrefixes as $key => $mappings) { + foreach($mappings as [$prefix, $driver]) { + $diff = substr_compare($namespace, $prefix, 0); + + if (null === $lowestCharacterDiff || $diff < $lowestCharacterDiff) { + $foundDriver = $driver; + } + } + } + + return $foundDriver; + } } diff --git a/src/Doctrine/EntityClassGenerator.php b/src/Doctrine/EntityClassGenerator.php index 20644ec3c..a37062607 100644 --- a/src/Doctrine/EntityClassGenerator.php +++ b/src/Doctrine/EntityClassGenerator.php @@ -33,7 +33,7 @@ public function __construct(Generator $generator, DoctrineHelper $doctrineHelper $this->doctrineHelper = $doctrineHelper; } - public function generateEntityClass(ClassNameDetails $entityClassDetails, bool $apiResource, bool $withPasswordUpgrade = false, bool $generateRepositoryClass = true, bool $broadcast = false): string + public function generateEntityClass(ClassNameDetails $entityClassDetails, bool $apiResource, bool $withPasswordUpgrade = false, bool $generateRepositoryClass = true, bool $broadcast = false, bool $doctrineAttributes = false): string { $repoClassDetails = $this->generator->createClassNameDetails( $entityClassDetails->getRelativeName(), @@ -53,6 +53,7 @@ public function generateEntityClass(ClassNameDetails $entityClassDetails, bool $ 'broadcast' => $broadcast, 'should_escape_table_name' => $this->doctrineHelper->isKeyword($tableName), 'table_name' => $tableName, + 'doctrine_use_attributes' => $doctrineAttributes ] ); diff --git a/src/Maker/MakeEntity.php b/src/Maker/MakeEntity.php index b880c63ff..1d2315eee 100644 --- a/src/Maker/MakeEntity.php +++ b/src/Maker/MakeEntity.php @@ -13,6 +13,9 @@ use ApiPlatform\Core\Annotation\ApiResource; use Doctrine\DBAL\Types\Type; +use Doctrine\ORM\Mapping\Driver\AttributeDriver; +use Doctrine\Persistence\Mapping\Driver\AnnotationDriver; +use Doctrine\Persistence\Mapping\Driver\MappingDriver; use Symfony\Bundle\MakerBundle\ConsoleStyle; use Symfony\Bundle\MakerBundle\DependencyBuilder; use Symfony\Bundle\MakerBundle\Doctrine\DoctrineHelper; @@ -160,6 +163,7 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen 'Entity\\' ); + $mappingDriver = $this->doctrineHelper->getMappingDriverForNamespace($entityClassDetails->getFullName()); $classExists = class_exists($entityClassDetails->getFullName()); if (!$classExists) { $broadcast = $input->getOption('broadcast'); @@ -168,7 +172,8 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen $input->getOption('api-resource'), false, true, - $broadcast + $broadcast, + $mappingDriver instanceof AttributeDriver ); if ($broadcast) { @@ -186,10 +191,6 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen $generator->writeChanges(); } - if (!$this->doesEntityUseAnnotationMapping($entityClassDetails->getFullName())) { - throw new RuntimeCommandException(sprintf('Only annotation mapping is supported by make:entity, but the %s class uses a different format. If you would like this command to generate the properties & getter/setter methods, add your mapping configuration, and then re-run this command with the --regenerate flag.', $entityClassDetails->getFullName())); - } - if ($classExists) { $entityPath = $this->getPathOfClass($entityClassDetails->getFullName()); $io->text([ @@ -204,7 +205,7 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen } $currentFields = $this->getPropertyNames($entityClassDetails->getFullName()); - $manipulator = $this->createClassManipulator($entityPath, $io, $overwrite); + $manipulator = $this->createClassManipulator($entityPath, $io, $overwrite, $mappingDriver); $isFirstField = true; while (true) { @@ -232,7 +233,7 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen $otherManipulator = $manipulator; } else { $otherManipulatorFilename = $this->getPathOfClass($newField->getInverseClass()); - $otherManipulator = $this->createClassManipulator($otherManipulatorFilename, $io, $overwrite); + $otherManipulator = $this->createClassManipulator($otherManipulatorFilename, $io, $overwrite, $mappingDriver); } switch ($newField->getType()) { case EntityRelation::MANY_TO_ONE: @@ -247,7 +248,7 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen // the new field being added to THIS entity is the inverse $newFieldName = $newField->getInverseProperty(); $otherManipulatorFilename = $this->getPathOfClass($newField->getOwningClass()); - $otherManipulator = $this->createClassManipulator($otherManipulatorFilename, $io, $overwrite); + $otherManipulator = $this->createClassManipulator($otherManipulatorFilename, $io, $overwrite, $mappingDriver); // The *other* class will receive the ManyToOne $otherManipulator->addManyToOneRelation($newField->getOwningRelation()); @@ -791,9 +792,13 @@ private function askRelationType(ConsoleStyle $io, string $entityClass, string $ return $io->askQuestion($question); } - private function createClassManipulator(string $path, ConsoleStyle $io, bool $overwrite): ClassSourceManipulator + private function createClassManipulator(string $path, ConsoleStyle $io, bool $overwrite, ?MappingDriver $mappingDriver = null): ClassSourceManipulator { - $manipulator = new ClassSourceManipulator($this->fileManager->getFileContents($path), $overwrite); + $useAnnotations = null === $mappingDriver || $mappingDriver instanceof AnnotationDriver && !$mappingDriver instanceof AttributeDriver; + $useAttributes = $mappingDriver instanceof AttributeDriver; + + $manipulator = new ClassSourceManipulator($this->fileManager->getFileContents($path), $overwrite, $useAnnotations, true, $useAttributes); + $manipulator->setIo($io); return $manipulator; @@ -832,6 +837,7 @@ private function getPropertyNames(string $class): array }, $reflClass->getProperties()); } + // not needed anymore private function doesEntityUseAnnotationMapping(string $className): bool { if (!class_exists($className)) { diff --git a/src/Resources/skeleton/doctrine/Entity.tpl.php b/src/Resources/skeleton/doctrine/Entity.tpl.php index 9c013803e..c56a04940 100644 --- a/src/Resources/skeleton/doctrine/Entity.tpl.php +++ b/src/Resources/skeleton/doctrine/Entity.tpl.php @@ -9,29 +9,39 @@ use Symfony\UX\Turbo\Attribute\Broadcast; + /** * @ApiResource() * @Broadcast() - * @ORM\Entity(repositoryClass=::class) + * @ORM\Entity(repositoryClass=::class) + * @ORM\Table(name="``") */ + #[ApiResource] #[Broadcast] + +#[ORM\Entity(repositoryClass: ::class)] + #[ORM\Table(name: '``')] + class { - /** + /** * @ORM\Id * @ORM\GeneratedValue * @ORM\Column(type="integer") */ - private $id; + #[ORM\Id] + #[ORM\GeneratedValue] + #[ORM\Column(type: 'integer')] + private $id; public function getId(): ?int { diff --git a/src/Util/ClassSourceManipulator.php b/src/Util/ClassSourceManipulator.php index 52c670c7d..713f484c8 100644 --- a/src/Util/ClassSourceManipulator.php +++ b/src/Util/ClassSourceManipulator.php @@ -41,6 +41,7 @@ final class ClassSourceManipulator private $overwrite; private $useAnnotations; + private $useAttributes; private $fluentMutators; private $parser; private $lexer; @@ -55,7 +56,7 @@ final class ClassSourceManipulator private $pendingComments = []; - public function __construct(string $sourceCode, bool $overwrite = false, bool $useAnnotations = true, bool $fluentMutators = true) + public function __construct(string $sourceCode, bool $overwrite = false, bool $useAnnotations = true, bool $fluentMutators = true, bool $useAttributes = false) { $this->overwrite = $overwrite; $this->useAnnotations = $useAnnotations; @@ -71,6 +72,7 @@ public function __construct(string $sourceCode, bool $overwrite = false, bool $u $this->printer = new PrettyPrinter(); $this->setSourceCode($sourceCode); + $this->useAttributes = $useAttributes; } public function setIo(ConsoleStyle $io) @@ -83,18 +85,23 @@ public function getSourceCode(): string return $this->sourceCode; } - public function addEntityField(string $propertyName, array $columnOptions, array $comments = []) + public function addEntityField(string $propertyName, array $columnOptions, array $comments = [], array $attributes = []) { $typeHint = $this->getEntityTypeHint($columnOptions['type']); $nullable = $columnOptions['nullable'] ?? false; $isId = (bool) ($columnOptions['id'] ?? false); - $comments[] = $this->buildAnnotationLine('@ORM\Column', $columnOptions); + if ($this->useAnnotations) { + $comments[] = $this->buildAnnotationLine('@ORM\Column', $columnOptions); + } elseif ($this->useAttributes) { + $attributes[] = $this->buildAttributeNode('ORM\Column', $columnOptions); + } + $defaultValue = null; if ('array' === $typeHint) { $defaultValue = new Node\Expr\Array_([], ['kind' => Node\Expr\Array_::KIND_SHORT]); } - $this->addProperty($propertyName, $comments, $defaultValue); + $this->addProperty($propertyName, $comments, $defaultValue, $attributes); $this->addGetter( $propertyName, @@ -123,7 +130,16 @@ public function addEmbeddedEntity(string $propertyName, string $className) ), ]; - $this->addProperty($propertyName, $annotations); + $attributes = [ + $this->buildAttributeNode( + 'ORM\\Embedded', + [ + 'class' => new ClassNameValue($className, $typeHint) + ] + ) + ]; + + $this->addProperty($propertyName, $annotations, null, $attributes); // logic to avoid re-adding the same ArrayCollection line $addEmbedded = true; @@ -311,7 +327,7 @@ public function createMethodLevelBlankLine() return $this->createBlankLineNode(self::CONTEXT_CLASS_METHOD); } - public function addProperty(string $name, array $annotationLines = [], $defaultValue = null) + public function addProperty(string $name, array $annotationLines = [], $defaultValue = null, array $attributes = []) { if ($this->propertyExists($name)) { // we never overwrite properties @@ -323,6 +339,12 @@ public function addProperty(string $name, array $annotationLines = [], $defaultV $newPropertyBuilder->setDocComment($this->createDocBlock($annotationLines)); } + if ($attributes && $this->useAttributes) { + foreach ($attributes as $attribute) { + $newPropertyBuilder->addAttribute($attribute); + } + } + if (null !== $defaultValue) { $newPropertyBuilder->setDefault($defaultValue); } @@ -502,13 +524,23 @@ private function addSingularRelation(BaseRelation $relation) ), ]; + $attributes = [ + $this->buildAttributeNode( + $relation instanceof RelationManyToOne ? 'ORM\\ManyToOne' : 'ORM\\OneToOne', + $annotationOptions + ) + ]; + if (!$relation->isNullable() && $relation->isOwning()) { $annotations[] = $this->buildAnnotationLine('@ORM\\JoinColumn', [ 'nullable' => false, ]); + $attributes[] = $this->buildAttributeNode('ORM\\JoinColumn', [ + 'nullable' => false + ]); } - $this->addProperty($relation->getPropertyName(), $annotations); + $this->addProperty($relation->getPropertyName(), $annotations, null, $attributes); $this->addGetter( $relation->getPropertyName(), @@ -579,8 +611,14 @@ private function addCollectionRelation(BaseCollectionRelation $relation) $annotationOptions ), ]; + $attributes = [ + $this->buildAttributeNode( + $relation instanceof RelationManyToMany ? 'ORM\\ManyToMany' : 'ORM\\OneToMany', + $annotationOptions + ) + ]; - $this->addProperty($relation->getPropertyName(), $annotations); + $this->addProperty($relation->getPropertyName(), $annotations, null, $attributes); // logic to avoid re-adding the same ArrayCollection line $addArrayCollection = true; @@ -1326,4 +1364,78 @@ private function addMethodParams(Builder\Method $methodBuilder, array $params) $methodBuilder->addParam($param); } } + + private function buildNodeExprByValue($value): Node\Expr + { + switch (gettype($value)) { + case 'string': + $nodeValue = new Node\Scalar\String_($value); + break; + case 'integer': + $nodeValue = new Node\Scalar\LNumber($value); + break; + case 'double': + $nodeValue = new Node\Scalar\DNumber($value); + break; + case 'boolean': + $nodeValue = new Node\Expr\ConstFetch(new Node\Name($value ? 'true' : 'false')); + break; + case 'array': + $context = $this; + $arrayItems = array_map(static function ($key, $value) use ($context) { + return new Node\Expr\ArrayItem( + $context->buildNodeExprByValue($value), + !is_int($key) ? $context->buildNodeExprByValue($key) : null + ); + }, array_keys($value), array_values($value)); + $nodeValue = new Node\Expr\Array_($arrayItems, ['kind' => Node\Expr\Array_::KIND_SHORT]); + break; + default: + $nodeValue = null; + } + + if ($nodeValue === null) { + if ($value instanceof ClassNameValue) { + $nodeValue = new Node\Expr\ConstFetch(new Node\Name(\sprintf('%s::class', $value->getShortName()))); + } + } + + return $nodeValue; + } + + private function buildAttributeNode(string $attributeClass, array $options) + { + $options = $this->sortOptionsByClassConstructorParameters($options, $attributeClass); + + $context = $this; + $nodeArguments = \array_map(static function ($option, $value) use ($context) { + return new Node\Arg($context->buildNodeExprByValue($value), false, false, [], new Node\Identifier($option)); + }, array_keys($options), array_values($options)); + + return new Node\Attribute( + new Node\Name($attributeClass), + $nodeArguments + ); + } + + private function sortOptionsByClassConstructorParameters(array $options, string $classString): array + { + if (substr($classString, 0, 4) === 'ORM\\') { + $classString = sprintf('Doctrine\\ORM\\Mapping\\%s', substr($classString, 4)); + } + + $constructorParameterNames = array_map(static function (\ReflectionParameter $reflectionParameter) { + return $reflectionParameter->getName(); + }, (new \ReflectionClass($classString))->getConstructor()->getParameters()); + + $sorted = []; + foreach ($constructorParameterNames as $name) { + if (array_key_exists($name, $options)) { + $sorted[$name] = $options[$name]; + unset($options[$name]); + } + } + + return array_merge($sorted, $options); + } } From dd4cf984561c9fd73a546136f12293ec3d4cf183 Mon Sep 17 00:00:00 2001 From: Simon Marx Date: Sun, 4 Jul 2021 10:51:59 +0200 Subject: [PATCH 2/9] add exception when expr node build failed --- src/Util/ClassSourceManipulator.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Util/ClassSourceManipulator.php b/src/Util/ClassSourceManipulator.php index 713f484c8..186f517ce 100644 --- a/src/Util/ClassSourceManipulator.php +++ b/src/Util/ClassSourceManipulator.php @@ -1397,6 +1397,8 @@ private function buildNodeExprByValue($value): Node\Expr if ($nodeValue === null) { if ($value instanceof ClassNameValue) { $nodeValue = new Node\Expr\ConstFetch(new Node\Name(\sprintf('%s::class', $value->getShortName()))); + } else { + throw new \Exception(sprintf('Cannot build a node expr for value of type "%s"', gettype($value))); } } From ede1cd2ff108f3a86fcc2e1df226a0edf1f3ee48 Mon Sep 17 00:00:00 2001 From: Simon Marx Date: Sun, 4 Jul 2021 11:03:23 +0200 Subject: [PATCH 3/9] add accidantily removed addDriver check --- .../CompilerPass/SetDoctrineAnnotatedPrefixesPass.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/DependencyInjection/CompilerPass/SetDoctrineAnnotatedPrefixesPass.php b/src/DependencyInjection/CompilerPass/SetDoctrineAnnotatedPrefixesPass.php index b3133a84c..e62104ad9 100644 --- a/src/DependencyInjection/CompilerPass/SetDoctrineAnnotatedPrefixesPass.php +++ b/src/DependencyInjection/CompilerPass/SetDoctrineAnnotatedPrefixesPass.php @@ -41,6 +41,10 @@ public function process(ContainerBuilder $container) $methodCalls = $metadataDriverImpl->getMethodCalls(); foreach ($methodCalls as $i => [$method, $arguments]) { + if ('addDriver' !== $method) { + continue; + } + if ($arguments[0] instanceof Definition) { $class = $arguments[0]->getClass(); $namespace = substr($class, 0, strrpos($class, '\\')); From f8b3faa14e52012505d38ecc4c308958728a1cd1 Mon Sep 17 00:00:00 2001 From: Simon Marx Date: Sun, 4 Jul 2021 12:34:12 +0200 Subject: [PATCH 4/9] add doc blocks --- src/Util/ClassSourceManipulator.php | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/Util/ClassSourceManipulator.php b/src/Util/ClassSourceManipulator.php index 186f517ce..91c00d253 100644 --- a/src/Util/ClassSourceManipulator.php +++ b/src/Util/ClassSourceManipulator.php @@ -1365,6 +1365,13 @@ private function addMethodParams(Builder\Method $methodBuilder, array $params) } } + /** + * builds a PHPParser Expr Node based on the value given in $value + * throws an Exception when the given $value is not resolvable by this method + * + * @param mixed $value + * @throws \Exception + */ private function buildNodeExprByValue($value): Node\Expr { switch (gettype($value)) { @@ -1405,6 +1412,12 @@ private function buildNodeExprByValue($value): Node\Expr return $nodeValue; } + /** + * builds an PHPParser attribute node + * + * @param string $attributeClass the attribute class which should be used for the attribute + * @param array $options the named arguments for the attribute ($key = argument name, $value = argument value) + */ private function buildAttributeNode(string $attributeClass, array $options) { $options = $this->sortOptionsByClassConstructorParameters($options, $attributeClass); @@ -1420,6 +1433,12 @@ private function buildAttributeNode(string $attributeClass, array $options) ); } + /** + * sort the given options based on the constructor parameters for the given $classString + * this prevents code inspections warnings for IDEs like intellij/phpstorm + * + * option keys that are not found in the constructor will be added at the end of the sorted array + */ private function sortOptionsByClassConstructorParameters(array $options, string $classString): array { if (substr($classString, 0, 4) === 'ORM\\') { From b9f701bfb1000d8fa96896b344e59486e4410607 Mon Sep 17 00:00:00 2001 From: Simon Marx Date: Sun, 4 Jul 2021 12:52:53 +0200 Subject: [PATCH 5/9] remove unused private method, remove unecessary doc blocks --- src/Doctrine/DoctrineHelper.php | 10 +++++++--- src/Maker/MakeEntity.php | 17 ----------------- 2 files changed, 7 insertions(+), 20 deletions(-) diff --git a/src/Doctrine/DoctrineHelper.php b/src/Doctrine/DoctrineHelper.php index 51344a29c..4ac05d9a8 100644 --- a/src/Doctrine/DoctrineHelper.php +++ b/src/Doctrine/DoctrineHelper.php @@ -86,7 +86,6 @@ public function getEntityNamespace(): string return $this->entityNamespace; } - // check is not needed anymore public function isClassAnnotated(string $className): bool { /** @var EntityManagerInterface $em */ @@ -235,7 +234,7 @@ public function isClassAMappedEntity(string $className): bool return false; } - return (bool) $this->getMetadata($className); + return (bool)$this->getMetadata($className); } private function isInstanceOf($object, string $class): bool @@ -271,13 +270,18 @@ public function isKeyword(string $name): bool return $connection->getDatabasePlatform()->getReservedKeywordsList()->isKeyword($name); } + /** + * this method try to find the correct MappingDriver for the given namespace/class + * To determine which MappingDriver belongs to the class we check the prefixes configured in Doctrine and use the + * prefix that has the closest match to the given $namespace + */ public function getMappingDriverForNamespace(string $namespace): ?MappingDriver { $lowestCharacterDiff = null; $foundDriver = null; foreach ($this->annotatedPrefixes as $key => $mappings) { - foreach($mappings as [$prefix, $driver]) { + foreach ($mappings as [$prefix, $driver]) { $diff = substr_compare($namespace, $prefix, 0); if (null === $lowestCharacterDiff || $diff < $lowestCharacterDiff) { diff --git a/src/Maker/MakeEntity.php b/src/Maker/MakeEntity.php index 1d2315eee..e511070b0 100644 --- a/src/Maker/MakeEntity.php +++ b/src/Maker/MakeEntity.php @@ -837,23 +837,6 @@ private function getPropertyNames(string $class): array }, $reflClass->getProperties()); } - // not needed anymore - private function doesEntityUseAnnotationMapping(string $className): bool - { - if (!class_exists($className)) { - $otherClassMetadatas = $this->doctrineHelper->getMetadata(Str::getNamespace($className).'\\', true); - - // if we have no metadata, we should assume this is the first class being mapped - if (empty($otherClassMetadatas)) { - return false; - } - - $className = reset($otherClassMetadatas)->getName(); - } - - return $this->doctrineHelper->isClassAnnotated($className); - } - private function getEntityNamespace(): string { return $this->doctrineHelper->getEntityNamespace(); From 32bec7f665346919a7a4dab57810f9b1a54088de Mon Sep 17 00:00:00 2001 From: Simon Marx Date: Mon, 5 Jul 2021 06:39:03 +0200 Subject: [PATCH 6/9] cs fixer run --- src/Doctrine/DoctrineHelper.php | 4 +-- src/Doctrine/EntityClassGenerator.php | 2 +- src/Maker/MakeEntity.php | 1 - src/Util/ClassSourceManipulator.php | 37 ++++++++++++++------------- 4 files changed, 22 insertions(+), 22 deletions(-) diff --git a/src/Doctrine/DoctrineHelper.php b/src/Doctrine/DoctrineHelper.php index 4ac05d9a8..fa1c32759 100644 --- a/src/Doctrine/DoctrineHelper.php +++ b/src/Doctrine/DoctrineHelper.php @@ -234,7 +234,7 @@ public function isClassAMappedEntity(string $className): bool return false; } - return (bool)$this->getMetadata($className); + return (bool) $this->getMetadata($className); } private function isInstanceOf($object, string $class): bool @@ -273,7 +273,7 @@ public function isKeyword(string $name): bool /** * this method try to find the correct MappingDriver for the given namespace/class * To determine which MappingDriver belongs to the class we check the prefixes configured in Doctrine and use the - * prefix that has the closest match to the given $namespace + * prefix that has the closest match to the given $namespace. */ public function getMappingDriverForNamespace(string $namespace): ?MappingDriver { diff --git a/src/Doctrine/EntityClassGenerator.php b/src/Doctrine/EntityClassGenerator.php index a37062607..cbc3eeea1 100644 --- a/src/Doctrine/EntityClassGenerator.php +++ b/src/Doctrine/EntityClassGenerator.php @@ -53,7 +53,7 @@ public function generateEntityClass(ClassNameDetails $entityClassDetails, bool $ 'broadcast' => $broadcast, 'should_escape_table_name' => $this->doctrineHelper->isKeyword($tableName), 'table_name' => $tableName, - 'doctrine_use_attributes' => $doctrineAttributes + 'doctrine_use_attributes' => $doctrineAttributes, ] ); diff --git a/src/Maker/MakeEntity.php b/src/Maker/MakeEntity.php index e511070b0..c62665575 100644 --- a/src/Maker/MakeEntity.php +++ b/src/Maker/MakeEntity.php @@ -23,7 +23,6 @@ use Symfony\Bundle\MakerBundle\Doctrine\EntityRegenerator; use Symfony\Bundle\MakerBundle\Doctrine\EntityRelation; use Symfony\Bundle\MakerBundle\Doctrine\ORMDependencyBuilder; -use Symfony\Bundle\MakerBundle\Exception\RuntimeCommandException; use Symfony\Bundle\MakerBundle\FileManager; use Symfony\Bundle\MakerBundle\Generator; use Symfony\Bundle\MakerBundle\InputAwareMakerInterface; diff --git a/src/Util/ClassSourceManipulator.php b/src/Util/ClassSourceManipulator.php index 91c00d253..d8615ea5d 100644 --- a/src/Util/ClassSourceManipulator.php +++ b/src/Util/ClassSourceManipulator.php @@ -134,9 +134,9 @@ public function addEmbeddedEntity(string $propertyName, string $className) $this->buildAttributeNode( 'ORM\\Embedded', [ - 'class' => new ClassNameValue($className, $typeHint) + 'class' => new ClassNameValue($className, $typeHint), ] - ) + ), ]; $this->addProperty($propertyName, $annotations, null, $attributes); @@ -528,7 +528,7 @@ private function addSingularRelation(BaseRelation $relation) $this->buildAttributeNode( $relation instanceof RelationManyToOne ? 'ORM\\ManyToOne' : 'ORM\\OneToOne', $annotationOptions - ) + ), ]; if (!$relation->isNullable() && $relation->isOwning()) { @@ -536,7 +536,7 @@ private function addSingularRelation(BaseRelation $relation) 'nullable' => false, ]); $attributes[] = $this->buildAttributeNode('ORM\\JoinColumn', [ - 'nullable' => false + 'nullable' => false, ]); } @@ -615,7 +615,7 @@ private function addCollectionRelation(BaseCollectionRelation $relation) $this->buildAttributeNode( $relation instanceof RelationManyToMany ? 'ORM\\ManyToMany' : 'ORM\\OneToMany', $annotationOptions - ) + ), ]; $this->addProperty($relation->getPropertyName(), $annotations, null, $attributes); @@ -1367,14 +1367,15 @@ private function addMethodParams(Builder\Method $methodBuilder, array $params) /** * builds a PHPParser Expr Node based on the value given in $value - * throws an Exception when the given $value is not resolvable by this method + * throws an Exception when the given $value is not resolvable by this method. * * @param mixed $value + * * @throws \Exception */ private function buildNodeExprByValue($value): Node\Expr { - switch (gettype($value)) { + switch (\gettype($value)) { case 'string': $nodeValue = new Node\Scalar\String_($value); break; @@ -1392,7 +1393,7 @@ private function buildNodeExprByValue($value): Node\Expr $arrayItems = array_map(static function ($key, $value) use ($context) { return new Node\Expr\ArrayItem( $context->buildNodeExprByValue($value), - !is_int($key) ? $context->buildNodeExprByValue($key) : null + !\is_int($key) ? $context->buildNodeExprByValue($key) : null ); }, array_keys($value), array_values($value)); $nodeValue = new Node\Expr\Array_($arrayItems, ['kind' => Node\Expr\Array_::KIND_SHORT]); @@ -1401,11 +1402,11 @@ private function buildNodeExprByValue($value): Node\Expr $nodeValue = null; } - if ($nodeValue === null) { + if (null === $nodeValue) { if ($value instanceof ClassNameValue) { - $nodeValue = new Node\Expr\ConstFetch(new Node\Name(\sprintf('%s::class', $value->getShortName()))); + $nodeValue = new Node\Expr\ConstFetch(new Node\Name(sprintf('%s::class', $value->getShortName()))); } else { - throw new \Exception(sprintf('Cannot build a node expr for value of type "%s"', gettype($value))); + throw new \Exception(sprintf('Cannot build a node expr for value of type "%s"', \gettype($value))); } } @@ -1413,17 +1414,17 @@ private function buildNodeExprByValue($value): Node\Expr } /** - * builds an PHPParser attribute node + * builds an PHPParser attribute node. * - * @param string $attributeClass the attribute class which should be used for the attribute - * @param array $options the named arguments for the attribute ($key = argument name, $value = argument value) + * @param string $attributeClass the attribute class which should be used for the attribute + * @param array $options the named arguments for the attribute ($key = argument name, $value = argument value) */ private function buildAttributeNode(string $attributeClass, array $options) { $options = $this->sortOptionsByClassConstructorParameters($options, $attributeClass); $context = $this; - $nodeArguments = \array_map(static function ($option, $value) use ($context) { + $nodeArguments = array_map(static function ($option, $value) use ($context) { return new Node\Arg($context->buildNodeExprByValue($value), false, false, [], new Node\Identifier($option)); }, array_keys($options), array_values($options)); @@ -1435,13 +1436,13 @@ private function buildAttributeNode(string $attributeClass, array $options) /** * sort the given options based on the constructor parameters for the given $classString - * this prevents code inspections warnings for IDEs like intellij/phpstorm + * this prevents code inspections warnings for IDEs like intellij/phpstorm. * * option keys that are not found in the constructor will be added at the end of the sorted array */ private function sortOptionsByClassConstructorParameters(array $options, string $classString): array { - if (substr($classString, 0, 4) === 'ORM\\') { + if ('ORM\\' === substr($classString, 0, 4)) { $classString = sprintf('Doctrine\\ORM\\Mapping\\%s', substr($classString, 4)); } @@ -1451,7 +1452,7 @@ private function sortOptionsByClassConstructorParameters(array $options, string $sorted = []; foreach ($constructorParameterNames as $name) { - if (array_key_exists($name, $options)) { + if (\array_key_exists($name, $options)) { $sorted[$name] = $options[$name]; unset($options[$name]); } From 44cf99dca7a21546b51de829dea24a5e4da16a19 Mon Sep 17 00:00:00 2001 From: Morgan ABRAHAM Date: Wed, 14 Jul 2021 00:11:42 +0200 Subject: [PATCH 7/9] Added attribute mapping detection --- src/Doctrine/DoctrineHelper.php | 38 ++++++++++++++++++++++++++++ src/Maker/MakeEntity.php | 44 +++++++++++++++++++++++++++++++++ tests/Maker/MakeEntityTest.php | 4 +-- 3 files changed, 84 insertions(+), 2 deletions(-) diff --git a/src/Doctrine/DoctrineHelper.php b/src/Doctrine/DoctrineHelper.php index fa1c32759..04857c0ee 100644 --- a/src/Doctrine/DoctrineHelper.php +++ b/src/Doctrine/DoctrineHelper.php @@ -16,6 +16,7 @@ use Doctrine\Common\Persistence\Mapping\MappingException as LegacyPersistenceMappingException; use Doctrine\DBAL\Connection; use Doctrine\ORM\EntityManagerInterface; +use Doctrine\ORM\Mapping\Driver\AttributeDriver; use Doctrine\ORM\Mapping\MappingException as ORMMappingException; use Doctrine\ORM\Mapping\NamingStrategy; use Doctrine\ORM\Tools\DisconnectedClassMetadataFactory; @@ -123,6 +124,43 @@ public function isClassAnnotated(string $className): bool return false; } + public function doesClassUsesAttributes(string $className): bool + { + /** @var EntityManagerInterface $em */ + $em = $this->getRegistry()->getManagerForClass($className); + + if (null === $em) { + throw new \InvalidArgumentException(sprintf('Cannot find the entity manager for class "%s"', $className)); + } + + if (null === $this->annotatedPrefixes) { + // doctrine-bundle <= 2.2 + $metadataDriver = $em->getConfiguration()->getMetadataDriverImpl(); + + if (!$this->isInstanceOf($metadataDriver, MappingDriverChain::class)) { + return $metadataDriver instanceof AttributeDriver; + } + + foreach ($metadataDriver->getDrivers() as $namespace => $driver) { + if (0 === strpos($className, $namespace)) { + return $driver instanceof AttributeDriver; + } + } + + return $metadataDriver->getDefaultDriver() instanceof AttributeDriver; + } + + $managerName = array_search($em, $this->getRegistry()->getManagers(), true); + + foreach ($this->annotatedPrefixes[$managerName] as [$prefix, $attributeDriver]) { + if (0 === strpos($className, $prefix)) { + return null !== $attributeDriver; + } + } + + return false; + } + public function getEntitiesForAutocomplete(): array { $entities = []; diff --git a/src/Maker/MakeEntity.php b/src/Maker/MakeEntity.php index c62665575..0b3f0cd7a 100644 --- a/src/Maker/MakeEntity.php +++ b/src/Maker/MakeEntity.php @@ -23,6 +23,7 @@ use Symfony\Bundle\MakerBundle\Doctrine\EntityRegenerator; use Symfony\Bundle\MakerBundle\Doctrine\EntityRelation; use Symfony\Bundle\MakerBundle\Doctrine\ORMDependencyBuilder; +use Symfony\Bundle\MakerBundle\Exception\RuntimeCommandException; use Symfony\Bundle\MakerBundle\FileManager; use Symfony\Bundle\MakerBundle\Generator; use Symfony\Bundle\MakerBundle\InputAwareMakerInterface; @@ -190,6 +191,13 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen $generator->writeChanges(); } + if ( + !$this->doesEntityUseAnnotationMapping($entityClassDetails->getFullName()) + xor !$this->doesEntityUseAttributeMapping($entityClassDetails->getFullName()) + ) { + throw new RuntimeCommandException(sprintf('Only annotation or attribute mapping is supported by make:entity, but the %s class uses a different format. If you would like this command to generate the properties & getter/setter methods, add your mapping configuration, and then re-run this command with the --regenerate flag.', $entityClassDetails->getFullName())); + } + if ($classExists) { $entityPath = $this->getPathOfClass($entityClassDetails->getFullName()); $io->text([ @@ -836,6 +844,42 @@ private function getPropertyNames(string $class): array }, $reflClass->getProperties()); } + private function doesEntityUseAnnotationMapping(string $className): bool + { + if (!class_exists($className)) { + $otherClassMetadatas = $this->doctrineHelper->getMetadata(Str::getNamespace($className) . '\\', true); + + // if we have no metadata, we should assume this is the first class being mapped + if (empty($otherClassMetadatas)) { + return false; + } + + $className = reset($otherClassMetadatas)->getName(); + } + + return $this->doctrineHelper->isClassAnnotated($className); + } + + private function doesEntityUseAttributeMapping(string $className): bool + { + if (PHP_VERSION < 80000) { + return false; + } + + if (!class_exists($className)) { + $otherClassMetadatas = $this->doctrineHelper->getMetadata(Str::getNamespace($className) . '\\', true); + + // if we have no metadata, we should assume this is the first class being mapped + if (empty($otherClassMetadatas)) { + return false; + } + + $className = reset($otherClassMetadatas)->getName(); + } + + return $this->doctrineHelper->doesClassUsesAttributes($className); + } + private function getEntityNamespace(): string { return $this->doctrineHelper->getEntityNamespace(); diff --git a/tests/Maker/MakeEntityTest.php b/tests/Maker/MakeEntityTest.php index bc040a2d7..ebcd199f7 100644 --- a/tests/Maker/MakeEntityTest.php +++ b/tests/Maker/MakeEntityTest.php @@ -506,7 +506,7 @@ public function getTestDetails() ->configureDatabase(false) ->setCommandAllowedToFail(true) ->assert(function (string $output, string $directory) { - $this->assertStringContainsString('Only annotation mapping is supported', $output); + $this->assertStringContainsString('Only annotation or attribute mapping is supported', $output); }), ]; @@ -529,7 +529,7 @@ public function getTestDetails() ->configureDatabase(false) ->setCommandAllowedToFail(true) ->assert(function (string $output, string $directory) { - $this->assertStringContainsString('Only annotation mapping is supported', $output); + $this->assertStringContainsString('Only annotation or attribute mapping is supported', $output); }), ]; From 1f30ddc9c2b34a02c4a7a0b831c885f9cefc9099 Mon Sep 17 00:00:00 2001 From: Morgan ABRAHAM Date: Wed, 14 Jul 2021 00:21:45 +0200 Subject: [PATCH 8/9] CS fix --- src/Maker/MakeEntity.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Maker/MakeEntity.php b/src/Maker/MakeEntity.php index 0b3f0cd7a..d1030abed 100644 --- a/src/Maker/MakeEntity.php +++ b/src/Maker/MakeEntity.php @@ -847,7 +847,7 @@ private function getPropertyNames(string $class): array private function doesEntityUseAnnotationMapping(string $className): bool { if (!class_exists($className)) { - $otherClassMetadatas = $this->doctrineHelper->getMetadata(Str::getNamespace($className) . '\\', true); + $otherClassMetadatas = $this->doctrineHelper->getMetadata(Str::getNamespace($className).'\\', true); // if we have no metadata, we should assume this is the first class being mapped if (empty($otherClassMetadatas)) { @@ -862,12 +862,12 @@ private function doesEntityUseAnnotationMapping(string $className): bool private function doesEntityUseAttributeMapping(string $className): bool { - if (PHP_VERSION < 80000) { + if (\PHP_VERSION < 80000) { return false; } if (!class_exists($className)) { - $otherClassMetadatas = $this->doctrineHelper->getMetadata(Str::getNamespace($className) . '\\', true); + $otherClassMetadatas = $this->doctrineHelper->getMetadata(Str::getNamespace($className).'\\', true); // if we have no metadata, we should assume this is the first class being mapped if (empty($otherClassMetadatas)) { From 8cd167c0cd75edd95a60710f68d254d47709a912 Mon Sep 17 00:00:00 2001 From: Morgan ABRAHAM Date: Wed, 14 Jul 2021 09:39:23 +0200 Subject: [PATCH 9/9] Mapping type check condition operator fix --- src/Maker/MakeEntity.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Maker/MakeEntity.php b/src/Maker/MakeEntity.php index d1030abed..ff5339d16 100644 --- a/src/Maker/MakeEntity.php +++ b/src/Maker/MakeEntity.php @@ -193,7 +193,7 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen if ( !$this->doesEntityUseAnnotationMapping($entityClassDetails->getFullName()) - xor !$this->doesEntityUseAttributeMapping($entityClassDetails->getFullName()) + && !$this->doesEntityUseAttributeMapping($entityClassDetails->getFullName()) ) { throw new RuntimeCommandException(sprintf('Only annotation or attribute mapping is supported by make:entity, but the %s class uses a different format. If you would like this command to generate the properties & getter/setter methods, add your mapping configuration, and then re-run this command with the --regenerate flag.', $entityClassDetails->getFullName())); }