Remove null-return on preg_replace() with valid patterns #3338
Conversation
staabm
force-pushed
the
preg-repl
branch
2 times, most recently
from
20f79f5 to
e434231
Compare
|
//cc @thg2k |
staabm
force-pushed
the
preg-repl
branch
4 times, most recently
from
e6474b4 to
4472f32
Compare
|
See the original issue. I was proven wrong: a perfectly valid regexp can still fail based on the input. So maybe if the regexp looks correct we return a benevolent union with null? |
|
This pull request has been marked as ready for review. |
|
failing build in phpdoc-parser repo is related to a baselined error which no longer happens after the PR (because benevolent) |
| @@ -18,7 +18,7 @@ public function replace(string $search, string $replacement, string $subject){ | |||
| assertType('non-empty-string', str_replace($search, $replacement, $subject)); | |||
| assertType('non-empty-string', str_ireplace($search, $replacement, $subject)); | |||
|
|
|||
| assertType('non-empty-string|null', preg_replace($search, $replacement, $subject)); | |||
| assertType('(non-empty-string|null)', preg_replace($search, $replacement, $subject)); | |||
|
|
|||
There was a problem hiding this comment.
Seems a preexisting problem, but it's wrong: preg_replace('/^a(b?)/', '\\1', 'a');
There was a problem hiding this comment.
Please open a new issue
|
Do we want this? Why it would be benevolent? Do we not want the user to always check for errors? |
|
I think it was one of the requests of the initial issue. dropped benevolence now and reduced it to the bugfix only |
|
Thank you! |
Remove null-return on preg_replace() with valid patternsReturn benevolent union from preg_replace()closes phpstan/phpstan#11547
requires #3339