-
Notifications
You must be signed in to change notification settings - Fork 45
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
3.x: stan update #277
3.x: stan update #277
Conversation
2692758
to
a2a280a
Compare
src/Command/PolicyCommand.php
Outdated
@@ -76,7 +76,7 @@ public function templateData(Arguments $arguments): array | |||
$data = parent::templateData($arguments); | |||
|
|||
$name = $arguments->getArgument('name'); | |||
if (empty($name)) { | |||
if ($name === null || $name === '') { |
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 don't think this is better but psalm thinks it is 🤷🏻
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 (!$name)
was what I had in mind :)
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.
but psalm is not happy about that one as I described in core chat.
Because then null
and ''
would be handled in the same if which "could lead to unexpected behavior" according to psalm.
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.
Thus the silencing on top of it :)
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.
Otherwise you would have to use
/** @var non-empty-string|null $name */
$name = ...
if ($name === null)
if you wanted to make this clean
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.
Well If I just look at https://www.php.net/manual/en/types.comparisons.php then we didn't really do anything here other than just suppress the warning...
a2a280a
to
db73f34
Compare
No description provided.