qdoc: Major clean-up of FunctionNode and parameter processingv5.13.0-alpha1

This update was motivated by the need to correct two known issues
in qdoc. First, linking to overloaded functions failed in some cases
because the overloads were not numbered consistently, causing links
to go to the wrong overload or to nowhere at all. Second, the
mechanism for handling the \relates command didn't support using it to
relate a function to more than one class.

For example, there are many global qHash() functions spread
around QtBase. Each is meant to compute a hash index for an
object of some class type, so the object can be inserted into a
QHash map for that class type. It is then desired to relate
qHash(type Xxx, int seed) to both QHash<type> and class Xxx, so
that the documentation for that qHash() function appears as a
related non-member function of both QHash<type> and class Xxx.

The example above also illustrates the overload numbering problem,
because all these qHash() functions are overloads of the name
qHash. To make matters worse, they are not all in the same module.
Most of them are in QtCore, but a few are in QtNetwork, and this
distribution problem will become worse over time as more qHash()
functions are added. Prior to this update, qdoc was unable to
relate a function to something in a different module, or it didn't
always work.

While designing a fix for these issues, it became clear that the
processing of the FunctionNode and the function parameters would
have to be rewritten. That's what this update does. These are
the main points:

1. A new subclass of Node is added to act as a proxy for a class
in another module. This ProxyNode acts as a place holder for the
functions (and possibly other elements) that are related to a
class in another module. This is used for the qHash() functions
in QtNetwork that are related to QHash in QtCore. qdoc generates
an html file named qtnetwork/qhash-proxy.html that contains the
documentation for these functions. But these functions are listed
as related non-members on the QHash class reference page in the
qtcore output directory. They are listed there in the summary,
but they link to the qhash-proxy.html page in qtnetwork.

2. A new, Parameters class is added to qdoc (parameters.h and
parameters.cpp), and the class Parameter is moved there from
node.h. class Parameters replaces the old QVector<Parameter>
wherever it was used. This encapsulates all the parameter
processing and matching in the Parameters class and simplifies
the code at all the places where QVector<Parameter> had been
used.

3. The assignment of overload numbers is now done in the
normalizeOverloads() function, which is called after all the
headers and sources have been processed but before the
generate phase begins. This assignment is a simple renumbering
now because all the overloads of a function are linked to
each other via a nextOverload_ link in the FunctionNode. The
first function named qHash() is inserted into the Aggregate
node's function map, but subsequent qHash() FunctionNodes
are not entered into the function map but are linked to the
first qHash() via its nextOverload_ link.

4. The \relates command can now be used multiple times in a
single qdoc comment. There remains some work to be done here
because this currently only works for global entities, but
there are several cases where \relates has been used in the
qdoc comment of a member of a class. This will be fixed soon,
I believe.

When qdoc sees the first \relates Xxx command, for example
for qHash(Yyy, seed), that qHash() is a child of the global
namespace. qdoc allows it to remain as a child of the global
namespace but it tells class Xxx to "adopt" that child (see
Node::adoptChild()). This function makes this instance of
qHash() be a child of class Xxx (in this case QHash<type>),
so that the parent of this qHash() becomes Xxx. After this
"adoption," qHash() is a child of both the global namespace
and class Xxx, but qHash() only knows it is a child of Xxx,
i.e. its parent pointer is Xxx. If this is the first qHash()
to become a child of Xxx, it is inserted into the function
map of Xxx, but its nextOverload_ link is not changed. This
is because all the global qHash() functions have already been
linked into the nextOverload_ linked list, and this list must
not be changed. Hence, when qdoc searches for qHash(something)
to make a link to it, it will find it as a child of the global
namespace, but it will correctly link to it using its actual
parent pointer.

When qdoc sees the second \relates Yyy for this qHash()
function, qdoc sees that this FunctionNode has already been
made a related non-member of Xxx, so it can't let Yyy "adopt"
it. Instead, it tells Yyy to clone this qHash(), which creates
a shallow copy of it but resets its nextOverload_ pointer to
nullptr. I believe this clone of qHash() won't be found in a
search for a function named qHash(), because the global one
(the adopted one) will be found first. Or, if it is found, a
link to the clone will be generated, but that's ok because
the documentation is identical.

Note that the existence of qHash in two child lists is taken
into account at destruction time. The only place where a Node
is destroyed is in the destructor of Tree, which destroys the
Node tree from the root down to the leaves. Each aggregate
node is responsible for deleting each of its child nodes, but
it only deletes a child node if it is the parent of that child
node.

All of the above revealed that some of the findFunctionNode()
functions were either no longer needed or weren't being called
in the first place, so they were deleted.

This change is now ready for testing.

Change-Id: I6da3e2e9e71d39a29d90e073ed614309a49e3d4c
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Paul Wicking <paul.wicking@qt.io>

1 files changed, 106 insertions, 131 deletions

diff --git a/src/qdoc/generator.cpp b/src/qdoc/generator.cpp
index 9d234475f..8a9e4b8e7 100644
--- a/src/qdoc/generator.cpp
+++ b/src/qdoc/generator.cpp
@@ -365,11 +365,9 @@ static void transmogrify(QString &input, QString &output)
 
 QString Generator::fileBase(const Node *node) const
 {
-    if (node->relates())
-        node = node->relates();
-    else if (!node->isPageNode() && !node->isCollectionNode())
+    if (!node->isPageNode() && !node->isCollectionNode())
         node = node->parent();
-    if (node->isQmlPropertyGroup())
+    if (node->isQmlPropertyGroup() || node->isJsPropertyGroup())
         node = node->parent();
 
     if (node->hasFileNameBase())
@@ -403,9 +401,8 @@ QString Generator::fileBase(const Node *node) const
         if (node->isExample()) {
             base.append(QLatin1String("-example"));
         }
-    }
-    else if (node->isQmlType() || node->isQmlBasicType() ||
-             node->isJsType() || node->isJsBasicType()) {
+    } else if (node->isQmlType() || node->isQmlBasicType() ||
+               node->isJsType() || node->isJsBasicType()) {
         base = node->name();
         /*
           To avoid file name conflicts in the html directory,
@@ -419,8 +416,10 @@ QString Generator::fileBase(const Node *node) const
                          + QLatin1Char('-'));
         }
         base.prepend(outputPrefix(node));
-    }
-    else {
+    } else if (node->isProxyNode()) {
+        base = node->name();
+        base.append("-proxy");
+    } else {
         const Node *p = node;
         forever {
             const Node *pp = p->parent();
@@ -600,10 +599,7 @@ QString Generator::fullDocumentLocation(const Node *node, bool useSubdir)
 
     Node *parentNode = 0;
 
-    if ((parentNode = node->relates())) {
-        parentName = fullDocumentLocation(node->relates());
-    }
-    else if ((parentNode = node->parent())) {
+    if ((parentNode = node->parent())) {
         if (parentNode->isQmlPropertyGroup() || parentNode->isJsPropertyGroup()) {
             parentNode = parentNode->parent();
             parentName = fullDocumentLocation(parentNode);
@@ -616,6 +612,7 @@ QString Generator::fullDocumentLocation(const Node *node, bool useSubdir)
     switch (node->nodeType()) {
     case Node::Class:
     case Node::Namespace:
+    case Node::Proxy:
         parentName = fileBase(node) + QLatin1Char('.') + currentGenerator()->fileExtension();
         break;
     case Node::Function:
@@ -843,28 +840,25 @@ void Generator::generateBody(const Node *node, CodeMarker *marker)
                 generateText(text, node, marker);
                 out() << "</p>";
             }
-            else if (!node->isWrapper() && !node->isReimplemented()) {
+            else if (!node->isWrapper() && !node->isMarkedReimp()) {
                 if (!func->isIgnored()) // undocumented functions added by Q_OBJECT
                     node->location().warning(tr("No documentation for '%1'").arg(node->plainSignature()));
             }
-        }
-        else if (!node->isWrapper() && !node->isReimplemented()) {
-            /*
-              Don't require documentation of things defined in Q_GADGET
-             */
+        } else if (!node->isWrapper() && !node->isMarkedReimp()) {
+            // Don't require documentation of things defined in Q_GADGET
             if (node->name() != QLatin1String("QtGadgetHelper"))
                 node->location().warning(tr("No documentation for '%1'").arg(node->plainSignature()));
         }
     }
     else if (!node->isSharingComment()) {
         if (node->isFunction()) {
-            const FunctionNode *func = static_cast<const FunctionNode *>(node);
-            if (!func->reimplementedFrom().isEmpty())
-                generateReimplementedFrom(func, marker);
+            const FunctionNode *fn = static_cast<const FunctionNode *>(node);
+            if (!fn->overridesThis().isEmpty())
+                generateReimplementsClause(fn, marker);
         }
 
         if (!generateText(node->doc().body(), node, marker)) {
-            if (node->isReimplemented())
+            if (node->isMarkedReimp())
                 return;
         }
 
@@ -901,62 +895,48 @@ void Generator::generateBody(const Node *node, CodeMarker *marker)
                 }
             }
         } else if (node->isFunction()) {
-            const FunctionNode *func = static_cast<const FunctionNode *>(node);
-            QSet<QString> definedParams;
-            QVector<Parameter>::ConstIterator p = func->parameters().constBegin();
-            while (p != func->parameters().constEnd()) {
-                if (!(*p).name().isEmpty())
-                    definedParams.insert((*p).name());
-                ++p;
-            }
-
-            QSet<QString> documentedParams = func->doc().parameterNames();
-            QSet<QString> allParams = definedParams + documentedParams;
-            if (allParams.count() > definedParams.count()
-                    || allParams.count() > documentedParams.count()) {
-                QSet<QString>::ConstIterator a = allParams.constBegin();
-                while (a != allParams.constEnd()) {
-                    if (!definedParams.contains(*a)) {
+            const FunctionNode *fn = static_cast<const FunctionNode *>(node);
+            QSet<QString> declaredNames;
+            fn->parameters().getNames(declaredNames);
+            QSet<QString> documentedNames = fn->doc().parameterNames();
+            if (declaredNames != documentedNames) {
+                QSet<QString>::const_iterator i = declaredNames.constBegin();
+                while (i != declaredNames.constEnd()) {
+                    if (!documentedNames.contains(*i)) {
+                        if (fn->isActive() || fn->isPreliminary()) {
+                            if (!fn->isMarkedReimp() && !fn->isOverload()) {
+                                fn->doc().location().warning(
+                                          tr("Undocumented parameter '%1' in %2")
+                                          .arg(*i).arg(node->plainFullName()));
+                            }
+                        }
+                    }
+                    ++i;
+                }
+                i = documentedNames.constBegin();
+                while (i != documentedNames.constEnd()) {
+                    if (!declaredNames.contains(*i)) {
+                        QString best = nearestName(*i, declaredNames);
                         QString details;
-                        QString best = nearestName(*a, definedParams);
                         if (!best.isEmpty())
                             details = tr("Maybe you meant '%1'?").arg(best);
-
-                        node->doc().location().warning(
-                                    tr("No such parameter '%1' in %2").arg(*a).arg(node->plainFullName()),
-                                    details);
-                    }
-                    else if (!(*a).isEmpty() && !documentedParams.contains(*a)) {
-                        bool needWarning = (func->status() > Node::Obsolete);
-                        if (func->overloadNumber() > 0) {
-                            FunctionNode *primaryFunc = func->parent()->findFunctionNode(func->name(), QString());
-                            if (primaryFunc) {
-                                foreach (const Parameter &param,
-                                         primaryFunc->parameters()) {
-                                    if (param.name() == *a) {
-                                        needWarning = false;
-                                        break;
-                                    }
-                                }
-                            }
-                        }
-                        if (needWarning && !func->isReimplemented() && !func->isOverload())
-                            node->doc().location().warning(
-                                        tr("Undocumented parameter '%1' in %2")
-                                        .arg(*a).arg(node->plainFullName()));
+                        fn->doc().location().warning(tr("No such parameter '%1' in %2")
+                                                     .arg(*i).arg(fn->plainFullName()),
+                                                     details);
                     }
-                    ++a;
+                    ++i;
                 }
             }
             /*
-              Something like this return value check should
-              be implemented at some point.
+              This return value check should be implemented
+              for all functions with a return type.
+              mws 13/12/2018
             */
-            if (func->status() > Node::Obsolete && func->returnType() == "bool"
-                    && func->reimplementedFrom() == 0 && !func->isOverload()) {
-                QString body = func->doc().body().toString();
-                if (!body.contains("return", Qt::CaseInsensitive))
-                    node->doc().location().warning(tr("Undocumented return value"));
+            if (!fn->isObsolete() && fn->returnsBool() &&
+                !fn->isMarkedReimp() && !fn->isOverload()) {
+                if (!fn->doc().body().contains("return"))
+                    node->doc().location().warning(tr("Undocumented return value "
+                                           "(hint: use 'return' or 'returns' in the text"));
             }
         }
     }
@@ -1146,6 +1126,16 @@ void Generator::generateDocumentation(Node* node)
                 beginSubPage(node, fileName(node));
                 generateCollectionNode(cn, marker);
                 endSubPage();
+            } else if (cn->isGenericCollection()) {
+                // Currently used only for the module's related orphans page
+                // but can be generalized for other kinds of collections if
+                // other use cases pop up.
+                QString name = cn->name().toLower();
+                name.replace(QChar(' '), QString("-"));
+                QString filename = cn->tree()->physicalModuleName() + "-" + name + "." + fileExtension();
+                beginSubPage(node, filename);
+                generateGenericCollectionPage(cn, marker);
+                endSubPage();
             }
         } else if (node->isTextPageNode()) {
             beginSubPage(node, fileName(node));
@@ -1157,30 +1147,30 @@ void Generator::generateDocumentation(Node* node)
                 beginSubPage(node, fileName(node));
                 generateCppReferencePage(static_cast<Aggregate*>(node), marker);
                 endSubPage();
-            }
-            else if (node->isQmlType() || node->isJsType()) {
+            } else if (node->isQmlType() || node->isJsType()) {
                 beginSubPage(node, fileName(node));
                 QmlTypeNode* qcn = static_cast<QmlTypeNode*>(node);
                 generateQmlTypePage(qcn, marker);
                 endSubPage();
-            }
-            else if (node->isQmlBasicType() || node->isJsBasicType()) {
+            } else if (node->isQmlBasicType() || node->isJsBasicType()) {
                 beginSubPage(node, fileName(node));
                 QmlBasicTypeNode* qbtn = static_cast<QmlBasicTypeNode*>(node);
                 generateQmlBasicTypePage(qbtn, marker);
                 endSubPage();
+            } else if (node->isProxyNode()) {
+                beginSubPage(node, fileName(node));
+                generateProxyPage(static_cast<Aggregate*>(node), marker);
+                endSubPage();
             }
         }
     }
 
     if (node->isAggregate()) {
         Aggregate* aggregate = static_cast<Aggregate*>(node);
-        int i = 0;
-        while (i < aggregate->childNodes().count()) {
-            Node *c = aggregate->childNodes().at(i);
-            if (c->isPageNode() && !c->isPrivate())
-                generateDocumentation(c);
-            ++i;
+        const NodeList &children = aggregate->childNodes();
+        foreach (Node *n, children) {
+            if (n->isPageNode() && !n->isPrivate())
+                generateDocumentation(n);
         }
     }
 }
@@ -1258,17 +1248,17 @@ bool Generator::generateQmlText(const Text& text,
     return result;
 }
 
-void Generator::generateReimplementedFrom(const FunctionNode *fn, CodeMarker *marker)
+void Generator::generateReimplementsClause(const FunctionNode *fn, CodeMarker *marker)
 {
-    if (!fn->reimplementedFrom().isEmpty()) {
+    if (!fn->overridesThis().isEmpty()) {
         if (fn->parent()->isClass()) {
             ClassNode* cn = static_cast<ClassNode*>(fn->parent());
-            const FunctionNode *from = cn->findOverriddenFunction(fn);
-            if (from && from->access() != Node::Private && from->parent()->access() != Node::Private) {
+            const FunctionNode *overrides = cn->findOverriddenFunction(fn);
+            if (overrides && !overrides->isPrivate() && !overrides->parent()->isPrivate()) {
                 Text text;
-                text << Atom::ParaLeft << "Reimplemented from ";
-                QString fullName =  from->parent()->name() + "::" + from->name() + "()";
-                appendFullName(text, from->parent(), fullName, from);
+                text << Atom::ParaLeft << "Reimplements: ";
+                QString fullName =  overrides->parent()->name() + "::" + overrides->signature(false, true);
+                appendFullName(text, overrides->parent(), fullName, overrides);
                 text << "." << Atom::ParaRight;
                 generateText(text, fn, marker);
             }
@@ -1423,30 +1413,28 @@ static bool hasExceptions(const Node* node,
 {
     bool result = false;
     Node::ThreadSafeness ts = node->threadSafeness();
-    const Aggregate* a = static_cast<const Aggregate*>(node);
-    NodeList::ConstIterator c = a->childNodes().constBegin();
-    while (c != a->childNodes().constEnd()) {
-        if (!(*c)->isObsolete()){
-            switch ((*c)->threadSafeness()) {
+    const NodeList &children = static_cast<const Aggregate*>(node)->childNodes();
+    foreach (Node *n, children) {
+        if (!n->isObsolete()){
+            switch (n->threadSafeness()) {
             case Node::Reentrant:
-                reentrant.append(*c);
+                reentrant.append(n);
                 if (ts == Node::ThreadSafe)
                     result = true;
                 break;
             case Node::ThreadSafe:
-                threadsafe.append(*c);
+                threadsafe.append(n);
                 if (ts == Node::Reentrant)
                     result = true;
                 break;
             case Node::NonReentrant:
-                nonreentrant.append(*c);
+                nonreentrant.append(n);
                 result = true;
                 break;
             default:
                 break;
             }
         }
-        ++c;
     }
     return result;
 }
@@ -1575,18 +1563,19 @@ void Generator::generateThreadSafeness(const Node *node, CodeMarker *marker)
 
 /*!
     If the node is an overloaded signal, and a node with an example on how to connect to it
+
+    Someone didn't finish writing this comment, and I don't know what this
+    function is supposed to do, so I have not tried to complete the comment
+    yet.
  */
 void Generator::generateOverloadedSignal(const Node* node, CodeMarker* marker)
 {
     if (!node->isFunction())
         return;
     const FunctionNode *func = static_cast<const FunctionNode *>(node);
-    if (!func->isSignal())
-        return;
-    if (node->parent()->overloads(node->name()).count() <= 1)
+    if (!func->isSignal() || !func->hasOverloads())
         return;
 
-
     // Compute a friendly name for the object of that instance.
     // e.g:  "QAbstractSocket" -> "abstractSocket"
     QString objectName = node->parent()->name();
@@ -1601,23 +1590,9 @@ void Generator::generateOverloadedSignal(const Node* node, CodeMarker* marker)
     // it is very unlikely that we will ever have public API overloading
     // signals by const.
     QString code = "connect(" + objectName + ", QOverload<";
-    for (int i = 0; i < func->parameters().size(); ++i) {
-        if (i != 0)
-            code += ", ";
-        code += func->parameters().at(i).dataType();
-    }
-
+    func->parameters().getTypeList(code);
     code += ">::of(&" + func->parent()->name() + "::" + func->name() + "),\n    [=](";
-
-    for (int i = 0; i < func->parameters().size(); ++i) {
-        if (i != 0)
-            code += ", ";
-        const Parameter &p = func->parameters().at(i);
-        code += p.dataType();
-        if (code[code.size()-1].isLetterOrNumber())
-            code += QLatin1Char(' ');
-        code += p.name();
-    }
+    func->parameters().getTypeAndNameList(code);
 
     code += "){ /* ... */ });";
 
@@ -2118,30 +2093,30 @@ void Generator::initializeTextOutput()
 void Generator::supplementAlsoList(const Node *node, QList<Text> &alsoList)
 {
     if (node->isFunction() && !node->isMacro()) {
-        const FunctionNode *func = static_cast<const FunctionNode *>(node);
-        if (func->overloadNumber() == 0) {
+        const FunctionNode *fn = static_cast<const FunctionNode *>(node);
+        if (fn->overloadNumber() == 0) {
             QString alternateName;
             const FunctionNode *alternateFunc = 0;
 
-            if (func->name().startsWith("set") && func->name().size() >= 4) {
-                alternateName = func->name()[3].toLower();
-                alternateName += func->name().mid(4);
-                alternateFunc = func->parent()->findFunctionNode(alternateName, QString());
+            if (fn->name().startsWith("set") && fn->name().size() >= 4) {
+                alternateName = fn->name()[3].toLower();
+                alternateName += fn->name().mid(4);
+                alternateFunc = fn->parent()->findFunctionChild(alternateName, QString());
 
                 if (!alternateFunc) {
-                    alternateName = "is" + func->name().mid(3);
-                    alternateFunc = func->parent()->findFunctionNode(alternateName, QString());
+                    alternateName = "is" + fn->name().mid(3);
+                    alternateFunc = fn->parent()->findFunctionChild(alternateName, QString());
                     if (!alternateFunc) {
-                        alternateName = "has" + func->name().mid(3);
-                        alternateFunc = func->parent()->findFunctionNode(alternateName, QString());
+                        alternateName = "has" + fn->name().mid(3);
+                        alternateFunc = fn->parent()->findFunctionChild(alternateName, QString());
                     }
                 }
             }
-            else if (!func->name().isEmpty()) {
+            else if (!fn->name().isEmpty()) {
                 alternateName = "set";
-                alternateName += func->name()[0].toUpper();
-                alternateName += func->name().mid(1);
-                alternateFunc = func->parent()->findFunctionNode(alternateName, QString());
+                alternateName += fn->name()[0].toUpper();
+                alternateName += fn->name().mid(1);
+                alternateFunc = fn->parent()->findFunctionChild(alternateName, QString());
             }
 
             if (alternateFunc && alternateFunc->access() != Node::Private) {