Bug 435340 - Add Include inserts an extra blank line

core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/util/TextUtil.java

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2013 Google, Inc and others.
+ * Copyright (c) 2013, 2014 Google, Inc and others.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -41,7 +41,26 @@ public class TextUtil {
 	}
 
 	/**
-	 * Returns {@code true} the line prior to the line corresponding to the given {@code offset} 
+	 * Returns {@code true} if the line corresponding to the given {@code offset} does not contain
+	 * non-whitespace characters.
+	 */
+	public static boolean isLineBlank(String text, int offset) {
+		while (--offset >= 0) {
+			if (text.charAt(offset) == '\n')
+				break;
+		}
+		while (++offset < text.length()) {
+			char c = text.charAt(offset);
+			if (c == '\n')
+				return true;
+			if (!Character.isWhitespace(c))
+				return false;
+		}
+		return true;
+	}
+
+	/**
+	 * Returns {@code true} if the line prior to the line corresponding to the given {@code offset} 
 	 * does not contain non-whitespace characters.
 	 */
 	public static boolean isPreviousLineBlank(String text, int offset) {
@@ -49,6 +68,9 @@ public class TextUtil {
 			if (text.charAt(offset) == '\n')
 				break;
 		}
+		if (offset < 0)
+			return false;
+
 		while (--offset >= 0) {
 			char c = text.charAt(offset);
 			if (c == '\n')
@@ -56,6 +78,6 @@ public class TextUtil {
 			if (!Character.isWhitespace(c))
 				return false;
 		}
-		return false;
+		return true;
 	}
 }

core/org.eclipse.cdt.ui.tests/resources/addInclude/Enumerator_307738.cpp.expected

@@ -1,4 +1,5 @@
 #include "Enumerator_307738.h"
+
 void test() {
   int i = Ns::ENUM_VALUE;
 }

core/org.eclipse.cdt.ui.tests/resources/addInclude/InsertionPoint_435340.cpp

@@ -0,0 +1,6 @@
+#include <string>
+
+#include "InsertionPoint_435340b.h"
+
+A435340 a;
+B435340 b;

core/org.eclipse.cdt.ui.tests/resources/addInclude/InsertionPoint_435340.cpp.expected

@@ -0,0 +1,7 @@
+#include <string>
+
+#include "InsertionPoint_435340a.h"
+#include "InsertionPoint_435340b.h"
+
+A435340 a;
+B435340 b;

core/org.eclipse.cdt.ui.tests/resources/addInclude/InsertionPoint_435340a.h

@@ -0,0 +1 @@
+class A435340 {};

core/org.eclipse.cdt.ui.tests/resources/addInclude/InsertionPoint_435340b.h

@@ -0,0 +1 @@
+class B435340 {};

core/org.eclipse.cdt.ui.tests/resources/addInclude/Macro.cpp.expected

@@ -1,2 +1,3 @@
 #include "Macro.h"
+
 int x = ONE;

core/org.eclipse.cdt.ui.tests/resources/addInclude/ResolvedName.cpp.expected

@@ -1,4 +1,5 @@
 #include "ResolvedName.h"
+
 namespace ns4 {
 
 A a;

core/org.eclipse.cdt.ui.tests/resources/addInclude/Template_306670.cpp.expected

@@ -1,4 +1,5 @@
 #include "Template_306670.h"
+
 void test() {
     func306670(1);
 }

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

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2009, 2010 Google, Inc and others.
+ * Copyright (c) 2009, 2014 Google, Inc and others.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -158,6 +158,11 @@ public class AddIncludeTest extends BaseTestCase {
 		assertAddIncludeResult();
 	}
 
+	public void testInsertionPoint_435340() throws Exception {
+		select("A435340");
+		assertAddIncludeResult();
+	}
+	
 	public void testTemplate_306670() throws Exception {
 		select("func306670");
 		assertAddIncludeResult();

core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/includes/IncludeCreator.java

@@ -83,6 +83,7 @@ import org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.SemanticUtil;
 import org.eclipse.cdt.internal.core.dom.rewrite.commenthandler.ASTCommenter;
 import org.eclipse.cdt.internal.core.dom.rewrite.commenthandler.NodeCommentMap;
 import org.eclipse.cdt.internal.core.dom.rewrite.util.ASTNodes;
+import org.eclipse.cdt.internal.core.dom.rewrite.util.TextUtil;
 import org.eclipse.cdt.internal.core.model.ASTStringUtil;
 import org.eclipse.cdt.internal.core.resources.ResourceLookup;
 import org.eclipse.cdt.internal.corext.codemanipulation.IncludeInfo;
@@ -275,8 +276,8 @@ public class IncludeCreator {
 
 		if (preferences.allowReordering) {
 			// Since the order of existing include statements may not match the include order
-			// preferences, we find positions for the new include statements by pushing them from
+			// preferences, we find positions for the new include statements by pushing them up
-			// them up from the bottom of the include insertion region.  
+			// from the bottom of the include insertion region.  
 			for (StyledInclude include : styledIncludes) {
 				int i = mergedIncludes.size();
 				while (--i >= 0 && preferences.compare(include, mergedIncludes.get(i)) < 0) {}
@@ -290,7 +291,8 @@ public class IncludeCreator {
 		StringBuilder text = new StringBuilder();
 		StyledInclude previousInclude = null;
 		for (StyledInclude include : mergedIncludes) {
-			if (include.getExistingInclude() == null) {
+			IASTPreprocessorIncludeStatement existingInclude = include.getExistingInclude();
+			if (existingInclude == null) {
 				if (previousInclude != null) {
 					IASTNode previousNode = previousInclude.getExistingInclude();
 					if (previousNode != null) {
@@ -299,20 +301,30 @@ public class IncludeCreator {
 						if (contents.charAt(offset - 1) != '\n')
 							text.append(fLineDelimiter);
 					}
-					if (include.getStyle().isBlankLineNeededAfter(previousInclude.getStyle(), preferences.includeStyles))
+					if (include.getStyle().isBlankLineNeededAfter(previousInclude.getStyle(), preferences.includeStyles)) {
+						if (TextUtil.isLineBlank(contents, offset)) {
+							offset = TextUtil.skipToNextLine(contents, offset);
+						} else {
 							text.append(fLineDelimiter);
 						}
+					}
+				}
 				text.append(include.getIncludeInfo().composeIncludeStatement());
 				text.append(fLineDelimiter);
 			} else {
 				if (previousInclude != null && previousInclude.getExistingInclude() == null &&
-						include.getStyle().isBlankLineNeededAfter(previousInclude.getStyle(), preferences.includeStyles)) {
+						include.getStyle().isBlankLineNeededAfter(previousInclude.getStyle(), preferences.includeStyles) &&
+						!TextUtil.isPreviousLineBlank(contents, ASTNodes.offset(existingInclude))) {
 					text.append(fLineDelimiter);
 				}
 				flushEditBuffer(offset, text, rootEdit);
 			}
 			previousInclude = include;
 		}
+		if (includeRegion.getLength() == 0 && !TextUtil.isLineBlank(contents, includeRegion.getOffset()) &&
+				!includes.isEmpty()) {
+			text.append(fLineDelimiter);
+		}
 		flushEditBuffer(offset, text, rootEdit);
 
 		List<UsingDeclaration> mergedUsingDeclarations = getUsingDeclarations(ast);