CppEditor: Fix Typedef Handling with 'Remove Using Directive' QuickFix

Previously, typedefs were ignored and the new code became invalid after
applying the quickfix.

Change-Id: I0d4295e90d02dfacc3edac5ac3f96d9edbeaf662
Reviewed-by: Christian Kandeler <christian.kandeler@qt.io>

3 files changed, 87 insertions, 2 deletions

diff --git a/src/plugins/cppeditor/cppeditorplugin.h b/src/plugins/cppeditor/cppeditorplugin.h
index ecb8694efe..869bc68bb5 100644
--- a/src/plugins/cppeditor/cppeditorplugin.h
+++ b/src/plugins/cppeditor/cppeditorplugin.h
@@ -213,6 +213,8 @@ private slots:
 
     void test_quickfix_removeUsingNamespace_data();
     void test_quickfix_removeUsingNamespace();
+    void test_quickfix_removeUsingNamespace_simple_data();
+    void test_quickfix_removeUsingNamespace_simple();
     void test_quickfix_removeUsingNamespace_differentSymbols();
 
     void test_quickfix_InsertVirtualMethods_data();
diff --git a/src/plugins/cppeditor/cppquickfix_test.cpp b/src/plugins/cppeditor/cppquickfix_test.cpp
index 0e4c26ef36..4da9174b4f 100644
--- a/src/plugins/cppeditor/cppquickfix_test.cpp
+++ b/src/plugins/cppeditor/cppquickfix_test.cpp
@@ -6479,6 +6479,49 @@ void CppEditorPlugin::test_quickfix_removeUsingNamespace()
     QuickFixOperationTest(testDocuments, &factory, ProjectExplorer::HeaderPaths(), operation);
 }
 
+void CppEditorPlugin::test_quickfix_removeUsingNamespace_simple_data()
+{
+    QTest::addColumn<QByteArray>("header");
+    QTest::addColumn<QByteArray>("expected");
+
+    const QByteArray common = R"--(
+namespace N{
+    template<typename T>
+    struct vector{
+        using iterator = T*;
+    };
+    using int_vector = vector<int>;
+}
+)--";
+    const QByteArray header = common + R"--(
+using namespace N@;
+int_vector ints;
+int_vector::iterator intIter;
+using vec = vector<int>;
+vec::iterator it;
+)--";
+    const QByteArray expected = common + R"--(
+N::int_vector ints;
+N::int_vector::iterator intIter;
+using vec = N::vector<int>;
+vec::iterator it;
+)--";
+
+    QTest::newRow("nested typedefs with Namespace") << header << expected;
+}
+
+void CppEditorPlugin::test_quickfix_removeUsingNamespace_simple()
+{
+    QFETCH(QByteArray, header);
+    QFETCH(QByteArray, expected);
+
+    QList<QuickFixTestDocument::Ptr> testDocuments;
+    testDocuments << QuickFixTestDocument::create("header.h", header, expected);
+
+    RemoveUsingNamespace factory;
+    QuickFixOperationTest(testDocuments, &factory, ProjectExplorer::HeaderPaths());
+}
+
 void CppEditorPlugin::test_quickfix_removeUsingNamespace_differentSymbols()
 {
     QByteArray header = "namespace test{\n"
diff --git a/src/plugins/cppeditor/cppquickfixes.cpp b/src/plugins/cppeditor/cppquickfixes.cpp
index b5f930efdb..c8929a6ff9 100644
--- a/src/plugins/cppeditor/cppquickfixes.cpp
+++ b/src/plugins/cppeditor/cppquickfixes.cpp
@@ -7291,6 +7291,33 @@ private:
 };
 
 /**
+ * @brief getBaseName returns the base name of a qualified name or nullptr.
+ * E.g.: foo::bar => foo; bar => bar
+ * @param name The Name, maybe qualified
+ * @return The base name of the qualified name or nullptr
+ */
+const Identifier *getBaseName(const Name *name)
+{
+    class GetBaseName : public NameVisitor
+    {
+        void visit(const Identifier *name) override { baseName = name; }
+        void visit(const QualifiedNameId *name) override
+        {
+            if (name->base())
+                accept(name->base());
+            else
+                accept(name->name());
+        }
+
+    public:
+        const Identifier *baseName = nullptr;
+    };
+    GetBaseName getter;
+    getter.accept(name);
+    return getter.baseName;
+}
+
+/**
  * @brief countNames counts the parts of the Name.
  * E.g. if the name is std::vector, the function returns 2, if the name is variant, returns 1
  * @param name The name that should be counted
@@ -7489,11 +7516,24 @@ private:
     {
         if (m_start) {
             Scope *scope = m_file->scopeAt(ast->firstToken());
-            const QList<LookupItem> lookups = m_context.lookup(ast->name->name, scope);
+            const Name *wantToLookup = ast->name->name;
+            // first check if the base name is a typedef. Consider the following example:
+            // using namespace std;
+            // using vec = std::vector<int>;
+            // vec::iterator it; // we have to lookup 'vec' and not iterator (would result in
+            //   std::vector<int>::iterator => std::vec::iterator, which is wrong)
+            const Name *baseName = getBaseName(wantToLookup);
+            QList<LookupItem> typedefCandidates = m_context.lookup(baseName, scope);
+            if (!typedefCandidates.isEmpty()) {
+                if (typedefCandidates.front().declaration()->isTypedef())
+                    wantToLookup = baseName;
+            }
+
+            const QList<LookupItem> lookups = m_context.lookup(wantToLookup, scope);
             if (!lookups.empty()) {
                 QList<const Name *> fullName = m_context.fullyQualifiedName(
                     lookups.first().declaration());
-                const int currentNameCount = countNames(ast->name->name);
+                const int currentNameCount = countNames(wantToLookup);
                 const bool needNamespace = needMissingNamespaces(std::move(fullName),
                                                                  currentNameCount);
                 if (needNamespace)