Merge pull request #74 from moufmouf/phpstan

[WIP] Adding PHPStan and starting fixing PHPStan reported issues

.travis.yml

@@ -60,6 +60,7 @@ before_script:
 script:
 # Let's run the Oracle script only when the password is available (it is not available in forks unfortunately)
 - if [ "$DB" != "oracle" ] ; then ./vendor/bin/phpunit $PHPUNITFILE; else docker run -v $(pwd):/app -v $(pwd)/tests/Fixtures/oracle-startup.sql:/docker-entrypoint-initdb.d/oracle-startup.sql moufmouf/oracle-xe-php vendor/bin/phpunit $PHPUNITFILE; fi
+- composer phpstan
 after_script:
 - if [ "$COVERALLS" = "true" ] ; then ./vendor/bin/coveralls -v; fi
 - if [ "$COVERALLS" = "true" ] ; then vendor/bin/couscous travis-auto-deploy --php-version=7.1 -vvv; fi

composer.json

@@ -41,7 +41,8 @@
         "friendsofphp/php-cs-fixer": "^2.5",
         "symfony/process": "^3.3.6",
         "couscous/couscous": "^1.6.1",
-        "thecodingmachine/dbal-fluid-schema-builder": "^1.3.0"
+        "thecodingmachine/dbal-fluid-schema-builder": "^1.3.0",
+        "phpstan/phpstan": "^0.9.2"
     },
     "suggest" : {
         "ext/weakref" : "Allows efficient memory management. Useful for batches."
@@ -57,6 +58,9 @@
             "TheCodingMachine\\TDBM\\" : "tests/"
         }
     },
+    "scripts": {
+        "phpstan": "phpstan analyse src -c phpstan.neon --level=2 --no-progress -vvv"
+    },
     "extra": {
         "branch-alias": {
             "dev-master": "5.0.x-dev"

phpstan.neon

@@ -0,0 +1,6 @@
+parameters:
+    ignoreErrors:
+     - "#Instantiated class WeakRef not found.#"
+     - "#Method JsonSerializable::jsonSerialize\\(\\) invoked with 1 parameter, 0 required.#"
+#includes:
+#    - vendor/thecodingmachine/phpstan-strict-rules/phpstan-strict-rules.neon

src/AbstractTDBMObject.php

@@ -343,10 +343,10 @@ public function _removeRelationship($pivotTableName, AbstractTDBMObject $remoteB
      * Sets many to many relationships for this bean.
      * Adds new relationships and removes unused ones.
      *
-     * @param $pivotTableName
+     * @param string $pivotTableName
      * @param array $remoteBeans
      */
-    protected function setRelationships($pivotTableName, array $remoteBeans)
+    protected function setRelationships(string $pivotTableName, array $remoteBeans)
     {
         $storage = $this->retrieveRelationshipsStorage($pivotTableName);
 
@@ -368,11 +368,11 @@ protected function setRelationships($pivotTableName, array $remoteBeans)
     /**
      * Returns the list of objects linked to this bean via $pivotTableName.
      *
-     * @param $pivotTableName
+     * @param string $pivotTableName
      *
      * @return \SplObjectStorage
      */
-    private function retrieveRelationshipsStorage($pivotTableName)
+    private function retrieveRelationshipsStorage(string $pivotTableName)
     {
         $storage = $this->getRelationshipStorage($pivotTableName);
         if ($this->status === TDBMObjectStateEnum::STATE_DETACHED || $this->status === TDBMObjectStateEnum::STATE_NEW || (isset($this->loadedRelationships[$pivotTableName]) && $this->loadedRelationships[$pivotTableName])) {
@@ -399,11 +399,11 @@ private function retrieveRelationshipsStorage($pivotTableName)
     /**
      * Internal TDBM method. Returns the list of objects linked to this bean via $pivotTableName.
      *
-     * @param $pivotTableName
+     * @param string $pivotTableName
      *
      * @return AbstractTDBMObject[]
      */
-    public function _getRelationships($pivotTableName)
+    public function _getRelationships(string $pivotTableName)
     {
         return $this->relationshipStorageToArray($this->retrieveRelationshipsStorage($pivotTableName));
     }

src/AlterableResultIterator.php

@@ -68,7 +68,7 @@ public function getUnderlyingResultIterator()
     /**
      * Adds an additional object to the result set (if not already available).
      *
-     * @param $object
+     * @param object $object
      */
     public function add($object)
     {
@@ -85,7 +85,7 @@ public function add($object)
     /**
      * Removes an object from the result set.
      *
-     * @param $object
+     * @param object $object
      */
     public function remove($object)
     {

src/MapIterator.php

@@ -22,8 +22,8 @@ class MapIterator implements Iterator, \JsonSerializable
     protected $callable;
 
     /**
-     * @param $iterator Iterator|array
-     * @param $callable callable This can have two parameters
+     * @param Iterator|array $iterator
+     * @param callable $callable This can have two parameters
      *
      * @throws TDBMException
      */

src/QueryFactory/AbstractQueryFactory.php

@@ -86,13 +86,12 @@ protected function getColumnsList(string $mainTable, array $additionalTablesFetc
 
         // Now, let's deal with "order by columns"
         if ($orderBy !== null) {
+            $securedOrderBy = true;
+            $reconstructedOrderBys = [];
             if ($orderBy instanceof UncheckedOrderBy) {
                 $securedOrderBy = false;
                 $orderBy = $orderBy->getOrderBy();
                 $reconstructedOrderBy = $orderBy;
-            } else {
-                $securedOrderBy = true;
-                $reconstructedOrderBys = [];
             }
             $orderByColumns = $this->orderByAnalyzer->analyzeOrderBy($orderBy);
 

src/QueryFactory/FindObjectsFromSqlQueryFactory.php

@@ -21,6 +21,7 @@ class FindObjectsFromSqlQueryFactory extends AbstractQueryFactory
     private $filterString;
     private $cache;
     private $cachePrefix;
+    private $schemaAnalyzer;
 
     public function __construct(string $mainTable, string $from, $filterString, $orderBy, TDBMService $tdbmService, Schema $schema, OrderByAnalyzer $orderByAnalyzer, SchemaAnalyzer $schemaAnalyzer, Cache $cache, string $cachePrefix)
     {

src/ResultIterator.php

@@ -54,15 +54,13 @@ class ResultIterator implements Result, \ArrayAccess, \JsonSerializable
      */
     private $innerResultIterator;
 
-    private $databasePlatform;
-
     private $totalCount;
 
     private $mode;
 
     private $logger;
 
-    public function __construct(QueryFactory $queryFactory, array $parameters, $objectStorage, $className, TDBMService $tdbmService, MagicQuery $magicQuery, $mode, LoggerInterface $logger)
+    public function __construct(QueryFactory $queryFactory, array $parameters, $objectStorage, $className, TDBMService $tdbmService, MagicQuery $magicQuery, int $mode, LoggerInterface $logger)
     {
         if ($mode !== null && $mode !== TDBMService::MODE_CURSOR && $mode !== TDBMService::MODE_ARRAY) {
             throw new TDBMException("Unknown fetch mode: '".$mode."'");
@@ -74,7 +72,6 @@ public function __construct(QueryFactory $queryFactory, array $parameters, $obje
         $this->tdbmService = $tdbmService;
         $this->parameters = $parameters;
         $this->magicQuery = $magicQuery;
-        $this->databasePlatform = $this->tdbmService->getConnection()->getDatabasePlatform();
         $this->mode = $mode;
         $this->logger = $logger;
     }
@@ -298,9 +295,9 @@ public function withOrder($orderBy) : ResultIterator
      *
      * For instance:
      *
-     *  $resultSet = $resultSet->withParameters('label ASC, status DESC');
+     *  $resultSet = $resultSet->withParameters([ 'status' => 'on' ]);
      *
-     * @param string|UncheckedOrderBy|null $orderBy
+     * @param array $parameters
      *
      * @return ResultIterator
      */

src/StandardObjectStorage.php

@@ -43,7 +43,7 @@ class StandardObjectStorage
      *
      * @param string     $tableName
      * @param string     $id
-     * @param TDBMObject $dbRow
+     * @param DbRow      $dbRow
      */
     public function set($tableName, $id, DbRow $dbRow)
     {

src/TDBMService.php

@@ -21,6 +21,7 @@
 namespace TheCodingMachine\TDBM;
 
 use Doctrine\Common\Cache\Cache;
+use Doctrine\Common\Cache\ClearableCache;
 use Doctrine\Common\Cache\VoidCache;
 use Doctrine\DBAL\Connection;
 use Doctrine\DBAL\DBALException;
@@ -437,7 +438,7 @@ public function buildFilterFromFilterBag($filter_bag, AbstractPlatform $platform
      *
      * @return string[]
      */
-    public function getPrimaryKeyColumns($table)
+    public function getPrimaryKeyColumns(string $table): array
     {
         if (!isset($this->primaryKeysColumns[$table])) {
             $this->primaryKeysColumns[$table] = $this->tdbmSchemaAnalyzer->getSchema()->getTable($table)->getPrimaryKey()->getUnquotedColumns();
@@ -509,13 +510,13 @@ public function _addToToSaveObjectList(DbRow $myObject)
 
     /**
      * Generates all the daos and beans.
-     *
-     * @return \string[] the list of tables (key) and bean name (value)
      */
-    public function generateAllDaosAndBeans()
+    public function generateAllDaosAndBeans() : void
     {
         // Purge cache before generating anything.
-        $this->cache->deleteAll();
+        if ($this->cache instanceof ClearableCache) {
+            $this->cache->deleteAll();
+        }
 
         $tdbmDaoGenerator = new TDBMDaoGenerator($this->configuration, $this->tdbmSchemaAnalyzer);
         $tdbmDaoGenerator->generateAllDaosAndBeans();
@@ -890,12 +891,12 @@ public function getPrimaryKeysForObjectFromDbRow(DbRow $dbRow)
      * Returns an array of primary keys for the given row.
      * The primary keys are extracted from the object columns.
      *
-     * @param $table
+     * @param string $table
      * @param array $columns
      *
      * @return array
      */
-    public function _getPrimaryKeysFromObjectData($table, array $columns)
+    public function _getPrimaryKeysFromObjectData(string $table, array $columns)
     {
         $primaryKeyColumns = $this->getPrimaryKeyColumns($table);
         $values = array();
@@ -1122,14 +1123,14 @@ private function exploreChildrenTablesRelationships(SchemaAnalyzer $schemaAnalyz
      * @param array                        $parameters
      * @param string|UncheckedOrderBy|null $orderString           The ORDER BY part of the query. Columns from tables different from $mainTable must be prefixed by the table name (in the form: table.column)
      * @param array                        $additionalTablesFetch
-     * @param int                          $mode
+     * @param int|null                     $mode
      * @param string                       $className             Optional: The name of the class to instantiate. This class must extend the TDBMObject class. If none is specified, a TDBMObject instance will be returned
      *
      * @return ResultIterator An object representing an array of results
      *
      * @throws TDBMException
      */
-    public function findObjects(string $mainTable, $filter = null, array $parameters = array(), $orderString = null, array $additionalTablesFetch = array(), $mode = null, string $className = null)
+    public function findObjects(string $mainTable, $filter = null, array $parameters = array(), $orderString = null, array $additionalTablesFetch = array(), ?int $mode = null, string $className = null)
     {
         // $mainTable is not secured in MagicJoin, let's add a bit of security to avoid SQL injection.
         if (!preg_match('/^[a-zA-Z_][a-zA-Z0-9_]*$/', $mainTable)) {
@@ -1183,7 +1184,7 @@ public function findObjectsFromSql(string $mainTable, string $from, $filter = nu
     }
 
     /**
-     * @param $table
+     * @param string $table
      * @param array  $primaryKeys
      * @param array  $additionalTablesFetch
      * @param bool   $lazy                  Whether to perform lazy loading on this object or not
@@ -1303,15 +1304,15 @@ public function findObjectFromSql($mainTable, $from, $filter = null, array $para
      * @param string $mainTable
      * @param string $sql
      * @param array $parameters
-     * @param $mode
+     * @param int|null $mode
      * @param string|null $className
      * @param string $sqlCount
      *
      * @return ResultIterator
      *
      * @throws TDBMException
      */
-    public function findObjectsFromRawSql(string $mainTable, string $sql, array $parameters = array(), $mode, string $className = null, string $sqlCount = null)
+    public function findObjectsFromRawSql(string $mainTable, string $sql, array $parameters = array(), ?int $mode = null, string $className = null, string $sqlCount = null)
     {
         // $mainTable is not secured in MagicJoin, let's add a bit of security to avoid SQL injection.
         if (!preg_match('/^[a-zA-Z_][a-zA-Z0-9_]*$/', $mainTable)) {
@@ -1433,7 +1434,7 @@ public function _getForeignKeyByName(string $table, string $fkName)
     }
 
     /**
-     * @param $pivotTableName
+     * @param string $pivotTableName
      * @param AbstractTDBMObject $bean
      *
      * @return AbstractTDBMObject[]
@@ -1456,7 +1457,7 @@ public function _getRelatedBeans(string $pivotTableName, AbstractTDBMObject $bea
     }
 
     /**
-     * @param $pivotTableName
+     * @param string $pivotTableName
      * @param AbstractTDBMObject $bean The LOCAL bean
      *
      * @return ForeignKeyConstraint[] First item: the LOCAL bean, second item: the REMOTE bean

src/UncheckedOrderBy.php

@@ -25,17 +25,11 @@ class UncheckedOrderBy
      */
     private $orderBy;
 
-    /**
-     * @param $orderBy
-     */
     public function __construct(string $orderBy)
     {
         $this->orderBy = $orderBy;
     }
 
-    /**
-     * @return string
-     */
     public function getOrderBy() : string
     {
         return $this->orderBy;

src/Utils/BeanDescriptor.php

@@ -230,7 +230,7 @@ private function getPropertiesForTable(Table $table)
         }
 
         // Now, let's get the name of all properties and let's check there is no duplicate.
-        /** @var $names AbstractBeanPropertyDescriptor[] */
+        /* @var $names AbstractBeanPropertyDescriptor[] */
         $names = [];
         foreach ($beanPropertyDescriptors as $beanDescriptor) {
             $name = $beanDescriptor->getGetterName();
@@ -610,8 +610,7 @@ private function generateFindByDaoCodeForIndex(Index $index, $beanNamespace, $be
             $params[] = $element->getParamAnnotation();
             if ($element instanceof ScalarBeanPropertyDescriptor) {
                 $filterArrayCode .= '            '.var_export($element->getColumnName(), true).' => '.$element->getVariableName().",\n";
-            } else {
-                /* @var $element ObjectBeanPropertyDescriptor */
+            } elseif ($element instanceof ObjectBeanPropertyDescriptor) {
                 $foreignKey = $element->getForeignKey();
                 $columns = array_combine($foreignKey->getLocalColumns(), $foreignKey->getForeignColumns());
                 ++$count;

src/Utils/DefaultNamingStrategy.php

@@ -149,7 +149,7 @@ public function getBaseDaoClassName(string $tableName): string
      * Tries to put string to the singular form (if it is plural) and camel case form.
      * We assume the table names are in english.
      *
-     * @param $str string
+     * @param string $str
      *
      * @return string
      */
@@ -178,7 +178,7 @@ private function toSingularCamelCase(string $str): string
     /**
      * Put string to camel case form.
      *
-     * @param $str string
+     * @param string $str
      *
      * @return string
      */

src/Utils/ObjectBeanPropertyDescriptor.php

@@ -6,6 +6,7 @@
 use Doctrine\DBAL\Schema\Table;
 use Doctrine\DBAL\Schema\ForeignKeyConstraint;
 use Mouf\Database\SchemaAnalyzer\SchemaAnalyzer;
+use TheCodingMachine\TDBM\TDBMException;
 
 /**
  * This class represent a property in a bean that points to another table.
@@ -118,7 +119,7 @@ public function hasDefault()
      */
     public function assignToDefaultCode()
     {
-        throw new \TDBMException('Foreign key based properties cannot be assigned a default value.');
+        throw new TDBMException('Foreign key based properties cannot be assigned a default value.');
     }
 
     /**

src/Utils/ScalarBeanPropertyDescriptor.php

@@ -264,7 +264,7 @@ public function %s(%s%s$%s) : void
             var_export($this->table->getName(), true),
             // Setter
             $this->column->getName(),
-            $normalizedType.($isNullable ? '|null' : ''),
+            $normalizedType.(($this->column->getNotnull() || !$this->isTypeHintable()) ? '' : '|null'),
             $this->column->getName(),
             $columnSetterName,
             ($this->column->getNotnull() || !$this->isTypeHintable()) ? '' : '?',