Add ReturnTypeWillChange to PDO implementations

[derrabus]

Q	A
Type	bug
BC Break	no
Fixed issues	N/A

Summary

PHP 8.1 will introduce a process to add return types to internal methods, see https://wiki.php.net/rfc/internal_method_return_types

This library extends various PDO classes and overrides public methods that are affected by this change. PHP 8.1 triggers deprecation warnings unless we declare that a future version of this library will add these return types as well. We can do so by adding the ReturnTypeWillChange attribute to those methods.

This PR suggests to do just that because adding the actual return types would break code that extends our classes. IIRC, the affected classes have been removed in 3.0 anyway, so let's just get rid of the warnings. 🙂

[derrabus]

I would consider all PHP CodeSniffer failures to be false positives. Please advice how I should tackle them.

Edit:

• The error Type ReturnTypeWillChange is not used in this file. will probably go away if we bump slevomat/coding-standard. fixed in Bump doctrine/coding-standard #4663
• The error Useless documentation comment with @inheritDoc. is a false positive, reported as UselessInheritDocComment is confused by method attributes slevomat/coding-standard#1233

[morozov]

I hope by the time when PHP 8.1 is released we'll drop the support for DBAL 2.x. It has already sucked a lot of blood from us.

I don't believe this patch is relevant.

[derrabus]

I hope by the time when PHP 8.1 is released we'll drop the support for DBAL 2.x.

I would hope so, but I wouldn't bet on it just yet. 🙈

It has already sucked a lot of blood from us.

True!

I don't believe this patch is relevant.

I've opened the PR because the deprecations bubbled up in Symfony's CI. But there's no urgency to fix them yet. If ORM is compatible with DBAL 3 before PHP 8.1.0 is out, we're fine I guess. That's the only blocker at the moment, isn't it?

Then again, once the issues with the CS ruleset have been sorted out, this change isn't particularly heavy.

[morozov]

If ORM is compatible with DBAL 3 before PHP 8.1.0 is out, we're fine I guess. That's the only blocker at the moment, isn't it?

I believe so, although I'm not an ORM maintainer or even a user. In any case, it would make more sense to channel the energy to improving the ORM rather than maintaining the 10-years old DBAL API.

[derrabus]

I'm ignoring the broken sniff for now. This should yield us a green build.

[derrabus]

I've reverted the changes to the CodeSniffer configuration because slevomat/coding-standard#1233 has been fixed in the meantime.

[alcaeus]

LGTM

[greg0ire]

Thanks @derrabus !

[greg0ire]

@morozov I added some labels, but I'm unsure this qualifies as a bug, feel free to add/remove some of them

[morozov]

Note, this fails Psalm on PHP 8:

ERROR: UndefinedAttributeClass - lib/Doctrine/DBAL/Driver/PDOConnection.php:51:7 - Attribute class ReturnTypeWillChange does not exist (see https://psalm.dev/241)
    #[ReturnTypeWillChange]

And PHPStan:

 ------ ------------------------------------------------------ 
  Line   Doctrine/DBAL/Driver/PDOConnection.php                
 ------ ------------------------------------------------------ 
  51     Attribute class ReturnTypeWillChange does not exist.  

[derrabus]

What would be your recommended course of action? imho, we can safely ignore those errors because defining attributes with an undefined class is a safe thing to do. Alternatively, we could add symfony/polyfill-php81 to the dev dependencies to make this class known.

[morozov]

Ignoring the errors in configuration is fine.

[derrabus]

#4695