Repository: decorators-squad/eo-yaml
Pull request #399 (2020-09-10 04:33:52)

An attempt to prevent discovering the same comment twice and not print it out.
A fix for #396.


Patch:
diff --git a/src/main/java/com/amihaiemil/eoyaml/AllYamlLines.java b/src/main/java/com/amihaiemil/eoyaml/AllYamlLines.java
index 2067a51..5953f77 100644
--- a/src/main/java/com/amihaiemil/eoyaml/AllYamlLines.java
+++ b/src/main/java/com/amihaiemil/eoyaml/AllYamlLines.java
@@ -129,7 +129,8 @@ final class AllYamlLines implements YamlLines {
         if (first.trimmed().startsWith("-")){
             node = new ReadYamlSequence(prev, this, guessIndentation);
         } else if(first.trimmed().contains(":")) {
-            node = new ReadYamlMapping(prev, this, guessIndentation);
+            node = new ReadYamlMapping(prev.number(), prev, this,
+                    guessIndentation);
         } else if(this.original().size() == 1) {
             node = new ReadPlainScalar(this, first);
         } else {
diff --git a/src/main/java/com/amihaiemil/eoyaml/FirstCommentFound.java b/src/main/java/com/amihaiemil/eoyaml/FirstCommentFound.java
index 1bc4940..ff33a72 100644
--- a/src/main/java/com/amihaiemil/eoyaml/FirstCommentFound.java
+++ b/src/main/java/com/amihaiemil/eoyaml/FirstCommentFound.java
@@ -115,7 +115,9 @@ final class FirstCommentFound implements YamlLines {
         final List<YamlLine> comment = new ArrayList<>();
         while (iterator.hasNext()) {
             final YamlLine line = iterator.next();
-            if(!"---".equals(line.trimmed()) && !line.comment().isEmpty()) {
+            boolean hasComment = !line.comment().isEmpty();
+            boolean notYamlStart = !"---".equals(line.trimmed());
+            if(notYamlStart && hasComment) {
                 if(line.trimmed().startsWith("#")) {
                     comment.add(line);
                 }
diff --git a/src/main/java/com/amihaiemil/eoyaml/ReadComment.java b/src/main/java/com/amihaiemil/eoyaml/ReadComment.java
index b9d6aaf..7b86e49 100644
--- a/src/main/java/com/amihaiemil/eoyaml/ReadComment.java
+++ b/src/main/java/com/amihaiemil/eoyaml/ReadComment.java
@@ -36,14 +36,15 @@ package com.amihaiemil.eoyaml;
 final class ReadComment implements Comment {
 
     /**
-     * Lines of this comment.
+     * Node to which this comment refers.
      */
-    private final YamlLines lines;
+    private final YamlNode node;
 
     /**
-     * Node to which this comment refers.
+     * Calculated comment.
      */
-    private final YamlNode node;
+    private final String comment;
+
 
     /**
      * Constructor.
@@ -51,8 +52,8 @@ final class ReadComment implements Comment {
      * @param node Node to which it refers.
      */
     ReadComment(final YamlLines lines, final YamlNode node) {
-        this.lines = lines;
         this.node = node;
+        this.comment = calculateComments(lines).toString().trim();
     }
 
     @Override
@@ -62,12 +63,22 @@ final class ReadComment implements Comment {
 
     @Override
     public String value() {
-        final StringBuilder comment = new StringBuilder();
-        for(final YamlLine line : this.lines) {
-            comment
-                .append(line.comment().trim())
-                .append(System.lineSeparator());
+        return this.comment;
+    }
+
+    /**
+     * Pre-compute the comment value.
+     *
+     * @param lines The lines to parse into comments.
+     * @return Comments.
+     */
+    private StringBuilder calculateComments(final YamlLines lines) {
+        final StringBuilder tmpComment = new StringBuilder();
+        for(final YamlLine line : lines) {
+            tmpComment
+                    .append(line.comment().trim())
+                    .append(System.lineSeparator());
         }
-        return comment.toString().trim();
+        return tmpComment;
     }
 }
diff --git a/src/main/java/com/amihaiemil/eoyaml/ReadYamlMapping.java b/src/main/java/com/amihaiemil/eoyaml/ReadYamlMapping.java
index 9b08d00..ec070f4 100644
--- a/src/main/java/com/amihaiemil/eoyaml/ReadYamlMapping.java
+++ b/src/main/java/com/amihaiemil/eoyaml/ReadYamlMapping.java
@@ -27,6 +27,8 @@
  */
 package com.amihaiemil.eoyaml;
 
+import static com.amihaiemil.eoyaml.YamlLine.UNKNOWN_LINE_NUMBER;
+
 import com.amihaiemil.eoyaml.exceptions.YamlReadingException;
 import java.util.*;
 import java.util.regex.Pattern;
@@ -75,6 +77,11 @@ final class ReadYamlMapping extends BaseYamlMapping {
      */
     private final boolean guessIndentation;
 
+    /**
+     * Where to stop looking for comments.
+     */
+    private final int commentStop;
+
     /**
      * Ctor.
      * @param lines Given lines.
@@ -93,30 +100,26 @@ final class ReadYamlMapping extends BaseYamlMapping {
         final AllYamlLines lines,
         final boolean guessIndentation
     ) {
-        this(new YamlLine.NullYamlLine(), lines, guessIndentation);
-    }
-
-    /**
-     * Ctor.
-     * @param previous Line just before the start of this mapping.
-     * @param lines Given lines.
-     */
-    ReadYamlMapping(final YamlLine previous, final AllYamlLines lines) {
-        this(previous, lines, Boolean.FALSE);
+        this(UNKNOWN_LINE_NUMBER, new YamlLine.NullYamlLine(), lines,
+            guessIndentation);
     }
 
     /**
      * Ctor.
+     * @checkstyle ParameterNumber (100 lines)
+     * @param commentStop Where to
      * @param previous Line just before the start of this mapping.
      * @param lines Given lines.
      * @param guessIndentation If true, we will try to guess the correct
      *  indentation of misplaced lines.
      */
     ReadYamlMapping(
+        final int commentStop,
         final YamlLine previous,
         final AllYamlLines lines,
         final boolean guessIndentation
     ) {
+        this.commentStop = commentStop;
         this.previous = previous;
         this.all = lines;
         this.significant = new SameIndentationLevel(
@@ -205,7 +208,7 @@ final class ReadYamlMapping extends BaseYamlMapping {
                                         skip = false;
                                     }
                                 } else {
-                                    skip = line.number() >= this.previous.number();
+                                    skip = line.number() >= commentStop;
                                 }
                                 return skip;
                             },
@@ -247,6 +250,7 @@ final class ReadYamlMapping extends BaseYamlMapping {
                     );
                 } else if (trimmed.matches(tryKey + ":[ ]*\\{}")) {
                     value = new EmptyYamlMapping(new ReadYamlMapping(
+                            line.number(),
                             this.all.line(line.number()),
                             this.all,
                             this.guessIndentation
diff --git a/src/main/java/com/amihaiemil/eoyaml/ReadYamlSequence.java b/src/main/java/com/amihaiemil/eoyaml/ReadYamlSequence.java
index 8e3f67f..d188d14 100644
--- a/src/main/java/com/amihaiemil/eoyaml/ReadYamlSequence.java
+++ b/src/main/java/com/amihaiemil/eoyaml/ReadYamlSequence.java
@@ -153,6 +153,7 @@ final class ReadYamlSequence extends BaseYamlSequence {
                     );
                 } else if (trimmed.matches("^-[ ]*\\{}")) {
                     kids.add(new EmptyYamlMapping(new ReadYamlMapping(
+                            line.number(),
                             this.all.line(line.number()),
                             this.all,
                             this.guessIndentation
@@ -167,7 +168,8 @@ final class ReadYamlSequence extends BaseYamlSequence {
                     if(this.mappingStartsAtDash(line)) {
                         kids.add(
                             new ReadYamlMapping(
-                                new RtYamlLine("", line.number()-1),
+                                line.number() + 1,
+                                this.all.line(line.number() - 1),
                                 this.all,
                                 this.guessIndentation
                             )
diff --git a/src/main/java/com/amihaiemil/eoyaml/RtYamlPrinter.java b/src/main/java/com/amihaiemil/eoyaml/RtYamlPrinter.java
index df6cc57..c672d9e 100644
--- a/src/main/java/com/amihaiemil/eoyaml/RtYamlPrinter.java
+++ b/src/main/java/com/amihaiemil/eoyaml/RtYamlPrinter.java
@@ -308,8 +308,9 @@ final class RtYamlPrinter implements YamlPrinter {
         final String alignment
     ) throws IOException {
         boolean printed = false;
-        if(node != null) {
-            final String com = node.comment().value();
+        if(node != null && node.comment() != null) {
+            Comment tmpComment = node.comment();
+            final String com = tmpComment.value();
             if (com.trim().length() != 0) {
                 String[] lines = com.split(System.lineSeparator());
                 for (final String line : lines) {
diff --git a/src/main/java/com/amihaiemil/eoyaml/YamlLine.java b/src/main/java/com/amihaiemil/eoyaml/YamlLine.java
index d4ef4af..9c5849b 100644
--- a/src/main/java/com/amihaiemil/eoyaml/YamlLine.java
+++ b/src/main/java/com/amihaiemil/eoyaml/YamlLine.java
@@ -34,6 +34,11 @@ package com.amihaiemil.eoyaml;
  * @since 1.0.0
  */
 interface YamlLine extends Comparable<YamlLine> {
+    /**
+     * Indicates we don't know or don't have a line number for the given
+     * comment (may not have come from a file).
+     */
+    int UNKNOWN_LINE_NUMBER = -1;
 
     /**
      * The line's trimmed contents with comments, aliases etc removed.
@@ -84,7 +89,7 @@ interface YamlLine extends Comparable<YamlLine> {
 
         @Override
         public int number() {
-            return -1;
+            return UNKNOWN_LINE_NUMBER;
         }
 
         @Override


Test patch:
diff --git a/src/test/java/com/amihaiemil/eoyaml/FirstCommentFoundTest.java b/src/test/java/com/amihaiemil/eoyaml/FirstCommentFoundTest.java
index e310e70..618d927 100644
--- a/src/test/java/com/amihaiemil/eoyaml/FirstCommentFoundTest.java
+++ b/src/test/java/com/amihaiemil/eoyaml/FirstCommentFoundTest.java
@@ -133,4 +133,20 @@ public final class FirstCommentFoundTest {
         );
         MatcherAssert.assertThat(comment, Matchers.emptyIterable());
     }
+
+    /**
+     * {@link FirstCommentFound} returns the comment when called multiple
+     *  times - check we re-parse correctly.
+     */
+    @Test
+    public void findCommentsMultipleTimes() {
+        final List<YamlLine> lines = new ArrayList<>();
+        lines.add(new RtYamlLine("# two", 0));
+        lines.add(new RtYamlLine("three: three", 1));
+        final YamlLines comment = new FirstCommentFound(
+                new AllYamlLines(lines)
+        );
+        MatcherAssert.assertThat(comment, Matchers.iterableWithSize(1));
+        MatcherAssert.assertThat(comment, Matchers.iterableWithSize(1));
+    }
 }
diff --git a/src/test/java/com/amihaiemil/eoyaml/ReadCommentTest.java b/src/test/java/com/amihaiemil/eoyaml/ReadCommentTest.java
index 19fc8dd..441cc47 100644
--- a/src/test/java/com/amihaiemil/eoyaml/ReadCommentTest.java
+++ b/src/test/java/com/amihaiemil/eoyaml/ReadCommentTest.java
@@ -62,11 +62,12 @@ public final class ReadCommentTest {
      */
     @Test
     public void returnsValueFromEmptyLines() {
+        Comment readComment = new ReadComment(
+            new AllYamlLines(new ArrayList<>()),
+            Mockito.mock(YamlNode.class)
+        );
         MatcherAssert.assertThat(
-            new ReadComment(
-                new AllYamlLines(new ArrayList<>()),
-                Mockito.mock(YamlNode.class)
-            ).value(),
+            readComment.value(),
             Matchers.isEmptyString()
         );
     }
@@ -78,11 +79,12 @@ public final class ReadCommentTest {
     public void returnsValueFromSingleLine() {
         final List<YamlLine> lines = new ArrayList<>();
         lines.add(new RtYamlLine("# comment line", 0));
+        Comment readComment = new ReadComment(
+            new AllYamlLines(lines),
+            Mockito.mock(YamlNode.class)
+        );
         MatcherAssert.assertThat(
-            new ReadComment(
-                new AllYamlLines(lines),
-                Mockito.mock(YamlNode.class)
-            ).value(),
+            readComment.value(),
             Matchers.equalTo("comment line")
         );
     }
@@ -101,11 +103,12 @@ public final class ReadCommentTest {
             .append("comment").append(System.lineSeparator())
             .append("on multiple").append(System.lineSeparator())
             .append("lines");
+        Comment readComment = new ReadComment(
+            new AllYamlLines(lines),
+            Mockito.mock(YamlNode.class)
+        );
         MatcherAssert.assertThat(
-            new ReadComment(
-                new AllYamlLines(lines),
-                Mockito.mock(YamlNode.class)
-            ).value(),
+            readComment.value(),
             Matchers.equalTo(expected.toString())
         );
     }
@@ -129,11 +132,31 @@ public final class ReadCommentTest {
             .append("on multiple").append(System.lineSeparator())
             .append(System.lineSeparator())
             .append("lines");
+        Comment readComment = new ReadComment(
+            new AllYamlLines(lines),
+            Mockito.mock(YamlNode.class)
+        );
         MatcherAssert.assertThat(
-            new ReadComment(
-                new AllYamlLines(lines),
-                Mockito.mock(YamlNode.class)
-            ).value(),
+            readComment.value(),
+            Matchers.equalTo(expected.toString())
+        );
+    }
+
+    /**
+     * ReadComment can return a comment starting on a different line.
+     */
+    @Test
+    public void singleLineCommentStartingOnTheSecondLine() {
+        final List<YamlLine> lines = new ArrayList<>();
+        lines.add(new RtYamlLine("# comment", 1));
+        final StringBuilder expected = new StringBuilder();
+        expected.append("comment");
+        Comment readComment = new ReadComment(
+            new AllYamlLines(lines),
+            Mockito.mock(YamlNode.class)
+        );
+        MatcherAssert.assertThat(
+            readComment.value(),
             Matchers.equalTo(expected.toString())
         );
     }
diff --git a/src/test/java/com/amihaiemil/eoyaml/ReflectedYamlMappingTest.java b/src/test/java/com/amihaiemil/eoyaml/ReflectedYamlMappingTest.java
index 90ebb0b..e04d099 100644
--- a/src/test/java/com/amihaiemil/eoyaml/ReflectedYamlMappingTest.java
+++ b/src/test/java/com/amihaiemil/eoyaml/ReflectedYamlMappingTest.java
@@ -107,6 +107,14 @@ public final class ReflectedYamlMappingTest {
         );
     }
 
+    /**
+     * Use the wrong object to construct a sequence - non-collection or array.
+     */
+    @Test(expected = IllegalArgumentException.class)
+    public void throwsExceptionWhenWrongObject() {
+        new ReflectedYamlMapping(new String[] {"wrong"});
+    }
+
     /**
      * Simple student pojo for test.
      * @checkstyle JavadocVariable (100 lines)
diff --git a/src/test/java/com/amihaiemil/eoyaml/ReflectedYamlSequenceTest.java b/src/test/java/com/amihaiemil/eoyaml/ReflectedYamlSequenceTest.java
index d79a9da..729bfb4 100644
--- a/src/test/java/com/amihaiemil/eoyaml/ReflectedYamlSequenceTest.java
+++ b/src/test/java/com/amihaiemil/eoyaml/ReflectedYamlSequenceTest.java
@@ -178,4 +178,12 @@ public final class ReflectedYamlSequenceTest {
             Matchers.equalTo("3")
         );
     }
+
+    /**
+     * Use the wrong object to construct a sequence - non-collection or array.
+     */
+    @Test(expected = IllegalArgumentException.class)
+    public void throwsExceptionWhenWrongObject() {
+        new ReflectedYamlSequence("wrong");
+    }
 }
diff --git a/src/test/java/com/amihaiemil/eoyaml/YamlMappingPrintTest.java b/src/test/java/com/amihaiemil/eoyaml/YamlMappingPrintTest.java
index c295d2b..8d499a2 100644
--- a/src/test/java/com/amihaiemil/eoyaml/YamlMappingPrintTest.java
+++ b/src/test/java/com/amihaiemil/eoyaml/YamlMappingPrintTest.java
@@ -323,6 +323,64 @@ public final class YamlMappingPrintTest {
         );
     }
 
+    /**
+     * Duplication of the test case described https://github.com/decorators-squad/eo-yaml/issues/396 where a sequence
+     * inside a map duplicated the comment.
+     *
+     * @throws Exception If something goes wrong.
+     */
+    @Test
+    public void printsReadYamlDuplicatedComments() throws Exception {
+        final YamlMapping read = Yaml.createYamlInput(
+                new File("src/test/resources/printing_tests/yamlDuplicatedComments.txt")
+        ).readYamlMapping();
+        MatcherAssert.assertThat(
+                read.toString(),
+                Matchers.equalTo(
+                        this.readExpected("yamlDuplicatedCommentsRoundTrip.txt")
+                )
+        );
+    }
+
+    /**
+     * When a map and sequence .
+     *
+     * @throws Exception If something goes wrong.
+     */
+    @Test
+    public void printsReadYamlCommentsOnMapOrSequence() throws Exception {
+        final YamlMapping read = Yaml.createYamlInput(
+            new File(
+                "src/test/resources/printing_tests/yamlCommentsOnMapOrSeq.txt")
+        ).readYamlMapping();
+        MatcherAssert.assertThat(
+                read.toString(),
+                Matchers.equalTo(
+                        this.readExpected("yamlCommentsOnMapOrSeqRoundTrip.txt")
+                )
+        );
+    }
+
+    /**
+     * When on separate lines we can distinguish which node to apply the
+     * comment to.
+     *
+     * @throws Exception If something goes wrong.
+     */
+    @Test
+    public void printsReadYamlCommentsOnMapAndSequence() throws Exception {
+        final YamlMapping read = Yaml.createYamlInput(
+            new File(
+                "src/test/resources/printing_tests/yamlCommentsOnMapAndSeq.txt")
+        ).readYamlMapping();
+        MatcherAssert.assertThat(
+                read.toString(),
+                Matchers.equalTo(
+                        this.readExpected(
+                                "yamlCommentsOnMapAndSeqRoundTrip.txt")
+                )
+        );
+    }
 
     /**
      * Read a test resource file's contents.
diff --git a/src/test/resources/printing_tests/yamlCommentsOnMapAndSeq.txt b/src/test/resources/printing_tests/yamlCommentsOnMapAndSeq.txt
new file mode 100644
index 0000000..17cb438
--- /dev/null
+++ b/src/test/resources/printing_tests/yamlCommentsOnMapAndSeq.txt
@@ -0,0 +1,21 @@
+title: 'Test Title'
+default_lang: de
+# Employee records
+list:
+  # On Seq
+  -
+    # On Map
+    martin:
+      name: Martin D'vloper
+      job: Developer
+      skills:
+        - python
+        - perl
+        - pascal
+  -  tabitha:
+      name: Tabitha Bitumen
+      job: Developer
+      skills:
+        - lisp
+        - fortran
+        - erlang
\ No newline at end of file
diff --git a/src/test/resources/printing_tests/yamlCommentsOnMapAndSeqRoundTrip.txt b/src/test/resources/printing_tests/yamlCommentsOnMapAndSeqRoundTrip.txt
new file mode 100644
index 0000000..f4aa0bf
--- /dev/null
+++ b/src/test/resources/printing_tests/yamlCommentsOnMapAndSeqRoundTrip.txt
@@ -0,0 +1,22 @@
+title: Test Title
+default_lang: de
+# Employee records
+list:
+  # On Seq
+  -
+    # On Map
+    martin:
+      name: Martin D'vloper
+      job: Developer
+      skills:
+        - python
+        - perl
+        - pascal
+  -
+    tabitha:
+      name: Tabitha Bitumen
+      job: Developer
+      skills:
+        - lisp
+        - fortran
+        - erlang
\ No newline at end of file
diff --git a/src/test/resources/printing_tests/yamlCommentsOnMapOrSeq.txt b/src/test/resources/printing_tests/yamlCommentsOnMapOrSeq.txt
new file mode 100644
index 0000000..b2e7afe
--- /dev/null
+++ b/src/test/resources/printing_tests/yamlCommentsOnMapOrSeq.txt
@@ -0,0 +1,19 @@
+title: 'Test Title'
+default_lang: de
+# Employee records
+list:
+  # On Map or Seq pick Map
+  - martin:
+      name: Martin D'vloper
+      job: Developer
+      skills:
+        - python
+        - perl
+        - pascal
+  -  tabitha:
+      name: Tabitha Bitumen
+      job: Developer
+      skills:
+        - lisp
+        - fortran
+        - erlang
\ No newline at end of file
diff --git a/src/test/resources/printing_tests/yamlCommentsOnMapOrSeqRoundTrip.txt b/src/test/resources/printing_tests/yamlCommentsOnMapOrSeqRoundTrip.txt
new file mode 100644
index 0000000..cdb7c44
--- /dev/null
+++ b/src/test/resources/printing_tests/yamlCommentsOnMapOrSeqRoundTrip.txt
@@ -0,0 +1,21 @@
+title: Test Title
+default_lang: de
+# Employee records
+list:
+  -
+    # On Map or Seq pick Map
+    martin:
+      name: Martin D'vloper
+      job: Developer
+      skills:
+        - python
+        - perl
+        - pascal
+  -
+    tabitha:
+      name: Tabitha Bitumen
+      job: Developer
+      skills:
+        - lisp
+        - fortran
+        - erlang
\ No newline at end of file
diff --git a/src/test/resources/printing_tests/yamlDuplicatedComments.txt b/src/test/resources/printing_tests/yamlDuplicatedComments.txt
new file mode 100644
index 0000000..81007e1
--- /dev/null
+++ b/src/test/resources/printing_tests/yamlDuplicatedComments.txt
@@ -0,0 +1,18 @@
+title: 'Test Title'
+default_lang: de
+# Employee records
+list:
+  -  martin:
+      name: Martin D'vloper
+      job: Developer
+      skills:
+        - python
+        - perl
+        - pascal
+  -  tabitha:
+      name: Tabitha Bitumen
+      job: Developer
+      skills:
+        - lisp
+        - fortran
+        - erlang
\ No newline at end of file
diff --git a/src/test/resources/printing_tests/yamlDuplicatedCommentsRoundTrip.txt b/src/test/resources/printing_tests/yamlDuplicatedCommentsRoundTrip.txt
new file mode 100644
index 0000000..ee9c862
--- /dev/null
+++ b/src/test/resources/printing_tests/yamlDuplicatedCommentsRoundTrip.txt
@@ -0,0 +1,20 @@
+title: Test Title
+default_lang: de
+# Employee records
+list:
+  -
+    martin:
+      name: Martin D'vloper
+      job: Developer
+      skills:
+        - python
+        - perl
+        - pascal
+  -
+    tabitha:
+      name: Tabitha Bitumen
+      job: Developer
+      skills:
+        - lisp
+        - fortran
+        - erlang
\ No newline at end of file