-
Notifications
You must be signed in to change notification settings - Fork 27
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
fix: cast the replaced value as string #95
Conversation
@@ -16,14 +16,14 @@ | |||
class ConsoleListener | |||
{ | |||
/** @var EventDispatcherInterface */ | |||
protected $eventDispatcher = null; | |||
protected $eventDispatcher; |
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.
You should update line 67 to check with isset
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.
To be honest, I trusted blindly the cs-fixer. Thank you.
|
||
/** | ||
* Time when command started | ||
* | ||
* @var float | ||
*/ | ||
protected $startTime = null; | ||
protected $startTime; |
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.
If for some reason onException or onTerminate are called before onCommand we will get an exception (L.76) because startTime will not have been set. We should check if startTime is set before dispatching.
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 am struggling with the cs-fixer. I initialized these vars inside the constructor
On PHP8.1 when a value is an object, str_replace failed because of the type checking.
As a solution, we can cast the value returned by the propertyAccessor.
I also apply cs-fix to make the CI green 🤷♂️