Open
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
Author
|
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 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.