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

Error parsing scientific notation with prototype access #676

Closed
edoardocavazza opened this issue Jan 19, 2022 · 2 comments · Fixed by #711
Closed

Error parsing scientific notation with prototype access #676

edoardocavazza opened this issue Jan 19, 2022 · 2 comments · Fixed by #711

Comments

@edoardocavazza
Copy link

Hello! We have encountered a bug while parsing a scientific notation followed by a prototype access.

console.log(8e-5.toFixed(2));

@alangpierce
Copy link
Owner

Thanks for the reporting, and sorry for the delay; I'm catching up on bug reports now. Looks like this is caused by the "e" being incorrectly handled as a hex digit from when I was trying to simplify number parsing. It should be reasonable to fix by splitting the cases out more explicitly. I should be able to put up a fix soon.

alangpierce added a commit that referenced this issue Jun 27, 2022
Fixes #676

Scientific notation parsing was using the shared `readInt` code path, which I
had made more flexible to handle hex digits, but that means that literals like
`1e2` were being treated as a complete number (with the hex digit `e`) rather
than as scientific notation. Normally this wouldn't be a problem since it's all
a number token anyway, but it caused trouble when using dot-style property
access like `1e2.toString()`. Expressions like `1.toString()` are expected to
fail because `.` is interpreted as a decimal point, but when using scientific
notation, the parser needs to be smart enough see that a decimal point wouldn't
be valid, and therefore not include it as part of the token.

The fix was to change `readInt` to only handle decimal digits, not hex digits,
and to inline the hex case into `readRadixNumber`. This should make number
parsing slightly faster because it's not checking for hex digits anymore.
alangpierce added a commit that referenced this issue Jun 27, 2022
Fixes #676

Scientific notation parsing was using the shared `readInt` code path, which I
had made more flexible to handle hex digits, but that means that literals like
`1e2` were being treated as a complete number (with the hex digit `e`) rather
than as scientific notation. Normally this wouldn't be a problem since it's all
a number token anyway, but it caused trouble when using dot-style property
access like `1e2.toString()`. Expressions like `1.toString()` are expected to
fail because `.` is interpreted as a decimal point, but when using scientific
notation, the parser needs to be smart enough see that a decimal point wouldn't
be valid, and therefore not include it as part of the token.

The fix was to change `readInt` to only handle decimal digits, not hex digits,
and to inline the hex case into `readRadixNumber`. This should make number
parsing slightly faster because it's not checking for hex digits anymore.
alangpierce added a commit that referenced this issue Jun 27, 2022
Fixes #676

Scientific notation parsing was using the shared `readInt` code path, which I
had made more flexible to handle hex digits, but that means that literals like
`1e2` were being treated as a complete number (with the hex digit `e`) rather
than as scientific notation. Normally this wouldn't be a problem since it's all
a number token anyway, but it caused trouble when using dot-style property
access like `1e2.toString()`. Expressions like `1.toString()` are expected to
fail because `.` is interpreted as a decimal point, but when using scientific
notation, the parser needs to be smart enough see that a decimal point wouldn't
be valid, and therefore not include it as part of the token.

The fix was to change `readInt` to only handle decimal digits, not hex digits,
and to inline the hex case into `readRadixNumber`. This should make number
parsing slightly faster because it's not checking for hex digits anymore.
@alangpierce
Copy link
Owner

The fix has been released in version 3.23.0.

1Lighty pushed a commit to Astra-mod/sucrase that referenced this issue Aug 14, 2022
Fixes alangpierce#676

Scientific notation parsing was using the shared `readInt` code path, which I
had made more flexible to handle hex digits, but that means that literals like
`1e2` were being treated as a complete number (with the hex digit `e`) rather
than as scientific notation. Normally this wouldn't be a problem since it's all
a number token anyway, but it caused trouble when using dot-style property
access like `1e2.toString()`. Expressions like `1.toString()` are expected to
fail because `.` is interpreted as a decimal point, but when using scientific
notation, the parser needs to be smart enough see that a decimal point wouldn't
be valid, and therefore not include it as part of the token.

The fix was to change `readInt` to only handle decimal digits, not hex digits,
and to inline the hex case into `readRadixNumber`. This should make number
parsing slightly faster because it's not checking for hex digits anymore.
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 a pull request may close this issue.

2 participants