From d3c604567b20d01a2112307611ebf910d7847ede Mon Sep 17 00:00:00 2001 From: Javier Spagnoletti Date: Tue, 4 Oct 2016 11:01:53 -0300 Subject: [PATCH 1/8] Update docblock generation for nullable fields --- lib/Doctrine/ORM/Tools/EntityGenerator.php | 25 ++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/lib/Doctrine/ORM/Tools/EntityGenerator.php b/lib/Doctrine/ORM/Tools/EntityGenerator.php index e48e8535b1b..b2bf1880e7b 100644 --- a/lib/Doctrine/ORM/Tools/EntityGenerator.php +++ b/lib/Doctrine/ORM/Tools/EntityGenerator.php @@ -1168,17 +1168,18 @@ protected function generateEntityStubMethods(ClassMetadataInfo $metadata) continue; } + $nullableField = $this->getNullableField($fieldMapping); if (( ! isset($fieldMapping['id']) || ! $fieldMapping['id'] || $metadata->generatorType == ClassMetadataInfo::GENERATOR_TYPE_NONE ) && (! $metadata->isEmbeddedClass || ! $this->embeddablesImmutable) ) { - if ($code = $this->generateEntityStubMethod($metadata, 'set', $fieldMapping['fieldName'], $fieldMapping['type'])) { + if ($code = $this->generateEntityStubMethod($metadata, 'set', $fieldMapping['fieldName'], $fieldMapping['type'], $nullableField)) { $methods[] = $code; } } - if ($code = $this->generateEntityStubMethod($metadata, 'get', $fieldMapping['fieldName'], $fieldMapping['type'])) { + if ($code = $this->generateEntityStubMethod($metadata, 'get', $fieldMapping['fieldName'], $fieldMapping['type'], $nullableField)) { $methods[] = $code; } } @@ -1205,7 +1206,7 @@ protected function generateEntityStubMethods(ClassMetadataInfo $metadata) if ($code = $this->generateEntityStubMethod($metadata, 'set', $associationMapping['fieldName'], $associationMapping['targetEntity'], $nullable)) { $methods[] = $code; } - if ($code = $this->generateEntityStubMethod($metadata, 'get', $associationMapping['fieldName'], $associationMapping['targetEntity'])) { + if ($code = $this->generateEntityStubMethod($metadata, 'get', $associationMapping['fieldName'], $associationMapping['targetEntity'], $nullable)) { $methods[] = $code; } } elseif ($associationMapping['type'] & ClassMetadataInfo::TO_MANY) { @@ -1355,7 +1356,7 @@ protected function generateEntityEmbeddedProperties(ClassMetadataInfo $metadata) * * @return string */ - protected function generateEntityStubMethod(ClassMetadataInfo $metadata, $type, $fieldName, $typeHint = null, $defaultValue = null) + protected function generateEntityStubMethod(ClassMetadataInfo $metadata, $type, $fieldName, $typeHint = null, $defaultValue = null) { $methodName = $type . Inflector::classify($fieldName); $variableName = Inflector::camelize($fieldName); @@ -1384,7 +1385,7 @@ protected function generateEntityStubMethod(ClassMetadataInfo $metadata, $type, $replacements = array( '' => ucfirst($type) . ' ' . $variableName . '.', '' => $methodTypeHint, - '' => $variableType, + '' => $variableType.(null !== $defaultValue ? ('|'.$defaultValue) : ''), '' => $variableName, '' => $methodName, '' => $fieldName, @@ -1627,7 +1628,7 @@ protected function generateFieldMappingPropertyDocBlock(array $fieldMapping, Cla { $lines = array(); $lines[] = $this->spaces . '/**'; - $lines[] = $this->spaces . ' * @var ' . $this->getType($fieldMapping['type']); + $lines[] = $this->spaces . ' * @var ' . $this->getType($fieldMapping['type']) . ($this->getNullableField($fieldMapping) ? '|' . $this->getNullableField($fieldMapping) : '');; if ($this->generateAnnotations) { $lines[] = $this->spaces . ' *'; @@ -1809,6 +1810,18 @@ protected function getIdGeneratorTypeString($type) return static::$generatorStrategyMap[$type]; } + /** + * @param array $fieldMapping + * + * @return string|null + */ + protected function getNullableField(array $fieldMapping) + { + if (isset($fieldMapping['nullable']) && true === $fieldMapping['nullable']) { + return 'null'; + } + } + /** * Exports (nested) option elements. * From 087d0816012c310b5ad90f5fe04353f4824cefab Mon Sep 17 00:00:00 2001 From: Javier Spagnoletti Date: Tue, 4 Oct 2016 11:07:37 -0300 Subject: [PATCH 2/8] Fix CS --- lib/Doctrine/ORM/Tools/EntityGenerator.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Doctrine/ORM/Tools/EntityGenerator.php b/lib/Doctrine/ORM/Tools/EntityGenerator.php index b2bf1880e7b..5220fad5584 100644 --- a/lib/Doctrine/ORM/Tools/EntityGenerator.php +++ b/lib/Doctrine/ORM/Tools/EntityGenerator.php @@ -1385,11 +1385,11 @@ protected function generateEntityStubMethod(ClassMetadataInfo $metadata, $type, $replacements = array( '' => ucfirst($type) . ' ' . $variableName . '.', '' => $methodTypeHint, - '' => $variableType.(null !== $defaultValue ? ('|'.$defaultValue) : ''), + '' => $variableType . (null !== $defaultValue ? ('|' . $defaultValue) : ''), '' => $variableName, '' => $methodName, '' => $fieldName, - '' => ($defaultValue !== null ) ? (' = '.$defaultValue) : '', + '' => ($defaultValue !== null ) ? (' = ' . $defaultValue) : '', '' => $this->getClassName($metadata) ); From ad6469b64a455fe70818fe6d7943774303aa4302 Mon Sep 17 00:00:00 2001 From: Javier Spagnoletti Date: Tue, 4 Oct 2016 11:40:12 -0300 Subject: [PATCH 3/8] Update tests --- tests/Doctrine/Tests/ORM/Tools/EntityGeneratorTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Tools/EntityGeneratorTest.php b/tests/Doctrine/Tests/ORM/Tools/EntityGeneratorTest.php index 0cbae94d7a3..d243bdbe467 100644 --- a/tests/Doctrine/Tests/ORM/Tools/EntityGeneratorTest.php +++ b/tests/Doctrine/Tests/ORM/Tools/EntityGeneratorTest.php @@ -368,8 +368,8 @@ public function testMethodDocBlockShouldStartWithBackSlash() $this->assertPhpDocReturnType('boolean', new \ReflectionMethod($book, 'removeComment')); $this->assertPhpDocVarType('\Doctrine\Tests\ORM\Tools\EntityGeneratorAuthor', new \ReflectionProperty($book, 'author')); - $this->assertPhpDocReturnType('\Doctrine\Tests\ORM\Tools\EntityGeneratorAuthor', new \ReflectionMethod($book, 'getAuthor')); - $this->assertPhpDocParamType('\Doctrine\Tests\ORM\Tools\EntityGeneratorAuthor', new \ReflectionMethod($book, 'setAuthor')); + $this->assertPhpDocReturnType('\Doctrine\Tests\ORM\Tools\EntityGeneratorAuthor|null', new \ReflectionMethod($book, 'getAuthor')); + $this->assertPhpDocParamType('\Doctrine\Tests\ORM\Tools\EntityGeneratorAuthor|null', new \ReflectionMethod($book, 'setAuthor')); $expectedClassName = '\\' . $embeddedMetadata->name; $this->assertPhpDocVarType($expectedClassName, new \ReflectionProperty($book, 'isbn')); From 1dfadef22146861d19bf3459c0376ec3dc0c6a45 Mon Sep 17 00:00:00 2001 From: Javier Spagnoletti Date: Fri, 18 Nov 2016 12:35:51 -0300 Subject: [PATCH 4/8] Removed surplus semicolon at `EntityGenerator::generateFieldMappingPropertyDocBlock()` --- lib/Doctrine/ORM/Tools/EntityGenerator.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Doctrine/ORM/Tools/EntityGenerator.php b/lib/Doctrine/ORM/Tools/EntityGenerator.php index 5220fad5584..5bcfc4d8907 100644 --- a/lib/Doctrine/ORM/Tools/EntityGenerator.php +++ b/lib/Doctrine/ORM/Tools/EntityGenerator.php @@ -1628,7 +1628,7 @@ protected function generateFieldMappingPropertyDocBlock(array $fieldMapping, Cla { $lines = array(); $lines[] = $this->spaces . '/**'; - $lines[] = $this->spaces . ' * @var ' . $this->getType($fieldMapping['type']) . ($this->getNullableField($fieldMapping) ? '|' . $this->getNullableField($fieldMapping) : '');; + $lines[] = $this->spaces . ' * @var ' . $this->getType($fieldMapping['type']) . ($this->getNullableField($fieldMapping) ? '|' . $this->getNullableField($fieldMapping) : ''); if ($this->generateAnnotations) { $lines[] = $this->spaces . ' *'; From 3341781f523e17a48c913d60adbed1e88e02ea13 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sat, 26 Nov 2016 06:20:12 +0100 Subject: [PATCH 5/8] #6068 inlined code generating expression --- lib/Doctrine/ORM/Tools/EntityGenerator.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/Doctrine/ORM/Tools/EntityGenerator.php b/lib/Doctrine/ORM/Tools/EntityGenerator.php index 5bcfc4d8907..1abd5a14c51 100644 --- a/lib/Doctrine/ORM/Tools/EntityGenerator.php +++ b/lib/Doctrine/ORM/Tools/EntityGenerator.php @@ -1173,10 +1173,9 @@ protected function generateEntityStubMethods(ClassMetadataInfo $metadata) ! $fieldMapping['id'] || $metadata->generatorType == ClassMetadataInfo::GENERATOR_TYPE_NONE ) && (! $metadata->isEmbeddedClass || ! $this->embeddablesImmutable) + && $code = $this->generateEntityStubMethod($metadata, 'set', $fieldMapping['fieldName'], $fieldMapping['type'], $nullableField) ) { - if ($code = $this->generateEntityStubMethod($metadata, 'set', $fieldMapping['fieldName'], $fieldMapping['type'], $nullableField)) { - $methods[] = $code; - } + $methods[] = $code; } if ($code = $this->generateEntityStubMethod($metadata, 'get', $fieldMapping['fieldName'], $fieldMapping['type'], $nullableField)) { From 1d2baedfd5b102f2a0f01b213147ddaa78feec2f Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sat, 26 Nov 2016 06:22:25 +0100 Subject: [PATCH 6/8] #6068 simplified variable docblock codegen for nullable instance properties --- lib/Doctrine/ORM/Tools/EntityGenerator.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/Doctrine/ORM/Tools/EntityGenerator.php b/lib/Doctrine/ORM/Tools/EntityGenerator.php index 1abd5a14c51..1fe26e79e2c 100644 --- a/lib/Doctrine/ORM/Tools/EntityGenerator.php +++ b/lib/Doctrine/ORM/Tools/EntityGenerator.php @@ -1627,7 +1627,9 @@ protected function generateFieldMappingPropertyDocBlock(array $fieldMapping, Cla { $lines = array(); $lines[] = $this->spaces . '/**'; - $lines[] = $this->spaces . ' * @var ' . $this->getType($fieldMapping['type']) . ($this->getNullableField($fieldMapping) ? '|' . $this->getNullableField($fieldMapping) : ''); + $lines[] = $this->spaces . ' * @var ' + . $this->getType($fieldMapping['type']) + . ($this->getNullableField($fieldMapping) ? '|null' : ''); if ($this->generateAnnotations) { $lines[] = $this->spaces . ' *'; From a4f76bda34ed50c8743834a60b1a214d8cb752c0 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sat, 26 Nov 2016 06:26:53 +0100 Subject: [PATCH 7/8] #6068 corrected nullable field expression generator, made it private to avoid misuse --- lib/Doctrine/ORM/Tools/EntityGenerator.php | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/Doctrine/ORM/Tools/EntityGenerator.php b/lib/Doctrine/ORM/Tools/EntityGenerator.php index 1fe26e79e2c..3c0f9264f5a 100644 --- a/lib/Doctrine/ORM/Tools/EntityGenerator.php +++ b/lib/Doctrine/ORM/Tools/EntityGenerator.php @@ -1168,7 +1168,8 @@ protected function generateEntityStubMethods(ClassMetadataInfo $metadata) continue; } - $nullableField = $this->getNullableField($fieldMapping); + $nullableField = $this->nullableFieldExpression($fieldMapping); + if (( ! isset($fieldMapping['id']) || ! $fieldMapping['id'] || $metadata->generatorType == ClassMetadataInfo::GENERATOR_TYPE_NONE @@ -1629,7 +1630,7 @@ protected function generateFieldMappingPropertyDocBlock(array $fieldMapping, Cla $lines[] = $this->spaces . '/**'; $lines[] = $this->spaces . ' * @var ' . $this->getType($fieldMapping['type']) - . ($this->getNullableField($fieldMapping) ? '|null' : ''); + . ($this->nullableFieldExpression($fieldMapping) ? '|null' : ''); if ($this->generateAnnotations) { $lines[] = $this->spaces . ' *'; @@ -1816,11 +1817,13 @@ protected function getIdGeneratorTypeString($type) * * @return string|null */ - protected function getNullableField(array $fieldMapping) + private function nullableFieldExpression(array $fieldMapping) { if (isset($fieldMapping['nullable']) && true === $fieldMapping['nullable']) { return 'null'; } + + return null; } /** From f8002ca27edebaecc554a543cb1729abe5e35f20 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sat, 26 Nov 2016 06:35:23 +0100 Subject: [PATCH 8/8] #6068 hardened test logic to verify that nothing is present after the `|null` in `@var` and `@return` types --- .../Tests/ORM/Tools/EntityGeneratorTest.php | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Tools/EntityGeneratorTest.php b/tests/Doctrine/Tests/ORM/Tools/EntityGeneratorTest.php index d243bdbe467..89c073fd221 100644 --- a/tests/Doctrine/Tests/ORM/Tools/EntityGeneratorTest.php +++ b/tests/Doctrine/Tests/ORM/Tools/EntityGeneratorTest.php @@ -1080,17 +1080,25 @@ public static function someMethod(){ */ private function assertPhpDocVarType($type, \ReflectionProperty $property) { - $this->assertEquals(1, preg_match('/@var\s+([^\s]+)/',$property->getDocComment(), $matches)); + $docComment = $property->getDocComment(); + $regex = '/@var\s+([\S]+)$/m'; + + $this->assertRegExp($regex, $docComment); + $this->assertEquals(1, preg_match($regex, $docComment, $matches)); $this->assertEquals($type, $matches[1]); } /** * @param string $type - * @param \ReflectionProperty $method + * @param \ReflectionMethod $method */ private function assertPhpDocReturnType($type, \ReflectionMethod $method) { - $this->assertEquals(1, preg_match('/@return\s+([^\s]+)/', $method->getDocComment(), $matches)); + $docComment = $method->getDocComment(); + $regex = '/@return\s+([\S]+)(\s+.*)$/m'; + + $this->assertRegExp($regex, $docComment); + $this->assertEquals(1, preg_match($regex, $docComment, $matches)); $this->assertEquals($type, $matches[1]); }