fix: prefix property starting with number with 'n'

Signed-off-by: Florian Levis <[email protected]>
sigwinhq · Feb 1, 2022 · d81d831 · d81d831
1 parent 793c05b
commit d81d831
Show file tree

Hide file tree

Showing 11 changed files with 101 additions and 64 deletions.
diff --git 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
@@ -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
@@ -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(<<<EOD
+        $description = sprintf(
+            <<<EOD
 /**
  * %s
  *
 
 EOD
-            , $property->getDescription());
+            ,
+            $property->getDescription()
+        );
 
         if ($property->isDeprecated()) {
             $description .= <<<EOD
@@ -104,25 +113,33 @@ protected function createGetterDoc(Property $property, string $namespace, bool $
 EOD;
         }
 
-        $description .= sprintf(<<<EOD
+        $description .= sprintf(
+            <<<EOD
  * @return %s
  */
 EOD
-            , $this->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(<<<EOD
+        $description = sprintf(
+            <<<EOD
 /**
  * %s
  *
  * @param %s %s
 
 EOD
-            , $property->getDescription(), $this->getDocType($property, $namespace, $strict), '$' . $property->getPhpName());
+            ,
+            $property->getDescription(),
+            $this->getDocType($property, $namespace, $strict),
+            '$' . $property->getPhpName()
+        );
 
         if ($property->isDeprecated()) {
             $description .= <<<EOD

diff --git a/src/Component/JsonSchema/Generator/Model/PropertyGenerator.php b/src/Component/JsonSchema/Generator/Model/PropertyGenerator.php
@@ -23,7 +23,7 @@ abstract protected function getParser(): Parser;
 
     protected function createProperty(Property $property, string $namespace, $default = null, bool $strict = true): Stmt
     {
-        $propertyName = $this->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
@@ -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<string, int> $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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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));
                 }