Bug 546391 - Fix on/off tags

Change-Id: If23ac8f7777c5088b892a31a543741c244dd1a4a
Signed-off-by: Marco Stornelli <marco.stornelli@gmail.com>

core/org.eclipse.cdt.core/src/org/eclipse/cdt/internal/formatter/CodeFormatterVisitor.java

@@ -439,7 +439,7 @@ public class CodeFormatterVisitor extends ASTVisitor implements ICPPASTVisitor,
 
 		localScanner.setSource(compilationUnitSource);
 		scribe.initializeScanner(compilationUnitSource);
-		List<Position> inactive = collectInactiveCodePositions(unit);
+		List<InactivePosition> inactive = collectInactiveCodePositions(unit);
 		inactive.addAll(collectNoFormatCodePositions(unit));
 		scribe.setSkipInactivePositions(inactive);
 
@@ -4530,7 +4530,7 @@ public class CodeFormatterVisitor extends ASTVisitor implements ICPPASTVisitor,
 	 * @param translationUnit  the {@link IASTTranslationUnit}, may be <code>null</code>
 	 * @return a {@link List} of {@link Position}s
 	 */
-	private List<Position> collectNoFormatCodePositions(IASTTranslationUnit translationUnit) {
+	private List<InactivePosition> collectNoFormatCodePositions(IASTTranslationUnit translationUnit) {
 		if (translationUnit == null || !this.preferences.use_fomatter_comment_tag) {
 			return Collections.emptyList();
 		}
@@ -4538,7 +4538,7 @@ public class CodeFormatterVisitor extends ASTVisitor implements ICPPASTVisitor,
 		if (fileName == null) {
 			return Collections.emptyList();
 		}
-		List<Position> positions = new ArrayList<>();
+		List<InactivePosition> positions = new ArrayList<>();
 		int inactiveCodeStart = -1;
 		boolean inInactiveCode = false;
 
@@ -4569,14 +4569,15 @@ public class CodeFormatterVisitor extends ASTVisitor implements ICPPASTVisitor,
 				}
 			} else if (onPos != -1 && onPos > offPos) {
 				if (inInactiveCode) {
-					int inactiveCodeEnd = nodeLocation.getNodeOffset();
+					int inactiveCodeEnd = nodeLocation.getNodeOffset() + nodeLocation.getNodeLength();
-					positions.add(new Position(inactiveCodeStart, inactiveCodeEnd - inactiveCodeStart));
+					positions.add(new InactivePosition(inactiveCodeStart, inactiveCodeEnd - inactiveCodeStart, false));
 				}
 				inInactiveCode = false;
 			}
 		}
 		if (inInactiveCode) {
-			positions.add(new Position(inactiveCodeStart, translationUnit.getFileLocation().getNodeLength()));
+			positions.add(
+					new InactivePosition(inactiveCodeStart, translationUnit.getFileLocation().getNodeLength(), false));
 			inInactiveCode = false;
 		}
 		return positions;
@@ -4589,7 +4590,7 @@ public class CodeFormatterVisitor extends ASTVisitor implements ICPPASTVisitor,
 	 * @param translationUnit  the {@link IASTTranslationUnit}, may be <code>null</code>
 	 * @return a {@link List} of {@link Position}s
 	 */
-	private static List<Position> collectInactiveCodePositions(IASTTranslationUnit translationUnit) {
+	private static List<InactivePosition> collectInactiveCodePositions(IASTTranslationUnit translationUnit) {
 		if (translationUnit == null) {
 			return Collections.emptyList();
 		}
@@ -4597,7 +4598,7 @@ public class CodeFormatterVisitor extends ASTVisitor implements ICPPASTVisitor,
 		if (fileName == null) {
 			return Collections.emptyList();
 		}
-		List<Position> positions = new ArrayList<>();
+		List<InactivePosition> positions = new ArrayList<>();
 		int inactiveCodeStart = -1;
 		boolean inInactiveCode = false;
 		Stack<Boolean> inactiveCodeStack = new Stack<>();
@@ -4648,7 +4649,7 @@ public class CodeFormatterVisitor extends ASTVisitor implements ICPPASTVisitor,
 					inInactiveCode = true;
 				} else if (elseStmt.taken() && inInactiveCode) {
 					int inactiveCodeEnd = nodeLocation.getNodeOffset();
-					positions.add(new Position(inactiveCodeStart, inactiveCodeEnd - inactiveCodeStart));
+					positions.add(new InactivePosition(inactiveCodeStart, inactiveCodeEnd - inactiveCodeStart, true));
 					inInactiveCode = false;
 				}
 			} else if (statement instanceof IASTPreprocessorElifStatement) {
@@ -4658,7 +4659,7 @@ public class CodeFormatterVisitor extends ASTVisitor implements ICPPASTVisitor,
 					inInactiveCode = true;
 				} else if (elifStmt.taken() && inInactiveCode) {
 					int inactiveCodeEnd = nodeLocation.getNodeOffset();
-					positions.add(new Position(inactiveCodeStart, inactiveCodeEnd - inactiveCodeStart));
+					positions.add(new InactivePosition(inactiveCodeStart, inactiveCodeEnd - inactiveCodeStart, true));
 					inInactiveCode = false;
 				}
 			} else if (statement instanceof IASTPreprocessorEndifStatement) {
@@ -4666,7 +4667,8 @@ public class CodeFormatterVisitor extends ASTVisitor implements ICPPASTVisitor,
 					boolean wasInInactiveCode = inactiveCodeStack.pop().booleanValue();
 					if (inInactiveCode && !wasInInactiveCode) {
 						int inactiveCodeEnd = nodeLocation.getNodeOffset();
-						positions.add(new Position(inactiveCodeStart, inactiveCodeEnd - inactiveCodeStart));
+						positions.add(
+								new InactivePosition(inactiveCodeStart, inactiveCodeEnd - inactiveCodeStart, true));
 					}
 					inInactiveCode = wasInInactiveCode;
 				} catch (EmptyStackException e) {

core/org.eclipse.cdt.core/src/org/eclipse/cdt/internal/formatter/InactivePosition.java

@@ -0,0 +1,47 @@
+package org.eclipse.cdt.internal.formatter;
+
+import org.eclipse.jface.text.Position;
+
+/*******************************************************************************
+ * Copyright (c) 2019 Marco Stornelli
+ *
+ * This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License 2.0
+ * which accompanies this distribution, and is available at
+ * https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ *******************************************************************************/
+public class InactivePosition extends Position {
+
+	private boolean preprocessorRegion;
+
+	public boolean isPreprocessorRegion() {
+		return preprocessorRegion;
+	}
+
+	/**
+	 * Creates a new position with the given offset and length 0.
+	 *
+	 * @param offset the position offset, must be >= 0
+	 * @param preprocessor True if the region is a preprocessor region, false otherwise
+	 */
+	public InactivePosition(int offset, boolean preprocessor) {
+		super(offset, 0);
+		preprocessorRegion = preprocessor;
+	}
+
+	/**
+	 * Creates a new position with the given offset and length.
+	 *
+	 * @param offset the position offset, must be >= 0
+	 * @param length the position length, must be >= 0
+	 * @param preprocessor True if the region is a preprocessor region, false otherwise
+	 */
+	public InactivePosition(int offset, int length, boolean preprocessor) {
+		super(offset, length);
+		preprocessorRegion = preprocessor;
+	}
+
+}

core/org.eclipse.cdt.core/src/org/eclipse/cdt/internal/formatter/Scribe.java

@@ -27,7 +27,6 @@ import org.eclipse.cdt.internal.formatter.align.Alignment;
 import org.eclipse.cdt.internal.formatter.align.AlignmentException;
 import org.eclipse.cdt.internal.formatter.scanner.Scanner;
 import org.eclipse.cdt.internal.formatter.scanner.Token;
-import org.eclipse.jface.text.Position;
 import org.eclipse.text.edits.MultiTextEdit;
 import org.eclipse.text.edits.ReplaceEdit;
 import org.eclipse.text.edits.TextEdit;
@@ -91,7 +90,7 @@ public class Scribe {
 	/**
 	 * It keeps the list of inactive region.
 	 */
-	private List<Position> fSkipInactivePositions = Collections.emptyList();
+	private List<InactivePosition> fSkipInactivePositions = Collections.emptyList();
 	/**
 	 * When scribe is in inactive state, source edits are not allowed.
 	 */
@@ -679,7 +678,7 @@ public class Scribe {
 	/**
 	 * @param list
 	 */
-	public void setSkipInactivePositions(List<Position> list) {
+	public void setSkipInactivePositions(List<InactivePosition> list) {
 		fSkipInactivePositions = list;
 		skipOverInactive = !list.isEmpty();
 	}
@@ -1086,7 +1085,7 @@ public class Scribe {
 		int lines = 0;
 		while ((currentToken = scanner.nextToken()) != null) {
 			if (skipOverInactive) {
-				Position inactivePos = getInactivePosAt(scanner.getCurrentTokenStartPosition());
+				InactivePosition inactivePos = getInactivePosAt(scanner.getCurrentTokenStartPosition());
 				if (inactivePos != null) {
 					int startOffset = Math.min(scanner.getCurrentTokenStartPosition(), inactivePos.getOffset());
 					int endOffset = Math.min(scannerEndPosition, inactivePos.getOffset() + inactivePos.getLength());
@@ -1098,7 +1097,8 @@ public class Scribe {
 						 * We are entering in inactive state so if we added a new line previously,
 						 * starting a new line, we need to remove it.
 						 */
-						if (editsIndex > 0 && lineSeparator.equals(edits[editsIndex - 1].replacement)) {
+						if (inactivePos.isPreprocessorRegion() && editsIndex > 0
+								&& lineSeparator.equals(edits[editsIndex - 1].replacement)) {
 							editsIndex--;
 						}
 						printRaw(startOffset, endOffset - startOffset);
@@ -1311,9 +1311,9 @@ public class Scribe {
 	 * @param offset
 	 * @return
 	 */
-	private Position getInactivePosAt(int offset) {
+	private InactivePosition getInactivePosAt(int offset) {
-		for (Iterator<Position> iter = fSkipInactivePositions.iterator(); iter.hasNext();) {
+		for (Iterator<InactivePosition> iter = fSkipInactivePositions.iterator(); iter.hasNext();) {
-			Position pos = iter.next();
+			InactivePosition pos = iter.next();
 			if (pos.includes(offset)) {
 				return pos;
 			}

core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/text/CodeFormatterTest.java

@@ -4018,4 +4018,69 @@ public class CodeFormatterTest extends BaseUITestCase {
 	public void testBoolInStructWithMacro_Bug397710() throws Exception {
 		assertFormatterResult();
 	}
+
+	///* @formatter:off */
+	//int xx2(){string s1="abc";string s2="def";}
+	///* @formatter:on */
+	//
+	///* @formatter:on */
+	//int xx(){string s1="abc";string s2="def";}
+	///* @formatter:off */
+	///* @formatter:on */
+	//int xx3(){string s1="abc";string s2="def";}
+
+	///* @formatter:off */
+	//int xx2(){string s1="abc";string s2="def";}
+	///* @formatter:on */
+	//
+	///* @formatter:on */
+	//int xx() {
+	//	string s1 = "abc";
+	//	string s2 = "def";
+	//}
+	///* @formatter:off */
+	///* @formatter:on */
+	//int xx3() {
+	//	string s1 = "abc";
+	//	string s2 = "def";
+	//}
+	public void testOnOffTags1_Bug546391() throws Exception {
+		fOptions.put(DefaultCodeFormatterConstants.FORMATTER_USE_COMMENT_TAG, true);
+		fOptions.put(DefaultCodeFormatterConstants.FORMATTER_COMMENT_ON_TAG, "@formatter:on");
+		fOptions.put(DefaultCodeFormatterConstants.FORMATTER_COMMENT_OFF_TAG, "@formatter:off");
+		assertFormatterResult();
+	}
+
+	//int xx(){string s1="abc";string s2="def";}
+	///* @formatter:off */
+
+	//int xx() {
+	//	string s1 = "abc";
+	//	string s2 = "def";
+	//}
+	///* @formatter:off */
+	public void testOnOffTags2_Bug546391() throws Exception {
+		fOptions.put(DefaultCodeFormatterConstants.FORMATTER_USE_COMMENT_TAG, true);
+		fOptions.put(DefaultCodeFormatterConstants.FORMATTER_COMMENT_ON_TAG, "@formatter:on");
+		fOptions.put(DefaultCodeFormatterConstants.FORMATTER_COMMENT_OFF_TAG, "@formatter:off");
+		assertFormatterResult();
+	}
+
+	///* @formatter:off */
+	//int xx(){string s1="abc";string s2="def";}
+	///* @formatter:on
+	//		*
+	//	*/
+
+	///* @formatter:off */
+	//int xx(){string s1="abc";string s2="def";}
+	///* @formatter:on
+	//		*
+	//	*/
+	public void testOnOffTags3_Bug546391() throws Exception {
+		fOptions.put(DefaultCodeFormatterConstants.FORMATTER_USE_COMMENT_TAG, true);
+		fOptions.put(DefaultCodeFormatterConstants.FORMATTER_COMMENT_ON_TAG, "@formatter:on");
+		fOptions.put(DefaultCodeFormatterConstants.FORMATTER_COMMENT_OFF_TAG, "@formatter:off");
+		assertFormatterResult();
+	}
 }