-
Notifications
You must be signed in to change notification settings - Fork 111
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
ENH PHP 8.1 compatibility #763
ENH PHP 8.1 compatibility #763
Conversation
@@ -166,9 +166,9 @@ protected function getRuleController($existingRule, $localeObj) | |||
*/ | |||
protected function insertRuleBefore(array $rules, $key, array $rule, $prependIfMissing = true) | |||
{ | |||
$i = array_search($key, array_keys($rules)); | |||
$i = array_search($key, array_keys($rules ?? [])); |
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.
I assume array
param type precludes null values. Is that not true in 8.1 anymore?
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.
ok, so it's more to do with changes to internal function arguments than typehints, apparently. I think in this case we can rely on the typehint. :)
In general, nothing harmful, but lots of unnecessary nullable checks. I can approve if you are ready. |
PHP 8.1 throws deprecation warnings if you pass null into non-nullable params on native functions now. A whole lot of fun. Solution was a code writer which does create lots of unnecessary nullable checks. We've done the same on framework and all other supported modules Happy to have approved, though I'd like to keep draft for now and unmerged on the chance that there's something that was missed |
@tractorcow This is ready for review now. Thanks. |
@tractorcow there's some discussion in silverstripe/silverstripe-admin#1294 about the general approach which may be useful context. |
Issue silverstripe/silverstripe-framework#10250