ref: replace str_starts_with with strpos in ClockMock#1936
Conversation
![seer-by-sentry[bot]](https://avatars.githubusercontent.com/in/801464?s=60&v=4){kind=small}
| $ns = str_replace('\\Tests\\', '\\', $class); | ||
| $mockedNs[] = substr($ns, 0, strrpos($ns, '\\')); | ||
| } elseif (str_starts_with($class, 'Tests\\')) { | ||
| } elseif (strpos($class, 'Tests\\') === 0) { |
There was a problem hiding this comment.
Potential bug: This change is an incomplete fix. Other calls to PHP 8+ functions like str_starts_with remain, which will cause fatal errors on older PHP versions.
-
Description: The change at
src/Util/ClockMock.phpcorrectly replaces a PHP 8.0+ function call. However, this fix is incomplete. The codebase still contains calls tostr_starts_with(inFrameBuilder.php) andstr_contains(inDynamicSamplingContext.php). The PR author's belief that a polyfill is provided bysymfony/options-resolveris incorrect. Since the project supports PHP 7.2+ and lacks a proper polyfill, these calls will cause fatal 'Call to undefined function' errors on PHP 7.2-7.4, crashing the application when processing stack traces or tracing headers. -
Suggested fix: Apply the same fix pattern to the remaining instances. Replace
str_starts_withinFrameBuilder.phpandstr_containsinDynamicSamplingContext.phpwith PHP 7-compatible alternatives. Alternatively, add thesymfony/polyfill-php80dependency to provide these functions on older PHP versions.
severity: 0.9, confidence: 0.95
Did we get this right? 👍 / 👎 to inform future reviews.
2 participants
Replaces the usage of
str_starts_within our vendoredClockMockwithstrposto remain PHP 7.2 compatible.It currently only works because we are using polyfill through
symfony/options-resolver, which will be removed in the future and will cause this code to fail.