Merge pull request #192 from homersimpsons/fix/column-name-starting-w…

…ith-digit Column starting with a digit (#184): Add test
thecodingmachine · Feb 3, 2020 · 347a348 · 347a348
2 parents 1b91945 + c2ebe74
commit 347a348
Show file tree

Hide file tree

Showing 8 changed files with 54 additions and 15 deletions.
diff --git a/composer.json b/composer.json
@@ -38,7 +38,8 @@
         "ext-PDO": "*",
         "ext-json": "*",
         "ext-hash": "*",
-        "ext-filter": "*"
+        "ext-filter": "*",
+        "ext-intl": "*"
     },
     "require-dev" : {
         "phpunit/phpunit": "^7.4.4 || ^8.0.0",

diff --git a/src/Utils/AbstractBeanPropertyDescriptor.php b/src/Utils/AbstractBeanPropertyDescriptor.php
@@ -75,6 +75,11 @@ public function getVariableName(): string
         return $this->namingStrategy->getVariableName($this);
     }
 
+    public function getSafeVariableName(): string
+    {
+        return '$' . StringUtils::getValidVariableName(ltrim($this->getVariableName(), '$'));
+    }
+
     public function getSetterName(): string
     {
         return $this->namingStrategy->getSetterName($this);
@@ -102,7 +107,7 @@ public function getConstructorAssignCode(): string
     {
         $str = '$this->%s(%s);';
 
-        return sprintf($str, $this->getSetterName(), $this->getVariableName());
+        return sprintf($str, $this->getSetterName(), $this->getSafeVariableName());
     }
 
     /**

diff --git a/src/Utils/BeanDescriptor.php b/src/Utils/BeanDescriptor.php
@@ -363,7 +363,7 @@ private function generateBeanConstructor() : MethodGenerator
         $parentConstructorArguments = [];
 
         foreach ($constructorProperties as $property) {
-            $parameter = new ParameterGenerator(ltrim($property->getVariableName(), '$'));
+            $parameter = new ParameterGenerator(ltrim($property->getSafeVariableName(), '$'));
             if ($property->isTypeHintable()) {
                 $parameter->setType($property->getPhpType());
             }
@@ -374,7 +374,7 @@ private function generateBeanConstructor() : MethodGenerator
             if ($property->getTable()->getName() === $this->table->getName()) {
                 $assigns[] = $property->getConstructorAssignCode()."\n";
             } else {
-                $parentConstructorArguments[] = $property->getVariableName();
+                $parentConstructorArguments[] = $property->getSafeVariableName();
             }
         }
 
@@ -1252,7 +1252,7 @@ private function generateFindByDaoCodeForIndex(Index $index, string $beanNamespa
         $first = true;
         /** @var AbstractBeanPropertyDescriptor $element */
         foreach ($elements as $element) {
-            $parameter = new ParameterGenerator(ltrim($element->getVariableName(), '$'));
+            $parameter = new ParameterGenerator(ltrim($element->getSafeVariableName(), '$'));
             if (!$first && !($element->isCompulsory() && $index->isUnique())) {
                 $parameterType = '?';
             //$functionParameter = '?';
@@ -1291,7 +1291,7 @@ private function generateFindByDaoCodeForIndex(Index $index, string $beanNamespa
         foreach ($elements as $element) {
             $params[] = $element->getParamAnnotation();
             if ($element instanceof ScalarBeanPropertyDescriptor) {
-                $filterArrayCode .= '            '.var_export($element->getColumnName(), true).' => '.$element->getVariableName().",\n";
+                $filterArrayCode .= '            '.var_export($element->getColumnName(), true).' => '.$element->getSafeVariableName().",\n";
             } elseif ($element instanceof ObjectBeanPropertyDescriptor) {
                 $foreignKey = $element->getForeignKey();
                 $columns = SafeFunctions::arrayCombine($foreignKey->getLocalColumns(), $foreignKey->getForeignColumns());
@@ -1302,14 +1302,14 @@ private function generateFindByDaoCodeForIndex(Index $index, string $beanNamespa
                     $targetedElement = new ScalarBeanPropertyDescriptor($foreignTable, $foreignTable->getColumn($foreignColumn), $this->namingStrategy, $this->annotationParser);
                     if ($first || $element->isCompulsory() && $index->isUnique()) {
                         // First parameter for index is not nullable
-                        $filterArrayCode .= '            '.var_export($localColumn, true).' => '.$element->getVariableName().'->'.$targetedElement->getGetterName()."(),\n";
+                        $filterArrayCode .= '            '.var_export($localColumn, true).' => '.$element->getSafeVariableName().'->'.$targetedElement->getGetterName()."(),\n";
                     } else {
                         // Other parameters for index is not nullable
-                        $filterArrayCode .= '            '.var_export($localColumn, true).' => ('.$element->getVariableName().' !== null) ? '.$element->getVariableName().'->'.$targetedElement->getGetterName()."() : null,\n";
+                        $filterArrayCode .= '            '.var_export($localColumn, true).' => ('.$element->getSafeVariableName().' !== null) ? '.$element->getSafeVariableName().'->'.$targetedElement->getGetterName()."() : null,\n";
                     }
                 }
             }
-            $commentArguments[] = substr($element->getVariableName(), 1);
+            $commentArguments[] = substr($element->getSafeVariableName(), 1);
             if ($first) {
                 $first = false;
             }

diff --git a/src/Utils/ScalarBeanPropertyDescriptor.php b/src/Utils/ScalarBeanPropertyDescriptor.php
@@ -205,6 +205,7 @@ public function getGetterSetterCode(): array
 
         $columnGetterName = $this->getGetterName();
         $columnSetterName = $this->getSetterName();
+        $variableName = ltrim($this->getSafeVariableName(), '$');
 
         // A column type can be forced if it is not nullable and not auto-incrementable (for auto-increment columns, we can get "null" as long as the bean is not saved).
         $isNullable = !$this->column->getNotnull() || $this->isAutoincrement();
@@ -220,7 +221,7 @@ public function getGetterSetterCode(): array
     throw \TheCodingMachine\TDBM\TDBMInvalidArgumentException::badType('resource', $%s, __METHOD__);
 }
 EOF;
-            $resourceTypeCheck = sprintf($resourceTypeCheck, $checkNullable, $this->column->getName(), $this->column->getName());
+            $resourceTypeCheck = sprintf($resourceTypeCheck, $checkNullable, $variableName, $variableName);
         }
 
         $types = [ $normalizedType ];
@@ -251,10 +252,10 @@ public function getGetterSetterCode(): array
 
         $setter = new MethodGenerator($columnSetterName);
         $setterDocBlock = new DocBlockGenerator(sprintf('The setter for the "%s" column.', $this->column->getName()));
-        $setterDocBlock->setTag(new ParamTag($this->column->getName(), $types))->setWordWrap(false);
+        $setterDocBlock->setTag(new ParamTag($variableName, $types))->setWordWrap(false);
         $setter->setDocBlock($setterDocBlock);
 
-        $parameter = new ParameterGenerator($this->column->getName(), $paramType);
+        $parameter = new ParameterGenerator($variableName, $paramType);
         $setter->setParameter($parameter);
         $setter->setReturnType('void');
 
@@ -263,7 +264,7 @@ public function getGetterSetterCode(): array
 $this->set(%s, $%s, %s);',
             $resourceTypeCheck,
             var_export($this->column->getName(), true),
-            $this->column->getName(),
+            $variableName,
             var_export($this->table->getName(), true)
         ));
 

diff --git a/src/Utils/StringUtils.php b/src/Utils/StringUtils.php
@@ -0,0 +1,15 @@
+<?php
+
+namespace TheCodingMachine\TDBM\Utils;
+
+class StringUtils
+{
+    public static function getValidVariableName(string $variableName): string
+    {
+        return preg_replace_callback('/^(\d+)/', static function (array $match) {
+            $f = new \NumberFormatter('en', \NumberFormatter::SPELLOUT);
+            $number = $f->format((int) $match[0]);
+            return preg_replace('/[^a-z]+/i', '_', $number);
+        }, $variableName);
+    }
+}
diff --git a/tests/TDBMAbstractServiceTest.php b/tests/TDBMAbstractServiceTest.php
@@ -285,7 +285,9 @@ private static function initSchema(Connection $connection): void
             ->column('login')->string(255)
             ->column('password')->string(255)->null()
             ->column('status')->string(10)->null()->default(null)
-            ->column('country_id')->references('country');
+            ->column('country_id')->references('country')
+            // Used to test generation for a column that starts with a digit
+            ->then()->column('3d_view')->boolean()->default(true);
 
         $db->table('rights')
             ->column('label')->string(255)->primaryKey()->comment('Non autoincrementable primary key');

diff --git a/tests/TDBMDaoGeneratorTest.php b/tests/TDBMDaoGeneratorTest.php
@@ -2161,7 +2161,7 @@ public function testLazyStopRecursionOnCompositeForeignKey(): void
         $this->assertEquals(1, $json['compositeFkTarget']['id2']);
     }
 
-    public function testMethodNameConflictsBetweenRegularAndAutoPivotProperties()
+    public function testMethodNameConflictsBetweenRegularAndAutoPivotProperties(): void
     {
         $artist = new ArtistBean('Super');
         $artist->getChildren(); // regular property

diff --git a/tests/Utils/StringUtilsTest.php b/tests/Utils/StringUtilsTest.php
@@ -0,0 +1,15 @@
+<?php
+
+namespace Test\Utils;
+
+use PHPUnit\Framework\TestCase;
+use TheCodingMachine\TDBM\Utils\StringUtils;
+
+class StringUtilsTest extends TestCase
+{
+    public function testGetValidVariableName(): void
+    {
+        $this->assertEquals('threed_view', StringUtils::getValidVariableName('3d_view'));
+        $this->assertEquals('one_thousand_four_hundred_thirty_two_var_1432_test', StringUtils::getValidVariableName('1432_var_1432_test'));
+    }
+}