Convert y to u automatically, but emit a comment about it

Author: dkhalanskyjb

core/common/src/format/LocalDateFormat.kt

@@ -182,7 +182,7 @@ internal class IncompleteLocalDate(
         "${year ?: "??"}-${monthNumber ?: "??"}-${dayOfMonth ?: "??"} (day of week is ${isoDayOfWeek ?: "??"})"
 }
 
-private class YearDirective(private val padding: Padding) :
+private class YearDirective(private val padding: Padding, private val isYearOfEra: Boolean = false) :
     SignedIntFieldFormatDirective<DateFieldContainer>(
         DateFields.year,
         minDigits = padding.minDigits(4),
@@ -193,22 +193,62 @@ private class YearDirective(private val padding: Padding) :
     override val builderRepresentation: String get() = when (padding) {
         Padding.ZERO -> "${DateTimeFormatBuilder.WithDate::year.name}()"
         else -> "${DateTimeFormatBuilder.WithDate::year.name}(${padding.toKotlinCode()})"
+    }.let {
+        if (isYearOfEra) { it + YEAR_OF_ERA_COMMENT } else it
     }
 
-    override fun equals(other: Any?): Boolean = other is YearDirective && padding == other.padding
-    override fun hashCode(): Int = padding.hashCode()
+    override fun equals(other: Any?): Boolean =
+        other is YearDirective && padding == other.padding && isYearOfEra == other.isYearOfEra
+    override fun hashCode(): Int = padding.hashCode() * 31 + isYearOfEra.hashCode()
 }
 
-private class ReducedYearDirective(val base: Int) :
+private class ReducedYearDirective(val base: Int, private val isYearOfEra: Boolean = false) :
     ReducedIntFieldDirective<DateFieldContainer>(
         DateFields.year,
         digits = 2,
         base = base,
     ) {
-    override val builderRepresentation: String get() = "${DateTimeFormatBuilder.WithDate::yearTwoDigits.name}($base)"
+    override val builderRepresentation: String get() =
+        "${DateTimeFormatBuilder.WithDate::yearTwoDigits.name}($base)".let {
+            if (isYearOfEra) { it + YEAR_OF_ERA_COMMENT } else it
+        }
+
+    override fun equals(other: Any?): Boolean =
+        other is ReducedYearDirective && base == other.base && isYearOfEra == other.isYearOfEra
+    override fun hashCode(): Int = base.hashCode() * 31 + isYearOfEra.hashCode()
+}
+
+private const val YEAR_OF_ERA_COMMENT =
+    " /** TODO: the original format had an `y` directive, so the behavior is different on years earlier than 1 AD. See the [kotlinx.datetime.format.byUnicodePattern] documentation for details. */"
+
+/**
+ * A special directive for year-of-era that behaves equivalently to [DateTimeFormatBuilder.WithDate.year].
+ * This is the result of calling [byUnicodePattern] on a pattern that uses the ubiquitous "y" symbol.
+ * We need a separate directive so that, when using [DateTimeFormat.formatAsKotlinBuilderDsl], we can print an
+ * additional comment and explain that the behavior was not preserved exactly.
+ */
+internal fun DateTimeFormatBuilder.WithDate.yearOfEra(padding: Padding) {
+    @Suppress("NO_ELSE_IN_WHEN")
+    when (this) {
+        is AbstractWithDateBuilder -> addFormatStructureForDate(
+            BasicFormatStructure(YearDirective(padding, isYearOfEra = true))
+        )
+    }
+}
 
-    override fun equals(other: Any?): Boolean = other is ReducedYearDirective && base == other.base
-    override fun hashCode(): Int = base.hashCode()
+/**
+ * A special directive for year-of-era that behaves equivalently to [DateTimeFormatBuilder.WithDate.year].
+ * This is the result of calling [byUnicodePattern] on a pattern that uses the ubiquitous "y" symbol.
+ * We need a separate directive so that, when using [DateTimeFormat.formatAsKotlinBuilderDsl], we can print an
+ * additional comment and explain that the behavior was not preserved exactly.
+ */
+internal fun DateTimeFormatBuilder.WithDate.yearOfEraTwoDigits(baseYear: Int) {
+    @Suppress("NO_ELSE_IN_WHEN")
+    when (this) {
+        is AbstractWithDateBuilder -> addFormatStructureForDate(
+            BasicFormatStructure(ReducedYearDirective(baseYear, isYearOfEra = true))
+        )
+    }
 }
 
 private class MonthDirective(private val padding: Padding) :

core/common/src/format/Unicode.kt

@@ -86,10 +86,16 @@ public annotation class FormatStringsInDatetimeFormats
  * | `xxxx`                 | always      | unless zero | none          | `+0000`                    |
  * | `xxxxx`                | always      | unless zero | colon         | `+00:00`                   |
  *
- * `y` is the most common way to represent the year, but it's not supported by [byUnicodePattern] because its
- * interpretation depends on which calendar is being used.
- * For the ISO chronology and years 0001 and later, one can obtain results that are equivalent to `yyyy` and `yy` like
- * this: `byUnicodePattern(pattern.replace("yyyy", "uuuu").replace("yy", "uu")`.
+ * Additionally, because the `y` directive is very often used instead of `u`, they are taken to mean the same.
+ * This may lead to unexpected results if the year is negative: `y` would always produce a positive number, whereas
+ * `u` may sometimes produce a negative one. For example:
+ * ```
+ * LocalDate(-10, 1, 5).format { byUnicodeFormat("yyyy-MM-dd") } // -0010-01-05
+ * LocalDate(-10, 1, 5).toJavaLocalDate().format(java.time.format.DateTimeFormatter.ofPattern("yyyy-MM-dd")) // 0011-01-05
+ * ```
+ *
+ * Note that, when the format includes the era directive, [byUnicodePattern] will fail with an exception, so almost all
+ * of the intentional usages of `y` will correctly report an error instead of behaving slightly differently.
  *
  * @throws IllegalArgumentException if the pattern is invalid or contains unsupported directives.
  * @throws IllegalArgumentException if the builder is incompatible with the specified directives.
@@ -260,9 +266,13 @@ internal sealed interface UnicodeFormat {
 
             class YearOfEra(override val formatLength: Int) : DateBased() {
                 override val formatLetter = 'y'
-                override fun addToFormat(builder: DateTimeFormatBuilder.WithDate) = localizedDirective(
-                    "The locale-invariant ISO year directive '${"u".repeat(formatLength)}' can be used instead."
-                )
+                override fun addToFormat(builder: DateTimeFormatBuilder.WithDate) = when (formatLength) {
+                    1 -> builder.yearOfEra(padding = Padding.NONE)
+                    2 -> builder.yearOfEraTwoDigits(baseYear = 2000)
+                    3 -> unsupportedPadding(formatLength)
+                    4 -> builder.yearOfEra(padding = Padding.ZERO)
+                    else -> unsupportedPadding(formatLength)
+                }
             }
 
             class CyclicYearName(override val formatLength: Int) : DateBased() {

core/common/test/format/DateTimeFormatTest.kt

@@ -76,6 +76,20 @@ class DateTimeFormatTest {
         """.trimIndent(), kotlinCode)
     }
 
+    /**
+     * Check that we mention [byUnicodePattern] in the string representation of the format when the conversion is
+     * incorrect.
+     */
+    @OptIn(FormatStringsInDatetimeFormats::class)
+    @Test
+    fun testStringRepresentationAfterIncorrectConversion() {
+        for (format in listOf("yyyy-MM-dd", "yy-MM-dd")) {
+            assertContains(DateTimeFormat.formatAsKotlinBuilderDsl(
+                DateTimeComponents.Format { byUnicodePattern(format) }
+            ), "byUnicodePattern")
+        }
+    }
+
     @Test
     fun testParseStringWithNumbers() {
         val formats = listOf(

core/jvm/test/UnicodeFormatTest.kt

@@ -106,9 +106,7 @@ class UnicodeFormatTest {
             directives.any { it is UnicodeFormat.Directive.ZoneBased } -> TimeZone.availableZoneIds
             else -> setOf("Europe/Berlin")
         }
-        val format = DateTimeComponents.Format {
-            byUnicodePattern(pattern.replace("yyyy", "uuuu").replace("yy", "uu"))
-        }
+        val format = DateTimeComponents.Format { byUnicodePattern(pattern) }
         val javaFormat = DateTimeFormatter.ofPattern(pattern)
         for (date in dates) {
             for (time in times) {