Skip to content
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

handle negative integers #325

Merged
merged 1 commit into from
Oct 24, 2016
Merged

handle negative integers #325

merged 1 commit into from
Oct 24, 2016

Conversation

shmax
Copy link
Collaborator

@shmax shmax commented Oct 24, 2016

Coercion of negative integers.
Fixes #323

@bighappyface

@@ -184,7 +184,7 @@ protected function toNumber($value)

protected function toInteger($value)
{
if(ctype_digit ($value)) {
if(is_numeric($value) && (string)(int)$value == $value) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is casting to string here necessary with loose equals?

-1 == "-1" // true
(int)"-1" == "-1" // true

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not totally sure of the implications, to be honest. I suppose at some point I should cover all the edges in the tests, but for now I removed the extra cast and the existing tests still pass.

Copy link
Collaborator

@bighappyface bighappyface left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@bighappyface bighappyface merged commit 43158d3 into jsonrainbow:master Oct 24, 2016
@shmax shmax deleted the negative-coerce branch October 25, 2016 02:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants