Skip to content

PHP Parser: handle INF/NaN nodes #71

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

Merged
merged 1 commit into from
Jan 4, 2016
Merged

PHP Parser: handle INF/NaN nodes #71

merged 1 commit into from
Jan 4, 2016

Conversation

wfleming
Copy link
Contributor

@wfleming wfleming commented Jan 4, 2016

I tracked this back to this constant number in a source file.

It turns out the literal INF constant in a PHP source is fine:
the parser represents constants directly using the identifier, so
it's fine in JSON.

But the constant number is larger than PHP's max number, so it basically
overflows & get represented as INF after parsing.

On the belief that similar code might result in a NaN constant, I'm also
covering that one. This represents both values using constant strings
that are unlikely to actually appear in anyone's source code.

👀 @codeclimate/review

I tracked this back to this constant number in a source file.

It turns out the literal INF constant in a PHP source if fine:
the parser represents constants directly using the identifier, so
it's fine in JSON.

But the constant number is larger than PHP's max number, so it basically
overflows & get represented as INF after parsing.

On the belief that similar code might result in a NaN constant, I'm also
covering that one. This represents both values using constant strings
that are unlikely to actually appear in anyone's source code.
@gdiggs
Copy link
Contributor

gdiggs commented Jan 4, 2016

LGTM

wfleming added a commit that referenced this pull request Jan 4, 2016
@wfleming wfleming merged commit 3765013 into master Jan 4, 2016
@wfleming wfleming deleted the will/php-inf-nan branch January 4, 2016 22:11
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