Bug 278903 - Do not format any code in inactive regions

Not active regions of code may not be formatted.

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

core/org.eclipse.cdt.core/META-INF/MANIFEST.MF

@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: %pluginName
 Bundle-SymbolicName: org.eclipse.cdt.core; singleton:=true
-Bundle-Version: 6.7.0.qualifier
+Bundle-Version: 6.7.100.qualifier
 Bundle-Activator: org.eclipse.cdt.core.CCorePlugin
 Bundle-Vendor: %providerName
 Bundle-Localization: plugin

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

@@ -439,8 +439,9 @@ public class CodeFormatterVisitor extends ASTVisitor implements ICPPASTVisitor,
 
 		localScanner.setSource(compilationUnitSource);
 		scribe.initializeScanner(compilationUnitSource);
-		scribe.setSkipInactivePositions(collectInactiveCodePositions(unit));
+		List<Position> inactive = collectInactiveCodePositions(unit);
-		scribe.setSkipForbiddenPositions(collectNoFormatCodePositions(unit));
+		inactive.addAll(collectNoFormatCodePositions(unit));
+		scribe.setSkipInactivePositions(inactive);
 
 		fStatus = new MultiStatus(CCorePlugin.PLUGIN_ID, 0, "Formatting problem(s) in '" + unit.getFilePath() + "'", //$NON-NLS-1$//$NON-NLS-2$
 				null);
@@ -4513,7 +4514,7 @@ public class CodeFormatterVisitor extends ASTVisitor implements ICPPASTVisitor,
 			int onPos = comment.lastIndexOf(this.preferences.comment_formatter_on_tag);
 			if (offPos != -1 && offPos > onPos) {
 				if (!inInactiveCode) {
-					inactiveCodeStart = nodeLocation.getNodeOffset();
+					inactiveCodeStart = nodeLocation.getNodeOffset() + nodeLocation.getNodeLength();
 					inInactiveCode = true;
 				}
 			} else if (onPos != -1 && onPos > offPos) {

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

@@ -93,10 +93,9 @@ public class Scribe {
 	 */
 	private List<Position> fSkipInactivePositions = Collections.emptyList();
 	/**
-	 * It keeps the list of no-format region.
+	 * When scribe is in inactive state, source edits are not allowed.
 	 */
-	private List<Position> fSkipForbiddenPositions = Collections.emptyList();
+	private boolean inactiveState = false;
-
 	private boolean skipOverInactive;
 
 	private int fSkipStartOffset = Integer.MAX_VALUE;
@@ -144,11 +143,13 @@ public class Scribe {
 	}
 
 	private final void addDeleteEdit(int start, int end) {
-		addOptimizedReplaceEdit(start, end - start + 1, EMPTY_STRING);
+		if (!inactiveState)
+			addOptimizedReplaceEdit(start, end - start + 1, EMPTY_STRING);
 	}
 
 	public final void addInsertEdit(int insertPosition, CharSequence insertedString) {
-		addOptimizedReplaceEdit(insertPosition, 0, insertedString);
+		if (!inactiveState)
+			addOptimizedReplaceEdit(insertPosition, 0, insertedString);
 	}
 
 	/**
@@ -158,7 +159,8 @@ public class Scribe {
 	 * @param replacement  the replacement string
 	 */
 	public final void addReplaceEdit(int start, int end, CharSequence replacement) {
-		addOptimizedReplaceEdit(start, end - start + 1, replacement);
+		if (!inactiveState)
+			addOptimizedReplaceEdit(start, end - start + 1, replacement);
 	}
 
 	private final void addOptimizedReplaceEdit(int offset, int length, CharSequence replacement) {
@@ -674,15 +676,6 @@ public class Scribe {
 		}
 	}
 
-	/**
-	 * Set the positions where we don't want to perform any check
-	 * @param list The list of positions
-	 */
-	public void setSkipForbiddenPositions(List<Position> list) {
-		if (list != null)
-			fSkipForbiddenPositions = list;
-	}
-
 	/**
 	 * @param list
 	 */
@@ -770,6 +763,10 @@ public class Scribe {
 	}
 
 	public void printRaw(int startOffset, int length) {
+		printRaw(startOffset, length, true);
+	}
+
+	private void printRaw(int startOffset, int length, boolean skipInactive) {
 		if (length <= 0) {
 			return;
 		}
@@ -788,7 +785,8 @@ public class Scribe {
 		boolean savedSkipOverInactive = skipOverInactive;
 		int savedScannerEndPos = scannerEndPosition;
 		preserveNewLines = true;
-		skipOverInactive = false;
+		if (skipInactive)
+			skipOverInactive = false;
 		scannerEndPosition = startOffset + length;
 		try {
 			scanner.resetTo(Math.max(startOffset, currentPosition), startOffset + length);
@@ -1061,7 +1059,7 @@ public class Scribe {
 	public void printEndOfTranslationUnit() {
 		int currentTokenStartPosition = scanner.getCurrentPosition();
 		if (currentTokenStartPosition <= scannerEndPosition) {
-			printRaw(currentTokenStartPosition, scannerEndPosition - currentTokenStartPosition + 1);
+			printRaw(currentTokenStartPosition, scannerEndPosition - currentTokenStartPosition + 1, false);
 		}
 	}
 
@@ -1095,7 +1093,16 @@ public class Scribe {
 					if (startOffset < endOffset) {
 						int savedIndentLevel = indentationLevel;
 						scanner.resetTo(scanner.getCurrentTokenStartPosition(), scanner.eofPosition);
+						inactiveState = true;
+						/**
+						 * 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)) {
+							editsIndex--;
+						}
 						printRaw(startOffset, endOffset - startOffset);
+						inactiveState = false;
 						while (indentationLevel > savedIndentLevel) {
 							unIndent();
 						}
@@ -1297,20 +1304,7 @@ public class Scribe {
 		}
 		scanner.resetTo(currentTokenStartPosition, scannerEndPosition);
 		return hasWhitespace;
-	}
 
-	/**
-	 * @param offset
-	 * @return
-	 */
-	private boolean isForbidden(int offset) {
-		for (Iterator<Position> iter = fSkipForbiddenPositions.iterator(); iter.hasNext();) {
-			Position pos = iter.next();
-			if (pos.includes(offset)) {
-				return true;
-			}
-		}
-		return false;
 	}
 
 	/**
@@ -2067,7 +2061,7 @@ public class Scribe {
 	}
 
 	boolean shouldSkip(int offset) {
-		return ((offset >= fSkipStartOffset && offset < fSkipEndOffset) || isForbidden(offset));
+		return offset >= fSkipStartOffset && offset < fSkipEndOffset;
 	}
 
 	void skipRange(int offset, int endOffset) {

core/org.eclipse.cdt.ui.tests/META-INF/MANIFEST.MF

@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: org.eclipse.cdt.ui.tests
 Bundle-SymbolicName: org.eclipse.cdt.ui.tests; singleton:=true
-Bundle-Version: 5.5.100.qualifier
+Bundle-Version: 5.5.200.qualifier
 Bundle-Activator: org.eclipse.cdt.ui.testplugin.CTestPlugin
 Bundle-Localization: plugin
 Export-Package: org.eclipse.cdt.ui.testplugin,

core/org.eclipse.cdt.ui.tests/pom.xml

@@ -11,7 +11,7 @@
 		<relativePath>../../pom.xml</relativePath>
 	</parent>
 
-	<version>5.5.100-SNAPSHOT</version>
+	<version>5.5.200-SNAPSHOT</version>
 	<artifactId>org.eclipse.cdt.ui.tests</artifactId>
 	<packaging>eclipse-test-plugin</packaging>
 

core/org.eclipse.cdt.ui.tests/resources/formatter/sample/After.cpp

@@ -26,8 +26,8 @@ const SimpleStruct simpleStruct = { 1, "mySimple", 0.1232 };
 
 const SimpleStruct array[] = { { SIZEOF(simpleStruct, num),
 #if FOO
-		"foo"
+				"foo"
-#  else
+   #  else
 		"bar"
 #endif
 		, 0.5 }, { SIZEOF(simpleStruct, floatNum), "name", 1.1 } };

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

@@ -527,7 +527,7 @@ public class CodeFormatterTest extends BaseUITestCase {
 	//
 	//const SimpleStruct array[] = { { SIZEOF(simpleStruct, num),
 	//#if FOO
-	//		"foo"
+	//"foo"
 	//#else
 	//		"bar"
 	//#endif
@@ -1799,10 +1799,10 @@ public class CodeFormatterTest extends BaseUITestCase {
 	//	foo ARGS;
 	//	CALL;
 	//#if X
-	//	if (1)
+	//                        if (1)
-	//	{
+	//                        {
-	//		t = 1;
+	//                                t = 1;
-	//	}
+	//                        }
 	//#endif
 	//}
 	public void testMacroAsFunctionArguments_Bug253039() throws Exception {
@@ -3656,4 +3656,38 @@ public class CodeFormatterTest extends BaseUITestCase {
 		fOptions.put(DefaultCodeFormatterConstants.FORMATTER_COMMENT_OFF_TAG, "@formatter:off");
 		assertFormatterResult();
 	}
+
+	//int code_before_inactive;
+	//#if 0
+	//    void this_line_intentionally_indented() {
+	// int x;
+	// }
+	//#endif
+	//int code_after_inactive;
+
+	//int code_before_inactive;
+	//#if 0
+	//    void this_line_intentionally_indented() {
+	// int x;
+	// }
+	//#endif
+	//int code_after_inactive;
+	public void testDoeNotFormatInactiveCode() throws Exception {
+		assertFormatterResult();
+	}
+
+	//#if 0
+	//    void this_line_intentionally_indented() {
+	// int x;
+	// }
+	//#endif
+
+	//#if 0
+	//    void this_line_intentionally_indented() {
+	// int x;
+	// }
+	//#endif
+	public void testDoeNotFormatInactiveCodeEntireFile() throws Exception {
+		assertFormatterResult();
+	}
 }