From 38d0517f8bbb9becd6e82423b34fb477355196b6 Mon Sep 17 00:00:00 2001 From: Topi Reinio Date: Fri, 10 Jun 2022 14:26:26 +0200 Subject: 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 Reviewed-by: Kai Koehne (cherry picked from commit a8cb221c7b3b07ab4b5a7dc2765248cd9350db02) Reviewed-by: Qt Cherry-pick Bot --- src/qdoc/qdocdatabase.cpp | 19 +++-- src/qdoc/tree.cpp | 84 ++++++++-------------- src/qdoc/tree.h | 15 ++-- .../docbook/qdoctests-qdocfileoutput.xml | 4 ++ .../html/qdoctests-qdocfileoutput.webxml | 5 ++ .../expected_output/qdoctests-qdocfileoutput.html | 3 + .../tocbreadcrumbs/qdoctests-qdocfileoutput.html | 3 + .../qdoctests-outputfromqdocfiles.qdoc | 5 ++ 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 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 TargetMap; typedef QMultiMap PageNodeMultiMap; typedef QMap QmlTypeMap; typedef QMultiMap ExampleNodeMap; -typedef QList 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 @@ + +QDoc Linking Test +This section title is overridden by another target which takes precedence. + 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 @@ + This is a simple page for testing purposes only. @@ -77,6 +78,10 @@ +
+ QDoc Linking Test + This section title is overridden by another target which takes precedence. +
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 @@
  • Supported file types
  • Further information
  • Linking
    • +
  • QDoc Linking Test
    • @@ -48,6 +49,8 @@
  • Linking using a \target string
  • Linking using a \keyword string
    • +

    QDoc Linking Test

    +

    This section title is overridden by another target which takes precedence.

    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 @@

  • Supported file types
  • Further information
  • Linking
    • +
  • QDoc Linking Test
    • @@ -50,6 +51,8 @@
  • Linking using a \target string
  • Linking using a \keyword string
    • +

    QDoc Linking Test

    +

    This section title is overridden by another target which takes precedence.

    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. */ /*! -- cgit v1.2.1