-
Notifications
You must be signed in to change notification settings - Fork 6.2k
8364752: java.time.Instant should be able to parse ISO 8601 offsets of the form HH:mm:ss #26708
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
7fc267a
5658f25
048f5ea
fa198b8
7523e34
8bc222a
a68b855
cd1507d
0d78a52
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -894,8 +894,10 @@ public Iterator<Entry<String, Long>> getTextIterator(TemporalField field, | |
* {@link DateTimeFormatter#parsedLeapSecond()} for full details. | ||
* <p> | ||
* When formatting, the instant will always be suffixed by 'Z' to indicate UTC. | ||
* When parsing, the behaviour of {@link DateTimeFormatterBuilder#appendOffsetId()} | ||
* will be used to parse the offset, converting the instant to UTC as necessary. | ||
* When parsing, the lenient mode behaviour of | ||
* {@link DateTimeFormatterBuilder#appendOffset(String, String) | ||
* appendOffset("+HH", "Z")} will be used to parse the offset, | ||
* converting the instant to UTC as necessary. | ||
* <p> | ||
* An alternative to this method is to format/parse the instant as a single | ||
* epoch-seconds value. That is achieved using {@code appendValue(INSTANT_SECONDS)}. | ||
|
@@ -956,7 +958,7 @@ public DateTimeFormatterBuilder appendInstant(int fractionalDigits) { | |
* 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")}. | ||
* This is equivalent to calling {@code appendOffset("+HH:MM:ss", "Z")}. | ||
* See {@link #appendOffset(String, String)} for details on formatting | ||
* and parsing. | ||
* | ||
|
@@ -3887,7 +3889,8 @@ public int parse(DateTimeParseContext context, CharSequence text, int position) | |
.appendValue(MINUTE_OF_HOUR, 2).appendLiteral(':') | ||
.appendValue(SECOND_OF_MINUTE, 2) | ||
.appendFraction(NANO_OF_SECOND, minDigits, maxDigits, true) | ||
.appendOffsetId() | ||
.parseLenient() | ||
.appendOffset("+HH", "Z") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The "+HH" pattern is supposed to be ignoring minutes and seconds but it does not appear to. In jshell, I see:
It would be more consistent with the original pattern to use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IIUC, "ignoring minutes and seconds" refers to formatting, i.e, "-02:10:12" only prints "-02" Parsing offsets in lenient mode always parses minute/seconds. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. True, but you have to read further into the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using the full pattern means the colons may not be optional. The
Among the supported patterns, only "+HH" (and I believe "+H" too) take the colons optional in lenient mode, i.e, both "+02:00" and "+0200" are allowed, which suits the ISO 8601 offset format. |
||
.toFormatter().toPrinterParser(false); | ||
DateTimeParseContext newContext = context.copy(); | ||
int pos = parser.parse(newContext, text, position); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
/* | ||
* Copyright (c) 2012, 2024, Oracle and/or its affiliates. All rights reserved. | ||
* Copyright (c) 2012, 2025, Oracle and/or its affiliates. All rights reserved. | ||
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. | ||
* | ||
* This code is free software; you can redistribute it and/or modify it | ||
|
@@ -61,6 +61,9 @@ | |
|
||
import java.time.Duration; | ||
import java.time.Instant; | ||
import java.time.OffsetDateTime; | ||
import java.time.ZoneOffset; | ||
import java.time.format.DateTimeParseException; | ||
import java.time.temporal.ChronoUnit; | ||
|
||
import org.testng.annotations.Test; | ||
|
@@ -70,7 +73,7 @@ | |
|
||
/** | ||
* Test Instant. | ||
* @bug 8273369 8331202 | ||
* @bug 8273369 8331202 8364752 | ||
*/ | ||
@Test | ||
public class TestInstant extends AbstractTest { | ||
|
@@ -151,4 +154,58 @@ public void test_until_1arg_NPE() { | |
Instant.now().until(null); | ||
}); | ||
} | ||
|
||
@DataProvider | ||
private Object[][] valid_instants() { | ||
var I1 = OffsetDateTime.of(2017, 1, 1, 0, 0, 0, 0, ZoneOffset.of("+02")).toInstant(); | ||
var I2 = OffsetDateTime.of(2017, 1, 1, 0, 0, 0, 0, ZoneOffset.of("+02:02")).toInstant(); | ||
var I3 = OffsetDateTime.of(2017, 1, 1, 0, 0, 0, 0, ZoneOffset.of("+02:02:02")).toInstant(); | ||
var I4 = OffsetDateTime.of(2017, 1, 1, 0, 0, 0, 0, ZoneOffset.of("Z")).toInstant(); | ||
return new Object[][] { | ||
{"2017-01-01T00:00:00.000+02", I1}, | ||
{"2017-01-01T00:00:00.000+0200", I1}, | ||
{"2017-01-01T00:00:00.000+02:00", I1}, | ||
{"2017-01-01T00:00:00.000+020000", I1}, | ||
{"2017-01-01T00:00:00.000+02:00:00", I1}, | ||
|
||
{"2017-01-01T00:00:00.000+0202", I2}, | ||
{"2017-01-01T00:00:00.000+02:02", I2}, | ||
|
||
{"2017-01-01T00:00:00.000+020202", I3}, | ||
{"2017-01-01T00:00:00.000+02:02:02", I3}, | ||
|
||
{"2017-01-01T00:00:00.000Z", I4}, | ||
}; | ||
} | ||
|
||
@Test(dataProvider = "valid_instants") | ||
public void test_parse_valid(String instant, Instant expected) { | ||
assertEquals(Instant.parse(instant), expected); | ||
} | ||
|
||
@DataProvider | ||
private Object[][] invalid_instants() { | ||
return new Object[][] { | ||
{"2017-01-01T00:00:00.000"}, | ||
{"2017-01-01T00:00:00.000+0"}, | ||
{"2017-01-01T00:00:00.000+0:"}, | ||
{"2017-01-01T00:00:00.000+02:"}, | ||
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is nice that leniency allows |
||
{"2017-01-01T00:00:00.000+020"}, | ||
{"2017-01-01T00:00:00.000+02:0"}, | ||
{"2017-01-01T00:00:00.000+02:0:"}, | ||
{"2017-01-01T00:00:00.000+02:00:"}, | ||
{"2017-01-01T00:00:00.000+02:000"}, | ||
{"2017-01-01T00:00:00.000+02:00:0"}, | ||
{"2017-01-01T00:00:00.000+02:00:0:"}, | ||
{"2017-01-01T00:00:00.000+0200000"}, | ||
{"2017-01-01T00:00:00.000+02:00:000"}, | ||
{"2017-01-01T00:00:00.000+02:00:00:"}, | ||
{"2017-01-01T00:00:00.000UTC"}, | ||
}; | ||
} | ||
|
||
@Test(dataProvider = "invalid_instants") | ||
public void test_parse_invalid(String instant) { | ||
assertThrows(DateTimeParseException.class, () -> Instant.parse(instant)); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't copyright year need a bump?