Enable PHPStan in tests by carusogabriel · Pull Request #3203 · doctrine/dbal

carusogabriel · 2018-07-01T17:49:30Z

Q	A
Type	improvement
BC Break	no
Fixed issues	-

Summary

Enable PHPStan on test at level 3.

carusogabriel · 2018-07-06T01:01:49Z

@Majkl578 @morozov There're a few errors left. Can you help me with them? 😇

morozov

@carusogabriel as for the remaining

 ------ ---------------------------------------------------------------------- 
  Line   tests/Doctrine/Tests/DBAL/Functional/Driver/PDOSqlsrv/DriverTest.php  
 ------ ---------------------------------------------------------------------- 
  72     Call to an undefined method                                           
         Doctrine\DBAL\Driver\Connection::getAttribute().                      
 ------ ----------------------------------------------------------------------

You can add a type hint like @var \PDO $connection.

morozov · 2018-07-06T04:56:47Z

tests/Doctrine/Tests/DBAL/ConnectionTest.php


    public function testConnectDispatchEvent()
    {
+        /** @var \PHPUnit_Framework_MockObject_MockObject $listenerMock */


Is there a problem detecting the type here? The code looks straightforward. AFAIK, PHPUnit will use namespaced class names if future versions, so it makes sense to use \PHPUnit\Framework\... instead.

Looks like it still PHPUnit_... https://github.com/sebastianbergmann/phpunit/blob/master/src/Framework/MockObject/MockObject.php#L21

It's also https://github.com/sebastianbergmann/phpunit/blob/0b52fd1df743e51b32becee1a4276c012c7b51d7/src/Framework/MockObject/ForwardCompatibility/MockObject.php

morozov · 2018-07-06T04:59:16Z

tests/Doctrine/Tests/DBAL/Functional/Driver/IBMDB2/DB2StatementTest.php


    public function testExecutionErrorsAreNotSuppressed()
    {
+        /** @var \Doctrine\DBAL\Statement $stmt */


This looks redundant. The @return annotation on the method declares the type.

morozov · 2018-07-06T05:00:45Z

tests/Doctrine/Tests/DBAL/Functional/Schema/SchemaManagerFunctionalTestCase.php

 {
    /**
-     * @var \Doctrine\DBAL\Schema\AbstractSchemaManager
+     * @var mixed


Is it supposed to be anything else than a schema manager?

morozov · 2018-07-06T05:01:11Z

tests/Doctrine/Tests/DBAL/Functional/Schema/SchemaManagerFunctionalTestCase.php


        $this->_sm->dropAndCreateTable($table);

+        /** @var \PHPUnit_Framework_MockObject_MockObject $listenerMock */


This shouldn't be needed.

I tried to remove, but, no success 😢

It still doesn't look right. PhpStorm unambiguously detects the $listenerMockType as PHPUnit\Framework\MockObject\MockObject based on the @return annotation of PHPUnit\Framework\MockObject\MockBuilder::getMock(). So should do PHPStan. Instead of annotating every call like this, we need to find what the underlying problem is and fix it.

morozov · 2018-07-06T05:01:21Z

tests/Doctrine/Tests/DBAL/Functional/Schema/SchemaManagerFunctionalTestCase.php


        $this->_sm->dropAndCreateTable($table);

+        /** @var \PHPUnit_Framework_MockObject_MockObject $listenerMock */


Shouldn't be needed.

morozov · 2018-07-06T05:10:02Z

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


    public function testConvertsDateTimeImmutableInstanceToDatabaseValue()
    {
+        /** @var \DateTimeImmutable|\Prophecy\Prophecy\ObjectProphecy $date */


Does phpstan/phpstan-phpunit handle prophecy mocks? Or there's a separate extension for them? In any event, it's better to have this type detection automated rather than manually entered every time.

morozov · 2018-07-06T05:10:20Z

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


    /**
-     * @var \Doctrine\DBAL\Types\DateIntervalType
+     * @var \Doctrine\DBAL\Types\Type


Please revert.

morozov · 2018-07-06T05:10:26Z

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


    /**
-     * @var DateTimeImmutableType
+     * @var Type


Please revert.

Should this parent/child issue be reported to upstream?

------ ----------------------------------------------------------------------------------------------------------------------------------------------------------- Line tests/Doctrine/Tests/DBAL/Types/DateTimeImmutableTypeTest.php ------ ----------------------------------------------------------------------------------------------------------------------------------------------------------- 26 Property Doctrine\Tests\DBAL\Types\DateTimeImmutableTypeTest::$type (Doctrine\DBAL\Types\DateTimeImmutableType) does not accept Doctrine\DBAL\Types\Type. ------ -----------------------------------------------------------------------------------------------------------------------------------------------------------

No. Please try using new DateTimeImmutableType() instead of the factory.

morozov · 2018-07-06T05:10:47Z

tests/Doctrine/Tests/DbalFunctionalTestCase.php


    /**
-     * @var \Doctrine\DBAL\Connection
+     * @var mixed


Please revert.

It breaks a lot of places:

------ ------------------------------------------------------------------------------------ Line tests/Doctrine/Tests/DBAL/Functional/Driver/IBMDB2/DB2StatementTest.php ------ ------------------------------------------------------------------------------------ 32 Call to an undefined method Doctrine\DBAL\Driver\Statement::getWrappedStatement(). ------ ------------------------------------------------------------------------------------ ------ --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- Line tests/Doctrine/Tests/DBAL/Functional/Driver/OCI8/OCI8ConnectionTest.php ------ --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- 29 Property Doctrine\Tests\DBAL\Functional\Driver\OCI8\OCI8ConnectionTest::$driverConnection (Doctrine\DBAL\Driver\OCI8\OCI8Connection) does not accept Doctrine\DBAL\Driver\Connection. ------ --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- ------ --------------------------------------------------------------------------------------------------------------------------------------------------------------------------- Line tests/Doctrine/Tests/DBAL/Functional/Driver/PDOConnectionTest.php ------ --------------------------------------------------------------------------------------------------------------------------------------------------------------------------- 27 Property Doctrine\Tests\DBAL\Functional\Driver\PDOConnectionTest::$driverConnection (Doctrine\DBAL\Driver\PDOConnection) does not accept Doctrine\DBAL\Driver\Connection. ------ --------------------------------------------------------------------------------------------------------------------------------------------------------------------------- ------ ------------------------------------------------------------------------------ Line tests/Doctrine/Tests/DBAL/Functional/Ticket/DBAL630Test.php ------ ------------------------------------------------------------------------------ 41 Call to an undefined method Doctrine\DBAL\Driver\Connection::setAttribute(). 75 Call to an undefined method Doctrine\DBAL\Driver\Connection::setAttribute(). 99 Call to an undefined method Doctrine\DBAL\Driver\Connection::setAttribute(). 123 Call to an undefined method Doctrine\DBAL\Driver\Connection::setAttribute(). ------ ------------------------------------------------------------------------------

Those breakages look valid though, looks PDO related. Explicitly mixed only disables some checks (so better to avoid mixed).

morozov · 2018-07-06T05:11:26Z

tests/Doctrine/Tests/Mocks/DriverMock.php

 {
    /**
-     * @var DatabasePlatformMock
+     * @var AbstractPlatform|DatabasePlatformMock


DatabasePlatformMock extends AbstractPlatform. The former is enough.

carusogabriel · 2018-07-09T02:44:18Z

phpcbf f*cked us in this review 🤦‍♂️

carusogabriel · 2018-07-09T16:17:30Z

@morozov Some comments that now are folded but I'd like your reply to continue to fix:

Majkl578 · 2018-08-02T13:06:07Z

@carusogabriel Hi, any progress?

carusogabriel · 2018-08-02T13:24:43Z

Closing as I won't be able to finish it. Anyone is encouraged to finish it 😊

Majkl578 · 2018-08-10T01:32:47Z

Great job.

morozov requested changes Jul 6, 2018

View reviewed changes

carusogabriel changed the title ~~[WIP] Enable PHPStan in tests~~ Enable PHPStan in tests Jul 9, 2018

Enable PHPStan in tests

82ee39f

carusogabriel closed this Aug 10, 2018

carusogabriel deleted the phpstan-tests branch August 10, 2018 00:21

Majkl578 added Improvement Incomplete Static Analysis labels Aug 10, 2018

Majkl578 mentioned this pull request Aug 18, 2018

Enable PHPStan in tests #3260

Closed

morozov assigned carusogabriel Oct 13, 2018

github-actions bot locked as resolved and limited conversation to collaborators Aug 10, 2022


		$this->_sm->dropAndCreateTable($table);

		/** @var \PHPUnit_Framework_MockObject_MockObject $listenerMock */

Uh oh!

Conversation

carusogabriel commented Jul 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

carusogabriel commented Jul 6, 2018

Uh oh!

morozov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

carusogabriel commented Jul 9, 2018

Uh oh!

carusogabriel commented Jul 9, 2018

Uh oh!

Majkl578 commented Aug 2, 2018

Uh oh!

carusogabriel commented Aug 2, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Majkl578 commented Aug 10, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

carusogabriel commented Jul 1, 2018 •

edited

Loading

carusogabriel commented Aug 2, 2018 •

edited

Loading