From 154ada1070eece3c7fb215577749533eab270456 Mon Sep 17 00:00:00 2001 From: rriggs Date: Thu, 26 Sep 2013 15:19:27 -0700 Subject: [PATCH] 8025718: Enhance error messages for parsing Summary: Add values and types to exception messages Reviewed-by: lancea Contributed-by: scolebourne@joda.org --- src/share/classes/java/time/DayOfWeek.java | 7 +- src/share/classes/java/time/Instant.java | 11 +- src/share/classes/java/time/LocalDate.java | 3 +- .../classes/java/time/LocalDateTime.java | 3 +- src/share/classes/java/time/LocalTime.java | 3 +- src/share/classes/java/time/Month.java | 3 +- src/share/classes/java/time/MonthDay.java | 3 +- .../classes/java/time/OffsetDateTime.java | 5 +- src/share/classes/java/time/OffsetTime.java | 3 +- src/share/classes/java/time/Year.java | 3 +- src/share/classes/java/time/YearMonth.java | 3 +- src/share/classes/java/time/ZoneId.java | 3 +- src/share/classes/java/time/ZoneOffset.java | 3 +- .../classes/java/time/ZonedDateTime.java | 3 +- .../classes/java/time/format/Parsed.java | 18 ++- .../time/format/TestDateTimeFormatter.java | 111 +++++++++++++++++- 16 files changed, 164 insertions(+), 21 deletions(-) diff --git a/src/share/classes/java/time/DayOfWeek.java b/src/share/classes/java/time/DayOfWeek.java index b48482241..605c57710 100644 --- a/src/share/classes/java/time/DayOfWeek.java +++ b/src/share/classes/java/time/DayOfWeek.java @@ -187,7 +187,12 @@ public enum DayOfWeek implements TemporalAccessor, TemporalAdjuster { if (temporal instanceof DayOfWeek) { return (DayOfWeek) temporal; } - return of(temporal.get(DAY_OF_WEEK)); + try { + return of(temporal.get(DAY_OF_WEEK)); + } catch (DateTimeException ex) { + throw new DateTimeException("Unable to obtain DayOfWeek from TemporalAccessor: " + + temporal + " of type " + temporal.getClass().getName(), ex); + } } //----------------------------------------------------------------------- diff --git a/src/share/classes/java/time/Instant.java b/src/share/classes/java/time/Instant.java index 0e22c7bf9..e7a264159 100644 --- a/src/share/classes/java/time/Instant.java +++ b/src/share/classes/java/time/Instant.java @@ -366,9 +366,14 @@ public final class Instant return (Instant) temporal; } Objects.requireNonNull(temporal, "temporal"); - long instantSecs = temporal.getLong(INSTANT_SECONDS); - int nanoOfSecond = temporal.get(NANO_OF_SECOND); - return Instant.ofEpochSecond(instantSecs, nanoOfSecond); + try { + long instantSecs = temporal.getLong(INSTANT_SECONDS); + int nanoOfSecond = temporal.get(NANO_OF_SECOND); + return Instant.ofEpochSecond(instantSecs, nanoOfSecond); + } catch (DateTimeException ex) { + throw new DateTimeException("Unable to obtain Instant from TemporalAccessor: " + + temporal + " of type " + temporal.getClass().getName()); + } } //----------------------------------------------------------------------- diff --git a/src/share/classes/java/time/LocalDate.java b/src/share/classes/java/time/LocalDate.java index f388959aa..4ac39968b 100644 --- a/src/share/classes/java/time/LocalDate.java +++ b/src/share/classes/java/time/LocalDate.java @@ -356,7 +356,8 @@ public final class LocalDate Objects.requireNonNull(temporal, "temporal"); LocalDate date = temporal.query(TemporalQuery.localDate()); if (date == null) { - throw new DateTimeException("Unable to obtain LocalDate from TemporalAccessor: " + temporal.getClass()); + throw new DateTimeException("Unable to obtain LocalDate from TemporalAccessor: " + + temporal + " of type " + temporal.getClass().getName()); } return date; } diff --git a/src/share/classes/java/time/LocalDateTime.java b/src/share/classes/java/time/LocalDateTime.java index aed44dd4b..30107f235 100644 --- a/src/share/classes/java/time/LocalDateTime.java +++ b/src/share/classes/java/time/LocalDateTime.java @@ -449,7 +449,8 @@ public final class LocalDateTime LocalTime time = LocalTime.from(temporal); return new LocalDateTime(date, time); } catch (DateTimeException ex) { - throw new DateTimeException("Unable to obtain LocalDateTime from TemporalAccessor: " + temporal.getClass(), ex); + throw new DateTimeException("Unable to obtain LocalDateTime from TemporalAccessor: " + + temporal + " of type " + temporal.getClass().getName(), ex); } } diff --git a/src/share/classes/java/time/LocalTime.java b/src/share/classes/java/time/LocalTime.java index 77ab2c7f2..1ac9737a1 100644 --- a/src/share/classes/java/time/LocalTime.java +++ b/src/share/classes/java/time/LocalTime.java @@ -397,7 +397,8 @@ public final class LocalTime Objects.requireNonNull(temporal, "temporal"); LocalTime time = temporal.query(TemporalQuery.localTime()); if (time == null) { - throw new DateTimeException("Unable to obtain LocalTime from TemporalAccessor: " + temporal.getClass()); + throw new DateTimeException("Unable to obtain LocalTime from TemporalAccessor: " + + temporal + " of type " + temporal.getClass().getName()); } return time; } diff --git a/src/share/classes/java/time/Month.java b/src/share/classes/java/time/Month.java index 85d12d8f1..578212d3d 100644 --- a/src/share/classes/java/time/Month.java +++ b/src/share/classes/java/time/Month.java @@ -217,7 +217,8 @@ public enum Month implements TemporalAccessor, TemporalAdjuster { } return of(temporal.get(MONTH_OF_YEAR)); } catch (DateTimeException ex) { - throw new DateTimeException("Unable to obtain Month from TemporalAccessor: " + temporal.getClass(), ex); + throw new DateTimeException("Unable to obtain Month from TemporalAccessor: " + + temporal + " of type " + temporal.getClass().getName(), ex); } } diff --git a/src/share/classes/java/time/MonthDay.java b/src/share/classes/java/time/MonthDay.java index 67f5d4fa8..13986b618 100644 --- a/src/share/classes/java/time/MonthDay.java +++ b/src/share/classes/java/time/MonthDay.java @@ -266,7 +266,8 @@ public final class MonthDay } return of(temporal.get(MONTH_OF_YEAR), temporal.get(DAY_OF_MONTH)); } catch (DateTimeException ex) { - throw new DateTimeException("Unable to obtain MonthDay from TemporalAccessor: " + temporal.getClass(), ex); + throw new DateTimeException("Unable to obtain MonthDay from TemporalAccessor: " + + temporal + " of type " + temporal.getClass().getName(), ex); } } diff --git a/src/share/classes/java/time/OffsetDateTime.java b/src/share/classes/java/time/OffsetDateTime.java index 410c7f3a7..df474cbc0 100644 --- a/src/share/classes/java/time/OffsetDateTime.java +++ b/src/share/classes/java/time/OffsetDateTime.java @@ -347,8 +347,8 @@ public final class OffsetDateTime if (temporal instanceof OffsetDateTime) { return (OffsetDateTime) temporal; } - ZoneOffset offset = ZoneOffset.from(temporal); try { + ZoneOffset offset = ZoneOffset.from(temporal); try { LocalDateTime ldt = LocalDateTime.from(temporal); return OffsetDateTime.of(ldt, offset); @@ -357,7 +357,8 @@ public final class OffsetDateTime return OffsetDateTime.ofInstant(instant, offset); } } catch (DateTimeException ex) { - throw new DateTimeException("Unable to obtain OffsetDateTime from TemporalAccessor: " + temporal.getClass(), ex); + throw new DateTimeException("Unable to obtain OffsetDateTime from TemporalAccessor: " + + temporal + " of type " + temporal.getClass().getName(), ex); } } diff --git a/src/share/classes/java/time/OffsetTime.java b/src/share/classes/java/time/OffsetTime.java index a8dbf8a7a..f3f646fa2 100644 --- a/src/share/classes/java/time/OffsetTime.java +++ b/src/share/classes/java/time/OffsetTime.java @@ -284,7 +284,8 @@ public final class OffsetTime ZoneOffset offset = ZoneOffset.from(temporal); return new OffsetTime(time, offset); } catch (DateTimeException ex) { - throw new DateTimeException("Unable to obtain OffsetTime from TemporalAccessor: " + temporal.getClass(), ex); + throw new DateTimeException("Unable to obtain OffsetTime from TemporalAccessor: " + + temporal + " of type " + temporal.getClass().getName(), ex); } } diff --git a/src/share/classes/java/time/Year.java b/src/share/classes/java/time/Year.java index 377dfd518..1ca262ebf 100644 --- a/src/share/classes/java/time/Year.java +++ b/src/share/classes/java/time/Year.java @@ -249,7 +249,8 @@ public final class Year } return of(temporal.get(YEAR)); } catch (DateTimeException ex) { - throw new DateTimeException("Unable to obtain Year from TemporalAccessor: " + temporal.getClass(), ex); + throw new DateTimeException("Unable to obtain Year from TemporalAccessor: " + + temporal + " of type " + temporal.getClass().getName(), ex); } } diff --git a/src/share/classes/java/time/YearMonth.java b/src/share/classes/java/time/YearMonth.java index 223c90ab0..3c12784d8 100644 --- a/src/share/classes/java/time/YearMonth.java +++ b/src/share/classes/java/time/YearMonth.java @@ -252,7 +252,8 @@ public final class YearMonth } return of(temporal.get(YEAR), temporal.get(MONTH_OF_YEAR)); } catch (DateTimeException ex) { - throw new DateTimeException("Unable to obtain YearMonth from TemporalAccessor: " + temporal.getClass(), ex); + throw new DateTimeException("Unable to obtain YearMonth from TemporalAccessor: " + + temporal + " of type " + temporal.getClass().getName(), ex); } } diff --git a/src/share/classes/java/time/ZoneId.java b/src/share/classes/java/time/ZoneId.java index 86b0c74fd..6d42601c5 100644 --- a/src/share/classes/java/time/ZoneId.java +++ b/src/share/classes/java/time/ZoneId.java @@ -504,7 +504,8 @@ public abstract class ZoneId implements Serializable { public static ZoneId from(TemporalAccessor temporal) { ZoneId obj = temporal.query(TemporalQuery.zone()); if (obj == null) { - throw new DateTimeException("Unable to obtain ZoneId from TemporalAccessor: " + temporal.getClass()); + throw new DateTimeException("Unable to obtain ZoneId from TemporalAccessor: " + + temporal + " of type " + temporal.getClass().getName()); } return obj; } diff --git a/src/share/classes/java/time/ZoneOffset.java b/src/share/classes/java/time/ZoneOffset.java index 3475e5f4d..e1e4993ec 100644 --- a/src/share/classes/java/time/ZoneOffset.java +++ b/src/share/classes/java/time/ZoneOffset.java @@ -336,7 +336,8 @@ public final class ZoneOffset Objects.requireNonNull(temporal, "temporal"); ZoneOffset offset = temporal.query(TemporalQuery.offset()); if (offset == null) { - throw new DateTimeException("Unable to obtain ZoneOffset from TemporalAccessor: " + temporal.getClass()); + throw new DateTimeException("Unable to obtain ZoneOffset from TemporalAccessor: " + + temporal + " of type " + temporal.getClass().getName()); } return offset; } diff --git a/src/share/classes/java/time/ZonedDateTime.java b/src/share/classes/java/time/ZonedDateTime.java index 1114ab4ed..679653f61 100644 --- a/src/share/classes/java/time/ZonedDateTime.java +++ b/src/share/classes/java/time/ZonedDateTime.java @@ -553,7 +553,8 @@ public final class ZonedDateTime return of(ldt, zone); } } catch (DateTimeException ex) { - throw new DateTimeException("Unable to create ZonedDateTime from TemporalAccessor: " + temporal.getClass(), ex); + throw new DateTimeException("Unable to obtain ZonedDateTime from TemporalAccessor: " + + temporal + " of type " + temporal.getClass().getName(), ex); } } diff --git a/src/share/classes/java/time/format/Parsed.java b/src/share/classes/java/time/format/Parsed.java index 741346782..908c7deca 100644 --- a/src/share/classes/java/time/format/Parsed.java +++ b/src/share/classes/java/time/format/Parsed.java @@ -588,11 +588,23 @@ final class Parsed implements TemporalAccessor { //----------------------------------------------------------------------- @Override public String toString() { - String str = fieldValues.toString() + "," + chrono + "," + zone; + StringBuilder buf = new StringBuilder(64); + buf.append(fieldValues).append(',').append(chrono); + if (zone != null) { + buf.append(',').append(zone); + } if (date != null || time != null) { - str += " resolved to " + date + "," + time; + buf.append(" resolved to "); + if (date != null) { + buf.append(date); + if (time != null) { + buf.append('T').append(time); + } + } else { + buf.append(time); + } } - return str; + return buf.toString(); } } diff --git a/test/java/time/test/java/time/format/TestDateTimeFormatter.java b/test/java/time/test/java/time/format/TestDateTimeFormatter.java index 540a0e89a..1bfa47993 100644 --- a/test/java/time/test/java/time/format/TestDateTimeFormatter.java +++ b/test/java/time/test/java/time/format/TestDateTimeFormatter.java @@ -61,12 +61,32 @@ package test.java.time.format; import static java.time.temporal.ChronoField.DAY_OF_MONTH; import static org.testng.Assert.assertSame; +import static org.testng.Assert.assertTrue; +import static org.testng.Assert.fail; -import java.time.format.DecimalStyle; +import java.time.DateTimeException; +import java.time.DayOfWeek; +import java.time.Instant; +import java.time.LocalDate; +import java.time.LocalDateTime; +import java.time.LocalTime; +import java.time.Month; +import java.time.MonthDay; +import java.time.OffsetDateTime; +import java.time.OffsetTime; +import java.time.Year; +import java.time.YearMonth; +import java.time.ZoneId; +import java.time.ZoneOffset; +import java.time.ZonedDateTime; +import java.time.chrono.ThaiBuddhistChronology; import java.time.format.DateTimeFormatter; import java.time.format.DateTimeFormatterBuilder; +import java.time.format.DecimalStyle; import java.time.format.SignStyle; +import java.time.temporal.TemporalAccessor; import java.util.Locale; +import java.util.function.Function; import org.testng.annotations.Test; @@ -87,4 +107,93 @@ public class TestDateTimeFormatter { assertSame(test, base); } + @Test + public void test_parse_errorMessage() throws Exception { + assertGoodErrorDate(DayOfWeek::from, "DayOfWeek"); + assertGoodErrorDate(Month::from, "Month"); + assertGoodErrorDate(YearMonth::from, "YearMonth"); + assertGoodErrorDate(MonthDay::from, "MonthDay"); + assertGoodErrorDate(LocalDate::from, "LocalDate"); + assertGoodErrorDate(LocalTime::from, "LocalTime"); + assertGoodErrorDate(LocalDateTime::from, "LocalDateTime"); + assertGoodErrorDate(OffsetTime::from, "OffsetTime"); + assertGoodErrorDate(OffsetDateTime::from, "OffsetDateTime"); + assertGoodErrorDate(ZonedDateTime::from, "ZonedDateTime"); + assertGoodErrorDate(Instant::from, "Instant"); + assertGoodErrorDate(ZoneOffset::from, "ZoneOffset"); + assertGoodErrorDate(ZoneId::from, "ZoneId"); + assertGoodErrorDate(ThaiBuddhistChronology.INSTANCE::date, ""); + + assertGoodErrorTime(DayOfWeek::from, "DayOfWeek"); + assertGoodErrorTime(Month::from, "Month"); + assertGoodErrorTime(Year::from, "Year"); + assertGoodErrorTime(YearMonth::from, "YearMonth"); + assertGoodErrorTime(MonthDay::from, "MonthDay"); + assertGoodErrorTime(LocalDate::from, "LocalDate"); + assertGoodErrorTime(LocalTime::from, "LocalTime"); + assertGoodErrorTime(LocalDateTime::from, "LocalDateTime"); + assertGoodErrorTime(OffsetTime::from, "OffsetTime"); + assertGoodErrorTime(OffsetDateTime::from, "OffsetDateTime"); + assertGoodErrorTime(ZonedDateTime::from, "ZonedDateTime"); + assertGoodErrorTime(Instant::from, "Instant"); + assertGoodErrorTime(ZoneOffset::from, "ZoneOffset"); + assertGoodErrorTime(ZoneId::from, "ZoneId"); + assertGoodErrorTime(ThaiBuddhistChronology.INSTANCE::date, ""); + } + + private void assertGoodErrorDate(Function function, String expectedText) { + DateTimeFormatter f = DateTimeFormatter.ofPattern("yyyy-mm-dd"); + TemporalAccessor temporal = f.parse("2010-06-30"); + try { + function.apply(temporal); + fail("Should have failed"); + } catch (DateTimeException ex) { + String msg = ex.getMessage(); + assertTrue(msg.contains(expectedText), msg); + assertTrue(msg.contains("Year"), msg); + assertTrue(msg.contains("MinuteOfHour"), msg); + assertTrue(msg.contains("DayOfMonth"), msg); + } + } + + private void assertGoodErrorTime(Function function, String expectedText) { + DateTimeFormatter f = DateTimeFormatter.ofPattern("HH:MM:ss"); + TemporalAccessor temporal = f.parse("11:30:56"); + try { + function.apply(temporal); + fail("Should have failed"); + } catch (DateTimeException ex) { + String msg = ex.getMessage(); + assertTrue(msg.contains(expectedText), msg); + assertTrue(msg.contains("HourOfDay"), msg); + assertTrue(msg.contains("MonthOfYear"), msg); + assertTrue(msg.contains("SecondOfMinute"), msg); + } + } + + @Test + public void test_parsed_toString_resolvedTime() { + DateTimeFormatter f = DateTimeFormatter.ofPattern("HH:mm:ss"); + TemporalAccessor temporal = f.parse("11:30:56"); + String msg = temporal.toString(); + assertTrue(msg.contains("11:30:56"), msg); + } + + @Test + public void test_parsed_toString_resolvedDate() { + DateTimeFormatter f = DateTimeFormatter.ofPattern("yyyy-MM-dd"); + TemporalAccessor temporal = f.parse("2010-06-30"); + String msg = temporal.toString(); + assertTrue(msg.contains("2010-06-30"), msg); + } + + @Test + public void test_parsed_toString_resolvedDateTime() { + DateTimeFormatter f = DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss"); + TemporalAccessor temporal = f.parse("2010-06-30 11:30:56"); + String msg = temporal.toString(); + assertTrue(msg.contains("2010-06-30"), msg); + assertTrue(msg.contains("11:30:56"), msg); + } + } -- GitLab