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

numeric_literal_separator breaks numbers < 1 #7736

Closed
keradus opened this issue Jan 15, 2024 · 7 comments · Fixed by #7737
Closed

numeric_literal_separator breaks numbers < 1 #7736

keradus opened this issue Jan 15, 2024 · 7 comments · Fixed by #7737
Labels

Comments

@keradus
Copy link
Member

keradus commented Jan 15, 2024

original code:

<?php echo 0.001;
is modified into
<?php echo 0._001;
which is syntax error

@keradus
Copy link
Member Author

keradus commented Jan 15, 2024

@muuvmuuv want to take a look, please?

@muuvmuuv
Copy link
Contributor

Do you have a few more cases so I can add them all to the tests? Kinda weird that it adds a underscore here.

@keradus
Copy link
Member Author

keradus commented Jan 15, 2024

I had previous case, you fixed it, it allowed me to discover new case.
This is not only unexpected that _ is added, but also making code invalid.

I am not familiar with more failing cases yet, i hope none will be there.

@muuvmuuv
Copy link
Contributor

Problem with this case is that I cannot test it actually because the test suite only accepts code to be not the exact match than expected code. Hm, any idea?

Yeah, this one is critical, will try to fix it asap.

@keradus
Copy link
Member Author

keradus commented Jan 15, 2024

in data provider, use expected param and keep input param missing/null

@Wirone
Copy link
Member

Wirone commented Jan 15, 2024

@muuvmuuv Just don't add this case to yieldCases() (which provides two-way testing), but provide separate test case like do not override existing separator. Or you can modify yieldCases() to return null instead of input if both are the same. The reason behind yieldCases() was to simplify testing for converting in both directions (add or remove separator), but it was prepared for cases where input and expected value was different.

@muuvmuuv
Copy link
Contributor

Oh... well :D haven't thought that through well enough

image

Hmm. Actually, I don't know how I should "except" this case prior 8.1. Maybe to also check for "dots" in the string but idk sounds dirty/hacky. Any ideas?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants