From fbe1b1b86aa8c4a89423711c68bab9bbc353a629 Mon Sep 17 00:00:00 2001 From: Piotr Krzeminski Date: Wed, 18 Dec 2024 23:09:23 +0100 Subject: [PATCH 1/8] [Port from snakeyaml-engine] Create a test for issue 54 --- .../issues/issue54/DumpWithoutSpaceTest.kt | 58 +++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 src/commonTest/kotlin/it/krzeminski/snakeyaml/engine/kmp/issues/issue54/DumpWithoutSpaceTest.kt diff --git a/src/commonTest/kotlin/it/krzeminski/snakeyaml/engine/kmp/issues/issue54/DumpWithoutSpaceTest.kt b/src/commonTest/kotlin/it/krzeminski/snakeyaml/engine/kmp/issues/issue54/DumpWithoutSpaceTest.kt new file mode 100644 index 000000000..f471fa5f9 --- /dev/null +++ b/src/commonTest/kotlin/it/krzeminski/snakeyaml/engine/kmp/issues/issue54/DumpWithoutSpaceTest.kt @@ -0,0 +1,58 @@ +package it.krzeminski.snakeyaml.engine.kmp.issues.issue54 + +import io.kotest.core.spec.style.FunSpec +import io.kotest.matchers.shouldNotBe +import io.kotest.matchers.string.shouldContain +import it.krzeminski.snakeyaml.engine.kmp.api.Dump +import it.krzeminski.snakeyaml.engine.kmp.api.DumpSettings +import it.krzeminski.snakeyaml.engine.kmp.api.Load +import it.krzeminski.snakeyaml.engine.kmp.api.LoadSettings + +class DumpWithoutSpaceTest : FunSpec({ + fun parse(data: String): Any? { + val loadSettings = LoadSettings.builder().setAllowRecursiveKeys(true).build(); + val load = Load(loadSettings); + return load.loadOne(data); + } + + test("The output does not include a space after the *1 alias") { + try { + val obj = parse( + """|--- &1 + |hash: + | :one: true + | :two: true + | *1: true""".trimMargin() + ) + obj shouldNotBe null + } catch (e: Exception) { + e.message shouldContain "could not find expected ':'" + } + } + + test("The output does include a space after the *1 alias") { + val obj = parse( + """|--- &1 + |hash: + | :one: true + | :two: true + | *1 : true""".trimMargin() + ) + obj shouldNotBe null + } + + test("Dump and load an alias") { + val map = mutableMapOf( + ":one" to true, + ) + map[map] = true + val dumpSettings = DumpSettings.builder().build() + val dump = Dump(dumpSettings) + val output = dump.dumpToString(map) + try { + parse(output) + } catch (e: Exception) { + e.message shouldContain "could not find expected ':'" + } + } +}) From a8d944e7de3042e958869ec34c88bf75c082ea5c Mon Sep 17 00:00:00 2001 From: Piotr Krzeminski Date: Wed, 18 Dec 2024 23:28:05 +0100 Subject: [PATCH 2/8] [Port from snakeyaml-engine] Issue 54: Insert a trailing space when alias is a simple key --- .../snakeyaml/engine/kmp/emitter/Emitter.kt | 26 +++++++++++++++---- .../engine/kmp/scanner/ScannerImpl.kt | 12 +++++---- .../issues/issue54/DumpWithoutSpaceTest.kt | 22 ++++++++++------ .../engine/v2/emitter/EmitterTest.java | 17 ++++-------- 4 files changed, 47 insertions(+), 30 deletions(-) diff --git a/src/commonMain/kotlin/it/krzeminski/snakeyaml/engine/kmp/emitter/Emitter.kt b/src/commonMain/kotlin/it/krzeminski/snakeyaml/engine/kmp/emitter/Emitter.kt index 75483930e..911895197 100644 --- a/src/commonMain/kotlin/it/krzeminski/snakeyaml/engine/kmp/emitter/Emitter.kt +++ b/src/commonMain/kotlin/it/krzeminski/snakeyaml/engine/kmp/emitter/Emitter.kt @@ -352,11 +352,11 @@ class Emitter( simpleKeyContext = simpleKey when (event?.eventId) { Event.ID.Alias -> { - expectAlias() + expectAlias(simpleKey) } Event.ID.Scalar, Event.ID.SequenceStart, Event.ID.MappingStart -> { - processAnchor("&") + processAnchor() processTag() handleNodeEvent(event!!.eventId) } @@ -396,9 +396,12 @@ class Emitter( } } - private fun expectAlias() { + /** + * @param simpleKey true when this is the alias for a simple key + */ + private fun expectAlias(simpleKey: Boolean) { state = if (event is AliasEvent) { - processAnchor("*") + processAlias(simpleKey) states.removeLast() } else { throw EmitterException("Expecting Alias.") @@ -782,7 +785,7 @@ class Emitter( //region Anchor, Tag, and Scalar processors. - private fun processAnchor(indicator: String) { + private fun processAnchorOrAlias(indicator: String, trailingWhitespace: Boolean) { val ev = event as NodeEvent val anchor: Anchor? = ev.anchor if (anchor != null) { @@ -792,6 +795,19 @@ class Emitter( writeIndicator(indicator = indicator + anchor, needWhitespace = true) } preparedAnchor = null + if (trailingWhitespace) { + writeWhitespace(1) + } + } + + private fun processAnchor() { + // no need for trailing space + processAnchorOrAlias("&", false) + } + + private fun processAlias(simpleKey: Boolean) { + // because of ':' it needs to add trailing space for simple keys + processAnchorOrAlias("*", simpleKey) } /** diff --git a/src/commonMain/kotlin/it/krzeminski/snakeyaml/engine/kmp/scanner/ScannerImpl.kt b/src/commonMain/kotlin/it/krzeminski/snakeyaml/engine/kmp/scanner/ScannerImpl.kt index e9b181385..f0ad7fa46 100644 --- a/src/commonMain/kotlin/it/krzeminski/snakeyaml/engine/kmp/scanner/ScannerImpl.kt +++ b/src/commonMain/kotlin/it/krzeminski/snakeyaml/engine/kmp/scanner/ScannerImpl.kt @@ -1279,11 +1279,10 @@ class ScannerImpl( /** * The YAML 1.2 specification does not restrict characters for anchors and - * aliases. This may lead to problems, see - * [issue 485](https://bitbucket.org/snakeyaml/snakeyaml/issues/485/alias-names-are-too-permissive-compared-to) + * aliases. This may lead to problems. * - * This implementation tries to follow - * [RFC-0003](https://github.com/yaml/yaml-spec/blob/master/rfc/RFC-0003.md) + * See [alias naming](https://github.com/yaml/libyaml/issues/205#issuecomment-693634465) + * See also [issue 485](https://bitbucket.org/snakeyaml/snakeyaml/issues/485/alias-names-are-too-permissive-compared-to) */ private fun scanAnchor(isAnchor: Boolean): Token { val startMark = reader.getMark() @@ -1292,7 +1291,10 @@ class ScannerImpl( reader.forward() var length = 0 var c = reader.peek(length) - // Anchor may not contain ",[]{}" + // The future implementation may try to follow RFC-0003: + // https://github.com/yaml/yaml-spec/blob/master/rfc/RFC-0003.md + // and exclude also ':' (colon) + // Anchor may not contain ,[]{}/.*& while (CharConstants.NULL_BL_T_LINEBR.hasNo(c, ",[]{}/.*&")) { length++ c = reader.peek(length) diff --git a/src/commonTest/kotlin/it/krzeminski/snakeyaml/engine/kmp/issues/issue54/DumpWithoutSpaceTest.kt b/src/commonTest/kotlin/it/krzeminski/snakeyaml/engine/kmp/issues/issue54/DumpWithoutSpaceTest.kt index f471fa5f9..702aad6ec 100644 --- a/src/commonTest/kotlin/it/krzeminski/snakeyaml/engine/kmp/issues/issue54/DumpWithoutSpaceTest.kt +++ b/src/commonTest/kotlin/it/krzeminski/snakeyaml/engine/kmp/issues/issue54/DumpWithoutSpaceTest.kt @@ -1,6 +1,8 @@ package it.krzeminski.snakeyaml.engine.kmp.issues.issue54 +import io.kotest.assertions.fail import io.kotest.core.spec.style.FunSpec +import io.kotest.matchers.shouldBe import io.kotest.matchers.shouldNotBe import io.kotest.matchers.string.shouldContain import it.krzeminski.snakeyaml.engine.kmp.api.Dump @@ -8,6 +10,9 @@ import it.krzeminski.snakeyaml.engine.kmp.api.DumpSettings import it.krzeminski.snakeyaml.engine.kmp.api.Load import it.krzeminski.snakeyaml.engine.kmp.api.LoadSettings +/** + * Issue 54: add a space after anchor (when it is a simple key) + */ class DumpWithoutSpaceTest : FunSpec({ fun parse(data: String): Any? { val loadSettings = LoadSettings.builder().setAllowRecursiveKeys(true).build(); @@ -15,16 +20,16 @@ class DumpWithoutSpaceTest : FunSpec({ return load.loadOne(data); } - test("The output does not include a space after the *1 alias") { + test("The document does not have a space after the *1 alias") { try { - val obj = parse( + parse( """|--- &1 |hash: | :one: true | :two: true | *1: true""".trimMargin() ) - obj shouldNotBe null + fail("Shouldn't reach here!") } catch (e: Exception) { e.message shouldContain "could not find expected ':'" } @@ -49,10 +54,11 @@ class DumpWithoutSpaceTest : FunSpec({ val dumpSettings = DumpSettings.builder().build() val dump = Dump(dumpSettings) val output = dump.dumpToString(map) - try { - parse(output) - } catch (e: Exception) { - e.message shouldContain "could not find expected ':'" - } + output shouldBe """|&id001 + |:one: true + |*id001 : true + |""".trimMargin() + val recursive = parse(output) + recursive shouldNotBe null } }) diff --git a/src/jvmTest/java/org/snakeyaml/engine/v2/emitter/EmitterTest.java b/src/jvmTest/java/org/snakeyaml/engine/v2/emitter/EmitterTest.java index f7801a872..1cc829b21 100644 --- a/src/jvmTest/java/org/snakeyaml/engine/v2/emitter/EmitterTest.java +++ b/src/jvmTest/java/org/snakeyaml/engine/v2/emitter/EmitterTest.java @@ -256,7 +256,7 @@ public void testAnchorInMaps() { } @Test - @DisplayName("Expected space to separate anchor from colon") + @DisplayName("Expected space to separate alias from colon") public void testAliasAsKey() { DumpSettingsBuilder builder = DumpSettings.builder().setDefaultFlowStyle(FlowStyle.FLOW); // this is VERY BAD code @@ -266,20 +266,13 @@ public void testAliasAsKey() { f.put(f, "a"); String output = dump(builder.build(), f); - // TODO FIXME this YAML is invalid, the colon will be part of Anchor and not the separator - // key:value in the flow. - assertEquals("&id001 {*id001: a}\n", output); - Load load = new Load(); - try { - load.loadOne(output); - fail("TODO fix anchor"); - } catch (ComposerException e) { - assertTrue(e.getMessage().contains("found undefined alias id001:"), e.getMessage()); - } + assertEquals("&id001 {*id001 : a}\n", output); + Load load = new Load(LoadSettings.builder().setAllowRecursiveKeys(true).build()); + Object obj = load.loadOne(output); + assertNotNull(obj); } public static class MyDumperWriter extends StringWriter implements StreamDataWriter { - } @Test From 4a41e78c84f519cbf09e40e0d04576f4f80d8216 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Krzemi=C5=84ski?= <3110813+krzema12@users.noreply.github.com> Date: Tue, 31 Dec 2024 08:58:31 +0100 Subject: [PATCH 3/8] Temporarily comment out an assertion --- .../engine/kmp/issues/issue54/DumpWithoutSpaceTest.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/commonTest/kotlin/it/krzeminski/snakeyaml/engine/kmp/issues/issue54/DumpWithoutSpaceTest.kt b/src/commonTest/kotlin/it/krzeminski/snakeyaml/engine/kmp/issues/issue54/DumpWithoutSpaceTest.kt index 702aad6ec..06c1b8aa8 100644 --- a/src/commonTest/kotlin/it/krzeminski/snakeyaml/engine/kmp/issues/issue54/DumpWithoutSpaceTest.kt +++ b/src/commonTest/kotlin/it/krzeminski/snakeyaml/engine/kmp/issues/issue54/DumpWithoutSpaceTest.kt @@ -43,7 +43,8 @@ class DumpWithoutSpaceTest : FunSpec({ | :two: true | *1 : true""".trimMargin() ) - obj shouldNotBe null + // REVERTME: commenting out to see if this is the source of stackoverflow + // obj shouldNotBe null } test("Dump and load an alias") { From 380ba48e182e3c395bed70cf6f08af9c645d227e Mon Sep 17 00:00:00 2001 From: Piotr Krzeminski Date: Tue, 31 Dec 2024 09:19:13 +0100 Subject: [PATCH 4/8] Revert "Temporarily comment out an assertion" This reverts commit 4a41e78c84f519cbf09e40e0d04576f4f80d8216. --- .../engine/kmp/issues/issue54/DumpWithoutSpaceTest.kt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/commonTest/kotlin/it/krzeminski/snakeyaml/engine/kmp/issues/issue54/DumpWithoutSpaceTest.kt b/src/commonTest/kotlin/it/krzeminski/snakeyaml/engine/kmp/issues/issue54/DumpWithoutSpaceTest.kt index 06c1b8aa8..702aad6ec 100644 --- a/src/commonTest/kotlin/it/krzeminski/snakeyaml/engine/kmp/issues/issue54/DumpWithoutSpaceTest.kt +++ b/src/commonTest/kotlin/it/krzeminski/snakeyaml/engine/kmp/issues/issue54/DumpWithoutSpaceTest.kt @@ -43,8 +43,7 @@ class DumpWithoutSpaceTest : FunSpec({ | :two: true | *1 : true""".trimMargin() ) - // REVERTME: commenting out to see if this is the source of stackoverflow - // obj shouldNotBe null + obj shouldNotBe null } test("Dump and load an alias") { From 0c8de6ccb6da4ae49ba844337f00fb84e1236e4c Mon Sep 17 00:00:00 2001 From: Piotr Krzeminski Date: Tue, 31 Dec 2024 09:21:36 +0100 Subject: [PATCH 5/8] Do not use kotest's shouldNotBe null to resolve StackOverflow --- .../engine/kmp/issues/issue54/DumpWithoutSpaceTest.kt | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/commonTest/kotlin/it/krzeminski/snakeyaml/engine/kmp/issues/issue54/DumpWithoutSpaceTest.kt b/src/commonTest/kotlin/it/krzeminski/snakeyaml/engine/kmp/issues/issue54/DumpWithoutSpaceTest.kt index 702aad6ec..0f20ef24a 100644 --- a/src/commonTest/kotlin/it/krzeminski/snakeyaml/engine/kmp/issues/issue54/DumpWithoutSpaceTest.kt +++ b/src/commonTest/kotlin/it/krzeminski/snakeyaml/engine/kmp/issues/issue54/DumpWithoutSpaceTest.kt @@ -43,7 +43,11 @@ class DumpWithoutSpaceTest : FunSpec({ | :two: true | *1 : true""".trimMargin() ) - obj shouldNotBe null + if (obj == null) { + // Kotest's 'shouldNotBe null' induces a StackOverflow: + // https://github.com/krzema12/snakeyaml-engine-kmp/actions/runs/12549424377/job/34990474592 + fail("The object shouldn't be null!") + } } test("Dump and load an alias") { From 48ed73a0b0fa6dddff1f99b36d5f00b69ba3967e Mon Sep 17 00:00:00 2001 From: Piotr Krzeminski Date: Tue, 31 Dec 2024 09:24:32 +0100 Subject: [PATCH 6/8] Use shouldNotBeNull --- .../engine/kmp/issues/issue54/DumpWithoutSpaceTest.kt | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/commonTest/kotlin/it/krzeminski/snakeyaml/engine/kmp/issues/issue54/DumpWithoutSpaceTest.kt b/src/commonTest/kotlin/it/krzeminski/snakeyaml/engine/kmp/issues/issue54/DumpWithoutSpaceTest.kt index 0f20ef24a..8c62e1685 100644 --- a/src/commonTest/kotlin/it/krzeminski/snakeyaml/engine/kmp/issues/issue54/DumpWithoutSpaceTest.kt +++ b/src/commonTest/kotlin/it/krzeminski/snakeyaml/engine/kmp/issues/issue54/DumpWithoutSpaceTest.kt @@ -2,6 +2,7 @@ package it.krzeminski.snakeyaml.engine.kmp.issues.issue54 import io.kotest.assertions.fail import io.kotest.core.spec.style.FunSpec +import io.kotest.matchers.nulls.shouldNotBeNull import io.kotest.matchers.shouldBe import io.kotest.matchers.shouldNotBe import io.kotest.matchers.string.shouldContain @@ -43,11 +44,7 @@ class DumpWithoutSpaceTest : FunSpec({ | :two: true | *1 : true""".trimMargin() ) - if (obj == null) { - // Kotest's 'shouldNotBe null' induces a StackOverflow: - // https://github.com/krzema12/snakeyaml-engine-kmp/actions/runs/12549424377/job/34990474592 - fail("The object shouldn't be null!") - } + obj.shouldNotBeNull() } test("Dump and load an alias") { From 7c1f7e8ee23dc520eec3462e309f0b004a5695e2 Mon Sep 17 00:00:00 2001 From: Piotr Krzeminski Date: Tue, 31 Dec 2024 09:27:38 +0100 Subject: [PATCH 7/8] One more replacement to shouldNotBeNull --- .../snakeyaml/engine/kmp/issues/issue54/DumpWithoutSpaceTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/commonTest/kotlin/it/krzeminski/snakeyaml/engine/kmp/issues/issue54/DumpWithoutSpaceTest.kt b/src/commonTest/kotlin/it/krzeminski/snakeyaml/engine/kmp/issues/issue54/DumpWithoutSpaceTest.kt index 8c62e1685..d80c27cc8 100644 --- a/src/commonTest/kotlin/it/krzeminski/snakeyaml/engine/kmp/issues/issue54/DumpWithoutSpaceTest.kt +++ b/src/commonTest/kotlin/it/krzeminski/snakeyaml/engine/kmp/issues/issue54/DumpWithoutSpaceTest.kt @@ -60,6 +60,6 @@ class DumpWithoutSpaceTest : FunSpec({ |*id001 : true |""".trimMargin() val recursive = parse(output) - recursive shouldNotBe null + recursive.shouldNotBeNull() } }) From 1faf96c932e272abc1b6d820cf004b322ade77e6 Mon Sep 17 00:00:00 2001 From: Piotr Krzeminski Date: Tue, 31 Dec 2024 10:13:00 +0100 Subject: [PATCH 8/8] Use shouldThrow --- .../engine/kmp/issues/issue54/DumpWithoutSpaceTest.kt | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/commonTest/kotlin/it/krzeminski/snakeyaml/engine/kmp/issues/issue54/DumpWithoutSpaceTest.kt b/src/commonTest/kotlin/it/krzeminski/snakeyaml/engine/kmp/issues/issue54/DumpWithoutSpaceTest.kt index d80c27cc8..46fd0e5b5 100644 --- a/src/commonTest/kotlin/it/krzeminski/snakeyaml/engine/kmp/issues/issue54/DumpWithoutSpaceTest.kt +++ b/src/commonTest/kotlin/it/krzeminski/snakeyaml/engine/kmp/issues/issue54/DumpWithoutSpaceTest.kt @@ -1,10 +1,9 @@ package it.krzeminski.snakeyaml.engine.kmp.issues.issue54 -import io.kotest.assertions.fail +import io.kotest.assertions.throwables.shouldThrow import io.kotest.core.spec.style.FunSpec import io.kotest.matchers.nulls.shouldNotBeNull import io.kotest.matchers.shouldBe -import io.kotest.matchers.shouldNotBe import io.kotest.matchers.string.shouldContain import it.krzeminski.snakeyaml.engine.kmp.api.Dump import it.krzeminski.snakeyaml.engine.kmp.api.DumpSettings @@ -22,7 +21,7 @@ class DumpWithoutSpaceTest : FunSpec({ } test("The document does not have a space after the *1 alias") { - try { + shouldThrow { parse( """|--- &1 |hash: @@ -30,9 +29,8 @@ class DumpWithoutSpaceTest : FunSpec({ | :two: true | *1: true""".trimMargin() ) - fail("Shouldn't reach here!") - } catch (e: Exception) { - e.message shouldContain "could not find expected ':'" + }.also { + it.message shouldContain "could not find expected ':'" } }