Change name of ThrowAwayNewlineWrapper to better indicate what it does.

Simplify? the regex to determine if newline should be kept or not (removes sonar warning)
Ekryd · Feb 20, 2024 · f28bf8f · f28bf8f
1 parent 5e57e8b
commit f28bf8f
Show file tree

Hide file tree

Showing 9 changed files with 65 additions and 46 deletions.
diff --git a/sorter/src/main/java/sortpom/content/NewlineText.java b/sorter/src/main/java/sortpom/content/NewlineText.java
@@ -3,8 +3,8 @@
 import org.dom4j.tree.AbstractText;
 
 /**
- * The NewlineText is not really a special case of text. The special handling of NewlineText is done
- * in XmlProcessor.PatchedXMLWriter
+ * The NewlineText is not really a special case of text, it is a placeholder that we want to keep a
+ * newline i the pom. The special handling of NewlineText is done in XmlProcessor.PatchedXMLWriter
  */
 public class NewlineText extends AbstractText {
   private static final long serialVersionUID = -7552189498553321263L;

diff --git a/sorter/src/main/java/sortpom/wrapper/TextWrapperCreator.java b/sorter/src/main/java/sortpom/wrapper/TextWrapperCreator.java
@@ -1,11 +1,13 @@
 package sortpom.wrapper;
 
+import static sortpom.wrapper.content.ThrowAwayNewlineWrapper.THROW_AWAY_NEWLINE_INSTANCE;
+import static sortpom.wrapper.content.UnsortedWrapper.*;
+
 import java.util.regex.Pattern;
 import org.dom4j.Element;
 import org.dom4j.Node;
 import org.dom4j.Text;
 import sortpom.parameter.PluginParameters;
-import sortpom.wrapper.content.SingleNewlineInTextWrapper;
 import sortpom.wrapper.content.UnsortedWrapper;
 import sortpom.wrapper.content.Wrapper;
 
@@ -15,25 +17,18 @@
  */
 public class TextWrapperCreator {
 
-  public static final Pattern REGEX_ONE_NEWLINE =
-      Pattern.compile("^[\\t ]*(\\r|\\n|\\r\\n)[\\t ]*$");
-  public static final Pattern REGEX_ONE_OR_MORE_NEWLINE = Pattern.compile("^\\s*?([\\r\\n])\\s*$");
+  public static final Pattern REGEX_ONE_OR_MORE_NEWLINE = Pattern.compile("(\\r\\n|\\r|\\n)+?");
   private boolean keepBlankLines;
 
   public void setup(PluginParameters pluginParameters) {
     keepBlankLines = pluginParameters.keepBlankLines;
   }
 
   Wrapper<Node> createWrapper(Text text) {
-    if (isElementSpacePreserved(text.getParent())) {
+    if (!text.getText().isBlank() || isElementSpacePreserved(text.getParent())) {
       return new UnsortedWrapper<>(text);
     }
-    if (isSingleNewLine(text)) {
-      return SingleNewlineInTextWrapper.INSTANCE;
-    } else if (isBlankLineOrLines(text)) {
-      return UnsortedWrapper.NEWLINE_TEXT_WRAPPER_INSTANCE;
-    }
-    return new UnsortedWrapper<>(text);
+    return blankTextNode(text);
   }
 
   boolean isElementSpacePreserved(Element element) {
@@ -47,14 +42,17 @@ boolean isElementSpacePreserved(Element element) {
         && "preserve".equals(attr.getText());
   }
 
-  private boolean isSingleNewLine(Text content) {
-    return REGEX_ONE_NEWLINE.matcher(content.getText()).matches();
-  }
-
-  boolean isBlankLineOrLines(Text content) {
+  Wrapper<Node> blankTextNode(Text text) {
     if (!keepBlankLines) {
-      return false;
+      return THROW_AWAY_NEWLINE_INSTANCE;
+    }
+    var textContent = text.getText();
+    var newLineCount = REGEX_ONE_OR_MORE_NEWLINE.matcher(textContent).results().count();
+    if (newLineCount <= 1) {
+      // One newline is just the linebreak between two XML elements
+      return THROW_AWAY_NEWLINE_INSTANCE;
     }
-    return REGEX_ONE_OR_MORE_NEWLINE.matcher(content.getText()).matches();
+    // Multiple linebreaks between XML elements indicate that a newline should be kept
+    return KEEP_NEWLINE_INSTANCE;
   }
 }
diff --git a/...r/content/SingleNewlineInTextWrapper.java → ...pper/content/ThrowAwayNewlineWrapper.java b/...r/content/SingleNewlineInTextWrapper.java → ...pper/content/ThrowAwayNewlineWrapper.java
@@ -8,8 +8,8 @@
  *
  * @author Bjorn
  */
-public enum SingleNewlineInTextWrapper implements Wrapper<Node> {
-  INSTANCE;
+public enum ThrowAwayNewlineWrapper implements Wrapper<Node> {
+  THROW_AWAY_NEWLINE_INSTANCE;
 
   @Override
   public Text getContent() {

diff --git a/sorter/src/main/java/sortpom/wrapper/content/UnsortedWrapper.java b/sorter/src/main/java/sortpom/wrapper/content/UnsortedWrapper.java
@@ -6,7 +6,7 @@
 
 /** A wrapper that lets is element be unsorted */
 public class UnsortedWrapper<T extends Node> implements Wrapper<T> {
-  public static final UnsortedWrapper<Node> NEWLINE_TEXT_WRAPPER_INSTANCE =
+  public static final UnsortedWrapper<Node> KEEP_NEWLINE_INSTANCE =
       new UnsortedWrapper<>(new NewlineText());
 
   /** The wrapped dom content. */

diff --git a/sorter/src/main/java/sortpom/wrapper/operation/HierarchyWrapper.java b/sorter/src/main/java/sortpom/wrapper/operation/HierarchyWrapper.java
@@ -4,7 +4,7 @@
 import java.util.List;
 import org.dom4j.Element;
 import org.dom4j.Node;
-import sortpom.wrapper.content.SingleNewlineInTextWrapper;
+import sortpom.wrapper.content.ThrowAwayNewlineWrapper;
 import sortpom.wrapper.content.Wrapper;
 
 /**
@@ -35,7 +35,7 @@ void createWrappedStructure(WrapperFactory factory) {
     HierarchyWrapper currentWrapper = null;
     for (var child : elementContent.getContent().content()) {
       Wrapper<?> wrapper = factory.create(child);
-      if (wrapper instanceof SingleNewlineInTextWrapper) {
+      if (wrapper instanceof ThrowAwayNewlineWrapper) {
         continue;
       }
       if (currentWrapper == null) {

diff --git a/sorter/src/test/java/sortpom/wrapper/TextWrapperCreatorTest.java b/sorter/src/test/java/sortpom/wrapper/TextWrapperCreatorTest.java
@@ -1,13 +1,15 @@
 package sortpom.wrapper;
 
+import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
-import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import org.dom4j.dom.DOMText;
 import org.dom4j.tree.DefaultText;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import sortpom.parameter.PluginParameters;
+import sortpom.wrapper.content.ThrowAwayNewlineWrapper;
+import sortpom.wrapper.content.UnsortedWrapper;
 
 /**
  * @author bjorn
@@ -27,19 +29,34 @@ void setup() {
 
   @Test
   void testIsEmptyLine() {
-    assertFalse(textWrapperCreator.isBlankLineOrLines(new DefaultText("\n      sortpom\n  ")));
-    assertFalse(textWrapperCreator.isBlankLineOrLines(new DefaultText("sortpom")));
-    assertTrue(textWrapperCreator.isBlankLineOrLines(new DefaultText("\n  ")));
-    assertTrue(textWrapperCreator.isBlankLineOrLines(new DefaultText("  \n  ")));
-    assertTrue(textWrapperCreator.isBlankLineOrLines(new DefaultText("  \n\n  ")));
-    assertTrue(textWrapperCreator.isBlankLineOrLines(new DefaultText("\n\n")));
-    assertTrue(textWrapperCreator.isBlankLineOrLines(new DefaultText("  \r  ")));
-    assertTrue(textWrapperCreator.isBlankLineOrLines(new DefaultText("  \r\r  ")));
-    assertTrue(textWrapperCreator.isBlankLineOrLines(new DefaultText("\r\r")));
-    assertTrue(textWrapperCreator.isBlankLineOrLines(new DefaultText("  \r\n  ")));
-    assertTrue(textWrapperCreator.isBlankLineOrLines(new DefaultText("  \r\n\r\n  ")));
-    assertTrue(textWrapperCreator.isBlankLineOrLines(new DefaultText("\r\n\r\n")));
-    assertFalse(textWrapperCreator.isBlankLineOrLines(new DefaultText("  ")));
+    assertKeepNewline("\n      \n  ");
+    assertKeepNewline("  \n\n  ");
+    assertKeepNewline("\n\n");
+    assertKeepNewline("\n\n\n");
+    assertKeepNewline("\n\n\n\n\n\n\n");
+    assertKeepNewline("  \r\r  ");
+    assertKeepNewline("\r\r");
+    assertKeepNewline("  \r\n\r\n  ");
+    assertKeepNewline("\r\n\r\n");
+
+    assertNoSpecialNewline("");
+    assertNoSpecialNewline("\n  ");
+    assertNoSpecialNewline("  \n  ");
+    assertNoSpecialNewline("  \r  ");
+    assertNoSpecialNewline("  \r\n  ");
+    assertNoSpecialNewline("  ");
+  }
+
+  private void assertKeepNewline(String text) {
+    assertEquals(
+        UnsortedWrapper.KEEP_NEWLINE_INSTANCE,
+        textWrapperCreator.blankTextNode(new DefaultText(text)));
+  }
+
+  private void assertNoSpecialNewline(String text) {
+    assertEquals(
+        ThrowAwayNewlineWrapper.THROW_AWAY_NEWLINE_INSTANCE,
+        textWrapperCreator.blankTextNode(new DefaultText(text)));
   }
 
   @Test

diff --git a/...apper/SingleNewlineInTextWrapperTest.java → .../wrapper/ThrowAwayNewlineWrapperTest.java b/...apper/SingleNewlineInTextWrapperTest.java → .../wrapper/ThrowAwayNewlineWrapperTest.java
@@ -7,7 +7,7 @@
 
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.function.Executable;
-import sortpom.wrapper.content.SingleNewlineInTextWrapper;
+import sortpom.wrapper.content.ThrowAwayNewlineWrapper;
 
 /**
  * All method should throw exception since the element should be throw away, except for the toString
@@ -16,11 +16,11 @@
  * @author bjorn
  * @since 2012-06-14
  */
-class SingleNewlineInTextWrapperTest {
+class ThrowAwayNewlineWrapperTest {
 
   @Test
   void testGetContent() {
-    Executable testMethod = SingleNewlineInTextWrapper.INSTANCE::getContent;
+    Executable testMethod = ThrowAwayNewlineWrapper.THROW_AWAY_NEWLINE_INSTANCE::getContent;
 
     var thrown = assertThrows(UnsupportedOperationException.class, testMethod);
 
@@ -29,7 +29,8 @@ void testGetContent() {
 
   @Test
   void testIsBefore() {
-    Executable testMethod = () -> SingleNewlineInTextWrapper.INSTANCE.isBefore(null);
+    Executable testMethod =
+        () -> ThrowAwayNewlineWrapper.THROW_AWAY_NEWLINE_INSTANCE.isBefore(null);
 
     var thrown = assertThrows(UnsupportedOperationException.class, testMethod);
 
@@ -38,7 +39,7 @@ void testIsBefore() {
 
   @Test
   void testIsContentElement() {
-    Executable testMethod = SingleNewlineInTextWrapper.INSTANCE::isContentElement;
+    Executable testMethod = ThrowAwayNewlineWrapper.THROW_AWAY_NEWLINE_INSTANCE::isContentElement;
 
     var thrown = assertThrows(UnsupportedOperationException.class, testMethod);
 
@@ -47,7 +48,7 @@ void testIsContentElement() {
 
   @Test
   void testIsResortable() {
-    Executable testMethod = SingleNewlineInTextWrapper.INSTANCE::isSortable;
+    Executable testMethod = ThrowAwayNewlineWrapper.THROW_AWAY_NEWLINE_INSTANCE::isSortable;
 
     var thrown = assertThrows(UnsupportedOperationException.class, testMethod);
 
@@ -57,6 +58,7 @@ void testIsResortable() {
   @Test
   void testToString() {
     assertThat(
-        SingleNewlineInTextWrapper.INSTANCE.toString("  "), is("  SingleNewlineInTextWrapper"));
+        ThrowAwayNewlineWrapper.THROW_AWAY_NEWLINE_INSTANCE.toString("  "),
+        is("  SingleNewlineInTextWrapper"));
   }
 }
diff --git a/sorter/src/test/resources/PreserveContent_input.xml b/sorter/src/test/resources/PreserveContent_input.xml
@@ -17,6 +17,7 @@
             MESSAGE!
   </message>
   <mapEnum>asEnum</mapEnum>
+          <noPreserve>  </noPreserve>
               <indentString xml:space="preserve">  </indentString>
           <noFileComment>true</noFileComment>
     <double xml:space="preserve">

diff --git a/sorter/src/test/resources/PreserveContent_output.xml b/sorter/src/test/resources/PreserveContent_output.xml
@@ -17,6 +17,7 @@
             MESSAGE!
   </message>
           <mapEnum>asEnum</mapEnum>
+          <noPreserve></noPreserve>
           <indentString xml:space="preserve">  </indentString>
           <noFileComment>true</noFileComment>
           <double xml:space="preserve">