[1.x] Fix running phpstan with newer PHP versions (e.g. 8.4)#4338
[1.x] Fix running phpstan with newer PHP versions (e.g. 8.4)#4338n-peugnet wants to merge 1 commit intoflarum:1.xfrom
Conversation
imorland
left a comment
There was a problem hiding this comment.
Thank you for this @n-peugnet. I've not ran this locally yet, but yes I think this will probably work out!
Can you please rebase this, and target the 1.x branch 🙏
phpstan 1.8 cannot run properly with PHP 8.4, but newer versions in the
1.x branch do, thus we change the requirement to phpstan ^1.9, to allow
version up to 2.0 (excluding).
Newer phpstan versions now detect common errors in regexes:
Ignored error #Template type T of function resolve[()]{2} is not referenced in a parameter.# has an unescaped '()' which leads to ignoring all errors. Use '\(\)' instead.
Ignored error #^Unsafe usage of new static[()]{2}.$# has an unescaped '()' which leads to ignoring all errors. Use '\(\)' instead.
So these regexes are also fixed by using non-evaluated string litterals
and escaping the parenthesis as recommended instead of using a weird
trick with the brackets.
74dee0d to
3a16c74
Compare
|
@imorland: just rebased, sorry I didn't find this |
|
Thank you for this PR @n-peugnet! I've tested the changes locally with PHP 8.4 and unfortunately discovered a compatibility issue that prevents us from merging this. The ProblemWhile PHPStan 1.9+ works perfectly with PHP 8.4, it's incompatible with the version of Larastan we're using. Specifically:
When running PHPStan 1.9+ with Larastan 1.x, we get this error: TestingI confirmed that:
Path ForwardFor Flarum 1.x, we'll need to keep PHPStan pinned to This should be revisited for Flarum 2.x when we upgrade to Laravel 10+, which will allow us to use Larastan 2.x with PHPStan 1.9+. I think the only option is to close this PR for now, but I really appreciate you taking the time to investigate and submit this fix! 🙏 |
|
I understand, then what about setting This would allow extension developers that do not use larastan to still use phpstan with PHP >= 8.4, and users of larastan to still be able to use it with PHP < 8.4. |
Changes proposed in this pull request:
phpstan 1.8 cannot run properly with PHP 8.4, but newer versions in the 1.x branch do, thus we change the requirement to phpstan ^1.9, to allow version up to 2.0 (excluding).
Newer phpstan versions now detect common errors in regexes:
So these regexes are also fixed by using non-evaluated string literals and escaping the parenthesis as recommended instead of using a weird trick with the brackets.
Reviewers should focus on:
???
Screenshot
Before:
After:

Necessity
Confirmed
composer test).