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

Issue:
Duplicate first comment on read mapping
Given the following files:
```
# A comment
mapping1:
  key1: value1
```
```
mapping2:
  key2: value2
```
And the following code:
```
File file1 = new File("yaml1.yml");
File file2 = new File("yaml2.yml");
YamlMapping mappingFile1 = Yaml.createYamlInput(file1).readYamlMapping();
YamlMapping mappingFile2 = Yaml.createYamlInput(file2).readYamlMapping();
YamlMapping merged = new MergedYamlMapping(mappingFile1, mappingFile2);
System.out.println(merged);
```
yields the following result:
```
# A comment
# A comment
mapping1:
  key1: value1
mapping2:
  key2: value2
```
As you see, the first comment is duplicated.
I've encountered this issue only with nested mappings, i.e. if files contain simple scalar mappings like `key:value` comments are not duplicated. Also, only comment at the top of the file seems to be duplicated.
I'm using version 5.1.4. Did not have enough time to investigate the reason behind this, but maybe I'll look into it later.

Pull request description:
#393 Differenciate between the Document comment and the comment of the first node
Fixes #393 

In order to differenciate between the Comment of the overall document and the Comment belonging to the first node, we need to take into account the start-marker of the document (``---``). Before:

```yaml
# We did not know if this comment refered to the whole document
# or to the architects mapping.
architects:
  - mihai
  - sherif
developers:
  - mihai
  - rultor
  - pdd
```

This was causing issues when printing, the first comment was being printed two times.
Now:

```yaml
# This comment refers to the whole document, because it is before the dash line.
---
# This comment refers to the architects mapping.
architects:
  - mihai
  - sherif
developers:
  - mihai
  - rultor
  - pdd
```

Patch:
diff --git a/src/main/java/com/amihaiemil/eoyaml/FirstCommentFound.java b/src/main/java/com/amihaiemil/eoyaml/FirstCommentFound.java
index 18aaaed..1bc4940 100644
--- a/src/main/java/com/amihaiemil/eoyaml/FirstCommentFound.java
+++ b/src/main/java/com/amihaiemil/eoyaml/FirstCommentFound.java
@@ -48,13 +48,11 @@ final class FirstCommentFound implements YamlLines {
     private final YamlLines lines;
 
     /**
-     * Are we looking for inline comments?
-     * E.g. Comments after a scalar:
-     * <pre>
-     *   some: scalar # this comment
-     * </pre>
+     * We need this flag to distinguish between the comment
+     * of the overall YAML document and the comment on top
+     * of the very first node.
      */
-    private final boolean inLine;
+    private final boolean documentComment;
 
     /**
      * Ctor.
@@ -67,11 +65,12 @@ final class FirstCommentFound implements YamlLines {
     /**
      * Ctor.
      * @param lines The Yaml lines where we look for the comment.
-     * @param inLine Looking for inline comment or not?
+     * @param documentComment Are we looking for the comment of
+     *  the overall YAML?
      */
-    FirstCommentFound(final YamlLines lines, final boolean inLine) {
+    FirstCommentFound(final YamlLines lines, final boolean documentComment) {
         this.lines = lines;
-        this.inLine = inLine;
+        this.documentComment = documentComment;
     }
 
     /**
@@ -80,22 +79,11 @@ final class FirstCommentFound implements YamlLines {
      */
     @Override
     public Iterator<YamlLine> iterator() {
-        Iterator<YamlLine> iterator = this.lines.iterator();
-        if (iterator.hasNext()) {
-            final List<YamlLine> comment = new ArrayList<>();
-            while (iterator.hasNext()) {
-                YamlLine line = iterator.next();
-                if(!line.comment().isEmpty()) {
-                    if(line.trimmed().startsWith("#")) {
-                        comment.add(line);
-                    } else if(this.inLine) {
-                        comment.add(line);
-                    }
-                } else {
-                    break;
-                }
-            }
-            iterator = comment.iterator();
+        final Iterator<YamlLine> iterator;
+        if(this.documentComment) {
+            iterator = this.documentComment();
+        } else {
+            iterator = this.nodeComment();
         }
         return iterator;
     }
@@ -112,4 +100,60 @@ final class FirstCommentFound implements YamlLines {
     ) {
         return this.lines.toYamlNode(prev, guessIndentation);
     }
+
+    /**
+     * Node comment. We are taking all the comment lines
+     * from above the node and stop when we find a non-comment
+     * line or the dash line (---), which signifies the start
+     * of the document: the start of the document is the end
+     * of the first node's comment, any comment above that is
+     * the comment of the overall YAML document..
+     * @return Iterator of YamlLine.
+     */
+    private Iterator<YamlLine> nodeComment() {
+        Iterator<YamlLine> iterator = this.lines.iterator();
+        final List<YamlLine> comment = new ArrayList<>();
+        while (iterator.hasNext()) {
+            final YamlLine line = iterator.next();
+            if(!"---".equals(line.trimmed()) && !line.comment().isEmpty()) {
+                if(line.trimmed().startsWith("#")) {
+                    comment.add(line);
+                }
+            } else {
+                break;
+            }
+        }
+        iterator = comment.iterator();
+        return iterator;
+    }
+
+    /**
+     * Document comment. We are interested only in the comment-lines
+     * which are above the "---" marker (the YAML document start-marker).
+     * @return Iterator of YamlLine.
+     */
+    private Iterator<YamlLine> documentComment() {
+        final List<YamlLine> comment = new ArrayList<>();
+        Iterator<YamlLine> iterator = this.lines.iterator();
+        while (iterator.hasNext()) {
+            YamlLine line = iterator.next();
+            if("---".equals(line.trimmed())) {
+                while(iterator.hasNext()) {
+                    line = iterator.next();
+                    if(!line.comment().isEmpty()) {
+                        if(line.trimmed().startsWith("#")) {
+                            comment.add(line);
+                        }
+                    } else {
+                        break;
+                    }
+                }
+            }
+            if(comment.size() > 0) {
+                break;
+            }
+        }
+        iterator = comment.iterator();
+        return iterator;
+    }
 }
diff --git a/src/main/java/com/amihaiemil/eoyaml/ReadPlainScalar.java b/src/main/java/com/amihaiemil/eoyaml/ReadPlainScalar.java
index b63bda5..4030403 100644
--- a/src/main/java/com/amihaiemil/eoyaml/ReadPlainScalar.java
+++ b/src/main/java/com/amihaiemil/eoyaml/ReadPlainScalar.java
@@ -95,13 +95,10 @@ final class ReadPlainScalar extends BaseScalar {
             comment = new BuiltComment(this, "");
         } else {
             comment = new ReadComment(
-                new FirstCommentFound(
                     new Skip(
                         this.all,
                         line -> line.number() != this.scalar.number()
                     ),
-                    Boolean.TRUE
-                ),
                 this
             );
         }
diff --git a/src/main/java/com/amihaiemil/eoyaml/ReadYamlMapping.java b/src/main/java/com/amihaiemil/eoyaml/ReadYamlMapping.java
index 41c690a..6d99a45 100644
--- a/src/main/java/com/amihaiemil/eoyaml/ReadYamlMapping.java
+++ b/src/main/java/com/amihaiemil/eoyaml/ReadYamlMapping.java
@@ -187,6 +187,7 @@ final class ReadYamlMapping extends BaseYamlMapping {
 
     @Override
     public Comment comment() {
+        boolean documentComment = this.previous.number() < 0;
         //@checkstyle LineLength (50 lines)
         return new ReadComment(
             new Backwards(
@@ -196,7 +197,7 @@ final class ReadYamlMapping extends BaseYamlMapping {
                             this.all,
                             line -> {
                                 final boolean skip;
-                                if(this.previous.number() < 0) {
+                                if(documentComment) {
                                     if(this.significant.iterator().hasNext()) {
                                         skip = line.number() >= this.significant
                                                 .iterator().next().number();
@@ -208,12 +209,12 @@ final class ReadYamlMapping extends BaseYamlMapping {
                                 }
                                 return skip;
                             },
-                            line -> line.trimmed().startsWith("---"),
                             line -> line.trimmed().startsWith("..."),
                             line -> line.trimmed().startsWith("%"),
                             line -> line.trimmed().startsWith("!!")
                         )
-                    )
+                    ),
+                    documentComment
                 )
             ),
             this
diff --git a/src/main/java/com/amihaiemil/eoyaml/ReadYamlSequence.java b/src/main/java/com/amihaiemil/eoyaml/ReadYamlSequence.java
index 3953cd5..d25574f 100644
--- a/src/main/java/com/amihaiemil/eoyaml/ReadYamlSequence.java
+++ b/src/main/java/com/amihaiemil/eoyaml/ReadYamlSequence.java
@@ -169,6 +169,7 @@ final class ReadYamlSequence extends BaseYamlSequence {
 
     @Override
     public Comment comment() {
+        boolean documentComment = this.previous.number() < 0;
         //@checkstyle LineLength (50 lines)
         return new ReadComment(
             new Backwards(
@@ -178,7 +179,7 @@ final class ReadYamlSequence extends BaseYamlSequence {
                             this.all,
                             line -> {
                                 final boolean skip;
-                                if(this.previous.number() < 0) {
+                                if(documentComment) {
                                     if(this.significant.iterator().hasNext()) {
                                         skip = line.number() >= this.significant
                                                 .iterator().next().number();
@@ -190,12 +191,12 @@ final class ReadYamlSequence extends BaseYamlSequence {
                                 }
                                 return skip;
                             },
-                            line -> line.trimmed().startsWith("---"),
                             line -> line.trimmed().startsWith("..."),
                             line -> line.trimmed().startsWith("%"),
                             line -> line.trimmed().startsWith("!!")
                         )
-                    )
+                    ),
+                    documentComment
                 )
             ),
             this
diff --git a/src/main/java/com/amihaiemil/eoyaml/RtYamlPrinter.java b/src/main/java/com/amihaiemil/eoyaml/RtYamlPrinter.java
index 04f0c60..8fc834e 100644
--- a/src/main/java/com/amihaiemil/eoyaml/RtYamlPrinter.java
+++ b/src/main/java/com/amihaiemil/eoyaml/RtYamlPrinter.java
@@ -63,10 +63,16 @@ final class RtYamlPrinter implements YamlPrinter {
                 this.printScalar((Scalar) node, 0);
                 this.writer.append(System.lineSeparator()).append("...");
             } else if (node instanceof YamlSequence) {
-                this.printPossibleComment(node, "");
+                boolean documentComment = this.printPossibleComment(node, "");
+                if(documentComment) {
+                    this.writer.append("---").append(System.lineSeparator());
+                }
                 this.printSequence((YamlSequence) node, 0);
             } else if (node instanceof YamlMapping) {
-                this.printPossibleComment(node, "");
+                boolean documentComment = this.printPossibleComment(node, "");
+                if(documentComment) {
+                    this.writer.append("---").append(System.lineSeparator());
+                }
                 this.printMapping((YamlMapping) node, 0);
             } else if (node instanceof YamlStream) {
                 this.printStream((YamlStream) node, 0);
@@ -288,12 +294,14 @@ final class RtYamlPrinter implements YamlPrinter {
      * line.
      * @param node Node containing the Comment.
      * @param alignment Indentation.
+     * @return True if a comment was printed, false otherwise.
      * @throws IOException If any I/O problem occurs.
      */
-    private void printPossibleComment(
+    private boolean printPossibleComment(
         final YamlNode node,
         final String alignment
     ) throws IOException {
+        boolean printed = false;
         if(node != null) {
             final String com = node.comment().value();
             if (com.trim().length() != 0) {
@@ -305,8 +313,10 @@ final class RtYamlPrinter implements YamlPrinter {
                             .append(line)
                             .append(System.lineSeparator());
                 }
+                printed = true;
             }
         }
+        return printed;
     }
 
     /**


Test patch:
diff --git a/src/test/java/com/amihaiemil/eoyaml/YamlMappingCommentsPrintTest.java b/src/test/java/com/amihaiemil/eoyaml/YamlMappingCommentsPrintTest.java
index dd96327..edcdba5 100644
--- a/src/test/java/com/amihaiemil/eoyaml/YamlMappingCommentsPrintTest.java
+++ b/src/test/java/com/amihaiemil/eoyaml/YamlMappingCommentsPrintTest.java
@@ -69,7 +69,7 @@ public final class YamlMappingCommentsPrintTest {
                 Yaml.createYamlScalarBuilder()
                     .addLine("eo-yaml")
                     .buildPlainScalar("name of the project")
-            ).build("YamlMapping for test");
+            ).build("Comment of the whole document");
         System.out.println(commented);
         MatcherAssert.assertThat(
             commented.toString(),
@@ -110,7 +110,7 @@ public final class YamlMappingCommentsPrintTest {
         System.out.println(read);
         MatcherAssert.assertThat(
             read.comment().value(),
-            Matchers.equalTo("YamlMapping for test")
+            Matchers.equalTo("Comment of the whole document")
         );
         MatcherAssert.assertThat(
             read.yamlSequence("developers").comment().value(),
@@ -135,7 +135,7 @@ public final class YamlMappingCommentsPrintTest {
         ).readYamlMapping();
         MatcherAssert.assertThat(
             read.comment().value(),
-            Matchers.equalTo("YamlMapping for test")
+            Matchers.equalTo("Comment of the whole document")
         );
         MatcherAssert.assertThat(
             read.yamlSequence("developers").comment().value(),
@@ -163,6 +163,38 @@ public final class YamlMappingCommentsPrintTest {
         );
     }
 
+    /**
+     * A read YamlMapping can access its document comment, as
+     * well as the comment of the very first node.
+     * @throws Exception If something goes wrong.
+     */
+    @Test
+    public void distinguishDocumentCommentFromNodeComment() throws Exception {
+        final YamlMapping read = Yaml.createYamlInput(
+            new File(
+                "src/test/resources/mappingWithDocumentComment.yml"
+            )
+        ).readYamlMapping();
+        MatcherAssert.assertThat(
+            read.comment().value(),
+            Matchers.equalTo("Comment of the whole document")
+        );
+        MatcherAssert.assertThat(
+            read.yamlSequence("architects").comment().value(),
+            Matchers.equalTo(
+                "Architects of the project"
+                + System.lineSeparator()
+                + "Feel free to contribute"
+            )
+        );
+        MatcherAssert.assertThat(
+            read.toString(),
+            Matchers.equalTo(
+                this.readExpected("mappingWithDocumentComment.yml")
+            )
+        );
+    }
+
     /**
      * Read a test resource file's contents.
      * @param fileName File to read.
diff --git a/src/test/java/com/amihaiemil/eoyaml/YamlSequenceCommentsPrintTest.java b/src/test/java/com/amihaiemil/eoyaml/YamlSequenceCommentsPrintTest.java
index 08d321c..5dd5c16 100644
--- a/src/test/java/com/amihaiemil/eoyaml/YamlSequenceCommentsPrintTest.java
+++ b/src/test/java/com/amihaiemil/eoyaml/YamlSequenceCommentsPrintTest.java
@@ -129,6 +129,35 @@ public final class YamlSequenceCommentsPrintTest {
         );
     }
 
+    /**
+     * A read YamlSequence can access its document comment, as
+     * well as the comment of the very first node.
+     * @throws Exception If something goes wrong.
+     */
+    @Test
+    public void distinguishDocumentCommentFromNodeComment() throws Exception {
+        final YamlSequence read = Yaml.createYamlInput(
+            new File(
+                "src/test/resources/sequenceWithDocumentComment.yml"
+            )
+        ).readYamlSequence();
+        MatcherAssert.assertThat(
+            read.comment().value(),
+            Matchers.equalTo("This is the document comment")
+        );
+        MatcherAssert.assertThat(
+            read.yamlMapping(0).comment().value(),
+            Matchers.equalTo("Comment referring to the mapping bellow.")
+        );
+        System.out.println(read);
+        MatcherAssert.assertThat(
+            read.toString(),
+            Matchers.equalTo(
+                this.readExpected("sequenceWithDocumentComment.yml")
+            )
+        );
+    }
+
     /**
      * Read a test resource file's contents.
      * @param fileName File to read.
diff --git a/src/test/java/com/amihaiemil/eoyaml/extensions/MergedYamlMappingTest.java b/src/test/java/com/amihaiemil/eoyaml/extensions/MergedYamlMappingTest.java
index 4b9e8af..7f2eb64 100644
--- a/src/test/java/com/amihaiemil/eoyaml/extensions/MergedYamlMappingTest.java
+++ b/src/test/java/com/amihaiemil/eoyaml/extensions/MergedYamlMappingTest.java
@@ -229,4 +229,77 @@ public final class MergedYamlMappingTest {
             Matchers.equalTo(expected)
         );
     }
+
+    /**
+     * MergedYamlMapping can merge 2 build YamlMapping, first one
+     * with a comment on top of its first element.
+     */
+    @Test
+    public void mergesBuiltWithCommentOnFirstElement() {
+        final YamlMapping first = Yaml.createYamlMappingBuilder()
+            .add(
+                "mapping1",
+                Yaml.createYamlMappingBuilder()
+                    .add("key1", "value1")
+                    .build("A comment")
+            ).build();
+        final YamlMapping second = Yaml.createYamlMappingBuilder()
+            .add(
+                "mapping2",
+                Yaml.createYamlMappingBuilder()
+                    .add("key2", "value2")
+                    .build()
+            ).build();
+        final YamlMapping merged = new MergedYamlMapping(
+            first, second
+        );
+        final StringBuilder expected = new StringBuilder();
+        expected
+            .append("# A comment").append(System.lineSeparator())
+            .append("mapping1:").append(System.lineSeparator())
+            .append("  key1: value1").append(System.lineSeparator())
+            .append("mapping2:").append(System.lineSeparator())
+            .append("  key2: value2");
+        MatcherAssert.assertThat(
+            merged.toString(),
+            Matchers.equalTo(expected.toString())
+        );
+    }
+
+    /**
+     * MergedYamlMapping can merge 2 read YamlMapping, first one
+     * with a comment on top of its first element.
+     * @throws Exception If something goes wrong.
+     */
+    @Test
+    public void mergesReadWithCommentOnFirstElement() throws Exception {
+        final YamlMapping first = Yaml.createYamlInput(
+            new StringBuilder()
+                .append("# A comment").append(System.lineSeparator())
+                .append("mapping1:").append(System.lineSeparator())
+                .append("  key1: value1")
+                .toString()
+        ).readYamlMapping();
+        System.out.println(first);
+        final YamlMapping second = Yaml.createYamlInput(
+            new StringBuilder()
+                .append("mapping2:").append(System.lineSeparator())
+                .append("  key2: value2")
+                .toString()
+        ).readYamlMapping();
+        final YamlMapping merged = new MergedYamlMapping(
+            first, second
+        );
+        final StringBuilder expected = new StringBuilder();
+        expected
+            .append("# A comment").append(System.lineSeparator())
+            .append("mapping1:").append(System.lineSeparator())
+            .append("  key1: value1").append(System.lineSeparator())
+            .append("mapping2:").append(System.lineSeparator())
+            .append("  key2: value2");
+        MatcherAssert.assertThat(
+            merged.toString(),
+            Matchers.equalTo(expected.toString())
+        );
+    }
 }
diff --git a/src/test/resources/commentedMapping.yml b/src/test/resources/commentedMapping.yml
index 1ec478a..4a832af 100644
--- a/src/test/resources/commentedMapping.yml
+++ b/src/test/resources/commentedMapping.yml
@@ -1,4 +1,5 @@
-# YamlMapping for test
+# Comment of the whole document
+---
 architect: mihai
 # all the contributors here
 developers:
diff --git a/src/test/resources/commentedSequence.yml b/src/test/resources/commentedSequence.yml
index 0a60d20..69aa0d9 100644
--- a/src/test/resources/commentedSequence.yml
+++ b/src/test/resources/commentedSequence.yml
@@ -1,4 +1,5 @@
 # a sequence with comments
+---
 - element1
 - element2 # a plain scalar string in a sequence
 - element3
diff --git a/src/test/resources/mappingWithDocumentComment.yml b/src/test/resources/mappingWithDocumentComment.yml
new file mode 100644
index 0000000..e9258b0
--- /dev/null
+++ b/src/test/resources/mappingWithDocumentComment.yml
@@ -0,0 +1,13 @@
+# Comment of the whole document
+---
+# Architects of the project
+# Feel free to contribute
+architects:
+  - mihai
+  - sherif
+# all the contributors here
+developers:
+  - rultor
+  - salikjan
+  - sherif
+name: "eo-yaml" # name of the project
\ No newline at end of file
diff --git a/src/test/resources/multilineCommentedMapping.yml b/src/test/resources/multilineCommentedMapping.yml
index 02a04a4..470becb 100644
--- a/src/test/resources/multilineCommentedMapping.yml
+++ b/src/test/resources/multilineCommentedMapping.yml
@@ -1,4 +1,5 @@
-# YamlMapping for test
+# Comment of the whole document
+---
 architect: mihai
 # all the contributors here
 developers:
diff --git a/src/test/resources/sequenceWithDocumentComment.yml b/src/test/resources/sequenceWithDocumentComment.yml
new file mode 100644
index 0000000..d2b4731
--- /dev/null
+++ b/src/test/resources/sequenceWithDocumentComment.yml
@@ -0,0 +1,11 @@
+# This is the document comment
+---
+# Comment referring to the mapping bellow.
+-
+  key: value
+- element2 # a plain scalar string in a sequence
+- element3
+# a mapping as an element of a sequence
+-
+  key: value
+  key2: value2
\ No newline at end of file