Merge pull request #3801 from BenMorel/return-null

Fix phpstan errors
doctrine · Jan 16, 2020 · e0a92b0 · e0a92b0
2 parents 489cb9e + 8b373f4
commit e0a92b0
Show file tree

Hide file tree

Showing 14 changed files with 46 additions and 41 deletions.
diff --git a/lib/Doctrine/DBAL/Cache/ArrayStatement.php b/lib/Doctrine/DBAL/Cache/ArrayStatement.php
@@ -49,7 +49,7 @@ public function __construct(array $data)
      */
     public function closeCursor() : void
     {
-        $this->data = null;
+        $this->data = [];
     }
 
     /**
@@ -65,10 +65,6 @@ public function columnCount() : int
      */
     public function rowCount() : int
     {
-        if ($this->data === null) {
-            return 0;
-        }
-
         return count($this->data);
     }
 

diff --git a/lib/Doctrine/DBAL/Connection.php b/lib/Doctrine/DBAL/Connection.php
@@ -1264,6 +1264,8 @@ public function getWrappedConnection() : DriverConnection
     {
         $this->connect();
 
+        assert($this->_conn !== null);
+
         return $this->_conn;
     }
 

diff --git a/lib/Doctrine/DBAL/Platforms/SqlitePlatform.php b/lib/Doctrine/DBAL/Platforms/SqlitePlatform.php
@@ -688,7 +688,7 @@ protected function getPreAlterTableIndexForeignKeySQL(TableDiff $diff) : array
      */
     protected function getPostAlterTableIndexForeignKeySQL(TableDiff $diff) : array
     {
-        if (! $diff->fromTable instanceof Table) {
+        if ($diff->fromTable === null) {
             throw new DBALException('Sqlite platform requires for alter table the table diff with reference to original table schema.');
         }
 
@@ -699,7 +699,7 @@ protected function getPostAlterTableIndexForeignKeySQL(TableDiff $diff) : array
             $tableName = $diff->getName($this);
         }
 
-        foreach ($this->getIndexesInAlteredTable($diff) as $index) {
+        foreach ($this->getIndexesInAlteredTable($diff, $diff->fromTable) as $index) {
             if ($index->isPrimary()) {
                 continue;
             }
@@ -823,7 +823,7 @@ public function getAlterTableSQL(TableDiff $diff) : array
         }
 
         $fromTable = $diff->fromTable;
-        if (! $fromTable instanceof Table) {
+        if ($fromTable === null) {
             throw new DBALException('Sqlite platform requires for alter table the table diff with reference to original table schema.');
         }
 
@@ -908,7 +908,7 @@ public function getAlterTableSQL(TableDiff $diff) : array
         if (! $this->onSchemaAlterTable($diff, $tableSql)) {
             $dataTable = new Table('__temp__' . $table->getName());
 
-            $newTable = new Table($table->getQuotedName($this), $columns, $this->getPrimaryIndexInAlteredTable($diff), [], $this->getForeignKeysInAlteredTable($diff), $table->getOptions());
+            $newTable = new Table($table->getQuotedName($this), $columns, $this->getPrimaryIndexInAlteredTable($diff, $fromTable), [], $this->getForeignKeysInAlteredTable($diff, $fromTable), $table->getOptions());
             $newTable->addOption('alter', true);
 
             $sql = $this->getPreAlterTableIndexForeignKeySQL($diff);
@@ -1016,11 +1016,11 @@ private function getSimpleAlterTableSQL(TableDiff $diff)
     /**
      * @return string[]
      */
-    private function getColumnNamesInAlteredTable(TableDiff $diff) : array
+    private function getColumnNamesInAlteredTable(TableDiff $diff, Table $fromTable) : array
     {
         $columns = [];
 
-        foreach ($diff->fromTable->getColumns() as $columnName => $column) {
+        foreach ($fromTable->getColumns() as $columnName => $column) {
             $columns[strtolower($columnName)] = $column->getName();
         }
 
@@ -1056,10 +1056,10 @@ private function getColumnNamesInAlteredTable(TableDiff $diff) : array
     /**
      * @return Index[]
      */
-    private function getIndexesInAlteredTable(TableDiff $diff) : array
+    private function getIndexesInAlteredTable(TableDiff $diff, Table $fromTable) : array
     {
-        $indexes     = $diff->fromTable->getIndexes();
-        $columnNames = $this->getColumnNamesInAlteredTable($diff);
+        $indexes     = $fromTable->getIndexes();
+        $columnNames = $this->getColumnNamesInAlteredTable($diff, $fromTable);
 
         foreach ($indexes as $key => $index) {
             foreach ($diff->renamedIndexes as $oldIndexName => $renamedIndex) {
@@ -1120,10 +1120,10 @@ private function getIndexesInAlteredTable(TableDiff $diff) : array
     /**
      * @return ForeignKeyConstraint[]
      */
-    private function getForeignKeysInAlteredTable(TableDiff $diff) : array
+    private function getForeignKeysInAlteredTable(TableDiff $diff, Table $fromTable) : array
     {
-        $foreignKeys = $diff->fromTable->getForeignKeys();
-        $columnNames = $this->getColumnNamesInAlteredTable($diff);
+        $foreignKeys = $fromTable->getForeignKeys();
+        $columnNames = $this->getColumnNamesInAlteredTable($diff, $fromTable);
 
         foreach ($foreignKeys as $key => $constraint) {
             $changed      = false;
@@ -1180,11 +1180,11 @@ private function getForeignKeysInAlteredTable(TableDiff $diff) : array
     /**
      * @return Index[]
      */
-    private function getPrimaryIndexInAlteredTable(TableDiff $diff) : array
+    private function getPrimaryIndexInAlteredTable(TableDiff $diff, Table $fromTable) : array
     {
         $primaryIndex = [];
 
-        foreach ($this->getIndexesInAlteredTable($diff) as $index) {
+        foreach ($this->getIndexesInAlteredTable($diff, $fromTable) as $index) {
             if (! $index->isPrimary()) {
                 continue;
             }

diff --git a/lib/Doctrine/DBAL/Portability/Connection.php b/lib/Doctrine/DBAL/Portability/Connection.php
@@ -35,7 +35,7 @@ class Connection extends \Doctrine\DBAL\Connection
     /** @var int */
     private $portability = self::PORTABILITY_NONE;
 
-    /** @var int */
+    /** @var int|null */
     private $case;
 
     /**

diff --git a/lib/Doctrine/DBAL/Portability/Statement.php b/lib/Doctrine/DBAL/Portability/Statement.php
@@ -26,7 +26,7 @@ final class Statement implements IteratorAggregate, DriverStatement
     /** @var DriverStatement|ResultStatement */
     private $stmt;
 
-    /** @var int */
+    /** @var int|null */
     private $case;
 
     /** @var int */
@@ -204,6 +204,7 @@ private function fixRow($row, bool $iterateRow, bool $fixCase)
         }
 
         if ($fixCase) {
+            assert($this->case !== null);
             $row = array_change_key_case($row, $this->case);
         }
 

diff --git a/lib/Doctrine/DBAL/Query/QueryBuilder.php b/lib/Doctrine/DBAL/Query/QueryBuilder.php
@@ -393,9 +393,9 @@ public function setMaxResults(int $maxResults) : self
      * Gets the maximum number of results the query object was set to retrieve (the "limit").
      * Returns NULL if {@link setMaxResults} was not applied to this query builder.
      *
-     * @return int The maximum number of results.
+     * @return int|null The maximum number of results.
      */
-    public function getMaxResults() : int
+    public function getMaxResults() : ?int
     {
         return $this->maxResults;
     }

diff --git a/lib/Doctrine/DBAL/Schema/AbstractSchemaManager.php b/lib/Doctrine/DBAL/Schema/AbstractSchemaManager.php
@@ -592,6 +592,8 @@ protected function getPortableNamespacesList(array $namespaces) : array
      */
     protected function _getPortableDatabaseDefinition(array $database) : string
     {
+        assert(! empty($database));
+
         return array_shift($database);
     }
 
@@ -774,6 +776,8 @@ protected function _getPortableTablesList(array $tables) : array
      */
     protected function _getPortableTableDefinition(array $table) : string
     {
+        assert(! empty($table));
+
         return array_shift($table);
     }
 

diff --git a/lib/Doctrine/DBAL/Schema/MySqlSchemaManager.php b/lib/Doctrine/DBAL/Schema/MySqlSchemaManager.php
@@ -9,7 +9,6 @@
 use Doctrine\DBAL\Types\Type;
 use const CASE_LOWER;
 use function array_change_key_case;
-use function array_shift;
 use function array_values;
 use function assert;
 use function explode;
@@ -53,14 +52,6 @@ protected function _getPortableViewDefinition(array $view) : View
         return new View($view['TABLE_NAME'], $view['VIEW_DEFINITION']);
     }
 
-    /**
-     * {@inheritdoc}
-     */
-    protected function _getPortableTableDefinition(array $table) : string
-    {
-        return array_shift($table);
-    }
-
     /**
      * {@inheritdoc}
      */

diff --git a/phpstan.neon.dist b/phpstan.neon.dist
@@ -36,7 +36,7 @@ parameters:
         - '~^Parameter #2 \$registeredAliases of static method Doctrine\\DBAL\\Query\\QueryException::nonUniqueAlias\(\) expects array<string>, array<int, int|string> given\.\z~'
 
         # PHPStan is too strict about preg_replace(): https://phpstan.org/r/993dc99f-0d43-4b51-868b-d01f982c1463
-        - '~^Method Doctrine\\DBAL\\Platforms\\AbstractPlatform::escapeStringForLike\(\) should return string but returns string|null\.\z~'
+        - '~^Method Doctrine\\DBAL\\Platforms\\AbstractPlatform::escapeStringForLike\(\) should return string but returns string\|null\.\z~'
 
         # legacy variadic-like signature
         - '~^Method Doctrine\\DBAL(\\.*)?Connection::query\(\) invoked with \d+ parameters?, 0 required\.\z~'

diff --git a/tests/Doctrine/Tests/DBAL/Functional/Schema/DefaultValueTest.php b/tests/Doctrine/Tests/DBAL/Functional/Schema/DefaultValueTest.php
@@ -41,8 +41,6 @@ protected function setUp() : void
     }
 
     /**
-     * @param mixed $expectedDefault
-     *
      * @dataProvider columnProvider
      */
     public function testEscapedDefaultValueCanBeIntrospected(string $name, ?string $expectedDefault) : void
@@ -58,8 +56,6 @@ public function testEscapedDefaultValueCanBeIntrospected(string $name, ?string $
     }
 
     /**
-     * @param mixed $expectedDefault
-     *
      * @dataProvider columnProvider
      */
     public function testEscapedDefaultValueCanBeInserted(string $name, ?string $expectedDefault) : void

diff --git a/tests/Doctrine/Tests/DBAL/Functional/Schema/OracleSchemaManagerTest.php b/tests/Doctrine/Tests/DBAL/Functional/Schema/OracleSchemaManagerTest.php
@@ -177,7 +177,10 @@ public function testListTableDetailsWithDifferentIdentifierQuotingRequirements()
         self::assertTrue($onlinePrimaryTable->hasColumn('"Id"'));
         self::assertSame('"Id"', $onlinePrimaryTable->getColumn('"Id"')->getQuotedName($platform));
         self::assertTrue($onlinePrimaryTable->hasPrimaryKey());
-        self::assertSame(['"Id"'], $onlinePrimaryTable->getPrimaryKey()->getQuotedColumns($platform));
+
+        $onlinePrimaryTablePrimaryKey = $onlinePrimaryTable->getPrimaryKey();
+        self::assertNotNull($onlinePrimaryTablePrimaryKey);
+        self::assertSame(['"Id"'], $onlinePrimaryTablePrimaryKey->getQuotedColumns($platform));
 
         self::assertTrue($onlinePrimaryTable->hasColumn('select'));
         self::assertSame('"select"', $onlinePrimaryTable->getColumn('select')->getQuotedName($platform));
@@ -211,7 +214,10 @@ public function testListTableDetailsWithDifferentIdentifierQuotingRequirements()
         self::assertTrue($onlineForeignTable->hasColumn('id'));
         self::assertSame('ID', $onlineForeignTable->getColumn('id')->getQuotedName($platform));
         self::assertTrue($onlineForeignTable->hasPrimaryKey());
-        self::assertSame(['ID'], $onlineForeignTable->getPrimaryKey()->getQuotedColumns($platform));
+
+        $onlineForeignTablePrimaryKey = $onlineForeignTable->getPrimaryKey();
+        self::assertNotNull($onlineForeignTablePrimaryKey);
+        self::assertSame(['ID'], $onlineForeignTablePrimaryKey->getQuotedColumns($platform));
 
         self::assertTrue($onlineForeignTable->hasColumn('"Fk"'));
         self::assertSame('"Fk"', $onlineForeignTable->getColumn('"Fk"')->getQuotedName($platform));

diff --git a/tests/Doctrine/Tests/DBAL/Functional/Schema/PostgreSqlSchemaManagerTest.php b/tests/Doctrine/Tests/DBAL/Functional/Schema/PostgreSqlSchemaManagerTest.php
@@ -178,11 +178,15 @@ public function testTableWithSchema() : void
 
         $nestedSchemaTable = $this->schemaManager->listTableDetails('nested.schematable');
         self::assertTrue($nestedSchemaTable->hasColumn('id'));
-        self::assertEquals(['id'], $nestedSchemaTable->getPrimaryKey()->getColumns());
+
+        $primaryKey = $nestedSchemaTable->getPrimaryKey();
+        self::assertNotNull($primaryKey);
+        self::assertEquals(['id'], $primaryKey->getColumns());
 
         $relatedFks = $nestedSchemaTable->getForeignKeys();
         self::assertCount(1, $relatedFks);
         $relatedFk = array_pop($relatedFks);
+        self::assertNotNull($relatedFk);
         self::assertEquals('nested.schemarelated', $relatedFk->getForeignTableName());
     }
 

diff --git a/tests/Doctrine/Tests/DBAL/Platforms/SqlitePlatformTest.php b/tests/Doctrine/Tests/DBAL/Platforms/SqlitePlatformTest.php
@@ -271,7 +271,7 @@ public function testGeneratesConstraintCreationSql() : void
 
     public function getGenerateForeignKeySql() : string
     {
-        return null;
+        $this->fail('Foreign key constraints are not yet supported for SQLite.');
     }
 
     public function testModifyLimitQuery() : void

diff --git a/tests/Doctrine/Tests/DBAL/Schema/ComparatorTest.php b/tests/Doctrine/Tests/DBAL/Schema/ComparatorTest.php
@@ -254,6 +254,7 @@ public function testCompareChangeColumnsMultipleNewColumnsRename() : void
         $c         = new Comparator();
         $tableDiff = $c->diffTable($tableA, $tableB);
 
+        self::assertNotNull($tableDiff);
         self::assertCount(1, $tableDiff->renamedColumns, 'we should have one rename datefield1 => new_datefield1.');
         self::assertArrayHasKey('datefield1', $tableDiff->renamedColumns, "'datefield1' should be set to be renamed to new_datefield1");
         self::assertCount(1, $tableDiff->addedColumns, "'new_datefield2' should be added");
@@ -723,6 +724,7 @@ public function testDetectRenameColumn() : void
         $c         = new Comparator();
         $tableDiff = $c->diffTable($tableA, $tableB);
 
+        self::assertNotNull($tableDiff);
         self::assertCount(0, $tableDiff->addedColumns);
         self::assertCount(0, $tableDiff->removedColumns);
         self::assertArrayHasKey('foo', $tableDiff->renamedColumns);
@@ -748,6 +750,7 @@ public function testDetectRenameColumnAmbiguous() : void
         $c         = new Comparator();
         $tableDiff = $c->diffTable($tableA, $tableB);
 
+        self::assertNotNull($tableDiff);
         self::assertCount(1, $tableDiff->addedColumns, "'baz' should be added, not created through renaming!");
         self::assertArrayHasKey('baz', $tableDiff->addedColumns, "'baz' should be added, not created through renaming!");
         self::assertCount(2, $tableDiff->removedColumns, "'foo' and 'bar' should both be dropped, an ambiguity exists which one could be renamed to 'baz'.");
@@ -773,6 +776,7 @@ public function testDetectRenameIndex() : void
         $comparator = new Comparator();
         $tableDiff  = $comparator->diffTable($table1, $table2);
 
+        self::assertNotNull($tableDiff);
         self::assertCount(0, $tableDiff->addedIndexes);
         self::assertCount(0, $tableDiff->removedIndexes);
         self::assertArrayHasKey('idx_foo', $tableDiff->renamedIndexes);
@@ -801,6 +805,7 @@ public function testDetectRenameIndexAmbiguous() : void
         $comparator = new Comparator();
         $tableDiff  = $comparator->diffTable($table1, $table2);
 
+        self::assertNotNull($tableDiff);
         self::assertCount(1, $tableDiff->addedIndexes);
         self::assertArrayHasKey('idx_baz', $tableDiff->addedIndexes);
         self::assertCount(2, $tableDiff->removedIndexes);