From f935c5560744f79c7873335c475bf13b1e206212 Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Wed, 24 Oct 2018 21:11:04 +0200 Subject: [PATCH 01/60] Add new make:dto command This command is added as a supplement to make:form. To prevent problems with invalid state of entities when used in forms, data transfer objects are a solution. This command generates these from existing entities. It can generate getters/setters and helper methods to fill and extract data from/to entities. Closes #162 --- src/Maker/MakeDTO.php | 248 ++++++++ src/Resources/config/makers.xml | 6 + src/Resources/help/MakeDTO.txt | 10 + src/Resources/skeleton/dto/Data.tpl.php | 81 +++ src/Util/DTOClassSourceManipulator.php | 776 ++++++++++++++++++++++++ 5 files changed, 1121 insertions(+) create mode 100644 src/Maker/MakeDTO.php create mode 100644 src/Resources/help/MakeDTO.txt create mode 100644 src/Resources/skeleton/dto/Data.tpl.php create mode 100644 src/Util/DTOClassSourceManipulator.php diff --git a/src/Maker/MakeDTO.php b/src/Maker/MakeDTO.php new file mode 100644 index 000000000..3dc205917 --- /dev/null +++ b/src/Maker/MakeDTO.php @@ -0,0 +1,248 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Bundle\MakerBundle\Maker; + +use Symfony\Bundle\MakerBundle\ConsoleStyle; +use Symfony\Bundle\MakerBundle\DependencyBuilder; +use Symfony\Bundle\MakerBundle\Doctrine\DoctrineHelper; +use Symfony\Bundle\MakerBundle\Generator; +use Symfony\Bundle\MakerBundle\InputConfiguration; +use Symfony\Bundle\MakerBundle\Str; +use Symfony\Bundle\MakerBundle\Validator; +use Symfony\Component\Console\Command\Command; +use Symfony\Component\Console\Input\InputArgument; +use Symfony\Component\Console\Input\InputOption; +use Symfony\Component\Console\Input\InputInterface; +use Symfony\Component\Console\Question\Question; +use Symfony\Component\Validator\Validation; +use Symfony\Bundle\MakerBundle\Maker\AbstractMaker; +use Symfony\Bundle\MakerBundle\FileManager; +use Symfony\Bundle\MakerBundle\Util\DTOClassSourceManipulator; +use Doctrine\ORM\Mapping\ClassMetadata; +use Doctrine\Common\Annotations\AnnotationReader; +use Symfony\Component\Validator\Constraint; + +/** + * @author Javier Eguiluz + * @author Ryan Weaver + * @author Clemens Krack + */ +final class MakeDTO extends AbstractMaker +{ + private $entityHelper; + private $fileManager; + + public function __construct( + DoctrineHelper $entityHelper, + FileManager $fileManager + ) { + $this->entityHelper = $entityHelper; + $this->fileManager = $fileManager; + } + + public static function getCommandName(): string + { + return 'make:dto'; + } + + public function configureCommand(Command $command, InputConfiguration $inputConf) + { + $command + ->setDescription('Creates a new DTO class') + ->addArgument('name', InputArgument::REQUIRED, sprintf('The name of the DTO class (e.g. %sData)', Str::asClassName(Str::getRandomTerm()))) + ->addArgument('bound-class', InputArgument::REQUIRED, 'The name of Entity that the DTO will be bound to') + ->setHelp(file_get_contents(__DIR__.'/../Resources/help/MakeDTO.txt')) + ; + + $inputConf->setArgumentAsNonInteractive('bound-class'); + } + + public function interact(InputInterface $input, ConsoleStyle $io, Command $command) + { + if (null === $input->getArgument('bound-class')) { + $argument = $command->getDefinition()->getArgument('bound-class'); + + $entities = $this->entityHelper->getEntitiesForAutocomplete(); + + $question = new Question($argument->getDescription()); + $question->setValidator(function ($answer) use ($entities) {return Validator::existsOrNull($answer, $entities); }); + $question->setAutocompleterValues($entities); + $question->setMaxAttempts(3); + + $input->setArgument('bound-class', $io->askQuestion($question)); + } + } + + public function generate(InputInterface $input, ConsoleStyle $io, Generator $generator) + { + $formClassNameDetails = $generator->createClassNameDetails( + $input->getArgument('name'), + 'Form\\', + 'Data' + ); + + $boundClass = $input->getArgument('bound-class'); + + $boundClassDetails = $generator->createClassNameDetails( + $boundClass, + 'Entity\\' + ); + + // get some doctrine details + $doctrineEntityDetails = $this->entityHelper->createDoctrineDetails($boundClassDetails->getFullName()); + + // get class metadata (used by regenerate) + $metaData = $this->entityHelper->getMetadata($boundClassDetails->getFullName()); + + // list of fields + $fields = $metaData->fieldMappings; + + if (null !== $doctrineEntityDetails) { + $formFields = $doctrineEntityDetails->getFormFields(); + } + + $boundClassVars = [ + 'bounded_full_class_name' => $boundClassDetails->getFullName(), + 'bounded_class_name' => $boundClassDetails->getShortName(), + ]; + + // the result is passed to the template + $addHelpers = $io->confirm('Add helper extract/fill methods?'); + + // Generate getters/setters + $addGettersSetters = $io->confirm('Generate getters/setters?'); + + // filter id from fields? + $omitId = $io->confirm('Omit Id field in DTO?'); + + if ($omitId) { + $fields = array_filter($fields, function ($field) { + // mapping includes id field when property is an id + if (!empty($field['id'])) { + return false; + } + + return true; + }); + } + + // Skeleton? + + $DTOClassPath = $generator->generateClass( + $formClassNameDetails->getFullName(), + __DIR__.'/../Resources/skeleton/dto/Data.tpl.php', + array_merge( + [ + 'fields' => $fields, + 'addHelpers' => $addHelpers, + 'addGettersSetters' => $addGettersSetters + ], + $boundClassVars + ) + ); + + $generator->writeChanges(); + $manipulator = $this->createClassManipulator($DTOClassPath, $addGettersSetters); + $mappedFields = $this->getMappedFieldsInEntity($metaData); + + // Did we import assert annotations? + $assertionsImported = false; + + foreach ($fields as $fieldName => $mapping) { + if (!\in_array($fieldName, $mappedFields)) { + continue; + } + + $annotationReader = new AnnotationReader(); + + // Lookup classname for inherited properties + if (array_key_exists('declared', $mapping)) { + $fullClassName = $mapping['declared']; + } else { + $fullClassName = $boundClassDetails->getFullName(); + } + + // Property Annotations + $reflectionProperty = new \ReflectionProperty($fullClassName, $fieldName); + $propertyAnnotations = $annotationReader->getPropertyAnnotations($reflectionProperty); + + $comments = []; + + foreach ($propertyAnnotations as $annotation) { + // we want to copy the asserts, so look for their interface + if($annotation instanceof Constraint) { + $assertionsImported = true; + $comments[] = $manipulator->buildAnnotationLine('@Assert\\'.(new \ReflectionClass($annotation))->getShortName(), (array) $annotation); + } + } + + $manipulator->addEntityField($fieldName, $mapping, $comments); + } + + $this->fileManager->dumpFile( + $DTOClassPath, + $manipulator->getSourceCode() + ); + + $this->writeSuccessMessage($io); + + if (true === $assertionsImported) { + $io->note([ + 'The maker imported assertion annotations.', + 'Consider removing them from the entity or make sure to keep them updated in both places.', + ]); + } + + $io->text([ + 'Next: Create your form with this DTO and start using it.', + 'Find the documentation at https://symfony.com/doc/current/forms.html', + ]); + } + + public function configureDependencies(DependencyBuilder $dependencies) + { + $dependencies->addClassDependency( + Validation::class, + 'validator', + // add as an optional dependency: the user *probably* wants validation + false + ); + } + + private function createClassManipulator(string $classPath, bool $addGettersSetters = true): DTOClassSourceManipulator + { + return new DTOClassSourceManipulator( + $this->fileManager->getFileContents($classPath), + // overwrite existing methods + true, + // use annotations + true, + // use fluent mutators + true, + // add getters setters? + $addGettersSetters + ); + } + + private function getMappedFieldsInEntity(ClassMetadata $classMetadata) + { + /* @var $classReflection \ReflectionClass */ + $classReflection = $classMetadata->reflClass; + + $targetFields = array_merge( + array_keys($classMetadata->fieldMappings), + array_keys($classMetadata->associationMappings) + ); + + return $targetFields; + } +} diff --git a/src/Resources/config/makers.xml b/src/Resources/config/makers.xml index f591b7fcb..29d9aa6ce 100644 --- a/src/Resources/config/makers.xml +++ b/src/Resources/config/makers.xml @@ -7,6 +7,12 @@ + + + + + + diff --git a/src/Resources/help/MakeDTO.txt b/src/Resources/help/MakeDTO.txt new file mode 100644 index 000000000..b61dc7546 --- /dev/null +++ b/src/Resources/help/MakeDTO.txt @@ -0,0 +1,10 @@ +The %command.name% command generates a new data transfer object class. + +php %command.full_name% Name Entity + +If the argument is missing, the command will ask for the classname and entity interactively. + +The command will further ask for: + - the generation of helper methods to extract/fill data from entities into DTO + - the generation of getters and setters for the properties + - whether the id field should be omitted in the DTO diff --git a/src/Resources/skeleton/dto/Data.tpl.php b/src/Resources/skeleton/dto/Data.tpl.php new file mode 100644 index 000000000..c1e95eac7 --- /dev/null +++ b/src/Resources/skeleton/dto/Data.tpl.php @@ -0,0 +1,81 @@ + + + +namespace ; + + +use ; + +use Symfony\Component\Validator\Constraints as Assert; + +/** + * Data transfer object for . + * Add your constraints as annotations to the properties. + */ +class + +{ + + /** + * Create DTO, optionally extracting data from a model. + * + * @param |null $ + + */ + public function __construct(? $ = null) + { + if ($ instanceof ) { + $this->extract($); + } + } + + /** + * Fill entity with data from the DTO. + * + * @param $ + + */ + public function fill( $): + { + + + $ + + $mapping): ?> + ->set($this->get()) + + ; + + $ + + $mapping): ?> + ->set($this->) + + ; + + + return $; + } + + /** + * Extract data from entity into the DTO. + * + * @param $ + + */ + public function extract( $): self + { + + $mapping): ?> + $this->set($->get()); + + + $mapping): ?> + $this-> = $->get(); + + + + return $this; + } + +} diff --git a/src/Util/DTOClassSourceManipulator.php b/src/Util/DTOClassSourceManipulator.php new file mode 100644 index 000000000..6e8b8527f --- /dev/null +++ b/src/Util/DTOClassSourceManipulator.php @@ -0,0 +1,776 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Bundle\MakerBundle\Util; + +use PhpParser\BuilderHelpers; +use PhpParser\Lexer; +use PhpParser\Node; +use PhpParser\Parser; +use PhpParser\NodeTraverser; +use PhpParser\NodeVisitor; +use PhpParser\Builder; +use Symfony\Bundle\MakerBundle\ConsoleStyle; +use Symfony\Bundle\MakerBundle\Str; +use Symfony\Bundle\MakerBundle\Util\PrettyPrinter; + +/** + * @internal + */ +final class DTOClassSourceManipulator +{ + const CONTEXT_OUTSIDE_CLASS = 'outside_class'; + const CONTEXT_CLASS = 'class'; + const CONTEXT_CLASS_METHOD = 'class_method'; + + private $overwrite; + private $useAnnotations; + private $fluentMutators; + private $parser; + private $lexer; + private $printer; + /** @var ConsoleStyle|null */ + private $io; + + private $sourceCode; + private $oldStmts; + private $oldTokens; + private $newStmts; + + private $pendingComments = []; + + public function __construct(string $sourceCode, bool $overwrite = false, bool $useAnnotations = true, bool $fluentMutators = true, bool $addGettersSetters = true) + { + $this->overwrite = $overwrite; + $this->useAnnotations = $useAnnotations; + $this->addGettersSetters = $addGettersSetters; + $this->fluentMutators = $fluentMutators; + $this->lexer = new Lexer\Emulative([ + 'usedAttributes' => [ + 'comments', + 'startLine', 'endLine', + 'startTokenPos', 'endTokenPos', + ], + ]); + $this->parser = new Parser\Php7($this->lexer); + $this->printer = new PrettyPrinter(); + + $this->setSourceCode($sourceCode); + } + + public function setIo(ConsoleStyle $io) + { + $this->io = $io; + } + + public function getSourceCode(): string + { + return $this->sourceCode; + } + + public function addEntityField(string $propertyName, array $columnOptions, array $comments = []) + { + $typeHint = $this->getEntityTypeHint($columnOptions['type']); + $nullable = $columnOptions['nullable'] ?? false; + $isId = (bool) ($columnOptions['id'] ?? false); + $defaultValue = null; + if ('array' === $typeHint) { + $defaultValue = new Node\Expr\Array_([], ['kind' => Node\Expr\Array_::KIND_SHORT]); + } + $this->addProperty($propertyName, $comments, $defaultValue); + + // return early when setters/getters should not be added. + if (false === $this->addGettersSetters) { + return; + } + + $this->addGetter( + $propertyName, + $typeHint, + // getter methods always have nullable return values + // because even though these are required in the db, they may not be set yet + true + ); + + // don't generate setters for id fields + if (!$isId) { + $this->addSetter($propertyName, $typeHint, $nullable); + } + } + + public function addAccessorMethod(string $propertyName, string $methodName, $returnType, bool $isReturnTypeNullable, array $commentLines = [], $typeCast = null) + { + $this->addCustomGetter($propertyName, $methodName, $returnType, $isReturnTypeNullable, $commentLines, $typeCast); + } + + public function addGetter(string $propertyName, $returnType, bool $isReturnTypeNullable, array $commentLines = []) + { + $methodName = 'get'.Str::asCamelCase($propertyName); + + $this->addCustomGetter($propertyName, $methodName, $returnType, $isReturnTypeNullable, $commentLines); + } + + public function addSetter(string $propertyName, $type, bool $isNullable, array $commentLines = []) + { + $builder = $this->createSetterNodeBuilder($propertyName, $type, $isNullable, $commentLines); + $this->makeMethodFluent($builder); + $this->addMethod($builder->getNode()); + } + + public function addMethodBuilder(Builder\Method $methodBuilder) + { + $this->addMethod($methodBuilder->getNode()); + } + + public function createMethodBuilder(string $methodName, $returnType, bool $isReturnTypeNullable, array $commentLines = []): Builder\Method + { + $methodNodeBuilder = (new Builder\Method($methodName)) + ->makePublic() + ; + + if (null !== $returnType) { + $methodNodeBuilder->setReturnType($isReturnTypeNullable ? new Node\NullableType($returnType) : $returnType); + } + + if ($commentLines) { + $methodNodeBuilder->setDocComment($this->createDocBlock($commentLines)); + } + + return $methodNodeBuilder; + } + + public function createMethodLevelCommentNode(string $comment) + { + return $this->createSingleLineCommentNode($comment, self::CONTEXT_CLASS_METHOD); + } + + public function createMethodLevelBlankLine() + { + return $this->createBlankLineNode(self::CONTEXT_CLASS_METHOD); + } + + public function addProperty(string $name, array $annotationLines = [], $defaultValue = null) + { + if ($this->propertyExists($name)) { + // we never overwrite properties + return; + } + $newPropertyBuilder = new Builder\Property($name); + + // if we do not add getters/setters, the fields must be public + if (false === $this->addGettersSetters) { + $newPropertyBuilder->makePublic(); + } else { + $newPropertyBuilder->makePrivate(); + } + + if ($annotationLines && $this->useAnnotations) { + $newPropertyBuilder->setDocComment($this->createDocBlock($annotationLines)); + } + + if (null !== $defaultValue) { + $newPropertyBuilder->setDefault($defaultValue); + } + $newPropertyNode = $newPropertyBuilder->getNode(); + + $this->addNodeAfterProperties($newPropertyNode); + } + + private function addCustomGetter(string $propertyName, string $methodName, $returnType, bool $isReturnTypeNullable, array $commentLines = [], $typeCast = null) + { + $propertyFetch = new Node\Expr\PropertyFetch(new Node\Expr\Variable('this'), $propertyName); + + if (null !== $typeCast) { + switch ($typeCast) { + case 'string': + $propertyFetch = new Node\Expr\Cast\String_($propertyFetch); + break; + default: + // implement other cases if/when the library needs them + throw new \Exception('Not implemented'); + } + } + + $getterNodeBuilder = (new Builder\Method($methodName)) + ->makePublic() + ->addStmt( + new Node\Stmt\Return_($propertyFetch) + ) + ; + + if (null !== $returnType) { + $getterNodeBuilder->setReturnType($isReturnTypeNullable ? new Node\NullableType($returnType) : $returnType); + } + + if ($commentLines) { + $getterNodeBuilder->setDocComment($this->createDocBlock($commentLines)); + } + + $this->addMethod($getterNodeBuilder->getNode()); + } + + private function createSetterNodeBuilder(string $propertyName, $type, bool $isNullable, array $commentLines = []) + { + $methodName = 'set'.Str::asCamelCase($propertyName); + $setterNodeBuilder = (new Builder\Method($methodName))->makePublic(); + + if ($commentLines) { + $setterNodeBuilder->setDocComment($this->createDocBlock($commentLines)); + } + + $paramBuilder = new Builder\Param($propertyName); + if (null !== $type) { + $paramBuilder->setTypeHint($isNullable ? new Node\NullableType($type) : $type); + } + $setterNodeBuilder->addParam($paramBuilder->getNode()); + + $setterNodeBuilder->addStmt( + new Node\Stmt\Expression(new Node\Expr\Assign( + new Node\Expr\PropertyFetch(new Node\Expr\Variable('this'), $propertyName), + new Node\Expr\Variable($propertyName) + )) + ); + + return $setterNodeBuilder; + } + + /** + * Modified to public from ClassSourceManipulator. + * + * @param string $annotationClass The annotation: e.g. "@ORM\Column" + * @param array $options Key-value pair of options for the annotation + * + * @return string + */ + public function buildAnnotationLine(string $annotationClass, array $options) + { + $formattedOptions = array_map(function ($option, $value) { + if (\is_array($value)) { + if (!isset($value[0])) { + return sprintf('%s={%s}', $option, implode(', ', array_map(function ($val, $key) { + return sprintf('"%s" = %s', $key, $this->quoteAnnotationValue($val)); + }, $value, array_keys($value)))); + } + + return sprintf('%s={%s}', $option, implode(', ', array_map(function ($val) { + return $this->quoteAnnotationValue($val); + }, $value))); + } + + return sprintf('%s=%s', $option, $this->quoteAnnotationValue($value)); + }, array_keys($options), array_values($options)); + + return sprintf('%s(%s)', $annotationClass, implode(', ', $formattedOptions)); + } + + private function quoteAnnotationValue($value) + { + if (\is_bool($value)) { + return $value ? 'true' : 'false'; + } + + if (null === $value) { + return 'null'; + } + + if (\is_int($value)) { + return $value; + } + + if (\is_array($value)) { + throw new \Exception('Invalid value: loop before quoting.'); + } + + return sprintf('"%s"', $value); + } + + private function addStatementToConstructor(Node\Stmt $stmt) + { + if (!$this->getConstructorNode()) { + $constructorNode = (new Builder\Method('__construct'))->makePublic()->getNode(); + + // add call to parent::__construct() if there is a need to + try { + $ref = new \ReflectionClass($this->getThisFullClassName()); + + if ($ref->getParentClass() && $ref->getParentClass()->getConstructor()) { + $constructorNode->stmts[] = new Node\Stmt\Expression( + new Node\Expr\StaticCall(new Node\Name('parent'), new Node\Identifier('__construct')) + ); + } + } catch (\ReflectionException $e) { + } + + $this->addNodeAfterProperties($constructorNode); + } + + $constructorNode = $this->getConstructorNode(); + $constructorNode->stmts[] = $stmt; + $this->updateSourceCodeFromNewStmts(); + } + + /** + * @return Node\Stmt\ClassMethod|null + * + * @throws \Exception + */ + private function getConstructorNode() + { + foreach ($this->getClassNode()->stmts as $classNode) { + if ($classNode instanceof Node\Stmt\ClassMethod && '__construct' == $classNode->name) { + return $classNode; + } + } + + return null; + } + + /** + * @param string $class + * + * @return string The alias to use when referencing this class + */ + private function addUseStatementIfNecessary(string $class): string + { + $shortClassName = Str::getShortClassName($class); + if ($this->isInSameNamespace($class)) { + return $shortClassName; + } + + $namespaceNode = $this->getNamespaceNode(); + + $targetIndex = null; + $addLineBreak = false; + $lastUseStmtIndex = null; + foreach ($namespaceNode->stmts as $index => $stmt) { + if ($stmt instanceof Node\Stmt\Use_) { + // I believe this is an array to account for use statements with {} + foreach ($stmt->uses as $use) { + $alias = $use->alias ? $use->alias->name : $use->name->getLast(); + + // the use statement already exists? Don't add it again + if ($class === (string) $use->name) { + return $alias; + } + + if ($alias === $shortClassName) { + // we have a conflicting alias! + // to be safe, use the fully-qualified class name + // everywhere and do not add another use statement + return '\\'.$class; + } + } + + // if $class is alphabetically before this use statement, place it before + // only set $targetIndex the first time you find it + if (null === $targetIndex && Str::areClassesAlphabetical($class, (string) $stmt->uses[0]->name)) { + $targetIndex = $index; + } + + $lastUseStmtIndex = $index; + } elseif ($stmt instanceof Node\Stmt\Class_) { + if (null !== $targetIndex) { + // we already found where to place the use statement + + break; + } + + // we hit the class! If there were any use statements, + // then put this at the bottom of the use statement list + if (null !== $lastUseStmtIndex) { + $targetIndex = $lastUseStmtIndex + 1; + } else { + $targetIndex = $index; + $addLineBreak = true; + } + + break; + } + } + + if (null === $targetIndex) { + throw new \Exception('Could not find a class!'); + } + + $newUseNode = (new Builder\Use_($class, Node\Stmt\Use_::TYPE_NORMAL))->getNode(); + array_splice( + $namespaceNode->stmts, + $targetIndex, + 0, + $addLineBreak ? [$newUseNode, $this->createBlankLineNode(self::CONTEXT_OUTSIDE_CLASS)] : [$newUseNode] + ); + + $this->updateSourceCodeFromNewStmts(); + + return $shortClassName; + } + + private function updateSourceCodeFromNewStmts() + { + $newCode = $this->printer->printFormatPreserving( + $this->newStmts, + $this->oldStmts, + $this->oldTokens + ); + + // replace the 3 "fake" items that may be in the code (allowing for different indentation) + $newCode = preg_replace('/(\ |\t)*private\ \$__EXTRA__LINE;/', '', $newCode); + $newCode = preg_replace('/use __EXTRA__LINE;/', '', $newCode); + $newCode = preg_replace('/(\ |\t)*\$__EXTRA__LINE;/', '', $newCode); + + // process comment lines + foreach ($this->pendingComments as $i => $comment) { + // sanity check + $placeholder = sprintf('$__COMMENT__VAR_%d;', $i); + if (false === strpos($newCode, $placeholder)) { + // this can happen if a comment is createSingleLineCommentNode() + // is called, but then that generated code is ultimately not added + continue; + } + + $newCode = str_replace($placeholder, '// '.$comment, $newCode); + } + $this->pendingComments = []; + + $this->setSourceCode($newCode); + } + + private function setSourceCode(string $sourceCode) + { + $this->sourceCode = $sourceCode; + $this->oldStmts = $this->parser->parse($sourceCode); + $this->oldTokens = $this->lexer->getTokens(); + + $traverser = new NodeTraverser(); + $traverser->addVisitor(new NodeVisitor\CloningVisitor()); + $traverser->addVisitor(new NodeVisitor\NameResolver(null, [ + 'replaceNodes' => false, + ])); + $this->newStmts = $traverser->traverse($this->oldStmts); + } + + private function getClassNode(): Node\Stmt\Class_ + { + $node = $this->findFirstNode(function ($node) { + return $node instanceof Node\Stmt\Class_; + }); + + if (!$node) { + throw new \Exception('Could not find class node'); + } + + return $node; + } + + private function getNamespaceNode(): Node\Stmt\Namespace_ + { + $node = $this->findFirstNode(function ($node) { + return $node instanceof Node\Stmt\Namespace_; + }); + + if (!$node) { + throw new \Exception('Could not find namespace node'); + } + + return $node; + } + + /** + * @param callable $filterCallback + * + * @return Node|null + */ + private function findFirstNode(callable $filterCallback) + { + $traverser = new NodeTraverser(); + $visitor = new NodeVisitor\FirstFindingVisitor($filterCallback); + $traverser->addVisitor($visitor); + $traverser->traverse($this->newStmts); + + return $visitor->getFoundNode(); + } + + /** + * @param callable $filterCallback + * @param array $ast + * + * @return Node|null + */ + private function findLastNode(callable $filterCallback, array $ast) + { + $traverser = new NodeTraverser(); + $visitor = new NodeVisitor\FindingVisitor($filterCallback); + $traverser->addVisitor($visitor); + $traverser->traverse($ast); + + $nodes = $visitor->getFoundNodes(); + $node = end($nodes); + + return false === $node ? null : $node; + } + + private function createBlankLineNode(string $context) + { + switch ($context) { + case self::CONTEXT_OUTSIDE_CLASS: + return (new Builder\Use_('__EXTRA__LINE', Node\Stmt\Use_::TYPE_NORMAL)) + ->getNode() + ; + case self::CONTEXT_CLASS: + return (new Builder\Property('__EXTRA__LINE')) + ->makePrivate() + ->getNode() + ; + case self::CONTEXT_CLASS_METHOD: + return new Node\Expr\Variable('__EXTRA__LINE'); + default: + throw new \Exception('Unknown context: '.$context); + } + } + + private function createSingleLineCommentNode(string $comment, string $context) + { + $this->pendingComments[] = $comment; + switch ($context) { + case self::CONTEXT_OUTSIDE_CLASS: + // just not needed yet + throw new \Exception('not supported'); + case self::CONTEXT_CLASS: + // just not needed yet + throw new \Exception('not supported'); + case self::CONTEXT_CLASS_METHOD: + return BuilderHelpers::normalizeStmt(new Node\Expr\Variable(sprintf('__COMMENT__VAR_%d', \count($this->pendingComments) - 1))); + default: + throw new \Exception('Unknown context: '.$context); + } + } + + private function createDocBlock(array $commentLines) + { + $docBlock = "/**\n"; + foreach ($commentLines as $commentLine) { + if ($commentLine) { + $docBlock .= " * $commentLine\n"; + } else { + // avoid the empty, extra space on blank lines + $docBlock .= " *\n"; + } + } + $docBlock .= "\n */"; + + return $docBlock; + } + + private function addMethod(Node\Stmt\ClassMethod $methodNode) + { + $classNode = $this->getClassNode(); + $methodName = $methodNode->name; + $existingIndex = null; + if ($this->methodExists($methodName)) { + if (!$this->overwrite) { + $this->writeNote(sprintf( + 'Not generating %s::%s(): method already exists', + Str::getShortClassName($this->getThisFullClassName()), + $methodName + )); + + return; + } + + // record, so we can overwrite in the same place + $existingIndex = $this->getMethodIndex($methodName); + } + + $newStatements = []; + + // put new method always at the bottom + if (!empty($classNode->stmts)) { + $newStatements[] = $this->createBlankLineNode(self::CONTEXT_CLASS); + } + + $newStatements[] = $methodNode; + + if (null === $existingIndex) { + // add them to the end! + + $classNode->stmts = array_merge($classNode->stmts, $newStatements); + } else { + array_splice( + $classNode->stmts, + $existingIndex, + 1, + $newStatements + ); + } + + $this->updateSourceCodeFromNewStmts(); + } + + private function makeMethodFluent(Builder\Method $methodBuilder) + { + if (!$this->fluentMutators) { + return; + } + + $methodBuilder + ->addStmt($this->createBlankLineNode(self::CONTEXT_CLASS_METHOD)) + ->addStmt(new Node\Stmt\Return_(new Node\Expr\Variable('this'))) + ; + $methodBuilder->setReturnType('self'); + } + + private function getEntityTypeHint($doctrineType) + { + switch ($doctrineType) { + case 'string': + case 'text': + case 'guid': + return 'string'; + + case 'array': + case 'simple_array': + case 'json': + return 'array'; + + case 'boolean': + return 'bool'; + + case 'integer': + case 'smallint': + case 'bigint': + return 'int'; + + case 'float': + return 'float'; + + case 'datetime': + case 'datetimetz': + case 'date': + case 'time': + return '\\'.\DateTimeInterface::class; + + case 'datetime_immutable': + case 'datetimetz_immutable': + case 'date_immutable': + case 'time_immutable': + return '\\'.\DateTimeImmutable::class; + + case 'dateinterval': + return '\\'.\DateInterval::class; + + case 'object': + case 'decimal': + case 'binary': + case 'blob': + default: + return null; + } + } + + private function isInSameNamespace($class) + { + $namespace = substr($class, 0, strrpos($class, '\\')); + + return $this->getNamespaceNode()->name->toCodeString() === $namespace; + } + + private function getThisFullClassName(): string + { + return (string) $this->getClassNode()->namespacedName; + } + + /** + * Adds this new node where a new property should go. + * + * Useful for adding properties, or adding a constructor. + * + * @param Node $newNode + */ + private function addNodeAfterProperties(Node $newNode) + { + $classNode = $this->getClassNode(); + + // try to add after last property + $targetNode = $this->findLastNode(function ($node) { + return $node instanceof Node\Stmt\Property; + }, [$classNode]); + + // otherwise, try to add after the last constant + if (!$targetNode) { + $targetNode = $this->findLastNode(function ($node) { + return $node instanceof Node\Stmt\ClassConst; + }, [$classNode]); + } + + // add the new property after this node + if ($targetNode) { + $index = array_search($targetNode, $classNode->stmts); + + array_splice( + $classNode->stmts, + $index + 1, + 0, + [$this->createBlankLineNode(self::CONTEXT_CLASS), $newNode] + ); + + $this->updateSourceCodeFromNewStmts(); + + return; + } + + // put right at the beginning of the class + // add an empty line, unless the class is totally empty + if (!empty($classNode->stmts)) { + array_unshift($classNode->stmts, $this->createBlankLineNode(self::CONTEXT_CLASS)); + } + array_unshift($classNode->stmts, $newNode); + $this->updateSourceCodeFromNewStmts(); + } + + private function createNullConstant() + { + return new Node\Expr\ConstFetch(new Node\Name('null')); + } + + private function methodExists(string $methodName): bool + { + return false !== $this->getMethodIndex($methodName); + } + + private function getMethodIndex(string $methodName) + { + foreach ($this->getClassNode()->stmts as $i => $node) { + if ($node instanceof Node\Stmt\ClassMethod && strtolower($node->name->toString()) === strtolower($methodName)) { + return $i; + } + } + + return false; + } + + private function propertyExists(string $propertyName) + { + foreach ($this->getClassNode()->stmts as $i => $node) { + if ($node instanceof Node\Stmt\Property && $node->props[0]->name->toString() === $propertyName) { + return true; + } + } + + return false; + } + + private function writeNote(string $note) + { + if (null !== $this->io) { + $this->io->text($note); + } + } +} From 11aa0a4caa1ba64e8064ec82fde14eb8a9bdf802 Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Wed, 24 Oct 2018 21:30:49 +0200 Subject: [PATCH 02/60] Cleanup code --- src/Maker/MakeDTO.php | 16 +++++++--------- src/Util/DTOClassSourceManipulator.php | 5 ++--- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/src/Maker/MakeDTO.php b/src/Maker/MakeDTO.php index 3dc205917..942f525a6 100644 --- a/src/Maker/MakeDTO.php +++ b/src/Maker/MakeDTO.php @@ -11,25 +11,23 @@ namespace Symfony\Bundle\MakerBundle\Maker; +use Doctrine\Common\Annotations\AnnotationReader; +use Doctrine\ORM\Mapping\ClassMetadata; use Symfony\Bundle\MakerBundle\ConsoleStyle; use Symfony\Bundle\MakerBundle\DependencyBuilder; use Symfony\Bundle\MakerBundle\Doctrine\DoctrineHelper; +use Symfony\Bundle\MakerBundle\FileManager; use Symfony\Bundle\MakerBundle\Generator; use Symfony\Bundle\MakerBundle\InputConfiguration; use Symfony\Bundle\MakerBundle\Str; +use Symfony\Bundle\MakerBundle\Util\DTOClassSourceManipulator; use Symfony\Bundle\MakerBundle\Validator; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputArgument; -use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Question\Question; -use Symfony\Component\Validator\Validation; -use Symfony\Bundle\MakerBundle\Maker\AbstractMaker; -use Symfony\Bundle\MakerBundle\FileManager; -use Symfony\Bundle\MakerBundle\Util\DTOClassSourceManipulator; -use Doctrine\ORM\Mapping\ClassMetadata; -use Doctrine\Common\Annotations\AnnotationReader; use Symfony\Component\Validator\Constraint; +use Symfony\Component\Validator\Validation; /** * @author Javier Eguiluz @@ -144,7 +142,7 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen [ 'fields' => $fields, 'addHelpers' => $addHelpers, - 'addGettersSetters' => $addGettersSetters + 'addGettersSetters' => $addGettersSetters, ], $boundClassVars ) @@ -179,7 +177,7 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen foreach ($propertyAnnotations as $annotation) { // we want to copy the asserts, so look for their interface - if($annotation instanceof Constraint) { + if ($annotation instanceof Constraint) { $assertionsImported = true; $comments[] = $manipulator->buildAnnotationLine('@Assert\\'.(new \ReflectionClass($annotation))->getShortName(), (array) $annotation); } diff --git a/src/Util/DTOClassSourceManipulator.php b/src/Util/DTOClassSourceManipulator.php index 6e8b8527f..dcdc6eeee 100644 --- a/src/Util/DTOClassSourceManipulator.php +++ b/src/Util/DTOClassSourceManipulator.php @@ -11,16 +11,15 @@ namespace Symfony\Bundle\MakerBundle\Util; +use PhpParser\Builder; use PhpParser\BuilderHelpers; use PhpParser\Lexer; use PhpParser\Node; -use PhpParser\Parser; use PhpParser\NodeTraverser; use PhpParser\NodeVisitor; -use PhpParser\Builder; +use PhpParser\Parser; use Symfony\Bundle\MakerBundle\ConsoleStyle; use Symfony\Bundle\MakerBundle\Str; -use Symfony\Bundle\MakerBundle\Util\PrettyPrinter; /** * @internal From 239302c75a924bed91fddf5b282bac2dc0bb37cf Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Thu, 25 Oct 2018 18:21:41 +0200 Subject: [PATCH 03/60] Prevent errors with non-entities --- src/Maker/MakeDTO.php | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/Maker/MakeDTO.php b/src/Maker/MakeDTO.php index 942f525a6..f0c842915 100644 --- a/src/Maker/MakeDTO.php +++ b/src/Maker/MakeDTO.php @@ -98,16 +98,20 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen // get some doctrine details $doctrineEntityDetails = $this->entityHelper->createDoctrineDetails($boundClassDetails->getFullName()); + if (null === $doctrineEntityDetails) { + $io->error([ + 'The bound class is not a valid doctrine entity.', + ]); + + return; + } + // get class metadata (used by regenerate) $metaData = $this->entityHelper->getMetadata($boundClassDetails->getFullName()); // list of fields $fields = $metaData->fieldMappings; - if (null !== $doctrineEntityDetails) { - $formFields = $doctrineEntityDetails->getFormFields(); - } - $boundClassVars = [ 'bounded_full_class_name' => $boundClassDetails->getFullName(), 'bounded_class_name' => $boundClassDetails->getShortName(), From 97d0ce1f1776ae1484abf1cedc27311eee4a635b Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Thu, 25 Oct 2018 18:22:20 +0200 Subject: [PATCH 04/60] Rephrase question and update meaning for getter/setter generation --- src/Maker/MakeDTO.php | 12 ++++++------ src/Resources/skeleton/dto/Data.tpl.php | 4 ++-- src/Util/DTOClassSourceManipulator.php | 8 ++++---- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/Maker/MakeDTO.php b/src/Maker/MakeDTO.php index f0c842915..fa9cf9e75 100644 --- a/src/Maker/MakeDTO.php +++ b/src/Maker/MakeDTO.php @@ -121,7 +121,7 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen $addHelpers = $io->confirm('Add helper extract/fill methods?'); // Generate getters/setters - $addGettersSetters = $io->confirm('Generate getters/setters?'); + $omitGettersSetters = $io->confirm('Omit generation of getters/setters?'); // filter id from fields? $omitId = $io->confirm('Omit Id field in DTO?'); @@ -146,14 +146,14 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen [ 'fields' => $fields, 'addHelpers' => $addHelpers, - 'addGettersSetters' => $addGettersSetters, + 'omitGettersSetters' => $omitGettersSetters, ], $boundClassVars ) ); $generator->writeChanges(); - $manipulator = $this->createClassManipulator($DTOClassPath, $addGettersSetters); + $manipulator = $this->createClassManipulator($DTOClassPath, $omitGettersSetters); $mappedFields = $this->getMappedFieldsInEntity($metaData); // Did we import assert annotations? @@ -220,7 +220,7 @@ public function configureDependencies(DependencyBuilder $dependencies) ); } - private function createClassManipulator(string $classPath, bool $addGettersSetters = true): DTOClassSourceManipulator + private function createClassManipulator(string $classPath, bool $omitGettersSetters = false): DTOClassSourceManipulator { return new DTOClassSourceManipulator( $this->fileManager->getFileContents($classPath), @@ -230,8 +230,8 @@ private function createClassManipulator(string $classPath, bool $addGettersSette true, // use fluent mutators true, - // add getters setters? - $addGettersSetters + // omit getters setters? + $omitGettersSetters ); } diff --git a/src/Resources/skeleton/dto/Data.tpl.php b/src/Resources/skeleton/dto/Data.tpl.php index c1e95eac7..765d6bebf 100644 --- a/src/Resources/skeleton/dto/Data.tpl.php +++ b/src/Resources/skeleton/dto/Data.tpl.php @@ -37,7 +37,7 @@ public function __construct(? $ $): { - + $ @@ -65,7 +65,7 @@ public function fill( $ $): self { - + $mapping): ?> $this->set($->get()); diff --git a/src/Util/DTOClassSourceManipulator.php b/src/Util/DTOClassSourceManipulator.php index dcdc6eeee..2094fad28 100644 --- a/src/Util/DTOClassSourceManipulator.php +++ b/src/Util/DTOClassSourceManipulator.php @@ -46,11 +46,11 @@ final class DTOClassSourceManipulator private $pendingComments = []; - public function __construct(string $sourceCode, bool $overwrite = false, bool $useAnnotations = true, bool $fluentMutators = true, bool $addGettersSetters = true) + public function __construct(string $sourceCode, bool $overwrite = false, bool $useAnnotations = true, bool $fluentMutators = true, bool $omitGettersSetters = false) { $this->overwrite = $overwrite; $this->useAnnotations = $useAnnotations; - $this->addGettersSetters = $addGettersSetters; + $this->omitGettersSetters = $omitGettersSetters; $this->fluentMutators = $fluentMutators; $this->lexer = new Lexer\Emulative([ 'usedAttributes' => [ @@ -87,7 +87,7 @@ public function addEntityField(string $propertyName, array $columnOptions, array $this->addProperty($propertyName, $comments, $defaultValue); // return early when setters/getters should not be added. - if (false === $this->addGettersSetters) { + if (true === $this->omitGettersSetters) { return; } @@ -165,7 +165,7 @@ public function addProperty(string $name, array $annotationLines = [], $defaultV $newPropertyBuilder = new Builder\Property($name); // if we do not add getters/setters, the fields must be public - if (false === $this->addGettersSetters) { + if (true === $this->omitGettersSetters) { $newPropertyBuilder->makePublic(); } else { $newPropertyBuilder->makePrivate(); From 59a9742b7c0b5269696b4f81d55a816994842b3d Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Thu, 25 Oct 2018 19:16:00 +0200 Subject: [PATCH 05/60] Fix variable name --- src/Maker/MakeDTO.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Maker/MakeDTO.php b/src/Maker/MakeDTO.php index fa9cf9e75..8e352a1b3 100644 --- a/src/Maker/MakeDTO.php +++ b/src/Maker/MakeDTO.php @@ -82,7 +82,7 @@ public function interact(InputInterface $input, ConsoleStyle $io, Command $comma public function generate(InputInterface $input, ConsoleStyle $io, Generator $generator) { - $formClassNameDetails = $generator->createClassNameDetails( + $dataClassNameDetails = $generator->createClassNameDetails( $input->getArgument('name'), 'Form\\', 'Data' @@ -140,7 +140,7 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen // Skeleton? $DTOClassPath = $generator->generateClass( - $formClassNameDetails->getFullName(), + $dataClassNameDetails->getFullName(), __DIR__.'/../Resources/skeleton/dto/Data.tpl.php', array_merge( [ From 0be8795841e837bbe7b435405a665f050c01709b Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Thu, 25 Oct 2018 19:17:13 +0200 Subject: [PATCH 06/60] Change namespace There is no best practice yet, so we'll make it one. DTOs should live in a separate directory 'Data' in the form directory. --- src/Maker/MakeDTO.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Maker/MakeDTO.php b/src/Maker/MakeDTO.php index 8e352a1b3..a7299f774 100644 --- a/src/Maker/MakeDTO.php +++ b/src/Maker/MakeDTO.php @@ -84,7 +84,7 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen { $dataClassNameDetails = $generator->createClassNameDetails( $input->getArgument('name'), - 'Form\\', + 'Form\\Data\\', 'Data' ); From 90e816df2c1cdb106df2575f13872c68ccd07db0 Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Thu, 25 Oct 2018 19:18:20 +0200 Subject: [PATCH 07/60] Make success message more helpful The message will show the next step including a command with the correct class name. It will also print the fully qualified name to be used in the prompt. --- src/Maker/MakeDTO.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Maker/MakeDTO.php b/src/Maker/MakeDTO.php index a7299f774..616749b92 100644 --- a/src/Maker/MakeDTO.php +++ b/src/Maker/MakeDTO.php @@ -205,7 +205,11 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen } $io->text([ - 'Next: Create your form with this DTO and start using it.', + 'Next: Create your form with this DTO and start using it:', + '$ php bin/console make:form ' . $boundClassDetails->getShortName(), + 'Enter fully qualified data class name to bind to the form:', + '\\' . $dataClassNameDetails->getFullName(), + '', 'Find the documentation at https://symfony.com/doc/current/forms.html', ]); } From 0a4deb53cbb418229f905f385dd559e00340a55f Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Thu, 25 Oct 2018 19:28:23 +0200 Subject: [PATCH 08/60] Make help look like the actual prompt --- src/Maker/MakeDTO.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Maker/MakeDTO.php b/src/Maker/MakeDTO.php index 616749b92..f0ae2aa6d 100644 --- a/src/Maker/MakeDTO.php +++ b/src/Maker/MakeDTO.php @@ -208,7 +208,7 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen 'Next: Create your form with this DTO and start using it:', '$ php bin/console make:form ' . $boundClassDetails->getShortName(), 'Enter fully qualified data class name to bind to the form:', - '\\' . $dataClassNameDetails->getFullName(), + '> \\' . $dataClassNameDetails->getFullName(), '', 'Find the documentation at https://symfony.com/doc/current/forms.html', ]); From 34a3ad37b697b5119a96edfc0cd8563f0e0371ed Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Fri, 26 Oct 2018 00:42:40 +0200 Subject: [PATCH 09/60] Deal with the spaces --- src/Resources/skeleton/dto/Data.tpl.php | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/Resources/skeleton/dto/Data.tpl.php b/src/Resources/skeleton/dto/Data.tpl.php index 765d6bebf..879799ac2 100644 --- a/src/Resources/skeleton/dto/Data.tpl.php +++ b/src/Resources/skeleton/dto/Data.tpl.php @@ -1,5 +1,7 @@ - + namespace ; @@ -36,19 +38,20 @@ public function __construct(? $ $): + { $ - $mapping): ?> + $mapping): ?> ->set($this->get()) ; $ - $mapping): ?> + $mapping): ?> ->set($this->) ; @@ -66,11 +69,11 @@ public function fill( $ $): self { - $mapping): ?> + $mapping): ?> $this->set($->get()); - $mapping): ?> + $mapping): ?> $this-> = $->get(); From 02a95e7d5176a179218204ae37b36443407585dd Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Fri, 26 Oct 2018 00:42:57 +0200 Subject: [PATCH 10/60] Run php-cs-fixer --- src/Maker/MakeDTO.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Maker/MakeDTO.php b/src/Maker/MakeDTO.php index f0ae2aa6d..0623a05e2 100644 --- a/src/Maker/MakeDTO.php +++ b/src/Maker/MakeDTO.php @@ -205,11 +205,11 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen } $io->text([ - 'Next: Create your form with this DTO and start using it:', - '$ php bin/console make:form ' . $boundClassDetails->getShortName(), - 'Enter fully qualified data class name to bind to the form:', - '> \\' . $dataClassNameDetails->getFullName(), - '', + 'Next: Create your form with this DTO and start using it:', + '$ php bin/console make:form '.$boundClassDetails->getShortName(), + 'Enter fully qualified data class name to bind to the form:', + '> \\'.$dataClassNameDetails->getFullName(), + '', 'Find the documentation at https://symfony.com/doc/current/forms.html', ]); } From ff12314a00a7ba60e759a066d14a1babf70483fd Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Fri, 26 Oct 2018 15:19:14 +0200 Subject: [PATCH 11/60] Remove blank line --- src/Resources/skeleton/dto/Data.tpl.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Resources/skeleton/dto/Data.tpl.php b/src/Resources/skeleton/dto/Data.tpl.php index 879799ac2..46717979f 100644 --- a/src/Resources/skeleton/dto/Data.tpl.php +++ b/src/Resources/skeleton/dto/Data.tpl.php @@ -41,7 +41,6 @@ public function fill( $ - $ $mapping): ?> From b97a047c50d216db7f69707dbd897b8003e8e4c8 Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Fri, 26 Oct 2018 15:49:00 +0200 Subject: [PATCH 12/60] Fix omitting getters This was mixed up after rephrasing the question and changing var name. --- src/Resources/skeleton/dto/Data.tpl.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Resources/skeleton/dto/Data.tpl.php b/src/Resources/skeleton/dto/Data.tpl.php index 46717979f..dcfd0be79 100644 --- a/src/Resources/skeleton/dto/Data.tpl.php +++ b/src/Resources/skeleton/dto/Data.tpl.php @@ -40,7 +40,7 @@ public function __construct(? $ $): { - + $ $mapping): ?> @@ -67,7 +67,7 @@ public function fill( $ $): self { - + $mapping): ?> $this->set($->get()); From 73283057d557293078e4144b9fe2c6bbeb3e81b2 Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Mon, 29 Oct 2018 12:13:17 +0100 Subject: [PATCH 13/60] Update classname and filename This eases the autoloads in testing. --- src/Maker/{MakeDTO.php => MakeDto.php} | 4 ++-- src/Resources/config/makers.xml | 2 +- src/Resources/help/{MakeDTO.txt => MakeDto.txt} | 0 3 files changed, 3 insertions(+), 3 deletions(-) rename src/Maker/{MakeDTO.php => MakeDto.php} (99%) rename src/Resources/help/{MakeDTO.txt => MakeDto.txt} (100%) diff --git a/src/Maker/MakeDTO.php b/src/Maker/MakeDto.php similarity index 99% rename from src/Maker/MakeDTO.php rename to src/Maker/MakeDto.php index 0623a05e2..9fbd6a5f1 100644 --- a/src/Maker/MakeDTO.php +++ b/src/Maker/MakeDto.php @@ -34,7 +34,7 @@ * @author Ryan Weaver * @author Clemens Krack */ -final class MakeDTO extends AbstractMaker +final class MakeDto extends AbstractMaker { private $entityHelper; private $fileManager; @@ -58,7 +58,7 @@ public function configureCommand(Command $command, InputConfiguration $inputConf ->setDescription('Creates a new DTO class') ->addArgument('name', InputArgument::REQUIRED, sprintf('The name of the DTO class (e.g. %sData)', Str::asClassName(Str::getRandomTerm()))) ->addArgument('bound-class', InputArgument::REQUIRED, 'The name of Entity that the DTO will be bound to') - ->setHelp(file_get_contents(__DIR__.'/../Resources/help/MakeDTO.txt')) + ->setHelp(file_get_contents(__DIR__.'/../Resources/help/MakeDto.txt')) ; $inputConf->setArgumentAsNonInteractive('bound-class'); diff --git a/src/Resources/config/makers.xml b/src/Resources/config/makers.xml index 29d9aa6ce..a4ce1f427 100644 --- a/src/Resources/config/makers.xml +++ b/src/Resources/config/makers.xml @@ -7,7 +7,7 @@ - + diff --git a/src/Resources/help/MakeDTO.txt b/src/Resources/help/MakeDto.txt similarity index 100% rename from src/Resources/help/MakeDTO.txt rename to src/Resources/help/MakeDto.txt From a1e0fab7eab029ba242dae537e429fca1242d11a Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Mon, 29 Oct 2018 14:41:25 +0100 Subject: [PATCH 14/60] Make Id-omitting mandatory Remove option --- src/Maker/MakeDto.php | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/src/Maker/MakeDto.php b/src/Maker/MakeDto.php index 9fbd6a5f1..43249aa59 100644 --- a/src/Maker/MakeDto.php +++ b/src/Maker/MakeDto.php @@ -123,19 +123,16 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen // Generate getters/setters $omitGettersSetters = $io->confirm('Omit generation of getters/setters?'); - // filter id from fields? - $omitId = $io->confirm('Omit Id field in DTO?'); - - if ($omitId) { + // filter id from fields $fields = array_filter($fields, function ($field) { - // mapping includes id field when property is an id - if (!empty($field['id'])) { - return false; - } + // mapping includes id field when property is an id + if (!empty($field['id'])) { + return false; + } + + return true; + }); - return true; - }); - } // Skeleton? From 71ed7572b82e6f51cca6c693993b1e93f9c0feef Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Mon, 29 Oct 2018 15:45:21 +0100 Subject: [PATCH 15/60] Remove unnecessary comment --- src/Maker/MakeDto.php | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/src/Maker/MakeDto.php b/src/Maker/MakeDto.php index 43249aa59..ee11c9d97 100644 --- a/src/Maker/MakeDto.php +++ b/src/Maker/MakeDto.php @@ -124,17 +124,14 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen $omitGettersSetters = $io->confirm('Omit generation of getters/setters?'); // filter id from fields - $fields = array_filter($fields, function ($field) { - // mapping includes id field when property is an id - if (!empty($field['id'])) { - return false; - } - - return true; - }); - + $fields = array_filter($fields, function ($field) { + // mapping includes id field when property is an id + if (!empty($field['id'])) { + return false; + } - // Skeleton? + return true; + }); $DTOClassPath = $generator->generateClass( $dataClassNameDetails->getFullName(), From c0effd5c64437164502c1913661dea991250b595 Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Mon, 29 Oct 2018 15:47:32 +0100 Subject: [PATCH 16/60] Add testsuite --- tests/Maker/MakeDtoTest.php | 98 +++++++++++++++++++ tests/fixtures/MakeDto/src/Entity/Task.php | 84 ++++++++++++++++ .../MakeDto/tests/GeneratedDtoTest.php | 70 +++++++++++++ .../MakeDtoGettersSetters/src/Entity/Task.php | 84 ++++++++++++++++ .../tests/GeneratedDtoTest.php | 38 +++++++ .../MakeDtoInvalidEntity/src/Entity/Task.php | 69 +++++++++++++ .../MakeDtoWithoutHelpers/src/Entity/Task.php | 84 ++++++++++++++++ .../tests/GeneratedDtoTest.php | 24 +++++ 8 files changed, 551 insertions(+) create mode 100644 tests/Maker/MakeDtoTest.php create mode 100644 tests/fixtures/MakeDto/src/Entity/Task.php create mode 100644 tests/fixtures/MakeDto/tests/GeneratedDtoTest.php create mode 100644 tests/fixtures/MakeDtoGettersSetters/src/Entity/Task.php create mode 100644 tests/fixtures/MakeDtoGettersSetters/tests/GeneratedDtoTest.php create mode 100644 tests/fixtures/MakeDtoInvalidEntity/src/Entity/Task.php create mode 100644 tests/fixtures/MakeDtoWithoutHelpers/src/Entity/Task.php create mode 100644 tests/fixtures/MakeDtoWithoutHelpers/tests/GeneratedDtoTest.php diff --git a/tests/Maker/MakeDtoTest.php b/tests/Maker/MakeDtoTest.php new file mode 100644 index 000000000..20bd9aab0 --- /dev/null +++ b/tests/Maker/MakeDtoTest.php @@ -0,0 +1,98 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Bundle\MakerBundle\Tests\Maker; + +use Symfony\Bundle\MakerBundle\Maker\MakeDto; +use Symfony\Bundle\MakerBundle\Test\MakerTestCase; +use Symfony\Bundle\MakerBundle\Test\MakerTestDetails; + +class MakeDtoTest extends MakerTestCase +{ + public function getTestDetails() + { + yield 'dto' => [MakerTestDetails::createTest( + $this->getMakerInstance(MakeDto::class), + [ + 'Task', + 'Task', + // generate helpers + 'yes', + // omit getters + 'yes', + ]) + ->addExtraDependencies('orm') + ->addExtraDependencies('validator') + ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeDto') + ->assert(function (string $output, string $directory) { + $this->assertContains('created: src/Form/Data/TaskData.php', $output); + $this->assertContains('updated: src/Form/Data/TaskData.php', $output); + $this->assertContains('\\App\\Form\\Data\\TaskData', $output); + }) + ]; + + yield 'dto_getters_setters' => [MakerTestDetails::createTest( + $this->getMakerInstance(MakeDto::class), + [ + 'Task', + 'Task', + // generate helpers + 'yes', + // omit getters + 'no', + ]) + ->addExtraDependencies('orm') + ->addExtraDependencies('validator') + ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeDtoGettersSetters') + ->assert(function (string $output, string $directory) { + $this->assertContains('created: src/Form/Data/TaskData.php', $output); + $this->assertContains('updated: src/Form/Data/TaskData.php', $output); + }) + ]; + + yield 'dto_without_helpers' => [MakerTestDetails::createTest( + $this->getMakerInstance(MakeDto::class), + [ + 'Task', + 'Task', + // generate helpers + 'no', + // omit getters + 'no', + ]) + ->addExtraDependencies('orm') + ->addExtraDependencies('validator') + ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeDtoWithoutHelpers') + ->assert(function (string $output, string $directory) { + $this->assertContains('created: src/Form/Data/TaskData.php', $output); + $this->assertContains('updated: src/Form/Data/TaskData.php', $output); + }) + ]; + + yield 'dto_invalid_entity' => [MakerTestDetails::createTest( + $this->getMakerInstance(MakeDto::class), + [ + 'Task', + // bound class, can not use "Task" because invalid entity is not in autocomplete + '\\App\\Entity\\Task', + // generate helpers + 'yes', + // omit getters + 'yes', + ]) + ->addExtraDependencies('orm') + ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeDtoInvalidEntity') + ->assert(function (string $output, string $directory) { + $this->assertContains('The bound class is not a valid doctrine entity.', $output); + }) + ]; + } +} diff --git a/tests/fixtures/MakeDto/src/Entity/Task.php b/tests/fixtures/MakeDto/src/Entity/Task.php new file mode 100644 index 000000000..30de1fa7d --- /dev/null +++ b/tests/fixtures/MakeDto/src/Entity/Task.php @@ -0,0 +1,84 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace App\Entity; + +use Doctrine\ORM\Mapping as ORM; +use Symfony\Component\Validator\Constraints as Assert; + +/** + * @ORM\Entity() + */ +class Task +{ + /** + * @ORM\Id + * @ORM\GeneratedValue + * @ORM\Column(type="integer") + */ + private $id; + + /** + * @ORM\Column(type="string", length=255, nullable=true) + * @Assert\NotBlank() + */ + private $task; + + /** + * @ORM\Column(type="datetime", nullable=true) + */ + private $dueDate; + + public function getId() + { + return $this->id; + } + + /** + * Get the value of task. + */ + public function getTask() + { + return $this->task; + } + + /** + * Set the value of task. + * + * @return self + */ + public function setTask($task) + { + $this->task = $task; + + return $this; + } + + /** + * Get the value of dueDate. + */ + public function getDueDate() + { + return $this->dueDate; + } + + /** + * Set the value of dueDate. + * + * @return self + */ + public function setDueDate($dueDate) + { + $this->dueDate = $dueDate; + + return $this; + } +} diff --git a/tests/fixtures/MakeDto/tests/GeneratedDtoTest.php b/tests/fixtures/MakeDto/tests/GeneratedDtoTest.php new file mode 100644 index 000000000..c2b93a864 --- /dev/null +++ b/tests/fixtures/MakeDto/tests/GeneratedDtoTest.php @@ -0,0 +1,70 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace App\Tests; + +use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; +use App\Entity\Task; +use App\Form\Data\TaskData; +use Doctrine\Common\Annotations\AnnotationReader; +use Symfony\Component\Validator\Constraints\NotBlank; + +class GeneratedDtoTest extends KernelTestCase +{ + public function testGeneratedDto() + { + $this->assertClassHasAttribute('task', TaskData::class); + $this->assertClassHasAttribute('dueDate', TaskData::class); + $this->assertClassNotHasAttribute('id', TaskData::class); + + $this->assertTrue(method_exists(TaskData::class, 'fill')); + $this->assertTrue(method_exists(TaskData::class, 'extract')); + + $this->assertFalse(method_exists(TaskData::class, 'setTask')); + $this->assertFalse(method_exists(TaskData::class, 'getTask')); + + $this->assertFalse(method_exists(TaskData::class, 'setDueDate')); + $this->assertFalse(method_exists(TaskData::class, 'getDueDate')); + } + + public function testHelpers() + { + $taskEntity = new Task(); + $taskEntity->setTask('Acme'); + $taskEntity->setDueDate(new \DateTime('2018-01-29 01:30')); + + $taskData = new TaskData($taskEntity); + + $this->assertEquals($taskEntity->getTask(), $taskData->task); + $this->assertEquals($taskEntity->getDueDate(), $taskData->dueDate); + + $taskData->task = 'Foo'; + + $taskEntity = new Task(); + $taskData->fill($taskEntity); + + $this->assertEquals($taskEntity->getTask(), $taskData->task); + $this->assertEquals($taskEntity->getDueDate(), $taskData->dueDate); + } + + public function testAnnotations() + { + $annotationReader = new AnnotationReader(); + $reflectionProperty = new \ReflectionProperty(TaskData::class, 'task'); + $propertyAnnotations = $annotationReader->getPropertyAnnotations($reflectionProperty); + $this->assertCount(1, $propertyAnnotations); + $this->assertContainsOnlyInstancesOf(NotBlank::class, $propertyAnnotations); + + $reflectionProperty = new \ReflectionProperty(TaskData::class, 'dueDate'); + $propertyAnnotations = $annotationReader->getPropertyAnnotations($reflectionProperty); + $this->assertCount(0, $propertyAnnotations); + } +} diff --git a/tests/fixtures/MakeDtoGettersSetters/src/Entity/Task.php b/tests/fixtures/MakeDtoGettersSetters/src/Entity/Task.php new file mode 100644 index 000000000..30de1fa7d --- /dev/null +++ b/tests/fixtures/MakeDtoGettersSetters/src/Entity/Task.php @@ -0,0 +1,84 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace App\Entity; + +use Doctrine\ORM\Mapping as ORM; +use Symfony\Component\Validator\Constraints as Assert; + +/** + * @ORM\Entity() + */ +class Task +{ + /** + * @ORM\Id + * @ORM\GeneratedValue + * @ORM\Column(type="integer") + */ + private $id; + + /** + * @ORM\Column(type="string", length=255, nullable=true) + * @Assert\NotBlank() + */ + private $task; + + /** + * @ORM\Column(type="datetime", nullable=true) + */ + private $dueDate; + + public function getId() + { + return $this->id; + } + + /** + * Get the value of task. + */ + public function getTask() + { + return $this->task; + } + + /** + * Set the value of task. + * + * @return self + */ + public function setTask($task) + { + $this->task = $task; + + return $this; + } + + /** + * Get the value of dueDate. + */ + public function getDueDate() + { + return $this->dueDate; + } + + /** + * Set the value of dueDate. + * + * @return self + */ + public function setDueDate($dueDate) + { + $this->dueDate = $dueDate; + + return $this; + } +} diff --git a/tests/fixtures/MakeDtoGettersSetters/tests/GeneratedDtoTest.php b/tests/fixtures/MakeDtoGettersSetters/tests/GeneratedDtoTest.php new file mode 100644 index 000000000..7940cb1fb --- /dev/null +++ b/tests/fixtures/MakeDtoGettersSetters/tests/GeneratedDtoTest.php @@ -0,0 +1,38 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace App\Tests; + +use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; +use App\Entity\Task; +use App\Form\Data\TaskData; + +class GeneratedDtoTest extends KernelTestCase +{ + public function testGeneratedDto() + { + $this->assertTrue(method_exists(TaskData::class, 'setTask')); + $this->assertTrue(method_exists(TaskData::class, 'getTask')); + + $this->assertTrue(method_exists(TaskData::class, 'setDueDate')); + $this->assertTrue(method_exists(TaskData::class, 'getDueDate')); + } + + public function testGettersSetters() + { + $taskData = new Task(); + $taskData->setTask('Acme'); + $taskData->setDueDate(new \DateTime('2018-01-29 01:30')); + + $this->assertEquals($taskData->getTask(), 'Acme'); + $this->assertEquals($taskData->getDueDate(), new \DateTime('2018-01-29 01:30')); + } +} diff --git a/tests/fixtures/MakeDtoInvalidEntity/src/Entity/Task.php b/tests/fixtures/MakeDtoInvalidEntity/src/Entity/Task.php new file mode 100644 index 000000000..b80214929 --- /dev/null +++ b/tests/fixtures/MakeDtoInvalidEntity/src/Entity/Task.php @@ -0,0 +1,69 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace App\Entity; + +/** + * Task, without ORM annotations. + */ +class Task +{ + private $id; + + private $task; + + private $dueDate; + + public function getId() + { + return $this->id; + } + + /** + * Get the value of task. + */ + public function getTask() + { + return $this->task; + } + + /** + * Set the value of task. + * + * @return self + */ + public function setTask($task) + { + $this->task = $task; + + return $this; + } + + /** + * Get the value of dueDate. + */ + public function getDueDate() + { + return $this->dueDate; + } + + /** + * Set the value of dueDate. + * + * @return self + */ + public function setDueDate($dueDate) + { + $this->dueDate = $dueDate; + + return $this; + } +} diff --git a/tests/fixtures/MakeDtoWithoutHelpers/src/Entity/Task.php b/tests/fixtures/MakeDtoWithoutHelpers/src/Entity/Task.php new file mode 100644 index 000000000..30de1fa7d --- /dev/null +++ b/tests/fixtures/MakeDtoWithoutHelpers/src/Entity/Task.php @@ -0,0 +1,84 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace App\Entity; + +use Doctrine\ORM\Mapping as ORM; +use Symfony\Component\Validator\Constraints as Assert; + +/** + * @ORM\Entity() + */ +class Task +{ + /** + * @ORM\Id + * @ORM\GeneratedValue + * @ORM\Column(type="integer") + */ + private $id; + + /** + * @ORM\Column(type="string", length=255, nullable=true) + * @Assert\NotBlank() + */ + private $task; + + /** + * @ORM\Column(type="datetime", nullable=true) + */ + private $dueDate; + + public function getId() + { + return $this->id; + } + + /** + * Get the value of task. + */ + public function getTask() + { + return $this->task; + } + + /** + * Set the value of task. + * + * @return self + */ + public function setTask($task) + { + $this->task = $task; + + return $this; + } + + /** + * Get the value of dueDate. + */ + public function getDueDate() + { + return $this->dueDate; + } + + /** + * Set the value of dueDate. + * + * @return self + */ + public function setDueDate($dueDate) + { + $this->dueDate = $dueDate; + + return $this; + } +} diff --git a/tests/fixtures/MakeDtoWithoutHelpers/tests/GeneratedDtoTest.php b/tests/fixtures/MakeDtoWithoutHelpers/tests/GeneratedDtoTest.php new file mode 100644 index 000000000..418cab359 --- /dev/null +++ b/tests/fixtures/MakeDtoWithoutHelpers/tests/GeneratedDtoTest.php @@ -0,0 +1,24 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace App\Tests; + +use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; +use App\Form\Data\TaskData; + +class GeneratedDtoTest extends KernelTestCase +{ + public function testGeneratedDto() + { + $this->assertFalse(method_exists(TaskData::class, 'fill')); + $this->assertFalse(method_exists(TaskData::class, 'extract')); + } +} From 2630a784d76cb290e60a1b625f06e75470363b21 Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Mon, 29 Oct 2018 16:51:00 +0100 Subject: [PATCH 17/60] Fix PHP 7.0 compatibility --- src/Resources/skeleton/dto/Data.tpl.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Resources/skeleton/dto/Data.tpl.php b/src/Resources/skeleton/dto/Data.tpl.php index dcfd0be79..8dae2ad82 100644 --- a/src/Resources/skeleton/dto/Data.tpl.php +++ b/src/Resources/skeleton/dto/Data.tpl.php @@ -24,7 +24,7 @@ class * @param |null $ */ - public function __construct(? $ = null) + public function __construct( $ = null) { if ($ instanceof ) { $this->extract($); From 85f6646449d1300f392702da3a14bdede573eac5 Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Mon, 29 Oct 2018 20:26:43 +0100 Subject: [PATCH 18/60] Require PHP 7.1 --- src/Command/MakerCommand.php | 2 +- src/Maker/MakeDto.php | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Command/MakerCommand.php b/src/Command/MakerCommand.php index c7c1ed404..e05a4c7e9 100644 --- a/src/Command/MakerCommand.php +++ b/src/Command/MakerCommand.php @@ -65,7 +65,7 @@ protected function initialize(InputInterface $input, OutputInterface $output) $this->maker->configureDependencies($dependencies, $input); if (!$dependencies->isPhpVersionSatisfied()) { - throw new RuntimeCommandException('The make:entity command requires that you use PHP 7.1 or higher.'); + throw new RuntimeCommandException('This maker command requires that you use PHP 7.1 or higher.'); } if ($missingPackagesMessage = $dependencies->getMissingPackagesMessage($this->getName())) { diff --git a/src/Maker/MakeDto.php b/src/Maker/MakeDto.php index ee11c9d97..ec31ff1db 100644 --- a/src/Maker/MakeDto.php +++ b/src/Maker/MakeDto.php @@ -210,7 +210,9 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen public function configureDependencies(DependencyBuilder $dependencies) { - $dependencies->addClassDependency( + $dependencies->requirePHP71(); + + $dependencies->addClassDependency( Validation::class, 'validator', // add as an optional dependency: the user *probably* wants validation From 2218c3b3a6afa37eda2977f93307e41a6dea0d33 Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Mon, 29 Oct 2018 20:27:14 +0100 Subject: [PATCH 19/60] Separate tests for DTO Separate to be able to use @requires annotation. --- tests/fixtures/MakeDto/tests/GeneratedDtoTest.php | 3 +++ .../fixtures/MakeDtoGettersSetters/tests/GeneratedDtoTest.php | 3 +++ .../fixtures/MakeDtoWithoutHelpers/tests/GeneratedDtoTest.php | 3 +++ 3 files changed, 9 insertions(+) diff --git a/tests/fixtures/MakeDto/tests/GeneratedDtoTest.php b/tests/fixtures/MakeDto/tests/GeneratedDtoTest.php index c2b93a864..1b0bd1921 100644 --- a/tests/fixtures/MakeDto/tests/GeneratedDtoTest.php +++ b/tests/fixtures/MakeDto/tests/GeneratedDtoTest.php @@ -17,6 +17,9 @@ use Doctrine\Common\Annotations\AnnotationReader; use Symfony\Component\Validator\Constraints\NotBlank; +/** + * @requires PHP 7.1 + */ class GeneratedDtoTest extends KernelTestCase { public function testGeneratedDto() diff --git a/tests/fixtures/MakeDtoGettersSetters/tests/GeneratedDtoTest.php b/tests/fixtures/MakeDtoGettersSetters/tests/GeneratedDtoTest.php index 7940cb1fb..5ed05050a 100644 --- a/tests/fixtures/MakeDtoGettersSetters/tests/GeneratedDtoTest.php +++ b/tests/fixtures/MakeDtoGettersSetters/tests/GeneratedDtoTest.php @@ -15,6 +15,9 @@ use App\Entity\Task; use App\Form\Data\TaskData; +/** + * @requires PHP 7.1 + */ class GeneratedDtoTest extends KernelTestCase { public function testGeneratedDto() diff --git a/tests/fixtures/MakeDtoWithoutHelpers/tests/GeneratedDtoTest.php b/tests/fixtures/MakeDtoWithoutHelpers/tests/GeneratedDtoTest.php index 418cab359..3e398b924 100644 --- a/tests/fixtures/MakeDtoWithoutHelpers/tests/GeneratedDtoTest.php +++ b/tests/fixtures/MakeDtoWithoutHelpers/tests/GeneratedDtoTest.php @@ -14,6 +14,9 @@ use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; use App\Form\Data\TaskData; +/** + * @requires PHP 7.1 + */ class GeneratedDtoTest extends KernelTestCase { public function testGeneratedDto() From b0fedd1c195fef3bdcb5d739470d956fd39a7c86 Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Mon, 29 Oct 2018 20:41:09 +0100 Subject: [PATCH 20/60] Fix spaces Somehow vscode does not obey it's settings and we don't have an editorconfig file. --- src/Maker/MakeDto.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Maker/MakeDto.php b/src/Maker/MakeDto.php index ec31ff1db..982055444 100644 --- a/src/Maker/MakeDto.php +++ b/src/Maker/MakeDto.php @@ -212,7 +212,7 @@ public function configureDependencies(DependencyBuilder $dependencies) { $dependencies->requirePHP71(); - $dependencies->addClassDependency( + $dependencies->addClassDependency( Validation::class, 'validator', // add as an optional dependency: the user *probably* wants validation From ff2c90e90c835507b003fc68daa36a9f478d001e Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Thu, 1 Nov 2018 10:39:58 +0100 Subject: [PATCH 21/60] Cleanup comments and use PHP_EOL Remove useless comments. Use PHP_EOL for linebreaks. --- src/Resources/skeleton/dto/Data.tpl.php | 23 +++++------------------ 1 file changed, 5 insertions(+), 18 deletions(-) diff --git a/src/Resources/skeleton/dto/Data.tpl.php b/src/Resources/skeleton/dto/Data.tpl.php index 8dae2ad82..53b57e8db 100644 --- a/src/Resources/skeleton/dto/Data.tpl.php +++ b/src/Resources/skeleton/dto/Data.tpl.php @@ -1,4 +1,4 @@ - + @@ -12,7 +12,6 @@ /** * Data transfer object for . - * Add your constraints as annotations to the properties. */ class @@ -20,11 +19,8 @@ class /** * Create DTO, optionally extracting data from a model. - * - * @param |null $ - */ - public function __construct( $ = null) + public function __construct(? $ = null) { if ($ instanceof ) { $this->extract($); @@ -33,23 +29,17 @@ public function __construct( $ $ - */ - public function fill( $): - + public function fill( $): { - $ - + $ $mapping): ?> ->set($this->get()) ; - $ - + $ $mapping): ?> ->set($this->) @@ -61,9 +51,6 @@ public function fill( $ $ - */ public function extract( $): self { From 14df70e93b9cbdde31fbe631326c7e0048cf8240 Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Sun, 4 Nov 2018 18:25:14 +0100 Subject: [PATCH 22/60] Update help text --- src/Resources/help/MakeDto.txt | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/Resources/help/MakeDto.txt b/src/Resources/help/MakeDto.txt index b61dc7546..dcbd2b272 100644 --- a/src/Resources/help/MakeDto.txt +++ b/src/Resources/help/MakeDto.txt @@ -1,10 +1,5 @@ The %command.name% command generates a new data transfer object class. -php %command.full_name% Name Entity +php %command.full_name% DtoClassName TargetEntityClassName If the argument is missing, the command will ask for the classname and entity interactively. - -The command will further ask for: - - the generation of helper methods to extract/fill data from entities into DTO - - the generation of getters and setters for the properties - - whether the id field should be omitted in the DTO From 97d71760cdb317aff1edbad831240346cf1251b3 Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Sun, 4 Nov 2018 18:26:38 +0100 Subject: [PATCH 23/60] Throw exception instead of using io-error --- src/Maker/MakeDto.php | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/Maker/MakeDto.php b/src/Maker/MakeDto.php index 982055444..896124bb5 100644 --- a/src/Maker/MakeDto.php +++ b/src/Maker/MakeDto.php @@ -99,11 +99,7 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen $doctrineEntityDetails = $this->entityHelper->createDoctrineDetails($boundClassDetails->getFullName()); if (null === $doctrineEntityDetails) { - $io->error([ - 'The bound class is not a valid doctrine entity.', - ]); - - return; + throw new RuntimeCommandException('The bound class is not a valid doctrine entity.'); } // get class metadata (used by regenerate) From f5b0c08cf825db7a315671f61c19ce603af8106d Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Sun, 4 Nov 2018 18:27:10 +0100 Subject: [PATCH 24/60] Rename property --- src/Maker/MakeDto.php | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/Maker/MakeDto.php b/src/Maker/MakeDto.php index 896124bb5..38183f1d6 100644 --- a/src/Maker/MakeDto.php +++ b/src/Maker/MakeDto.php @@ -36,14 +36,14 @@ */ final class MakeDto extends AbstractMaker { - private $entityHelper; + private $doctrineHelper; private $fileManager; public function __construct( - DoctrineHelper $entityHelper, + DoctrineHelper $doctrineHelper, FileManager $fileManager ) { - $this->entityHelper = $entityHelper; + $this->doctrineHelper = $doctrineHelper; $this->fileManager = $fileManager; } @@ -55,7 +55,7 @@ public static function getCommandName(): string public function configureCommand(Command $command, InputConfiguration $inputConf) { $command - ->setDescription('Creates a new DTO class') + ->setDescription('Creates a new "data transfer object" (DTO) class from a Doctrine entity') ->addArgument('name', InputArgument::REQUIRED, sprintf('The name of the DTO class (e.g. %sData)', Str::asClassName(Str::getRandomTerm()))) ->addArgument('bound-class', InputArgument::REQUIRED, 'The name of Entity that the DTO will be bound to') ->setHelp(file_get_contents(__DIR__.'/../Resources/help/MakeDto.txt')) @@ -69,7 +69,7 @@ public function interact(InputInterface $input, ConsoleStyle $io, Command $comma if (null === $input->getArgument('bound-class')) { $argument = $command->getDefinition()->getArgument('bound-class'); - $entities = $this->entityHelper->getEntitiesForAutocomplete(); + $entities = $this->doctrineHelper->getEntitiesForAutocomplete(); $question = new Question($argument->getDescription()); $question->setValidator(function ($answer) use ($entities) {return Validator::existsOrNull($answer, $entities); }); @@ -96,14 +96,14 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen ); // get some doctrine details - $doctrineEntityDetails = $this->entityHelper->createDoctrineDetails($boundClassDetails->getFullName()); + $doctrineEntityDetails = $this->doctrineHelper->createDoctrineDetails($boundClassDetails->getFullName()); if (null === $doctrineEntityDetails) { throw new RuntimeCommandException('The bound class is not a valid doctrine entity.'); } // get class metadata (used by regenerate) - $metaData = $this->entityHelper->getMetadata($boundClassDetails->getFullName()); + $metaData = $this->doctrineHelper->getMetadata($boundClassDetails->getFullName()); // list of fields $fields = $metaData->fieldMappings; From a44b27f1e6f59358bedf1559f7e52fd1b0bd828c Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Mon, 5 Nov 2018 13:02:41 +0100 Subject: [PATCH 25/60] Improve Exception --- src/Command/MakerCommand.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Command/MakerCommand.php b/src/Command/MakerCommand.php index e05a4c7e9..83722b94c 100644 --- a/src/Command/MakerCommand.php +++ b/src/Command/MakerCommand.php @@ -65,7 +65,7 @@ protected function initialize(InputInterface $input, OutputInterface $output) $this->maker->configureDependencies($dependencies, $input); if (!$dependencies->isPhpVersionSatisfied()) { - throw new RuntimeCommandException('This maker command requires that you use PHP 7.1 or higher.'); + throw new RuntimeCommandException(sprintf('The %s command requires that you use PHP 7.1 or higher.', $this->maker->getCommandName())); } if ($missingPackagesMessage = $dependencies->getMissingPackagesMessage($this->getName())) { From f49b5d5b19b1d3425ec8d5ccc4f491971cb4e11e Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Mon, 5 Nov 2018 15:05:54 +0100 Subject: [PATCH 26/60] Rename skeleton file --- src/Maker/MakeDto.php | 2 +- src/Resources/skeleton/dto/{Data.tpl.php => DTO.tpl.php} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename src/Resources/skeleton/dto/{Data.tpl.php => DTO.tpl.php} (100%) diff --git a/src/Maker/MakeDto.php b/src/Maker/MakeDto.php index 38183f1d6..d50a1f02c 100644 --- a/src/Maker/MakeDto.php +++ b/src/Maker/MakeDto.php @@ -131,7 +131,7 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen $DTOClassPath = $generator->generateClass( $dataClassNameDetails->getFullName(), - __DIR__.'/../Resources/skeleton/dto/Data.tpl.php', + __DIR__.'/../Resources/skeleton/dto/DTO.tpl.php', array_merge( [ 'fields' => $fields, diff --git a/src/Resources/skeleton/dto/Data.tpl.php b/src/Resources/skeleton/dto/DTO.tpl.php similarity index 100% rename from src/Resources/skeleton/dto/Data.tpl.php rename to src/Resources/skeleton/dto/DTO.tpl.php From 7269a304a6083e0b70ede9405b57e1f2f38fc2a5 Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Mon, 5 Nov 2018 15:09:05 +0100 Subject: [PATCH 27/60] Add missing use statement for RuntimeCommandException --- src/Maker/MakeDto.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Maker/MakeDto.php b/src/Maker/MakeDto.php index d50a1f02c..6e732252b 100644 --- a/src/Maker/MakeDto.php +++ b/src/Maker/MakeDto.php @@ -16,6 +16,7 @@ use Symfony\Bundle\MakerBundle\ConsoleStyle; use Symfony\Bundle\MakerBundle\DependencyBuilder; use Symfony\Bundle\MakerBundle\Doctrine\DoctrineHelper; +use Symfony\Bundle\MakerBundle\Exception\RuntimeCommandException; use Symfony\Bundle\MakerBundle\FileManager; use Symfony\Bundle\MakerBundle\Generator; use Symfony\Bundle\MakerBundle\InputConfiguration; From aed765aa39dcbb2e192396393ee057d83c63dd21 Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Mon, 5 Nov 2018 15:09:43 +0100 Subject: [PATCH 28/60] Use existing method to check for entity --- src/Maker/MakeDto.php | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/Maker/MakeDto.php b/src/Maker/MakeDto.php index 6e732252b..e35c12270 100644 --- a/src/Maker/MakeDto.php +++ b/src/Maker/MakeDto.php @@ -96,10 +96,8 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen 'Entity\\' ); - // get some doctrine details - $doctrineEntityDetails = $this->doctrineHelper->createDoctrineDetails($boundClassDetails->getFullName()); - - if (null === $doctrineEntityDetails) { + // verify that class is an entity + if (false === $this->doctrineHelper->isClassAMappedEntity($boundClassDetails->getFullName())) { throw new RuntimeCommandException('The bound class is not a valid doctrine entity.'); } From 94893095ee0a57be59185fdaa6293024d9eff354 Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Tue, 6 Nov 2018 15:47:00 +0100 Subject: [PATCH 29/60] Remove dead codeblock --- src/Maker/MakeDto.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/Maker/MakeDto.php b/src/Maker/MakeDto.php index e35c12270..75cc98f32 100644 --- a/src/Maker/MakeDto.php +++ b/src/Maker/MakeDto.php @@ -232,9 +232,6 @@ private function createClassManipulator(string $classPath, bool $omitGettersSett private function getMappedFieldsInEntity(ClassMetadata $classMetadata) { - /* @var $classReflection \ReflectionClass */ - $classReflection = $classMetadata->reflClass; - $targetFields = array_merge( array_keys($classMetadata->fieldMappings), array_keys($classMetadata->associationMappings) From c209033d6390ed7a96f81610366f96862e8bef4b Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Tue, 6 Nov 2018 15:58:13 +0100 Subject: [PATCH 30/60] Remove useless check We changed the source of $fields, so we no longer need this check. --- src/Maker/MakeDto.php | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/Maker/MakeDto.php b/src/Maker/MakeDto.php index 75cc98f32..3845f222d 100644 --- a/src/Maker/MakeDto.php +++ b/src/Maker/MakeDto.php @@ -149,10 +149,6 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen $assertionsImported = false; foreach ($fields as $fieldName => $mapping) { - if (!\in_array($fieldName, $mappedFields)) { - continue; - } - $annotationReader = new AnnotationReader(); // Lookup classname for inherited properties From afffbfd8f6cbc5228bb6eb0f68930af22426fedc Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Tue, 6 Nov 2018 16:00:35 +0100 Subject: [PATCH 31/60] Move codeblock closer to where it's used --- src/Maker/MakeDto.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Maker/MakeDto.php b/src/Maker/MakeDto.php index 3845f222d..a8e446d4a 100644 --- a/src/Maker/MakeDto.php +++ b/src/Maker/MakeDto.php @@ -107,11 +107,6 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen // list of fields $fields = $metaData->fieldMappings; - $boundClassVars = [ - 'bounded_full_class_name' => $boundClassDetails->getFullName(), - 'bounded_class_name' => $boundClassDetails->getShortName(), - ]; - // the result is passed to the template $addHelpers = $io->confirm('Add helper extract/fill methods?'); @@ -128,6 +123,11 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen return true; }); + $boundClassVars = [ + 'bounded_full_class_name' => $boundClassDetails->getFullName(), + 'bounded_class_name' => $boundClassDetails->getShortName(), + ]; + $DTOClassPath = $generator->generateClass( $dataClassNameDetails->getFullName(), __DIR__.'/../Resources/skeleton/dto/DTO.tpl.php', From c36abf9f2a0db94bf2ca4e9e01744444a3bf5c97 Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Tue, 6 Nov 2018 16:00:47 +0100 Subject: [PATCH 32/60] Fix comments --- src/Maker/MakeDto.php | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/Maker/MakeDto.php b/src/Maker/MakeDto.php index a8e446d4a..3444176ff 100644 --- a/src/Maker/MakeDto.php +++ b/src/Maker/MakeDto.php @@ -101,19 +101,17 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen throw new RuntimeCommandException('The bound class is not a valid doctrine entity.'); } - // get class metadata (used by regenerate) + // get class metadata (used to copy annotations and generate properties) $metaData = $this->doctrineHelper->getMetadata($boundClassDetails->getFullName()); - // list of fields + // get list of fields $fields = $metaData->fieldMappings; - // the result is passed to the template + // The result is passed to the template $addHelpers = $io->confirm('Add helper extract/fill methods?'); - - // Generate getters/setters $omitGettersSetters = $io->confirm('Omit generation of getters/setters?'); - // filter id from fields + // Filter id from fields $fields = array_filter($fields, function ($field) { // mapping includes id field when property is an id if (!empty($field['id'])) { From fd2f7f227d51ad384e18b4b0f6cb8edb93a8edb6 Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Tue, 6 Nov 2018 16:04:30 +0100 Subject: [PATCH 33/60] Restructure tests Move tests back into testCommands method. Use setRequiredPhpVersion() instead of @require. Allow dto_invalid_entity test to fail (needed because we throw RunTimeCommandException). --- tests/Maker/MakeDtoTest.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/Maker/MakeDtoTest.php b/tests/Maker/MakeDtoTest.php index 20bd9aab0..5aec1942d 100644 --- a/tests/Maker/MakeDtoTest.php +++ b/tests/Maker/MakeDtoTest.php @@ -19,7 +19,7 @@ class MakeDtoTest extends MakerTestCase { public function getTestDetails() { - yield 'dto' => [MakerTestDetails::createTest( + yield 'dto' => [MakerTestDetails::createTest( $this->getMakerInstance(MakeDto::class), [ 'Task', @@ -37,6 +37,7 @@ public function getTestDetails() $this->assertContains('updated: src/Form/Data/TaskData.php', $output); $this->assertContains('\\App\\Form\\Data\\TaskData', $output); }) + ->setRequiredPhpVersion(70100) ]; yield 'dto_getters_setters' => [MakerTestDetails::createTest( @@ -56,6 +57,7 @@ public function getTestDetails() $this->assertContains('created: src/Form/Data/TaskData.php', $output); $this->assertContains('updated: src/Form/Data/TaskData.php', $output); }) + ->setRequiredPhpVersion(70100) ]; yield 'dto_without_helpers' => [MakerTestDetails::createTest( @@ -75,6 +77,7 @@ public function getTestDetails() $this->assertContains('created: src/Form/Data/TaskData.php', $output); $this->assertContains('updated: src/Form/Data/TaskData.php', $output); }) + ->setRequiredPhpVersion(70100) ]; yield 'dto_invalid_entity' => [MakerTestDetails::createTest( @@ -89,10 +92,12 @@ public function getTestDetails() 'yes', ]) ->addExtraDependencies('orm') + ->setCommandAllowedToFail(true) ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeDtoInvalidEntity') ->assert(function (string $output, string $directory) { $this->assertContains('The bound class is not a valid doctrine entity.', $output); }) + ->setRequiredPhpVersion(70100) ]; } } From 6728035482a4d31a003d815c5804e4f0dd4b7628 Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Tue, 6 Nov 2018 16:05:38 +0100 Subject: [PATCH 34/60] Add composite id test This tests whether both composite id fields are omitted from the DTO. --- tests/Maker/MakeDtoTest.php | 21 +++ .../MakeDtoCompositeId/src/Entity/Task.php | 130 ++++++++++++++++++ .../tests/GeneratedDtoTest.php | 31 +++++ 3 files changed, 182 insertions(+) create mode 100644 tests/fixtures/MakeDtoCompositeId/src/Entity/Task.php create mode 100644 tests/fixtures/MakeDtoCompositeId/tests/GeneratedDtoTest.php diff --git a/tests/Maker/MakeDtoTest.php b/tests/Maker/MakeDtoTest.php index 5aec1942d..eba220235 100644 --- a/tests/Maker/MakeDtoTest.php +++ b/tests/Maker/MakeDtoTest.php @@ -99,5 +99,26 @@ public function getTestDetails() }) ->setRequiredPhpVersion(70100) ]; + + yield 'dto_composite_id' => [MakerTestDetails::createTest( + $this->getMakerInstance(MakeDto::class), + [ + 'Task', + 'Task', + // generate helpers + 'yes', + // omit getters + 'yes', + ]) + ->addExtraDependencies('orm') + ->addExtraDependencies('validator') + ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeDtoCompositeId') + ->assert(function (string $output, string $directory) { + $this->assertContains('created: src/Form/Data/TaskData.php', $output); + $this->assertContains('updated: src/Form/Data/TaskData.php', $output); + $this->assertContains('\\App\\Form\\Data\\TaskData', $output); + }) + ->setRequiredPhpVersion(70100) + ]; } } diff --git a/tests/fixtures/MakeDtoCompositeId/src/Entity/Task.php b/tests/fixtures/MakeDtoCompositeId/src/Entity/Task.php new file mode 100644 index 000000000..73c405604 --- /dev/null +++ b/tests/fixtures/MakeDtoCompositeId/src/Entity/Task.php @@ -0,0 +1,130 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace App\Entity; + +use Doctrine\ORM\Mapping as ORM; +use Symfony\Component\Validator\Constraints as Assert; + +/** + * @ORM\Entity() + */ +class Task +{ + /** + * @ORM\Id + * @ORM\Column(type="string", length=255, nullable=false) + */ + private $id; + + /** + * @ORM\Id + * @ORM\Column(type="string", length=255, nullable=true) + * @Assert\NotBlank() + */ + private $group; + + /** + * @ORM\Column(type="string", length=255, nullable=true) + * @Assert\NotBlank() + */ + private $task; + + /** + * @ORM\Column(type="datetime", nullable=true) + */ + private $dueDate; + + public function __construct() + { + $this->id = uniqid(); + } + + /** + * Get the value of task. + */ + public function getTask() + { + return $this->task; + } + + /** + * Set the value of task. + * + * @return self + */ + public function setTask($task) + { + $this->task = $task; + + return $this; + } + + /** + * Get the value of dueDate. + */ + public function getDueDate() + { + return $this->dueDate; + } + + /** + * Set the value of dueDate. + * + * @return self + */ + public function setDueDate($dueDate) + { + $this->dueDate = $dueDate; + + return $this; + } + + /** + * Get the value of group + */ + public function getGroup() + { + return $this->group; + } + + /** + * Set the value of group + * + * @return self + */ + public function setGroup($group) + { + $this->group = $group; + + return $this; + } + + /** + * Get the value of id + */ + public function getId() + { + return $this->id; + } + + /** + * Set the value of id + * + * @return self + */ + public function setId($id) + { + $this->id = $id; + + return $this; + } +} diff --git a/tests/fixtures/MakeDtoCompositeId/tests/GeneratedDtoTest.php b/tests/fixtures/MakeDtoCompositeId/tests/GeneratedDtoTest.php new file mode 100644 index 000000000..c71c7bff0 --- /dev/null +++ b/tests/fixtures/MakeDtoCompositeId/tests/GeneratedDtoTest.php @@ -0,0 +1,31 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace App\Tests; + +use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; +use App\Entity\Task; +use App\Form\Data\TaskData; +use Doctrine\Common\Annotations\AnnotationReader; +use Symfony\Component\Validator\Constraints\NotBlank; + +/** + * @requires PHP 7.1 + */ +class GeneratedDtoTest extends KernelTestCase +{ + public function testGeneratedDto() + { + // the Entity has a composite Id - test that the attributes are both omitted from the DTO + $this->assertClassNotHasAttribute('id', TaskData::class); + $this->assertClassNotHasAttribute('group', TaskData::class); + } +} From e72dd7199a0808ff16f4b666fba6754e5dcea079 Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Tue, 6 Nov 2018 16:06:27 +0100 Subject: [PATCH 35/60] Add mappedsuperclass test This tests proper generation of properties for mappedsuperclasses. --- tests/Maker/MakeDtoTest.php | 21 ++++++ .../src/Entity/Deadline.php | 58 +++++++++++++++ .../src/Entity/Task.php | 47 ++++++++++++ .../tests/GeneratedDtoTest.php | 73 +++++++++++++++++++ 4 files changed, 199 insertions(+) create mode 100644 tests/fixtures/MakeDtoMappedSuperClass/src/Entity/Deadline.php create mode 100644 tests/fixtures/MakeDtoMappedSuperClass/src/Entity/Task.php create mode 100644 tests/fixtures/MakeDtoMappedSuperClass/tests/GeneratedDtoTest.php diff --git a/tests/Maker/MakeDtoTest.php b/tests/Maker/MakeDtoTest.php index eba220235..2bd249ba1 100644 --- a/tests/Maker/MakeDtoTest.php +++ b/tests/Maker/MakeDtoTest.php @@ -100,6 +100,27 @@ public function getTestDetails() ->setRequiredPhpVersion(70100) ]; + yield 'dto_mapped_super_class' => [MakerTestDetails::createTest( + $this->getMakerInstance(MakeDto::class), + [ + 'Task', + 'Task', + // generate helpers + 'yes', + // omit getters + 'yes', + ]) + ->addExtraDependencies('orm') + ->addExtraDependencies('validator') + ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeDtoMappedSuperClass') + ->assert(function (string $output, string $directory) { + $this->assertContains('created: src/Form/Data/TaskData.php', $output); + $this->assertContains('updated: src/Form/Data/TaskData.php', $output); + $this->assertContains('\\App\\Form\\Data\\TaskData', $output); + }) + ->setRequiredPhpVersion(70100) + ]; + yield 'dto_composite_id' => [MakerTestDetails::createTest( $this->getMakerInstance(MakeDto::class), [ diff --git a/tests/fixtures/MakeDtoMappedSuperClass/src/Entity/Deadline.php b/tests/fixtures/MakeDtoMappedSuperClass/src/Entity/Deadline.php new file mode 100644 index 000000000..258206a06 --- /dev/null +++ b/tests/fixtures/MakeDtoMappedSuperClass/src/Entity/Deadline.php @@ -0,0 +1,58 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace App\Entity; + +use Doctrine\ORM\Mapping as ORM; +use Symfony\Component\Validator\Constraints as Assert; + +/** + * @ORM\MappedSuperclass + */ +class Deadline +{ + /** + * @ORM\Id + * @ORM\GeneratedValue + * @ORM\Column(type="integer") + */ + private $id; + + /** + * @ORM\Column(type="datetime", nullable=true) + */ + private $dueDate; + + public function getId() + { + return $this->id; + } + + /** + * Get the value of dueDate. + */ + public function getDueDate() + { + return $this->dueDate; + } + + /** + * Set the value of dueDate. + * + * @return self + */ + public function setDueDate($dueDate) + { + $this->dueDate = $dueDate; + + return $this; + } +} diff --git a/tests/fixtures/MakeDtoMappedSuperClass/src/Entity/Task.php b/tests/fixtures/MakeDtoMappedSuperClass/src/Entity/Task.php new file mode 100644 index 000000000..7e5d034fe --- /dev/null +++ b/tests/fixtures/MakeDtoMappedSuperClass/src/Entity/Task.php @@ -0,0 +1,47 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace App\Entity; + +use Doctrine\ORM\Mapping as ORM; +use Symfony\Component\Validator\Constraints as Assert; + +/** + * @ORM\Entity() + */ +class Task extends Deadline +{ + /** + * @ORM\Column(type="string", length=255, nullable=true) + * @Assert\NotBlank() + */ + private $task; + + /** + * Get the value of task. + */ + public function getTask() + { + return $this->task; + } + + /** + * Set the value of task. + * + * @return self + */ + public function setTask($task) + { + $this->task = $task; + + return $this; + } +} diff --git a/tests/fixtures/MakeDtoMappedSuperClass/tests/GeneratedDtoTest.php b/tests/fixtures/MakeDtoMappedSuperClass/tests/GeneratedDtoTest.php new file mode 100644 index 000000000..1b0bd1921 --- /dev/null +++ b/tests/fixtures/MakeDtoMappedSuperClass/tests/GeneratedDtoTest.php @@ -0,0 +1,73 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace App\Tests; + +use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; +use App\Entity\Task; +use App\Form\Data\TaskData; +use Doctrine\Common\Annotations\AnnotationReader; +use Symfony\Component\Validator\Constraints\NotBlank; + +/** + * @requires PHP 7.1 + */ +class GeneratedDtoTest extends KernelTestCase +{ + public function testGeneratedDto() + { + $this->assertClassHasAttribute('task', TaskData::class); + $this->assertClassHasAttribute('dueDate', TaskData::class); + $this->assertClassNotHasAttribute('id', TaskData::class); + + $this->assertTrue(method_exists(TaskData::class, 'fill')); + $this->assertTrue(method_exists(TaskData::class, 'extract')); + + $this->assertFalse(method_exists(TaskData::class, 'setTask')); + $this->assertFalse(method_exists(TaskData::class, 'getTask')); + + $this->assertFalse(method_exists(TaskData::class, 'setDueDate')); + $this->assertFalse(method_exists(TaskData::class, 'getDueDate')); + } + + public function testHelpers() + { + $taskEntity = new Task(); + $taskEntity->setTask('Acme'); + $taskEntity->setDueDate(new \DateTime('2018-01-29 01:30')); + + $taskData = new TaskData($taskEntity); + + $this->assertEquals($taskEntity->getTask(), $taskData->task); + $this->assertEquals($taskEntity->getDueDate(), $taskData->dueDate); + + $taskData->task = 'Foo'; + + $taskEntity = new Task(); + $taskData->fill($taskEntity); + + $this->assertEquals($taskEntity->getTask(), $taskData->task); + $this->assertEquals($taskEntity->getDueDate(), $taskData->dueDate); + } + + public function testAnnotations() + { + $annotationReader = new AnnotationReader(); + $reflectionProperty = new \ReflectionProperty(TaskData::class, 'task'); + $propertyAnnotations = $annotationReader->getPropertyAnnotations($reflectionProperty); + $this->assertCount(1, $propertyAnnotations); + $this->assertContainsOnlyInstancesOf(NotBlank::class, $propertyAnnotations); + + $reflectionProperty = new \ReflectionProperty(TaskData::class, 'dueDate'); + $propertyAnnotations = $annotationReader->getPropertyAnnotations($reflectionProperty); + $this->assertCount(0, $propertyAnnotations); + } +} From 3e595a3df7061f65db697c899260abfb981fb4f7 Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Tue, 6 Nov 2018 16:07:27 +0100 Subject: [PATCH 36/60] Add test for missing getters/setters This tests whether the helper methods run properly, when the entity is missing getters/setters. --- tests/Maker/MakeDtoTest.php | 21 +++++ .../src/Entity/Task.php | 85 +++++++++++++++++++ .../tests/GeneratedDtoTest.php | 42 +++++++++ 3 files changed, 148 insertions(+) create mode 100644 tests/fixtures/MakeDtoMissingGettersSetters/src/Entity/Task.php create mode 100644 tests/fixtures/MakeDtoMissingGettersSetters/tests/GeneratedDtoTest.php diff --git a/tests/Maker/MakeDtoTest.php b/tests/Maker/MakeDtoTest.php index 2bd249ba1..6c0fac133 100644 --- a/tests/Maker/MakeDtoTest.php +++ b/tests/Maker/MakeDtoTest.php @@ -141,5 +141,26 @@ public function getTestDetails() }) ->setRequiredPhpVersion(70100) ]; + + yield 'dto_missing_getters_setters' => [MakerTestDetails::createTest( + $this->getMakerInstance(MakeDto::class), + [ + 'Task', + 'Task', + // generate helpers + 'yes', + // omit getters + 'no', + ]) + ->addExtraDependencies('orm') + ->addExtraDependencies('validator') + ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeDtoMissingGettersSetters') + ->assert(function (string $output, string $directory) { + $this->assertContains('created: src/Form/Data/TaskData.php', $output); + $this->assertContains('updated: src/Form/Data/TaskData.php', $output); + $this->assertContains('\\App\\Form\\Data\\TaskData', $output); + }) + ->setRequiredPhpVersion(70100) + ]; } } diff --git a/tests/fixtures/MakeDtoMissingGettersSetters/src/Entity/Task.php b/tests/fixtures/MakeDtoMissingGettersSetters/src/Entity/Task.php new file mode 100644 index 000000000..d6cb4924c --- /dev/null +++ b/tests/fixtures/MakeDtoMissingGettersSetters/src/Entity/Task.php @@ -0,0 +1,85 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace App\Entity; + +use Doctrine\ORM\Mapping as ORM; +use Symfony\Component\Validator\Constraints as Assert; + +/** + * @ORM\Entity() + */ +class Task +{ + /** + * @ORM\Id + * @ORM\GeneratedValue + * @ORM\Column(type="integer") + */ + private $id; + + /** + * @ORM\Column(type="string", length=255, nullable=true) + * @Assert\NotBlank() + */ + private $task; + + /** + * @ORM\Column(type="datetime", nullable=true) + */ + private $dueDate; + + public function getId() + { + return $this->id; + } + + // commented out for the test. + // /** + // * Get the value of task. + // */ + // public function getTask() + // { + // return $this->task; + // } + + // /** + // * Set the value of task. + // * + // * @return self + // */ + // public function setTask($task) + // { + // $this->task = $task; + + // return $this; + // } + + /** + * Get the value of dueDate. + */ + public function getDueDate() + { + return $this->dueDate; + } + + /** + * Set the value of dueDate. + * + * @return self + */ + public function setDueDate($dueDate) + { + $this->dueDate = $dueDate; + + return $this; + } +} diff --git a/tests/fixtures/MakeDtoMissingGettersSetters/tests/GeneratedDtoTest.php b/tests/fixtures/MakeDtoMissingGettersSetters/tests/GeneratedDtoTest.php new file mode 100644 index 000000000..240795021 --- /dev/null +++ b/tests/fixtures/MakeDtoMissingGettersSetters/tests/GeneratedDtoTest.php @@ -0,0 +1,42 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace App\Tests; + +use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; +use App\Form\Data\TaskData; +use App\Entity\Task; + +/** + * @requires PHP 7.1 + */ +class GeneratedDtoTest extends KernelTestCase +{ + /** + * Use the helpers to make sure that they work, even though there are missing getters/setters + */ + public function testHelpers() + { + $taskEntity = new Task(); + $taskEntity->setDueDate(new \DateTime('2018-01-29 01:30')); + + $taskData = new TaskData($taskEntity); + + $this->assertEquals($taskEntity->getDueDate(), $taskData->dueDate); + + $taskData->task = 'Foo'; + + $taskEntity = new Task(); + $taskData->fill($taskEntity); + + $this->assertEquals($taskEntity->getDueDate(), $taskData->dueDate); + } +} From 5fe16edda52ede919e3c49d98d1ad7a24c4701e5 Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Wed, 7 Nov 2018 10:08:49 +0100 Subject: [PATCH 37/60] Fix indentation --- tests/Maker/MakeDtoTest.php | 100 ++++++++++++++++++------------------ 1 file changed, 50 insertions(+), 50 deletions(-) diff --git a/tests/Maker/MakeDtoTest.php b/tests/Maker/MakeDtoTest.php index 6c0fac133..cfc74c52d 100644 --- a/tests/Maker/MakeDtoTest.php +++ b/tests/Maker/MakeDtoTest.php @@ -24,31 +24,31 @@ public function getTestDetails() [ 'Task', 'Task', - // generate helpers - 'yes', - // omit getters - 'yes', + // generate helpers + 'yes', + // omit getters + 'yes', ]) ->addExtraDependencies('orm') ->addExtraDependencies('validator') ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeDto') ->assert(function (string $output, string $directory) { $this->assertContains('created: src/Form/Data/TaskData.php', $output); - $this->assertContains('updated: src/Form/Data/TaskData.php', $output); - $this->assertContains('\\App\\Form\\Data\\TaskData', $output); + $this->assertContains('updated: src/Form/Data/TaskData.php', $output); + $this->assertContains('\\App\\Form\\Data\\TaskData', $output); }) - ->setRequiredPhpVersion(70100) - ]; + ->setRequiredPhpVersion(70100), + ]; yield 'dto_getters_setters' => [MakerTestDetails::createTest( $this->getMakerInstance(MakeDto::class), [ 'Task', - 'Task', - // generate helpers - 'yes', - // omit getters - 'no', + 'Task', + // generate helpers + 'yes', + // omit getters + 'no', ]) ->addExtraDependencies('orm') ->addExtraDependencies('validator') @@ -57,18 +57,18 @@ public function getTestDetails() $this->assertContains('created: src/Form/Data/TaskData.php', $output); $this->assertContains('updated: src/Form/Data/TaskData.php', $output); }) - ->setRequiredPhpVersion(70100) - ]; + ->setRequiredPhpVersion(70100), + ]; yield 'dto_without_helpers' => [MakerTestDetails::createTest( $this->getMakerInstance(MakeDto::class), [ 'Task', - 'Task', - // generate helpers - 'no', - // omit getters - 'no', + 'Task', + // generate helpers + 'no', + // omit getters + 'no', ]) ->addExtraDependencies('orm') ->addExtraDependencies('validator') @@ -77,19 +77,19 @@ public function getTestDetails() $this->assertContains('created: src/Form/Data/TaskData.php', $output); $this->assertContains('updated: src/Form/Data/TaskData.php', $output); }) - ->setRequiredPhpVersion(70100) - ]; + ->setRequiredPhpVersion(70100), + ]; yield 'dto_invalid_entity' => [MakerTestDetails::createTest( $this->getMakerInstance(MakeDto::class), [ - 'Task', - // bound class, can not use "Task" because invalid entity is not in autocomplete + 'Task', + // bound class, can not use "Task" because invalid entity is not in autocomplete '\\App\\Entity\\Task', - // generate helpers - 'yes', - // omit getters - 'yes', + // generate helpers + 'yes', + // omit getters + 'yes', ]) ->addExtraDependencies('orm') ->setCommandAllowedToFail(true) @@ -97,7 +97,7 @@ public function getTestDetails() ->assert(function (string $output, string $directory) { $this->assertContains('The bound class is not a valid doctrine entity.', $output); }) - ->setRequiredPhpVersion(70100) + ->setRequiredPhpVersion(70100), ]; yield 'dto_mapped_super_class' => [MakerTestDetails::createTest( @@ -105,20 +105,20 @@ public function getTestDetails() [ 'Task', 'Task', - // generate helpers - 'yes', - // omit getters - 'yes', + // generate helpers + 'yes', + // omit getters + 'yes', ]) ->addExtraDependencies('orm') ->addExtraDependencies('validator') ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeDtoMappedSuperClass') ->assert(function (string $output, string $directory) { $this->assertContains('created: src/Form/Data/TaskData.php', $output); - $this->assertContains('updated: src/Form/Data/TaskData.php', $output); - $this->assertContains('\\App\\Form\\Data\\TaskData', $output); + $this->assertContains('updated: src/Form/Data/TaskData.php', $output); + $this->assertContains('\\App\\Form\\Data\\TaskData', $output); }) - ->setRequiredPhpVersion(70100) + ->setRequiredPhpVersion(70100), ]; yield 'dto_composite_id' => [MakerTestDetails::createTest( @@ -126,20 +126,20 @@ public function getTestDetails() [ 'Task', 'Task', - // generate helpers - 'yes', - // omit getters - 'yes', + // generate helpers + 'yes', + // omit getters + 'yes', ]) ->addExtraDependencies('orm') ->addExtraDependencies('validator') ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeDtoCompositeId') ->assert(function (string $output, string $directory) { $this->assertContains('created: src/Form/Data/TaskData.php', $output); - $this->assertContains('updated: src/Form/Data/TaskData.php', $output); - $this->assertContains('\\App\\Form\\Data\\TaskData', $output); + $this->assertContains('updated: src/Form/Data/TaskData.php', $output); + $this->assertContains('\\App\\Form\\Data\\TaskData', $output); }) - ->setRequiredPhpVersion(70100) + ->setRequiredPhpVersion(70100), ]; yield 'dto_missing_getters_setters' => [MakerTestDetails::createTest( @@ -147,20 +147,20 @@ public function getTestDetails() [ 'Task', 'Task', - // generate helpers - 'yes', - // omit getters - 'no', + // generate helpers + 'yes', + // omit getters + 'no', ]) ->addExtraDependencies('orm') ->addExtraDependencies('validator') ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeDtoMissingGettersSetters') ->assert(function (string $output, string $directory) { $this->assertContains('created: src/Form/Data/TaskData.php', $output); - $this->assertContains('updated: src/Form/Data/TaskData.php', $output); - $this->assertContains('\\App\\Form\\Data\\TaskData', $output); + $this->assertContains('updated: src/Form/Data/TaskData.php', $output); + $this->assertContains('\\App\\Form\\Data\\TaskData', $output); }) - ->setRequiredPhpVersion(70100) + ->setRequiredPhpVersion(70100), ]; } } From 3a55a5add65ec05aee0c40e4322824e9c8637490 Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Wed, 7 Nov 2018 10:26:36 +0100 Subject: [PATCH 38/60] Use null comparison instead of instanceof Typecheck is already done in the method signature. --- src/Resources/skeleton/dto/DTO.tpl.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Resources/skeleton/dto/DTO.tpl.php b/src/Resources/skeleton/dto/DTO.tpl.php index 53b57e8db..809667d02 100644 --- a/src/Resources/skeleton/dto/DTO.tpl.php +++ b/src/Resources/skeleton/dto/DTO.tpl.php @@ -22,7 +22,7 @@ class */ public function __construct(? $ = null) { - if ($ instanceof ) { + if (null !== $) { $this->extract($); } } From b3d7512e044ee120a53a05a7cc0e06dfddc2426c Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Wed, 7 Nov 2018 11:41:06 +0100 Subject: [PATCH 39/60] Use existing method to filter identifiers --- src/Maker/MakeDto.php | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/Maker/MakeDto.php b/src/Maker/MakeDto.php index 3444176ff..2954d8b9c 100644 --- a/src/Maker/MakeDto.php +++ b/src/Maker/MakeDto.php @@ -111,14 +111,9 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen $addHelpers = $io->confirm('Add helper extract/fill methods?'); $omitGettersSetters = $io->confirm('Omit generation of getters/setters?'); - // Filter id from fields - $fields = array_filter($fields, function ($field) { - // mapping includes id field when property is an id - if (!empty($field['id'])) { - return false; - } - - return true; + // Filter identifiers from generated fields + $fields = array_filter($fields, function ($field) use ($metaData) { + return !$metaData->isIdentifier($field['fieldName']); }); $boundClassVars = [ From e18aefd80121b37971ce985fc1a46f02af1d0a5a Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Wed, 7 Nov 2018 11:42:53 +0100 Subject: [PATCH 40/60] Use more robust method to convert constraints to annotations --- src/Maker/MakeDto.php | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/Maker/MakeDto.php b/src/Maker/MakeDto.php index 2954d8b9c..3a59065e7 100644 --- a/src/Maker/MakeDto.php +++ b/src/Maker/MakeDto.php @@ -161,7 +161,7 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen // we want to copy the asserts, so look for their interface if ($annotation instanceof Constraint) { $assertionsImported = true; - $comments[] = $manipulator->buildAnnotationLine('@Assert\\'.(new \ReflectionClass($annotation))->getShortName(), (array) $annotation); + $comments[] = $manipulator->buildAnnotationLine('@Assert\\'.(new \ReflectionClass($annotation))->getShortName(), $this->getAnnotationAsString($annotation)); } } @@ -228,4 +228,10 @@ private function getMappedFieldsInEntity(ClassMetadata $classMetadata) return $targetFields; } + + private function getAnnotationAsString(Constraint $annotation) + { + // We typecast, because array_diff expects arrays and both functions can return null. + return (array_diff((array) get_object_vars($annotation), (array) get_class_vars(get_class($annotation)))); + } } From a98a2aba82b99b004df4da7ee6ea165f5cb320c1 Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Thu, 8 Nov 2018 00:22:24 +0100 Subject: [PATCH 41/60] Update missing-getters-test to require a warning message --- tests/Maker/MakeDtoTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/Maker/MakeDtoTest.php b/tests/Maker/MakeDtoTest.php index cfc74c52d..8a50cee75 100644 --- a/tests/Maker/MakeDtoTest.php +++ b/tests/Maker/MakeDtoTest.php @@ -159,6 +159,7 @@ public function getTestDetails() $this->assertContains('created: src/Form/Data/TaskData.php', $output); $this->assertContains('updated: src/Form/Data/TaskData.php', $output); $this->assertContains('\\App\\Form\\Data\\TaskData', $output); + $this->assertContains('The maker found missing getters/setters for properties in the entity.', $output); }) ->setRequiredPhpVersion(70100), ]; From d381a733ae4f3e88f188c0763af3ac1719d9403c Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Thu, 8 Nov 2018 00:24:00 +0100 Subject: [PATCH 42/60] Prevent using missing getters/setters of an entity Check for existance of methods. Print warning and comment code out, adding @todo comments. --- src/Maker/MakeDto.php | 40 +++++++++++++++++++++++--- src/Resources/skeleton/dto/DTO.tpl.php | 28 ++++++++++++++---- 2 files changed, 58 insertions(+), 10 deletions(-) diff --git a/src/Maker/MakeDto.php b/src/Maker/MakeDto.php index 3a59065e7..a25616db7 100644 --- a/src/Maker/MakeDto.php +++ b/src/Maker/MakeDto.php @@ -96,15 +96,19 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen 'Entity\\' ); - // verify that class is an entity + // Verify that class is an entity if (false === $this->doctrineHelper->isClassAMappedEntity($boundClassDetails->getFullName())) { throw new RuntimeCommandException('The bound class is not a valid doctrine entity.'); } - // get class metadata (used to copy annotations and generate properties) + /** + * Get class metadata (used to copy annotations and generate properties). + * + * @var ClassMetaData + */ $metaData = $this->doctrineHelper->getMetadata($boundClassDetails->getFullName()); - // get list of fields + // Get list of fields $fields = $metaData->fieldMappings; // The result is passed to the template @@ -116,6 +120,17 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen return !$metaData->isIdentifier($field['fieldName']); }); + // See, whether there are missing methods + $missingGettersSetters = false; + foreach ($fields as $fieldName => $mapping) { + $fields[$fieldName]['hasSetter'] = $this->entityHasSetter($boundClassDetails->getFullName(), $fieldName); + $fields[$fieldName]['hasGetter'] = $this->entityHasGetter($boundClassDetails->getFullName(), $fieldName); + + if (!$fields[$fieldName]['hasGetter'] || !$fields[$fieldName]['hasSetter']) { + $missingGettersSetters = true; + } + } + $boundClassVars = [ 'bounded_full_class_name' => $boundClassDetails->getFullName(), 'bounded_class_name' => $boundClassDetails->getShortName(), @@ -182,6 +197,13 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen ]); } + if (true === $missingGettersSetters) { + $io->note([ + 'The maker found missing getters/setters for properties in the entity.', + 'Please review the generated DTO for @todo comments.', + ]); + } + $io->text([ 'Next: Create your form with this DTO and start using it:', '$ php bin/console make:form '.$boundClassDetails->getShortName(), @@ -232,6 +254,16 @@ private function getMappedFieldsInEntity(ClassMetadata $classMetadata) private function getAnnotationAsString(Constraint $annotation) { // We typecast, because array_diff expects arrays and both functions can return null. - return (array_diff((array) get_object_vars($annotation), (array) get_class_vars(get_class($annotation)))); + return array_diff((array) get_object_vars($annotation), (array) get_class_vars(\get_class($annotation))); + } + + private function entityHasGetter($entityClassName, $propertyName) + { + return method_exists($entityClassName, sprintf('get%s', Str::asCamelCase($propertyName))); + } + + private function entityHasSetter($entityClassName, $propertyName) + { + return method_exists($entityClassName, sprintf('set%s', Str::asCamelCase($propertyName))); } } diff --git a/src/Resources/skeleton/dto/DTO.tpl.php b/src/Resources/skeleton/dto/DTO.tpl.php index 809667d02..f3c37019f 100644 --- a/src/Resources/skeleton/dto/DTO.tpl.php +++ b/src/Resources/skeleton/dto/DTO.tpl.php @@ -33,17 +33,23 @@ public function __construct(? $ $): { - $ $mapping): ?> - ->set($this->get()) + + // @todo implement setter on the Entity + //$->set($this->get()); + + $->set($this->get()); + - ; - $ $mapping): ?> - ->set($this->) + + // @todo implement setter on the Entity + //$->set($this->); + + $->set($this->); + - ; return $; @@ -56,11 +62,21 @@ public function extract( $ $mapping): ?> + + // @todo implement getter on the Entity + //$this->set($->get()); + $this->set($->get()); + $mapping): ?> + + // @todo implement getter on the Entity + //$this-> = $->get(); + $this-> = $->get(); + From b35f0cb5a021306c6cec2c6d1fb23623208123f4 Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Thu, 8 Nov 2018 00:37:47 +0100 Subject: [PATCH 43/60] Fix coding style --- src/Resources/skeleton/dto/DTO.tpl.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Resources/skeleton/dto/DTO.tpl.php b/src/Resources/skeleton/dto/DTO.tpl.php index f3c37019f..e62dc32d4 100644 --- a/src/Resources/skeleton/dto/DTO.tpl.php +++ b/src/Resources/skeleton/dto/DTO.tpl.php @@ -1,4 +1,4 @@ - + From 44f50801f483ebacfdd82207efc119899b40d526 Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Thu, 8 Nov 2018 00:38:19 +0100 Subject: [PATCH 44/60] Reverse comparison --- src/Resources/skeleton/dto/DTO.tpl.php | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/Resources/skeleton/dto/DTO.tpl.php b/src/Resources/skeleton/dto/DTO.tpl.php index e62dc32d4..2b6473824 100644 --- a/src/Resources/skeleton/dto/DTO.tpl.php +++ b/src/Resources/skeleton/dto/DTO.tpl.php @@ -32,22 +32,22 @@ public function __construct(? $ $): { - + $mapping): ?> // @todo implement setter on the Entity - //$->set($this->get()); + //$->set($this->); - $->set($this->get()); + $->set($this->); $mapping): ?> // @todo implement setter on the Entity - //$->set($this->); + //$->set($this->get()); - $->set($this->); + $->set($this->get()); @@ -60,22 +60,22 @@ public function fill( $ $): self { - + $mapping): ?> // @todo implement getter on the Entity - //$this->set($->get()); + //$this-> = $->get(); - $this->set($->get()); + $this-> = $->get(); $mapping): ?> // @todo implement getter on the Entity - //$this-> = $->get(); + //$this->set($->get()); - $this-> = $->get(); + $this->set($->get()); From e87ebad8462717902f6abdd1d7a332b68b662715 Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Thu, 8 Nov 2018 00:49:04 +0100 Subject: [PATCH 45/60] Improve command message --- src/Maker/MakeDto.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Maker/MakeDto.php b/src/Maker/MakeDto.php index a25616db7..a081d0e7f 100644 --- a/src/Maker/MakeDto.php +++ b/src/Maker/MakeDto.php @@ -205,12 +205,12 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen } $io->text([ - 'Next: Create your form with this DTO and start using it:', - '$ php bin/console make:form '.$boundClassDetails->getShortName(), - 'Enter fully qualified data class name to bind to the form:', - '> \\'.$dataClassNameDetails->getFullName(), + sprintf('Next: Review the new DTO %s', $DTOClassPath), + 'Then: Create a form for this DTO by running:', + sprintf('$ php bin/console make:form %s', $boundClassDetails->getShortName()), + sprintf('and enter \\%s', $dataClassNameDetails->getFullName()), '', - 'Find the documentation at https://symfony.com/doc/current/forms.html', + 'Find the documentation at https://symfony.com/doc/current/forms/data_transfer_objects.html', ]); } From 4888dbde9d771de138499250df54a5f6c63c017e Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Thu, 8 Nov 2018 12:50:26 +0100 Subject: [PATCH 46/60] Add test for warning when using yaml/xml validation definitions --- tests/Maker/MakeDtoTest.php | 22 +++++ .../config/validator/validation.xml | 12 +++ .../config/validator/validation.yaml | 4 + .../src/Entity/Task.php | 84 +++++++++++++++++++ .../tests/GeneratedDtoTest.php | 67 +++++++++++++++ 5 files changed, 189 insertions(+) create mode 100644 tests/fixtures/MakeDtoValidatorYamlXml/config/validator/validation.xml create mode 100644 tests/fixtures/MakeDtoValidatorYamlXml/config/validator/validation.yaml create mode 100644 tests/fixtures/MakeDtoValidatorYamlXml/src/Entity/Task.php create mode 100644 tests/fixtures/MakeDtoValidatorYamlXml/tests/GeneratedDtoTest.php diff --git a/tests/Maker/MakeDtoTest.php b/tests/Maker/MakeDtoTest.php index 8a50cee75..016db7acd 100644 --- a/tests/Maker/MakeDtoTest.php +++ b/tests/Maker/MakeDtoTest.php @@ -60,6 +60,28 @@ public function getTestDetails() ->setRequiredPhpVersion(70100), ]; + yield 'dto_validator_yaml_xml' => [MakerTestDetails::createTest( + $this->getMakerInstance(MakeDto::class), + [ + 'Task', + 'Task', + // generate helpers + 'yes', + // omit getters + 'yes', + ]) + ->addExtraDependencies('orm') + ->addExtraDependencies('validator') + ->addExtraDependencies('yaml') + ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeDtoValidatorYamlXml') + ->assert(function (string $output, string $directory) { + $this->assertContains('created: src/Form/Data/TaskData.php', $output); + $this->assertContains('updated: src/Form/Data/TaskData.php', $output); + $this->assertContains('The entity possibly uses Yaml/Xml validators.', $output); + }) + ->setRequiredPhpVersion(70100) + ]; + yield 'dto_without_helpers' => [MakerTestDetails::createTest( $this->getMakerInstance(MakeDto::class), [ diff --git a/tests/fixtures/MakeDtoValidatorYamlXml/config/validator/validation.xml b/tests/fixtures/MakeDtoValidatorYamlXml/config/validator/validation.xml new file mode 100644 index 000000000..3273ee8c0 --- /dev/null +++ b/tests/fixtures/MakeDtoValidatorYamlXml/config/validator/validation.xml @@ -0,0 +1,12 @@ + + + + + + + + + diff --git a/tests/fixtures/MakeDtoValidatorYamlXml/config/validator/validation.yaml b/tests/fixtures/MakeDtoValidatorYamlXml/config/validator/validation.yaml new file mode 100644 index 000000000..356348e57 --- /dev/null +++ b/tests/fixtures/MakeDtoValidatorYamlXml/config/validator/validation.yaml @@ -0,0 +1,4 @@ +App\Entity\Task: + properties: + dueDate: + - Type: DateTime diff --git a/tests/fixtures/MakeDtoValidatorYamlXml/src/Entity/Task.php b/tests/fixtures/MakeDtoValidatorYamlXml/src/Entity/Task.php new file mode 100644 index 000000000..8b7a20595 --- /dev/null +++ b/tests/fixtures/MakeDtoValidatorYamlXml/src/Entity/Task.php @@ -0,0 +1,84 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace App\Entity; + +use Doctrine\ORM\Mapping as ORM; +use Symfony\Component\Validator\Constraints as Assert; + +/** + * @ORM\Entity() + */ +class Task +{ + /** + * @ORM\Id + * @ORM\GeneratedValue + * @ORM\Column(type="integer") + */ + private $id; + + /** + * @ORM\Column(type="string", length=255, nullable=true) + * @Assert\Type("string") + */ + private $task; + + /** + * @ORM\Column(type="datetime", nullable=true) + */ + private $dueDate; + + public function getId() + { + return $this->id; + } + + /** + * Get the value of task. + */ + public function getTask() + { + return $this->task; + } + + /** + * Set the value of task. + * + * @return self + */ + public function setTask($task) + { + $this->task = $task; + + return $this; + } + + /** + * Get the value of dueDate. + */ + public function getDueDate() + { + return $this->dueDate; + } + + /** + * Set the value of dueDate. + * + * @return self + */ + public function setDueDate($dueDate) + { + $this->dueDate = $dueDate; + + return $this; + } +} diff --git a/tests/fixtures/MakeDtoValidatorYamlXml/tests/GeneratedDtoTest.php b/tests/fixtures/MakeDtoValidatorYamlXml/tests/GeneratedDtoTest.php new file mode 100644 index 000000000..6f3ecda0b --- /dev/null +++ b/tests/fixtures/MakeDtoValidatorYamlXml/tests/GeneratedDtoTest.php @@ -0,0 +1,67 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace App\Tests; + +use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; +use App\Entity\Task; +use App\Form\Data\TaskData; +use Doctrine\Common\Annotations\AnnotationReader; +use Symfony\Component\Validator\Constraints\Type; +use Symfony\Component\Validator\Validation; + +/** + * @requires PHP 7.1 + */ +class GeneratedDtoTest extends KernelTestCase +{ + public function testGeneratedDto() + { + // simply test if DTO validates with only the field with annotation being valid. + $taskData = new TaskData(); + + // valid + $taskData->task = 'foobar'; + + // invalid, but only with constraint from validation.yaml + $taskData->dueDate = 'foo'; + + // create validator + + $validatorBuilder = Validation::createValidatorBuilder() + ->enableAnnotationMapping() + ->addXmlMapping(__DIR__.'/../config/validator/validation.xml') + ->addYamlMapping('config/validator/validation.yaml') + ->getValidator(); + + $this->assertEmpty($validator->validate($taskData)); + + // invalid + $taskData->task = 123; + + $this->assertCount(1, $validator->validate($taskData)); + } + + public function testAnnotations() + { + // "task" property may only have Type constraint from annotation + $annotationReader = new AnnotationReader(); + $reflectionProperty = new \ReflectionProperty(TaskData::class, 'task'); + $propertyAnnotations = $annotationReader->getPropertyAnnotations($reflectionProperty); + $this->assertCount(1, $propertyAnnotations); + $this->assertContainsOnlyInstancesOf(Type::class, $propertyAnnotations); + + // dueDate may not have an annotation + $reflectionProperty = new \ReflectionProperty(TaskData::class, 'dueDate'); + $propertyAnnotations = $annotationReader->getPropertyAnnotations($reflectionProperty); + $this->assertCount(0, $propertyAnnotations); + } +} From ac7a37e8c3b2ff1cccad6691f9da2c449a7f7737 Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Thu, 8 Nov 2018 13:39:27 +0100 Subject: [PATCH 47/60] Fix test for generated DTO --- .../MakeDtoValidatorYamlXml/tests/GeneratedDtoTest.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/fixtures/MakeDtoValidatorYamlXml/tests/GeneratedDtoTest.php b/tests/fixtures/MakeDtoValidatorYamlXml/tests/GeneratedDtoTest.php index 6f3ecda0b..8edba197a 100644 --- a/tests/fixtures/MakeDtoValidatorYamlXml/tests/GeneratedDtoTest.php +++ b/tests/fixtures/MakeDtoValidatorYamlXml/tests/GeneratedDtoTest.php @@ -36,10 +36,8 @@ public function testGeneratedDto() // create validator - $validatorBuilder = Validation::createValidatorBuilder() + $validator = Validation::createValidatorBuilder() ->enableAnnotationMapping() - ->addXmlMapping(__DIR__.'/../config/validator/validation.xml') - ->addYamlMapping('config/validator/validation.yaml') ->getValidator(); $this->assertEmpty($validator->validate($taskData)); From 4e5877dd0483a8ebc2d4932fef1e366998b5a816 Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Thu, 8 Nov 2018 18:26:40 +0100 Subject: [PATCH 48/60] Use validator to compare annotation validations The validator is used to get all validations for a property. This is counted and compared against the validations from annotations in the entity class. --- src/Maker/MakeDto.php | 40 ++++++++++++++++++- src/Resources/config/makers.xml | 1 + .../tests/GeneratedDtoTest.php | 1 - 3 files changed, 39 insertions(+), 3 deletions(-) diff --git a/src/Maker/MakeDto.php b/src/Maker/MakeDto.php index a081d0e7f..ecda01708 100644 --- a/src/Maker/MakeDto.php +++ b/src/Maker/MakeDto.php @@ -29,6 +29,7 @@ use Symfony\Component\Console\Question\Question; use Symfony\Component\Validator\Constraint; use Symfony\Component\Validator\Validation; +use Symfony\Component\Validator\Validator\ValidatorInterface; /** * @author Javier Eguiluz @@ -39,13 +40,16 @@ final class MakeDto extends AbstractMaker { private $doctrineHelper; private $fileManager; + private $validator; public function __construct( DoctrineHelper $doctrineHelper, - FileManager $fileManager + FileManager $fileManager, + ValidatorInterface $validator ) { $this->doctrineHelper = $doctrineHelper; $this->fileManager = $fileManager; + $this->validator = $validator; } public static function getCommandName(): string @@ -156,6 +160,10 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen // Did we import assert annotations? $assertionsImported = false; + // Are there differences in the validation constraints between metadata (includes annotations, xml, yaml) and annotations? + $suspectYamlXmlValidations = false; + $validatorClassMetadata = $this->validator->getMetadataFor($boundClassDetails->getFullName()); + foreach ($fields as $fieldName => $mapping) { $annotationReader = new AnnotationReader(); @@ -170,16 +178,26 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen $reflectionProperty = new \ReflectionProperty($fullClassName, $fieldName); $propertyAnnotations = $annotationReader->getPropertyAnnotations($reflectionProperty); + // passed to the ClassManipulator $comments = []; + // Count the Constraints for comparison with the Validator + $constraintCount = 0; foreach ($propertyAnnotations as $annotation) { - // we want to copy the asserts, so look for their interface + // We want to copy the asserts, so look for their interface if ($annotation instanceof Constraint) { + // Set flag for use in result message $assertionsImported = true; + $constraintCount++; $comments[] = $manipulator->buildAnnotationLine('@Assert\\'.(new \ReflectionClass($annotation))->getShortName(), $this->getAnnotationAsString($annotation)); } } + // Compare the amount of constraints in annotations with those in the complete validator-metadata for the entity + if (false === $this->hasAsManyValidations($validatorClassMetadata->getPropertyMetadata($fieldName), $constraintCount)) { + $suspectYamlXmlValidations = true; + } + $manipulator->addEntityField($fieldName, $mapping, $comments); } @@ -197,6 +215,13 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen ]); } + if (true === $suspectYamlXmlValidations) { + $io->note([ + 'The entity possibly uses Yaml/Xml validators.', + 'Make sure to update the validations to include the new DTO class.', + ]); + } + if (true === $missingGettersSetters) { $io->note([ 'The maker found missing getters/setters for properties in the entity.', @@ -257,6 +282,17 @@ private function getAnnotationAsString(Constraint $annotation) return array_diff((array) get_object_vars($annotation), (array) get_class_vars(\get_class($annotation))); } + private function hasAsManyValidations($propertyMetadata, $constraintCount) + { + $metadataConstraintCount = 0; + foreach ($propertyMetadata as $metadata) { + if (isset($metadata->constraints)) { + $metadataConstraintCount = $metadataConstraintCount + count($metadata->constraints); + } + } + return ($metadataConstraintCount == $constraintCount); + } + private function entityHasGetter($entityClassName, $propertyName) { return method_exists($entityClassName, sprintf('get%s', Str::asCamelCase($propertyName))); diff --git a/src/Resources/config/makers.xml b/src/Resources/config/makers.xml index a4ce1f427..6281056c7 100644 --- a/src/Resources/config/makers.xml +++ b/src/Resources/config/makers.xml @@ -10,6 +10,7 @@ + diff --git a/tests/fixtures/MakeDtoValidatorYamlXml/tests/GeneratedDtoTest.php b/tests/fixtures/MakeDtoValidatorYamlXml/tests/GeneratedDtoTest.php index 8edba197a..b297ee578 100644 --- a/tests/fixtures/MakeDtoValidatorYamlXml/tests/GeneratedDtoTest.php +++ b/tests/fixtures/MakeDtoValidatorYamlXml/tests/GeneratedDtoTest.php @@ -35,7 +35,6 @@ public function testGeneratedDto() $taskData->dueDate = 'foo'; // create validator - $validator = Validation::createValidatorBuilder() ->enableAnnotationMapping() ->getValidator(); From f9a29e591477fb1e75f8effd641402456037eb79 Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Thu, 8 Nov 2018 19:09:31 +0100 Subject: [PATCH 49/60] php-cs-fix --- src/Maker/MakeDto.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Maker/MakeDto.php b/src/Maker/MakeDto.php index ecda01708..61ee77a0b 100644 --- a/src/Maker/MakeDto.php +++ b/src/Maker/MakeDto.php @@ -188,7 +188,7 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen if ($annotation instanceof Constraint) { // Set flag for use in result message $assertionsImported = true; - $constraintCount++; + ++$constraintCount; $comments[] = $manipulator->buildAnnotationLine('@Assert\\'.(new \ReflectionClass($annotation))->getShortName(), $this->getAnnotationAsString($annotation)); } } @@ -287,10 +287,11 @@ private function hasAsManyValidations($propertyMetadata, $constraintCount) $metadataConstraintCount = 0; foreach ($propertyMetadata as $metadata) { if (isset($metadata->constraints)) { - $metadataConstraintCount = $metadataConstraintCount + count($metadata->constraints); + $metadataConstraintCount = $metadataConstraintCount + \count($metadata->constraints); } } - return ($metadataConstraintCount == $constraintCount); + + return $metadataConstraintCount == $constraintCount; } private function entityHasGetter($entityClassName, $propertyName) From 3b6622039b7b8508b72c1253e857f2574278b80f Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Fri, 9 Nov 2018 15:42:20 +0100 Subject: [PATCH 50/60] Remove validator dependency Refactor Maker to allow independency of validator. Remove default use statement from template. Make validator optional in constructor. Add tests without validator. --- src/Maker/MakeDto.php | 30 +++++-- src/Resources/config/makers.xml | 2 +- src/Resources/skeleton/dto/DTO.tpl.php | 1 - src/Util/DTOClassSourceManipulator.php | 4 +- tests/Maker/MakeDtoTest.php | 19 +++++ .../src/Entity/Task.php | 82 +++++++++++++++++++ .../tests/GeneratedDtoTest.php | 28 +++++++ 7 files changed, 156 insertions(+), 10 deletions(-) create mode 100644 tests/fixtures/MakeDtoWithoutValidations/src/Entity/Task.php create mode 100644 tests/fixtures/MakeDtoWithoutValidations/tests/GeneratedDtoTest.php diff --git a/src/Maker/MakeDto.php b/src/Maker/MakeDto.php index 61ee77a0b..c6bdcd6aa 100644 --- a/src/Maker/MakeDto.php +++ b/src/Maker/MakeDto.php @@ -32,8 +32,6 @@ use Symfony\Component\Validator\Validator\ValidatorInterface; /** - * @author Javier Eguiluz - * @author Ryan Weaver * @author Clemens Krack */ final class MakeDto extends AbstractMaker @@ -41,11 +39,12 @@ final class MakeDto extends AbstractMaker private $doctrineHelper; private $fileManager; private $validator; + private $validatorClassMetadata; public function __construct( DoctrineHelper $doctrineHelper, FileManager $fileManager, - ValidatorInterface $validator + ValidatorInterface $validator = null ) { $this->doctrineHelper = $doctrineHelper; $this->fileManager = $fileManager; @@ -162,7 +161,6 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen // Are there differences in the validation constraints between metadata (includes annotations, xml, yaml) and annotations? $suspectYamlXmlValidations = false; - $validatorClassMetadata = $this->validator->getMetadataFor($boundClassDetails->getFullName()); foreach ($fields as $fieldName => $mapping) { $annotationReader = new AnnotationReader(); @@ -178,8 +176,9 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen $reflectionProperty = new \ReflectionProperty($fullClassName, $fieldName); $propertyAnnotations = $annotationReader->getPropertyAnnotations($reflectionProperty); - // passed to the ClassManipulator + // Passed to the ClassManipulator $comments = []; + // Count the Constraints for comparison with the Validator $constraintCount = 0; @@ -194,13 +193,19 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen } // Compare the amount of constraints in annotations with those in the complete validator-metadata for the entity - if (false === $this->hasAsManyValidations($validatorClassMetadata->getPropertyMetadata($fieldName), $constraintCount)) { + if (false === $this->hasAsManyValidations($boundClassDetails->getFullName(), $fieldName, $constraintCount)) { $suspectYamlXmlValidations = true; } $manipulator->addEntityField($fieldName, $mapping, $comments); } + // Add use statement for validation annotations if necessary + if (true == $assertionsImported) { + // The use of an alias is not supposed, but it works fine and we don't use the returned value. + $manipulator->addUseStatementIfNecessary('Symfony\Component\Validator\Constraints as Assert'); + } + $this->fileManager->dumpFile( $DTOClassPath, $manipulator->getSourceCode() @@ -282,8 +287,19 @@ private function getAnnotationAsString(Constraint $annotation) return array_diff((array) get_object_vars($annotation), (array) get_class_vars(\get_class($annotation))); } - private function hasAsManyValidations($propertyMetadata, $constraintCount) + private function hasAsManyValidations($entityClassname, $fieldName, $constraintCount) { + if (null === $this->validator) { + return 0 == $constraintCount; + } + + // lazily build validatorClassMetadata + if (null === $this->validatorClassMetadata) { + $this->validatorClassMetadata = $this->validator->getMetadataFor($entityClassname); + } + + $propertyMetadata = $this->validatorClassMetadata->getPropertyMetadata($fieldName); + $metadataConstraintCount = 0; foreach ($propertyMetadata as $metadata) { if (isset($metadata->constraints)) { diff --git a/src/Resources/config/makers.xml b/src/Resources/config/makers.xml index 6281056c7..7780cc065 100644 --- a/src/Resources/config/makers.xml +++ b/src/Resources/config/makers.xml @@ -10,7 +10,7 @@ - + diff --git a/src/Resources/skeleton/dto/DTO.tpl.php b/src/Resources/skeleton/dto/DTO.tpl.php index 2b6473824..824147f66 100644 --- a/src/Resources/skeleton/dto/DTO.tpl.php +++ b/src/Resources/skeleton/dto/DTO.tpl.php @@ -8,7 +8,6 @@ use ; -use Symfony\Component\Validator\Constraints as Assert; /** * Data transfer object for . diff --git a/src/Util/DTOClassSourceManipulator.php b/src/Util/DTOClassSourceManipulator.php index 2094fad28..102382a77 100644 --- a/src/Util/DTOClassSourceManipulator.php +++ b/src/Util/DTOClassSourceManipulator.php @@ -333,11 +333,13 @@ private function getConstructorNode() } /** + * Modified to public from ClassSourceManipulator. + * * @param string $class * * @return string The alias to use when referencing this class */ - private function addUseStatementIfNecessary(string $class): string + public function addUseStatementIfNecessary(string $class): string { $shortClassName = Str::getShortClassName($class); if ($this->isInSameNamespace($class)) { diff --git a/tests/Maker/MakeDtoTest.php b/tests/Maker/MakeDtoTest.php index 016db7acd..ff9cdaac8 100644 --- a/tests/Maker/MakeDtoTest.php +++ b/tests/Maker/MakeDtoTest.php @@ -102,6 +102,25 @@ public function getTestDetails() ->setRequiredPhpVersion(70100), ]; + yield 'dto_without_validations' => [MakerTestDetails::createTest( + $this->getMakerInstance(MakeDto::class), + [ + 'Task', + 'Task', + // generate helpers + 'yes', + // omit getters + 'yes', + ]) + ->addExtraDependencies('orm') + ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeDtoWithoutValidations') + ->assert(function (string $output, string $directory) { + $this->assertContains('created: src/Form/Data/TaskData.php', $output); + $this->assertContains('updated: src/Form/Data/TaskData.php', $output); + }) + ->setRequiredPhpVersion(70100) + ]; + yield 'dto_invalid_entity' => [MakerTestDetails::createTest( $this->getMakerInstance(MakeDto::class), [ diff --git a/tests/fixtures/MakeDtoWithoutValidations/src/Entity/Task.php b/tests/fixtures/MakeDtoWithoutValidations/src/Entity/Task.php new file mode 100644 index 000000000..40734ed1c --- /dev/null +++ b/tests/fixtures/MakeDtoWithoutValidations/src/Entity/Task.php @@ -0,0 +1,82 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace App\Entity; + +use Doctrine\ORM\Mapping as ORM; + +/** + * @ORM\Entity() + */ +class Task +{ + /** + * @ORM\Id + * @ORM\GeneratedValue + * @ORM\Column(type="integer") + */ + private $id; + + /** + * @ORM\Column(type="string", length=255, nullable=true) + */ + private $task; + + /** + * @ORM\Column(type="datetime", nullable=true) + */ + private $dueDate; + + public function getId() + { + return $this->id; + } + + /** + * Get the value of task. + */ + public function getTask() + { + return $this->task; + } + + /** + * Set the value of task. + * + * @return self + */ + public function setTask($task) + { + $this->task = $task; + + return $this; + } + + /** + * Get the value of dueDate. + */ + public function getDueDate() + { + return $this->dueDate; + } + + /** + * Set the value of dueDate. + * + * @return self + */ + public function setDueDate($dueDate) + { + $this->dueDate = $dueDate; + + return $this; + } +} diff --git a/tests/fixtures/MakeDtoWithoutValidations/tests/GeneratedDtoTest.php b/tests/fixtures/MakeDtoWithoutValidations/tests/GeneratedDtoTest.php new file mode 100644 index 000000000..001194f28 --- /dev/null +++ b/tests/fixtures/MakeDtoWithoutValidations/tests/GeneratedDtoTest.php @@ -0,0 +1,28 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace App\Tests; + +use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; +use App\Form\Data\TaskData; + +/** + * @requires PHP 7.1 + */ +class GeneratedDtoTest extends KernelTestCase +{ + public function testGeneratedDto() + { + $this->assertClassHasAttribute('task', TaskData::class); + $this->assertClassHasAttribute('dueDate', TaskData::class); + $this->assertClassNotHasAttribute('id', TaskData::class); + } +} From 3a9f82851e8d7f8e698393c8a27ec47bb0ff8d34 Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Fri, 9 Nov 2018 18:02:37 +0100 Subject: [PATCH 51/60] Change namespace of generated DTO --- src/Maker/MakeDto.php | 2 +- tests/Maker/MakeDtoTest.php | 40 +++++++++---------- .../MakeDto/tests/GeneratedDtoTest.php | 2 +- .../tests/GeneratedDtoTest.php | 2 +- .../tests/GeneratedDtoTest.php | 2 +- .../tests/GeneratedDtoTest.php | 2 +- .../tests/GeneratedDtoTest.php | 2 +- .../tests/GeneratedDtoTest.php | 2 +- .../tests/GeneratedDtoTest.php | 2 +- .../tests/GeneratedDtoTest.php | 2 +- 10 files changed, 29 insertions(+), 29 deletions(-) diff --git a/src/Maker/MakeDto.php b/src/Maker/MakeDto.php index c6bdcd6aa..c1da463e5 100644 --- a/src/Maker/MakeDto.php +++ b/src/Maker/MakeDto.php @@ -88,7 +88,7 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen { $dataClassNameDetails = $generator->createClassNameDetails( $input->getArgument('name'), - 'Form\\Data\\', + 'Dto\\', 'Data' ); diff --git a/tests/Maker/MakeDtoTest.php b/tests/Maker/MakeDtoTest.php index ff9cdaac8..ee3f74f13 100644 --- a/tests/Maker/MakeDtoTest.php +++ b/tests/Maker/MakeDtoTest.php @@ -33,9 +33,9 @@ public function getTestDetails() ->addExtraDependencies('validator') ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeDto') ->assert(function (string $output, string $directory) { - $this->assertContains('created: src/Form/Data/TaskData.php', $output); - $this->assertContains('updated: src/Form/Data/TaskData.php', $output); - $this->assertContains('\\App\\Form\\Data\\TaskData', $output); + $this->assertContains('created: src/Dto/TaskData.php', $output); + $this->assertContains('updated: src/Dto/TaskData.php', $output); + $this->assertContains('\\App\\Dto\\TaskData', $output); }) ->setRequiredPhpVersion(70100), ]; @@ -54,8 +54,8 @@ public function getTestDetails() ->addExtraDependencies('validator') ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeDtoGettersSetters') ->assert(function (string $output, string $directory) { - $this->assertContains('created: src/Form/Data/TaskData.php', $output); - $this->assertContains('updated: src/Form/Data/TaskData.php', $output); + $this->assertContains('created: src/Dto/TaskData.php', $output); + $this->assertContains('updated: src/Dto/TaskData.php', $output); }) ->setRequiredPhpVersion(70100), ]; @@ -75,8 +75,8 @@ public function getTestDetails() ->addExtraDependencies('yaml') ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeDtoValidatorYamlXml') ->assert(function (string $output, string $directory) { - $this->assertContains('created: src/Form/Data/TaskData.php', $output); - $this->assertContains('updated: src/Form/Data/TaskData.php', $output); + $this->assertContains('created: src/Dto/TaskData.php', $output); + $this->assertContains('updated: src/Dto/TaskData.php', $output); $this->assertContains('The entity possibly uses Yaml/Xml validators.', $output); }) ->setRequiredPhpVersion(70100) @@ -96,8 +96,8 @@ public function getTestDetails() ->addExtraDependencies('validator') ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeDtoWithoutHelpers') ->assert(function (string $output, string $directory) { - $this->assertContains('created: src/Form/Data/TaskData.php', $output); - $this->assertContains('updated: src/Form/Data/TaskData.php', $output); + $this->assertContains('created: src/Dto/TaskData.php', $output); + $this->assertContains('updated: src/Dto/TaskData.php', $output); }) ->setRequiredPhpVersion(70100), ]; @@ -115,8 +115,8 @@ public function getTestDetails() ->addExtraDependencies('orm') ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeDtoWithoutValidations') ->assert(function (string $output, string $directory) { - $this->assertContains('created: src/Form/Data/TaskData.php', $output); - $this->assertContains('updated: src/Form/Data/TaskData.php', $output); + $this->assertContains('created: src/Dto/TaskData.php', $output); + $this->assertContains('updated: src/Dto/TaskData.php', $output); }) ->setRequiredPhpVersion(70100) ]; @@ -155,9 +155,9 @@ public function getTestDetails() ->addExtraDependencies('validator') ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeDtoMappedSuperClass') ->assert(function (string $output, string $directory) { - $this->assertContains('created: src/Form/Data/TaskData.php', $output); - $this->assertContains('updated: src/Form/Data/TaskData.php', $output); - $this->assertContains('\\App\\Form\\Data\\TaskData', $output); + $this->assertContains('created: src/Dto/TaskData.php', $output); + $this->assertContains('updated: src/Dto/TaskData.php', $output); + $this->assertContains('\\App\\Dto\\TaskData', $output); }) ->setRequiredPhpVersion(70100), ]; @@ -176,9 +176,9 @@ public function getTestDetails() ->addExtraDependencies('validator') ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeDtoCompositeId') ->assert(function (string $output, string $directory) { - $this->assertContains('created: src/Form/Data/TaskData.php', $output); - $this->assertContains('updated: src/Form/Data/TaskData.php', $output); - $this->assertContains('\\App\\Form\\Data\\TaskData', $output); + $this->assertContains('created: src/Dto/TaskData.php', $output); + $this->assertContains('updated: src/Dto/TaskData.php', $output); + $this->assertContains('\\App\\Dto\\TaskData', $output); }) ->setRequiredPhpVersion(70100), ]; @@ -197,9 +197,9 @@ public function getTestDetails() ->addExtraDependencies('validator') ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeDtoMissingGettersSetters') ->assert(function (string $output, string $directory) { - $this->assertContains('created: src/Form/Data/TaskData.php', $output); - $this->assertContains('updated: src/Form/Data/TaskData.php', $output); - $this->assertContains('\\App\\Form\\Data\\TaskData', $output); + $this->assertContains('created: src/Dto/TaskData.php', $output); + $this->assertContains('updated: src/Dto/TaskData.php', $output); + $this->assertContains('\\App\\Dto\\TaskData', $output); $this->assertContains('The maker found missing getters/setters for properties in the entity.', $output); }) ->setRequiredPhpVersion(70100), diff --git a/tests/fixtures/MakeDto/tests/GeneratedDtoTest.php b/tests/fixtures/MakeDto/tests/GeneratedDtoTest.php index 1b0bd1921..a469021b9 100644 --- a/tests/fixtures/MakeDto/tests/GeneratedDtoTest.php +++ b/tests/fixtures/MakeDto/tests/GeneratedDtoTest.php @@ -13,7 +13,7 @@ use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; use App\Entity\Task; -use App\Form\Data\TaskData; +use App\Dto\TaskData; use Doctrine\Common\Annotations\AnnotationReader; use Symfony\Component\Validator\Constraints\NotBlank; diff --git a/tests/fixtures/MakeDtoCompositeId/tests/GeneratedDtoTest.php b/tests/fixtures/MakeDtoCompositeId/tests/GeneratedDtoTest.php index c71c7bff0..967cb2e57 100644 --- a/tests/fixtures/MakeDtoCompositeId/tests/GeneratedDtoTest.php +++ b/tests/fixtures/MakeDtoCompositeId/tests/GeneratedDtoTest.php @@ -13,7 +13,7 @@ use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; use App\Entity\Task; -use App\Form\Data\TaskData; +use App\Dto\TaskData; use Doctrine\Common\Annotations\AnnotationReader; use Symfony\Component\Validator\Constraints\NotBlank; diff --git a/tests/fixtures/MakeDtoGettersSetters/tests/GeneratedDtoTest.php b/tests/fixtures/MakeDtoGettersSetters/tests/GeneratedDtoTest.php index 5ed05050a..ed304dc29 100644 --- a/tests/fixtures/MakeDtoGettersSetters/tests/GeneratedDtoTest.php +++ b/tests/fixtures/MakeDtoGettersSetters/tests/GeneratedDtoTest.php @@ -13,7 +13,7 @@ use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; use App\Entity\Task; -use App\Form\Data\TaskData; +use App\Dto\TaskData; /** * @requires PHP 7.1 diff --git a/tests/fixtures/MakeDtoMappedSuperClass/tests/GeneratedDtoTest.php b/tests/fixtures/MakeDtoMappedSuperClass/tests/GeneratedDtoTest.php index 1b0bd1921..a469021b9 100644 --- a/tests/fixtures/MakeDtoMappedSuperClass/tests/GeneratedDtoTest.php +++ b/tests/fixtures/MakeDtoMappedSuperClass/tests/GeneratedDtoTest.php @@ -13,7 +13,7 @@ use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; use App\Entity\Task; -use App\Form\Data\TaskData; +use App\Dto\TaskData; use Doctrine\Common\Annotations\AnnotationReader; use Symfony\Component\Validator\Constraints\NotBlank; diff --git a/tests/fixtures/MakeDtoMissingGettersSetters/tests/GeneratedDtoTest.php b/tests/fixtures/MakeDtoMissingGettersSetters/tests/GeneratedDtoTest.php index 240795021..60e0fff7a 100644 --- a/tests/fixtures/MakeDtoMissingGettersSetters/tests/GeneratedDtoTest.php +++ b/tests/fixtures/MakeDtoMissingGettersSetters/tests/GeneratedDtoTest.php @@ -12,7 +12,7 @@ namespace App\Tests; use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; -use App\Form\Data\TaskData; +use App\Dto\TaskData; use App\Entity\Task; /** diff --git a/tests/fixtures/MakeDtoValidatorYamlXml/tests/GeneratedDtoTest.php b/tests/fixtures/MakeDtoValidatorYamlXml/tests/GeneratedDtoTest.php index b297ee578..31f127b5a 100644 --- a/tests/fixtures/MakeDtoValidatorYamlXml/tests/GeneratedDtoTest.php +++ b/tests/fixtures/MakeDtoValidatorYamlXml/tests/GeneratedDtoTest.php @@ -13,7 +13,7 @@ use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; use App\Entity\Task; -use App\Form\Data\TaskData; +use App\Dto\TaskData; use Doctrine\Common\Annotations\AnnotationReader; use Symfony\Component\Validator\Constraints\Type; use Symfony\Component\Validator\Validation; diff --git a/tests/fixtures/MakeDtoWithoutHelpers/tests/GeneratedDtoTest.php b/tests/fixtures/MakeDtoWithoutHelpers/tests/GeneratedDtoTest.php index 3e398b924..7387fca51 100644 --- a/tests/fixtures/MakeDtoWithoutHelpers/tests/GeneratedDtoTest.php +++ b/tests/fixtures/MakeDtoWithoutHelpers/tests/GeneratedDtoTest.php @@ -12,7 +12,7 @@ namespace App\Tests; use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; -use App\Form\Data\TaskData; +use App\Dto\TaskData; /** * @requires PHP 7.1 diff --git a/tests/fixtures/MakeDtoWithoutValidations/tests/GeneratedDtoTest.php b/tests/fixtures/MakeDtoWithoutValidations/tests/GeneratedDtoTest.php index 001194f28..7b5f08bd7 100644 --- a/tests/fixtures/MakeDtoWithoutValidations/tests/GeneratedDtoTest.php +++ b/tests/fixtures/MakeDtoWithoutValidations/tests/GeneratedDtoTest.php @@ -12,7 +12,7 @@ namespace App\Tests; use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; -use App\Form\Data\TaskData; +use App\Dto\TaskData; /** * @requires PHP 7.1 From 0e0c831a4b6867d11f3fc9b47a380faa0f49f0d4 Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Mon, 3 Dec 2018 11:52:24 +0100 Subject: [PATCH 52/60] Reverse question for getter/setter generation Keep the default (false). Reverse question to make it more natural language wise. Fix conditions to keep code simple to read. --- src/Maker/MakeDto.php | 12 ++++----- src/Resources/skeleton/dto/DTO.tpl.php | 20 +++++++------- src/Util/DTOClassSourceManipulator.php | 8 +++--- tests/Maker/MakeDtoTest.php | 36 +++++++++++++------------- 4 files changed, 38 insertions(+), 38 deletions(-) diff --git a/src/Maker/MakeDto.php b/src/Maker/MakeDto.php index c1da463e5..71ffc5fa7 100644 --- a/src/Maker/MakeDto.php +++ b/src/Maker/MakeDto.php @@ -116,7 +116,7 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen // The result is passed to the template $addHelpers = $io->confirm('Add helper extract/fill methods?'); - $omitGettersSetters = $io->confirm('Omit generation of getters/setters?'); + $generateGettersSetters = $io->confirm('Generate getters/setters?', false); // Filter identifiers from generated fields $fields = array_filter($fields, function ($field) use ($metaData) { @@ -146,14 +146,14 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen [ 'fields' => $fields, 'addHelpers' => $addHelpers, - 'omitGettersSetters' => $omitGettersSetters, + 'generateGettersSetters' => $generateGettersSetters, ], $boundClassVars ) ); $generator->writeChanges(); - $manipulator = $this->createClassManipulator($DTOClassPath, $omitGettersSetters); + $manipulator = $this->createClassManipulator($DTOClassPath, $generateGettersSetters); $mappedFields = $this->getMappedFieldsInEntity($metaData); // Did we import assert annotations? @@ -256,7 +256,7 @@ public function configureDependencies(DependencyBuilder $dependencies) ); } - private function createClassManipulator(string $classPath, bool $omitGettersSetters = false): DTOClassSourceManipulator + private function createClassManipulator(string $classPath, bool $generateGettersSetters = false): DTOClassSourceManipulator { return new DTOClassSourceManipulator( $this->fileManager->getFileContents($classPath), @@ -266,8 +266,8 @@ private function createClassManipulator(string $classPath, bool $omitGettersSett true, // use fluent mutators true, - // omit getters setters? - $omitGettersSetters + // generate getters setters? + $generateGettersSetters ); } diff --git a/src/Resources/skeleton/dto/DTO.tpl.php b/src/Resources/skeleton/dto/DTO.tpl.php index 824147f66..3aac34896 100644 --- a/src/Resources/skeleton/dto/DTO.tpl.php +++ b/src/Resources/skeleton/dto/DTO.tpl.php @@ -31,22 +31,22 @@ public function __construct(? $ $): { - + $mapping): ?> // @todo implement setter on the Entity - //$->set($this->); + //$->set($this->get()); - $->set($this->); + $->set($this->get()); $mapping): ?> // @todo implement setter on the Entity - //$->set($this->get()); + //$->set($this->); - $->set($this->get()); + $->set($this->); @@ -59,22 +59,22 @@ public function fill( $ $): self { - + $mapping): ?> // @todo implement getter on the Entity - //$this-> = $->get(); + //$this->set($->get()); - $this-> = $->get(); + $this->set($->get()); $mapping): ?> // @todo implement getter on the Entity - //$this->set($->get()); + //$this-> = $->get(); - $this->set($->get()); + $this-> = $->get(); diff --git a/src/Util/DTOClassSourceManipulator.php b/src/Util/DTOClassSourceManipulator.php index 102382a77..a889e7957 100644 --- a/src/Util/DTOClassSourceManipulator.php +++ b/src/Util/DTOClassSourceManipulator.php @@ -46,11 +46,11 @@ final class DTOClassSourceManipulator private $pendingComments = []; - public function __construct(string $sourceCode, bool $overwrite = false, bool $useAnnotations = true, bool $fluentMutators = true, bool $omitGettersSetters = false) + public function __construct(string $sourceCode, bool $overwrite = false, bool $useAnnotations = true, bool $fluentMutators = true, bool $generateGettersSetters = true) { $this->overwrite = $overwrite; $this->useAnnotations = $useAnnotations; - $this->omitGettersSetters = $omitGettersSetters; + $this->generateGettersSetters = $generateGettersSetters; $this->fluentMutators = $fluentMutators; $this->lexer = new Lexer\Emulative([ 'usedAttributes' => [ @@ -87,7 +87,7 @@ public function addEntityField(string $propertyName, array $columnOptions, array $this->addProperty($propertyName, $comments, $defaultValue); // return early when setters/getters should not be added. - if (true === $this->omitGettersSetters) { + if (false === $this->generateGettersSetters) { return; } @@ -165,7 +165,7 @@ public function addProperty(string $name, array $annotationLines = [], $defaultV $newPropertyBuilder = new Builder\Property($name); // if we do not add getters/setters, the fields must be public - if (true === $this->omitGettersSetters) { + if (false === $this->generateGettersSetters) { $newPropertyBuilder->makePublic(); } else { $newPropertyBuilder->makePrivate(); diff --git a/tests/Maker/MakeDtoTest.php b/tests/Maker/MakeDtoTest.php index ee3f74f13..9550d2747 100644 --- a/tests/Maker/MakeDtoTest.php +++ b/tests/Maker/MakeDtoTest.php @@ -26,8 +26,8 @@ public function getTestDetails() 'Task', // generate helpers 'yes', - // omit getters - 'yes', + // generate getters + 'no', ]) ->addExtraDependencies('orm') ->addExtraDependencies('validator') @@ -47,8 +47,8 @@ public function getTestDetails() 'Task', // generate helpers 'yes', - // omit getters - 'no', + // generate getters + 'yes', ]) ->addExtraDependencies('orm') ->addExtraDependencies('validator') @@ -67,8 +67,8 @@ public function getTestDetails() 'Task', // generate helpers 'yes', - // omit getters - 'yes', + // generate getters + 'no', ]) ->addExtraDependencies('orm') ->addExtraDependencies('validator') @@ -89,8 +89,8 @@ public function getTestDetails() 'Task', // generate helpers 'no', - // omit getters - 'no', + // generate getters + 'yes', ]) ->addExtraDependencies('orm') ->addExtraDependencies('validator') @@ -109,8 +109,8 @@ public function getTestDetails() 'Task', // generate helpers 'yes', - // omit getters - 'yes', + // generate getters + 'no', ]) ->addExtraDependencies('orm') ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeDtoWithoutValidations') @@ -129,8 +129,8 @@ public function getTestDetails() '\\App\\Entity\\Task', // generate helpers 'yes', - // omit getters - 'yes', + // generate getters + 'no', ]) ->addExtraDependencies('orm') ->setCommandAllowedToFail(true) @@ -148,8 +148,8 @@ public function getTestDetails() 'Task', // generate helpers 'yes', - // omit getters - 'yes', + // generate getters + 'no', ]) ->addExtraDependencies('orm') ->addExtraDependencies('validator') @@ -169,8 +169,8 @@ public function getTestDetails() 'Task', // generate helpers 'yes', - // omit getters - 'yes', + // generate getters + 'no', ]) ->addExtraDependencies('orm') ->addExtraDependencies('validator') @@ -190,8 +190,8 @@ public function getTestDetails() 'Task', // generate helpers 'yes', - // omit getters - 'no', + // generate getters + 'yes', ]) ->addExtraDependencies('orm') ->addExtraDependencies('validator') From 868628c104b7ff8302c8f4697f01540036901d30 Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Mon, 3 Dec 2018 11:53:58 +0100 Subject: [PATCH 53/60] Improve comment --- src/Maker/MakeDto.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Maker/MakeDto.php b/src/Maker/MakeDto.php index 71ffc5fa7..a28413152 100644 --- a/src/Maker/MakeDto.php +++ b/src/Maker/MakeDto.php @@ -123,7 +123,7 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen return !$metaData->isIdentifier($field['fieldName']); }); - // See, whether there are missing methods + // Check, whether there are missing methods $missingGettersSetters = false; foreach ($fields as $fieldName => $mapping) { $fields[$fieldName]['hasSetter'] = $this->entityHasSetter($boundClassDetails->getFullName(), $fieldName); From b4792261885d096ec2b2efd6fc2cb5e4913edfa1 Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Sun, 21 Jun 2020 00:32:39 +0200 Subject: [PATCH 54/60] style: rename bound-class to entity --- src/Maker/MakeDto.php | 38 +++++++++++++------------- src/Resources/skeleton/dto/DTO.tpl.php | 34 +++++++++++------------ 2 files changed, 36 insertions(+), 36 deletions(-) diff --git a/src/Maker/MakeDto.php b/src/Maker/MakeDto.php index a28413152..3fabf21ca 100644 --- a/src/Maker/MakeDto.php +++ b/src/Maker/MakeDto.php @@ -61,17 +61,17 @@ public function configureCommand(Command $command, InputConfiguration $inputConf $command ->setDescription('Creates a new "data transfer object" (DTO) class from a Doctrine entity') ->addArgument('name', InputArgument::REQUIRED, sprintf('The name of the DTO class (e.g. %sData)', Str::asClassName(Str::getRandomTerm()))) - ->addArgument('bound-class', InputArgument::REQUIRED, 'The name of Entity that the DTO will be bound to') + ->addArgument('entity', InputArgument::REQUIRED, 'The name of Entity that the DTO will be bound to') ->setHelp(file_get_contents(__DIR__.'/../Resources/help/MakeDto.txt')) ; - $inputConf->setArgumentAsNonInteractive('bound-class'); + $inputConf->setArgumentAsNonInteractive('entity'); } public function interact(InputInterface $input, ConsoleStyle $io, Command $command) { - if (null === $input->getArgument('bound-class')) { - $argument = $command->getDefinition()->getArgument('bound-class'); + if (null === $input->getArgument('entity')) { + $argument = $command->getDefinition()->getArgument('entity'); $entities = $this->doctrineHelper->getEntitiesForAutocomplete(); @@ -80,7 +80,7 @@ public function interact(InputInterface $input, ConsoleStyle $io, Command $comma $question->setAutocompleterValues($entities); $question->setMaxAttempts(3); - $input->setArgument('bound-class', $io->askQuestion($question)); + $input->setArgument('entity', $io->askQuestion($question)); } } @@ -92,15 +92,15 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen 'Data' ); - $boundClass = $input->getArgument('bound-class'); + $entity = $input->getArgument('entity'); - $boundClassDetails = $generator->createClassNameDetails( - $boundClass, + $entityDetails = $generator->createClassNameDetails( + $entity, 'Entity\\' ); // Verify that class is an entity - if (false === $this->doctrineHelper->isClassAMappedEntity($boundClassDetails->getFullName())) { + if (false === $this->doctrineHelper->isClassAMappedEntity($entityDetails->getFullName())) { throw new RuntimeCommandException('The bound class is not a valid doctrine entity.'); } @@ -109,7 +109,7 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen * * @var ClassMetaData */ - $metaData = $this->doctrineHelper->getMetadata($boundClassDetails->getFullName()); + $metaData = $this->doctrineHelper->getMetadata($entityDetails->getFullName()); // Get list of fields $fields = $metaData->fieldMappings; @@ -126,17 +126,17 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen // Check, whether there are missing methods $missingGettersSetters = false; foreach ($fields as $fieldName => $mapping) { - $fields[$fieldName]['hasSetter'] = $this->entityHasSetter($boundClassDetails->getFullName(), $fieldName); - $fields[$fieldName]['hasGetter'] = $this->entityHasGetter($boundClassDetails->getFullName(), $fieldName); + $fields[$fieldName]['hasSetter'] = $this->entityHasSetter($entityDetails->getFullName(), $fieldName); + $fields[$fieldName]['hasGetter'] = $this->entityHasGetter($entityDetails->getFullName(), $fieldName); if (!$fields[$fieldName]['hasGetter'] || !$fields[$fieldName]['hasSetter']) { $missingGettersSetters = true; } } - $boundClassVars = [ - 'bounded_full_class_name' => $boundClassDetails->getFullName(), - 'bounded_class_name' => $boundClassDetails->getShortName(), + $entityVars = [ + 'entity_full_class_name' => $entityDetails->getFullName(), + 'entity_class_name' => $entityDetails->getShortName(), ]; $DTOClassPath = $generator->generateClass( @@ -148,7 +148,7 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen 'addHelpers' => $addHelpers, 'generateGettersSetters' => $generateGettersSetters, ], - $boundClassVars + $entityVars ) ); @@ -169,7 +169,7 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen if (array_key_exists('declared', $mapping)) { $fullClassName = $mapping['declared']; } else { - $fullClassName = $boundClassDetails->getFullName(); + $fullClassName = $entityDetails->getFullName(); } // Property Annotations @@ -193,7 +193,7 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen } // Compare the amount of constraints in annotations with those in the complete validator-metadata for the entity - if (false === $this->hasAsManyValidations($boundClassDetails->getFullName(), $fieldName, $constraintCount)) { + if (false === $this->hasAsManyValidations($entityDetails->getFullName(), $fieldName, $constraintCount)) { $suspectYamlXmlValidations = true; } @@ -237,7 +237,7 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen $io->text([ sprintf('Next: Review the new DTO %s', $DTOClassPath), 'Then: Create a form for this DTO by running:', - sprintf('$ php bin/console make:form %s', $boundClassDetails->getShortName()), + sprintf('$ php bin/console make:form %s', $entityDetails->getShortName()), sprintf('and enter \\%s', $dataClassNameDetails->getFullName()), '', 'Find the documentation at https://symfony.com/doc/current/forms/data_transfer_objects.html', diff --git a/src/Resources/skeleton/dto/DTO.tpl.php b/src/Resources/skeleton/dto/DTO.tpl.php index 3aac34896..3573fc2e8 100644 --- a/src/Resources/skeleton/dto/DTO.tpl.php +++ b/src/Resources/skeleton/dto/DTO.tpl.php @@ -5,12 +5,12 @@ namespace ; - -use ; + +use ; /** - * Data transfer object for . + * Data transfer object for . */ class @@ -19,62 +19,62 @@ class /** * Create DTO, optionally extracting data from a model. */ - public function __construct(? $ = null) + public function __construct(? $ = null) { - if (null !== $) { - $this->extract($); + if (null !== $) { + $this->extract($); } } /** * Fill entity with data from the DTO. */ - public function fill( $): + public function fill( $): { $mapping): ?> // @todo implement setter on the Entity - //$->set($this->get()); + //$->set($this->get()); - $->set($this->get()); + $->set($this->get()); $mapping): ?> // @todo implement setter on the Entity - //$->set($this->); + //$->set($this->); - $->set($this->); + $->set($this->); - return $; + return $; } /** * Extract data from entity into the DTO. */ - public function extract( $): self + public function extract( $): self { $mapping): ?> // @todo implement getter on the Entity - //$this->set($->get()); + //$this->set($->get()); - $this->set($->get()); + $this->set($->get()); $mapping): ?> // @todo implement getter on the Entity - //$this-> = $->get(); + //$this-> = $->get(); - $this-> = $->get(); + $this-> = $->get(); From 258726af6751accb70dd032bd46a928c413a317a Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Sun, 21 Jun 2020 00:37:54 +0200 Subject: [PATCH 55/60] style: early return --- src/Maker/MakeDto.php | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/Maker/MakeDto.php b/src/Maker/MakeDto.php index 3fabf21ca..72b01ebb4 100644 --- a/src/Maker/MakeDto.php +++ b/src/Maker/MakeDto.php @@ -70,18 +70,20 @@ public function configureCommand(Command $command, InputConfiguration $inputConf public function interact(InputInterface $input, ConsoleStyle $io, Command $command) { - if (null === $input->getArgument('entity')) { - $argument = $command->getDefinition()->getArgument('entity'); + if (null !== $input->getArgument('entity')) { + return; + } - $entities = $this->doctrineHelper->getEntitiesForAutocomplete(); + $argument = $command->getDefinition()->getArgument('entity'); - $question = new Question($argument->getDescription()); - $question->setValidator(function ($answer) use ($entities) {return Validator::existsOrNull($answer, $entities); }); - $question->setAutocompleterValues($entities); - $question->setMaxAttempts(3); + $entities = $this->doctrineHelper->getEntitiesForAutocomplete(); - $input->setArgument('entity', $io->askQuestion($question)); - } + $question = new Question($argument->getDescription()); + $question->setValidator(function ($answer) use ($entities) {return Validator::existsOrNull($answer, $entities); }); + $question->setAutocompleterValues($entities); + $question->setMaxAttempts(3); + + $input->setArgument('entity', $io->askQuestion($question)); } public function generate(InputInterface $input, ConsoleStyle $io, Generator $generator) From ef1ed3fff17715a01be355c32b0086620f794061 Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Mon, 22 Jun 2020 12:42:02 +0200 Subject: [PATCH 56/60] Remove PHP 7.1 requirement & run php-cs-fixer The requirement is no longer needed, the package requires 7.1.3 itself. --- src/Maker/MakeDto.php | 2 +- tests/Maker/MakeDtoTest.php | 27 +++++++------------ .../MakeDto/tests/GeneratedDtoTest.php | 7 ++--- .../MakeDtoCompositeId/src/Entity/Task.php | 12 ++++----- .../tests/GeneratedDtoTest.php | 8 +----- .../tests/GeneratedDtoTest.php | 7 ++--- .../src/Entity/Deadline.php | 1 - .../tests/GeneratedDtoTest.php | 7 ++--- .../tests/GeneratedDtoTest.php | 13 +++------ .../tests/GeneratedDtoTest.php | 7 ++--- .../tests/GeneratedDtoTest.php | 5 +--- .../tests/GeneratedDtoTest.php | 5 +--- 12 files changed, 31 insertions(+), 70 deletions(-) diff --git a/src/Maker/MakeDto.php b/src/Maker/MakeDto.php index 72b01ebb4..8dfe3c09c 100644 --- a/src/Maker/MakeDto.php +++ b/src/Maker/MakeDto.php @@ -168,7 +168,7 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen $annotationReader = new AnnotationReader(); // Lookup classname for inherited properties - if (array_key_exists('declared', $mapping)) { + if (\array_key_exists('declared', $mapping)) { $fullClassName = $mapping['declared']; } else { $fullClassName = $entityDetails->getFullName(); diff --git a/tests/Maker/MakeDtoTest.php b/tests/Maker/MakeDtoTest.php index 9550d2747..88ffa2863 100644 --- a/tests/Maker/MakeDtoTest.php +++ b/tests/Maker/MakeDtoTest.php @@ -36,8 +36,7 @@ public function getTestDetails() $this->assertContains('created: src/Dto/TaskData.php', $output); $this->assertContains('updated: src/Dto/TaskData.php', $output); $this->assertContains('\\App\\Dto\\TaskData', $output); - }) - ->setRequiredPhpVersion(70100), + }), ]; yield 'dto_getters_setters' => [MakerTestDetails::createTest( @@ -56,8 +55,7 @@ public function getTestDetails() ->assert(function (string $output, string $directory) { $this->assertContains('created: src/Dto/TaskData.php', $output); $this->assertContains('updated: src/Dto/TaskData.php', $output); - }) - ->setRequiredPhpVersion(70100), + }), ]; yield 'dto_validator_yaml_xml' => [MakerTestDetails::createTest( @@ -78,8 +76,7 @@ public function getTestDetails() $this->assertContains('created: src/Dto/TaskData.php', $output); $this->assertContains('updated: src/Dto/TaskData.php', $output); $this->assertContains('The entity possibly uses Yaml/Xml validators.', $output); - }) - ->setRequiredPhpVersion(70100) + }), ]; yield 'dto_without_helpers' => [MakerTestDetails::createTest( @@ -98,8 +95,7 @@ public function getTestDetails() ->assert(function (string $output, string $directory) { $this->assertContains('created: src/Dto/TaskData.php', $output); $this->assertContains('updated: src/Dto/TaskData.php', $output); - }) - ->setRequiredPhpVersion(70100), + }), ]; yield 'dto_without_validations' => [MakerTestDetails::createTest( @@ -117,8 +113,7 @@ public function getTestDetails() ->assert(function (string $output, string $directory) { $this->assertContains('created: src/Dto/TaskData.php', $output); $this->assertContains('updated: src/Dto/TaskData.php', $output); - }) - ->setRequiredPhpVersion(70100) + }), ]; yield 'dto_invalid_entity' => [MakerTestDetails::createTest( @@ -137,8 +132,7 @@ public function getTestDetails() ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeDtoInvalidEntity') ->assert(function (string $output, string $directory) { $this->assertContains('The bound class is not a valid doctrine entity.', $output); - }) - ->setRequiredPhpVersion(70100), + }), ]; yield 'dto_mapped_super_class' => [MakerTestDetails::createTest( @@ -158,8 +152,7 @@ public function getTestDetails() $this->assertContains('created: src/Dto/TaskData.php', $output); $this->assertContains('updated: src/Dto/TaskData.php', $output); $this->assertContains('\\App\\Dto\\TaskData', $output); - }) - ->setRequiredPhpVersion(70100), + }), ]; yield 'dto_composite_id' => [MakerTestDetails::createTest( @@ -179,8 +172,7 @@ public function getTestDetails() $this->assertContains('created: src/Dto/TaskData.php', $output); $this->assertContains('updated: src/Dto/TaskData.php', $output); $this->assertContains('\\App\\Dto\\TaskData', $output); - }) - ->setRequiredPhpVersion(70100), + }), ]; yield 'dto_missing_getters_setters' => [MakerTestDetails::createTest( @@ -201,8 +193,7 @@ public function getTestDetails() $this->assertContains('updated: src/Dto/TaskData.php', $output); $this->assertContains('\\App\\Dto\\TaskData', $output); $this->assertContains('The maker found missing getters/setters for properties in the entity.', $output); - }) - ->setRequiredPhpVersion(70100), + }), ]; } } diff --git a/tests/fixtures/MakeDto/tests/GeneratedDtoTest.php b/tests/fixtures/MakeDto/tests/GeneratedDtoTest.php index a469021b9..58b8a1657 100644 --- a/tests/fixtures/MakeDto/tests/GeneratedDtoTest.php +++ b/tests/fixtures/MakeDto/tests/GeneratedDtoTest.php @@ -11,15 +11,12 @@ namespace App\Tests; -use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; -use App\Entity\Task; use App\Dto\TaskData; +use App\Entity\Task; use Doctrine\Common\Annotations\AnnotationReader; +use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; use Symfony\Component\Validator\Constraints\NotBlank; -/** - * @requires PHP 7.1 - */ class GeneratedDtoTest extends KernelTestCase { public function testGeneratedDto() diff --git a/tests/fixtures/MakeDtoCompositeId/src/Entity/Task.php b/tests/fixtures/MakeDtoCompositeId/src/Entity/Task.php index 73c405604..50cb74c47 100644 --- a/tests/fixtures/MakeDtoCompositeId/src/Entity/Task.php +++ b/tests/fixtures/MakeDtoCompositeId/src/Entity/Task.php @@ -89,7 +89,7 @@ public function setDueDate($dueDate) } /** - * Get the value of group + * Get the value of group. */ public function getGroup() { @@ -97,9 +97,9 @@ public function getGroup() } /** - * Set the value of group + * Set the value of group. * - * @return self + * @return self */ public function setGroup($group) { @@ -109,7 +109,7 @@ public function setGroup($group) } /** - * Get the value of id + * Get the value of id. */ public function getId() { @@ -117,9 +117,9 @@ public function getId() } /** - * Set the value of id + * Set the value of id. * - * @return self + * @return self */ public function setId($id) { diff --git a/tests/fixtures/MakeDtoCompositeId/tests/GeneratedDtoTest.php b/tests/fixtures/MakeDtoCompositeId/tests/GeneratedDtoTest.php index 967cb2e57..9c2d3c3ac 100644 --- a/tests/fixtures/MakeDtoCompositeId/tests/GeneratedDtoTest.php +++ b/tests/fixtures/MakeDtoCompositeId/tests/GeneratedDtoTest.php @@ -11,15 +11,9 @@ namespace App\Tests; -use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; -use App\Entity\Task; use App\Dto\TaskData; -use Doctrine\Common\Annotations\AnnotationReader; -use Symfony\Component\Validator\Constraints\NotBlank; +use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; -/** - * @requires PHP 7.1 - */ class GeneratedDtoTest extends KernelTestCase { public function testGeneratedDto() diff --git a/tests/fixtures/MakeDtoGettersSetters/tests/GeneratedDtoTest.php b/tests/fixtures/MakeDtoGettersSetters/tests/GeneratedDtoTest.php index ed304dc29..82589fa44 100644 --- a/tests/fixtures/MakeDtoGettersSetters/tests/GeneratedDtoTest.php +++ b/tests/fixtures/MakeDtoGettersSetters/tests/GeneratedDtoTest.php @@ -11,13 +11,10 @@ namespace App\Tests; -use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; -use App\Entity\Task; use App\Dto\TaskData; +use App\Entity\Task; +use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; -/** - * @requires PHP 7.1 - */ class GeneratedDtoTest extends KernelTestCase { public function testGeneratedDto() diff --git a/tests/fixtures/MakeDtoMappedSuperClass/src/Entity/Deadline.php b/tests/fixtures/MakeDtoMappedSuperClass/src/Entity/Deadline.php index 258206a06..e3f30123e 100644 --- a/tests/fixtures/MakeDtoMappedSuperClass/src/Entity/Deadline.php +++ b/tests/fixtures/MakeDtoMappedSuperClass/src/Entity/Deadline.php @@ -12,7 +12,6 @@ namespace App\Entity; use Doctrine\ORM\Mapping as ORM; -use Symfony\Component\Validator\Constraints as Assert; /** * @ORM\MappedSuperclass diff --git a/tests/fixtures/MakeDtoMappedSuperClass/tests/GeneratedDtoTest.php b/tests/fixtures/MakeDtoMappedSuperClass/tests/GeneratedDtoTest.php index a469021b9..58b8a1657 100644 --- a/tests/fixtures/MakeDtoMappedSuperClass/tests/GeneratedDtoTest.php +++ b/tests/fixtures/MakeDtoMappedSuperClass/tests/GeneratedDtoTest.php @@ -11,15 +11,12 @@ namespace App\Tests; -use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; -use App\Entity\Task; use App\Dto\TaskData; +use App\Entity\Task; use Doctrine\Common\Annotations\AnnotationReader; +use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; use Symfony\Component\Validator\Constraints\NotBlank; -/** - * @requires PHP 7.1 - */ class GeneratedDtoTest extends KernelTestCase { public function testGeneratedDto() diff --git a/tests/fixtures/MakeDtoMissingGettersSetters/tests/GeneratedDtoTest.php b/tests/fixtures/MakeDtoMissingGettersSetters/tests/GeneratedDtoTest.php index 60e0fff7a..69f18461c 100644 --- a/tests/fixtures/MakeDtoMissingGettersSetters/tests/GeneratedDtoTest.php +++ b/tests/fixtures/MakeDtoMissingGettersSetters/tests/GeneratedDtoTest.php @@ -11,17 +11,14 @@ namespace App\Tests; -use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; use App\Dto\TaskData; use App\Entity\Task; +use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; -/** - * @requires PHP 7.1 - */ class GeneratedDtoTest extends KernelTestCase { /** - * Use the helpers to make sure that they work, even though there are missing getters/setters + * Use the helpers to make sure that they work, even though there are missing getters/setters. */ public function testHelpers() { @@ -30,13 +27,11 @@ public function testHelpers() $taskData = new TaskData($taskEntity); - $this->assertEquals($taskEntity->getDueDate(), $taskData->dueDate); - - $taskData->task = 'Foo'; + $this->assertEquals($taskEntity->getDueDate(), $taskData->getDueDate()); $taskEntity = new Task(); $taskData->fill($taskEntity); - $this->assertEquals($taskEntity->getDueDate(), $taskData->dueDate); + $this->assertEquals($taskEntity->getDueDate(), $taskData->getDueDate()); } } diff --git a/tests/fixtures/MakeDtoValidatorYamlXml/tests/GeneratedDtoTest.php b/tests/fixtures/MakeDtoValidatorYamlXml/tests/GeneratedDtoTest.php index 31f127b5a..d2bd4c084 100644 --- a/tests/fixtures/MakeDtoValidatorYamlXml/tests/GeneratedDtoTest.php +++ b/tests/fixtures/MakeDtoValidatorYamlXml/tests/GeneratedDtoTest.php @@ -11,16 +11,13 @@ namespace App\Tests; -use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; -use App\Entity\Task; use App\Dto\TaskData; +use App\Entity\Task; use Doctrine\Common\Annotations\AnnotationReader; +use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; use Symfony\Component\Validator\Constraints\Type; use Symfony\Component\Validator\Validation; -/** - * @requires PHP 7.1 - */ class GeneratedDtoTest extends KernelTestCase { public function testGeneratedDto() diff --git a/tests/fixtures/MakeDtoWithoutHelpers/tests/GeneratedDtoTest.php b/tests/fixtures/MakeDtoWithoutHelpers/tests/GeneratedDtoTest.php index 7387fca51..2c3f108e0 100644 --- a/tests/fixtures/MakeDtoWithoutHelpers/tests/GeneratedDtoTest.php +++ b/tests/fixtures/MakeDtoWithoutHelpers/tests/GeneratedDtoTest.php @@ -11,12 +11,9 @@ namespace App\Tests; -use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; use App\Dto\TaskData; +use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; -/** - * @requires PHP 7.1 - */ class GeneratedDtoTest extends KernelTestCase { public function testGeneratedDto() diff --git a/tests/fixtures/MakeDtoWithoutValidations/tests/GeneratedDtoTest.php b/tests/fixtures/MakeDtoWithoutValidations/tests/GeneratedDtoTest.php index 7b5f08bd7..27b1a2e5e 100644 --- a/tests/fixtures/MakeDtoWithoutValidations/tests/GeneratedDtoTest.php +++ b/tests/fixtures/MakeDtoWithoutValidations/tests/GeneratedDtoTest.php @@ -11,12 +11,9 @@ namespace App\Tests; -use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; use App\Dto\TaskData; +use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; -/** - * @requires PHP 7.1 - */ class GeneratedDtoTest extends KernelTestCase { public function testGeneratedDto() From 536951253837d54808f003047df99f5976528b71 Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Mon, 22 Jun 2020 13:04:58 +0200 Subject: [PATCH 57/60] style: fix codestyle --- src/Util/DTOClassSourceManipulator.php | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/Util/DTOClassSourceManipulator.php b/src/Util/DTOClassSourceManipulator.php index a889e7957..806e0e474 100644 --- a/src/Util/DTOClassSourceManipulator.php +++ b/src/Util/DTOClassSourceManipulator.php @@ -335,8 +335,6 @@ private function getConstructorNode() /** * Modified to public from ClassSourceManipulator. * - * @param string $class - * * @return string The alias to use when referencing this class */ public function addUseStatementIfNecessary(string $class): string @@ -485,8 +483,6 @@ private function getNamespaceNode(): Node\Stmt\Namespace_ } /** - * @param callable $filterCallback - * * @return Node|null */ private function findFirstNode(callable $filterCallback) @@ -500,9 +496,6 @@ private function findFirstNode(callable $filterCallback) } /** - * @param callable $filterCallback - * @param array $ast - * * @return Node|null */ private function findLastNode(callable $filterCallback, array $ast) @@ -692,8 +685,6 @@ private function getThisFullClassName(): string * Adds this new node where a new property should go. * * Useful for adding properties, or adding a constructor. - * - * @param Node $newNode */ private function addNodeAfterProperties(Node $newNode) { From 3bb3f23e451e83bc043acae654330109dee2c595 Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Thu, 2 Jul 2020 23:20:19 +0200 Subject: [PATCH 58/60] feat: allow choosing of three dto styles Change MakeDto to allow a choice between three styles of a DTO: - Mutable with getters & setters - Mutable with public properties - Immutable with getters Use individual templates for the styles. Add mutator method to Entity when confirmed by the user. Add constructor creation to DTOClassSourceManipulator. Refactor tests to adhere to the changes. Rename tests in fixtures. --- src/Maker/MakeDto.php | 376 ++++++++++++------ src/Resources/skeleton/dto/DTO.tpl.php | 85 ---- .../skeleton/dto/ImmutableGettersDto.tpl.php | 11 + .../dto/MutableGettersSettersDto.tpl.php | 34 ++ .../skeleton/dto/MutablePublicDto.tpl.php | 34 ++ src/Util/DTOClassSourceManipulator.php | 130 +++++- tests/Maker/MakeDtoTest.php | 217 +++++----- tests/Util/DTOClassSourceManipulatorTest.php | 158 ++++++++ .../src/Entity/Task.php | 0 .../MakeDtoAnnotations/tests/TaskDataTest.php | 45 +++ .../MakeDtoCompositeId/src/Entity/Task.php | 53 --- ...{GeneratedDtoTest.php => TaskDataTest.php} | 4 +- .../MakeDtoGettersSetters/src/Entity/Task.php | 2 - ...{GeneratedDtoTest.php => TaskDataTest.php} | 19 +- .../src/Entity/Task.php | 0 .../MakeDtoImmutable/tests/TaskDataTest.php | 41 ++ .../src/Entity/Task.php | 2 - .../tests/TaskDataTest.php} | 24 +- .../src/Entity/Task.php | 60 ++- .../tests/TaskDataTest.php | 90 +++++ .../src/Entity/Task.php | 2 - .../tests/TaskDataTest.php} | 24 +- .../MakeDtoNoMutator/src/Entity/Task.php | 82 ++++ .../tests/TaskDataTest.php} | 9 +- .../MakeDtoPublic/src/Entity/Task.php | 82 ++++ .../tests/TaskDataTest.php} | 29 +- ...{GeneratedDtoTest.php => TaskDataTest.php} | 2 +- .../tests/GeneratedDtoTest.php | 25 -- 28 files changed, 1156 insertions(+), 484 deletions(-) delete mode 100644 src/Resources/skeleton/dto/DTO.tpl.php create mode 100644 src/Resources/skeleton/dto/ImmutableGettersDto.tpl.php create mode 100644 src/Resources/skeleton/dto/MutableGettersSettersDto.tpl.php create mode 100644 src/Resources/skeleton/dto/MutablePublicDto.tpl.php create mode 100644 tests/Util/DTOClassSourceManipulatorTest.php rename tests/fixtures/{MakeDto => MakeDtoAnnotations}/src/Entity/Task.php (100%) create mode 100644 tests/fixtures/MakeDtoAnnotations/tests/TaskDataTest.php rename tests/fixtures/MakeDtoCompositeId/tests/{GeneratedDtoTest.php => TaskDataTest.php} (87%) rename tests/fixtures/MakeDtoGettersSetters/tests/{GeneratedDtoTest.php => TaskDataTest.php} (61%) rename tests/fixtures/{MakeDtoWithoutValidations => MakeDtoImmutable}/src/Entity/Task.php (100%) create mode 100644 tests/fixtures/MakeDtoImmutable/tests/TaskDataTest.php rename tests/fixtures/{MakeDto/tests/GeneratedDtoTest.php => MakeDtoMappedSuperClass/tests/TaskDataTest.php} (62%) create mode 100644 tests/fixtures/MakeDtoMissingGettersSetters/tests/TaskDataTest.php rename tests/fixtures/{MakeDtoWithoutHelpers => MakeDtoMutator}/src/Entity/Task.php (94%) rename tests/fixtures/{MakeDtoMissingGettersSetters/tests/GeneratedDtoTest.php => MakeDtoMutator/tests/TaskDataTest.php} (50%) create mode 100644 tests/fixtures/MakeDtoNoMutator/src/Entity/Task.php rename tests/fixtures/{MakeDtoWithoutHelpers/tests/GeneratedDtoTest.php => MakeDtoNoMutator/tests/TaskDataTest.php} (58%) create mode 100644 tests/fixtures/MakeDtoPublic/src/Entity/Task.php rename tests/fixtures/{MakeDtoMappedSuperClass/tests/GeneratedDtoTest.php => MakeDtoPublic/tests/TaskDataTest.php} (56%) rename tests/fixtures/MakeDtoValidatorYamlXml/tests/{GeneratedDtoTest.php => TaskDataTest.php} (97%) delete mode 100644 tests/fixtures/MakeDtoWithoutValidations/tests/GeneratedDtoTest.php diff --git a/src/Maker/MakeDto.php b/src/Maker/MakeDto.php index 8dfe3c09c..014905665 100644 --- a/src/Maker/MakeDto.php +++ b/src/Maker/MakeDto.php @@ -21,11 +21,15 @@ use Symfony\Bundle\MakerBundle\Generator; use Symfony\Bundle\MakerBundle\InputConfiguration; use Symfony\Bundle\MakerBundle\Str; +use Symfony\Bundle\MakerBundle\Util\ClassDetails; +use Symfony\Bundle\MakerBundle\Util\ClassNameDetails; +use Symfony\Bundle\MakerBundle\Util\ClassSourceManipulator; use Symfony\Bundle\MakerBundle\Util\DTOClassSourceManipulator; use Symfony\Bundle\MakerBundle\Validator; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; +use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Question\Question; use Symfony\Component\Validator\Constraint; use Symfony\Component\Validator\Validation; @@ -41,6 +45,30 @@ final class MakeDto extends AbstractMaker private $validator; private $validatorClassMetadata; + private const DTO_STYLES = [ + 1 => 'Mutable, with getters & setters (default)', + 2 => 'Mutable, with public properties', + 3 => 'Immutable, with getters only', + ]; + + private const TEMPLATE_NAMES = [ + 1 => 'MutableGettersSetters', + 2 => 'MutablePublic', + 3 => 'ImmutableGetters', + ]; + + private const MUTATOR_NAME_PREFIX = 'updateFrom'; + private const ARGUMENT_NAME = 'name'; + private const ARGUMENT_ENTITY = 'entity'; + private const OPTION_STYLE = 'style'; + private const OPTION_MUTATOR = 'mutator'; + + // Did we import assert annotations? + private $assertionsImported = false; + + // Are there differences in the validation constraints between metadata (includes annotations, xml, yaml) and annotations? + private $suspectYamlXmlValidations = false; + public function __construct( DoctrineHelper $doctrineHelper, FileManager $fileManager, @@ -60,41 +88,52 @@ public function configureCommand(Command $command, InputConfiguration $inputConf { $command ->setDescription('Creates a new "data transfer object" (DTO) class from a Doctrine entity') - ->addArgument('name', InputArgument::REQUIRED, sprintf('The name of the DTO class (e.g. %sData)', Str::asClassName(Str::getRandomTerm()))) - ->addArgument('entity', InputArgument::REQUIRED, 'The name of Entity that the DTO will be bound to') + ->addArgument(self::ARGUMENT_NAME, InputArgument::OPTIONAL, sprintf('The name of the DTO class (e.g. %sData)', Str::asClassName(Str::getRandomTerm()))) + ->addArgument(self::ARGUMENT_ENTITY, InputArgument::OPTIONAL, 'The name of Entity that the DTO will be bound to') + ->addOption(self::OPTION_STYLE, null, InputOption::VALUE_REQUIRED, 'The style of the DTO') + ->addOption(self::OPTION_MUTATOR, null, InputOption::VALUE_REQUIRED, 'Whether a mutator should be added to the entity', true) ->setHelp(file_get_contents(__DIR__.'/../Resources/help/MakeDto.txt')) ; - $inputConf->setArgumentAsNonInteractive('entity'); + $inputConf->setArgumentAsNonInteractive(self::ARGUMENT_NAME); + $inputConf->setArgumentAsNonInteractive(self::ARGUMENT_ENTITY); } public function interact(InputInterface $input, ConsoleStyle $io, Command $command) { - if (null !== $input->getArgument('entity')) { - return; + if (null === $input->getArgument(self::ARGUMENT_NAME)) { + $argument = $command->getDefinition()->getArgument(self::ARGUMENT_NAME); + $question = $this->createDataClassQuestion($argument->getDescription()); + $value = $io->askQuestion($question); + $input->setArgument(self::ARGUMENT_NAME, $value); } - $argument = $command->getDefinition()->getArgument('entity'); - - $entities = $this->doctrineHelper->getEntitiesForAutocomplete(); + if (null === $input->getArgument(self::ARGUMENT_ENTITY)) { + $argument = $command->getDefinition()->getArgument(self::ARGUMENT_ENTITY); + $question = $this->createEntityClassQuestion($argument->getDescription()); + $value = $io->askQuestion($question); + $input->setArgument(self::ARGUMENT_ENTITY, $value); + } - $question = new Question($argument->getDescription()); - $question->setValidator(function ($answer) use ($entities) {return Validator::existsOrNull($answer, $entities); }); - $question->setAutocompleterValues($entities); - $question->setMaxAttempts(3); + $input->setOption(self::OPTION_STYLE, $io->choice( + 'Specify the type of DTO you want:', + self::DTO_STYLES + )); - $input->setArgument('entity', $io->askQuestion($question)); + $input->setOption(self::OPTION_MUTATOR, $io->confirm( + 'Add mutator method to Entity (to set data from the DTO)?' + )); } public function generate(InputInterface $input, ConsoleStyle $io, Generator $generator) { $dataClassNameDetails = $generator->createClassNameDetails( - $input->getArgument('name'), + $input->getArgument(self::ARGUMENT_NAME), 'Dto\\', 'Data' ); - $entity = $input->getArgument('entity'); + $entity = $input->getArgument(self::ARGUMENT_ENTITY); $entityDetails = $generator->createClassNameDetails( $entity, @@ -102,39 +141,13 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen ); // Verify that class is an entity - if (false === $this->doctrineHelper->isClassAMappedEntity($entityDetails->getFullName())) { + if (!$this->doctrineHelper->isClassAMappedEntity($entityDetails->getFullName())) { throw new RuntimeCommandException('The bound class is not a valid doctrine entity.'); } - /** - * Get class metadata (used to copy annotations and generate properties). - * - * @var ClassMetaData - */ - $metaData = $this->doctrineHelper->getMetadata($entityDetails->getFullName()); - - // Get list of fields - $fields = $metaData->fieldMappings; - - // The result is passed to the template - $addHelpers = $io->confirm('Add helper extract/fill methods?'); - $generateGettersSetters = $io->confirm('Generate getters/setters?', false); + $fields = $this->getFilteredFieldMappings($entityDetails->getFullName()); - // Filter identifiers from generated fields - $fields = array_filter($fields, function ($field) use ($metaData) { - return !$metaData->isIdentifier($field['fieldName']); - }); - - // Check, whether there are missing methods - $missingGettersSetters = false; - foreach ($fields as $fieldName => $mapping) { - $fields[$fieldName]['hasSetter'] = $this->entityHasSetter($entityDetails->getFullName(), $fieldName); - $fields[$fieldName]['hasGetter'] = $this->entityHasGetter($entityDetails->getFullName(), $fieldName); - - if (!$fields[$fieldName]['hasGetter'] || !$fields[$fieldName]['hasSetter']) { - $missingGettersSetters = true; - } - } + $missingGettersSetters = $this->checkMissingGettersSetters($fields, $entityDetails->getFullName()); $entityVars = [ 'entity_full_class_name' => $entityDetails->getFullName(), @@ -143,113 +156,88 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen $DTOClassPath = $generator->generateClass( $dataClassNameDetails->getFullName(), - __DIR__.'/../Resources/skeleton/dto/DTO.tpl.php', + $this->getDtoTemplateName($input), array_merge( [ 'fields' => $fields, - 'addHelpers' => $addHelpers, - 'generateGettersSetters' => $generateGettersSetters, ], $entityVars ) ); $generator->writeChanges(); - $manipulator = $this->createClassManipulator($DTOClassPath, $generateGettersSetters); - $mappedFields = $this->getMappedFieldsInEntity($metaData); - - // Did we import assert annotations? - $assertionsImported = false; - - // Are there differences in the validation constraints between metadata (includes annotations, xml, yaml) and annotations? - $suspectYamlXmlValidations = false; - - foreach ($fields as $fieldName => $mapping) { - $annotationReader = new AnnotationReader(); - - // Lookup classname for inherited properties - if (\array_key_exists('declared', $mapping)) { - $fullClassName = $mapping['declared']; - } else { - $fullClassName = $entityDetails->getFullName(); - } - - // Property Annotations - $reflectionProperty = new \ReflectionProperty($fullClassName, $fieldName); - $propertyAnnotations = $annotationReader->getPropertyAnnotations($reflectionProperty); - - // Passed to the ClassManipulator - $comments = []; - // Count the Constraints for comparison with the Validator - $constraintCount = 0; - - foreach ($propertyAnnotations as $annotation) { - // We want to copy the asserts, so look for their interface - if ($annotation instanceof Constraint) { - // Set flag for use in result message - $assertionsImported = true; - ++$constraintCount; - $comments[] = $manipulator->buildAnnotationLine('@Assert\\'.(new \ReflectionClass($annotation))->getShortName(), $this->getAnnotationAsString($annotation)); - } - } + $dtoManipulator = $this->createDTOClassManipulator($DTOClassPath, $input); + $this->createProperties($dtoManipulator, $entityDetails, $fields); - // Compare the amount of constraints in annotations with those in the complete validator-metadata for the entity - if (false === $this->hasAsManyValidations($entityDetails->getFullName(), $fieldName, $constraintCount)) { - $suspectYamlXmlValidations = true; - } - - $manipulator->addEntityField($fieldName, $mapping, $comments); - } - - // Add use statement for validation annotations if necessary - if (true == $assertionsImported) { - // The use of an alias is not supposed, but it works fine and we don't use the returned value. - $manipulator->addUseStatementIfNecessary('Symfony\Component\Validator\Constraints as Assert'); + if ($this->shouldCreateConstructor($input)) { + $dtoManipulator->createConstructor(); } $this->fileManager->dumpFile( $DTOClassPath, - $manipulator->getSourceCode() + $dtoManipulator->getSourceCode() ); + if ($input->getOption(self::OPTION_MUTATOR)) { + $entityClassDetails = new ClassDetails($entityDetails->getFullName()); + $entityPath = $entityClassDetails->getPath(); + $entityManipulator = $this->createEntityClassManipulator($entityPath, $io, false); + + $this->createEntityMutator( + $entityManipulator, + $entityDetails, + $dataClassNameDetails, + $fields, + $this->shouldGenerateGetters($input) + ); + + $this->fileManager->dumpFile($entityPath, $entityManipulator->getSourceCode()); + } + $this->writeSuccessMessage($io); - if (true === $assertionsImported) { + if (true === $this->assertionsImported) { $io->note([ 'The maker imported assertion annotations.', 'Consider removing them from the entity or make sure to keep them updated in both places.', ]); } - if (true === $suspectYamlXmlValidations) { + if (true === $this->suspectYamlXmlValidations) { $io->note([ 'The entity possibly uses Yaml/Xml validators.', 'Make sure to update the validations to include the new DTO class.', ]); } - if (true === $missingGettersSetters) { + if ($missingGettersSetters) { $io->note([ 'The maker found missing getters/setters for properties in the entity.', 'Please review the generated DTO for @todo comments.', ]); } - $io->text([ - sprintf('Next: Review the new DTO %s', $DTOClassPath), + $nextSteps = [ + 'Next:', + sprintf('- Review the new DTO %s', $DTOClassPath), + $input->getOption(self::OPTION_MUTATOR) ? sprintf( + '- Review the generated mutator method %s() in %s', + $entityDetails->getShortName().'::'.self::MUTATOR_NAME_PREFIX.$dataClassNameDetails->getShortName(), + $this->fileManager->relativizePath($entityClassDetails->getPath()) + ) : null, 'Then: Create a form for this DTO by running:', - sprintf('$ php bin/console make:form %s', $entityDetails->getShortName()), + sprintf('$ php bin/console make:form %s', $entityDetails->getShortName()), sprintf('and enter \\%s', $dataClassNameDetails->getFullName()), '', 'Find the documentation at https://symfony.com/doc/current/forms/data_transfer_objects.html', - ]); + ]; + + $io->text($nextSteps); } public function configureDependencies(DependencyBuilder $dependencies) { - $dependencies->requirePHP71(); - $dependencies->addClassDependency( Validation::class, 'validator', @@ -258,7 +246,127 @@ public function configureDependencies(DependencyBuilder $dependencies) ); } - private function createClassManipulator(string $classPath, bool $generateGettersSetters = false): DTOClassSourceManipulator + /** + * Get field mappings from class metadata (used to copy annotations and generate properties). + */ + private function getFilteredFieldMappings(string $entityClassName): array + { + /** + * @var ClassMetaData + */ + $metaData = $this->doctrineHelper->getMetadata($entityClassName); + + return $this->filterIdentifiersFromFields($metaData->fieldMappings, $metaData); + } + + private function filterIdentifiersFromFields($fields, $metaData): array + { + return array_filter($fields, function ($field) use ($metaData) { + return !$metaData->isIdentifier($field['fieldName']); + }); + } + + private function checkMissingGettersSetters(array &$fields, string $entityClassName): bool + { + $missingGettersSetters = false; + foreach (array_keys($fields) as $fieldName) { + $fields[$fieldName]['hasSetter'] = $this->entityHasSetter($entityClassName, $fieldName); + $fields[$fieldName]['hasGetter'] = $this->entityHasGetter($entityClassName, $fieldName); + + if (!$fields[$fieldName]['hasGetter'] || !$fields[$fieldName]['hasSetter']) { + $missingGettersSetters = true; + } + } + + return $missingGettersSetters; + } + + private function createEntityClassQuestion(string $questionText): Question + { + $question = new Question($questionText); + $entities = $this->doctrineHelper->getEntitiesForAutocomplete(); + $question->setAutocompleterValues($entities); + $question->setMaxAttempts(10); + $question->setValidator(function ($answer) use ($entities) {return Validator::existsOrNull($answer, $entities); }); + + return $question; + } + + private function createDataClassQuestion(string $questionText): Question + { + $question = new Question($questionText); + $question->setValidator([Validator::class, 'notBlank']); + $question->setMaxAttempts(10); + + return $question; + } + + private function createProperties($dtoManipulator, $entityDetails, $fields) + { + foreach ($fields as $fieldName => $mapping) { + $annotationReader = new AnnotationReader(); + + $fullClassName = $mapping['declared'] ?? $entityDetails->getFullName(); + + // Property Annotations + $reflectionProperty = new \ReflectionProperty($fullClassName, $fieldName); + $propertyAnnotations = $annotationReader->getPropertyAnnotations($reflectionProperty); + + // Passed to the ClassManipulator + $annotationComments = []; + + // Count the Constraints for comparison with the Validator + $constraintCount = 0; + + foreach ($propertyAnnotations as $annotation) { + // We want to copy the asserts, so look for their interface + if ($annotation instanceof Constraint) { + // Set flag for use in result message + $this->assertionsImported = true; + ++$constraintCount; + $annotationComments[] = $dtoManipulator->buildAnnotationLine('@Assert\\'.(new \ReflectionClass($annotation))->getShortName(), $this->getAnnotationAsString($annotation)); + } + } + + // Compare the amount of constraints in annotations with those in the complete validator-metadata for the entity + if (false === $this->hasAsManyValidations($entityDetails->getFullName(), $fieldName, $constraintCount)) { + $this->suspectYamlXmlValidations = true; + } + + $dtoManipulator->addEntityField($fieldName, $mapping, $annotationComments); + } + + // Add use statement for validation annotations if necessary + if (true == $this->assertionsImported) { + // The use of an alias is not supposed, but it works fine and we don't use the returned value. + $dtoManipulator->addUseStatementIfNecessary('Symfony\Component\Validator\Constraints as Assert'); + } + } + + private function createEntityMutator(ClassSourceManipulator $entityManipulator, ClassNameDetails $entityDetails, ClassNameDetails $dataClassNameDetails, array $fields, bool $dtoHasGetters) + { + $dataClassUseName = $entityManipulator->addUseStatementIfNecessary($dataClassNameDetails->getFullName()); + + $updateFromMethodBuilder = $entityManipulator->createMethodBuilder(self::MUTATOR_NAME_PREFIX.$dataClassNameDetails->getShortName(), null, true); + $updateFromMethodBuilder->addParam( + (new \PhpParser\Builder\Param(lcfirst($dataClassNameDetails->getShortName())))->setTypeHint($dataClassUseName) + ); + + $methodBody = ' $mapping) { + $assignedValue = ($dtoHasGetters) ? '$'.lcfirst($dataClassNameDetails->getShortName()).'->get'.Str::asCamelCase($propertyName).'()' : '$'.lcfirst($dataClassNameDetails->getShortName()).'->'.$propertyName; + if (false === $mapping['hasSetter']) { + $methodBody .= '$this->'.$propertyName.' = '.$assignedValue.';'.PHP_EOL; + } else { + $methodBody .= '$this->set'.Str::asCamelCase($propertyName).'('.$assignedValue.');'.PHP_EOL; + } + } + + $entityManipulator->addMethodBody($updateFromMethodBuilder, $methodBody); + $entityManipulator->addMethodBuilder($updateFromMethodBuilder); + } + + private function createDTOClassManipulator(string $classPath, InputInterface $input): DTOClassSourceManipulator { return new DTOClassSourceManipulator( $this->fileManager->getFileContents($classPath), @@ -268,19 +376,23 @@ private function createClassManipulator(string $classPath, bool $generateGetters true, // use fluent mutators true, - // generate getters setters? - $generateGettersSetters + // generate getters? + $this->shouldGenerateGetters($input), + // generate setters? + $this->shouldGenerateSetters($input), + // Public properties? + $this->shouldGeneratePublicProperties($input), + // Constructor? + $this->shouldCreateConstructor($input) ); } - private function getMappedFieldsInEntity(ClassMetadata $classMetadata) + private function createEntityClassManipulator(string $path, ConsoleStyle $io, bool $overwrite): ClassSourceManipulator { - $targetFields = array_merge( - array_keys($classMetadata->fieldMappings), - array_keys($classMetadata->associationMappings) - ); + $manipulator = new ClassSourceManipulator($this->fileManager->getFileContents($path), $overwrite); + $manipulator->setIo($io); - return $targetFields; + return $manipulator; } private function getAnnotationAsString(Constraint $annotation) @@ -305,20 +417,48 @@ private function hasAsManyValidations($entityClassname, $fieldName, $constraintC $metadataConstraintCount = 0; foreach ($propertyMetadata as $metadata) { if (isset($metadata->constraints)) { - $metadataConstraintCount = $metadataConstraintCount + \count($metadata->constraints); + $metadataConstraintCount += is_countable($metadata->constraints) ? \count($metadata->constraints) : 0; } } return $metadataConstraintCount == $constraintCount; } - private function entityHasGetter($entityClassName, $propertyName) + private function entityHasGetter($entityClassName, $propertyName): bool { return method_exists($entityClassName, sprintf('get%s', Str::asCamelCase($propertyName))); } - private function entityHasSetter($entityClassName, $propertyName) + private function entityHasSetter($entityClassName, $propertyName): bool { return method_exists($entityClassName, sprintf('set%s', Str::asCamelCase($propertyName))); } + + private function getDtoTemplateName(InputInterface $input): string + { + return __DIR__ + .'/../Resources/skeleton/dto/' + .self::TEMPLATE_NAMES[array_search($input->getOption(self::OPTION_STYLE), self::DTO_STYLES)] + .'Dto.tpl.php'; + } + + private function shouldGeneratePublicProperties(InputInterface $input): bool + { + return 2 === array_search($input->getOption(self::OPTION_STYLE), self::DTO_STYLES); + } + + private function shouldGenerateSetters(InputInterface $input): bool + { + return 1 === array_search($input->getOption(self::OPTION_STYLE), self::DTO_STYLES); + } + + private function shouldGenerateGetters(InputInterface $input): bool + { + return \in_array(array_search($input->getOption(self::OPTION_STYLE), self::DTO_STYLES), [1, 3]); + } + + private function shouldCreateConstructor(InputInterface $input): bool + { + return 3 === array_search($input->getOption(self::OPTION_STYLE), self::DTO_STYLES); + } } diff --git a/src/Resources/skeleton/dto/DTO.tpl.php b/src/Resources/skeleton/dto/DTO.tpl.php deleted file mode 100644 index 3573fc2e8..000000000 --- a/src/Resources/skeleton/dto/DTO.tpl.php +++ /dev/null @@ -1,85 +0,0 @@ - - - -namespace ; - - -use ; - - -/** - * Data transfer object for . - */ -class - -{ - - /** - * Create DTO, optionally extracting data from a model. - */ - public function __construct(? $ = null) - { - if (null !== $) { - $this->extract($); - } - } - - /** - * Fill entity with data from the DTO. - */ - public function fill( $): - { - - $mapping): ?> - - // @todo implement setter on the Entity - //$->set($this->get()); - - $->set($this->get()); - - - - $mapping): ?> - - // @todo implement setter on the Entity - //$->set($this->); - - $->set($this->); - - - - - return $; - } - - /** - * Extract data from entity into the DTO. - */ - public function extract( $): self - { - - $mapping): ?> - - // @todo implement getter on the Entity - //$this->set($->get()); - - $this->set($->get()); - - - - $mapping): ?> - - // @todo implement getter on the Entity - //$this-> = $->get(); - - $this-> = $->get(); - - - - - return $this; - } - -} diff --git a/src/Resources/skeleton/dto/ImmutableGettersDto.tpl.php b/src/Resources/skeleton/dto/ImmutableGettersDto.tpl.php new file mode 100644 index 000000000..765bef422 --- /dev/null +++ b/src/Resources/skeleton/dto/ImmutableGettersDto.tpl.php @@ -0,0 +1,11 @@ + + +namespace ; + +/** + * Data transfer object for . + */ +class + +{ +} diff --git a/src/Resources/skeleton/dto/MutableGettersSettersDto.tpl.php b/src/Resources/skeleton/dto/MutableGettersSettersDto.tpl.php new file mode 100644 index 000000000..e03020b0f --- /dev/null +++ b/src/Resources/skeleton/dto/MutableGettersSettersDto.tpl.php @@ -0,0 +1,34 @@ + + + +namespace ; + + +use ; + + +/** + * Data transfer object for . + */ +class + +{ + /** + * Create DTO, optionally extracting data from a model. + */ + public function __construct( $ = null) + { + if ($) { + $mapping): ?> + + // @todo implement getter on the Entity + //$this->set($->get()); + + $this->set($->get()); + + + } + } +} diff --git a/src/Resources/skeleton/dto/MutablePublicDto.tpl.php b/src/Resources/skeleton/dto/MutablePublicDto.tpl.php new file mode 100644 index 000000000..4f772d7e8 --- /dev/null +++ b/src/Resources/skeleton/dto/MutablePublicDto.tpl.php @@ -0,0 +1,34 @@ + + + +namespace ; + + +use ; + + +/** + * Data transfer object for . + */ +class + +{ + /** + * Create DTO, optionally extracting data from a model. + */ + public function __construct( $ = null) + { + if ($) { + $mapping): ?> + + // @todo implement getter on the Entity + //$this-> = $->get(); + + $this-> = $->get(); + + + } + } +} diff --git a/src/Util/DTOClassSourceManipulator.php b/src/Util/DTOClassSourceManipulator.php index 806e0e474..e70cd42f9 100644 --- a/src/Util/DTOClassSourceManipulator.php +++ b/src/Util/DTOClassSourceManipulator.php @@ -12,6 +12,7 @@ namespace Symfony\Bundle\MakerBundle\Util; use PhpParser\Builder; +use PhpParser\Builder\Param; use PhpParser\BuilderHelpers; use PhpParser\Lexer; use PhpParser\Node; @@ -33,6 +34,10 @@ final class DTOClassSourceManipulator private $overwrite; private $useAnnotations; private $fluentMutators; + private $generateGetters; + private $generateSetters; + private $generatePublicProperties; + private $generateConstructor; private $parser; private $lexer; private $printer; @@ -46,11 +51,16 @@ final class DTOClassSourceManipulator private $pendingComments = []; - public function __construct(string $sourceCode, bool $overwrite = false, bool $useAnnotations = true, bool $fluentMutators = true, bool $generateGettersSetters = true) + private $pendingConstructorParams = []; + + public function __construct(string $sourceCode, bool $overwrite = false, bool $useAnnotations = true, bool $fluentMutators = true, bool $generateGetters = true, bool $generateSetters = true, bool $generatePublicProperties = true, bool $generateConstructor = true) { $this->overwrite = $overwrite; $this->useAnnotations = $useAnnotations; - $this->generateGettersSetters = $generateGettersSetters; + $this->generateGetters = $generateGetters; + $this->generateSetters = $generateSetters; + $this->generatePublicProperties = $generatePublicProperties; + $this->generateConstructor = $generateConstructor; $this->fluentMutators = $fluentMutators; $this->lexer = new Lexer\Emulative([ 'usedAttributes' => [ @@ -86,23 +96,30 @@ public function addEntityField(string $propertyName, array $columnOptions, array } $this->addProperty($propertyName, $comments, $defaultValue); - // return early when setters/getters should not be added. - if (false === $this->generateGettersSetters) { - return; + if ($this->generateGetters) { + $this->addGetter( + $propertyName, + $typeHint, + // getter methods always have nullable return values + // because even though these are required in the db, they may not be set yet + true + ); } - $this->addGetter( - $propertyName, - $typeHint, - // getter methods always have nullable return values - // because even though these are required in the db, they may not be set yet - true - ); - // don't generate setters for id fields - if (!$isId) { + if ($this->generateSetters && !$isId) { $this->addSetter($propertyName, $typeHint, $nullable); } + + if ($this->generateConstructor) { + $this->pendingConstructorParams[$propertyName] = [ + 'name' => $propertyName, + 'nullable' => $nullable, + 'type' => $typeHint, + 'default' => $defaultValue, + 'isId' => $isId, + ]; + } } public function addAccessorMethod(string $propertyName, string $methodName, $returnType, bool $isReturnTypeNullable, array $commentLines = [], $typeCast = null) @@ -124,11 +141,36 @@ public function addSetter(string $propertyName, $type, bool $isNullable, array $ $this->addMethod($builder->getNode()); } + /** + * @param Node[] $params + */ + public function addConstructor(array $params, string $methodBody) + { + if (null !== $this->getConstructorNode()) { + throw new \LogicException('Constructor already exists.'); + } + + $methodBuilder = $this->createMethodBuilder('__construct', null, false); + + $this->addMethodParams($methodBuilder, $params); + + $this->addMethodBody($methodBuilder, $methodBody); + + $this->addNodeAfterProperties($methodBuilder->getNode()); + $this->updateSourceCodeFromNewStmts(); + } + public function addMethodBuilder(Builder\Method $methodBuilder) { $this->addMethod($methodBuilder->getNode()); } + public function addMethodBody(Builder\Method $methodBuilder, string $methodBody) + { + $nodes = $this->parser->parse($methodBody); + $methodBuilder->addStmts($nodes); + } + public function createMethodBuilder(string $methodName, $returnType, bool $isReturnTypeNullable, array $commentLines = []): Builder\Method { $methodNodeBuilder = (new Builder\Method($methodName)) @@ -164,8 +206,7 @@ public function addProperty(string $name, array $annotationLines = [], $defaultV } $newPropertyBuilder = new Builder\Property($name); - // if we do not add getters/setters, the fields must be public - if (false === $this->generateGettersSetters) { + if ($this->generatePublicProperties) { $newPropertyBuilder->makePublic(); } else { $newPropertyBuilder->makePrivate(); @@ -412,6 +453,56 @@ public function addUseStatementIfNecessary(string $class): string return $shortClassName; } + public function createConstructor() + { + $pendingConstructorParams = $this->getPendingConstructorParams(); + if (!\count($pendingConstructorParams)) { + return; + } + + // sort to put optional params to the end. + uasort($pendingConstructorParams, [$this, 'sortConstructorParams']); + + $params = []; + $methodBody = 'setType($paramOptions['nullable'] ? new Node\NullableType($paramOptions['type']) : $paramOptions['type']); + if (null !== $paramOptions['default']) { + $param->setDefault($paramOptions['default']); + } + $params[] = $param->getNode(); + + $methodBody .= '$this->'.$paramOptions['name'].' = $'.$paramOptions['name'].';'.PHP_EOL; + } + $this->addConstructor($params, $methodBody); + } + + private function sortConstructorParams(array $paramA, array $paramB): int + { + if ($paramA['nullable'] || $paramB['nullable']) { + if ($paramA['nullable'] === $paramB['nullable']) { + return 0; + } + + return ($paramA['nullable'] > $paramB['nullable']) ? 1 : -1; + } + if (null !== $paramA['default']) { + return 1; + } + if (null !== $paramB['default']) { + return -1; + } + + return 0; + } + + public function getPendingConstructorParams(): array + { + return $this->pendingConstructorParams; + } + private function updateSourceCodeFromNewStmts() { $newCode = $this->printer->printFormatPreserving( @@ -759,6 +850,13 @@ private function propertyExists(string $propertyName) return false; } + private function addMethodParams(Builder\Method $methodBuilder, array $params) + { + foreach ($params as $param) { + $methodBuilder->addParam($param); + } + } + private function writeNote(string $note) { if (null !== $this->io) { diff --git a/tests/Maker/MakeDtoTest.php b/tests/Maker/MakeDtoTest.php index 88ffa2863..9990026aa 100644 --- a/tests/Maker/MakeDtoTest.php +++ b/tests/Maker/MakeDtoTest.php @@ -19,180 +19,199 @@ class MakeDtoTest extends MakerTestCase { public function getTestDetails() { - yield 'dto' => [MakerTestDetails::createTest( + yield 'dto_annotations' => [MakerTestDetails::createTest( $this->getMakerInstance(MakeDto::class), [ - 'Task', - 'Task', - // generate helpers - 'yes', - // generate getters - 'no', + 1, + // Add mutator to Entity (default) + '', ]) - ->addExtraDependencies('orm') + ->setArgumentsString('TaskData Task') + ->addExtraDependencies('doctrine') ->addExtraDependencies('validator') - ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeDto') - ->assert(function (string $output, string $directory) { + ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeDtoAnnotations') + ->assert(function (string $output) { $this->assertContains('created: src/Dto/TaskData.php', $output); $this->assertContains('updated: src/Dto/TaskData.php', $output); $this->assertContains('\\App\\Dto\\TaskData', $output); }), ]; + yield 'dto_composite_id' => [MakerTestDetails::createTest( + $this->getMakerInstance(MakeDto::class), + [ + // Mutable public (for simplicity) + 2, + // Add mutator to Entity (default) + '', + ]) + ->setArgumentsString('TaskData Task') + ->addExtraDependencies('doctrine') + ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeDtoCompositeId') + ->assert(function (string $output) { + $this->assertContains('created: src/Dto/TaskData.php', $output); + $this->assertContains('\\App\\Dto\\TaskData', $output); + }), + ]; + yield 'dto_getters_setters' => [MakerTestDetails::createTest( $this->getMakerInstance(MakeDto::class), [ - 'Task', - 'Task', - // generate helpers - 'yes', - // generate getters - 'yes', + // Mutable, with getters & setters + 1, + // Add mutator to Entity (default) + '', ]) - ->addExtraDependencies('orm') - ->addExtraDependencies('validator') + ->setArgumentsString('TaskData Task') + ->addExtraDependencies('doctrine') ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeDtoGettersSetters') - ->assert(function (string $output, string $directory) { + ->assert(function (string $output) { $this->assertContains('created: src/Dto/TaskData.php', $output); $this->assertContains('updated: src/Dto/TaskData.php', $output); }), ]; - yield 'dto_validator_yaml_xml' => [MakerTestDetails::createTest( + yield 'dto_invalid_entity' => [MakerTestDetails::createTest( $this->getMakerInstance(MakeDto::class), [ - 'Task', - 'Task', - // generate helpers - 'yes', - // generate getters - 'no', + // Mutable, with getters & setters + 1, + // Add mutator to Entity (default) + '', ]) - ->addExtraDependencies('orm') - ->addExtraDependencies('validator') - ->addExtraDependencies('yaml') - ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeDtoValidatorYamlXml') - ->assert(function (string $output, string $directory) { + // bound class, can not use "Task" because invalid entity is not in autocomplete + ->setArgumentsString('TaskData \\App\\Entity\\Task') + ->addExtraDependencies('doctrine') + ->setCommandAllowedToFail(true) + ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeDtoInvalidEntity') + ->assert(function (string $output) { + $this->assertContains('The bound class is not a valid doctrine entity.', $output); + }), + ]; + + yield 'dto_immutable' => [MakerTestDetails::createTest( + $this->getMakerInstance(MakeDto::class), + [ + // Immutable, with getters only + 3, + // Add mutator to Entity (default) + '', + ]) + ->setArgumentsString('TaskData Task') + ->addExtraDependencies('doctrine') + ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeDtoImmutable') + ->assert(function (string $output) { $this->assertContains('created: src/Dto/TaskData.php', $output); $this->assertContains('updated: src/Dto/TaskData.php', $output); - $this->assertContains('The entity possibly uses Yaml/Xml validators.', $output); + $this->assertContains('\\App\\Dto\\TaskData', $output); }), ]; - yield 'dto_without_helpers' => [MakerTestDetails::createTest( + yield 'dto_mapped_super_class' => [MakerTestDetails::createTest( $this->getMakerInstance(MakeDto::class), [ - 'Task', - 'Task', - // generate helpers - 'no', - // generate getters - 'yes', + // Mutable public (for simplicity) + 2, + // Add mutator to Entity (default) + '', ]) - ->addExtraDependencies('orm') - ->addExtraDependencies('validator') - ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeDtoWithoutHelpers') - ->assert(function (string $output, string $directory) { + ->setArgumentsString('TaskData Task') + ->addExtraDependencies('doctrine') + ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeDtoMappedSuperClass') + ->assert(function (string $output) { $this->assertContains('created: src/Dto/TaskData.php', $output); $this->assertContains('updated: src/Dto/TaskData.php', $output); + $this->assertContains('\\App\\Dto\\TaskData', $output); }), ]; - yield 'dto_without_validations' => [MakerTestDetails::createTest( + yield 'dto_missing_getters_setters' => [MakerTestDetails::createTest( $this->getMakerInstance(MakeDto::class), [ - 'Task', - 'Task', - // generate helpers + // Mutable, with getters & setters + 1, + // Add mutator to Entity 'yes', - // generate getters - 'no', ]) - ->addExtraDependencies('orm') - ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeDtoWithoutValidations') - ->assert(function (string $output, string $directory) { + ->setArgumentsString('TaskData Task') + ->addExtraDependencies('doctrine') + ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeDtoMissingGettersSetters') + ->assert(function (string $output) { $this->assertContains('created: src/Dto/TaskData.php', $output); $this->assertContains('updated: src/Dto/TaskData.php', $output); + $this->assertContains('\\App\\Dto\\TaskData', $output); + $this->assertContains('The maker found missing getters/setters for properties in the entity.', $output); }), ]; - yield 'dto_invalid_entity' => [MakerTestDetails::createTest( + yield 'dto_mutator' => [MakerTestDetails::createTest( $this->getMakerInstance(MakeDto::class), [ - 'Task', - // bound class, can not use "Task" because invalid entity is not in autocomplete - '\\App\\Entity\\Task', - // generate helpers + // Mutable public (for simplicity) + 2, + // Add mutator to Entity 'yes', - // generate getters - 'no', ]) - ->addExtraDependencies('orm') - ->setCommandAllowedToFail(true) - ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeDtoInvalidEntity') - ->assert(function (string $output, string $directory) { - $this->assertContains('The bound class is not a valid doctrine entity.', $output); + ->setArgumentsString('TaskData Task') + ->addExtraDependencies('doctrine') + ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeDtoMutator') + ->assert(function (string $output) { + $this->assertContains('created: src/Dto/TaskData.php', $output); + $this->assertContains('updated: src/Dto/TaskData.php', $output); }), ]; - yield 'dto_mapped_super_class' => [MakerTestDetails::createTest( + yield 'dto_no_mutator' => [MakerTestDetails::createTest( $this->getMakerInstance(MakeDto::class), [ - 'Task', - 'Task', - // generate helpers - 'yes', - // generate getters + // Mutable public (for simplicity) + 2, + // Add no mutator to Entity 'no', ]) - ->addExtraDependencies('orm') - ->addExtraDependencies('validator') - ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeDtoMappedSuperClass') - ->assert(function (string $output, string $directory) { + ->setArgumentsString('TaskData Task') + ->addExtraDependencies('doctrine') + ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeDtoNoMutator') + ->assert(function (string $output) { $this->assertContains('created: src/Dto/TaskData.php', $output); $this->assertContains('updated: src/Dto/TaskData.php', $output); - $this->assertContains('\\App\\Dto\\TaskData', $output); }), ]; - yield 'dto_composite_id' => [MakerTestDetails::createTest( + yield 'dto_public' => [MakerTestDetails::createTest( $this->getMakerInstance(MakeDto::class), [ - 'Task', - 'Task', - // generate helpers - 'yes', - // generate getters - 'no', + // Mutable public + 2, + // Add mutator to Entity (default) + '', ]) - ->addExtraDependencies('orm') - ->addExtraDependencies('validator') - ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeDtoCompositeId') - ->assert(function (string $output, string $directory) { + ->setArgumentsString('TaskData Task') + ->addExtraDependencies('doctrine') + ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeDtoPublic') + ->assert(function (string $output) { $this->assertContains('created: src/Dto/TaskData.php', $output); $this->assertContains('updated: src/Dto/TaskData.php', $output); - $this->assertContains('\\App\\Dto\\TaskData', $output); }), ]; - yield 'dto_missing_getters_setters' => [MakerTestDetails::createTest( + yield 'dto_validator_yaml_xml' => [MakerTestDetails::createTest( $this->getMakerInstance(MakeDto::class), [ - 'Task', - 'Task', - // generate helpers - 'yes', - // generate getters - 'yes', + // Mutable public (for simplicity) + 2, + // Add mutator to Entity (default) + '', ]) - ->addExtraDependencies('orm') + ->setArgumentsString('TaskData Task') + ->addExtraDependencies('doctrine') ->addExtraDependencies('validator') - ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeDtoMissingGettersSetters') - ->assert(function (string $output, string $directory) { + ->addExtraDependencies('yaml') + ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeDtoValidatorYamlXml') + ->assert(function (string $output) { $this->assertContains('created: src/Dto/TaskData.php', $output); $this->assertContains('updated: src/Dto/TaskData.php', $output); - $this->assertContains('\\App\\Dto\\TaskData', $output); - $this->assertContains('The maker found missing getters/setters for properties in the entity.', $output); + $this->assertContains('The entity possibly uses Yaml/Xml validators.', $output); }), ]; } diff --git a/tests/Util/DTOClassSourceManipulatorTest.php b/tests/Util/DTOClassSourceManipulatorTest.php new file mode 100644 index 000000000..ff9649574 --- /dev/null +++ b/tests/Util/DTOClassSourceManipulatorTest.php @@ -0,0 +1,158 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Bundle\MakerBundle\Tests\Util; + +use PHPUnit\Framework\TestCase; +use ReflectionMethod; +use Symfony\Bundle\MakerBundle\Util\DTOClassSourceManipulator; + +class DTOClassSourceManipulatorTest extends TestCase +{ + /** + * @dataProvider getSortConstructorParamsTests + */ + public function testSortConstructorParams(array $params, array $sortedParams) + { + $manipulator = new DTOClassSourceManipulator('setAccessible(true); + + $sortClosure = function (array $paramA, array $paramB) use ($sortMethod, $manipulator) { + return $sortMethod->invoke($manipulator, $paramA, $paramB); + }; + + usort($params, $sortClosure); + + $this->assertSame($params, $sortedParams); + } + + public function getSortConstructorParamsTests() + { + yield 'sort_nullable' => [ + [ + [ + 'name' => 'a', + 'nullable' => 'true', + 'default' => null, + ], + [ + 'name' => 'b', + 'nullable' => false, + 'default' => null, + ], + ], + [ + [ + 'name' => 'b', + 'nullable' => false, + 'default' => null, + ], + [ + 'name' => 'a', + 'nullable' => 'true', + 'default' => null, + ], + ], + ]; + + yield 'sort_default' => [ + [ + [ + 'name' => 'a', + 'nullable' => false, + 'default' => [], + ], + [ + 'name' => 'b', + 'nullable' => false, + 'default' => null, + ], + ], + [ + [ + 'name' => 'b', + 'nullable' => false, + 'default' => null, + ], + [ + 'name' => 'a', + 'nullable' => false, + 'default' => [], + ], + ], + ]; + + yield 'sort_nullable_and_default' => [ + [ + [ + 'name' => 'a', + 'nullable' => false, + 'default' => [], + ], + [ + 'name' => 'b', + 'nullable' => true, + 'default' => null, + ], + ], + [ + [ + 'name' => 'a', + 'nullable' => false, + 'default' => [], + ], + [ + 'name' => 'b', + 'nullable' => true, + 'default' => null, + ], + ], + ]; + + yield 'sort_nullable_and_default_multiple' => [ + [ + [ + 'name' => 'a', + 'nullable' => false, + 'default' => [], + ], + [ + 'name' => 'b', + 'nullable' => true, + 'default' => null, + ], + [ + 'name' => 'c', + 'nullable' => false, + 'default' => null, + ], + ], + [ + [ + 'name' => 'c', + 'nullable' => false, + 'default' => null, + ], + [ + 'name' => 'a', + 'nullable' => false, + 'default' => [], + ], + [ + 'name' => 'b', + 'nullable' => true, + 'default' => null, + ], + ], + ]; + } +} diff --git a/tests/fixtures/MakeDto/src/Entity/Task.php b/tests/fixtures/MakeDtoAnnotations/src/Entity/Task.php similarity index 100% rename from tests/fixtures/MakeDto/src/Entity/Task.php rename to tests/fixtures/MakeDtoAnnotations/src/Entity/Task.php diff --git a/tests/fixtures/MakeDtoAnnotations/tests/TaskDataTest.php b/tests/fixtures/MakeDtoAnnotations/tests/TaskDataTest.php new file mode 100644 index 000000000..db9f2849c --- /dev/null +++ b/tests/fixtures/MakeDtoAnnotations/tests/TaskDataTest.php @@ -0,0 +1,45 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace App\Tests; + +use App\Dto\TaskData; +use Doctrine\Common\Annotations\AnnotationReader; +use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; +use Symfony\Component\Validator\Constraints\NotBlank; +use Symfony\Component\Validator\Validation; + +class TaskDataTest extends KernelTestCase +{ + public function testAnnotations() + { + $annotationReader = new AnnotationReader(); + $reflectionProperty = new \ReflectionProperty(TaskData::class, 'task'); + $propertyAnnotations = $annotationReader->getPropertyAnnotations($reflectionProperty); + $this->assertCount(1, $propertyAnnotations); + $this->assertContainsOnlyInstancesOf(NotBlank::class, $propertyAnnotations); + + $reflectionProperty = new \ReflectionProperty(TaskData::class, 'dueDate'); + $propertyAnnotations = $annotationReader->getPropertyAnnotations($reflectionProperty); + $this->assertCount(0, $propertyAnnotations); + } + + public function testValidation() + { + $validator = Validation::createValidatorBuilder()->enableAnnotationMapping()->getValidator(); + $task = new TaskData; + $errors = $validator->validate($task); + $this->assertEquals(1, count($errors)); + $task->setTask('foo'); + $errors = $validator->validate($task); + $this->assertEquals(0, count($errors)); + } +} diff --git a/tests/fixtures/MakeDtoCompositeId/src/Entity/Task.php b/tests/fixtures/MakeDtoCompositeId/src/Entity/Task.php index 50cb74c47..eaacfe4e9 100644 --- a/tests/fixtures/MakeDtoCompositeId/src/Entity/Task.php +++ b/tests/fixtures/MakeDtoCompositeId/src/Entity/Task.php @@ -12,7 +12,6 @@ namespace App\Entity; use Doctrine\ORM\Mapping as ORM; -use Symfony\Component\Validator\Constraints as Assert; /** * @ORM\Entity() @@ -28,66 +27,14 @@ class Task /** * @ORM\Id * @ORM\Column(type="string", length=255, nullable=true) - * @Assert\NotBlank() */ private $group; - /** - * @ORM\Column(type="string", length=255, nullable=true) - * @Assert\NotBlank() - */ - private $task; - - /** - * @ORM\Column(type="datetime", nullable=true) - */ - private $dueDate; - public function __construct() { $this->id = uniqid(); } - /** - * Get the value of task. - */ - public function getTask() - { - return $this->task; - } - - /** - * Set the value of task. - * - * @return self - */ - public function setTask($task) - { - $this->task = $task; - - return $this; - } - - /** - * Get the value of dueDate. - */ - public function getDueDate() - { - return $this->dueDate; - } - - /** - * Set the value of dueDate. - * - * @return self - */ - public function setDueDate($dueDate) - { - $this->dueDate = $dueDate; - - return $this; - } - /** * Get the value of group. */ diff --git a/tests/fixtures/MakeDtoCompositeId/tests/GeneratedDtoTest.php b/tests/fixtures/MakeDtoCompositeId/tests/TaskDataTest.php similarity index 87% rename from tests/fixtures/MakeDtoCompositeId/tests/GeneratedDtoTest.php rename to tests/fixtures/MakeDtoCompositeId/tests/TaskDataTest.php index 9c2d3c3ac..a10a1bb61 100644 --- a/tests/fixtures/MakeDtoCompositeId/tests/GeneratedDtoTest.php +++ b/tests/fixtures/MakeDtoCompositeId/tests/TaskDataTest.php @@ -14,9 +14,9 @@ use App\Dto\TaskData; use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; -class GeneratedDtoTest extends KernelTestCase +class TaskDataTest extends KernelTestCase { - public function testGeneratedDto() + public function testCompositeIdEmitted() { // the Entity has a composite Id - test that the attributes are both omitted from the DTO $this->assertClassNotHasAttribute('id', TaskData::class); diff --git a/tests/fixtures/MakeDtoGettersSetters/src/Entity/Task.php b/tests/fixtures/MakeDtoGettersSetters/src/Entity/Task.php index 30de1fa7d..40734ed1c 100644 --- a/tests/fixtures/MakeDtoGettersSetters/src/Entity/Task.php +++ b/tests/fixtures/MakeDtoGettersSetters/src/Entity/Task.php @@ -12,7 +12,6 @@ namespace App\Entity; use Doctrine\ORM\Mapping as ORM; -use Symfony\Component\Validator\Constraints as Assert; /** * @ORM\Entity() @@ -28,7 +27,6 @@ class Task /** * @ORM\Column(type="string", length=255, nullable=true) - * @Assert\NotBlank() */ private $task; diff --git a/tests/fixtures/MakeDtoGettersSetters/tests/GeneratedDtoTest.php b/tests/fixtures/MakeDtoGettersSetters/tests/TaskDataTest.php similarity index 61% rename from tests/fixtures/MakeDtoGettersSetters/tests/GeneratedDtoTest.php rename to tests/fixtures/MakeDtoGettersSetters/tests/TaskDataTest.php index 82589fa44..8afde4b20 100644 --- a/tests/fixtures/MakeDtoGettersSetters/tests/GeneratedDtoTest.php +++ b/tests/fixtures/MakeDtoGettersSetters/tests/TaskDataTest.php @@ -15,9 +15,9 @@ use App\Entity\Task; use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; -class GeneratedDtoTest extends KernelTestCase +class TaskDataTest extends KernelTestCase { - public function testGeneratedDto() + public function testGetters() { $this->assertTrue(method_exists(TaskData::class, 'setTask')); $this->assertTrue(method_exists(TaskData::class, 'getTask')); @@ -26,11 +26,18 @@ public function testGeneratedDto() $this->assertTrue(method_exists(TaskData::class, 'getDueDate')); } - public function testGettersSetters() + public function testConstructor() { - $taskData = new Task(); - $taskData->setTask('Acme'); - $taskData->setDueDate(new \DateTime('2018-01-29 01:30')); + $this->assertTrue(method_exists(TaskData::class, '__construct')); + + $taskEntity = new Task(); + $taskEntity->setTask('Acme'); + $taskEntity->setDueDate(new \DateTime('2018-01-29 01:30')); + + $taskData = new TaskData($taskEntity); + + $this->assertEquals($taskEntity->getTask(), $taskData->getTask()); + $this->assertEquals($taskEntity->getDueDate(), $taskData->getDueDate()); $this->assertEquals($taskData->getTask(), 'Acme'); $this->assertEquals($taskData->getDueDate(), new \DateTime('2018-01-29 01:30')); diff --git a/tests/fixtures/MakeDtoWithoutValidations/src/Entity/Task.php b/tests/fixtures/MakeDtoImmutable/src/Entity/Task.php similarity index 100% rename from tests/fixtures/MakeDtoWithoutValidations/src/Entity/Task.php rename to tests/fixtures/MakeDtoImmutable/src/Entity/Task.php diff --git a/tests/fixtures/MakeDtoImmutable/tests/TaskDataTest.php b/tests/fixtures/MakeDtoImmutable/tests/TaskDataTest.php new file mode 100644 index 000000000..f5142636f --- /dev/null +++ b/tests/fixtures/MakeDtoImmutable/tests/TaskDataTest.php @@ -0,0 +1,41 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace App\Tests; + +use App\Dto\TaskData; +use App\Entity\Task; +use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; + +class TaskDataTest extends KernelTestCase +{ + public function testSetters() + { + $this->assertFalse(method_exists(TaskData::class, 'setTask')); + $this->assertFalse(method_exists(TaskData::class, 'setDueDate')); + } + + public function testGetters() + { + $this->assertTrue(method_exists(TaskData::class, 'getTask')); + $this->assertTrue(method_exists(TaskData::class, 'getDueDate')); + } + + public function testConstructor() + { + $this->assertTrue(method_exists(TaskData::class, '__construct')); + + $taskData = new TaskData('Acme', new \DateTime('2018-01-29 01:30')); + + $this->assertEquals($taskData->getTask(), 'Acme'); + $this->assertEquals($taskData->getDueDate(), new \DateTime('2018-01-29 01:30')); + } +} diff --git a/tests/fixtures/MakeDtoMappedSuperClass/src/Entity/Task.php b/tests/fixtures/MakeDtoMappedSuperClass/src/Entity/Task.php index 7e5d034fe..eb417858f 100644 --- a/tests/fixtures/MakeDtoMappedSuperClass/src/Entity/Task.php +++ b/tests/fixtures/MakeDtoMappedSuperClass/src/Entity/Task.php @@ -12,7 +12,6 @@ namespace App\Entity; use Doctrine\ORM\Mapping as ORM; -use Symfony\Component\Validator\Constraints as Assert; /** * @ORM\Entity() @@ -21,7 +20,6 @@ class Task extends Deadline { /** * @ORM\Column(type="string", length=255, nullable=true) - * @Assert\NotBlank() */ private $task; diff --git a/tests/fixtures/MakeDto/tests/GeneratedDtoTest.php b/tests/fixtures/MakeDtoMappedSuperClass/tests/TaskDataTest.php similarity index 62% rename from tests/fixtures/MakeDto/tests/GeneratedDtoTest.php rename to tests/fixtures/MakeDtoMappedSuperClass/tests/TaskDataTest.php index 58b8a1657..abcb0bd6e 100644 --- a/tests/fixtures/MakeDto/tests/GeneratedDtoTest.php +++ b/tests/fixtures/MakeDtoMappedSuperClass/tests/TaskDataTest.php @@ -17,17 +17,14 @@ use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; use Symfony\Component\Validator\Constraints\NotBlank; -class GeneratedDtoTest extends KernelTestCase +class TaskDataTest extends KernelTestCase { - public function testGeneratedDto() + public function testMappedSuperClass() { $this->assertClassHasAttribute('task', TaskData::class); $this->assertClassHasAttribute('dueDate', TaskData::class); $this->assertClassNotHasAttribute('id', TaskData::class); - $this->assertTrue(method_exists(TaskData::class, 'fill')); - $this->assertTrue(method_exists(TaskData::class, 'extract')); - $this->assertFalse(method_exists(TaskData::class, 'setTask')); $this->assertFalse(method_exists(TaskData::class, 'getTask')); @@ -35,7 +32,7 @@ public function testGeneratedDto() $this->assertFalse(method_exists(TaskData::class, 'getDueDate')); } - public function testHelpers() + public function testMutator() { $taskEntity = new Task(); $taskEntity->setTask('Acme'); @@ -49,22 +46,9 @@ public function testHelpers() $taskData->task = 'Foo'; $taskEntity = new Task(); - $taskData->fill($taskEntity); + $taskEntity->updateFromTaskData($taskData); $this->assertEquals($taskEntity->getTask(), $taskData->task); $this->assertEquals($taskEntity->getDueDate(), $taskData->dueDate); } - - public function testAnnotations() - { - $annotationReader = new AnnotationReader(); - $reflectionProperty = new \ReflectionProperty(TaskData::class, 'task'); - $propertyAnnotations = $annotationReader->getPropertyAnnotations($reflectionProperty); - $this->assertCount(1, $propertyAnnotations); - $this->assertContainsOnlyInstancesOf(NotBlank::class, $propertyAnnotations); - - $reflectionProperty = new \ReflectionProperty(TaskData::class, 'dueDate'); - $propertyAnnotations = $annotationReader->getPropertyAnnotations($reflectionProperty); - $this->assertCount(0, $propertyAnnotations); - } } diff --git a/tests/fixtures/MakeDtoMissingGettersSetters/src/Entity/Task.php b/tests/fixtures/MakeDtoMissingGettersSetters/src/Entity/Task.php index d6cb4924c..142352e12 100644 --- a/tests/fixtures/MakeDtoMissingGettersSetters/src/Entity/Task.php +++ b/tests/fixtures/MakeDtoMissingGettersSetters/src/Entity/Task.php @@ -12,7 +12,6 @@ namespace App\Entity; use Doctrine\ORM\Mapping as ORM; -use Symfony\Component\Validator\Constraints as Assert; /** * @ORM\Entity() @@ -28,21 +27,28 @@ class Task /** * @ORM\Column(type="string", length=255, nullable=true) - * @Assert\NotBlank() */ private $task; /** - * @ORM\Column(type="datetime", nullable=true) + * @ORM\Column(type="string", length=255, nullable=true) + */ + private $iCanGetNo; + + /** + * @ORM\Column(type="string", length=255, nullable=true) */ - private $dueDate; + private $iCanSetNo; public function getId() { return $this->id; } - // commented out for the test. + /** + * Commented out for the test. + * We want the setter/getter to be missing. + */ // /** // * Get the value of task. // */ @@ -64,22 +70,50 @@ public function getId() // } /** - * Get the value of dueDate. + * Commented out for the test. + * We want the getter to be missing. */ - public function getDueDate() - { - return $this->dueDate; - } + // /** + // * Get the value of iCanGetNo + // */ + // public function getICanGetNo() + // { + // return $this->iCanGetNo; + // } /** - * Set the value of dueDate. + * Set the value of iCanGetNo. * * @return self */ - public function setDueDate($dueDate) + public function setICanGetNo($iCanGetNo) { - $this->dueDate = $dueDate; + $this->iCanGetNo = $iCanGetNo; return $this; } + + /** + * Commented out for the test. + * We want the setter to be missing. + */ + // /** + // * Set the value of iCanSetNo. + // * + // * @return self + // */ + // public function setICanSetNo($iCanSetNo) + // { + // $this->iCanSetNo = $iCanSetNo; + + // return $this; + // } + + /** + * Get the value of iCanSetNo. + */ + public function getICanSetNo() + { + return $this->iCanSetNo; + } } diff --git a/tests/fixtures/MakeDtoMissingGettersSetters/tests/TaskDataTest.php b/tests/fixtures/MakeDtoMissingGettersSetters/tests/TaskDataTest.php new file mode 100644 index 000000000..ba48c3716 --- /dev/null +++ b/tests/fixtures/MakeDtoMissingGettersSetters/tests/TaskDataTest.php @@ -0,0 +1,90 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace App\Tests; + +use App\Dto\TaskData; +use App\Entity\Task; +use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; + +class TaskDataTest extends KernelTestCase +{ + public function testSettersGetters() + { + // no setters / getters + $this->assertFalse(method_exists(Task::class, 'setTask')); + $this->assertFalse(method_exists(Task::class, 'getTask')); + $this->assertTrue(method_exists(TaskData::class, 'setTask')); + $this->assertTrue(method_exists(TaskData::class, 'getTask')); + + // missing getter but having setter + $this->assertTrue(method_exists(Task::class, 'setICanGetNo')); + $this->assertFalse(method_exists(Task::class, 'getICanGetNo')); + $this->assertTrue(method_exists(TaskData::class, 'setICanGetNo')); + $this->assertTrue(method_exists(TaskData::class, 'getICanGetNo')); + + // having getter but missing setter + $this->assertFalse(method_exists(Task::class, 'setICanSetNo')); + $this->assertTrue(method_exists(Task::class, 'getICanSetNo')); + $this->assertTrue(method_exists(TaskData::class, 'setICanSetNo')); + $this->assertTrue(method_exists(TaskData::class, 'getICanSetNo')); + } + + public function testConstructorWithMissingGetter() + { + $taskEntity = new Task(); + $taskEntity->setICanGetNo('Satisfaction'); + + // set private task property of the entity via Reflection - we do not have a setter + $entityReflection = new \ReflectionClass($taskEntity); + $taskProperty = $entityReflection->getProperty('task'); + $taskProperty->setAccessible(true); + $taskProperty->setValue($taskEntity, 'Foo'); + + // set the private iCanSetNo property of the entity via Reflection - we do not have a setter + $entityReflection = new \ReflectionClass($taskEntity); + $taskProperty = $entityReflection->getProperty('iCanSetNo'); + $taskProperty->setAccessible(true); + $taskProperty->setValue($taskEntity, 'Satisfaction'); + + $taskData = new TaskData($taskEntity); + + $this->assertEquals('Satisfaction', $taskData->getICanSetNo()); + $this->assertNull($taskData->getICanGetNo()); + $this->assertNull($taskData->getTask()); + } + + public function testMutator() + { + $taskData = new TaskData; + $taskData->setTask('Acme'); + $taskData->setICanGetNo('Satisfaction'); + $taskData->setICanSetNo('Bar'); + + $taskEntity = new Task(); + // will update the entity including the private properties + $taskEntity->updateFromTaskData($taskData); + + // make the private task property accessible to compare the values + $entityReflection = new \ReflectionClass($taskEntity); + $taskProperty = $entityReflection->getProperty('task'); + $taskProperty->setAccessible(true); + $this->assertEquals($taskProperty->getValue($taskEntity), 'Acme'); + + // make the private iCanGetNo property accessible to compare the values + $entityReflection = new \ReflectionClass($taskEntity); + $iCanGetNoProperty = $entityReflection->getProperty('iCanGetNo'); + $iCanGetNoProperty->setAccessible(true); + $this->assertEquals($iCanGetNoProperty->getValue($taskEntity), 'Satisfaction'); + + $this->assertEquals($taskEntity->getICanSetNo(), 'Bar'); + } +} diff --git a/tests/fixtures/MakeDtoWithoutHelpers/src/Entity/Task.php b/tests/fixtures/MakeDtoMutator/src/Entity/Task.php similarity index 94% rename from tests/fixtures/MakeDtoWithoutHelpers/src/Entity/Task.php rename to tests/fixtures/MakeDtoMutator/src/Entity/Task.php index 30de1fa7d..40734ed1c 100644 --- a/tests/fixtures/MakeDtoWithoutHelpers/src/Entity/Task.php +++ b/tests/fixtures/MakeDtoMutator/src/Entity/Task.php @@ -12,7 +12,6 @@ namespace App\Entity; use Doctrine\ORM\Mapping as ORM; -use Symfony\Component\Validator\Constraints as Assert; /** * @ORM\Entity() @@ -28,7 +27,6 @@ class Task /** * @ORM\Column(type="string", length=255, nullable=true) - * @Assert\NotBlank() */ private $task; diff --git a/tests/fixtures/MakeDtoMissingGettersSetters/tests/GeneratedDtoTest.php b/tests/fixtures/MakeDtoMutator/tests/TaskDataTest.php similarity index 50% rename from tests/fixtures/MakeDtoMissingGettersSetters/tests/GeneratedDtoTest.php rename to tests/fixtures/MakeDtoMutator/tests/TaskDataTest.php index 69f18461c..03bc1f643 100644 --- a/tests/fixtures/MakeDtoMissingGettersSetters/tests/GeneratedDtoTest.php +++ b/tests/fixtures/MakeDtoMutator/tests/TaskDataTest.php @@ -15,23 +15,23 @@ use App\Entity\Task; use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; -class GeneratedDtoTest extends KernelTestCase +class TaskDataTest extends KernelTestCase { - /** - * Use the helpers to make sure that they work, even though there are missing getters/setters. - */ - public function testHelpers() + public function testTaskData() { - $taskEntity = new Task(); - $taskEntity->setDueDate(new \DateTime('2018-01-29 01:30')); - - $taskData = new TaskData($taskEntity); + $this->assertTrue(method_exists(Task::class, 'updateFromTaskData')); + } - $this->assertEquals($taskEntity->getDueDate(), $taskData->getDueDate()); + public function testMutator() + { + $taskData = new TaskData(); + $taskData->dueDate = new \DateTime('2018-01-29 01:30'); + $taskData->task = 'Acme'; $taskEntity = new Task(); - $taskData->fill($taskEntity); + $taskEntity->updateFromTaskData($taskData); - $this->assertEquals($taskEntity->getDueDate(), $taskData->getDueDate()); + $this->assertEquals($taskEntity->getDueDate(), $taskData->dueDate); + $this->assertEquals($taskEntity->getTask(), $taskData->task); } } diff --git a/tests/fixtures/MakeDtoNoMutator/src/Entity/Task.php b/tests/fixtures/MakeDtoNoMutator/src/Entity/Task.php new file mode 100644 index 000000000..40734ed1c --- /dev/null +++ b/tests/fixtures/MakeDtoNoMutator/src/Entity/Task.php @@ -0,0 +1,82 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace App\Entity; + +use Doctrine\ORM\Mapping as ORM; + +/** + * @ORM\Entity() + */ +class Task +{ + /** + * @ORM\Id + * @ORM\GeneratedValue + * @ORM\Column(type="integer") + */ + private $id; + + /** + * @ORM\Column(type="string", length=255, nullable=true) + */ + private $task; + + /** + * @ORM\Column(type="datetime", nullable=true) + */ + private $dueDate; + + public function getId() + { + return $this->id; + } + + /** + * Get the value of task. + */ + public function getTask() + { + return $this->task; + } + + /** + * Set the value of task. + * + * @return self + */ + public function setTask($task) + { + $this->task = $task; + + return $this; + } + + /** + * Get the value of dueDate. + */ + public function getDueDate() + { + return $this->dueDate; + } + + /** + * Set the value of dueDate. + * + * @return self + */ + public function setDueDate($dueDate) + { + $this->dueDate = $dueDate; + + return $this; + } +} diff --git a/tests/fixtures/MakeDtoWithoutHelpers/tests/GeneratedDtoTest.php b/tests/fixtures/MakeDtoNoMutator/tests/TaskDataTest.php similarity index 58% rename from tests/fixtures/MakeDtoWithoutHelpers/tests/GeneratedDtoTest.php rename to tests/fixtures/MakeDtoNoMutator/tests/TaskDataTest.php index 2c3f108e0..0668352c7 100644 --- a/tests/fixtures/MakeDtoWithoutHelpers/tests/GeneratedDtoTest.php +++ b/tests/fixtures/MakeDtoNoMutator/tests/TaskDataTest.php @@ -11,14 +11,13 @@ namespace App\Tests; -use App\Dto\TaskData; +use App\Entity\Task; use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; -class GeneratedDtoTest extends KernelTestCase +class TaskDataTest extends KernelTestCase { - public function testGeneratedDto() + public function testNoMutators() { - $this->assertFalse(method_exists(TaskData::class, 'fill')); - $this->assertFalse(method_exists(TaskData::class, 'extract')); + $this->assertFalse(method_exists(Task::class, 'updateFromTaskData')); } } diff --git a/tests/fixtures/MakeDtoPublic/src/Entity/Task.php b/tests/fixtures/MakeDtoPublic/src/Entity/Task.php new file mode 100644 index 000000000..40734ed1c --- /dev/null +++ b/tests/fixtures/MakeDtoPublic/src/Entity/Task.php @@ -0,0 +1,82 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace App\Entity; + +use Doctrine\ORM\Mapping as ORM; + +/** + * @ORM\Entity() + */ +class Task +{ + /** + * @ORM\Id + * @ORM\GeneratedValue + * @ORM\Column(type="integer") + */ + private $id; + + /** + * @ORM\Column(type="string", length=255, nullable=true) + */ + private $task; + + /** + * @ORM\Column(type="datetime", nullable=true) + */ + private $dueDate; + + public function getId() + { + return $this->id; + } + + /** + * Get the value of task. + */ + public function getTask() + { + return $this->task; + } + + /** + * Set the value of task. + * + * @return self + */ + public function setTask($task) + { + $this->task = $task; + + return $this; + } + + /** + * Get the value of dueDate. + */ + public function getDueDate() + { + return $this->dueDate; + } + + /** + * Set the value of dueDate. + * + * @return self + */ + public function setDueDate($dueDate) + { + $this->dueDate = $dueDate; + + return $this; + } +} diff --git a/tests/fixtures/MakeDtoMappedSuperClass/tests/GeneratedDtoTest.php b/tests/fixtures/MakeDtoPublic/tests/TaskDataTest.php similarity index 56% rename from tests/fixtures/MakeDtoMappedSuperClass/tests/GeneratedDtoTest.php rename to tests/fixtures/MakeDtoPublic/tests/TaskDataTest.php index 58b8a1657..091594330 100644 --- a/tests/fixtures/MakeDtoMappedSuperClass/tests/GeneratedDtoTest.php +++ b/tests/fixtures/MakeDtoPublic/tests/TaskDataTest.php @@ -13,21 +13,16 @@ use App\Dto\TaskData; use App\Entity\Task; -use Doctrine\Common\Annotations\AnnotationReader; use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; -use Symfony\Component\Validator\Constraints\NotBlank; -class GeneratedDtoTest extends KernelTestCase +class TaskDataTest extends KernelTestCase { - public function testGeneratedDto() + public function testPublicVariables() { $this->assertClassHasAttribute('task', TaskData::class); $this->assertClassHasAttribute('dueDate', TaskData::class); $this->assertClassNotHasAttribute('id', TaskData::class); - $this->assertTrue(method_exists(TaskData::class, 'fill')); - $this->assertTrue(method_exists(TaskData::class, 'extract')); - $this->assertFalse(method_exists(TaskData::class, 'setTask')); $this->assertFalse(method_exists(TaskData::class, 'getTask')); @@ -35,7 +30,7 @@ public function testGeneratedDto() $this->assertFalse(method_exists(TaskData::class, 'getDueDate')); } - public function testHelpers() + public function testMutator() { $taskEntity = new Task(); $taskEntity->setTask('Acme'); @@ -48,23 +43,11 @@ public function testHelpers() $taskData->task = 'Foo'; - $taskEntity = new Task(); - $taskData->fill($taskEntity); + $this->assertNotEquals($taskEntity->getTask(), $taskData->task); + + $taskEntity->updateFromTaskData($taskData); $this->assertEquals($taskEntity->getTask(), $taskData->task); $this->assertEquals($taskEntity->getDueDate(), $taskData->dueDate); } - - public function testAnnotations() - { - $annotationReader = new AnnotationReader(); - $reflectionProperty = new \ReflectionProperty(TaskData::class, 'task'); - $propertyAnnotations = $annotationReader->getPropertyAnnotations($reflectionProperty); - $this->assertCount(1, $propertyAnnotations); - $this->assertContainsOnlyInstancesOf(NotBlank::class, $propertyAnnotations); - - $reflectionProperty = new \ReflectionProperty(TaskData::class, 'dueDate'); - $propertyAnnotations = $annotationReader->getPropertyAnnotations($reflectionProperty); - $this->assertCount(0, $propertyAnnotations); - } } diff --git a/tests/fixtures/MakeDtoValidatorYamlXml/tests/GeneratedDtoTest.php b/tests/fixtures/MakeDtoValidatorYamlXml/tests/TaskDataTest.php similarity index 97% rename from tests/fixtures/MakeDtoValidatorYamlXml/tests/GeneratedDtoTest.php rename to tests/fixtures/MakeDtoValidatorYamlXml/tests/TaskDataTest.php index d2bd4c084..102ee1a71 100644 --- a/tests/fixtures/MakeDtoValidatorYamlXml/tests/GeneratedDtoTest.php +++ b/tests/fixtures/MakeDtoValidatorYamlXml/tests/TaskDataTest.php @@ -18,7 +18,7 @@ use Symfony\Component\Validator\Constraints\Type; use Symfony\Component\Validator\Validation; -class GeneratedDtoTest extends KernelTestCase +class TaskDataTest extends KernelTestCase { public function testGeneratedDto() { diff --git a/tests/fixtures/MakeDtoWithoutValidations/tests/GeneratedDtoTest.php b/tests/fixtures/MakeDtoWithoutValidations/tests/GeneratedDtoTest.php deleted file mode 100644 index 27b1a2e5e..000000000 --- a/tests/fixtures/MakeDtoWithoutValidations/tests/GeneratedDtoTest.php +++ /dev/null @@ -1,25 +0,0 @@ - - * - * For the full copyright and license information, please view the LICENSE - * file that was distributed with this source code. - */ - -namespace App\Tests; - -use App\Dto\TaskData; -use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; - -class GeneratedDtoTest extends KernelTestCase -{ - public function testGeneratedDto() - { - $this->assertClassHasAttribute('task', TaskData::class); - $this->assertClassHasAttribute('dueDate', TaskData::class); - $this->assertClassNotHasAttribute('id', TaskData::class); - } -} From 42a62640572f6e41bba68fa1b71d175b08adbd9a Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Fri, 3 Jul 2020 00:45:34 +0200 Subject: [PATCH 59/60] feat: create named constructor fromEntity --- src/Maker/MakeDto.php | 4 +- src/Util/DTOClassSourceManipulator.php | 38 +++++++++++++++++++ .../MakeDtoImmutable/tests/TaskDataTest.php | 13 +++++++ 3 files changed, 53 insertions(+), 2 deletions(-) diff --git a/src/Maker/MakeDto.php b/src/Maker/MakeDto.php index 014905665..cf9b4a3d8 100644 --- a/src/Maker/MakeDto.php +++ b/src/Maker/MakeDto.php @@ -171,6 +171,7 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen $this->createProperties($dtoManipulator, $entityDetails, $fields); if ($this->shouldCreateConstructor($input)) { + $dtoManipulator->createNamedConstructor($entityDetails, $dataClassNameDetails->getShortName()); $dtoManipulator->createConstructor(); } @@ -186,7 +187,6 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen $this->createEntityMutator( $entityManipulator, - $entityDetails, $dataClassNameDetails, $fields, $this->shouldGenerateGetters($input) @@ -343,7 +343,7 @@ private function createProperties($dtoManipulator, $entityDetails, $fields) } } - private function createEntityMutator(ClassSourceManipulator $entityManipulator, ClassNameDetails $entityDetails, ClassNameDetails $dataClassNameDetails, array $fields, bool $dtoHasGetters) + private function createEntityMutator(ClassSourceManipulator $entityManipulator, ClassNameDetails $dataClassNameDetails, array $fields, bool $dtoHasGetters) { $dataClassUseName = $entityManipulator->addUseStatementIfNecessary($dataClassNameDetails->getFullName()); diff --git a/src/Util/DTOClassSourceManipulator.php b/src/Util/DTOClassSourceManipulator.php index e70cd42f9..dd42b7526 100644 --- a/src/Util/DTOClassSourceManipulator.php +++ b/src/Util/DTOClassSourceManipulator.php @@ -479,6 +479,44 @@ public function createConstructor() $this->addConstructor($params, $methodBody); } + public function createNamedConstructor(ClassNameDetails $entityDetails, $dtoName) + { + $pendingConstructorParams = $this->getPendingConstructorParams(); + if (!\count($pendingConstructorParams)) { + return; + } + + // sort to put optional params to the end. + uasort($pendingConstructorParams, [$this, 'sortConstructorParams']); + + $entityName = $entityDetails->getShortName(); + $this->addUseStatementIfNecessary($entityDetails->getFullName()); + + $methodBody = ' $paramOptions) { + $methodBody .= '$'.lcfirst($entityName).'->get'.Str::asCamelCase($paramOptions['name']).'()'; + $methodBody .= (end($keys) === $key) ? PHP_EOL : ','.PHP_EOL; + } + $methodBody .= ');'.PHP_EOL; + + $methodBuilder = $this->createMethodBuilder('from'.ucfirst($entityName), 'self', false); + $methodBuilder->makeStatic(); + $params = [ + (new Param(lcfirst($entityName)))->setType($entityName)->getNode(), + ]; + + $this->addMethodParams($methodBuilder, $params); + + $this->addMethodBody($methodBuilder, $methodBody); + + $this->addNodeAfterProperties($methodBuilder->getNode()); + $this->updateSourceCodeFromNewStmts(); + } + private function sortConstructorParams(array $paramA, array $paramB): int { if ($paramA['nullable'] || $paramB['nullable']) { diff --git a/tests/fixtures/MakeDtoImmutable/tests/TaskDataTest.php b/tests/fixtures/MakeDtoImmutable/tests/TaskDataTest.php index f5142636f..1af467db3 100644 --- a/tests/fixtures/MakeDtoImmutable/tests/TaskDataTest.php +++ b/tests/fixtures/MakeDtoImmutable/tests/TaskDataTest.php @@ -38,4 +38,17 @@ public function testConstructor() $this->assertEquals($taskData->getTask(), 'Acme'); $this->assertEquals($taskData->getDueDate(), new \DateTime('2018-01-29 01:30')); } + + public function testNamedConstructor() + { + $this->assertTrue(method_exists(TaskData::class, 'fromTask')); + + $taskEntity = new Task; + $taskEntity->setTask('Acme'); + $taskEntity->setDueDate(new \DateTime('2018-01-29 01:30')); + $taskData = TaskData::fromTask($taskEntity); + + $this->assertEquals($taskData->getTask(), 'Acme'); + $this->assertEquals($taskData->getDueDate(), new \DateTime('2018-01-29 01:30')); + } } From 249ea832ed93160b80c04891880b3d2fe80b1af1 Mon Sep 17 00:00:00 2001 From: Clemens Krack Date: Fri, 3 Jul 2020 01:10:52 +0200 Subject: [PATCH 60/60] chore: fix whitespaces --- src/Util/DTOClassSourceManipulator.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Util/DTOClassSourceManipulator.php b/src/Util/DTOClassSourceManipulator.php index dd42b7526..8e86a2a5e 100644 --- a/src/Util/DTOClassSourceManipulator.php +++ b/src/Util/DTOClassSourceManipulator.php @@ -501,7 +501,7 @@ public function createNamedConstructor(ClassNameDetails $entityDetails, $dtoName $methodBody .= '$'.lcfirst($entityName).'->get'.Str::asCamelCase($paramOptions['name']).'()'; $methodBody .= (end($keys) === $key) ? PHP_EOL : ','.PHP_EOL; } - $methodBody .= ');'.PHP_EOL; + $methodBody .= ');'.PHP_EOL; $methodBuilder = $this->createMethodBuilder('from'.ucfirst($entityName), 'self', false); $methodBuilder->makeStatic();