From aae3ce15aa72631e290b4eb6d0e163706b10390b Mon Sep 17 00:00:00 2001
From: Christian Kandeler <christian.kandeler@qt.io>
Date: Fri, 29 Jan 2021 10:18:31 +0100
Subject: [PATCH] CplusPlus: Fix minimal name construction

... when using declarations are involved.
Second attempt.
I have a suspicion that this is not the "correct" approach, however:
    a) It appears to fix the problem.
    b) It does not break any tests.
    c) It's rather high in the call tree, so there's a low likelihood
       of breaking something else.
    d) Considering the hacky-looking pre-existing code dealing with
       using declarations (e.g. LookupContext::lookupByUsing()),
       it seems quite possible that there is no "correct" way with
       the current infrastructure.

Fixes: QTCREATORBUG-14524
Change-Id: I28c6e53137c21ce158bbd03b92b3db39146116d5
Reviewed-by: Christian Stenger <christian.stenger@qt.io>
---
 src/libs/cplusplus/LookupContext.cpp       | 58 +++++++++++++++++-----
 src/libs/cplusplus/LookupContext.h         |  1 +
 src/plugins/cppeditor/cppquickfix_test.cpp |  5 +-
 3 files changed, 49 insertions(+), 15 deletions(-)

diff --git a/src/libs/cplusplus/LookupContext.cpp b/src/libs/cplusplus/LookupContext.cpp
index c878f3658aa..3f5d3c4f342 100644
--- a/src/libs/cplusplus/LookupContext.cpp
+++ b/src/libs/cplusplus/LookupContext.cpp
@@ -270,6 +270,27 @@ const Name *LookupContext::minimalName(Symbol *symbol, ClassOrNamespace *target,
 {
     const Name *n = nullptr;
     QList<const Name *> names = LookupContext::fullyQualifiedName(symbol);
+    ClassOrNamespace *current = target;
+
+    const auto getNameFromItems = [symbol, target, control](const QList<LookupItem> &items,
+            const QList<const Name *> &names, bool checkSymbols) -> const Name * {
+        for (const LookupItem &item : items) {
+            if (checkSymbols && !symbolIdentical(item.declaration(), symbol))
+                continue;
+
+            // eliminate inline namespaces
+            QList<const Name *> minimal = names;
+            for (int i = minimal.size() - 2; i >= 0; --i) {
+                const Name *candidate = toName(minimal.mid(0, i + 1), control);
+                if (isInlineNamespace(target, candidate))
+                    minimal.removeAt(i);
+            }
+
+            return toName(minimal, control);
+        }
+
+        return nullptr;
+    };
 
     for (int i = names.size() - 1; i >= 0; --i) {
         if (! n)
@@ -279,20 +300,24 @@ const Name *LookupContext::minimalName(Symbol *symbol, ClassOrNamespace *target,
 
         // once we're qualified enough to get the same symbol, break
         if (target) {
-            const QList<LookupItem> tresults = target->lookup(n);
-            foreach (const LookupItem &tr, tresults) {
-                if (symbolIdentical(tr.declaration(), symbol)) {
-                    // eliminate inline namespaces
-                    QList<const Name *> minimal = names.mid(i);
-                    for (int i = minimal.size() - 2; i >= 0; --i) {
-                        const Name *candidate = toName(minimal.mid(0, i + 1), control);
-                        if (isInlineNamespace(target, candidate))
-                            minimal.removeAt(i);
-                    }
-
-                    return toName(minimal, control);
+            const Name * const minimal = getNameFromItems(target->lookup(n), names.mid(i), true);
+            if (minimal)
+                return minimal;
+        }
+        if (current) {
+            const ClassOrNamespace * const nested = current->getNested(names.last());
+            if (nested) {
+                const QList<const Name *> nameList
+                        = names.mid(0, names.size() - i - 1) << names.last();
+                const QList<ClassOrNamespace *> usings = nested->usings();
+                for (ClassOrNamespace * const u : usings) {
+                    const Name * const minimal = getNameFromItems(u->lookup(symbol->name()),
+                                                                  nameList, false);
+                    if (minimal)
+                        return minimal;
                 }
             }
+            current = current->getNested(names.at(names.size() - i - 1));
         }
     }
 
@@ -923,6 +948,15 @@ ClassOrNamespace *ClassOrNamespace::findBlock(Block *block)
     return findBlock_helper(block, &processed, true);
 }
 
+ClassOrNamespace *ClassOrNamespace::getNested(const Name *name)
+{
+    flush();
+    const auto it = _classOrNamespaces.find(name);
+    if (it != _classOrNamespaces.cend())
+        return it->second;
+    return nullptr;
+}
+
 Symbol *ClassOrNamespace::lookupInScope(const QList<const Name *> &fullName)
 {
     if (!_scopeLookupCache) {
diff --git a/src/libs/cplusplus/LookupContext.h b/src/libs/cplusplus/LookupContext.h
index f4c2bdd440e..ce52feec426 100644
--- a/src/libs/cplusplus/LookupContext.h
+++ b/src/libs/cplusplus/LookupContext.h
@@ -83,6 +83,7 @@ public:
     ClassOrNamespace *lookupType(const Name *name, Block *block);
     ClassOrNamespace *findType(const Name *name);
     ClassOrNamespace *findBlock(Block *block);
+    ClassOrNamespace *getNested(const Name *name);
 
     Symbol *lookupInScope(const QList<const Name *> &fullName);
 
diff --git a/src/plugins/cppeditor/cppquickfix_test.cpp b/src/plugins/cppeditor/cppquickfix_test.cpp
index deeb31ff5ed..91f6df74b48 100644
--- a/src/plugins/cppeditor/cppquickfix_test.cpp
+++ b/src/plugins/cppeditor/cppquickfix_test.cpp
@@ -4322,7 +4322,6 @@ void Foo::otherFunc()
     QuickFixOperationTest(testDocuments, &factory);
 }
 
-// FIXME: QTCREATORBUG-14524.
 void CppEditorPlugin::test_quickfix_InsertDefFromDecl_usingDecl()
 {
     QList<QuickFixTestDocument::Ptr> testDocuments;
@@ -4347,7 +4346,7 @@ void @func(const S &s);
     expected = R"(
 #include "file.h"
 
-void func(const N::S &s)
+void func(const S &s)
 {
 
 }
@@ -4376,7 +4375,7 @@ void @func(const N1::S &s);
     expected = R"(
 #include "file.h"
 
-void func(const N1::N2::S &s)
+void func(const N1::S &s)
 {
 
 }