Merge pull request #3822 from morozov/issues/3603

Fix some whitelisted PHPStan errors

UPGRADE.md

@@ -113,6 +113,7 @@ Table columns are no longer indexed by column name. Use the `name` attribute of
 
 - Method `Doctrine\DBAL\Schema\AbstractSchemaManager::_getPortableViewDefinition()` no longer optionally returns false. It will always return a `Doctrine\DBAL\Schema\View` instance.
 - Method `Doctrine\DBAL\Schema\Comparator::diffTable()` now optionally returns null instead of false.
+- Property `Doctrine\DBAL\Schema\Table::$_primaryKeyName` is now optionally null instead of false.
 - Property `Doctrine\DBAL\Schema\TableDiff::$newName` is now optionally null instead of false.
 - Method `Doctrine\DBAL\Schema\AbstractSchemaManager::tablesExist()` no longer accepts a string. Use `Doctrine\DBAL\Schema\AbstractSchemaManager::tableExists()` instead.
 - Method `Doctrine\DBAL\Schema\OracleSchemaManager::createDatabase()` no longer accepts `null` for `$database` argument.

lib/Doctrine/DBAL/Platforms/AbstractPlatform.php

@@ -3141,11 +3141,15 @@ public function getStringLiteralQuoteCharacter() : string
      */
     final public function escapeStringForLike(string $inputString, string $escapeChar) : string
     {
-        return preg_replace(
+        $sql = preg_replace(
             '~([' . preg_quote($this->getLikeWildcardCharacters() . $escapeChar, '~') . '])~u',
             addcslashes($escapeChar, '\\') . '$1',
             $inputString
         );
+
+        assert(is_string($sql));
+
+        return $sql;
     }
 
     protected function getLikeWildcardCharacters() : string

lib/Doctrine/DBAL/Schema/Table.php

@@ -41,8 +41,8 @@ class Table extends AbstractAsset
     /** @var Index[] */
     protected $_indexes = [];
 
-    /** @var string */
-    protected $_primaryKeyName = false;
+    /** @var string|null */
+    protected $_primaryKeyName;
 
     /** @var UniqueConstraint[] */
     protected $_uniqueConstraints = [];
@@ -163,8 +163,12 @@ public function addIndex(array $columnNames, ?string $indexName = null, array $f
      */
     public function dropPrimaryKey() : void
     {
+        if ($this->_primaryKeyName === null) {
+            return;
+        }
+
         $this->dropIndex($this->_primaryKeyName);
-        $this->_primaryKeyName = false;
+        $this->_primaryKeyName = null;
     }
 
     /**
@@ -500,9 +504,11 @@ public function getColumn(string $columnName) : Column
      */
     public function getPrimaryKey() : ?Index
     {
-        return $this->hasPrimaryKey()
-            ? $this->getIndex($this->_primaryKeyName)
-            : null;
+        if ($this->_primaryKeyName !== null) {
+            return $this->getIndex($this->_primaryKeyName);
+        }
+
+        return null;
     }
 
     /**
@@ -528,7 +534,7 @@ public function getPrimaryKeyColumns() : array
      */
     public function hasPrimaryKey() : bool
     {
-        return $this->_primaryKeyName && $this->hasIndex($this->_primaryKeyName);
+        return $this->_primaryKeyName !== null && $this->hasIndex($this->_primaryKeyName);
     }
 
     /**
@@ -684,7 +690,7 @@ protected function _addIndex(Index $indexCandidate) : self
         }
 
         if ((isset($this->_indexes[$indexName]) && ! in_array($indexName, $replacedImplicitIndexes, true)) ||
-            ($this->_primaryKeyName !== false && $indexCandidate->isPrimary())
+            ($this->_primaryKeyName !== null && $indexCandidate->isPrimary())
         ) {
             throw IndexAlreadyExists::new($indexName, $this->_name);
         }

phpstan.neon.dist

@@ -10,76 +10,25 @@ parameters:
         # extension not available
         - '~^(Used )?(Function|Constant) sasql_\S+ not found\.\z~i'
 
-        # removing it would be BC break
-        - '~^Constructor of class Doctrine\\DBAL\\Schema\\Table has an unused parameter \$idGeneratorType\.\z~'
-
-        # declaring $tableName in AbstractSchemaManager::_getPortableTableIndexesList() non-optional will be a BC break
-        - '~^Parameter #2 \$table of class Doctrine\\DBAL\\Event\\SchemaIndexDefinitionEventArgs constructor expects string, string\|null given\.\z~'
-
         # changing these would be a BC break, to be done in next major
-        - "~^Casting to bool something that's already bool.~"
-        - "~^Casting to int something that's already int.~"
-        - '~^Method Doctrine\\DBAL\\Driver\\IBMDB2\\DB2Connection::exec\(\) should return int but returns bool\.\z~'
         - '~^Method Doctrine\\DBAL\\Query\\QueryBuilder::execute\(\) should return Doctrine\\DBAL\\Driver\\Statement\|int but returns Doctrine\\DBAL\\Driver\\ResultStatement\.\z~'
-        - '~^Property Doctrine\\DBAL\\Schema\\Table::\$_primaryKeyName \(string\) does not accept (default value of type )?false\.\z~'
-        - '~^Property Doctrine\\DBAL\\Schema\\Schema::\$_schemaConfig \(Doctrine\\DBAL\\Schema\\SchemaConfig\) does not accept default value of type false\.\z~'
-        - '~^Method Doctrine\\DBAL\\Schema\\ForeignKeyConstraint::onEvent\(\) should return string\|null but returns false\.\z~'
-        - '~^Method Doctrine\\DBAL\\Schema\\(Oracle|PostgreSql|SQLServer)SchemaManager::_getPortableTableDefinition\(\) should return array but returns string\.\z~'
-        - '~^Method Doctrine\\DBAL\\Platforms\\(|SQLAnywhere|Sqlite)Platform::_getTransactionIsolationLevelSQL\(\) should return string but returns int\.\z~'
-        - '~^Method Doctrine\\DBAL\\Driver\\OCI8\\OCI8Connection::lastInsertId\(\) should return string but returns (int|false)\.\z~'
 
         # https://bugs.php.net/bug.php?id=78126
         - '~^Call to an undefined method PDO::sqliteCreateFunction\(\)\.\z~'
 
         # https://github.com/phpstan/phpstan/issues/1847
-        - '~^Parameter #2 \$registeredAliases of static method Doctrine\\DBAL\\Query\\QueryException::unknownAlias\(\) expects array<string>, array<int, int|string> given\.\z~'
         - '~^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~'
-
-        # legacy variadic-like signature
-        - '~^Method Doctrine\\DBAL(\\.*)?Connection::query\(\) invoked with \d+ parameters?, 0 required\.\z~'
-
-        # some drivers actually do accept 2nd parameter...
-        - '~^Method Doctrine\\DBAL\\Platforms\\AbstractPlatform::getListTableForeignKeysSQL\(\) invoked with \d+ parameters, 1 required\.\z~'
-
         # legacy remnants from doctrine/common
         - '~^Class Doctrine\\Common\\(Collections\\Collection|Persistence\\Proxy) not found\.\z~'
         - '~^.+ on an unknown class Doctrine\\Common\\(Collections\\Collection|Persistence\\Proxy)\.\z~'
 
-        # inheritance variance inference issue
-        - '~^Method Doctrine\\DBAL\\Driver\\PDOConnection::\w+\(\) should return Doctrine\\DBAL\\Driver\\Statement but returns PDOStatement\.\z~'
-
         # may not exist when pdo_sqlsrv is not loaded but PDO is
         - '~^Access to undefined constant PDO::SQLSRV_ENCODING_BINARY\.\z~'
 
         # weird class name, represented in stubs as OCI_(Lob|Collection)
         - '~unknown class OCI-(Lob|Collection)~'
 
-        # https://github.com/doctrine/dbal/issues/3237
-        - '~^Call to an undefined method Doctrine\\DBAL\\Driver\\PDOStatement::nextRowset\(\)~'
-
-        # https://github.com/phpstan/phpstan/pull/1886
-        -
-            message: '~^Strict comparison using === between string|false and null will always evaluate to false\.~'
-            path: %currentWorkingDirectory%/lib/Doctrine/DBAL/Driver/PDOStatement.php
-
-        # impossible inference for covariance
-        - '~^Property Doctrine\\Tests\\DBAL\\Types\\\S+Test::\$type \(Doctrine\\DBAL\\Types\\\S+Type\) does not accept Doctrine\\DBAL\\Types\\Type\.\z~'
-        - '~^Property Doctrine\\Tests\\DBAL\\Tools\\Console\\RunSqlCommandTest::\$command \(Doctrine\\DBAL\\Tools\\Console\\Command\\RunSqlCommand\) does not accept Symfony\\Component\\Console\\Command\\Command\.\z~'
-
-        # https://github.com/phpstan/phpstan-phpunit/pull/28
-        -
-            message: '~Call to method expects\(\) on an unknown class \S+~'
-            path: %currentWorkingDirectory%/tests/Doctrine/Tests/DBAL/Platforms/AbstractPlatformTestCase.php
-        -
-            message: '~Call to method expects\(\) on an unknown class \S+~'
-            path: %currentWorkingDirectory%/tests/Doctrine/Tests/DBAL/ConnectionTest.php
-        -
-            message: '~Call to method expects\(\) on an unknown class \S+~'
-            path: %currentWorkingDirectory%/tests/Doctrine/Tests/DBAL/Functional/Schema/SchemaManagerFunctionalTestCase.php
-
         # https://github.com/doctrine/dbal/pull/3582/files#r290847062
         -
             message: '~Parameter #3 \$type of method Doctrine\\DBAL\\Driver\\Statement::bindValue\(\) expects int, string given\.~'

tests/Doctrine/Tests/DBAL/Tools/Console/RunSqlCommandTest.php

@@ -26,10 +26,10 @@ class RunSqlCommandTest extends TestCase
 
     protected function setUp() : void
     {
-        $application = new Application();
-        $application->add(new RunSqlCommand());
+        $this->command = new RunSqlCommand();
+
+        (new Application())->add($this->command);
 
-        $this->command       = $application->find('dbal:run-sql');
         $this->commandTester = new CommandTester($this->command);
 
         $this->connectionMock = $this->createMock(Connection::class);

tests/Doctrine/Tests/DBAL/Types/ArrayTest.php

@@ -7,7 +7,6 @@
 use Doctrine\DBAL\Platforms\AbstractPlatform;
 use Doctrine\DBAL\Types\ArrayType;
 use Doctrine\DBAL\Types\ConversionException;
-use Doctrine\DBAL\Types\Type;
 use Doctrine\Tests\DbalTestCase;
 use PHPUnit\Framework\MockObject\MockObject;
 use function serialize;
@@ -23,7 +22,7 @@ class ArrayTest extends DbalTestCase
     protected function setUp() : void
     {
         $this->platform = $this->createMock(AbstractPlatform::class);
-        $this->type     = Type::getType('array');
+        $this->type     = new ArrayType();
     }
 
     public function testArrayConvertsToDatabaseValue() : void

tests/Doctrine/Tests/DBAL/Types/BinaryTest.php

@@ -8,7 +8,6 @@
 use Doctrine\DBAL\Platforms\AbstractPlatform;
 use Doctrine\DBAL\Types\BinaryType;
 use Doctrine\DBAL\Types\ConversionException;
-use Doctrine\DBAL\Types\Type;
 use Doctrine\DBAL\Types\Types;
 use Doctrine\Tests\DbalTestCase;
 use PHPUnit\Framework\MockObject\MockObject;
@@ -32,7 +31,7 @@ class BinaryTest extends DbalTestCase
     protected function setUp() : void
     {
         $this->platform = $this->createMock(AbstractPlatform::class);
-        $this->type     = Type::getType('binary');
+        $this->type     = new BinaryType();
     }
 
     public function testReturnsBindingType() : void

tests/Doctrine/Tests/DBAL/Types/BlobTest.php

@@ -6,7 +6,6 @@
 
 use Doctrine\DBAL\Platforms\AbstractPlatform;
 use Doctrine\DBAL\Types\BlobType;
-use Doctrine\DBAL\Types\Type;
 use Doctrine\Tests\DbalTestCase;
 use PHPUnit\Framework\MockObject\MockObject;
 
@@ -24,7 +23,7 @@ class BlobTest extends DbalTestCase
     protected function setUp() : void
     {
         $this->platform = $this->createMock(AbstractPlatform::class);
-        $this->type     = Type::getType('blob');
+        $this->type     = new BlobType();
     }
 
     public function testBlobNullConvertsToPHPValue() : void

tests/Doctrine/Tests/DBAL/Types/BooleanTest.php

@@ -6,7 +6,6 @@
 
 use Doctrine\DBAL\Platforms\AbstractPlatform;
 use Doctrine\DBAL\Types\BooleanType;
-use Doctrine\DBAL\Types\Type;
 use Doctrine\Tests\DbalTestCase;
 use PHPUnit\Framework\MockObject\MockObject;
 
@@ -20,18 +19,28 @@ class BooleanTest extends DbalTestCase
 
     protected function setUp() : void
     {
-        $this->platform = $this->getMockForAbstractClass(AbstractPlatform::class);
-        $this->type     = Type::getType('boolean');
+        $this->platform = $this->createMock(AbstractPlatform::class);
+        $this->type     = new BooleanType();
     }
 
     public function testBooleanConvertsToDatabaseValue() : void
     {
-        self::assertIsInt($this->type->convertToDatabaseValue(1, $this->platform));
+        $this->platform->expects($this->once())
+            ->method('convertBooleansToDatabaseValue')
+            ->with(true)
+            ->willReturn(1);
+
+        self::assertSame(1, $this->type->convertToDatabaseValue(true, $this->platform));
     }
 
     public function testBooleanConvertsToPHPValue() : void
     {
-        self::assertIsBool($this->type->convertToPHPValue(0, $this->platform));
+        $this->platform->expects($this->once())
+            ->method('convertFromBoolean')
+            ->with(0)
+            ->willReturn(false);
+
+        self::assertFalse($this->type->convertToPHPValue(0, $this->platform));
     }
 
     public function testBooleanNullConvertsToPHPValue() : void

tests/Doctrine/Tests/DBAL/Types/DateImmutableTypeTest.php

@@ -10,7 +10,6 @@
 use Doctrine\DBAL\Platforms\AbstractPlatform;
 use Doctrine\DBAL\Types\ConversionException;
 use Doctrine\DBAL\Types\DateImmutableType;
-use Doctrine\DBAL\Types\Type;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
 use function get_class;
@@ -25,8 +24,8 @@ class DateImmutableTypeTest extends TestCase
 
     protected function setUp() : void
     {
-        $this->type     = Type::getType('date_immutable');
         $this->platform = $this->createMock(AbstractPlatform::class);
+        $this->type     = new DateImmutableType();
     }
 
     public function testFactoryCreatesCorrectType() : void

tests/Doctrine/Tests/DBAL/Types/DateIntervalTest.php

@@ -9,7 +9,6 @@
 use Doctrine\DBAL\Platforms\AbstractPlatform;
 use Doctrine\DBAL\Types\ConversionException;
 use Doctrine\DBAL\Types\DateIntervalType;
-use Doctrine\DBAL\Types\Type;
 use Doctrine\Tests\DbalTestCase;
 use PHPUnit\Framework\MockObject\MockObject;
 use stdClass;
@@ -28,9 +27,7 @@ final class DateIntervalTest extends DbalTestCase
     protected function setUp() : void
     {
         $this->platform = $this->createMock(AbstractPlatform::class);
-        $this->type     = Type::getType('dateinterval');
-
-        self::assertInstanceOf(DateIntervalType::class, $this->type);
+        $this->type     = new DateIntervalType();
     }
 
     public function testDateIntervalConvertsToDatabaseValue() : void

tests/Doctrine/Tests/DBAL/Types/DateTest.php

@@ -6,7 +6,7 @@
 
 use DateTime;
 use Doctrine\DBAL\Types\ConversionException;
-use Doctrine\DBAL\Types\Type;
+use Doctrine\DBAL\Types\DateType;
 use function date_default_timezone_set;
 
 class DateTest extends BaseDateTypeTestCase
@@ -16,7 +16,7 @@ class DateTest extends BaseDateTypeTestCase
      */
     protected function setUp() : void
     {
-        $this->type = Type::getType('date');
+        $this->type = new DateType();
 
         parent::setUp();
     }

tests/Doctrine/Tests/DBAL/Types/DateTimeImmutableTypeTest.php

@@ -10,7 +10,6 @@
 use Doctrine\DBAL\Platforms\AbstractPlatform;
 use Doctrine\DBAL\Types\ConversionException;
 use Doctrine\DBAL\Types\DateTimeImmutableType;
-use Doctrine\DBAL\Types\Type;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
 use function get_class;
@@ -25,8 +24,8 @@ class DateTimeImmutableTypeTest extends TestCase
 
     protected function setUp() : void
     {
-        $this->type     = Type::getType('datetime_immutable');
-        $this->platform = $this->getMockBuilder(AbstractPlatform::class)->getMock();
+        $this->platform = $this->createMock(AbstractPlatform::class);
+        $this->type     = new DateTimeImmutableType();
     }
 
     public function testFactoryCreatesCorrectType() : void
@@ -46,7 +45,7 @@ public function testReturnsBindingType() : void
 
     public function testConvertsDateTimeImmutableInstanceToDatabaseValue() : void
     {
-        $date = $this->getMockBuilder(DateTimeImmutable::class)->getMock();
+        $date = $this->createMock(DateTimeImmutable::class);
 
         $this->platform->expects($this->once())
             ->method('getDateTimeFormatString')

tests/Doctrine/Tests/DBAL/Types/DateTimeTest.php

@@ -6,7 +6,7 @@
 
 use DateTime;
 use Doctrine\DBAL\Types\ConversionException;
-use Doctrine\DBAL\Types\Type;
+use Doctrine\DBAL\Types\DateTimeType;
 
 class DateTimeTest extends BaseDateTypeTestCase
 {
@@ -15,7 +15,7 @@ class DateTimeTest extends BaseDateTypeTestCase
      */
     protected function setUp() : void
     {
-        $this->type = Type::getType('datetime');
+        $this->type = new DateTimeType();
 
         parent::setUp();
     }

tests/Doctrine/Tests/DBAL/Types/DateTimeTzImmutableTypeTest.php

@@ -10,7 +10,6 @@
 use Doctrine\DBAL\Platforms\AbstractPlatform;
 use Doctrine\DBAL\Types\ConversionException;
 use Doctrine\DBAL\Types\DateTimeTzImmutableType;
-use Doctrine\DBAL\Types\Type;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
 use function get_class;
@@ -25,8 +24,8 @@ class DateTimeTzImmutableTypeTest extends TestCase
 
     protected function setUp() : void
     {
-        $this->type     = Type::getType('datetimetz_immutable');
         $this->platform = $this->createMock(AbstractPlatform::class);
+        $this->type     = new DateTimeTzImmutableType();
     }
 
     public function testFactoryCreatesCorrectType() : void