-
Notifications
You must be signed in to change notification settings - Fork 8k
Fixing incorrect error message when passing null to join/implode's array parameter #12683
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
Conversation
iluuu1994
left a comment
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 think you misread the error message.
|
Related to #11991 |
|
Doing a simple if/else would be better IMHO. You coul keep the However, the issue @iluuu1994 is talking about (without me looking at test failures) is that the errors might reference the wrong variable+type name when providing an invalid type for the 1 parameter version. |
Got it. Just pushed the change.
I'll take a look at the failing tests and see if that's the case. |
|
Yes. That would cause
So I'll go back to my original approach, ok? |
2ecfccc to
54ad47f
Compare
b7b2467 to
e983de1
Compare
|
If after this change everything is ok, I have one question: I'd like to merge the |
Girgias
left a comment
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.
@iluuu1994 do you think this is fine or should we still split ZPP into two arity cases?
iluuu1994
left a comment
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.
do you think this is fine or should we still split ZPP into two arity cases?
It's fine. I think that only the message may be adjusted.
iluuu1994
left a comment
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.
LGTM otherwise
a812eee to
95b8db9
Compare
|
Are there any actions I need to take here or is this PR good? 😁 |
When passing
nullto the second parameter ofjoin/implodefunction, the error message said "string given" instead of "null given". This PR changes the error message to display the correct type.I also added the test to it in a separate file since the
implode1.phptwas huge already and it doesn't follow the recommendation of having_errorsuffix.Fixes #12682