Repository: decorators-squad/eo-yaml (java)

Issue:
Block literal scalar fails to round trip
Starting with:
```
-
  key: |
    literal
    block
    scalar
```

Produces:
```
-
  key: |
  literal
  block
  scalar
```

Which is not a valid way to produce block literal scalars - see https://www.yaml.info/learn/quote.html#literal.

Also, most of the tests assumed that block literals don't include the line separator character.

Pull request description:
Block literals don't print out in a valid way
A YamlLine only produces a trimmed version of the content available. For certain scalar values, like block literals you need to keep the spaces and tabs as they can be meaningful as listed in the issue #409 - the link to https://www.yaml.info/learn/quote.html#literal has some good examples.

For example:
```
literal: |
  line 1
   line 2
```

Should be interpreted as: "line 1\n line 2\n" (there's an extra space before "line 2").

There are also trailing spaces (to the right) that need to be maintained.

The contents method keeps the whitespace characters but keeps track of the indent - so as to remove 2 spaces per indent on the left but keep all the others.

There was a related issue where some of the tests were expecting there not to be a line separator when there should be (https://github.com/decorators-squad/eo-yaml/blob/4a28de4c419be6d2cf31185a06e9915c154a3508/src/test/java/com/amihaiemil/eoyaml/ReadLiteralBlockScalarTest.java#L60)

Fixes #409.

Patch:
diff --git a/src/main/java/com/amihaiemil/eoyaml/CachedYamlLine.java b/src/main/java/com/amihaiemil/eoyaml/CachedYamlLine.java
index 0c17524..9579aaa 100644
--- a/src/main/java/com/amihaiemil/eoyaml/CachedYamlLine.java
+++ b/src/main/java/com/amihaiemil/eoyaml/CachedYamlLine.java
@@ -47,6 +47,11 @@ final class CachedYamlLine implements YamlLine {
      */
     private String trimmed;
 
+    /**
+     * Cached contents line.
+     */
+    private String contents;
+
     /**
      * Cached indentation.
      */
@@ -78,6 +83,14 @@ final class CachedYamlLine implements YamlLine {
         return this.trimmed;
     }
 
+    @Override
+    public String contents(final int previousIndent) {
+        if(this.contents == null) {
+            this.contents = this.line.contents(previousIndent);
+        }
+        return this.contents;
+    }
+
     @Override
     public String comment() {
         return this.line.comment();
diff --git a/src/main/java/com/amihaiemil/eoyaml/Indented.java b/src/main/java/com/amihaiemil/eoyaml/Indented.java
index 053a3c6..1006c4b 100644
--- a/src/main/java/com/amihaiemil/eoyaml/Indented.java
+++ b/src/main/java/com/amihaiemil/eoyaml/Indented.java
@@ -61,6 +61,11 @@ final class Indented implements YamlLine {
         return this.original.trimmed();
     }
 
+    @Override
+    public String contents(final int previousIndent) {
+        return this.original.contents(previousIndent);
+    }
+
     @Override
     public String comment() {
         return this.original.comment();
diff --git a/src/main/java/com/amihaiemil/eoyaml/ReadLiteralBlockScalar.java b/src/main/java/com/amihaiemil/eoyaml/ReadLiteralBlockScalar.java
index 679f052..6f6e798 100644
--- a/src/main/java/com/amihaiemil/eoyaml/ReadLiteralBlockScalar.java
+++ b/src/main/java/com/amihaiemil/eoyaml/ReadLiteralBlockScalar.java
@@ -27,8 +27,6 @@
  */
 package com.amihaiemil.eoyaml;
 
-import java.util.Iterator;
-
 /**
  * Read Yaml literal block Scalar. This is a Scalar spanning multiple lines.
  * This Scalar's lines will be treated as separate lines and won't be folded
@@ -104,12 +102,10 @@ final class ReadLiteralBlockScalar extends BaseScalar {
      */
     public String value() {
         StringBuilder builder = new StringBuilder();
-        final Iterator<YamlLine> linesIt = this.significant.iterator();
-        while(linesIt.hasNext()) {
-            builder.append(linesIt.next().trimmed());
-            if(linesIt.hasNext()) {
-                builder.append(System.lineSeparator());
-            }
+        for (final YamlLine yamlLine: this.significant) {
+            int previousIndent = previous.indentation();
+            builder.append(yamlLine.contents(Math.max(previousIndent, 0)));
+            builder.append(System.lineSeparator());
         }
         return builder.toString();
     }
diff --git a/src/main/java/com/amihaiemil/eoyaml/RtYamlLine.java b/src/main/java/com/amihaiemil/eoyaml/RtYamlLine.java
index bea3bbf..8383a0b 100644
--- a/src/main/java/com/amihaiemil/eoyaml/RtYamlLine.java
+++ b/src/main/java/com/amihaiemil/eoyaml/RtYamlLine.java
@@ -27,6 +27,8 @@
  */
 package com.amihaiemil.eoyaml;
 
+import com.amihaiemil.eoyaml.exceptions.YamlReadingException;
+
 /**
  * Default implementation of {@link YamlLine}.
  * "Rt" stands for "Runtime".
@@ -81,6 +83,21 @@ final class RtYamlLine implements YamlLine {
         return trimmed.trim();
     }
 
+    @Override
+    public String contents(final int previousIndent) {
+        String contents;
+        int indentation = indentation();
+        if (indentation == 0 && previousIndent <= 0) {
+            contents = this.value;
+        } else if (indentation > previousIndent) {
+            contents = this.value.substring(previousIndent + 2);
+        } else {
+            throw new YamlReadingException("Literal must be indented "
+                    + "at least 2 spaces from previous element.");
+        }
+        return contents;
+    }
+
     @Override
     public String comment() {
         String comment = "";
@@ -148,11 +165,7 @@ final class RtYamlLine implements YamlLine {
             final String specialCharacters = ":>|-?";
             final CharSequence prevLineLastChar =
                 this.trimmed().substring(this.trimmed().length() - 1);
-            if (specialCharacters.contains(prevLineLastChar)) {
-                result = true;
-            } else {
-                result = false;
-            }
+            result = specialCharacters.contains(prevLineLastChar);
         }
         return result;
     }
diff --git a/src/main/java/com/amihaiemil/eoyaml/RtYamlPrinter.java b/src/main/java/com/amihaiemil/eoyaml/RtYamlPrinter.java
index e4a23af..7f6cb1a 100644
--- a/src/main/java/com/amihaiemil/eoyaml/RtYamlPrinter.java
+++ b/src/main/java/com/amihaiemil/eoyaml/RtYamlPrinter.java
@@ -151,7 +151,7 @@ final class RtYamlPrinter implements YamlPrinter {
                     .append(":");
             }
             if (value instanceof Scalar) {
-                this.printNode(value, false, 0);
+                this.printNode(value, false, indentation);
             } else  {
                 this.printNode(value, true, indentation + 2);
             }
@@ -211,8 +211,7 @@ final class RtYamlPrinter implements YamlPrinter {
     ) throws IOException {
         if (scalar instanceof BaseFoldedScalar) {
             final BaseFoldedScalar foldedScalar = (BaseFoldedScalar) scalar;
-            this.writer
-                    .append(">");
+            this.writer.append(">");
             if(!scalar.comment().value().isEmpty()) {
                 this.writer.append(" # ").append(scalar.comment().value());
             }
@@ -232,8 +231,7 @@ final class RtYamlPrinter implements YamlPrinter {
         } else if (scalar instanceof RtYamlScalarBuilder.BuiltLiteralBlockScalar
                 || scalar instanceof ReadLiteralBlockScalar
         ) {
-            this.writer
-                .append("|");
+            this.writer.append("|");
             if(!scalar.comment().value().isEmpty()) {
                 this.writer.append(" # ").append(scalar.comment().value());
             }
@@ -246,7 +244,7 @@ final class RtYamlPrinter implements YamlPrinter {
             this.writer.append(
                 this.indent(
                     new Escaped(scalar).value(),
-                    indentation
+                    0
                 )
             );
             if(!scalar.comment().value().isEmpty()) {
diff --git a/src/main/java/com/amihaiemil/eoyaml/YamlLine.java b/src/main/java/com/amihaiemil/eoyaml/YamlLine.java
index 9c5849b..a528b60 100644
--- a/src/main/java/com/amihaiemil/eoyaml/YamlLine.java
+++ b/src/main/java/com/amihaiemil/eoyaml/YamlLine.java
@@ -42,10 +42,18 @@ interface YamlLine extends Comparable<YamlLine> {
 
     /**
      * The line's trimmed contents with comments, aliases etc removed.
-     * @return String contents.
+     * @return Trimmed string (leading and trailing spaces) contents.
      */
     String trimmed();
 
+    /**
+     * The line's contents with spaces, tabs, etc maintained.
+     * @param previousIndent How deeply nested is the Yaml line - this is used
+     *                       to remove leading spaces.
+     * @return String line contents.
+     */
+    String contents(int previousIndent);
+
     /**
      * Return the comment, if any, from this line.
      * @return Comment of empty string.
@@ -82,6 +90,11 @@ interface YamlLine extends Comparable<YamlLine> {
             return "";
         }
 
+        @Override
+        public String contents(final int previousIndent) {
+            return "";
+        }
+
         @Override
         public String comment() {
             return "";


Test patch:
diff --git a/src/test/java/com/amihaiemil/eoyaml/CachedYamlLineTest.java b/src/test/java/com/amihaiemil/eoyaml/CachedYamlLineTest.java
index b70007d..389e45a 100644
--- a/src/test/java/com/amihaiemil/eoyaml/CachedYamlLineTest.java
+++ b/src/test/java/com/amihaiemil/eoyaml/CachedYamlLineTest.java
@@ -79,4 +79,18 @@ public final class CachedYamlLineTest {
         MatcherAssert.assertThat(cachedLine.indentation(), Matchers.is(12));
         MatcherAssert.assertThat(cachedLine.indentation(), Matchers.is(12));
     }
+
+    /**
+     * CachedIndentedLine caches contents value and doesn't recalculate it.
+     */
+    @Test
+    public void cachesContentValue() {
+        YamlLine mock = Mockito.mock(YamlLine.class);
+        Mockito.when(mock.contents(12))
+                .thenReturn("this line")
+                .thenThrow(new RuntimeException());
+        YamlLine line = new CachedYamlLine(mock);
+        MatcherAssert.assertThat(line.contents(12), Matchers.is("this line"));
+        MatcherAssert.assertThat(line.contents(12), Matchers.is("this line"));
+    }
 }
diff --git a/src/test/java/com/amihaiemil/eoyaml/ReadFoldedBlockScalarTest.java b/src/test/java/com/amihaiemil/eoyaml/ReadFoldedBlockScalarTest.java
index ccacdef..5f64b14 100644
--- a/src/test/java/com/amihaiemil/eoyaml/ReadFoldedBlockScalarTest.java
+++ b/src/test/java/com/amihaiemil/eoyaml/ReadFoldedBlockScalarTest.java
@@ -86,6 +86,9 @@ public final class ReadFoldedBlockScalarTest {
 
     /**
      * ReadPointerScalar can compare itself to other ReadPipeScalar.
+     * Literal block scalar keeps newlines, folded doesn't.
+     *
+     * @see <a href="https://yaml.org/spec/1.2/spec.html#id2773653">Block Scalar Indicators</a>
      */
     @Test
     public void comparesToReadPipeScalar() {
@@ -95,7 +98,15 @@ public final class ReadFoldedBlockScalarTest {
             new ReadFoldedBlockScalar(new AllYamlLines(lines));
         final ReadLiteralBlockScalar second =
             new ReadLiteralBlockScalar(new AllYamlLines(lines));
-        MatcherAssert.assertThat(first.compareTo(second), Matchers.is(0));
+        MatcherAssert.assertThat(
+                first.compareTo(second),
+                Matchers.is(-1));
+        MatcherAssert.assertThat(
+                new PlainStringScalar("Java").compareTo(first),
+                Matchers.is(0));
+        MatcherAssert.assertThat(
+                new PlainStringScalar("Java\n").compareTo(second),
+                Matchers.is(0));
     }
 
     /**
diff --git a/src/test/java/com/amihaiemil/eoyaml/ReadLiteralBlockScalarTest.java b/src/test/java/com/amihaiemil/eoyaml/ReadLiteralBlockScalarTest.java
index 3ea7d4d..a0fbc6f 100644
--- a/src/test/java/com/amihaiemil/eoyaml/ReadLiteralBlockScalarTest.java
+++ b/src/test/java/com/amihaiemil/eoyaml/ReadLiteralBlockScalarTest.java
@@ -56,8 +56,8 @@ public final class ReadLiteralBlockScalarTest {
             scalar.value(),
             Matchers.is(
             "First Line." + System.lineSeparator()
-                + "Second Line."+ System.lineSeparator()
-                + "Third Line."
+                + "Second Line." + System.lineSeparator()
+                + "Third Line." + System.lineSeparator()
             )
         );
     }
@@ -84,6 +84,42 @@ public final class ReadLiteralBlockScalarTest {
         MatcherAssert.assertThat(comment.yamlNode(), Matchers.is(scalar));
     }
 
+    /**
+     * ReadLiteralBlockScalar can return properly indented values.
+     */
+    @Test
+    public void handlesIndenting() {
+        final List<YamlLine> lines = new ArrayList<>();
+        lines.add(new RtYamlLine("literal: |", 0));
+        lines.add(new RtYamlLine("  line 1", 1));
+        lines.add(new RtYamlLine("   line 2", 2));
+        lines.add(new RtYamlLine("  end", 3));
+        final ReadLiteralBlockScalar scalar =
+            new ReadLiteralBlockScalar(lines.get(0), new AllYamlLines(lines));
+        MatcherAssert.assertThat(
+            scalar.value(),
+            Matchers.equalTo("line 1\n line 2\nend\n")
+        );
+    }
+
+    /**
+     * ReadLiteralBlockScalar can return properly indented
+     * values and trailing spaces are preserved.
+     */
+    @Test
+    public void handlesTrailingSpaces() {
+        final List<YamlLine> lines = new ArrayList<>();
+        lines.add(new RtYamlLine("literal: |", 0));
+        lines.add(new RtYamlLine("  trailing spaces   ", 1));
+        lines.add(new RtYamlLine("  trailing tab\t", 2));
+        final ReadLiteralBlockScalar scalar =
+            new ReadLiteralBlockScalar(lines.get(0), new AllYamlLines(lines));
+        MatcherAssert.assertThat(
+            scalar.value(),
+            Matchers.equalTo("trailing spaces   \ntrailing tab\t\n")
+        );
+    }
+
     /**
      * ReadPipeScalar can compare itself to a Mapping.
      */
@@ -110,7 +146,7 @@ public final class ReadLiteralBlockScalarTest {
         lines.add(new RtYamlLine("Third Line.", 3));
         final ReadLiteralBlockScalar scalar =
             new ReadLiteralBlockScalar(new AllYamlLines(lines));
-        RtYamlSequence seq = new RtYamlSequence(new LinkedList<YamlNode>());
+        RtYamlSequence seq = new RtYamlSequence(new LinkedList<>());
         MatcherAssert.assertThat(scalar.compareTo(seq), Matchers.lessThan(0));
     }
 
@@ -123,7 +159,7 @@ public final class ReadLiteralBlockScalarTest {
         lines.add(new RtYamlLine("Java", 1));
         final ReadLiteralBlockScalar pipeScalar =
             new ReadLiteralBlockScalar(new AllYamlLines(lines));
-        final PlainStringScalar scalar = new PlainStringScalar("Java");
+        final PlainStringScalar scalar = new PlainStringScalar("Java\n");
         MatcherAssert.assertThat(pipeScalar.compareTo(scalar), Matchers.is(0));
     }
 
diff --git a/src/test/java/com/amihaiemil/eoyaml/RtYamlInputTest.java b/src/test/java/com/amihaiemil/eoyaml/RtYamlInputTest.java
index 4b34c5c..ec1c3fa 100644
--- a/src/test/java/com/amihaiemil/eoyaml/RtYamlInputTest.java
+++ b/src/test/java/com/amihaiemil/eoyaml/RtYamlInputTest.java
@@ -588,11 +588,23 @@ public final class RtYamlInputTest {
         MatcherAssert.assertThat(
             Yaml.createYamlInput(
                 "|" + System.lineSeparator()
-              + "line1" + System.lineSeparator()
-              + "line2"
+              + "  line1" + System.lineSeparator()
+              + "  line2"
             ).readLiteralBlockScalar().value(),
-            Matchers.equalTo("line1" + System.lineSeparator() + "line2")
+            Matchers.equalTo(
+                    "line1" + System.lineSeparator()
+                            + "line2" + System.lineSeparator())
         );
+    }
+
+    /**
+     * RtYamlInput can read a folded block scalar with document
+     * start/end markers.
+     * @throws Exception If something goes wrong.
+     */
+    @Test
+    public void readsLiteralBlockScalarWithDocumentStartEnd()
+            throws Exception {
         MatcherAssert.assertThat(
             Yaml.createYamlInput(
                 "---" + System.lineSeparator()
@@ -601,7 +613,9 @@ public final class RtYamlInputTest {
               + "line2" + System.lineSeparator()
               + "..."
             ).readLiteralBlockScalar().value(),
-            Matchers.equalTo("line1" + System.lineSeparator() + "line2")
+            Matchers.equalTo(
+                    "line1" + System.lineSeparator()
+                            + "line2" + System.lineSeparator())
         );
     }
 
@@ -684,6 +698,17 @@ public final class RtYamlInputTest {
         );
     }
 
+    /**
+     * Compare two RtYamlLines by trimmed value.
+     */
+    @Test
+    public void comparesTo() {
+        RtYamlLine java = new RtYamlLine("Java", 1);
+        RtYamlLine scala = new RtYamlLine("Scala", 1);
+        MatcherAssert.assertThat(java.compareTo(scala), Matchers.lessThan(0));
+        MatcherAssert.assertThat(java.compareTo(java), Matchers.is(0));
+    }
+
     /**
      * Read a test resource file's contents.
      * @param fileName File to read.
diff --git a/src/test/java/com/amihaiemil/eoyaml/RtYamlLineTest.java b/src/test/java/com/amihaiemil/eoyaml/RtYamlLineTest.java
index b9e927b..f7c2a71 100644
--- a/src/test/java/com/amihaiemil/eoyaml/RtYamlLineTest.java
+++ b/src/test/java/com/amihaiemil/eoyaml/RtYamlLineTest.java
@@ -27,10 +27,13 @@
  */
 package com.amihaiemil.eoyaml;
 
+import com.amihaiemil.eoyaml.exceptions.YamlReadingException;
 import org.hamcrest.MatcherAssert;
 import org.hamcrest.Matchers;
 import org.junit.Ignore;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 
 /**
  * Unit tests for {@link RtYamlLine}.
@@ -39,6 +42,11 @@ import org.junit.Test;
  * @since 1.0.0
  */
 public final class RtYamlLineTest {
+    /**
+     * Expect no exceptions thrown in tests.
+     */
+    @Rule
+    public ExpectedException thrown = ExpectedException.none();
 
     /**
      * RtYamlLine knows its number.
@@ -191,4 +199,41 @@ public final class RtYamlLineTest {
         YamlLine line = new RtYamlLine("this: line", 5);
         MatcherAssert.assertThat(line.indentation(), Matchers.is(0));
     }
+
+    /**
+     * RtYamlLine ignores previous indentation if line isn't indented.
+     */
+    @Test
+    public void contentsInline() {
+        YamlLine line = new RtYamlLine("this: line ", 5);
+        MatcherAssert.assertThat(
+                line.contents(-1),
+                Matchers.equalTo("this: line "));
+    }
+
+    /**
+     * RtYamlLine throws exception if indented but previous doesn't match.
+     */
+    @Test
+    public void badIndentationForContent() {
+        thrown.expect(YamlReadingException.class);
+        thrown.expectMessage("Literal must be indented at least 2 spaces"
+                + " from previous element.");
+        YamlLine line = new RtYamlLine("  this: line", 5);
+        MatcherAssert.assertThat(
+                line.contents(4),
+                Matchers.is(0));
+    }
+
+    /**
+     * RtYamlLine if literal is indented more than the previous extract the
+     * string.
+     */
+    @Test
+    public void correctIndentationForContent() {
+        YamlLine line = new RtYamlLine("      this: line  ", 5);
+        MatcherAssert.assertThat(
+                line.contents(4),
+                Matchers.equalTo("this: line  "));
+    }
 }
diff --git a/src/test/java/com/amihaiemil/eoyaml/YamlSequencePrintTest.java b/src/test/java/com/amihaiemil/eoyaml/YamlSequencePrintTest.java
index 359e788..565ebcb 100644
--- a/src/test/java/com/amihaiemil/eoyaml/YamlSequencePrintTest.java
+++ b/src/test/java/com/amihaiemil/eoyaml/YamlSequencePrintTest.java
@@ -77,15 +77,14 @@ public final class YamlSequencePrintTest {
      */
     @Test
     public void printsBuiltYamlSequenceWithAllNodes() throws Exception {
+        Scalar block = Yaml.createYamlScalarBuilder()
+                .addLine("literal")
+                .addLine("block")
+                .addLine("scalar")
+                .buildLiteralBlockScalar();
         final YamlSequence built = Yaml.createYamlSequenceBuilder()
             .add("plain scalar")
-            .add(
-                Yaml.createYamlScalarBuilder()
-                    .addLine("literal")
-                    .addLine("block")
-                    .addLine("scalar")
-                    .buildLiteralBlockScalar()
-            )
+            .add(block)
             .add(
                 Yaml.createYamlScalarBuilder()
                     .addLine("a scalar folded")
@@ -98,6 +97,11 @@ public final class YamlSequencePrintTest {
                     .add("key", "value")
                     .build()
             )
+            .add(
+                Yaml.createYamlMappingBuilder()
+                    .add("key", block)
+                    .build()
+            )
             .add(
                 Yaml.createYamlSequenceBuilder()
                     .add("a sequence")
diff --git a/src/test/resources/printing_tests/yamlSequenceAllNodes.txt b/src/test/resources/printing_tests/yamlSequenceAllNodes.txt
index 29be0c6..1fefc76 100644
--- a/src/test/resources/printing_tests/yamlSequenceAllNodes.txt
+++ b/src/test/resources/printing_tests/yamlSequenceAllNodes.txt
@@ -9,6 +9,11 @@
   for readability
 -
   key: value
+-
+  key: |
+    literal
+    block
+    scalar
 -
   - a sequence
   - of plain scalars