-
-
Notifications
You must be signed in to change notification settings - Fork 946
Description
I can make a PR against phpstan/phpstan-src as suggested, but I wanted to make sure I'm looking at this correctly before suggesting a fix.
I have a case where, in PHP 8 on Github Actions, some code that makes use of PDO::lastInsertId (https://www.php.net/manual/en/pdo.lastinsertid.php) is failing a PHPStan check because PHPStan believes lastInsertId may return false. The same code does not fail in PHP 7.4 or lower.
I tried to put together a basic test on phpstan.org (https://phpstan.org/r/6559b0c3-2c32-4180-8503-23ef1cc1dff5) but that test case doesn't fail in any PHP version.
The PHP docs for lastInsertId don't show that it can return false, but looking at PHP's source code for both PHP 7.4 (https://github.com/php/php-src/blob/PHP-7.4.14/ext/pdo/pdo_dbh.c#L960) and PHP 8.0 (https://github.com/php/php-src/blob/PHP-8.0.1/ext/pdo/pdo_dbh.c#L945) it does appear that lastInsertId may return false in some situations.
PHPStan's function map file has PDO::lastInsertId's signature defined as so:
'PDO::lastInsertId' => ['string', 'seqname='=>'string'],https://github.com/phpstan/phpstan-src/blob/master/resources/functionMap.php#L8448
I don't see any overrides for this method in the php 8.0 delta file.
I'm not clear why this fails for me on PHP 8.0 but not on PHP 7.4 and why it works fine on phpstan.org, I'll try to flesh out the test case so that it better matches the code being tested to see if I can reproduce.
Am I correct in assuming that PDO::lastInsertId may return false in both versions of PHP?