fix LocalDeclarationOrderRule for multiple empty lines (#195)

com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/parser/Token.java

@@ -1254,6 +1254,19 @@ public final boolean setWhitespace(int lineBreaks, int spacesLeft) {
 		return true;
 	}
 
+	/**
+	 * returns true if whitespace was changed
+	 * 
+	 * @param lineBreaks
+	 * @return
+	 */
+	public final boolean setLineBreaks(int lineBreaks) {
+		if (this.lineBreaks == lineBreaks)
+			return false;
+		this.lineBreaks = lineBreaks;
+		return true;
+	}
+
 	public final boolean setSpacesLeftAdjustingIndent(int newSpacesLeft) {
 		if (newSpacesLeft == spacesLeft)
 			return false;

com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rules/declarations/LocalDeclarationOrderRule.java

@@ -429,6 +429,7 @@ private void moveSection(Section section, Command methodStart, Command parent, C
 		}
 		Token firstToken = section.firstCommand.getFirstToken(); 
 
+		boolean wasSectionMoved = false;
 		if (section.contains(writePos)) {
 			// for sections that are already in the right place, keep existing empty lines; after rearranging declarations 
 			// once, this allows for purposefully introducing extra lines which are then kept
@@ -483,7 +484,8 @@ private void moveSection(Section section, Command methodStart, Command parent, C
 			// move the section to its new place
 			section.removeFromCode();
 			writePos.insertLeftSibling(section);
-
+			wasSectionMoved = true;
+
 			code.addRuleUses(this, section);
 		}
 
@@ -494,7 +496,13 @@ private void moveSection(Section section, Command methodStart, Command parent, C
 			// if the next Command is not a declaration, always put an empty line
 			Command nextNonCommentSibling = section.lastCommand.getNextNonCommentSibling();
 			if (nextNonCommentSibling != null && !nextNonCommentSibling.isDeclaration()) {
-				lineBreaks = 2;
+				if (wasSectionMoved) {
+					// if the section was just moved in front of the next Command, put exactly one empty line 
+					lineBreaks = 2;
+				} else {
+					// if the next Command was there already, ensure an empty line, but possibly keep more 
+					lineBreaks = Math.max(nextSibling.getFirstToken().lineBreaks, 2);
+				}
 			} else {
 				// find the last declaration command of the section (.lastCommand may be a pragma line)
 				Command lastDeclaration = section.lastCommand;
@@ -695,10 +703,11 @@ private void moveTermInChain(Term term, Command declaration, Code code, HashMap<
 			// adjust whitespace of declaration after the term before it is moved
 			Token tokenAfterTerm = term.lastToken.getNext();
 			if (tokenAfterTerm != null) {
-				if (tokenAfterTerm.isAsteriskCommentLine())
+				if (tokenAfterTerm.isAsteriskCommentLine()) {
 					tokenAfterTerm.lineBreaks = 1;
-				else
+				} else {
 					tokenAfterTerm.setWhitespace(1, indent);
+				}
 			}
 
 			// if the term ends with a period, change that into a comma, and change the previous comma into a period  
@@ -724,10 +733,11 @@ private void moveTermInChain(Term term, Command declaration, Code code, HashMap<
 		// adjust whitespace of the declaration that is now behind the term
 		Token next = term.lastToken.getNext();
 		if (next != null) {
-			if (next.isAsteriskCommentLine())
+			if (next.isAsteriskCommentLine()) {
 				next.lineBreaks = 1;
-			else
+			} else {
 				next.setWhitespace(1, indent);
+			}
 		}
 	}
 

test/com.sap.adt.abapcleaner.test/src/com/sap/adt/abapcleaner/parser/TokenTest.java

@@ -1541,4 +1541,15 @@ void testOffsetWithAsterisk() {
 		// ensure that the type of the '*' token is corrected from COMMENT to OTHER_OP (see Token.addNext)
 		assertTrue(buildCommand("lv_any+4(*) = lv_other.", 1).isOtherOp());
 	}
+
+	@Test
+	void testSetLineBreaks() {
+		assertFalse(buildCommand("a = 1.", 1).setLineBreaks(0));
+		assertTrue(buildCommand("a = 1.", 1).setLineBreaks(1));
+		assertTrue(buildCommand("a = 1.", 1).setLineBreaks(2));
+
+		assertTrue(buildCommand("a" + SEP + " = 1.", 1).setLineBreaks(0));
+		assertFalse(buildCommand("a" + SEP + " = 1.", 1).setLineBreaks(1));
+		assertTrue(buildCommand("a" + SEP + " = 1.", 1).setLineBreaks(2));
+	}
 }

test/com.sap.adt.abapcleaner.test/src/com/sap/adt/abapcleaner/rules/declarations/LocalDeclarationOrderTest.java

@@ -1367,4 +1367,58 @@ void testIdentifiersWithEscapeChar() {
 
 		testRule();
 	}
-}
+
+	@Test
+	void testTwoEmptyLinesKept() {
+		// ensure that the two empty lines before the executable command are kept
+		buildSrc("METHOD any_method.");
+		buildSrc("  DATA lv_other TYPE i.");
+		buildSrc("  DATA lv_any TYPE i.");
+		buildSrc("");
+		buildSrc("");
+		buildSrc("  rv_result = lv_any + lv_other.");
+		buildSrc("ENDMETHOD.");
+
+		buildExp("METHOD any_method.");
+		buildExp("  DATA lv_any TYPE i.");
+		buildExp("  DATA lv_other TYPE i.");
+		buildExp("");
+		buildExp("");
+		buildExp("  rv_result = lv_any + lv_other.");
+		buildExp("ENDMETHOD.");
+
+		testRule();
+	}
+
+	@Test
+	void testTwoEmptyLinesInBlock() {
+		// ensure that the two empty lines after IF are kept, while only one empty line is introduced 
+		// between DATA and the next executable command
+
+		rule.configDataOrder.setEnumValue(LocalDeclarationOrder.ENCLOSING_BLOCK_CHANGE_ORDER);
+
+		buildSrc("METHOD any_method.");
+		buildSrc("  DATA lv_any TYPE i.");
+		buildSrc("");
+		buildSrc("  IF iv_any_condition = abap_true.");
+		buildSrc("");
+		buildSrc("");
+		buildSrc("    lv_any = 1.");
+		buildSrc("    rv_result = lv_any.");
+		buildSrc("  ENDIF.");
+		buildSrc("ENDMETHOD.");
+
+		buildExp("METHOD any_method.");
+		buildExp("  IF iv_any_condition = abap_true.");
+		buildExp("");
+		buildExp("");
+		buildExp("    DATA lv_any TYPE i.");
+		buildExp("");
+		buildExp("    lv_any = 1.");
+		buildExp("    rv_result = lv_any.");
+		buildExp("  ENDIF.");
+		buildExp("ENDMETHOD.");
+
+		testRule();
+	}
+}