Support @JsonNames for enum values (#1473)

Fixes #1458

Move JsonNames implementation to internal package

formats/json/commonMain/src/kotlinx/serialization/json/JsonNames.kt

@@ -6,6 +6,7 @@ package kotlinx.serialization.json
 
 import kotlinx.serialization.*
 import kotlinx.serialization.descriptors.*
+import kotlinx.serialization.encoding.*
 import kotlinx.serialization.json.internal.*
 import kotlin.native.concurrent.*
 
@@ -36,28 +37,3 @@ import kotlin.native.concurrent.*
 @Target(AnnotationTarget.PROPERTY)
 @ExperimentalSerializationApi
 public annotation class JsonNames(vararg val names: String)
-
-@SharedImmutable
-internal val JsonAlternativeNamesKey = DescriptorSchemaCache.Key<Map<String, Int>>()
-
-@OptIn(ExperimentalSerializationApi::class)
-internal fun SerialDescriptor.buildAlternativeNamesMap(): Map<String, Int> {
-    fun MutableMap<String, Int>.putOrThrow(name: String, index: Int) {
-        if (name in this) {
-            throw JsonException(
-                "The suggested name '$name' for property ${getElementName(index)} is already one of the names for property " +
-                        "${getElementName(getValue(name))} in ${this@buildAlternativeNamesMap}"
-            )
-        }
-        this[name] = index
-    }
-
-    var builder: MutableMap<String, Int>? = null
-    for (i in 0 until elementsCount) {
-        getElementAnnotations(i).filterIsInstance<JsonNames>().singleOrNull()?.names?.forEach { name ->
-            if (builder == null) builder = createMapForCache(elementsCount)
-            builder!!.putOrThrow(name, i)
-        }
-    }
-    return builder ?: emptyMap()
-}

formats/json/commonMain/src/kotlinx/serialization/json/internal/JsonNamesMap.kt

@@ -0,0 +1,65 @@
+/*
+ * Copyright 2017-2021 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
+ */
+
+package kotlinx.serialization.json.internal
+
+import kotlinx.serialization.*
+import kotlinx.serialization.descriptors.*
+import kotlinx.serialization.encoding.*
+import kotlinx.serialization.json.*
+import kotlin.native.concurrent.*
+
+@SharedImmutable
+internal val JsonAlternativeNamesKey = DescriptorSchemaCache.Key<Map<String, Int>>()
+
+@OptIn(ExperimentalSerializationApi::class)
+internal fun SerialDescriptor.buildAlternativeNamesMap(): Map<String, Int> {
+    fun MutableMap<String, Int>.putOrThrow(name: String, index: Int) {
+        if (name in this) {
+            throw JsonException(
+                "The suggested name '$name' for property ${getElementName(index)} is already one of the names for property " +
+                        "${getElementName(getValue(name))} in ${this@buildAlternativeNamesMap}"
+            )
+        }
+        this[name] = index
+    }
+
+    var builder: MutableMap<String, Int>? = null
+    for (i in 0 until elementsCount) {
+        getElementAnnotations(i).filterIsInstance<JsonNames>().singleOrNull()?.names?.forEach { name ->
+            if (builder == null) builder = createMapForCache(elementsCount)
+            builder!!.putOrThrow(name, i)
+        }
+    }
+    return builder ?: emptyMap()
+}
+
+/**
+ * Serves same purpose as [SerialDescriptor.getElementIndex] but respects
+ * [JsonNames] annotation and [JsonConfiguration.useAlternativeNames] state.
+ */
+@OptIn(ExperimentalSerializationApi::class)
+internal fun SerialDescriptor.getJsonNameIndex(json: Json, name: String): Int {
+    val index = getElementIndex(name)
+    // Fast path, do not go through ConcurrentHashMap.get
+    // Note, it blocks ability to detect collisions between the primary name and alternate,
+    // but it eliminates a significant performance penalty (about -15% without this optimization)
+    if (index != CompositeDecoder.UNKNOWN_NAME) return index
+    if (!json.configuration.useAlternativeNames) return index
+    // Slow path
+    val alternativeNamesMap =
+        json.schemaCache.getOrPut(this, JsonAlternativeNamesKey, this::buildAlternativeNamesMap)
+    return alternativeNamesMap[name] ?: CompositeDecoder.UNKNOWN_NAME
+}
+
+/**
+ * Throws on [CompositeDecoder.UNKNOWN_NAME]
+ */
+@OptIn(ExperimentalSerializationApi::class)
+internal fun SerialDescriptor.getJsonNameIndexOrThrow(json: Json, name: String): Int {
+    val index = getJsonNameIndex(json, name)
+    if (index == CompositeDecoder.UNKNOWN_NAME)
+        throw SerializationException("$serialName does not contain element with name '$name'")
+    return index
+}

formats/json/commonMain/src/kotlinx/serialization/json/internal/StreamingJsonDecoder.kt

@@ -1,5 +1,5 @@
 /*
- * Copyright 2017-2020 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
+ * Copyright 2017-2021 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
  */
 
 package kotlinx.serialization.json.internal
@@ -101,19 +101,6 @@ internal open class StreamingJsonDecoder(
         }
     }
 
-    private fun SerialDescriptor.getJsonElementIndex(key: String): Int {
-        val index = this.getElementIndex(key)
-        // Fast path, do not go through ConcurrentHashMap.get
-        // Note, it blocks ability to detect collisions between the primary name and alternate,
-        // but it eliminates a significant performance penalty (about -15% without this optimization)
-        if (index != UNKNOWN_NAME) return index
-        if (!json.configuration.useAlternativeNames) return index
-        // Slow path
-        val alternativeNamesMap =
-            json.schemaCache.getOrPut(this, JsonAlternativeNamesKey, this::buildAlternativeNamesMap)
-        return alternativeNamesMap[key] ?: UNKNOWN_NAME
-    }
-
     /*
      * Checks whether JSON has `null` value for non-null property or unknown enum value for enum property
      */
@@ -122,8 +109,8 @@ internal open class StreamingJsonDecoder(
         if (!elementDescriptor.isNullable && !lexer.tryConsumeNotNull()) return true
         if (elementDescriptor.kind == SerialKind.ENUM) {
             val enumValue = lexer.peekString(configuration.isLenient)
-                ?: return false // if value is not a string, decodeEnum() will throw correct exception
-            val enumIndex = elementDescriptor.getElementIndex(enumValue)
+                    ?: return false // if value is not a string, decodeEnum() will throw correct exception
+            val enumIndex = elementDescriptor.getJsonNameIndex(json, enumValue)
             if (enumIndex == UNKNOWN_NAME) {
                 // Encountered unknown enum value, have to skip it
                 lexer.consumeString()
@@ -140,7 +127,7 @@ internal open class StreamingJsonDecoder(
             hasComma = false
             val key = decodeStringKey()
             lexer.consumeNextToken(COLON)
-            val index = descriptor.getJsonElementIndex(key)
+            val index = descriptor.getJsonNameIndex(json, key)
             val isUnknown = if (index != UNKNOWN_NAME) {
                 if (configuration.coerceInputValues && coerceInputValue(descriptor, index)) {
                     hasComma = lexer.tryConsumeComma()
@@ -264,7 +251,7 @@ internal open class StreamingJsonDecoder(
         else super.decodeInline(inlineDescriptor)
 
     override fun decodeEnum(enumDescriptor: SerialDescriptor): Int {
-        return enumDescriptor.getElementIndexOrThrow(decodeString())
+        return enumDescriptor.getJsonNameIndexOrThrow(json, decodeString())
     }
 }
 

formats/json/commonMain/src/kotlinx/serialization/json/internal/TreeJsonDecoder.kt

@@ -83,7 +83,7 @@ private sealed class AbstractJsonTreeDecoder(
     protected abstract fun currentElement(tag: String): JsonElement
 
     override fun decodeTaggedEnum(tag: String, enumDescriptor: SerialDescriptor): Int =
-        enumDescriptor.getElementIndexOrThrow(getValue(tag).content)
+        enumDescriptor.getJsonNameIndexOrThrow(json, getValue(tag).content)
 
     override fun decodeTaggedNull(tag: String): Nothing? = null
 
@@ -193,7 +193,7 @@ private open class JsonTreeDecoder(
         if (elementDescriptor.kind == SerialKind.ENUM) {
             val enumValue = (currentElement(tag) as? JsonPrimitive)?.contentOrNull
                     ?: return false // if value is not a string, decodeEnum() will throw correct exception
-            val enumIndex = elementDescriptor.getElementIndex(enumValue)
+            val enumIndex = elementDescriptor.getJsonNameIndex(json, enumValue)
             if (enumIndex == CompositeDecoder.UNKNOWN_NAME) return true
         }
         return false
@@ -297,14 +297,3 @@ private class JsonTreeListDecoder(json: Json, override val value: JsonArray) : A
         return CompositeDecoder.DECODE_DONE
     }
 }
-
-/**
- * Same as [SerialDescriptor.getElementIndex], but throws [SerializationException] if
- * given [name] is not associated with any element in the descriptor.
- */
-internal fun SerialDescriptor.getElementIndexOrThrow(name: String): Int {
-    val index = getElementIndex(name)
-    if (index == CompositeDecoder.UNKNOWN_NAME)
-        throw SerializationException("$serialName does not contain element with name '$name'")
-    return index
-}

formats/json/commonTest/src/kotlinx/serialization/features/JsonAlternativeNamesTest.kt

@@ -16,6 +16,19 @@ class JsonAlternativeNamesTest : JsonTestBase() {
     @Serializable
     data class WithNames(@JsonNames("foo", "_foo") val data: String)
 
+    @Serializable
+    enum class AlternateEnumNames {
+        @JsonNames("someValue", "some_value")
+        VALUE_A,
+        VALUE_B
+    }
+
+    @Serializable
+    data class WithEnumNames(
+        val enumList: List<AlternateEnumNames>,
+        val checkCoercion: AlternateEnumNames = AlternateEnumNames.VALUE_B
+    )
+
     @Serializable
     data class CollisionWithAlternate(
         @JsonNames("_foo") val data: String,
@@ -24,12 +37,49 @@ class JsonAlternativeNamesTest : JsonTestBase() {
 
     private val inputString1 = """{"foo":"foo"}"""
     private val inputString2 = """{"_foo":"foo"}"""
-    private val json = Json { useAlternativeNames = true }
+
+    private fun parameterizedCoercingTest(test: (json: Json, streaming: Boolean, msg: String) -> Unit) {
+        for (coercing in listOf(true, false)) {
+            val json = Json {
+                coerceInputValues = coercing
+                useAlternativeNames = true
+            }
+            parametrizedTest { streaming ->
+                test(
+                    json, streaming,
+                    "Failed test with coercing=$coercing and streaming=$streaming"
+                )
+            }
+        }
+    }
+
+    @Test
+    fun testEnumSupportsAlternativeNames() = noLegacyJs {
+        val input = """{"enumList":["VALUE_A", "someValue", "some_value", "VALUE_B"], "checkCoercion":"someValue"}"""
+        val expected = WithEnumNames(
+            listOf(
+                AlternateEnumNames.VALUE_A,
+                AlternateEnumNames.VALUE_A,
+                AlternateEnumNames.VALUE_A,
+                AlternateEnumNames.VALUE_B
+            ), AlternateEnumNames.VALUE_A
+        )
+        parameterizedCoercingTest { json, streaming, msg ->
+            assertEquals(expected, json.decodeFromString(input, streaming), msg)
+        }
+    }
+
+    @Test
+    fun topLevelEnumSupportAlternativeNames() = noLegacyJs {
+        parameterizedCoercingTest { json, streaming, msg ->
+            assertEquals(AlternateEnumNames.VALUE_A, json.decodeFromString("\"someValue\"", streaming), msg)
+        }
+    }
 
     @Test
     fun testParsesAllAlternativeNames() = noLegacyJs {
         for (input in listOf(inputString1, inputString2)) {
-            for (streaming in listOf(true, false)) {
+            parameterizedCoercingTest { json, streaming, _ ->
                 val data = json.decodeFromString(WithNames.serializer(), input, useStreaming = streaming)
                 assertEquals("foo", data.data, "Failed to parse input '$input' with streaming=$streaming")
             }
@@ -39,7 +89,7 @@ class JsonAlternativeNamesTest : JsonTestBase() {
     @Test
     fun testThrowsAnErrorOnDuplicateNames2() = noLegacyJs {
         val serializer = CollisionWithAlternate.serializer()
-        parametrizedTest { streaming ->
+        parameterizedCoercingTest { json, streaming, _ ->
             assertFailsWithMessage<SerializationException>(
                 """The suggested name '_foo' for property foo is already one of the names for property data""",
                 "Class ${serializer.descriptor.serialName} did not fail with streaming=$streaming"

formats/json/jsMain/src/kotlinx/serialization/json/internal/DynamicDecoders.kt

@@ -1,5 +1,5 @@
 /*
- * Copyright 2017-2020 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
+ * Copyright 2017-2021 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
  */
 
 package kotlinx.serialization.json.internal
@@ -79,7 +79,7 @@ private open class DynamicInput(
     }
 
     override fun decodeTaggedEnum(tag: String, enumDescriptor: SerialDescriptor): Int =
-        enumDescriptor.getElementIndexOrThrow(getByTag(tag) as String)
+        enumDescriptor.getJsonNameIndexOrThrow(json, getByTag(tag) as String)
 
     protected open fun getByTag(tag: String): dynamic = value[tag]