qdoc: Fix incorrect fragment identifier appearing in resolved links

When resolving a link target for a string, QDoc does it in stages; first, it checks if a target matches a page title. Then, it matches
against a stored map of target records that include TOC entries (section
titles), \keyword, and \target entries. Finally, a path constructed
from the string (e.g. Module::Type::Member) is matched against an API
entity.

Of these, section titles (TargetType::Contents) are handled in a special
way: Instead of returning a match immediately, QDoc checks for an API
match and only falls back to the section title if an API match fails.
This is done to give a higher priority to API entities over section
titles as link targets.

QDoc stored the type of the matched item from the map and returned it
to the calling function. However, it failed to clear the target type
when it found a higher-priority target. This, in turn, lead to QDoc
using the incorrect ref (fragment identifier) for the final link
target, if the link string matched a section title earlier in the
process.

To fix, set the targetType in Tree::findNodeForTarget() only when
returning a matched Node instance.

Streamline the implementation by extracting a recurring piece of logic
as a lambda. Remove unused enum values from TargetRec::TargetType
- these are remnants that only cause confusion.

Pick-to: 6.5
Fixes: QTBUG-116335
Change-Id: I970c2b121858ad228ef21dead51a3edb30e86450
Reviewed-by: Luca Di Sera <luca.disera@qt.io>
(cherry picked from commit 1f563d2430b3a9e3152d8b4e35bdcef34bb74b36)
Reviewed-by: Paul Wicking <paul.wicking@qt.io>

9 files changed, 58 insertions, 38 deletions

diff --git a/src/qdoc/qdoc/tree.cpp b/src/qdoc/qdoc/tree.cpp
index 900b299c3..91fa5fa72 100644
--- a/src/qdoc/qdoc/tree.cpp
+++ b/src/qdoc/qdoc/tree.cpp
@@ -446,11 +446,14 @@ Node *Tree::findNodeRecursive(const QStringList &path, int pathIndex, const Node
   begins at the root.
 
   The \a flags can indicate whether to search base classes and/or
-  the enum values in enum types. \a genus can be a further restriction
-  on what kind of node is an acceptible match, i.e. CPP or QML.
-
-  If a matching node is found, \a ref is an output parameter that
-  is set to the HTML reference to use for the link.
+  the enum values in enum types. \a genus further restricts
+  the type of nodes to match, i.e. CPP or QML.
+
+  If a matching node is found, \a ref is set to the HTML fragment
+  identifier to use for the link. On return, the optional
+  \a targetType parameter contains the type of the resolved
+  target; section title (Contents), \\target, \\keyword, or other
+  (Unknown).
  */
 const Node *Tree::findNodeForTarget(const QStringList &path, const QString &target,
                                     const Node *start, int flags, Node::Genus genus,
@@ -458,35 +461,35 @@ const Node *Tree::findNodeForTarget(const QStringList &path, const QString &targ
 {
     const Node *node = nullptr;
 
-    if ((genus == Node::DontCare) || (genus == Node::DOC)) {
-        node = findPageNodeByTitle(path.at(0));
-        if (node) {
-            if (!target.isEmpty()) {
-                if (ref = getRef(target, node); ref.isEmpty())
-                    node = nullptr;
-            }
-            if (node)
+    // Retrieves and sets ref from target for Node n.
+    // Returns n on valid (or empty) target, or nullptr on an invalid target.
+    auto set_ref_from_target = [this, &ref, &target](const Node *n) -> const Node* {
+        if (!target.isEmpty()) {
+            if (ref = getRef(target, n); ref.isEmpty())
+                return nullptr;
+        }
+        return n;
+    };
+
+    if (genus == Node::DontCare || genus == Node::DOC) {
+        if (node = findPageNodeByTitle(path.at(0)); node) {
+            if (node = set_ref_from_target(node); node)
                 return node;
         }
     }
 
     const TargetRec *result = findUnambiguousTarget(path.join(QLatin1String("::")), genus);
     if (result) {
-        node = result->m_node;
         ref = result->m_ref;
-        if (!target.isEmpty()) {
-            if (ref = getRef(target, node); ref.isEmpty())
-                node = nullptr;
-        }
-        if (node) {
-            if (targetType)
-                *targetType = result->m_type;
+        if (node = set_ref_from_target(result->m_node); node) {
             // Delay returning references to section titles as we
             // may find a better match below
-            if (!targetType || *targetType != TargetRec::Contents)
+            if (result->m_type != TargetRec::Contents) {
+                if (targetType)
+                    *targetType = result->m_type;
                 return node;
-            else
-                ref.clear();
+            }
+            ref.clear();
         }
     }
 
@@ -498,18 +501,13 @@ const Node *Tree::findNodeForTarget(const QStringList &path, const QString &targ
       the type.
     */
     int path_idx = 0;
-    if (((genus == Node::QML) || (genus == Node::DontCare)) && (path.size() >= 2)
-        && !path[0].isEmpty()) {
-        QmlTypeNode *qcn = lookupQmlType(QString(path[0] + "::" + path[1]));
-        if (qcn) {
+    if ((genus == Node::QML || genus == Node::DontCare)
+        && path.size() >= 2 && !path[0].isEmpty()) {
+        if (auto *qcn = lookupQmlType(path.sliced(0, 2).join(QLatin1String("::"))); qcn) {
             current = qcn;
-            if (path.size() == 2) {
-                if (!target.isEmpty()) {
-                    ref = getRef(target, current);
-                    return (!ref.isEmpty()) ? current : nullptr;
-                }
-                return current;
-            }
+            // No further elements in the path, return the type
+            if (path.size() == 2)
+                return set_ref_from_target(qcn);
             path_idx = 2;
         }
     }
@@ -525,8 +523,12 @@ const Node *Tree::findNodeForTarget(const QStringList &path, const QString &targ
         path_idx = 0;
     }
 
-    if (node && result)
-        ref = result->m_ref; // Restore section title's ref
+    if (node && result) {
+        // Fall back to previously found section title
+        ref = result->m_ref;
+        if (targetType)
+            *targetType = result->m_type;
+    }
     return node;
 }
 
diff --git a/src/qdoc/qdoc/tree.h b/src/qdoc/qdoc/tree.h
index 00c65a787..188746831 100644
--- a/src/qdoc/qdoc/tree.h
+++ b/src/qdoc/qdoc/tree.h
@@ -24,7 +24,7 @@ class QDocDatabase;
 struct TargetRec
 {
 public:
-    enum TargetType { Unknown, Target, Keyword, Contents, Class, Function, Page, Subtitle };
+    enum TargetType { Unknown, Target, Keyword, Contents };
 
     TargetRec(QString name, TargetRec::TargetType type, Node *node, int priority)
         : m_node(node), m_ref(std::move(name)), m_type(type), m_priority(priority)
diff --git a/tests/auto/qdoc/generatedoutput/expected_output/autolinking.html b/tests/auto/qdoc/generatedoutput/expected_output/autolinking.html
index 0b68b1de8..444c3d0d1 100644
--- a/tests/auto/qdoc/generatedoutput/expected_output/autolinking.html
+++ b/tests/auto/qdoc/generatedoutput/expected_output/autolinking.html
@@ -11,6 +11,7 @@
 <h3 id="toc">Contents</h3>
 <ul>
 <li class="level1"><a href="#testqdoc">TestQDoc</a></li>
+<li class="level1"><a href="#someprop">someProp</a></li>
 </ul>
 </div>
 <div class="sidebar-content" id="sidebar-content"></div></div>
@@ -28,6 +29,7 @@
 <li><a href="testqdoc-testderived.html" translate="no">TestQDoc::TestDerived</a></li>
 <li><a href="obsolete-classes.html#testqdoc" translate="no">Obsolete Classes#TestQDoc</a></li>
 </ul>
+<h2 id="someprop">someProp</h2>
 </div>
 <!-- @@@autolinking.html -->
 </body>
diff --git a/tests/auto/qdoc/generatedoutput/expected_output/properties-docbook/testqdoc-testderived.xml b/tests/auto/qdoc/generatedoutput/expected_output/properties-docbook/testqdoc-testderived.xml
index d875a0614..22c25d1dd 100644
--- a/tests/auto/qdoc/generatedoutput/expected_output/properties-docbook/testqdoc-testderived.xml
+++ b/tests/auto/qdoc/generatedoutput/expected_output/properties-docbook/testqdoc-testderived.xml
@@ -75,6 +75,14 @@
 <db:title>[bindable] bindableProp : QString</db:title>
 <db:para>This property supports <db:link xlink:href="https://wiki.qt.io/QProperty">QProperty</db:link> bindings.</db:para>
 <db:para>Some property.</db:para>
+<db:section>
+<db:title>See Also</db:title>
+<db:para><db:emphasis>See also </db:emphasis>
+<db:simplelist type="vert" role="see-also">
+<db:member><db:link xlink:href="testqdoc-testderived.xml#someProp-prop">someProp</db:link></db:member>
+</db:simplelist>
+</db:para>
+</db:section>
 </db:section>
 <db:section xml:id="boolProp-prop">
 <db:title>boolProp : bool</db:title>
diff --git a/tests/auto/qdoc/generatedoutput/expected_output/properties/testcpp.index b/tests/auto/qdoc/generatedoutput/expected_output/properties/testcpp.index
index 28cc5d998..09b3ebb3c 100644
--- a/tests/auto/qdoc/generatedoutput/expected_output/properties/testcpp.index
+++ b/tests/auto/qdoc/generatedoutput/expected_output/properties/testcpp.index
@@ -8,6 +8,7 @@
         </function>
         <page name="autolinking.html" href="autolinking.html" status="active" location="classlists.qdoc" documented="true" subtype="page" title="Autolinking" fulltitle="Autolinking" subtitle="">
             <contents name="testqdoc" title="TestQDoc" level="1"/>
+            <contents name="someprop" title="someProp" level="1"/>
         </page>
         <namespace name="CrossModuleRef" href="crossmoduleref.html" status="active" access="public" location="testcpp.h" since="3.0" documented="true" module="TestCPP" brief="Namespace that has documented functions in multiple modules">
             <function name="documentMe" fullname="CrossModuleRef::documentMe" href="crossmoduleref.html#documentMe" status="active" access="public" location="testcpp.h" documented="true" meta="plain" type="void" signature="void documentMe()"/>
diff --git a/tests/auto/qdoc/generatedoutput/expected_output/properties/testqdoc-testderived.html b/tests/auto/qdoc/generatedoutput/expected_output/properties/testqdoc-testderived.html
index 1b0a6e087..6a947bdbc 100644
--- a/tests/auto/qdoc/generatedoutput/expected_output/properties/testqdoc-testderived.html
+++ b/tests/auto/qdoc/generatedoutput/expected_output/properties/testqdoc-testderived.html
@@ -107,6 +107,7 @@ target_link_libraries(mytarget PRIVATE Qt6::QDocTest)</td></tr>
 <h3 class="fn" translate="no" id="bindableProp-prop"><code translate="no">[bindable] </code><span class="name">bindableProp</span> : <span class="type">QString</span></h3>
 <div class="admonition note"><p><b>Note: </b>This property supports <a href="https://wiki.qt.io/QProperty" translate="no">QProperty</a> bindings.</p>
 </div><p>Some property.</p>
+<p><b>See also </b><a href="testqdoc-testderived.html#someProp-prop" translate="no">someProp</a>.</p>
 <!-- @@@bindableProp -->
 <!-- $$$boolProp-prop$$$boolProp$$$setBoolPropbool$$$resetBoolProp$$$boolPropChanged -->
 <h3 class="fn" translate="no" id="boolProp-prop"><span class="name">boolProp</span> : <span class="type">bool</span></h3>
diff --git a/tests/auto/qdoc/generatedoutput/expected_output/testcpp.index b/tests/auto/qdoc/generatedoutput/expected_output/testcpp.index
index 44157278b..8ad29a1ee 100644
--- a/tests/auto/qdoc/generatedoutput/expected_output/testcpp.index
+++ b/tests/auto/qdoc/generatedoutput/expected_output/testcpp.index
@@ -8,6 +8,7 @@
         </function>
         <page name="autolinking.html" href="autolinking.html" status="active" location="classlists.qdoc" documented="true" subtype="page" title="Autolinking" fulltitle="Autolinking" subtitle="">
             <contents name="testqdoc" title="TestQDoc" level="1"/>
+            <contents name="someprop" title="someProp" level="1"/>
         </page>
         <namespace name="CrossModuleRef" href="crossmoduleref.html" status="active" access="public" location="testcpp.h" since="3.0" documented="true" module="TestCPP" brief="Namespace that has documented functions in multiple modules">
             <function name="documentMe" fullname="CrossModuleRef::documentMe" href="crossmoduleref.html#documentMe" status="active" access="public" location="testcpp.h" documented="true" meta="plain" type="void" signature="void documentMe()"/>
diff --git a/tests/auto/qdoc/generatedoutput/testdata/testcpp/classlists.qdoc b/tests/auto/qdoc/generatedoutput/testdata/testcpp/classlists.qdoc
index 89ed2a1b5..2954e5beb 100644
--- a/tests/auto/qdoc/generatedoutput/testdata/testcpp/classlists.qdoc
+++ b/tests/auto/qdoc/generatedoutput/testdata/testcpp/classlists.qdoc
@@ -33,6 +33,9 @@
         \li \l [CPP] {TestQDoc::TestDerived}
         \li \l {Obsolete Classes#TestQDoc}
     \endlist
+
+    //! a section title shadowing a known property name
+    \section1 someProp
 */
 
 /*!
diff --git a/tests/auto/qdoc/generatedoutput/testdata/testcpp/properties.qdoc b/tests/auto/qdoc/generatedoutput/testdata/testcpp/properties.qdoc
index 97438e1ae..79cb8c781 100644
--- a/tests/auto/qdoc/generatedoutput/testdata/testcpp/properties.qdoc
+++ b/tests/auto/qdoc/generatedoutput/testdata/testcpp/properties.qdoc
@@ -4,6 +4,8 @@
 /*!
     \property TestQDoc::TestDerived::bindableProp
     Some property.
+
+    \sa someProp
 */
 
 /*!