From 30da7ad3e285b1f4b59104d4a8a8b06553e87e39 Mon Sep 17 00:00:00 2001
From: Nikolai Kosjar <nikolai.kosjar@digia.com>
Date: Tue, 12 Feb 2013 11:57:01 +0100
Subject: [PATCH] C++: Pass only reasonable ASTs to pointer declaration
 formatter

Change-Id: Ide829a8084d3fef79f252dc7724bd90ce8ebba04
Reviewed-by: Erik Verbruggen <erik.verbruggen@digia.com>
---
 src/plugins/cppeditor/cppplugin.h          |  1 +
 src/plugins/cppeditor/cppquickfix_test.cpp | 12 ++++
 src/plugins/cppeditor/cppquickfixes.cpp    | 64 +++++++++++++++++++++-
 3 files changed, 74 insertions(+), 3 deletions(-)

diff --git a/src/plugins/cppeditor/cppplugin.h b/src/plugins/cppeditor/cppplugin.h
index 5f3b2f70c4d..d78692ca60b 100644
--- a/src/plugins/cppeditor/cppplugin.h
+++ b/src/plugins/cppeditor/cppplugin.h
@@ -102,6 +102,7 @@ private slots: // quickfix tests
     void test_quickfix_GenerateGetterSetter_notTriggeringOnMemberFunction();
     void test_quickfix_GenerateGetterSetter_notTriggeringOnMemberArray();
     void test_quickfix_GenerateGetterSetter_notTriggeringWhenGetterOrSetterExist();
+    void test_quickfix_ReformatPointerDeclaration();
 #endif // WITH_TESTS
 
 private:
diff --git a/src/plugins/cppeditor/cppquickfix_test.cpp b/src/plugins/cppeditor/cppquickfix_test.cpp
index f134043a8cd..899a567046c 100644
--- a/src/plugins/cppeditor/cppquickfix_test.cpp
+++ b/src/plugins/cppeditor/cppquickfix_test.cpp
@@ -43,6 +43,7 @@
 #include <cppeditor/cppplugin.h>
 #include <cppeditor/cppquickfixassistant.h>
 #include <cppeditor/cppquickfix.h>
+#include <cppeditor/cppquickfixes.h>
 #include <extensionsystem/pluginmanager.h>
 #include <texteditor/basetextdocument.h>
 #include <texteditor/codeassist/basicproposalitemlistmodel.h>
@@ -543,3 +544,14 @@ void CppPlugin::test_quickfix_GenerateGetterSetter_notTriggeringWhenGetterOrSett
     GenerateGetterSetter factory(/*testMode=*/ true);
     data.run(&factory, expected, /*changesExpected=*/ false);
 }
+
+/// Check: Just a basic test since the main functionality is tested in
+/// cpppointerdeclarationformatter_test.cpp
+void CppPlugin::test_quickfix_ReformatPointerDeclaration()
+{
+    TestCase data("char@*s;");
+    QByteArray expected = "char *s;\n";
+
+    ReformatPointerDeclaration factory;
+    data.run(&factory, expected);
+}
diff --git a/src/plugins/cppeditor/cppquickfixes.cpp b/src/plugins/cppeditor/cppquickfixes.cpp
index 9967ef84fe0..fd7f18add08 100644
--- a/src/plugins/cppeditor/cppquickfixes.cpp
+++ b/src/plugins/cppeditor/cppquickfixes.cpp
@@ -1859,6 +1859,64 @@ private:
     Utils::ChangeSet m_change;
 };
 
+/// Filter the results of ASTPath.
+/// The resulting list contains the supported AST types only once.
+/// For this, the results of ASTPath are iterated in reverse order.
+class ReformatPointerDeclarationASTPathResultsFilter
+{
+public:
+    ReformatPointerDeclarationASTPathResultsFilter()
+        : m_hasSimpleDeclaration(false)
+        , m_hasFunctionDefinition(false)
+        , m_hasParameterDeclaration(false)
+        , m_hasIfStatement(false)
+        , m_hasWhileStatement(false)
+        , m_hasForStatement(false)
+        , m_hasForeachStatement(false) {}
+
+    QList<AST*> filter(const QList<AST*> &astPathList)
+    {
+        QList<AST*> filtered;
+
+        for (int i = astPathList.size() - 1; i >= 0; --i) {
+            AST *ast = astPathList.at(i);
+
+            if (! m_hasSimpleDeclaration && ast->asSimpleDeclaration()) {
+                m_hasSimpleDeclaration = true;
+                filtered.append(ast);
+            } else if (! m_hasFunctionDefinition && ast->asFunctionDefinition()) {
+                m_hasFunctionDefinition = true;
+                filtered.append(ast);
+            } else if (! m_hasParameterDeclaration && ast->asParameterDeclaration()) {
+                m_hasParameterDeclaration = true;
+                filtered.append(ast);
+            } else if (! m_hasIfStatement && ast->asIfStatement()) {
+                m_hasIfStatement = true;
+                filtered.append(ast);
+            } else if (! m_hasWhileStatement && ast->asWhileStatement()) {
+                m_hasWhileStatement = true;
+                filtered.append(ast);
+            } else if (! m_hasForStatement && ast->asForStatement()) {
+                m_hasForStatement = true;
+                filtered.append(ast);
+            } else if (! m_hasForeachStatement && ast->asForeachStatement()) {
+                m_hasForeachStatement = true;
+                filtered.append(ast);
+            }
+        }
+
+        return filtered;
+    }
+private:
+    bool m_hasSimpleDeclaration;
+    bool m_hasFunctionDefinition;
+    bool m_hasParameterDeclaration;
+    bool m_hasIfStatement;
+    bool m_hasWhileStatement;
+    bool m_hasForStatement;
+    bool m_hasForeachStatement;
+};
+
 void ReformatPointerDeclaration::match(const CppQuickFixInterface &interface,
                                        QuickFixOperations &result)
 {
@@ -1885,9 +1943,9 @@ void ReformatPointerDeclaration::match(const CppQuickFixInterface &interface,
                 new ReformatPointerDeclarationOp(interface, change)));
         }
     } else {
-        for (int index = path.size() - 1; index >= 0; --index) {
-            AST *ast = path.at(index);
-
+        const QList<AST *> suitableASTs
+            = ReformatPointerDeclarationASTPathResultsFilter().filter(path);
+        foreach (AST *ast, suitableASTs) {
             change = formatter.format(ast);
             if (! change.isEmpty()) {
                 result.append(QuickFixOperation::Ptr(