qdoc: Down-prioritize linking to section titles

It was too easy to inadvertently divert links generated by QDoc by
simply introducing a \section with a title that duplicates an existing
link target in the documentation. This happened quite frequently, for
example, by having names of Qt modules as section titles.

Treat section titles as a fallback when resolving links; internally
within a single Tree, section titles were already the lowest
priority candidates; extend this to apply to the search across the
entire forest of trees.

This has a slight performance impact for modules that pull in a lot of
dependencies (e.g. qtdoc), but it's arguably worth it.

Fixes: QTBUG-104137
Change-Id: I2904c7023690369ea1c707791b25a9b0cb0361fd
Reviewed-by: Luca Di Sera <luca.disera@qt.io>
Reviewed-by: Kai Koehne <kai.koehne@qt.io>
(cherry picked from commit a8cb221c7b3b07ab4b5a7dc2765248cd9350db02)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>

8 files changed, 68 insertions, 70 deletions

diff --git a/src/qdoc/qdocdatabase.cpp b/src/qdoc/qdocdatabase.cpp
index f17d7fb76..19ef8919a 100644
--- a/src/qdoc/qdocdatabase.cpp
+++ b/src/qdoc/qdocdatabase.cpp
@@ -273,6 +273,10 @@ void QDocForest::newPrimaryTree(const QString &module)
   other trees, which are all index trees. With relative set
   to 0, the starting point for each index tree is the root
   of the index tree.
+
+  If \a targetPath is resolved successfully but it refers to
+  a \\section title, continue the search, keeping the section
+  title as a fallback if no higher-priority targets are found.
  */
 const Node *QDocForest::findNodeForTarget(QStringList &targetPath, const Node *relative,
                                           Node::Genus genus, QString &ref)
@@ -286,13 +290,20 @@ const Node *QDocForest::findNodeForTarget(QStringList &targetPath, const Node *r
     if (!targetPath.isEmpty())
         target = targetPath.takeFirst();
 
+    TargetRec::TargetType type = TargetRec::Unknown;
+    const Node *tocNode = nullptr;
     for (const auto *tree : searchOrder()) {
-        const Node *n = tree->findNodeForTarget(entityPath, target, relative, flags, genus, ref);
-        if (n)
-            return n;
+        const Node *n = tree->findNodeForTarget(entityPath, target, relative, flags, genus, ref, &type);
+        if (n) {
+            // Targets referring to non-section titles are returned immediately
+            if (type != TargetRec::Contents)
+                return n;
+            if (!tocNode)
+                tocNode = n;
+        }
         relative = nullptr;
     }
-    return nullptr;
+    return tocNode;
 }
 
 /*!
diff --git a/src/qdoc/tree.cpp b/src/qdoc/tree.cpp
index 1cd19c656..468b46815 100644
--- a/src/qdoc/tree.cpp
+++ b/src/qdoc/tree.cpp
@@ -419,7 +419,7 @@ Node *Tree::findNodeRecursive(const QStringList &path, int pathIndex, const Node
  */
 const Node *Tree::findNodeForTarget(const QStringList &path, const QString &target,
                                     const Node *start, int flags, Node::Genus genus,
-                                    QString &ref) const
+                                    QString &ref, TargetRec::TargetType *targetType) const
 {
     const Node *node = nullptr;
     if ((genus == Node::DontCare) || (genus == Node::DOC)) {
@@ -435,15 +435,21 @@ const Node *Tree::findNodeForTarget(const QStringList &path, const QString &targ
         }
     }
 
-    node = findUnambiguousTarget(path.join(QLatin1String("::")), genus, ref);
-    if (node) {
+
+    const TargetRec *result = findUnambiguousTarget(path.join(QLatin1String("::")), genus);
+    if (result) {
+        node = result->m_node;
+        ref = result->m_ref;
         if (!target.isEmpty()) {
             ref = getRef(target, node);
             if (ref.isEmpty())
                 node = nullptr;
         }
-        if (node)
+        if (node) {
+            if (targetType)
+                *targetType = result->m_type;
             return node;
+        }
     }
 
     const Node *current = start;
@@ -780,64 +786,30 @@ void Tree::resolveTargets(Aggregate *root)
 }
 
 /*!
-  This function searches for a \a target anchor node. If it
-  finds one, it sets \a ref and returns the found node.
+  Searches for a \a target anchor, matching the given \a genus, and returns
+  the associated TargetRec instance.
  */
-const Node *Tree::findUnambiguousTarget(const QString &target, Node::Genus genus,
-                                        QString &ref) const
+const TargetRec *Tree::findUnambiguousTarget(const QString &target, Node::Genus genus) const
 {
-    int numBestTargets = 0;
-    TargetRec *bestTarget = nullptr;
-    QList<TargetRec *> bestTargetList;
-
-    QString key = target;
-    for (auto it = m_nodesByTargetTitle.find(key); it != m_nodesByTargetTitle.constEnd(); ++it) {
-        if (it.key() != key)
-            break;
-        TargetRec *candidate = it.value();
-        if ((genus == Node::DontCare) || (genus & candidate->genus())) {
-            if (!bestTarget || (candidate->m_priority < bestTarget->m_priority)) {
-                bestTarget = candidate;
-                bestTargetList.clear();
-                bestTargetList.append(candidate);
-                numBestTargets = 1;
-            } else if (candidate->m_priority == bestTarget->m_priority) {
-                bestTargetList.append(candidate);
-                ++numBestTargets;
+    auto findBestCandidate = [&](const TargetMap &tgtMap, const QString &key) {
+        TargetRec *best = nullptr;
+        auto [it, end] = tgtMap.equal_range(key);
+        while (it != end) {
+            TargetRec *candidate = it.value();
+            if ((genus == Node::DontCare) || (genus & candidate->genus())) {
+                if (!best || (candidate->m_priority < best->m_priority))
+                    best = candidate;
             }
+            ++it;
         }
-    }
-    if (bestTarget) {
-        ref = bestTarget->m_ref;
-        return bestTarget->m_node;
-    }
+        return best;
+    };
 
-    numBestTargets = 0;
-    bestTarget = nullptr;
-    key = Doc::canonicalTitle(target);
-    for (auto it = m_nodesByTargetRef.find(key); it != m_nodesByTargetRef.constEnd(); ++it) {
-        if (it.key() != key)
-            break;
-        TargetRec *candidate = it.value();
-        if ((genus == Node::DontCare) || (genus & candidate->genus())) {
-            if (!bestTarget || (candidate->m_priority < bestTarget->m_priority)) {
-                bestTarget = candidate;
-                bestTargetList.clear();
-                bestTargetList.append(candidate);
-                numBestTargets = 1;
-            } else if (candidate->m_priority == bestTarget->m_priority) {
-                bestTargetList.append(candidate);
-                ++numBestTargets;
-            }
-        }
-    }
-    if (bestTarget) {
-        ref = bestTarget->m_ref;
-        return bestTarget->m_node;
-    }
+    TargetRec *bestTarget = findBestCandidate(m_nodesByTargetTitle, target);
+    if (!bestTarget)
+        bestTarget = findBestCandidate(m_nodesByTargetRef, Doc::canonicalTitle(target));
 
-    ref.clear();
-    return nullptr;
+    return bestTarget;
 }
 
 /*!
diff --git a/src/qdoc/tree.h b/src/qdoc/tree.h
index e500fb1df..2321d0e84 100644
--- a/src/qdoc/tree.h
+++ b/src/qdoc/tree.h
@@ -27,7 +27,7 @@ public:
     enum TargetType { Unknown, Target, Keyword, Contents, Class, Function, Page, Subtitle };
 
     TargetRec(QString name, TargetRec::TargetType type, Node *node, int priority)
-        : m_node(node), m_ref(std::move(name)), m_priority(priority)
+        : m_node(node), m_ref(std::move(name)), m_type(type), m_priority(priority)
     {
         // Discard the dedicated ref for keywords - they always
         // link to the top of the QDoc comment they appear in
@@ -40,20 +40,14 @@ public:
 
     Node *m_node { nullptr };
     QString m_ref {};
+    TargetType m_type {};
     int m_priority {};
 };
 
-struct TargetLoc
-{
-public:
-    TargetLoc() = default;
-};
-
 typedef QMultiMap<QString, TargetRec *> TargetMap;
 typedef QMultiMap<QString, PageNode *> PageNodeMultiMap;
 typedef QMap<QString, QmlTypeNode *> QmlTypeMap;
 typedef QMultiMap<QString, const ExampleNode *> ExampleNodeMap;
-typedef QList<TargetLoc *> TargetList;
 
 class Tree
 {
@@ -88,7 +82,8 @@ private: // The rest of the class is private.
     Node *findNodeRecursive(const QStringList &path, int pathIndex, const Node *start,
                             bool (Node::*)() const) const;
     const Node *findNodeForTarget(const QStringList &path, const QString &target, const Node *node,
-                                  int flags, Node::Genus genus, QString &ref) const;
+                                  int flags, Node::Genus genus, QString &ref,
+                                  TargetRec::TargetType *targetType = nullptr) const;
     const Node *matchPathAndTarget(const QStringList &path, int idx, const QString &target,
                                    const Node *node, int flags, Node::Genus genus,
                                    QString &ref) const;
@@ -104,7 +99,7 @@ private: // The rest of the class is private.
     void insertTarget(const QString &name, const QString &title, TargetRec::TargetType type,
                       Node *node, int priority);
     void resolveTargets(Aggregate *root);
-    const Node *findUnambiguousTarget(const QString &target, Node::Genus genus, QString &ref) const;
+    const TargetRec *findUnambiguousTarget(const QString &target, Node::Genus genus) const;
     [[nodiscard]] const PageNode *findPageNodeByTitle(const QString &title) const;
 
     void addPropertyFunction(PropertyNode *property, const QString &funcName,
diff --git a/tests/auto/qdoc/generatedoutput/expected_output/docbook/qdoctests-qdocfileoutput.xml b/tests/auto/qdoc/generatedoutput/expected_output/docbook/qdoctests-qdocfileoutput.xml
index 720aeacc7..0fe4c530d 100644
--- a/tests/auto/qdoc/generatedoutput/expected_output/docbook/qdoctests-qdocfileoutput.xml
+++ b/tests/auto/qdoc/generatedoutput/expected_output/docbook/qdoctests-qdocfileoutput.xml
@@ -66,4 +66,8 @@
 </db:listitem>
 </db:itemizedlist>
 </db:section>
+<db:section xml:id="qdoc-linking-test">
+<db:title>QDoc Linking Test</db:title>
+<db:para>This section title is overridden by another target which takes precedence.</db:para>
+</db:section>
 </db:article>
diff --git a/tests/auto/qdoc/generatedoutput/expected_output/html/qdoctests-qdocfileoutput.webxml b/tests/auto/qdoc/generatedoutput/expected_output/html/qdoctests-qdocfileoutput.webxml
index 33f3c5879..5c9758445 100644
--- a/tests/auto/qdoc/generatedoutput/expected_output/html/qdoctests-qdocfileoutput.webxml
+++ b/tests/auto/qdoc/generatedoutput/expected_output/html/qdoctests-qdocfileoutput.webxml
@@ -5,6 +5,7 @@
             <contents name="supported-file-types" title="Supported file types" level="1"/>
             <contents name="further-information" title="Further information" level="1"/>
             <contents name="linking" title="Linking" level="1"/>
+            <contents name="qdoc-linking-test" title="QDoc Linking Test" level="1"/>
             <description>
                 <relation href="qdoctests-qdocfileoutput-linking.html" type="page" meta="next" description="Testing QDoc's link command"/>
                 <brief>This is a simple page for testing purposes only.</brief>
@@ -77,6 +78,10 @@
                         </item>
                     </list>
                 </section>
+                <section id="qdoc-linking-test">
+                    <heading level="1">QDoc Linking Test</heading>
+                    <para>This section title is overridden by another target which takes precedence.</para>
+                </section>
             </description>
         </page>
     </document>
diff --git a/tests/auto/qdoc/generatedoutput/expected_output/qdoctests-qdocfileoutput.html b/tests/auto/qdoc/generatedoutput/expected_output/qdoctests-qdocfileoutput.html
index 325fbdec9..ab7fc566f 100644
--- a/tests/auto/qdoc/generatedoutput/expected_output/qdoctests-qdocfileoutput.html
+++ b/tests/auto/qdoc/generatedoutput/expected_output/qdoctests-qdocfileoutput.html
@@ -18,6 +18,7 @@
 <li class="level1"><a href="#supported-file-types">Supported file types</a></li>
 <li class="level1"><a href="#further-information">Further information</a></li>
 <li class="level1"><a href="#linking">Linking</a></li>
+<li class="level1"><a href="#qdoc-linking-test">QDoc Linking Test</a></li>
 </ul>
 </div>
 <div class="sidebar-content" id="sidebar-content"></div></div>
@@ -48,6 +49,8 @@
 <li><a href="qdoctests-qdocfileoutput-linking.html#link-test-target">Linking using a \target string</a></li>
 <li><a href="qdoctests-qdocfileoutput-linking.html">Linking using a \keyword string</a></li>
 </ul>
+<h2 id="qdoc-linking-test">QDoc Linking Test</h2>
+<p>This section title is overridden by another target which takes precedence.</p>
 </div>
 <!-- @@@qdoctests-qdocfileoutput.html -->
 <p class="naviNextPrevious footerNavi">
diff --git a/tests/auto/qdoc/generatedoutput/expected_output/tocbreadcrumbs/qdoctests-qdocfileoutput.html b/tests/auto/qdoc/generatedoutput/expected_output/tocbreadcrumbs/qdoctests-qdocfileoutput.html
index 3be7cc65f..d0b3aa7f1 100644
--- a/tests/auto/qdoc/generatedoutput/expected_output/tocbreadcrumbs/qdoctests-qdocfileoutput.html
+++ b/tests/auto/qdoc/generatedoutput/expected_output/tocbreadcrumbs/qdoctests-qdocfileoutput.html
@@ -20,6 +20,7 @@
 <li class="level1"><a href="#supported-file-types">Supported file types</a></li>
 <li class="level1"><a href="#further-information">Further information</a></li>
 <li class="level1"><a href="#linking">Linking</a></li>
+<li class="level1"><a href="#qdoc-linking-test">QDoc Linking Test</a></li>
 </ul>
 </div>
 <div class="sidebar-content" id="sidebar-content"></div></div>
@@ -50,6 +51,8 @@
 <li><a href="qdoctests-qdocfileoutput-linking.html#link-test-target">Linking using a \target string</a></li>
 <li><a href="qdoctests-qdocfileoutput-linking.html">Linking using a \keyword string</a></li>
 </ul>
+<h2 id="qdoc-linking-test">QDoc Linking Test</h2>
+<p>This section title is overridden by another target which takes precedence.</p>
 </div>
 <!-- @@@qdoctests-qdocfileoutput.html -->
 <p class="naviNextPrevious footerNavi">
diff --git a/tests/auto/qdoc/generatedoutput/testdata/outputfromqdocfiles/qdoctests-outputfromqdocfiles.qdoc b/tests/auto/qdoc/generatedoutput/testdata/outputfromqdocfiles/qdoctests-outputfromqdocfiles.qdoc
index 1c7ba405d..0d14c5fff 100644
--- a/tests/auto/qdoc/generatedoutput/testdata/outputfromqdocfiles/qdoctests-outputfromqdocfiles.qdoc
+++ b/tests/auto/qdoc/generatedoutput/testdata/outputfromqdocfiles/qdoctests-outputfromqdocfiles.qdoc
@@ -46,6 +46,11 @@
     \li \l {link-test-target}{Linking using a \\target string}
     \li \l {QDoc Linking Test}{Linking using a \\keyword string}
     \endlist
+
+    \section1 QDoc Linking Test
+
+    This section title is overridden by another target which takes
+    precedence.
 */
 
 /*!