-
Notifications
You must be signed in to change notification settings - Fork 1
Support PHP 8.5 #154
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Support PHP 8.5 #154
Conversation
sukhwinder33445
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
e925968 to
40c7f0b
Compare
lippserd
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In addition to the requested changes, please note a recent PR comment on commit messages and descriptions, as there is room for improvement here.
Please also justify the adjustment to the PHPStan Baseline. Why is it necessary, what has been changed, and so on.
src/Behavior/Binary.php
Outdated
| */ | ||
| $column = $condition->metaData()->get('columnName'); | ||
| if (isset($this->properties[$column])) { | ||
| if (isset($column) && isset($this->properties[$column])) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using isset() on a variable that just holds a function’s return value is not idiomatic. A strict comparison with null is clearer and more appropriate because it expresses that you only care about distinguishing null from any other value, not about existence vs. non‑existence.
Furthermore, this is also a change in behavior, as $array[null] is treated as $array[‘’] in previous versions. I have no objection to this change, but it should be justified in the commit description.
This change is also not yet included in the PR description.
Since PHP 8.4 implicitly nullable parameters are deprecated.
The methods `contains()`, `attach()` and `detach()` of `SplObjectStorage` are deprecated since PHP 8.5 The functions `offsetExists()`, `offsetSet()` and `offsetUnset()` must be used instead.
Errors that were no longer reported have been removed, keeping them in the baseline could hide future errors.
Since PHP 8.5 using null as array key is deprecated. If used as an array key is converted to an empty string, which is most likely not a valid key in the properties array.
40c7f0b to
be36f28
Compare
Changes that had to be addressed
PHP 8.3 -> PHP 8.4
Migration docs: https://www.php.net/manual/en/migration84
PHP 8.4 -> PHP 8.5
Migration Docs: https://www.php.net/manual/en/migration85
In addition the
phpstan-baseline.neonhas been updated, because some of it's errors were no longer reported.resolves: #153