NumberUtils: accept leading "+" for integers, reject overflowing exponents - #104
Merged
Merged
Conversation
…ing exponents Three follow-ups to the locale parsing work: - parseIntegerStrict() rejected a leading "+", while displayStringToDouble() accepts one, so "+3" parsed as a double but not as an int. DecimalFormat cannot parse "+" in any locale, so strip it rather than widening the pattern. - displayStringToDouble() allows an exponent, and Double.parseDouble() silently overflows it to Infinity, so "1e400" returned Infinity even though the literal "Infinity" is explicitly rejected. Reject any non-finite result. - NumberUtilsTest's @before guard skipped based on Locale.getDefault(), but the code it guards formats via Locale.getDefault(Locale.Category.FORMAT). On a split-locale system (UI locale en-US, regional format en-CH) those disagree, so the guard passed and doubleToStringIsCorrect then failed. Guard on the same category the code under test uses.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The three small leftovers from my last round of testing on the split-locale machine in micro-manager#2437. All three verified on that machine (Windows UI locale
en-US, regional formaten-CH).1. Integer fields rejected a leading
+.displayStringToDouble("+3")returns 3.0, butdisplayStringToInt("+3")failed, so the same string parsed in one field and not the other. Widening the pattern alone would not have worked:DecimalFormat.parse()cannot handle a leading+in any locale (I checkeden_CH,en_USandde_DE-- all return null), so it is stripped before parsing instead.2. An overflowing exponent silently became
Infinity. The pattern allows an exponent andDouble.parseDoubleoverflows quietly, so1e400returnedInfinityeven though the literal"Infinity"is explicitly rejected a few lines earlier. Now any non-finite result is rejected. I deliberately left1e-400->0.0alone: underflowing to zero is a reasonable answer for a physical quantity, overflowing to infinity is not.3.
NumberUtilsTest's@Beforeguard checked the wrong locale category. It skips based onLocale.getDefault(), but the code it guards formats viaNumberFormat.getInstance(), i.e.Locale.getDefault(Locale.Category.FORMAT). On a split-locale system those disagree, so the guard passed anddoubleToStringIsCorrectthen failed withexpected:<0[.]0001> but was:<0[,]0001>. Guarding on the same category the code uses fixes it.Worth noting for later: a test cannot pin the locale here, since
FORMATis built in a static initializer andLocale.setDefault()after class load has no effect on it. Skipping is the only option unless the assertions derive the separator themselves.Verification
Both suites, on this machine and under simulated
de-DEanden-US:NumberUtilsTestpasses on this machine for the first time. I also re-ran a brute force over ~1.1M short strings underen-CH(split),de-DE,en-USandfr-FR: no input is silently parsed as a different number, and no plain decimal number is rejected, in any of them. Round trip, negatives,Integer.MIN_VALUE/MAX_VALUEandLong.MAX_VALUEall still fine.