From dc63d60efd30dd30bd374ebcedf738cded4e36a5 Mon Sep 17 00:00:00 2001 From: Sergey Prigogin Date: Tue, 20 May 2014 14:02:55 -0700 Subject: [PATCH] Bug 435340 - Add Include inserts an extra blank line --- .../core/dom/rewrite/util/TextUtil.java | 28 +++++++++++++++++-- .../addInclude/Enumerator_307738.cpp.expected | 1 + .../addInclude/InsertionPoint_435340.cpp | 6 ++++ .../InsertionPoint_435340.cpp.expected | 7 +++++ .../addInclude/InsertionPoint_435340a.h | 1 + .../addInclude/InsertionPoint_435340b.h | 1 + .../resources/addInclude/Macro.cpp.expected | 1 + .../addInclude/ResolvedName.cpp.expected | 1 + .../addInclude/Template_306670.cpp.expected | 1 + .../cdt/ui/tests/text/AddIncludeTest.java | 7 ++++- .../refactoring/includes/IncludeCreator.java | 24 ++++++++++++---- 11 files changed, 68 insertions(+), 10 deletions(-) create mode 100644 core/org.eclipse.cdt.ui.tests/resources/addInclude/InsertionPoint_435340.cpp create mode 100644 core/org.eclipse.cdt.ui.tests/resources/addInclude/InsertionPoint_435340.cpp.expected create mode 100644 core/org.eclipse.cdt.ui.tests/resources/addInclude/InsertionPoint_435340a.h create mode 100644 core/org.eclipse.cdt.ui.tests/resources/addInclude/InsertionPoint_435340b.h diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/util/TextUtil.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/util/TextUtil.java index 3e0ab14a3ce..5eadb0cab73 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/util/TextUtil.java +++ b/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; } } diff --git a/core/org.eclipse.cdt.ui.tests/resources/addInclude/Enumerator_307738.cpp.expected b/core/org.eclipse.cdt.ui.tests/resources/addInclude/Enumerator_307738.cpp.expected index 0c5202a7730..048a4151f40 100644 --- a/core/org.eclipse.cdt.ui.tests/resources/addInclude/Enumerator_307738.cpp.expected +++ b/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; } diff --git a/core/org.eclipse.cdt.ui.tests/resources/addInclude/InsertionPoint_435340.cpp b/core/org.eclipse.cdt.ui.tests/resources/addInclude/InsertionPoint_435340.cpp new file mode 100644 index 00000000000..e95877fad7d --- /dev/null +++ b/core/org.eclipse.cdt.ui.tests/resources/addInclude/InsertionPoint_435340.cpp @@ -0,0 +1,6 @@ +#include + +#include "InsertionPoint_435340b.h" + +A435340 a; +B435340 b; diff --git a/core/org.eclipse.cdt.ui.tests/resources/addInclude/InsertionPoint_435340.cpp.expected b/core/org.eclipse.cdt.ui.tests/resources/addInclude/InsertionPoint_435340.cpp.expected new file mode 100644 index 00000000000..293c752cacc --- /dev/null +++ b/core/org.eclipse.cdt.ui.tests/resources/addInclude/InsertionPoint_435340.cpp.expected @@ -0,0 +1,7 @@ +#include + +#include "InsertionPoint_435340a.h" +#include "InsertionPoint_435340b.h" + +A435340 a; +B435340 b; diff --git a/core/org.eclipse.cdt.ui.tests/resources/addInclude/InsertionPoint_435340a.h b/core/org.eclipse.cdt.ui.tests/resources/addInclude/InsertionPoint_435340a.h new file mode 100644 index 00000000000..61aae43e9bf --- /dev/null +++ b/core/org.eclipse.cdt.ui.tests/resources/addInclude/InsertionPoint_435340a.h @@ -0,0 +1 @@ +class A435340 {}; diff --git a/core/org.eclipse.cdt.ui.tests/resources/addInclude/InsertionPoint_435340b.h b/core/org.eclipse.cdt.ui.tests/resources/addInclude/InsertionPoint_435340b.h new file mode 100644 index 00000000000..aa7a4a7d215 --- /dev/null +++ b/core/org.eclipse.cdt.ui.tests/resources/addInclude/InsertionPoint_435340b.h @@ -0,0 +1 @@ +class B435340 {}; diff --git a/core/org.eclipse.cdt.ui.tests/resources/addInclude/Macro.cpp.expected b/core/org.eclipse.cdt.ui.tests/resources/addInclude/Macro.cpp.expected index 7b5054edfce..435fa9777d3 100644 --- a/core/org.eclipse.cdt.ui.tests/resources/addInclude/Macro.cpp.expected +++ b/core/org.eclipse.cdt.ui.tests/resources/addInclude/Macro.cpp.expected @@ -1,2 +1,3 @@ #include "Macro.h" + int x = ONE; \ No newline at end of file diff --git a/core/org.eclipse.cdt.ui.tests/resources/addInclude/ResolvedName.cpp.expected b/core/org.eclipse.cdt.ui.tests/resources/addInclude/ResolvedName.cpp.expected index 93773200182..78290ad1bd4 100644 --- a/core/org.eclipse.cdt.ui.tests/resources/addInclude/ResolvedName.cpp.expected +++ b/core/org.eclipse.cdt.ui.tests/resources/addInclude/ResolvedName.cpp.expected @@ -1,4 +1,5 @@ #include "ResolvedName.h" + namespace ns4 { A a; diff --git a/core/org.eclipse.cdt.ui.tests/resources/addInclude/Template_306670.cpp.expected b/core/org.eclipse.cdt.ui.tests/resources/addInclude/Template_306670.cpp.expected index 01482ac274f..9a012b3ae38 100644 --- a/core/org.eclipse.cdt.ui.tests/resources/addInclude/Template_306670.cpp.expected +++ b/core/org.eclipse.cdt.ui.tests/resources/addInclude/Template_306670.cpp.expected @@ -1,4 +1,5 @@ #include "Template_306670.h" + void test() { func306670(1); } diff --git a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/text/AddIncludeTest.java b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/text/AddIncludeTest.java index ed396126a5c..e85cc320504 100644 --- a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/text/AddIncludeTest.java +++ b/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 @@ -157,6 +157,11 @@ public class AddIncludeTest extends BaseTestCase { select("XXX"); assertAddIncludeResult(); } + + public void testInsertionPoint_435340() throws Exception { + select("A435340"); + assertAddIncludeResult(); + } public void testTemplate_306670() throws Exception { select("func306670"); diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/includes/IncludeCreator.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/includes/IncludeCreator.java index 2b256e8ebc8..af223c3b18b 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/includes/IncludeCreator.java +++ b/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 - // them up from the bottom of the include insertion region. + // preferences, we find positions for the new include statements by pushing them up + // 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)) - text.append(fLineDelimiter); + 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 mergedUsingDeclarations = getUsingDeclarations(ast);