Skip to content

Commit 80f03be

Browse files
nedtwiggnedtwigg
authored
Fix facet parsing (#518)
2 parents a5073e7 + a1707bb commit 80f03be

File tree

5 files changed

+77
-50
lines changed

5 files changed

+77
-50
lines changed

‎jvm/CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
1111
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
1212

1313
## [Unreleased]
14+
### Fixed
15+
- Fixed a bug when saving facets containing keys with the `]` character ([#518](https://github.com/diffplug/selfie/pull/518))
1416

1517
## [2.4.2] - 2025-01-01
1618
### Fixed

‎jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/SnapshotFile.kt‎

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (C) 2023-2024 DiffPlug
2+
* Copyright (C) 2023-2025 DiffPlug
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -242,20 +242,20 @@ class SnapshotFile {
242242

243243
class SnapshotReader(val valueReader: SnapshotValueReader) {
244244
fun peekKey(): String? {
245-
val next = valueReader.peekKey() ?: return null
245+
val next = valueReader.peekKeyRaw() ?: return null
246246
if (next == SnapshotFile.END_OF_FILE) {
247247
return null
248248
}
249249
require(next.indexOf('[') == -1) {
250250
"Missing root snapshot, square brackets not allowed: '$next'"
251251
}
252-
return next
252+
return SnapshotValueReader.nameEsc.unescape(next)
253253
}
254254
fun nextSnapshot(): Snapshot {
255255
val rootName = peekKey()
256256
var snapshot = Snapshot.of(valueReader.nextValue())
257257
while (true) {
258-
val nextKey = valueReader.peekKey() ?: return snapshot
258+
val nextKey = valueReader.peekKeyRaw() ?: return snapshot
259259
val facetIdx = nextKey.indexOf('[')
260260
if (facetIdx == -1 || (facetIdx == 0 && nextKey == SnapshotFile.END_OF_FILE)) {
261261
return snapshot
@@ -267,7 +267,9 @@ class SnapshotReader(val valueReader: SnapshotValueReader) {
267267
val facetEndIdx = nextKey.indexOf(']', facetIdx + 1)
268268
require(facetEndIdx != -1) { "Missing ] in $nextKey" }
269269
val facetName = nextKey.substring(facetIdx + 1, facetEndIdx)
270-
snapshot = snapshot.plusFacet(facetName, valueReader.nextValue())
270+
snapshot =
271+
snapshot.plusFacet(
272+
SnapshotValueReader.nameEsc.unescape(facetName), valueReader.nextValue())
271273
}
272274
}
273275
fun skipSnapshot() {
@@ -285,15 +287,15 @@ class SnapshotValueReader(val lineReader: LineReader) {
285287
val unixNewlines = lineReader.unixNewlines()
286288

287289
/** The key of the next value, does not increment anything about the reader's state. */
288-
fun peekKey(): String? {
289-
return nextKey()
290+
fun peekKeyRaw(): String? {
291+
return nextKeyRaw()
290292
}
291293

292294
/** Reads the next value. */
293295
@OptIn(ExperimentalEncodingApi::class)
294296
fun nextValue(): SnapshotValue {
295297
// validate key
296-
nextKey()
298+
nextKeyRaw()
297299
val isBase64 = nextLine()!!.contains(FLAG_BASE64)
298300
resetLine()
299301

@@ -321,7 +323,7 @@ class SnapshotValueReader(val lineReader: LineReader) {
321323
/** Same as nextValue, but faster. */
322324
fun skipValue() {
323325
// Ignore key
324-
nextKey()
326+
nextKeyRaw()
325327
resetLine()
326328

327329
scanValue {
@@ -340,7 +342,7 @@ class SnapshotValueReader(val lineReader: LineReader) {
340342
nextLine = nextLine()
341343
}
342344
}
343-
private fun nextKey(): String? {
345+
private fun nextKeyRaw(): String? {
344346
val line = nextLine() ?: return null
345347
val startIndex = line.indexOf(KEY_START)
346348
val endIndex = line.indexOf(KEY_END)
@@ -357,7 +359,7 @@ class SnapshotValueReader(val lineReader: LineReader) {
357359
} else if (key.endsWith(" ")) {
358360
throw ParseException(lineReader, "Trailing spaces are disallowed: '$key'")
359361
} else {
360-
nameEsc.unescape(key)
362+
key
361363
}
362364
}
363365
private fun nextLine(): String? {

‎jvm/selfie-lib/src/commonTest/kotlin/com/diffplug/selfie/SnapshotFileTest.kt‎

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (C) 2023-2024 DiffPlug
2+
* Copyright (C) 2023-2025 DiffPlug
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -91,4 +91,33 @@ class SnapshotFileTest {
9191
"""
9292
.trimIndent()
9393
}
94+
95+
@Test
96+
fun escapingBug() {
97+
val file =
98+
SnapshotFile.parse(
99+
SnapshotValueReader.of(
100+
"""
101+
╔═ trialStarted/stripe ═╗
102+
103+
╔═ trialStarted/stripe[«1»{\n "params": {\n "line_items": "line_items=\({quantity=1, price=price_xxxx}\)"\n },\n "apiMode": "V1"\n}] ═╗
104+
{}
105+
╔═ [end of file] ═╗
106+
107+
"""
108+
.trimIndent()))
109+
val keys = file.snapshots.keys.toList()
110+
keys.size shouldBe 1
111+
keys[0] shouldBe "trialStarted/stripe"
112+
val snapshot = file.snapshots.get(keys[0])!!
113+
114+
snapshot.facets.keys.size shouldBe 1
115+
snapshot.facets.keys.first() shouldBe
116+
"""«1»{
117+
"params": {
118+
"line_items": "line_items=[{quantity=1, price=price_xxxx}]"
119+
},
120+
"apiMode": "V1"
121+
}"""
122+
}
94123
}

‎jvm/selfie-lib/src/commonTest/kotlin/com/diffplug/selfie/SnapshotValueReaderTest.kt‎

Lines changed: 30 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (C) 2023-2024 DiffPlug
2+
* Copyright (C) 2023-2025 DiffPlug
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -46,42 +46,42 @@ class SnapshotValueReaderTest {
4646
╔═ 05_notSureHowKotlinMultilineWorks ═╗
4747
"""
4848
.trimIndent())
49-
reader.peekKey() shouldBe "00_empty"
50-
reader.peekKey() shouldBe "00_empty"
49+
reader.peekKeyRaw() shouldBe "00_empty"
50+
reader.peekKeyRaw() shouldBe "00_empty"
5151
reader.nextValue().valueString() shouldBe ""
52-
reader.peekKey() shouldBe "01_singleLineString"
53-
reader.peekKey() shouldBe "01_singleLineString"
52+
reader.peekKeyRaw() shouldBe "01_singleLineString"
53+
reader.peekKeyRaw() shouldBe "01_singleLineString"
5454
reader.nextValue().valueString() shouldBe "this is one line"
55-
reader.peekKey() shouldBe "01a_singleLineLeadingSpace"
55+
reader.peekKeyRaw() shouldBe "01a_singleLineLeadingSpace"
5656
reader.nextValue().valueString() shouldBe " the leading space is significant"
57-
reader.peekKey() shouldBe "01b_singleLineTrailingSpace"
57+
reader.peekKeyRaw() shouldBe "01b_singleLineTrailingSpace"
5858
reader.nextValue().valueString() shouldBe "the trailing space is significant "
59-
reader.peekKey() shouldBe "02_multiLineStringTrimmed"
59+
reader.peekKeyRaw() shouldBe "02_multiLineStringTrimmed"
6060
reader.nextValue().valueString() shouldBe "Line 1\nLine 2"
6161
// note that leading and trailing newlines in the snapshots are significant
6262
// this is critical so that snapshots can accurately capture the exact number of newlines
63-
reader.peekKey() shouldBe "03_multiLineStringTrailingNewline"
63+
reader.peekKeyRaw() shouldBe "03_multiLineStringTrailingNewline"
6464
reader.nextValue().valueString() shouldBe "Line 1\nLine 2\n"
65-
reader.peekKey() shouldBe "04_multiLineStringLeadingNewline"
65+
reader.peekKeyRaw() shouldBe "04_multiLineStringLeadingNewline"
6666
reader.nextValue().valueString() shouldBe "\nLine 1\nLine 2"
67-
reader.peekKey() shouldBe "05_notSureHowKotlinMultilineWorks"
67+
reader.peekKeyRaw() shouldBe "05_notSureHowKotlinMultilineWorks"
6868
reader.nextValue().valueString() shouldBe ""
6969
}
7070

7171
@Test
7272
fun invalidNames() {
73-
shouldThrow<ParseException> { SnapshotValueReader.of("╔═name ═╗").peekKey() }
73+
shouldThrow<ParseException> { SnapshotValueReader.of("╔═name ═╗").peekKeyRaw() }
7474
.let { it.message shouldBe "L1:Expected to start with '╔═ '" }
75-
shouldThrow<ParseException> { SnapshotValueReader.of("╔═ name═╗").peekKey() }
75+
shouldThrow<ParseException> { SnapshotValueReader.of("╔═ name═╗").peekKeyRaw() }
7676
.let { it.message shouldBe "L1:Expected to contain ' ═╗'" }
77-
shouldThrow<ParseException> { SnapshotValueReader.of("╔═ name ═╗").peekKey() }
77+
shouldThrow<ParseException> { SnapshotValueReader.of("╔═ name ═╗").peekKeyRaw() }
7878
.let { it.message shouldBe "L1:Leading spaces are disallowed: ' name'" }
79-
shouldThrow<ParseException> { SnapshotValueReader.of("╔═ name ═╗").peekKey() }
79+
shouldThrow<ParseException> { SnapshotValueReader.of("╔═ name ═╗").peekKeyRaw() }
8080
.let { it.message shouldBe "L1:Trailing spaces are disallowed: 'name '" }
81-
SnapshotValueReader.of("╔═ name ═╗ comment okay").peekKey() shouldBe "name"
82-
SnapshotValueReader.of("╔═ name ═╗okay here too").peekKey() shouldBe "name"
81+
SnapshotValueReader.of("╔═ name ═╗ comment okay").peekKeyRaw() shouldBe "name"
82+
SnapshotValueReader.of("╔═ name ═╗okay here too").peekKeyRaw() shouldBe "name"
8383
SnapshotValueReader.of("╔═ name ═╗ okay ╔═ ═╗ (it's the first ' ═╗' that counts)")
84-
.peekKey() shouldBe "name"
84+
.peekKeyRaw() shouldBe "name"
8585
}
8686

8787
@Test
@@ -96,21 +96,15 @@ class SnapshotValueReaderTest {
9696
╔═ test with \┌\─ ascii art \─\┐ in name ═╗
9797
"""
9898
.trimIndent())
99-
reader.peekKey() shouldBe "test with [square brackets] in name"
99+
reader.peekKeyRaw() shouldBe "test with \\(square brackets\\) in name"
100100
reader.nextValue().valueString() shouldBe ""
101-
reader.peekKey() shouldBe """test with \backslash\ in name"""
101+
reader.peekKeyRaw() shouldBe """test with \\backslash\\ in name"""
102102
reader.nextValue().valueString() shouldBe ""
103-
reader.peekKey() shouldBe
104-
"""
105-
test with
106-
newline
107-
in name
108-
"""
109-
.trimIndent()
103+
reader.peekKeyRaw() shouldBe "test with\\nnewline\\nin name"
110104
reader.nextValue().valueString() shouldBe ""
111-
reader.peekKey() shouldBe "test with \ttab\t in name"
105+
reader.peekKeyRaw() shouldBe "test with \\ttab\\t in name"
112106
reader.nextValue().valueString() shouldBe ""
113-
reader.peekKey() shouldBe "test with ╔═ ascii art ═╗ in name"
107+
reader.peekKeyRaw() shouldBe "test with \\┌\\─ ascii art \\─\\┐ in name"
114108
reader.nextValue().valueString() shouldBe ""
115109
}
116110

@@ -127,11 +121,11 @@ class SnapshotValueReaderTest {
127121
𐝃𐝁𐝃𐝃 linear a is dead
128122
"""
129123
.trimIndent())
130-
reader.peekKey() shouldBe "ascii art okay"
124+
reader.peekKeyRaw() shouldBe "ascii art okay"
131125
reader.nextValue().valueString() shouldBe """ ╔══╗"""
132-
reader.peekKey() shouldBe "escaped iff on first line"
126+
reader.peekKeyRaw() shouldBe "escaped iff on first line"
133127
reader.nextValue().valueString() shouldBe """╔══╗"""
134-
reader.peekKey() shouldBe "body escape characters"
128+
reader.peekKeyRaw() shouldBe "body escape characters"
135129
reader.nextValue().valueString() shouldBe """𐝁𐝃 linear a is dead"""
136130
}
137131

@@ -154,12 +148,12 @@ class SnapshotValueReaderTest {
154148
}
155149
private fun assertKeyValueWithSkip(input: String, key: String, value: String) {
156150
val reader = SnapshotValueReader.of(input)
157-
while (reader.peekKey() != key) {
151+
while (reader.peekKeyRaw() != key) {
158152
reader.skipValue()
159153
}
160-
reader.peekKey() shouldBe key
154+
reader.peekKeyRaw() shouldBe key
161155
reader.nextValue().valueString() shouldBe value
162-
while (reader.peekKey() != null) {
156+
while (reader.peekKeyRaw() != null) {
163157
reader.skipValue()
164158
}
165159
}
@@ -169,7 +163,7 @@ class SnapshotValueReaderTest {
169163
val reader = SnapshotValueReader.of("""╔═ Apple ═╗ base64 length 3 bytes
170164
c2Fk
171165
""")
172-
reader.peekKey() shouldBe "Apple"
166+
reader.peekKeyRaw() shouldBe "Apple"
173167
reader.nextValue().valueBinary() shouldBe "sad".encodeToByteArray()
174168
}
175169
}

‎jvm/selfie-runner-kotest/src/commonTest/kotlin/com/diffplug/selfie/kotest/HarnessKotest.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (C) 2023-2024 DiffPlug
2+
* Copyright (C) 2023-2025 DiffPlug
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -206,7 +206,7 @@ open class HarnessKotest() : FunSpec() {
206206
argList.add("-c")
207207
}
208208
argList.add(
209-
"${if (IS_WINDOWS) "" else "./"}gradlew :undertest-kotest:$actualTask --configuration-cache ${args.joinToString(" ")}")
209+
"${if (IS_WINDOWS) "" else "./"}gradlew :undertest-kotest:$actualTask --configuration-cache --console=plain ${args.joinToString(" ")}")
210210
val output =
211211
exec(TypedPath.ofFolder(subprojectFolder.parent.toString()), *argList.toTypedArray())
212212
if (output.contains("BUILD SUCCESSFUL")) {

0 commit comments

Comments
 (0)