Skip to content

fix: silence PDOStatement::setFetchMode signature notice on PHP 8 - #74

Open
rtm-ctrlz wants to merge 1 commit into
vimeo:masterfrom
rtm-ctrlz:fix/php8-pdoStatement-setFetchMode-return-type
Open

rtm-ctrlz wants to merge 1 commit into
vimeo:masterfrom
rtm-ctrlz:fix/php8-pdoStatement-setFetchMode-return-type

Conversation

@rtm-ctrlz

Copy link
Copy Markdown
Contributor

PHP's stubs declare PDOStatement::setFetchMode(): true, so the bool override in Vimeo\MysqlEngine\Php8\FakePdoStatement is reported as an incompatible signature by IDEs and static analysis.

The return type stays bool — the true return type would require PHP 8.2, while Php8\FakePdoStatement is loaded for every PHP 8.x (FakePdo::getFakePdo() only checks PHP_MAJOR_VERSION === 8) and composer.json still allows ^8.0. Instead the method is marked with #[\ReturnTypeWillChange], as execute() and fetchObject() in the same class already are, and the phpdoc now states @return true (universalSetFetchMode() only ever returns true).

Also adds a regression test: setFetchMode() had no coverage at all.

vendor/bin/phpunit: 176 tests, 263 assertions, OK (1 pre-existing skip in SelectParseTest::testBracketedFirstSelect).

🤖 Generated with Claude Code

PHP's stubs declare `PDOStatement::setFetchMode(): true`, so the `bool`
override was reported as an incompatible signature. Keep `bool` to stay
compatible with PHP 8.0/8.1 (the `true` return type needs PHP 8.2) and
mark the method with `#[\ReturnTypeWillChange]`, as the other overrides
in this class already do. `universalSetFetchMode()` only ever returns
`true`, which the phpdoc of both methods now states.

Adds a regression test for `setFetchMode()`, which had no coverage.
@rtm-ctrlz
rtm-ctrlz force-pushed the fix/php8-pdoStatement-setFetchMode-return-type branch from 95b4fe5 to 00cd8e5 Compare September 29, 2026 17:31

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant