Conversation
BastianLedererIcinga
force-pushed
the
8.4/8.5-compatibility
branch
2 times, most recently
from
e925968 to
40c7f0b
Compare
lippserd
left a comment
lippserd
left a comment
There was a problem hiding this comment.
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.
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.
BastianLedererIcinga
force-pushed
the
8.4/8.5-compatibility
branch
from
40c7f0b to
be36f28
Compare
3 participants
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