From 1064bfc303ee97769f8974f1d67e4b2f35c57a47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=A9grier?= Date: Fri, 16 Mar 2018 16:01:04 +0100 Subject: [PATCH 1/3] Adding PHPStan and starting fixing PHPStan reported issues --- .travis.yml | 1 + composer.json | 6 +++++- phpstan.neon | 5 +++++ src/AlterableResultIterator.php | 4 ++-- src/QueryFactory/AbstractQueryFactory.php | 5 ++--- .../FindObjectsFromSqlQueryFactory.php | 1 + src/TDBMService.php | 21 ++++++++++--------- src/Utils/ObjectBeanPropertyDescriptor.php | 3 ++- 8 files changed, 29 insertions(+), 17 deletions(-) create mode 100644 phpstan.neon diff --git a/.travis.yml b/.travis.yml index cc1199a8..a3494b05 100644 --- a/.travis.yml +++ b/.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 diff --git a/composer.json b/composer.json index 1543bcd3..a6e85425 100644 --- a/composer.json +++ b/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" diff --git a/phpstan.neon b/phpstan.neon new file mode 100644 index 00000000..d177928c --- /dev/null +++ b/phpstan.neon @@ -0,0 +1,5 @@ +parameters: + ignoreErrors: + - "#Instantiated class WeakRef not found.#" +#includes: +# - vendor/thecodingmachine/phpstan-strict-rules/phpstan-strict-rules.neon \ No newline at end of file diff --git a/src/AlterableResultIterator.php b/src/AlterableResultIterator.php index 3dec2737..8b13e70c 100644 --- a/src/AlterableResultIterator.php +++ b/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) { diff --git a/src/QueryFactory/AbstractQueryFactory.php b/src/QueryFactory/AbstractQueryFactory.php index 587eb08c..ff614763 100644 --- a/src/QueryFactory/AbstractQueryFactory.php +++ b/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); diff --git a/src/QueryFactory/FindObjectsFromSqlQueryFactory.php b/src/QueryFactory/FindObjectsFromSqlQueryFactory.php index a65b1192..4cdaa20a 100644 --- a/src/QueryFactory/FindObjectsFromSqlQueryFactory.php +++ b/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) { diff --git a/src/TDBMService.php b/src/TDBMService.php index 0e7bdd98..40bc34cb 100644 --- a/src/TDBMService.php +++ b/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(); @@ -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,7 +1304,7 @@ public function findObjectFromSql($mainTable, $from, $filter = null, array $para * @param string $mainTable * @param string $sql * @param array $parameters - * @param $mode + * @param int $mode * @param string|null $className * @param string $sqlCount * @@ -1311,7 +1312,7 @@ public function findObjectFromSql($mainTable, $from, $filter = null, array $para * * @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 = 0, 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 diff --git a/src/Utils/ObjectBeanPropertyDescriptor.php b/src/Utils/ObjectBeanPropertyDescriptor.php index c45fc560..6c910e38 100644 --- a/src/Utils/ObjectBeanPropertyDescriptor.php +++ b/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.'); } /** From 2fe1c5ae3374f9e43220b0df27b34c706da92b69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=A9grier?= Date: Fri, 16 Mar 2018 16:56:15 +0100 Subject: [PATCH 2/3] Fixing type-hinting issues in PHPDoc comments --- phpstan.neon | 1 + src/AbstractTDBMObject.php | 12 ++++++------ src/MapIterator.php | 4 ++-- src/ResultIterator.php | 4 ++-- src/StandardObjectStorage.php | 2 +- src/TDBMService.php | 4 ++-- src/UncheckedOrderBy.php | 6 ------ src/Utils/BeanDescriptor.php | 5 ++--- src/Utils/DefaultNamingStrategy.php | 4 ++-- src/Utils/ScalarBeanPropertyDescriptor.php | 2 +- src/Utils/TDBMDaoGenerator.php | 10 +++++----- 11 files changed, 24 insertions(+), 30 deletions(-) diff --git a/phpstan.neon b/phpstan.neon index d177928c..bc3ee7bc 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -1,5 +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 \ No newline at end of file diff --git a/src/AbstractTDBMObject.php b/src/AbstractTDBMObject.php index c929f34c..66c97d26 100644 --- a/src/AbstractTDBMObject.php +++ b/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)); } diff --git a/src/MapIterator.php b/src/MapIterator.php index c47ab60e..5b6c94ab 100644 --- a/src/MapIterator.php +++ b/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 */ diff --git a/src/ResultIterator.php b/src/ResultIterator.php index cf263fd5..fd0a93a9 100644 --- a/src/ResultIterator.php +++ b/src/ResultIterator.php @@ -298,9 +298,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 */ diff --git a/src/StandardObjectStorage.php b/src/StandardObjectStorage.php index 4a0bf624..afd894bd 100644 --- a/src/StandardObjectStorage.php +++ b/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) { diff --git a/src/TDBMService.php b/src/TDBMService.php index 40bc34cb..79d6fda6 100644 --- a/src/TDBMService.php +++ b/src/TDBMService.php @@ -891,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(); diff --git a/src/UncheckedOrderBy.php b/src/UncheckedOrderBy.php index 162dd200..07cfe5aa 100644 --- a/src/UncheckedOrderBy.php +++ b/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; diff --git a/src/Utils/BeanDescriptor.php b/src/Utils/BeanDescriptor.php index 7b113104..8607df7d 100644 --- a/src/Utils/BeanDescriptor.php +++ b/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; diff --git a/src/Utils/DefaultNamingStrategy.php b/src/Utils/DefaultNamingStrategy.php index dd81e335..199796f9 100644 --- a/src/Utils/DefaultNamingStrategy.php +++ b/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 */ diff --git a/src/Utils/ScalarBeanPropertyDescriptor.php b/src/Utils/ScalarBeanPropertyDescriptor.php index d6f2e6ff..dba5e400 100644 --- a/src/Utils/ScalarBeanPropertyDescriptor.php +++ b/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()) ? '' : '?', diff --git a/src/Utils/TDBMDaoGenerator.php b/src/Utils/TDBMDaoGenerator.php index 701bb4d4..0c0e7d06 100644 --- a/src/Utils/TDBMDaoGenerator.php +++ b/src/Utils/TDBMDaoGenerator.php @@ -306,7 +306,7 @@ public function findAll() : iterable * Get $beanClassWithoutNameSpace specified by its ID (its primary key) * If the primary key does not exist, an exception is thrown. * - * @param string|int \$id + * @param $primaryKeyPhpType \$id * @param bool \$lazyLoading If set to true, the object will not be loaded right away. Instead, it will be loaded when you first try to access a method of the object. * @return $beanClassWithoutNameSpace * @throws TDBMException @@ -553,11 +553,11 @@ public function set'.$daoClassName.'('.$daoClassName.' $'.$daoInstanceName.') : * Underscores and spaces are removed and the first letter after the underscore is uppercased. * Quoting is removed if present. * - * @param $str string + * @param string $str * * @return string */ - public static function toCamelCase($str) + public static function toCamelCase($str) : string { $str = str_replace(array('`', '"', '[', ']'), '', $str); @@ -583,7 +583,7 @@ public static function toCamelCase($str) * Tries to put string to the singular form (if it is plural). * We assume the table names are in english. * - * @param $str string + * @param string $str * * @return string */ @@ -596,7 +596,7 @@ public static function toSingular($str) * Put the first letter of the string in lower case. * Very useful to transform a class name into a variable name. * - * @param $str string + * @param string $str * * @return string */ From b91858c3e86509f89552537fc3f3ce7eab7bec56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=A9grier?= Date: Sat, 17 Mar 2018 07:49:08 +0100 Subject: [PATCH 3/3] Fixing errors introduced when fixing PHPStan issues --- src/ResultIterator.php | 5 +---- src/TDBMService.php | 8 ++++---- tests/TDBMServiceTest.php | 2 +- 3 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/ResultIterator.php b/src/ResultIterator.php index fd0a93a9..a2a71395 100644 --- a/src/ResultIterator.php +++ b/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; } diff --git a/src/TDBMService.php b/src/TDBMService.php index 79d6fda6..5b3b6edc 100644 --- a/src/TDBMService.php +++ b/src/TDBMService.php @@ -1123,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)) { @@ -1304,7 +1304,7 @@ public function findObjectFromSql($mainTable, $from, $filter = null, array $para * @param string $mainTable * @param string $sql * @param array $parameters - * @param int $mode + * @param int|null $mode * @param string|null $className * @param string $sqlCount * @@ -1312,7 +1312,7 @@ public function findObjectFromSql($mainTable, $from, $filter = null, array $para * * @throws TDBMException */ - public function findObjectsFromRawSql(string $mainTable, string $sql, array $parameters = array(), int $mode = 0, 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)) { diff --git a/tests/TDBMServiceTest.php b/tests/TDBMServiceTest.php index e3736a4d..77d0a00c 100644 --- a/tests/TDBMServiceTest.php +++ b/tests/TDBMServiceTest.php @@ -581,7 +581,7 @@ public function testInvalidSetFetchMode() */ public function testCursorModeException() { - $beans = $this->tdbmService->findObjects('contact', 'contact.id = :id', ['id' => 1], null, [], 'foobaz'); + $beans = $this->tdbmService->findObjects('contact', 'contact.id = :id', ['id' => 1], null, [], 99); } /**