ICU-22853 Do not try to format java.time.Instant and Clock

unicode-org · Sep 19, 2024 · 531f595 · 531f595
1 parent 30efee0
commit 531f595
Show file tree

Hide file tree

Showing 8 changed files with 38 additions and 101 deletions.
diff --git a/icu4j/main/core/src/main/java/com/ibm/icu/impl/JavaTimeConverters.java b/icu4j/main/core/src/main/java/com/ibm/icu/impl/JavaTimeConverters.java
@@ -5,7 +5,6 @@
 
 import static java.time.temporal.ChronoField.MILLI_OF_SECOND;
 
-import java.time.Clock;
 import java.time.Instant;
 import java.time.LocalDate;
 import java.time.LocalDateTime;
@@ -34,7 +33,7 @@
  *
  * <p>
  * The class includes methods for converting various temporal types, such as
- * {@link Instant}, {@link ZonedDateTime}, {@link OffsetTime}, {@link OffsetDateTime}, {@link LocalTime},
+ * {@link ZonedDateTime}, {@link OffsetTime}, {@link OffsetDateTime}, {@link LocalTime},
  * {@link ChronoLocalDate}, and {@link ChronoLocalDateTime}, to {@link Calendar} instances.
  *
  * <p>
@@ -52,47 +51,6 @@ private JavaTimeConverters() {
         // Prevent instantiation, making this an utility class
     }
 
-    /**
-     * Converts the current instant from a {@link Clock} to a {@link Calendar}.
-     *
-     * <p>
-     * This method creates a {@link Calendar} instance that represents the current
-     * instant as provided by the specified {@link Clock}.
-     *
-     * @param clock The {@link Clock} providing the current instant.
-     * @return A {@link Calendar} instance representing the current instant as
-     *         provided by the specified {@link Clock}.
-     *
-     * @deprecated This API is ICU internal only.
-     */
-    @Deprecated
-    public static Calendar temporalToCalendar(Clock clock) {
-        long epochMillis = clock.millis();
-        String timeZone = clock.getZone().getId();
-        TimeZone icuTimeZone = TimeZone.getTimeZone(timeZone);
-        return millisToCalendar(epochMillis, icuTimeZone);
-    }
-
-    /**
-     * Converts an {@link Instant} to a {@link Calendar}.
-     *
-     * <p>
-     * This method creates a {@link Calendar} instance that represents the same
-     * point in time as the specified {@link Instant}. The resulting
-     * {@link Calendar} will be in the default time zone of the JVM.
-     *
-     * @param instant The {@link Instant} to convert.
-     * @return A {@link Calendar} instance representing the same point in time as
-     *         the specified {@link Instant}.
-     *
-     * @deprecated This API is ICU internal only.
-     */
-    @Deprecated
-    public static Calendar temporalToCalendar(Instant instant) {
-        long epochMillis = instant.toEpochMilli();
-        return millisToCalendar(epochMillis, TimeZone.GMT_ZONE);
-    }
-
     /**
      * Converts a {@link ZonedDateTime} to a {@link Calendar}.
      *
@@ -232,10 +190,9 @@ public static Calendar temporalToCalendar(LocalDateTime dateTime) {
      */
     @Deprecated
     public static Calendar temporalToCalendar(Temporal temp) {
-        if (temp instanceof Clock) {
-            return temporalToCalendar((Clock) temp);
-        } else if (temp instanceof Instant) {
-            return temporalToCalendar((Instant) temp);
+        if (temp instanceof Instant) {
+            throw new IllegalArgumentException("java.time.Instant cannot be formatted,"
+                    + " it does not have enough information");
         } else if (temp instanceof ZonedDateTime) {
             return temporalToCalendar((ZonedDateTime) temp);
         } else if (temp instanceof OffsetDateTime) {
@@ -253,8 +210,8 @@ public static Calendar temporalToCalendar(Temporal temp) {
         } else if (temp instanceof ChronoLocalDateTime) {
             return temporalToCalendar((ChronoLocalDateTime<?>) temp);
         } else {
-            System.out.println("WTF is " + temp.getClass());
-            return null;
+            throw new IllegalArgumentException("This type cannot be formatted: "
+                    + temp.getClass().getName());
         }
     }
 

diff --git a/icu4j/main/core/src/main/java/com/ibm/icu/message2/DateTimeFormatterFactory.java b/icu4j/main/core/src/main/java/com/ibm/icu/message2/DateTimeFormatterFactory.java
@@ -3,7 +3,6 @@
 
 package com.ibm.icu.message2;
 
-import java.time.Clock;
 import java.time.temporal.Temporal;
 import java.util.Calendar;
 import java.util.Locale;
@@ -352,12 +351,10 @@ public FormattedPlaceholder format(Object toFormat, Map<String, Object> variable
                     return new FormattedPlaceholder(
                             toFormat, new PlainStringFormattedValue("{|" + toFormat + "|}"));
                 }
-            } else if (toFormat instanceof Clock) {
-            	toFormat = JavaTimeConverters.temporalToCalendar((Clock) toFormat);
             } else if (toFormat instanceof Temporal) {
             	toFormat = JavaTimeConverters.temporalToCalendar((Temporal) toFormat);
             }
-            // Not an else-if here, because the `Clock` & `Temporal` conditions before make `toFormat` a `Calendar`
+            // Not an else-if here, because the `Temporal` conditions before make `toFormat` a `Calendar`
             if (toFormat instanceof Calendar) {
                 TimeZone tz = ((Calendar) toFormat).getTimeZone();
                 long milis = ((Calendar) toFormat).getTimeInMillis();

diff --git a/icu4j/main/core/src/main/java/com/ibm/icu/message2/MFDataModelFormatter.java b/icu4j/main/core/src/main/java/com/ibm/icu/message2/MFDataModelFormatter.java
@@ -3,7 +3,6 @@
 
 package com.ibm.icu.message2;
 
-import java.time.Clock;
 import java.time.temporal.Temporal;
 import java.util.ArrayList;
 import java.util.Date;
@@ -62,7 +61,6 @@ class MFDataModelFormatter {
                         .setDefaultFormatterNameForType(Date.class, "datetime")
                         .setDefaultFormatterNameForType(Calendar.class, "datetime")
                         .setDefaultFormatterNameForType(java.util.Calendar.class, "datetime")
-                        .setDefaultFormatterNameForType(Clock.class, "datetime")
                         .setDefaultFormatterNameForType(Temporal.class, "datetime")
 
                         // Number formatting

diff --git a/icu4j/main/core/src/main/java/com/ibm/icu/text/DateFormat.java b/icu4j/main/core/src/main/java/com/ibm/icu/text/DateFormat.java
@@ -14,7 +14,6 @@
 import java.text.Format;
 import java.text.ParseException;
 import java.text.ParsePosition;
-import java.time.Clock;
 import java.time.temporal.Temporal;
 import java.util.Arrays;
 import java.util.Date;
@@ -633,9 +632,6 @@ public final StringBuffer format(Object obj, StringBuffer toAppendTo,
         } else if (obj instanceof Number) {
             return format( new Date(((Number)obj).longValue()),
                           toAppendTo, fieldPosition );
-        } else if (obj instanceof Clock) {
-            return format(JavaTimeConverters.temporalToCalendar((Clock) obj),
-                          toAppendTo, fieldPosition);
         } else if (obj instanceof Temporal) {
             return format( (Temporal)obj, toAppendTo, fieldPosition );
         } else {

diff --git a/icu4j/main/core/src/main/java/com/ibm/icu/text/MessageFormat.java b/icu4j/main/core/src/main/java/com/ibm/icu/text/MessageFormat.java
@@ -24,7 +24,6 @@
 import java.text.Format;
 import java.text.ParseException;
 import java.text.ParsePosition;
-import java.time.Clock;
 import java.time.temporal.Temporal;
 import java.util.ArrayList;
 import java.util.Date;
@@ -1754,9 +1753,6 @@ private void format(int msgStart, PluralSelectorContext pluralNumber,
                 } else if (arg instanceof Calendar) {
                     // format a Calendar if can
                     dest.formatAndAppend(getStockDateFormatter(), arg);
-                } else if (arg instanceof Clock) {
-                    // format a Clock if can
-                    dest.formatAndAppend(getStockDateFormatter(), arg);
                 } else if (arg instanceof Temporal) {
                     // format a Temporal if can
                     dest.formatAndAppend(getStockDateFormatter(), arg);

diff --git a/icu4j/main/core/src/main/java/com/ibm/icu/text/SimpleDateFormat.java b/icu4j/main/core/src/main/java/com/ibm/icu/text/SimpleDateFormat.java
@@ -17,7 +17,6 @@
 import java.text.FieldPosition;
 import java.text.Format;
 import java.text.ParsePosition;
-import java.time.Clock;
 import java.time.temporal.Temporal;
 import java.util.ArrayList;
 import java.util.Date;
@@ -3920,8 +3919,6 @@ public AttributedCharacterIterator formatToCharacterIterator(Object obj) {
             calendar.setTime((Date)obj);
         } else if (obj instanceof Number) {
             calendar.setTimeInMillis(((Number)obj).longValue());
-        } else if (obj instanceof Clock) {
-            cal = JavaTimeConverters.temporalToCalendar((Clock) obj);
         } else if (obj instanceof Temporal) {
             cal = JavaTimeConverters.temporalToCalendar((Temporal) obj);
         } else {

diff --git a/icu4j/main/core/src/test/java/com/ibm/icu/dev/test/format/JavaTimeConvertersTest.java b/icu4j/main/core/src/test/java/com/ibm/icu/dev/test/format/JavaTimeConvertersTest.java
@@ -3,7 +3,6 @@
 
 package com.ibm.icu.dev.test.format;
 
-import java.time.Clock;
 import java.time.Instant;
 import java.time.LocalDate;
 import java.time.LocalDateTime;
@@ -125,25 +124,10 @@ public void testDateAndTimes() {
 
     }
 
-    @Test
-    public void testInstantAndClock() {
-        // Instant has no time zone, assumes GMT.
-        EXPECTED_CALENDAR.setTimeZone(TimeZone.GMT_ZONE);
+    @Test(expected = IllegalArgumentException.class)
+    public void testInstantFails() {
         Instant instant = Instant.ofEpochMilli(EXPECTED_CALENDAR.getTimeInMillis());
-        Calendar calendar = JavaTimeConverters.temporalToCalendar(instant);
-        assertCalendarsEquals(EXPECTED_CALENDAR, calendar, DATE_ONLY_FIELDS);
-        assertCalendarsEquals(EXPECTED_CALENDAR, calendar, TIME_ONLY_FIELDS);
-        assertEquals("", EXPECTED_CALENDAR.getTimeZone().getID(), calendar.getTimeZone().getID());
-        assertEquals("", EXPECTED_CALENDAR.getTimeZone().getRawOffset(), calendar.getTimeZone().getRawOffset());
-        // Restore the time zone on the expected calendar
-        EXPECTED_CALENDAR.setTimeZone(TimeZone.getTimeZone(TIME_ZONE_ID));
-
-        Clock clock = Clock.fixed(instant, ZoneId.of(TIME_ZONE_ID));
-        calendar = JavaTimeConverters.temporalToCalendar(clock);
-        assertCalendarsEquals(EXPECTED_CALENDAR, calendar, DATE_ONLY_FIELDS);
-        assertCalendarsEquals(EXPECTED_CALENDAR, calendar, TIME_ONLY_FIELDS);
-        assertEquals("", EXPECTED_CALENDAR.getTimeZone().getID(), calendar.getTimeZone().getID());
-        assertEquals("", EXPECTED_CALENDAR.getTimeZone().getRawOffset(), calendar.getTimeZone().getRawOffset());
+        JavaTimeConverters.temporalToCalendar(instant);
     }
 
     // Compare the expected / actual calendar, but using an allowlist

diff --git a/icu4j/main/core/src/test/java/com/ibm/icu/dev/test/format/JavaTimeFormatTest.java b/icu4j/main/core/src/test/java/com/ibm/icu/dev/test/format/JavaTimeFormatTest.java
@@ -142,15 +142,11 @@ public void testNonGregorianDateFormatting() {
         }
     }
 
-    @Test
-    public void testInstantAndClockFormatting() {
+    @Test(expected = IllegalArgumentException.class)
+    public void testInstantFormattingFails() {
         DateFormat formatFromSkeleton = DateFormat.getInstanceForSkeleton("yMMMMd jmsSSSvvvv", Locale.US);
-
         Instant instant = LDT.toInstant(ZoneOffset.UTC);
-        assertEquals("", "September 27, 2013 at 7:43:56.123 PM Greenwich Mean Time", formatFromSkeleton.format(instant));
-
-        Clock clock = Clock.fixed(instant, ZoneId.of("America/Los_Angeles"));
-        assertEquals("", "September 27, 2013 at 12:43:56.123 PM Pacific Time", formatFromSkeleton.format(clock));
+        formatFromSkeleton.format(instant);
     }
 
     @Test
@@ -208,15 +204,6 @@ public void testMessageFormat() {
         arguments.put("expDate", LDT.atOffset(ZoneOffset.ofHours(2)));
         assertEquals("", expectedMf1Result, mf.format(arguments));
         assertEquals("", expectedMf2Result, mf2.formatToString(arguments));
-        // Instant
-        Instant instant = LDT.toInstant(ZoneOffset.UTC);
-        arguments.put("expDate", instant);
-        assertEquals("", expectedMf1Result, mf.format(arguments));
-        assertEquals("", expectedMf2Result, mf2.formatToString(arguments));
-        // Clock
-        arguments.put("expDate", Clock.fixed(instant, ZoneId.of("Europe/Paris")));
-        assertEquals("", expectedMf1Result, mf.format(arguments));
-        assertEquals("", expectedMf2Result, mf2.formatToString(arguments));
 
         // Test that both JDK and ICU Calendar are recognized as types.
         arguments.put("expDate", new java.util.GregorianCalendar(2013, 8, 27));
@@ -225,6 +212,31 @@ public void testMessageFormat() {
         // I filed https://unicode-org.atlassian.net/browse/ICU-22852
         // MF2 converts the JDK Calendar to an ICU Calendar, so it works.
         assertEquals("", expectedMf2Result, mf2.formatToString(arguments));
+
+        // Make sure that Instant and Clock are not formatted
+
+        // Instant
+        Instant instant = LDT.toInstant(ZoneOffset.UTC);
+        arguments.put("expDate", instant);
+        try {
+            mf.format(arguments);
+            fail("Should not be able to format java.time.Instant");
+        } catch (IllegalArgumentException ex) { /* expected to throw */ }
+        try {
+            mf2.formatToString(arguments);
+            fail("Should not be able to format java.time.Instant");
+        } catch (IllegalArgumentException ex) { /* expected to throw */ }
+
+        // Clock
+        arguments.put("expDate", Clock.fixed(instant, ZoneId.of("Europe/Paris")));
+        try {
+            mf.format(arguments);
+            fail("Should not be able to format java.time.Clock");
+        } catch (IllegalArgumentException ex) { /* expected to throw */ }
+        try {
+            mf2.formatToString(arguments);
+            fail("Should not be able to format java.time.Clock");
+        } catch (IllegalArgumentException ex) { /* expected to throw */ }
     }
 
     @Test