ENH PHP 8.1 compatibility #53
Conversation
emteknetnz
force-pushed
the
pulls/2/php81
branch
from
633c6e8 to
f5634db
Compare
| $b = preg_replace($this->filterPattern ?? '', '', $second ?? ''); | ||
| return preg_replace($this->filterPattern ?? '', '', $first ?? '') === $b; |
There was a problem hiding this comment.
| $b = preg_replace($this->filterPattern ?? '', '', $second ?? ''); | |
| return preg_replace($this->filterPattern ?? '', '', $first ?? '') === $b; | |
| $b = preg_replace($this->filterPattern ?? '//', '', $second ?? ''); | |
| return preg_replace($this->filterPattern ?? '//', '', $first ?? '') === $b; |
Passing an empty string instead of an empty regex results in a different warning being emitted.
There was a problem hiding this comment.
Not clear what the reasoning was for assigning the $b variable like that but it doesn't really affect anything so I'm fine with it. I'm guessing it was a phpcs issue with too many characters on one line, maybe?
There was a problem hiding this comment.
As per comments on another similar PR - the warning that gets emitted for the empty string is correct and points at the issue which already exists with the null value. This is only about resolving the deprecation warning emitted by passing null.
| @@ -42,7 +42,7 @@ public function testInvalidFileExtensionValidatingMimeType() | |||
| $this->assertFalse($result, 'Load failed because file extension does not match excepted MIME type'); | |||
| $this->assertEquals('File is not a valid upload', $errors[0]); | |||
|
|
|||
| unlink($tmpFilePath); | |||
| unlink($tmpFilePath ?? ''); | |||
There was a problem hiding this comment.
This will emit a warning whether it's null or an empty string:
[Warning]: unlink(): No such file or directory
Does it also get a deprecation warning emitted when it's null?
There was a problem hiding this comment.
Confirmed that this does emit a deprecation warning as well as the error if the value passed in is null.
There was a problem hiding this comment.
For posterity: See discussion in silverstripe/silverstripe-admin#1294 about the general approach. tl;dr:
the purposes of this PR is to avoid passing null (specifically null) from built-in functions that as of php 8.1 will throw deprecation warnings if null is passed in. The approach is necessarily heavy-handed, and while there are many situations where it isn't needed, it would be prohibitively laborious to find all of those situations by hand.
Looks good to me
Issue silverstripe/silverstripe-framework#10250