RFR:JDK-8032051:"ZonedDateTime" class "parse" method fails with short time zone offset ("+01")
Roger Riggs
Roger.Riggs at Oracle.com
Fri Feb 26 14:15:42 UTC 2016
HI Stephen,
How lenient is lenient supposed to be? Looking at the offset test cases,
it seems to allow minutes
and seconds digits to be parsed even if the pattern did not include them.
+ @DataProvider(name="lenientOffsetParseData")
+ Object[][] data_lenient_offset_parse() {
+ return new Object[][] {
+ {"+HH", "+01", 3600},
+ {"+HH", "+0101", 3660},
+ {"+HH", "+010101", 3661},
+ {"+HH", "+01", 3600},
+ {"+HH", "+01:01", 3660},
+ {"+HH", "+01:01:01", 3661},
Thanks, Roger
On 2/26/2016 6:16 AM, Stephen Colebourne wrote:
> I don't think this is quite right.
>
> if ((length > position + 3) && (text.charAt(position + 3) == ':')) {
> parseType = 10;
> }
>
> This code will *always* select type 10 (colons) if a colon is found at
> position+3. Whereas the spec now says that it should only do this if
> the pattern is "HH". For other patterns, the colon/no-colon choice is
> defined to be based on the pattern.
>
> That said, I'm thinking it is better to make the spec more lenient to
> match the behaviour as implemented:
>
>
> When parsing in lenient mode, only the hours are mandatory - minutes
> and seconds are optional. If the character after the hour digits is a colon
> then the parser will parse using the pattern "HH:mm:ss", otherwise the
> parser will parse using the pattern "HHmmss".
>
>
> Additional TCKDateTimeFormatterBuilder tests will be needed to
> demonstrate the above. There should also be a test for data following
> the lenient parse. The following should all succeed:
>
> DateTimeFormatterBuilder().parseLenient().appendOffset("HH:MM").appendZoneId();
> "+01:00Europe/London"
> "+0100Europe/London"
>
> DateTimeFormatterBuilder().parseLenient().appendOffset("HH:MM").appendLiteral(":").appendZoneId();
> "+01:Europe/London"
>
> Note this special case, where the colon affects the parse type, but is
> not ultimately part of the offset, thus it is left to match the
> appendLiteral(":")
>
> You may want to think of some additional nasty edge cases!
>
> Stephen
>
> On 25 February 2016 at 15:44, nadeesh tv <nadeesh.tv at oracle.com> wrote:
>> Hi all,
>> Please see the updated webrev
>> http://cr.openjdk.java.net/~ntv/8032051/webrev.02/
>>
>> Thanks and Regards,
>> Nadeesh
>>
>> On 2/23/2016 5:17 PM, Stephen Colebourne wrote:
>>> Thanks for the changes.
>>>
>>> In `DateTimeFormatter`, the code should be
>>>
>>> .parseLenient()
>>> .appendOffsetId()
>>> .parseStrict()
>>>
>>> and the same in the other case. This ensures that existing callers who
>>> then embed the formatter in another formatter (like the
>>> ZONED_DATE_TIME constant) are unaffected.
>>>
>>>
>>> The logic for lenient parsing does not look right as it only handles
>>> types 5 and 6. This table shows the mappings needed:
>>>
>>> "+HH", -> "+HHmmss" or "+HH:mm:ss"
>>> "+HHmm", -> "+HHmmss",
>>> "+HH:mm", -> "+HH:mm:ss",
>>> "+HHMM", -> "+HHmmss",
>>> "+HH:MM", -> "+HH:mm:ss",
>>> "+HHMMss", -> "+HHmmss",
>>> "+HH:MM:ss", -> "+HH:mm:ss",
>>> "+HHMMSS", -> "+HHmmss",
>>> "+HH:MM:SS", -> "+HH:mm:ss",
>>> "+HHmmss",
>>> "+HH:mm:ss",
>>>
>>> Note that the "+HH" pattern is a special case, as we don't know
>>> whether to use the colon or non-colon pattern. Whether to require
>>> colon or not is based on whether the next character after the HH is a
>>> colon or not.
>>>
>>> Proposed appendOffsetId() Javadoc:
>>>
>>> * Appends the zone offset, such as '+01:00', to the formatter.
>>> * <p>
>>> * This appends an instruction to format/parse the offset ID to the
>>> builder.
>>> * This is equivalent to calling {@code appendOffset("+HH:MM:ss", "Z")}.
>>> * See {@link #appendOffset(String, String)} for details on formatting
>>> and parsing.
>>>
>>> Proposed appendOffset(String, String) Javadoc:
>>>
>>> * During parsing, the offset...
>>>
>>> changed to:
>>>
>>> * When parsing in strict mode, the input must contain the mandatory
>>> and optional elements are defined by the specified pattern.
>>> * If the offset cannot be parsed then an exception is thrown unless
>>> the section of the formatter is optional.
>>> * <p>
>>> * When parsing in lenient mode, only the hours are mandatory - minutes
>>> and seconds are optional.
>>> * The colons are required if the specified pattern contains a colon.
>>> * If the specified pattern is "+HH", the presence of colons is
>>> determined by whether the character after the hour digits is a colon
>>> or not.
>>> * If the offset cannot be parsed then an exception is thrown unless
>>> the section of the formatter is optional.
>>>
>>> thanks and sorry for delay
>>> Stephen
>>>
>>>
>>>
>>> On 11 February 2016 at 20:22, nadeesh tv <nadeesh.tv at oracle.com> wrote:
>>>> Hi all,
>>>>
>>>> Please review a fix for
>>>>
>>>> Bug Id https://bugs.openjdk.java.net/browse/JDK-8032051
>>>>
>>>> webrev http://cr.openjdk.java.net/~ntv/8032051/webrev.01/
>>>>
>>>> --
>>>> Thanks and Regards,
>>>> Nadeesh TV
>>>>
>> --
>> Thanks and Regards,
>> Nadeesh TV
>>
More information about the core-libs-dev
mailing list