qdoc: Fix issues with shared comment nodes

QDoc stores <sharedcomment> elements to the .index correctly, but
the nodes that were sharing a comment were added after the
shared comment. This meant that a shared comment node (SCN) could
not find any of its members as they hadn't been read in yet.

Fix this by delaying the creation of a new SCN until we have a list
of sharing nodes.

Also fix other related issues:

 - Generate content also for non-function SCN members.

 - Make SCN adopt the genus of a member when adding one.

 - Fix Node::isPropertyGroup() to return true for actual property
   groups (not all QML properties that share a comment) and fix usage
   of that method.

 - Fix linking to property groups when searching by the group name.

 - Restore indentation of property group members both in the summary
   section and in the All Members file.

Fixes: QTBUG-79204
Change-Id: I1702f39b178f52444436b800d4fe9cb99f976a60
Reviewed-by: Paul Wicking <paul.wicking@qt.io>
Reviewed-by: Martin Smith <martin.smith@qt.io>

6 files changed, 82 insertions, 67 deletions

diff --git a/src/qdoc/cppcodeparser.cpp b/src/qdoc/cppcodeparser.cpp
index 47f3f52a6..b05e2ab3c 100644
--- a/src/qdoc/cppcodeparser.cpp
+++ b/src/qdoc/cppcodeparser.cpp
@@ -458,22 +458,11 @@ void CppCodeParser::processQmlProperties(const Doc &doc, NodeList &nodes, DocLis
             group = property.left(i);
     }
 
+    NodeList sharedNodes;
     QmlTypeNode *qmlType = qdb_->findQmlType(module, qmlTypeName);
     if (qmlType == nullptr)
         qmlType = new QmlTypeNode(qdb_->primaryTreeRoot(), qmlTypeName);
 
-    SharedCommentNode *scn = nullptr;
-    if (topics.size() > 1) {
-        scn = new SharedCommentNode(qmlType, topics.size(), group);
-        scn->setLocation(doc.startLocation());
-        if (jsProps)
-            scn->setGenus(Node::JS);
-        else
-            scn->setGenus(Node::QML);
-        nodes.append(scn);
-        docs.append(doc);
-    }
-
     for (int i=0; i<topics.size(); ++i) {
         QString cmd = topics.at(i).topic;
         arg = topics.at(i).args;
@@ -492,20 +481,29 @@ void CppCodeParser::processQmlProperties(const Doc &doc, NodeList &nodes, DocLis
                     continue;
                 }
                 QmlPropertyNode *qpn = new QmlPropertyNode(qmlType, property, type, attached);
-                if (scn != nullptr)
-                    qpn->setSharedCommentNode(scn);
                 qpn->setLocation(doc.startLocation());
-                if (jsProps)
-                    qpn->setGenus(Node::JS);
-                else
-                    qpn->setGenus(Node::QML);
+                qpn->setGenus(jsProps ? Node::JS : Node::QML);
                 nodes.append(qpn);
                 docs.append(doc);
+                sharedNodes << qpn;
             }
         } else {
             doc.startLocation().warning(tr("Command '\\%1'; not allowed with QML/JS property commands").arg(cmd));
         }
     }
+
+    // Construct a SharedCommentNode (scn) if multiple topics generated
+    // valid nodes. Note that it's important to do this *after* constructing
+    // the topic nodes - which need to be written to index before the related
+    // scn.
+    if (sharedNodes.count() > 1) {
+        SharedCommentNode *scn = new SharedCommentNode(qmlType, sharedNodes.count(), group);
+        scn->setLocation(doc.startLocation());
+        nodes.append(scn);
+        docs.append(doc);
+        for (const auto n : sharedNodes)
+            scn->append(n);
+    }
 }
 
 /*!
@@ -1005,7 +1003,7 @@ void CppCodeParser::processTopicArgs(const Doc &doc, const QString &topic, NodeL
                     bool found = false;
                     for (SharedCommentNode *scn : sharedCommentNodes) {
                         if (scn->parent() == node->parent()) {
-                            node->setSharedCommentNode(scn);
+                            scn->append(node);
                             found = true;
                             break;
                         }
diff --git a/src/qdoc/htmlgenerator.cpp b/src/qdoc/htmlgenerator.cpp
index 6905483e9..0430e832f 100644
--- a/src/qdoc/htmlgenerator.cpp
+++ b/src/qdoc/htmlgenerator.cpp
@@ -2738,20 +2738,30 @@ QString HtmlGenerator::generateAllQmlMembersFile(const Sections &sections, CodeM
             }
             out() << "<ul>\n";
             for (int j=0; j<keys.size(); j++) {
-                if (nodes[j]->access() == Node::Private || nodes[j]->isInternal()) {
+                Node *node = nodes[j];
+                if (node->access() == Node::Private || node->isInternal())
                     continue;
-                }
-                out() << "<li class=\"fn\">";
-                QString prefix;
-                if (!keys.isEmpty()) {
-                    prefix = keys.at(j).mid(1);
-                    prefix = prefix.left(keys.at(j).indexOf("::")+1);
-                }
-                generateQmlItem(nodes[j], aggregate, marker, true);
-                if (nodes[j]->isAttached())
-                    out() << " [attached]";
-                //generateSynopsis(nodes[j], qcn, marker, Section::AllMembers, false, &prefix);
-                out() << "</li>\n";
+                if (node->isSharingComment() &&
+                    node->sharedCommentNode()->isPropertyGroup())
+                    continue;
+
+                std::function<void(Node *)> generate = [&](Node *n) {
+                    out() << "<li class=\"fn\">";
+                    generateQmlItem(n, aggregate, marker, true);
+                    if (n->isDefault())
+                        out() << " [default]";
+                    else if (n->isAttached())
+                        out() << " [attached]";
+                    // Indent property group members
+                    if (n->isPropertyGroup()) {
+                        out() << "<ul>\n";
+                        const QVector<Node *> &collective = static_cast<SharedCommentNode *>(n)->collective();
+                        std::for_each(collective.begin(), collective.end(), generate);
+                        out() << "</ul>\n";
+                    }
+                    out() << "</li>\n";
+                };
+                generate(node);
             }
             out() << "</ul>\n";
         }
@@ -4038,13 +4048,11 @@ void HtmlGenerator::generateDetailedMember(const Node *node,
         if (collective.size() > 1)
             out() << "<div class=\"fngroup\">\n";
         for (const auto *node : collective) {
-            if (node->isFunction()) {
-                nodeRef = refForNode(node);
-                out() << "<h3 class=\"fn fngroupitem\" id=\"" << nodeRef << "\">";
-                out() << "<a name=\"" + nodeRef + "\"></a>";
-                generateSynopsis(node, relative, marker, Section::Details);
-                out() << "</h3>";
-            }
+            nodeRef = refForNode(node);
+            out() << "<h3 class=\"fn fngroupitem\" id=\"" << nodeRef << "\">";
+            out() << "<a name=\"" + nodeRef + "\"></a>";
+            generateSynopsis(node, relative, marker, Section::Details);
+            out() << "</h3>";
         }
         if (collective.size() > 1)
             out() << "</div>";
diff --git a/src/qdoc/node.cpp b/src/qdoc/node.cpp
index 06f2d6bd6..839dd1336 100644
--- a/src/qdoc/node.cpp
+++ b/src/qdoc/node.cpp
@@ -1239,15 +1239,6 @@ void Node::setLocation(const Location& t)
 }
 
 /*!
-  Adds this node to the shared comment node \a t.
- */
-void Node::setSharedCommentNode(SharedCommentNode *t)
-{
-    sharedCommentNode_ = t;
-    t->append(this);
-}
-
-/*!
   Returns true if this node is sharing a comment and the
   shared comment is not empty.
  */
@@ -2455,10 +2446,10 @@ void Aggregate::addChild(Node *child)
     child->setOutputSubdirectory(this->outputSubdirectory());
     child->setUrl(QString());
     child->setIndexNodeFlag(isIndexNode());
+
     if (child->isFunction()) {
         addFunction(static_cast<FunctionNode *>(child));
-    }
-    else {
+    } else if (!child->name().isEmpty()) {
         nonfunctionMap_.insertMulti(child->name(), child);
         if (child->isEnumType())
             enumChildren_.append(child);
@@ -2483,16 +2474,15 @@ void Aggregate::adoptChild(Node *child)
         child->setParent(this);
         if (child->isFunction()) {
             adoptFunction(static_cast<FunctionNode *>(child));
-        }
-        else {
+        } else if (!child->name().isEmpty()) {
             nonfunctionMap_.insertMulti(child->name(), child);
-            if (child->isEnumType()) {
+            if (child->isEnumType())
                 enumChildren_.append(child);
-            } else if (child->isSharedCommentNode()) {
-                SharedCommentNode *scn = static_cast<SharedCommentNode *>(child);
-                for (Node *n : scn->collective())
-                    adoptChild(n);
-            }
+        }
+        if (child->isSharedCommentNode()) {
+            SharedCommentNode *scn = static_cast<SharedCommentNode *>(child);
+            for (Node *n : scn->collective())
+                adoptChild(n);
         }
     }
 }
diff --git a/src/qdoc/node.h b/src/qdoc/node.h
index 71a603146..0b6eb9e62 100644
--- a/src/qdoc/node.h
+++ b/src/qdoc/node.h
@@ -323,7 +323,8 @@ public:
 
     bool isSharingComment() const { return (sharedCommentNode_ != nullptr); }
     bool hasSharedDoc() const;
-    void setSharedCommentNode(SharedCommentNode *t);
+    void setSharedCommentNode(SharedCommentNode *t) { sharedCommentNode_ = t; }
+    SharedCommentNode *sharedCommentNode() { return sharedCommentNode_; }
 
     //QString guid() const;
     QString extractClassName(const QString &string) const;
@@ -899,7 +900,8 @@ class SharedCommentNode : public Node
 public:
     SharedCommentNode(Node *n)
         : Node(Node::SharedComment, n->parent(), QString()) {
-        collective_.reserve(1); n->setSharedCommentNode(this);
+        collective_.reserve(1);
+        append(n);
     }
  SharedCommentNode(QmlTypeNode *parent, int count, QString &group)
         : Node(Node::SharedComment, parent, group) {
@@ -907,11 +909,13 @@ public:
     }
     ~SharedCommentNode() override { collective_.clear(); }
 
-    bool isPropertyGroup() const override { return !collective_.isEmpty() &&
-            (collective_.at(0)->isQmlProperty() || collective_.at(0)->isJsProperty());
+    bool isPropertyGroup() const override {
+        return !name().isEmpty() &&
+               !collective_.isEmpty() &&
+               (collective_.at(0)->isQmlProperty() || collective_.at(0)->isJsProperty());
     }
     int count() const { return collective_.size(); }
-    void append(Node *n) { collective_.append(n); }
+    void append(Node *n) { collective_.append(n);  n->setSharedCommentNode(this); setGenus(n->genus()); }
     const QVector<Node *> &collective() const { return collective_; }
     void setOverloadFlags();
     void setRelatedNonmember(bool b) override;
diff --git a/src/qdoc/sections.cpp b/src/qdoc/sections.cpp
index 4b4712180..8ba71f17c 100644
--- a/src/qdoc/sections.cpp
+++ b/src/qdoc/sections.cpp
@@ -904,9 +904,14 @@ void Sections::distributeQmlNodeInDetailsVector(SectionVector &dv, Node *n)
     }
 }
 
-void Sections::distributeQmlNodeInSummaryVector(SectionVector &sv, Node *n)
+/*!
+  Distributes a node \a n into the correct place in the summary section vector \a sv.
+  Nodes that are sharing a comment are handled recursively - for recursion, the \a
+  sharing parameter is set to \c true.
+ */
+void Sections::distributeQmlNodeInSummaryVector(SectionVector &sv, Node *n, bool sharing)
 {
-    if (n->isSharedCommentNode())
+    if (n->isSharingComment() && !sharing)
         return;
     if (n->isQmlProperty() || n->isJsProperty()) {
         QmlPropertyNode *pn = static_cast<QmlPropertyNode *>(n);
@@ -929,6 +934,14 @@ void Sections::distributeQmlNodeInSummaryVector(SectionVector &sv, Node *n)
             else
                 sv[QmlMethods].insert(fn);
         }
+    } else if (n->isSharedCommentNode()) {
+        SharedCommentNode *scn = static_cast<SharedCommentNode*>(n);
+        if (scn->isPropertyGroup()) {
+            sv[QmlProperties].insert(scn);
+        } else {
+            for (auto child : scn->collective())
+                distributeQmlNodeInSummaryVector(sv, child, true);
+        }
     }
 }
 
@@ -1023,7 +1036,9 @@ void Sections::buildStdQmlTypeRefPageSections()
                 ++c;
                 continue;
             }
-            allMembers.add(classMap, n);
+            if (!n->isSharedCommentNode() || n->isPropertyGroup())
+                allMembers.add(classMap, n);
+
             distributeQmlNodeInSummaryVector(sv, n);
             distributeQmlNodeInDetailsVector(dv, n);
             ++c;
diff --git a/src/qdoc/sections.h b/src/qdoc/sections.h
index c2f07cf65..92c2cdd2b 100644
--- a/src/qdoc/sections.h
+++ b/src/qdoc/sections.h
@@ -233,7 +233,7 @@ class Sections
     void distributeNodeInSummaryVector(SectionVector &sv, Node *n);
     void distributeNodeInDetailsVector(SectionVector &dv, Node *n);
     void distributeQmlNodeInDetailsVector(SectionVector &dv, Node *n);
-    void distributeQmlNodeInSummaryVector(SectionVector &sv, Node *n);
+    void distributeQmlNodeInSummaryVector(SectionVector &sv, Node *n, bool sharing = false);
     void initAggregate(SectionVector &v, Aggregate *aggregate);
 
  private: