On Tue, 17 Nov 2020 19:58:29 GMT, Ian Graves <igra...@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
