Merge pull request #132 from mringler/strict_types_on_models

Strict types on models and CI for TableMaps

Author: mringler

.github/workflows/ci.yml

@@ -154,17 +154,19 @@ jobs:
         strategy:
           fail-fast: false
           matrix:
-            type: ['query', 'model']
+            type: ['query', 'model', 'tableMap']
             check: ['PHPCS', 'Stan']
             include:
               - check: PHPCS
                 command: 'cs-check -- --exclude=SlevomatCodingStandard.TypeHints.DeclareStrictTypes'
               - check: Stan
                 command: 'stan -- -a autoload.php.dist'
               - type: query
-                file-pattern: '*Query.php'
+                file-pattern: 'Base/*Query.php'
               - type: model
-                file-pattern: '*[^Query].php'
+                file-pattern: 'Base/*[^Query].php'
+              - type: tableMap
+                file-pattern: 'Map/*TableMap.php'
         name: '${{ matrix.check }} on ${{ matrix.type }} code'
         steps:
             - uses: actions/checkout@v3
@@ -177,4 +179,4 @@ jobs:
             - name: Run checks on generated code
               run: |
                 bin/perpl test:prepare --vendor=sqlite --dsn='sqlite:/tmp/perpl.sq3' --exclude-database \
-                && composer ${{ matrix.command }} tests/Fixtures/bookstore/build/classes/Propel/Tests/Bookstore/Base/${{ matrix.file-pattern }}
+                && composer ${{ matrix.command }} tests/Fixtures/bookstore/build/classes/Propel/Tests/Bookstore/${{ matrix.file-pattern }}

.gitignore

@@ -27,6 +27,9 @@ tests/Fixtures/namespaced/propel.yaml
 
 tests/Fixtures/schemas/propel.yaml
 
+tests/Fixtures/instance-pool/build/
+tests/Fixtures/instance-pool/propel.yaml
+
 tests/Fixtures/reverse/mysql/build/
 tests/Fixtures/reverse/mysql/propel.yaml
 

autoload.php.dist

@@ -9,7 +9,8 @@ if (file_exists($file = __DIR__.'/vendor/autoload.php')) {
         __DIR__ . '/tests',
         __DIR__ . '/tests/Fixtures/bookstore/build/classes',
         __DIR__ . '/tests/Fixtures/schemas/build/classes',
-        __DIR__ . '/tests/Fixtures/quoting/build/classes'
+        __DIR__ . '/tests/Fixtures/quoting/build/classes',
+        __DIR__ . '/tests/Fixtures/instance-pool/build/classes',
     ));
     $loader->add('Propel\Tests\Fixtures', __DIR__ . '/tests/Fixtures/classes');
     $loader->register();

phpstan-baseline.neon

@@ -1,5 +1,5 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<files psalm-version="6.13.1@1e3b7f0a8ab32b23197b91107adc0a7ed8a05b51">
+<files psalm-version="6.16.1@f1f5de594dc76faf8784e02d3dc4716c91c6f6ac">
   <file src="src/Propel/Common/Config/ConfigurationManager.php">
     <UnsupportedPropertyReferenceUsage>
       <code><![CDATA[$configSection = &$this->config[$section]]]></code>
@@ -90,6 +90,14 @@
       <code><![CDATA[$table->getColumn($columnName)]]></code>
     </NullableReturnStatement>
   </file>
+  <file src="src/Propel/Generator/Builder/Om/InstancePoolCodeProducer/InstancePoolCodeProducer.php">
+    <ParadoxicalCondition>
+      <code><![CDATA[$col->isLobType(),
+            $col->isPhpObjectType() => "is_callable([$varName, '__toString']) ? (string)$varName : $varName"]]></code>
+      <code><![CDATA[$col->isNumericType(),
+            $col->isPhpPrimitiveNumericType() => "(string)$varName"]]></code>
+    </ParadoxicalCondition>
+  </file>
   <file src="src/Propel/Generator/Builder/Util/SchemaReader.php">
     <UnsupportedPropertyReferenceUsage>
       <code><![CDATA[$items = &$this->currParameterListCollector['value']]]></code>

psalm-baseline.xml

@@ -73,13 +73,21 @@ protected function addPathsSection(ArrayNodeDefinition $node): void
                 ->arrayNode('paths')
                     ->addDefaultsIfNotSet()
                     ->children()
+                        // optional variable
                         ->scalarNode('projectDir')->defaultValue('.')->end()
+                        // location of schema.xml
                         ->scalarNode('schemaDir')->defaultValue('.')->end()
+                        // default value for commands with --output-dir parameter
                         ->scalarNode('outputDir')->defaultValue('.')->end()
+                        // generated classes base (root of namespace)
                         ->scalarNode('phpDir')->defaultValue('./generated-classes')->end()
+                        // generated config files (config:convert, loader script from model:build)
                         ->scalarNode('phpConfDir')->defaultValue('./generated-conf')->end()
+                        // table loader file (defaults to phpConfDir)
                         ->scalarNode('loaderScriptDir')->end()
+                        // DB initialization scripts (sql:insert & user generated)
                         ->scalarNode('sqlDir')->defaultValue('./generated-sql')->end()
+                        // target for migration files
                         ->scalarNode('migrationDir')->defaultValue('./generated-migrations')->end()
                         ->scalarNode('composerDir')->defaultNull()->end()
                     ->end()
@@ -354,6 +362,7 @@ protected function addGeneratorSection(ArrayNodeDefinition $node): void
                         ->booleanNode('packageObjectModel')->defaultTrue()->end()
                         ->booleanNode('namespaceAutoPackage')->defaultTrue()->end()
                         ->booleanNode('recursive')->defaultFalse()->end()
+                        ->booleanNode('declareStrictTypesInBuilders')->defaultFalse()->end() // optional, as it might break user-written behaviors
                         ->booleanNode('defaultToNativeEnumeratedColumnTypes')->defaultFalse()->end()
                         ->arrayNode('connections')
                             ->prototype('scalar')->end()

src/Propel/Common/Config/PropelConfiguration.php

@@ -6,8 +6,6 @@
 
 use Propel\Generator\Builder\Om\ObjectBuilder;
 use Propel\Generator\Model\Column;
-use Propel\Generator\Model\PropelTypes;
-use function in_array;
 use function preg_match;
 use function preg_replace;
 use function ucfirst;
@@ -286,19 +284,4 @@ public function objectFilter(string &$script, ObjectBuilder $builder): void
         $replacement = "\$0$addition";
         $script = preg_replace($pattern, $replacement, $script);
     }
-
-    /**
-     * @param string $columnType
-     *
-     * @return bool
-     */
-    protected function isDateType(string $columnType): bool
-    {
-        return in_array($columnType, [
-            PropelTypes::DATE,
-            PropelTypes::DATETIME,
-            PropelTypes::TIME,
-            PropelTypes::TIMESTAMP,
-        ], true);
-    }
 }

src/Propel/Generator/Behavior/I18n/I18nBehaviorObjectBuilderModifier.php

@@ -1050,17 +1050,13 @@ static public function fixLevels(" . ($useScope ? '$scope, ' : '') . "?Connectio
             $script .= "
             // class must be set each time from the record row
             \$cls = $tableMapClassName::getOMClass(\$row, 0);
-            \$cls = substr('.'.\$cls, strrpos('.'.\$cls, '.') + 1);
-            " . $this->builder->buildObjectInstanceCreationCode('$obj', '$cls') . "
-            \$obj->hydrate(\$row);
-            $tableMapClassName::addInstanceToPool(\$obj, \$key);";
-        } else {
-            $script .= "
-            " . $this->builder->buildObjectInstanceCreationCode('$obj', '$cls') . "
-            \$obj->hydrate(\$row);
-            $tableMapClassName::addInstanceToPool(\$obj, \$key);";
+            \$cls = substr('.'.\$cls, strrpos('.'.\$cls, '.') + 1);";
         }
         $script .= "
+            \$obj = new \$cls();
+            \$obj->hydrate(\$row);
+            $tableMapClassName::addInstanceToPool(\$obj, \$key);
+            
         }
 
         // compute level

src/Propel/Generator/Behavior/NestedSet/NestedSetBehaviorQueryBuilderModifier.php

@@ -52,7 +52,7 @@ protected function getTemporalColumnIndexesByFormatter(ObjectBuilder $builder):
             if (!$col->isTemporalType()) {
                 continue;
             }
-            $formatter = $builder->getTemporalFormatter($col);
+            $formatter = $builder->getPlatformOrFail()->getTemporalFormatter($col);
             $temporalColumnIndexesByFormatter[$formatter][] = $num;
         }
 

src/Propel/Generator/Behavior/OutputGroup/OgObjectModifier.php

@@ -221,6 +221,12 @@ protected function createSlug(): string
     protected function addCreateRawSlug(string &$script): void
     {
         $pattern = $this->getParameter('slug_pattern');
+        if (!$pattern) {
+            $returnStatement = '$this->cleanupSlugPart($this->__toString())';
+        } else {
+            $getter = str_replace(['{', '}'], ['\' . $this->cleanupSlugPart((string)$this->get', '()) . \''], $pattern);
+            $returnStatement = "'$getter'";
+        }
         $script .= "
 /**
  * Create the slug from the appropriate columns
@@ -229,13 +235,7 @@ protected function addCreateRawSlug(string &$script): void
  */
 protected function createRawSlug(): string
 {
-    ";
-        if ($pattern) {
-            $script .= "return '" . str_replace(['{', '}'], ['\' . $this->cleanupSlugPart((string)$this->get', '()) . \''], $pattern) . "';";
-        } else {
-            $script .= 'return $this->cleanupSlugPart($this->__toString());';
-        }
-        $script .= "
+    return $returnStatement;
 }
 ";
     }
@@ -247,6 +247,8 @@ protected function createRawSlug(): string
      */
     public function addCleanupSlugPart(string &$script): void
     {
+        $this->builder->declareGlobalFunction('setlocale', 'str_replace', 'preg_replace', 'trim', 'strtolower');
+        $this->builder->declareGlobalConstant('LC_CTYPE');
         $script .= "
 /**
  * Cleanup a string to make a slug of it
@@ -259,7 +261,7 @@ public function addCleanupSlugPart(string &$script): void
 protected static function cleanupSlugPart(string \$slug, string \$replacement = '" . $this->getParameter('replacement') . "'): string
 {
     // set locale explicitly
-    \$localeOrigin = setlocale(LC_CTYPE, 0);
+    \$localeOrigin = setlocale(LC_CTYPE, '0');
     setlocale(LC_CTYPE, 'C.UTF-8');
 
     // transliterate
@@ -268,28 +270,19 @@ protected static function cleanupSlugPart(string \$slug, string \$replacement =
     }
 
     // lowercase
-    if (function_exists('mb_strtolower')) {
-        \$slug = mb_strtolower(\$slug);
-    } else {
-        \$slug = strtolower(\$slug);
-    }
+    \$slug = function_exists('mb_strtolower') ? mb_strtolower(\$slug) : strtolower(\$slug);
 
     // remove accents resulting from OSX's iconv
     \$slug = str_replace(array('\'', '`', '^'), '', \$slug);
 
     // replace non letter or digits with separator
     \$slug = preg_replace('" . $this->getParameter('replace_pattern') . "', \$replacement, \$slug);
 
-    // trim
     \$slug = trim(\$slug, \$replacement);
 
     setlocale(LC_CTYPE, \$localeOrigin);
 
-    if (empty(\$slug)) {
-        return 'n-a';
-    }
-
-    return \$slug;
+    return empty(\$slug) ? 'n-a' : \$slug;
 }
 ";
     }