v3: PHP 8.1 - PHP 8.5#3442
Conversation
Source fixes for PHP 8.5: - Replace method_exists(x, __toString) guards with x instanceof \Stringable in Http/Response.php and Http/Uri.php (5 sites). On PHP 8+, method_exists() throws TypeError when passed null or an array, so the old guard no longer protected the very call sites it was written for. - Add explicit ?Type to implicitly-nullable parameters (PHP 8.4 deprecation): App::subRequest, MiddlewareAwareTrait::seedMiddlewareStack, Router::__construct, Response::__construct, RouteGroup::__invoke, DeferredCallable::__construct, Request::getParams. - Collection: add return types to ArrayAccess/Countable/IteratorAggregate methods; keep offsetGet mixed via #[\ReturnTypeWillChange]. - Cast potentially-null scalars before strtoupper/ltrim/preg_replace_callback/ explode in App.php, Http/Request.php, Http/Uri.php. - Container::get passes 0 (not null) as Exception $code. Test suite migrated from PHPUnit 4 to PHPUnit 10.5: - PHPUnit_Framework_TestCase -> PHPUnit\Framework\TestCase across all tests. - setUp/tearDown/setUpBeforeClass/tearDownAfterClass get : void return types. - setExpectedException() and @ExpectedException / @expectedExceptionMessage docblocks converted to expectException() / expectExceptionMessage() calls. - assertInternalType(type, x) -> assertIs{Type}(x). - setMethods() -> onlyMethods() where the method exists, addMethods() (PHPUnit 10 MockBuilder idiom) for StdClass stubs of nonexistent methods. - Legacy getMock(Class, [methods]) rewritten as getMockBuilder() ->onlyMethods()->getMock(). - assertAttributeEquals/Contains/Same removed in PHPUnit 9 — rewritten to assert against public API where available (getPattern, getMethods, getStatusCode, getBody, getUserInfo, etc.) and inline ReflectionProperty where private state is the only observable thing. - Prophecy removed from PHPUnit 10 — two prophesize() tests rewritten to use PHPUnit MockBuilder. - Renamed assertions: assertRegExp -> assertMatchesRegularExpression, assertNotRegExp -> assertDoesNotMatchRegularExpression, assertFileNotExists -> assertFileDoesNotExist, assertContains(string, string) -> assertStringContainsString. - Data provider methods made static (PHPUnit 10 requirement). - Removed no-op ReflectionProperty/Method::setAccessible(true) calls (deprecated in 8.5, no effect since 8.1). phpunit.xml.dist modernized to PHPUnit 10 schema: dropped removed attributes (backupStaticAttributes, convertErrorsToExceptions, etc.), added <exclude>tests/Mocks</exclude> so helper classes with Test suffix don't get picked up as test cases, replaced <filter>/<whitelist> with <source>/<include>. composer.json bumps: php ^8.1, psr/http-message ^1.1, psr/container ^1.1, phpunit/phpunit ^10.5. Notes on tests still skipped: testPhpError5, testPhpErrorDisplayDetails5, testNotFoundContentType5 (PhpErrorTest), and testHandlePhpError (AppTest) use a skipIfPhp70() helper that marks the test skipped when PHP >= 7.0. With the PHP 8.1 floor these are dead code on every supported runtime; left in place here to keep the diff scoped to the migration, not behavioral cleanup. Suite result on PHP 8.5.3: 625 tests, 1025 assertions, 14 skipped, no errors, no failures, no deprecations, no warnings.
- @phpstan-consistent-constructor on Collection, Uri, UploadedFile, Request - Move two inner named functions (handle, testCallable) to namespace scope - Group closure uses $app instead of $this rebinding in one test - Drop unused $res from three closure use-lists - Wrap intentional undefined-function call via call_user_func - phpcbf auto-fixes from the migration sweep - Require phpstan/phpstan ^2.1, add paths to phpstan.neon.dist
- CI: drop Travis, matrix PHP 8.1-8.5, bump action versions,
coverage on 8.5 leg
- composer scripts: add test:coverage and analyse (stan)
- Tests: remove dead tests, improve code coverage
|
P.S. Looks like there are branch protection rules preventing the updated workflow to kick in. There's a PR with these changes here, where tests can be seen in action: maksimovic#1 |
… are always converted to exceptions)
|
Thanks for you work on this. When I run the tests locally on PHP 8.5.5 after doing a I see that |
|
My local setup is nearly identical to yours, but all good: I'll try finding out why things are blowing up on your end. |
|
wait... that output shows PHPUnit picking up your local |
|
I've also renamed those "Test" classes that aren't really tests, and that were excluded in the .dist file so PHPUnit doesn't pick them up. Those annoyed me so I just excluded them while working on other things, but I forgot to get back to that little bit later. |
Aha! I had forgotten that I had a custom Removing it and it's all good. |
|
I have pushed a README change. I think it's good to go. |
akrabat
left a comment
There was a problem hiding this comment.
LGTM 👍
Thank you for this work @maksimovic.
Hello!
This is my first contribution to Slim, and it goes to (maybe surprisingly) v3. The reason for that is that the company I work for is relying on v3 a lot in some critical parts, where, at the moment, is a much more safe path to ensure v3 is still doing fine on PHP 8.5 than to swap v3 for v4. This change also cuts ties to anything below PHP 8.1, which is also something that only we need, which may not be something that you consider as an adequate move. For example, making sure it runs on PHP 8.5 bumps the requirement for PHPUnit from the very old v4 to v10 (which is fine for PHP 8.1-8.5).
A brief summary of changes:
method_exists($x, '__toString')to$x instanceof \Stringablefixes theTypeErrorwhen non-objects are passed on PHP 8+?Typeon implicitly-nullable parameters ($x = nulldeprecated since PHP 8.4)Container::getpassingnullas$codeto exception also deprecated since 8.1@phpstan-consistent-constructordocblocks silences phpstan 2.xnew.staticwarnings without runtime change.None of the above are BC breaks. Tests also got a little revamp to get them aligned with PHPUnit 10, and line coverage is at 99.26% (too hard or impossible to make it 100%). Little bit of updates to GitHub actions, too.
I can re-publish this from the fork, but I first wanted to check if it's something that would get accepted in the official repository as a new minor version. I'm open to discussing the proposed changes and/or elaborating on decisions that have been made.