Skip to content

Commit fa75b84

Browse files
sandwwraithsandwwraith
committed
Add Json configuration flag to allow comments
in C/Java style for both string and stream parser. This flag together with allowTrailingCommas and isLenient will help to cover most use-cases for Json5, for example, configuration files. Fixes #2221 Fixes #797
1 parent 1f7372a commit fa75b84

File tree

13 files changed

+509
-76
lines changed

13 files changed

+509
-76
lines changed

‎benchmark/src/jmh/kotlin/kotlinx/benchmarks/json/TwitterFeedCommentsBenchmark.kt

+64
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
/*
2+
* Copyright 2017-2024 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
3+
*/
4+
5+
package kotlinx.benchmarks.json
6+
7+
import kotlinx.benchmarks.model.*
8+
import kotlinx.serialization.json.*
9+
import org.openjdk.jmh.annotations.*
10+
import java.io.*
11+
import java.util.concurrent.*
12+
13+
@Warmup(iterations = 7, time = 1)
14+
@Measurement(iterations = 7, time = 1)
15+
@BenchmarkMode(Mode.Throughput)
16+
@OutputTimeUnit(TimeUnit.SECONDS)
17+
@State(Scope.Benchmark)
18+
@Fork(2)
19+
open class TwitterFeedCommentsBenchmark {
20+
val inputBytes = TwitterFeedBenchmark::class.java.getResource("/twitter_macro.json").readBytes()
21+
private val input = inputBytes.decodeToString()
22+
private val inputWithComments = prepareInputWithComments(input)
23+
private val inputWithCommentsBytes = inputWithComments.encodeToByteArray()
24+
25+
private val jsonComments = Json { ignoreUnknownKeys = true; allowComments = true; }
26+
private val jsonNoComments = Json { ignoreUnknownKeys = true; allowComments = false; }
27+
28+
fun prepareInputWithComments(inp: String): String {
29+
val result = inp.lineSequence().map { s ->
30+
// "id", "in_...", "is_...", etc
31+
if (!s.trimStart().startsWith("\"i")) s else "$s // json comment"
32+
}.joinToString("\n")
33+
assert(result.contains("// json comment"))
34+
return result
35+
}
36+
37+
@Setup
38+
fun init() {
39+
// Explicitly invoking both variants before benchmarking so we know that both parser implementation classes are loaded
40+
require("foobar" == jsonComments.decodeFromString<String>("\"foobar\""))
41+
require("foobar" == jsonNoComments.decodeFromString<String>("\"foobar\""))
42+
}
43+
44+
// The difference with TwitterFeedBenchmark.decodeMicroTwitter shows if we slow down when both StringJsonLexer and CommentsJsonLexer
45+
// are loaded by JVM. Should be almost non-existent on modern JVMs (but on OpenJDK-Corretto-11.0.14.1 there is one. 17 is fine.)
46+
@Benchmark
47+
fun decodeMicroTwitter() = jsonNoComments.decodeFromString(MicroTwitterFeed.serializer(), input)
48+
49+
// The difference with this.decodeMicroTwitter shows if we slow down when comments are enabled but no comments present
50+
// in the input. It is around 13% slower than without comments support, mainly because skipWhitespaces is a separate function
51+
// that sometimes is not inlined by JIT.
52+
@Benchmark
53+
fun decodeMicroTwitterCommentSupport() = jsonComments.decodeFromString(MicroTwitterFeed.serializer(), input)
54+
55+
// Shows how much actual skipping of the comments takes: around 10%.
56+
@Benchmark
57+
fun decodeMicroTwitterCommentInData() = jsonComments.decodeFromString(MicroTwitterFeed.serializer(), inputWithComments)
58+
59+
@Benchmark
60+
fun decodeMicroTwitterCommentSupportStream() = jsonComments.decodeFromStream(MicroTwitterFeed.serializer(), ByteArrayInputStream(inputBytes))
61+
62+
@Benchmark
63+
fun decodeMicroTwitterCommentInDataStream() = jsonComments.decodeFromStream(MicroTwitterFeed.serializer(), ByteArrayInputStream(inputWithCommentsBytes))
64+
}

‎benchmark/src/jmh/kotlin/kotlinx/benchmarks/json/TwitterFeedStreamBenchmark.kt

+9
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,14 @@ open class TwitterFeedStreamBenchmark {
3030
jacksonObjectMapper().configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false)
3131

3232

33+
@Setup
34+
fun init() {
35+
// Explicitly invoking decodeFromStream before benchmarking so we know that both parser implementation classes are loaded
36+
require("foobar" == Json.decodeFromStream<String>(ByteArrayInputStream("\"foobar\"".encodeToByteArray())))
37+
require("foobar" == Json.decodeFromString<String>("\"foobar\""))
38+
}
39+
40+
3341
private val inputStream: InputStream
3442
get() = ByteArrayInputStream(bytes)
3543
private val outputStream: OutputStream
@@ -59,6 +67,7 @@ open class TwitterFeedStreamBenchmark {
5967
}
6068
}
6169

70+
// Difference with TwitterFeedBenchmark.decodeMicroTwitter shows how heavy Java's standard UTF-8 decoding actually is.
6271
@Benchmark
6372
fun decodeMicroTwitterReadText(): MicroTwitterFeed {
6473
return inputStream.use {

‎formats/json-tests/commonTest/src/kotlinx/serialization/features/JsonCommentsTest.kt

+126
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
/*
2+
* Copyright 2017-2024 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
3+
*/
4+
5+
package kotlinx.serialization.features
6+
7+
import kotlinx.serialization.*
8+
import kotlinx.serialization.json.*
9+
import kotlin.test.*
10+
11+
class JsonCommentsTest: JsonTestBase() {
12+
val json = Json(default) {
13+
allowComments = true
14+
allowTrailingComma = true
15+
}
16+
17+
val withLenient = Json(json) {
18+
isLenient = true
19+
ignoreUnknownKeys = true
20+
}
21+
22+
@Test
23+
fun testBasic() = parametrizedTest { mode ->
24+
val inputBlock = """{"data": "b" /*value b*/ }"""
25+
val inputLine = "{\"data\": \"b\" // value b \n }"
26+
assertEquals(StringData("b"), json.decodeFromString(inputBlock, mode))
27+
assertEquals(StringData("b"), json.decodeFromString(inputLine, mode))
28+
}
29+
30+
@Serializable
31+
data class Target(val key: String, val key2: List<Int>, val key3: NestedTarget, val key4: String)
32+
33+
@Serializable
34+
data class NestedTarget(val nestedKey: String)
35+
36+
private fun target(key4: String): Target = Target("value", listOf(1, 2), NestedTarget("foo"), key4)
37+
38+
@Test
39+
fun testAllBlocks() = parametrizedTest { mode ->
40+
val input = """{ /*beginning*/
41+
/*before key*/ "key" /*after key*/ : /*after colon*/ "value" /*before comma*/,
42+
"key2": [ /*array1*/ 1, /*array2*/ 2, /*end array*/],
43+
"key3": { /*nested obj*/ "nestedKey": "foo"} /*after nested*/,
44+
"key4": "/*comment inside quotes is a part of value*/",
45+
/*before end*/
46+
}"""
47+
assertEquals(target("/*comment inside quotes is a part of value*/"), json.decodeFromString(input, mode))
48+
}
49+
50+
@Test
51+
fun testAllLines() = parametrizedTest { mode ->
52+
val input = """{ //beginning
53+
//before key
54+
"key" // after key
55+
: // after colon
56+
"value" //before comma
57+
,
58+
"key2": [ //array1
59+
1, //array2
60+
2, //end array
61+
],
62+
"key3": { //nested obj
63+
"nestedKey": "foo"
64+
} , //after nested
65+
"key4": "//comment inside quotes is a part of value",
66+
//before end
67+
}"""
68+
assertEquals(target("//comment inside quotes is a part of value"), json.decodeFromString(input, mode))
69+
}
70+
71+
@Test
72+
fun testMixed() = parametrizedTest { mode ->
73+
val input = """{ // begin
74+
"key": "value", // after
75+
"key2": /* array */ [1, 2],
76+
"key3": /* //this is a block comment */ { "nestedKey": // /*this is a line comment*/ "bar"
77+
"foo" },
78+
"key4": /* nesting block comments /* not supported */ "*/"
79+
/* end */}"""
80+
assertEquals(target("*/"), json.decodeFromString(input, mode))
81+
}
82+
83+
@Test
84+
fun testWithLenient() = parametrizedTest { mode ->
85+
val input = """{ //beginning
86+
//before key
87+
key // after key
88+
: // after colon
89+
value //before comma
90+
,
91+
key2: [ //array1
92+
1, //array2
93+
2, //end array
94+
],
95+
key3: { //nested obj
96+
nestedKey: "foo"
97+
} , //after nested
98+
key4: value//comment_cannot_break_value_apart,
99+
key5: //comment without quotes where new token expected is still a comment
100+
value5,
101+
//before end
102+
}"""
103+
assertEquals(target("value//comment_cannot_break_value_apart"), withLenient.decodeFromString(input, mode))
104+
}
105+
106+
@Test
107+
fun testUnclosedCommentsErrorMsg() = parametrizedTest { mode ->
108+
val input = """{"data": "x"} // no newline"""
109+
assertEquals(StringData("x"), json.decodeFromString<StringData>(input, mode))
110+
val input2 = """{"data": "x"} /* no endblock"""
111+
assertFailsWith<SerializationException>("Expected end of the block comment: \"*/\", but had EOF instead at path: \$") {
112+
json.decodeFromString<StringData>(input2, mode)
113+
}
114+
}
115+
116+
@Test
117+
fun testVeryLargeComments() = parametrizedTest { mode ->
118+
// 16 * 1024 is ReaderJsonLexer.BATCH_SIZE
119+
val strLen = 16 * 1024 * 2 + 42
120+
val inputLine = """{"data": //a""" + "a".repeat(strLen) + "\n\"x\"}"
121+
assertEquals(StringData("x"), json.decodeFromString<StringData>(inputLine, mode))
122+
val inputBlock = """{"data": /*a""" + "a".repeat(strLen) + "*/\"x\"}"
123+
assertEquals(StringData("x"), json.decodeFromString<StringData>(inputBlock, mode))
124+
}
125+
126+
}

‎formats/json/api/kotlinx-serialization-json.api

+3
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,7 @@ public final class kotlinx/serialization/json/JsonBuilder {
108108
public final fun getPrettyPrint ()Z
109109
public final fun getPrettyPrintIndent ()Ljava/lang/String;
110110
public final fun getSerializersModule ()Lkotlinx/serialization/modules/SerializersModule;
111+
public final fun getSupportComments ()Z
111112
public final fun getUseAlternativeNames ()Z
112113
public final fun getUseArrayPolymorphism ()Z
113114
public final fun isLenient ()Z
@@ -126,6 +127,7 @@ public final class kotlinx/serialization/json/JsonBuilder {
126127
public final fun setPrettyPrint (Z)V
127128
public final fun setPrettyPrintIndent (Ljava/lang/String;)V
128129
public final fun setSerializersModule (Lkotlinx/serialization/modules/SerializersModule;)V
130+
public final fun setSupportComments (Z)V
129131
public final fun setUseAlternativeNames (Z)V
130132
public final fun setUseArrayPolymorphism (Z)V
131133
}
@@ -154,6 +156,7 @@ public final class kotlinx/serialization/json/JsonConfiguration {
154156
public final fun getNamingStrategy ()Lkotlinx/serialization/json/JsonNamingStrategy;
155157
public final fun getPrettyPrint ()Z
156158
public final fun getPrettyPrintIndent ()Ljava/lang/String;
159+
public final fun getSupportComments ()Z
157160
public final fun getUseAlternativeNames ()Z
158161
public final fun getUseArrayPolymorphism ()Z
159162
public final fun isLenient ()Z

‎formats/json/commonMain/src/kotlinx/serialization/json/Json.kt

+19-2
Original file line numberDiff line numberDiff line change
@@ -102,12 +102,13 @@ public sealed class Json(
102102
* @throws [IllegalArgumentException] if the decoded input cannot be represented as a valid instance of type [T]
103103
*/
104104
public final override fun <T> decodeFromString(deserializer: DeserializationStrategy<T>, @FormatLanguage("json", "", "") string: String): T {
105-
val lexer = StringJsonLexer(string)
105+
val lexer = StringJsonLexer(this, string)
106106
val input = StreamingJsonDecoder(this, WriteMode.OBJ, lexer, deserializer.descriptor, null)
107107
val result = input.decodeSerializableValue(deserializer)
108108
lexer.expectEof()
109109
return result
110110
}
111+
111112
/**
112113
* Serializes the given [value] into an equivalent [JsonElement] using the given [serializer]
113114
*
@@ -385,6 +386,22 @@ public class JsonBuilder internal constructor(json: Json) {
385386
@ExperimentalSerializationApi
386387
public var allowTrailingComma: Boolean = json.configuration.allowTrailingComma
387388

389+
/**
390+
* Allows parser to accept C/Java-style comments in JSON input.
391+
*
392+
* Comments are being skipped and are not stored anywhere; this setting does not affect encoding in any way.
393+
*
394+
* More specifically, a comment is a substring that is not a part of JSON key or value, confirming to one of those:
395+
*
396+
* 1. Starts with `//` characters and ends with a newline character `\n`.
397+
* 2. Starts with `/*` characters and ends with `*/` characters. Nesting block comments
398+
* is not supported: no matter how many `/*` characters you have, first `*/` will end the comment.
399+
*
400+
* `false` by default.
401+
*/
402+
@ExperimentalSerializationApi
403+
public var allowComments: Boolean = json.configuration.allowComments
404+
388405
/**
389406
* Module with contextual and polymorphic serializers to be used in the resulting [Json] instance.
390407
*
@@ -422,7 +439,7 @@ public class JsonBuilder internal constructor(json: Json) {
422439
allowStructuredMapKeys, prettyPrint, explicitNulls, prettyPrintIndent,
423440
coerceInputValues, useArrayPolymorphism,
424441
classDiscriminator, allowSpecialFloatingPointValues, useAlternativeNames,
425-
namingStrategy, decodeEnumsCaseInsensitive, allowTrailingComma, classDiscriminatorMode
442+
namingStrategy, decodeEnumsCaseInsensitive, allowTrailingComma, allowComments, classDiscriminatorMode
426443
)
427444
}
428445
}

‎formats/json/commonMain/src/kotlinx/serialization/json/JsonConfiguration.kt

+2
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ public class JsonConfiguration @OptIn(ExperimentalSerializationApi::class) inter
3838
@ExperimentalSerializationApi
3939
public val allowTrailingComma: Boolean = false,
4040
@ExperimentalSerializationApi
41+
public val allowComments: Boolean = false,
42+
@ExperimentalSerializationApi
4143
public var classDiscriminatorMode: ClassDiscriminatorMode = ClassDiscriminatorMode.POLYMORPHIC,
4244
) {
4345

‎formats/json/commonMain/src/kotlinx/serialization/json/internal/JsonStreams.kt

+2-2
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ public fun <T> decodeByReader(
3737
deserializer: DeserializationStrategy<T>,
3838
reader: InternalJsonReader
3939
): T {
40-
val lexer = ReaderJsonLexer(reader)
40+
val lexer = ReaderJsonLexer(json, reader)
4141
try {
4242
val input = StreamingJsonDecoder(json, WriteMode.OBJ, lexer, deserializer.descriptor, null)
4343
val result = input.decodeSerializableValue(deserializer)
@@ -56,7 +56,7 @@ public fun <T> decodeToSequenceByReader(
5656
deserializer: DeserializationStrategy<T>,
5757
format: DecodeSequenceMode = DecodeSequenceMode.AUTO_DETECT
5858
): Sequence<T> {
59-
val lexer = ReaderJsonLexer(reader, CharArray(BATCH_SIZE)) // Unpooled buffer due to lazy nature of sequence
59+
val lexer = ReaderJsonLexer(json, reader, CharArray(BATCH_SIZE)) // Unpooled buffer due to lazy nature of sequence
6060
val iter = JsonIterator(format, json, lexer, deserializer)
6161
return Sequence { iter }.constrainOnce()
6262
}

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -359,7 +359,7 @@ public fun <T> decodeStringToJsonTree(
359359
deserializer: DeserializationStrategy<T>,
360360
source: String
361361
): JsonElement {
362-
val lexer = StringJsonLexer(source)
362+
val lexer = StringJsonLexer(json, source)
363363
val input = StreamingJsonDecoder(json, WriteMode.OBJ, lexer, deserializer.descriptor, null)
364364
val tree = input.decodeJsonElement()
365365
lexer.expectEof()

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

+6-3
Original file line numberDiff line numberDiff line change
@@ -153,9 +153,12 @@ private sealed class AbstractJsonTreeDecoder(
153153
return this as? JsonLiteral ?: throw JsonDecodingException(-1, "Unexpected 'null' literal when non-nullable $type was expected")
154154
}
155155

156-
override fun decodeTaggedInline(tag: String, inlineDescriptor: SerialDescriptor): Decoder =
157-
if (inlineDescriptor.isUnsignedNumber) JsonDecoderForUnsignedTypes(StringJsonLexer(getPrimitiveValue(tag).content), json)
158-
else super.decodeTaggedInline(tag, inlineDescriptor)
156+
override fun decodeTaggedInline(tag: String, inlineDescriptor: SerialDescriptor): Decoder {
157+
return if (inlineDescriptor.isUnsignedNumber) {
158+
val lexer = StringJsonLexer(json, getPrimitiveValue(tag).content)
159+
JsonDecoderForUnsignedTypes(lexer, json)
160+
} else super.decodeTaggedInline(tag, inlineDescriptor)
161+
}
159162

160163
override fun decodeInline(descriptor: SerialDescriptor): Decoder {
161164
return if (currentTagOrNull != null) super.decodeInline(descriptor)

0 commit comments

Comments
 (0)