<i18n dev> RFR: 8253459: Formatter treats index, width and precision > Integer.MAX_VALUE incorrectly [v9]

[Roger Riggs]

On Tue, 17 Nov 2020 19:58:29 GMT, Ian Graves <igraves at openjdk.org> wrote:

>> The `java.util.Formatter` format specifies support for field widths, argument indexes, or precision lengths of a field that relate to the variadic arguments supplied to the formatter. These numbers are specified by integers, sometimes negative. For argument index, it's specified in the documentation that the highest allowed argument is limited by the largest possible index of an array (ie the largest possible variadic index), but for the other two it's not defined. Moreover, what happens when a number field in a string is too large or too small to be represented by a 32-bit integer type is not defined.
>> 
>> This fix adds documentation to specify what error behavior occurs during these cases. Additionally it adds an additional exception type to throw when an invalid argument index is observed.
>> 
>> A CSR will be required for this PR.
>
> Ian Graves has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Adding test coverage. Tweaking wording in docs.

test/jdk/java/util/IllegalFormatException/ArgumentIndexException.java line 52:

> 50:             }
> 51:         }
> 52:         throw new RuntimeException("Expected IllegalFormatException for zero argument index.");

The wording of errors around exceptions can be misinterpreted. Did an expected exception occur or not?
If all you saw was the exception without the line of code, would it be unambiguous?

src/java.base/share/classes/java/util/Formatter.java line 2802:

> 2800:                     // skip the trailing '$'
> 2801:                     index = Integer.parseInt(s, start, end - 1, 10);
> 2802:                     if(index <= 0) {

Add a space after 'if' please.

test/jdk/java/util/IllegalFormatException/ArgumentIndexException.java line 1:

> 1: /*

Typically, using the TestNG framework is preferable for new tests.
In addition to a convenient set of Asserts that check and print expected vs actual and message
it includes patterns for testing for expected exceptions.

test/jdk/java/util/IllegalFormatException/ArgumentIndexException.java line 98:

> 96:     }
> 97: 
> 98: }

Add a newline at end-of-file.

src/java.base/share/classes/java/util/IllegalFormatArgumentIndexException.java line 64:

> 62:     public String getMessage() {
> 63:         return String.format("Illegal format argument index = %d", getIndex());
> 64:     }

The exception with a very large negative number isn't going to easy to recognize.
Can the exception message say (for the Integer.MIN_VALUE) that the index is not valid index.
Its probably too much to ask have an indication where in the format string the offending index occurs.

-------------

PR: https://git.openjdk.java.net/jdk/pull/516