From d81d831dc96daef22293159dfb3d5f9971458055 Mon Sep 17 00:00:00 2001 From: Florian Levis Date: Sat, 15 Jan 2022 22:17:41 +0100 Subject: [PATCH] fix: prefix property starting with number with 'n' Signed-off-by: Florian Levis --- CHANGELOG.md | 8 ++-- replace-all-expected-fixtures.sh | 2 +- .../Generator/Model/GetterSetterGenerator.php | 47 +++++++++++++------ .../Generator/Model/PropertyGenerator.php | 2 +- src/Component/JsonSchema/Generator/Naming.php | 26 ++++++++++ .../Normalizer/DenormalizerGenerator.php | 4 +- .../Normalizer/NormalizerGenerator.php | 2 +- .../JsonSchema/Guesser/Guess/Property.php | 17 ++++++- src/Component/JsonSchema/Jane.php | 19 ++------ .../github/expected/Model/ReactionRollup.php | 20 ++++---- src/Component/OpenApiCommon/JaneOpenApi.php | 18 ++----- 11 files changed, 101 insertions(+), 64 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e63ed7ffb..05cb7c01fe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Fixed +- [JsonSchema] [OpenApi] [GH#587](https://github.com/janephp/janephp/pull/587) Prefix property starting with number with 'n' + ## [7.1.6] - 2022-01-27 - [AutoMapper] [GH#589](https://github.com/janephp/janephp/pull/589) Fix setting properties when using target to populate object @@ -559,10 +562,7 @@ See : * https://github.com/janephp/jane/releases * https://github.com/janephp/openapi/releases -[Unreleased]: https://github.com/janephp/janephp/compare/v7.1.6...HEAD -[7.1.6]: https://github.com/janephp/janephp/compare/v7.1.5...v7.1.6 -[7.1.5]: https://github.com/janephp/janephp/compare/v7.1.4...v7.1.5 -[7.1.4]: https://github.com/janephp/janephp/compare/v7.1.3...v7.1.4 +[Unreleased]: https://github.com/janephp/janephp/compare/v7.1.3...HEAD [7.1.3]: https://github.com/janephp/janephp/compare/v7.1.2...v7.1.3 [7.1.2]: https://github.com/janephp/janephp/compare/v7.1.1...v7.1.2 [7.1.1]: https://github.com/janephp/janephp/compare/v7.1.0...v7.1.1 diff --git a/replace-all-expected-fixtures.sh b/replace-all-expected-fixtures.sh index df896ab3f7..0b7cde0852 100755 --- a/replace-all-expected-fixtures.sh +++ b/replace-all-expected-fixtures.sh @@ -3,6 +3,6 @@ for D in src/Component/*/Tests/fixtures/*; do if [ -d "${D}" ]; then rm -r "${D}/expected" - cp -R "${D}/generated" "${D}/expected" + cp -R "${D}/generated" "${D}/expected" fi done diff --git a/src/Component/JsonSchema/Generator/Model/GetterSetterGenerator.php b/src/Component/JsonSchema/Generator/Model/GetterSetterGenerator.php index cc70a87aac..5e0b1d8c6a 100644 --- a/src/Component/JsonSchema/Generator/Model/GetterSetterGenerator.php +++ b/src/Component/JsonSchema/Generator/Model/GetterSetterGenerator.php @@ -28,7 +28,7 @@ protected function createGetter(Property $property, string $namespace, bool $str return new Stmt\ClassMethod( // getProperty - $this->getNaming()->getPrefixedMethodName('get', $property->getPhpName()), + $this->getNaming()->getPrefixedMethodName('get', $property->getAccessorName()), [ // public function 'type' => Stmt\Class_::MODIFIER_PUBLIC, @@ -55,12 +55,14 @@ protected function createSetter(Property $property, string $namespace, bool $str $stmts = [ // $this->property = $property; - new Stmt\Expression(new Expr\Assign( - new Expr\PropertyFetch( - new Expr\Variable('this'), - $this->getNaming()->getPropertyName($property->getPhpName()) - ), new Expr\Variable($this->getNaming()->getPropertyName($property->getPhpName())) - )), + new Stmt\Expression( + new Expr\Assign( + new Expr\PropertyFetch( + new Expr\Variable('this'), + $property->getPhpName() + ), new Expr\Variable($property->getPhpName()) + ) + ), ]; if ($fluent) { @@ -70,13 +72,17 @@ protected function createSetter(Property $property, string $namespace, bool $str return new Stmt\ClassMethod( // setProperty - $this->getNaming()->getPrefixedMethodName('set', $property->getPhpName()), + $this->getNaming()->getPrefixedMethodName('set', $property->getAccessorName()), [ // public function 'type' => Stmt\Class_::MODIFIER_PUBLIC, // ($property) 'params' => [ - new Param(new Expr\Variable($this->getNaming()->getPropertyName($property->getPhpName())), null, $setType), + new Param( + new Expr\Variable($property->getPhpName()), + null, + $setType + ), ], 'stmts' => $stmts, 'returnType' => $fluent ? 'self' : null, @@ -88,13 +94,16 @@ protected function createSetter(Property $property, string $namespace, bool $str protected function createGetterDoc(Property $property, string $namespace, bool $strict): Doc { - $description = sprintf(<<getDescription()); + , + $property->getDescription() + ); if ($property->isDeprecated()) { $description .= <<getDocType($property, $namespace, $strict)); + , + $this->getDocType($property, $namespace, $strict) + ); return new Doc($description); } protected function createSetterDoc(Property $property, string $namespace, bool $strict, bool $fluent): Doc { - $description = sprintf(<<getDescription(), $this->getDocType($property, $namespace, $strict), '$' . $property->getPhpName()); + , + $property->getDescription(), + $this->getDocType($property, $namespace, $strict), + '$' . $property->getPhpName() + ); if ($property->isDeprecated()) { $description .= <<getNaming()->getPropertyName($property->getPhpName()); + $propertyName = $property->getPhpName(); $propertyStmt = new Stmt\PropertyProperty($propertyName); if (null === $default) { diff --git a/src/Component/JsonSchema/Generator/Naming.php b/src/Component/JsonSchema/Generator/Naming.php index d6be8a69c9..484f3a313e 100644 --- a/src/Component/JsonSchema/Generator/Naming.php +++ b/src/Component/JsonSchema/Generator/Naming.php @@ -3,6 +3,8 @@ namespace Jane\Component\JsonSchema\Generator; use Jane\Component\JsonSchema\Tools\InflectorTrait; +use function strtolower; +use function substr; /** * Helper to generate name for property / class / .... @@ -34,13 +36,37 @@ class Naming public function getPropertyName(string $name): string { $name = $this->cleaning($name); + // php property can't start with a number + if (is_numeric(substr($name, 0, 1))) { + $name = 'n' . $name; + } return $name; } + /** + * @param string $name Property name to be cleaned/deduplicated + * @param array $otherPropertiesName + */ + public function getDeduplicatedName(string $name, array &$otherPropertiesName): string + { + $cleanedName = $this->cleaning($name); + + $duplicateName = strtolower($cleanedName); + if (\array_key_exists($duplicateName, $otherPropertiesName)) { + ++$otherPropertiesName[$duplicateName]; + $cleanedName .= '' . $otherPropertiesName[$duplicateName]; + } else { + $otherPropertiesName[$duplicateName] = 1; + } + + return $cleanedName; + } + public function getPrefixedMethodName(string $prefix, string $name): string { $name = $this->cleaning($name); + // since it's prefixed, it doesn't require to check if it start with a number return sprintf('%s%s', $prefix, $this->getInflector()->classify($name)); } diff --git a/src/Component/JsonSchema/Generator/Normalizer/DenormalizerGenerator.php b/src/Component/JsonSchema/Generator/Normalizer/DenormalizerGenerator.php index 74038fceda..5ecb9d1225 100644 --- a/src/Component/JsonSchema/Generator/Normalizer/DenormalizerGenerator.php +++ b/src/Component/JsonSchema/Generator/Normalizer/DenormalizerGenerator.php @@ -105,7 +105,7 @@ protected function createDenormalizeMethod(string $modelFqdn, Context $context, $fullCondition = $baseCondition; $mutatorStmt = array_merge($denormalizationStatements, [ - new Stmt\Expression(new Expr\MethodCall($objectVariable, $this->getNaming()->getPrefixedMethodName('set', $property->getPhpName()), [$outputVar])), + new Stmt\Expression(new Expr\MethodCall($objectVariable, $this->getNaming()->getPrefixedMethodName('set', $property->getAccessorName()), [$outputVar])), ], $unset ? [new Stmt\Unset_([$propertyVar])] : []); if (!$context->isStrict() || $property->isNullable()) { @@ -132,7 +132,7 @@ protected function createDenormalizeMethod(string $modelFqdn, Context $context, ); $statements[] = new Stmt\ElseIf_($invertCondition, [ - new Stmt\Expression(new Expr\MethodCall($objectVariable, $this->getNaming()->getPrefixedMethodName('set', $property->getPhpName()), [new Expr\ConstFetch(new Name('null'))])), + new Stmt\Expression(new Expr\MethodCall($objectVariable, $this->getNaming()->getPrefixedMethodName('set', $property->getAccessorName()), [new Expr\ConstFetch(new Name('null'))])), ]); } } diff --git a/src/Component/JsonSchema/Generator/Normalizer/NormalizerGenerator.php b/src/Component/JsonSchema/Generator/Normalizer/NormalizerGenerator.php index 068f4bd731..1710857ed7 100644 --- a/src/Component/JsonSchema/Generator/Normalizer/NormalizerGenerator.php +++ b/src/Component/JsonSchema/Generator/Normalizer/NormalizerGenerator.php @@ -96,7 +96,7 @@ protected function createNormalizeMethod(string $modelFqdn, Context $context, Cl /** @var Property $property */ foreach ($classGuess->getProperties() as $property) { if (!$property->isReadOnly()) { - $propertyVar = new Expr\MethodCall($objectVariable, $this->getNaming()->getPrefixedMethodName('get', $property->getPhpName())); + $propertyVar = new Expr\MethodCall($objectVariable, $this->getNaming()->getPrefixedMethodName('get', $property->getAccessorName())); list($normalizationStatements, $outputVar) = $property->getType()->createNormalizationStatement($context, $propertyVar); diff --git a/src/Component/JsonSchema/Guesser/Guess/Property.php b/src/Component/JsonSchema/Guesser/Guess/Property.php index d9afc40379..92b5523bfd 100644 --- a/src/Component/JsonSchema/Guesser/Guess/Property.php +++ b/src/Component/JsonSchema/Guesser/Guess/Property.php @@ -45,7 +45,7 @@ class Property private $default; /** - * @var string + * @var string Used for generate class properties */ private $phpName; @@ -55,6 +55,11 @@ class Property /** @var bool */ private $deprecated = false; + /** + * @var string Used for generate getter/setter name + */ + private $accessorName; + public function __construct(object $object, string $name, string $reference, bool $nullable = false, bool $required = false, Type $type = null, string $description = null, $default = null, ?bool $readOnly = null) { $this->name = $name; @@ -78,6 +83,16 @@ public function getPhpName(): string return $this->phpName; } + public function setAccessorName(string $name): void + { + $this->accessorName = $name; + } + + public function getAccessorName(): string + { + return $this->accessorName; + } + public function getObject(): object { return $this->object; diff --git a/src/Component/JsonSchema/Jane.php b/src/Component/JsonSchema/Jane.php index bd55d4f290..78271b0dc8 100644 --- a/src/Component/JsonSchema/Jane.php +++ b/src/Component/JsonSchema/Jane.php @@ -58,24 +58,13 @@ public function createContext(Registry $registry): Context foreach ($registry->getSchemas() as $schema) { foreach ($schema->getClasses() as $class) { $properties = $this->chainGuesser->guessProperties($class->getObject(), $schema->getRootName(), $class->getReference(), $registry); - $names = []; + $names = []; foreach ($properties as $property) { - $property->setPhpName($this->naming->getPropertyName($property->getName())); - - $i = 2; - $newName = $property->getPhpName(); - - while (\in_array(strtolower($newName), $names, true)) { - $newName = $property->getPhpName() . $i; - ++$i; - } - - if ($newName !== $property->getPhpName()) { - $property->setPhpName($newName); - } + $deduplicatedName = $this->naming->getDeduplicatedName($property->getName(), $names); - $names[] = strtolower($property->getPhpName()); + $property->setAccessorName($deduplicatedName); + $property->setPhpName($this->naming->getPropertyName($deduplicatedName)); $property->setType($this->chainGuesser->guessType($property->getObject(), $property->getName(), $property->getReference(), $registry)); } diff --git a/src/Component/OpenApi3/Tests/fixtures/github/expected/Model/ReactionRollup.php b/src/Component/OpenApi3/Tests/fixtures/github/expected/Model/ReactionRollup.php index 478515cfa4..021ce00d28 100644 --- a/src/Component/OpenApi3/Tests/fixtures/github/expected/Model/ReactionRollup.php +++ b/src/Component/OpenApi3/Tests/fixtures/github/expected/Model/ReactionRollup.php @@ -21,13 +21,13 @@ class ReactionRollup * * @var int */ - protected $1; + protected $n1; /** * * * @var int */ - protected $12; + protected $n12; /** * * @@ -113,18 +113,18 @@ public function setTotalCount(int $totalCount) : self */ public function get1() : int { - return $this->1; + return $this->n1; } /** * * - * @param int $1 + * @param int $n1 * * @return self */ - public function set1(int $1) : self + public function set1(int $n1) : self { - $this->1 = $1; + $this->n1 = $n1; return $this; } /** @@ -134,18 +134,18 @@ public function set1(int $1) : self */ public function get12() : int { - return $this->12; + return $this->n12; } /** * * - * @param int $12 + * @param int $n12 * * @return self */ - public function set12(int $12) : self + public function set12(int $n12) : self { - $this->12 = $12; + $this->n12 = $n12; return $this; } /** diff --git a/src/Component/OpenApiCommon/JaneOpenApi.php b/src/Component/OpenApiCommon/JaneOpenApi.php index b733027e07..1acdd2ad43 100644 --- a/src/Component/OpenApiCommon/JaneOpenApi.php +++ b/src/Component/OpenApiCommon/JaneOpenApi.php @@ -74,24 +74,14 @@ public function createContext(Registry $registry): Context foreach ($schemas as $schema) { foreach ($schema->getClasses() as $class) { $properties = $this->chainGuesser->guessProperties($class->getObject(), $schema->getRootName(), $class->getReference(), $registry); - $names = []; + $names = []; foreach ($properties as $property) { - $property->setPhpName($this->naming->getPropertyName($property->getName())); - - $i = 2; - $newName = $property->getPhpName(); + $deduplicatedName = $this->naming->getDeduplicatedName($property->getName(), $names); - while (\in_array(strtolower($newName), $names, true)) { - $newName = $property->getPhpName() . $i; - ++$i; - } - - if ($newName !== $property->getPhpName()) { - $property->setPhpName($newName); - } + $property->setAccessorName($deduplicatedName); + $property->setPhpName($this->naming->getPropertyName($deduplicatedName)); - $names[] = strtolower($property->getPhpName()); $property->setType($this->chainGuesser->guessType($property->getObject(), $property->getName(), $property->getReference(), $registry)); }