Php8 1 fixes #497
Php8 1 fixes #497
Conversation
Added #[\ReturnTypeWillChange] or appropriate return values Typed parameters where appropriate
composer.json
Outdated
| @@ -17,12 +17,12 @@ | |||
| } | |||
| ], | |||
| "require": { | |||
| "php": "^7.1|^8", | |||
| "php": ">=7.1 <8.2", | |||
There was a problem hiding this comment.
Not a fan of this, because it is already tagged in previous releases like this.
There was a problem hiding this comment.
OK, I will remove. I just like to be more strict. What happens is you can silently break someone's system by allowing them to upgrade their PHP version, but the package has incompatibilities with the newer PHP version. This is what happened to me. I did not notice DebugBar was throwing warnings since I don't normally run it. But then I did, and it was not working, leading to this PR.
I think it is better for someone to know the package has not been vetted on a newer version of PHP when Composer will not upgrade them. Since it is a preexisting problem, this would just fix it going forward. So what would happen after someone upgrades to 8.2, Composer will downgrade DebugBar to the version before this. So maybe at some point once we are sure it is 8.1 compatible, we can make this change. Then when they upgrade to 8.2, they will get a message from Composer. At that point, the package can be updated if there are 8.2 incompatibilities.
| "psr/log": "^1|^2|^3", | ||
| "symfony/var-dumper": "^2.6|^3|^4|^5|^6" | ||
| }, | ||
| "require-dev": { | ||
| "phpunit/phpunit": "^7.5.20 || ^9.4.2", | ||
| "phpunit/phpunit": ">=7.5.20 <10.0", |
There was a problem hiding this comment.
Why is this required? We don't need phpunit 8, do we?
There was a problem hiding this comment.
We don't need PHPUnit 8, but PHPUnit 7 is officially no longer supported, so this puts us on a supported version for older PHP versions.
There was a problem hiding this comment.
I see unit tests are failing for old PHP versions. Will fix. I was unable to test older versions locally due to my machine setup. Will try to correct that. Also will clean up temp directory creation.
…cal backported version Avoids Fatal error: Declaration of DebugBar\DataFormatter\VarDumper\SeekingData::seek($key) must be compatible with Symfony\Component\VarDumper\Cloner\Data::seek(string|int $key): ?static
|
Since this package only supports PHP 7.1 and higher (yeah, good call to no longer support 7.0 and lower), I removed the old symfony/var-dumper versions that support the older PHP versions. Since we no longer need to support the older versions with a backport, I removed the locally copied class DebugBar\DataFormatter\VarDumper\SeekingData, as we should be able use the official version, as this is where the old class was copied from. |
|
Where are we with this PR? Do you plan to support 8.1? |
|
Any updates on this? Why are we not merging? |
PHP 8.1 has depreciated mismatched return types on method overloads. I added appropriate return types when they were compatible with 7.1, and #[\ReturnTypeWillChange] where mixed types were returned, which are not compatible with 7.1.
I also had to update the unit tests to work better under Windows and fixed more warnings from the unit tests. These fixes may actually be in another PR already.
I also typed a few parameters in the modified files where it made sense to continue improving the code base. And I updated the composer.json file to support PHPUnit 8 and removed allowing PHP 8.2 and higher from working.
This code now runs on my 8.1 server with no issues, but there may still be more 8.1 fixes needed, as my use case is probably pretty limited.