Skip to content
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

Jump to bottom

8177819: DateTimeFormatterBuilder zone parsing should recognise DST #6527

Closed
wants to merge 5 commits into from
+121 −130
Closed

8177819: DateTimeFormatterBuilder zone parsing should recognise DST #6527

Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
40daaa2
8177819: DateTimeFormatterBuilder zone parsing should recognise DST
naotoj Nov 20, 2021
99ad3ca
Refined wording
naotoj Nov 23, 2021
4e9e061
Refined wording #2
naotoj Nov 24, 2021
89c894a
Replaced integer literals.
naotoj Nov 24, 2021
2be302f
Refined wording. Removed `LENIENT` parser. Added tests for CI.
naotoj Nov 29, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 src/java.base/share/classes/java/time/format/DateTimeFormatter.java
View file
Open in desktop
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2012, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2012, 2021, 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
@@ -81,6 +81,7 @@
import java.time.ZoneId;
import java.time.ZoneOffset;
import java.time.chrono.ChronoLocalDateTime;
import java.time.chrono.ChronoZonedDateTime;
import java.time.chrono.Chronology;
import java.time.chrono.IsoChronology;
import java.time.format.DateTimeFormatterBuilder.CompositePrinterParser;
@@ -373,15 +374,15 @@
* letters throws {@code IllegalArgumentException}.
* <p>
* <b>Zone names</b>: This outputs the display name of the time-zone ID. If the
* pattern letter is 'z' the output is the daylight savings aware zone name.
* pattern letter is 'z' the output is the daylight saving aware zone name.
* If there is insufficient information to determine whether DST applies,
* the name ignoring daylight savings time will be used.
* the name ignoring daylight saving time will be used.
* If the count of letters is one, two or three, then the short name is output.
* If the count of letters is four, then the full name is output.
* Five or more letters throws {@code IllegalArgumentException}.
* <p>
* If the pattern letter is 'v' the output provides the zone name ignoring
* daylight savings time. If the count of letters is one, then the short name is output.
* daylight saving time. If the count of letters is one, then the short name is output.
* If the count of letters is four, then the full name is output.
* Two, three and five or more letters throw {@code IllegalArgumentException}.
* <p>
@@ -502,7 +503,9 @@
* {@code LocalDateTime} to form the instant, with any zone ignored.
* If a {@code ZoneId} was parsed without an offset then the zone will be
* combined with the {@code LocalDateTime} to form the instant using the rules
* of {@link ChronoLocalDateTime#atZone(ZoneId)}.
* of {@link ChronoLocalDateTime#atZone(ZoneId)}. If the {@code ZoneId} was
* parsed from the zone name that does not indicate daylight saving time, then
* the standard offset will be used at the local time-line overlap.
Copy link
Contributor

@jodastephen jodastephen Nov 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As written, I would read it as being the generic zone name that gets altered, rather than a zone name that explicitly indicates "winter" time. maybe:

"If the {@code ZoneId} was parsed from a zone name that indicates whether daylight saving time in in operation or not, then that fact will be used to select the correct offset at the local time-line overlap."

Copy link
Member Author

@naotoj naotoj Nov 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Modified as suggested.

* </ol>
*
* @implSpec
94 src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java
View file
Open in desktop
@@ -4324,9 +4324,10 @@ static final class ZoneTextPrinterParser extends ZoneIdPrinterParser {
}
}

private static final int STD = 0;
private static final int DST = 1;
private static final int GENERIC = 2;
static final int UNDEFINED = -1;
static final int STD = 0;
static final int DST = 1;
static final int GENERIC = 2;
private static final Map<String, SoftReference<Map<Locale, String[]>>> cache =
new ConcurrentHashMap<>();

@@ -4433,11 +4434,11 @@ protected PrefixTree getTree(DateTimeParseContext context) {
nonRegionIds.add(zid);
continue;
}
tree.add(zid, zid); // don't convert zid -> metazone
tree.add(zid, zid, UNDEFINED); // don't convert zid -> metazone
zid = ZoneName.toZid(zid, locale);
int i = textStyle == TextStyle.FULL ? 1 : 2;
for (; i < names.length; i += 2) {
tree.add(names[i], zid);
tree.add(names[i], zid, (i - 1) / 2);
}
}

@@ -4450,7 +4451,7 @@ protected PrefixTree getTree(DateTimeParseContext context) {
int i = textStyle == TextStyle.FULL ? 1 : 2;
for (; i < cidNames.length; i += 2) {
if (cidNames[i] != null && !cidNames[i].isEmpty()) {
t.add(cidNames[i], cid);
t.add(cidNames[i], cid, (i - 1) / 2);
}
}
});
@@ -4465,7 +4466,7 @@ protected PrefixTree getTree(DateTimeParseContext context) {
}
int i = textStyle == TextStyle.FULL ? 1 : 2;
for (; i < names.length; i += 2) {
tree.add(names[i], zid);
tree.add(names[i], zid, (i - 1) / 2);
}
}
}
@@ -4571,15 +4572,16 @@ public int parse(DateTimeParseContext context, CharSequence text, int position)
// parse
PrefixTree tree = getTree(context);
ParsePosition ppos = new ParsePosition(position);
String parsedZoneId = tree.match(text, ppos);
if (parsedZoneId == null) {
PrefixTree parsedZoneId = tree.match(text, ppos);
if (parsedZoneId.value == null) {
if (context.charEquals(nextChar, 'Z')) {
context.setParsed(ZoneOffset.UTC);
return position + 1;
}
return ~position;
}
context.setParsed(ZoneId.of(parsedZoneId));
context.setParsed(ZoneId.of(parsedZoneId.value));
context.setParsedZoneNameType(parsedZoneId.type);
return ppos.getIndex();
}

@@ -4641,14 +4643,16 @@ public String toString() {
static class PrefixTree {
protected String key;
protected String value;
protected int type;
protected char c0; // performance optimization to avoid the
// boundary check cost of key.charat(0)
protected PrefixTree child;
protected PrefixTree sibling;

private PrefixTree(String k, String v, PrefixTree child) {
private PrefixTree(String k, String v, int type, PrefixTree child) {
this.key = k;
this.value = v;
this.type = type;
this.child = child;
if (k.isEmpty()) {
c0 = 0xffff;
@@ -4668,9 +4672,9 @@ public static PrefixTree newTree(DateTimeParseContext context) {
// return new LENIENT("", null, null);
//}
if (context.isCaseSensitive()) {
return new PrefixTree("", null, null);
return new PrefixTree("", null, ZoneTextPrinterParser.UNDEFINED, null);
}
return new CI("", null, null);
return new CI("", null, ZoneTextPrinterParser.UNDEFINED, null);
}

/**
@@ -4683,7 +4687,7 @@ public static PrefixTree newTree(DateTimeParseContext context) {
public static PrefixTree newTree(Set<String> keys, DateTimeParseContext context) {
PrefixTree tree = newTree(context);
for (String k : keys) {
tree.add0(k, k);
tree.add0(k, k, ZoneTextPrinterParser.UNDEFINED);
}
return tree;
}
@@ -4692,7 +4696,7 @@ public static PrefixTree newTree(Set<String> keys, DateTimeParseContext context
* Clone a copy of this tree
*/
public PrefixTree copyTree() {
PrefixTree copy = new PrefixTree(key, value, null);
PrefixTree copy = new PrefixTree(key, value, type, null);
if (child != null) {
copy.child = child.copyTree();
}
@@ -4710,11 +4714,11 @@ public PrefixTree copyTree() {
* @param v the value, not null
* @return true if the pair is added successfully
*/
public boolean add(String k, String v) {
return add0(k, v);
public boolean add(String k, String v, int t) {
return add0(k, v, t);
}

private boolean add0(String k, String v) {
private boolean add0(String k, String v, int t) {
k = toKey(k);
int prefixLen = prefixLength(k);
if (prefixLen == key.length()) {
@@ -4723,12 +4727,12 @@ private boolean add0(String k, String v) {
PrefixTree c = child;
while (c != null) {
if (isEqual(c.c0, subKey.charAt(0))) {
return c.add0(subKey, v);
return c.add0(subKey, v, t);
}
c = c.sibling;
}
// add the node as the child of the current node
c = newNode(subKey, v, null);
c = newNode(subKey, v, t, null);
c.sibling = child;
child = c;
return true;
@@ -4738,18 +4742,20 @@ private boolean add0(String k, String v) {
// return false;
//}
value = v;
type = t;
return true;
}
// split the existing node
PrefixTree n1 = newNode(key.substring(prefixLen), value, child);
PrefixTree n1 = newNode(key.substring(prefixLen), value, type, child);
key = k.substring(0, prefixLen);
child = n1;
if (prefixLen < k.length()) {
PrefixTree n2 = newNode(k.substring(prefixLen), v, null);
PrefixTree n2 = newNode(k.substring(prefixLen), v, t, null);
child.sibling = n2;
value = null;
} else {
value = v;
type = t;
}
return true;
}
@@ -4760,26 +4766,26 @@ private boolean add0(String k, String v) {
* @param text the input text to parse, not null
* @param off the offset position to start parsing at
* @param end the end position to stop parsing
* @return the resulting string, or null if no match found.
* @return the resulting tree, or null if no match found.
*/
public String match(CharSequence text, int off, int end) {
public PrefixTree match(CharSequence text, int off, int end) {
if (!prefixOf(text, off, end)){
return null;
}
if (child != null && (off += key.length()) != end) {
PrefixTree c = child;
do {
if (isEqual(c.c0, text.charAt(off))) {
String found = c.match(text, off, end);
PrefixTree found = c.match(text, off, end);
if (found != null) {
return found;
}
return value;
return this;
}
c = c.sibling;
} while (c != null);
}
return value;
return this;
}

/**
@@ -4789,9 +4795,9 @@ public String match(CharSequence text, int off, int end) {
* @param pos the position to start parsing at, from 0 to the text
* length. Upon return, position will be updated to the new parse
* position, or unchanged, if no match found.
* @return the resulting string, or null if no match found.
* @return the resulting tree, or null if no match found.
*/
public String match(CharSequence text, ParsePosition pos) {
public PrefixTree match(CharSequence text, ParsePosition pos) {
int off = pos.getIndex();
int end = text.length();
if (!prefixOf(text, off, end)){
@@ -4803,7 +4809,7 @@ public String match(CharSequence text, ParsePosition pos) {
do {
if (isEqual(c.c0, text.charAt(off))) {
pos.setIndex(off);
String found = c.match(text, pos);
PrefixTree found = c.match(text, pos);
if (found != null) {
return found;
}
@@ -4813,15 +4819,15 @@ public String match(CharSequence text, ParsePosition pos) {
} while (c != null);
}
pos.setIndex(off);
return value;
return this;
}

protected String toKey(String k) {
return k;
}

protected PrefixTree newNode(String k, String v, PrefixTree child) {
return new PrefixTree(k, v, child);
protected PrefixTree newNode(String k, String v, int t, PrefixTree child) {
return new PrefixTree(k, v, t, child);
}

protected boolean isEqual(char c1, char c2) {
@@ -4861,13 +4867,13 @@ private int prefixLength(String k) {
*/
private static class CI extends PrefixTree {

private CI(String k, String v, PrefixTree child) {
super(k, v, child);
private CI(String k, String v, int t, PrefixTree child) {
super(k, v, t, child);
}

@Override
protected CI newNode(String k, String v, PrefixTree child) {
return new CI(k, v, child);
protected CI newNode(String k, String v, int t, PrefixTree child) {
return new CI(k, v, t, child);
}

@Override
@@ -4897,13 +4903,13 @@ protected boolean prefixOf(CharSequence text, int off, int end) {
*/
private static class LENIENT extends CI {

private LENIENT(String k, String v, PrefixTree child) {
super(k, v, child);
private LENIENT(String k, String v, int t, PrefixTree child) {
Copy link
Contributor

@jodastephen jodastephen Nov 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is class LENIENT actually in use?

Copy link
Member Author

@naotoj naotoj Nov 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed.

super(k, v, t, child);
}

@Override
protected CI newNode(String k, String v, PrefixTree child) {
return new LENIENT(k, v, child);
protected CI newNode(String k, String v, int t, PrefixTree child) {
return new LENIENT(k, v, t, child);
}

private boolean isLenientChar(char c) {
@@ -4929,7 +4935,7 @@ protected String toKey(String k) {
}

@Override
public String match(CharSequence text, ParsePosition pos) {
public PrefixTree match(CharSequence text, ParsePosition pos) {
int off = pos.getIndex();
int end = text.length();
int len = key.length();
@@ -4956,7 +4962,7 @@ public String match(CharSequence text, ParsePosition pos) {
do {
if (isEqual(c.c0, text.charAt(off0))) {
pos.setIndex(off0);
String found = c.match(text, pos);
PrefixTree found = c.match(text, pos);
if (found != null) {
return found;
}
@@ -4967,7 +4973,7 @@ public String match(CharSequence text, ParsePosition pos) {
}
}
pos.setIndex(off);
return value;
return this;
}
}
}
20 src/java.base/share/classes/java/time/format/DateTimeParseContext.java
View file
Open in desktop
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2012, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2012, 2021, 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
@@ -417,6 +417,24 @@ void setParsed(ZoneId zone) {
currentParsed().zone = zone;
}

/**
* Stores the parsed zone name type.
* <p>
* This stores the zone name type that has been parsed.
* The parsed type should either be;
* <ul>
* <li>{@link DateTimeFormatterBuilder.ZoneTextPrinterParser#UNDEFINED}</li>
* <li>{@link DateTimeFormatterBuilder.ZoneTextPrinterParser#STD}</li>
* <li>{@link DateTimeFormatterBuilder.ZoneTextPrinterParser#DST}</li>
* <li>{@link DateTimeFormatterBuilder.ZoneTextPrinterParser#GENERIC}</li>
* </ul>
*
* @param type the parsed zone name type
*/
void setParsedZoneNameType(int type) {
currentParsed().zoneNameType = type;
}

/**
* Stores the parsed leap second.
*/