Re: RFR: JDK-8327640: Allow NumberFormat strict parsing [v2]

2024-03-21 Thread Justin Lu 
On Tue, 19 Mar 2024 08:56:46 GMT, Andrey Turbanov  wrote:

>> Justin Lu has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Replace protected field with a public getter -> isStrict().
>>   Replace setLenient() with setStrict() to avoid messy inversion of boolean.
>>   Add a leniency section to NumberFormat.
>>   Overhaul of NumberFormat class specification to be much more 
>> organized/readable.
>
> src/java.base/share/classes/java/text/DecimalFormat.java line 2482:
> 
>> 2480: // process digits or Inf, find decimal position
>> 2481: status[STATUS_INFINITE] = false;
>> 2482: if (!isExponent && text.regionMatches(position, 
>> symbols.getInfinity(),0,
> 
> Suggestion:
> 
> if (!isExponent && text.regionMatches(position, 
> symbols.getInfinity(), 0,

Thanks, this and the other suggestions you provided should be fixed.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18339#discussion_r1534407476

Re: RFR: JDK-8327640: Allow NumberFormat strict parsing [v2]

2024-03-19 Thread Andrey Turbanov 
On Tue, 19 Mar 2024 00:07:37 GMT, Justin Lu  wrote:

>> Please review this PR and associated 
>> [CSR](https://bugs.openjdk.org/browse/JDK-8327703) which introduces strict 
>> parsing for NumberFormat.
>> 
>> The concrete subclasses that will utilize this leniency value are 
>> `DecimalFormat` and `CompactNumberFormat`. Strict leniency allows for 
>> parsing to be used as an input validation technique for localized formatted 
>> numbers. The default leniency mode will remain lenient, and can only be set 
>> to strict through an intentional `setLenient(boolean)` method call. Leniency 
>> operates differently based off the values returned by `isGroupingUsed()`, 
>> `getGroupingSize()`, and `isParseIntegerOnly()`.
>> 
>> Below is an example of the change, the CSR can be viewed for further detail.
>> 
>> 
>> DecimalFormat fmt = (DecimalFormat) 
>> NumberFormat.getNumberInstance(Locale.US);
>> fmt.parse("1,,,0,,,00,.23Zabced45");  // returns 1000.23
>> fmt.setLenient(false);
>> fmt.parse("1,,,0,,,00,.23Zabced45"); // Now throws a ParseException
>> fmt.setGroupingSize(2);
>> fmt.parse("12,34,567"); // throws ParseException
>> fmt.setParseIntegerOnly(true)
>> fmt.parse("12,34.56"); // throws ParseException
>> fmt.parse("12,34"); // successfully returns the Number 1234
>> 
>> 
>> The associated tests are all localized, and the data is converted properly 
>> during runtime.
>
> Justin Lu has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Replace protected field with a public getter -> isStrict().
>   Replace setLenient() with setStrict() to avoid messy inversion of boolean.
>   Add a leniency section to NumberFormat.
>   Overhaul of NumberFormat class specification to be much more 
> organized/readable.

src/java.base/share/classes/java/text/DecimalFormat.java line 2419:

> 2417: if (gotPositive) {
> 2418: boolean containsPosSuffix =
> 2419: text.regionMatches(position, positiveSuffix,0, 
> positiveSuffix.length());

Suggestion:

text.regionMatches(position, positiveSuffix, 0, 
positiveSuffix.length());

src/java.base/share/classes/java/text/DecimalFormat.java line 2426:

> 2424: if (gotNegative) {
> 2425: boolean containsNegSuffix =
> 2426: text.regionMatches(position, negativeSuffix,0, 
> negativeSuffix.length());

Suggestion:

text.regionMatches(position, negativeSuffix, 0, 
negativeSuffix.length());

src/java.base/share/classes/java/text/DecimalFormat.java line 2428:

> 2426: text.regionMatches(position, negativeSuffix,0, 
> negativeSuffix.length());
> 2427: boolean endsWithNegSuffix =
> 2428: containsNegSuffix && text.length() == position  
> + negativeSuffix.length();

Suggestion:

containsNegSuffix && text.length() == position + 
negativeSuffix.length();

src/java.base/share/classes/java/text/DecimalFormat.java line 2482:

> 2480: // process digits or Inf, find decimal position
> 2481: status[STATUS_INFINITE] = false;
> 2482: if (!isExponent && text.regionMatches(position, 
> symbols.getInfinity(),0,

Suggestion:

if (!isExponent && text.regionMatches(position, symbols.getInfinity(), 
0,

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18339#discussion_r1529965478
PR Review Comment: https://git.openjdk.org/jdk/pull/18339#discussion_r1529965780
PR Review Comment: https://git.openjdk.org/jdk/pull/18339#discussion_r1529966184
PR Review Comment: https://git.openjdk.org/jdk/pull/18339#discussion_r1529966645

Re: RFR: JDK-8327640: Allow NumberFormat strict parsing [v2]

2024-03-18 Thread Justin Lu 
> Please review this PR and associated 
> [CSR](https://bugs.openjdk.org/browse/JDK-8327703) which introduces strict 
> parsing for NumberFormat.
> 
> The concrete subclasses that will utilize this leniency value are 
> `DecimalFormat` and `CompactNumberFormat`. Strict leniency allows for parsing 
> to be used as an input validation technique for localized formatted numbers. 
> The default leniency mode will remain lenient, and can only be set to strict 
> through an intentional `setLenient(boolean)` method call. Leniency operates 
> differently based off the values returned by `isGroupingUsed()`, 
> `getGroupingSize()`, and `isParseIntegerOnly()`.
> 
> Below is an example of the change, the CSR can be viewed for further detail.
> 
> 
> DecimalFormat fmt = (DecimalFormat) NumberFormat.getNumberInstance(Locale.US);
> fmt.parse("1,,,0,,,00,.23Zabced45");  // returns 1000.23
> fmt.setLenient(false);
> fmt.parse("1,,,0,,,00,.23Zabced45"); // Now throws a ParseException
> fmt.setGroupingSize(2);
> fmt.parse("12,34,567"); // throws ParseException
> fmt.setParseIntegerOnly(true)
> fmt.parse("12,34.56"); // throws ParseException
> fmt.parse("12,34"); // successfully returns the Number 1234
> 
> 
> The associated tests are all localized, and the data is converted properly 
> during runtime.

Justin Lu has updated the pull request incrementally with one additional commit 
since the last revision:

  Replace protected field with a public getter -> isStrict().
  Replace setLenient() with setStrict() to avoid messy inversion of boolean.
  Add a leniency section to NumberFormat.
  Overhaul of NumberFormat class specification to be much more 
organized/readable.

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18339/files
  - new: https://git.openjdk.org/jdk/pull/18339/files/8843c56f..c3a32500

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=18339=01
 - incr: https://webrevs.openjdk.org/?repo=jdk=18339=00-01

  Stats: 208 lines in 5 files changed: 77 ins; 54 del; 77 mod
  Patch: https://git.openjdk.org/jdk/pull/18339.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18339/head:pull/18339

PR: https://git.openjdk.org/jdk/pull/18339