qdoc: Make QDoc warn about undocumented public classes

QDoc was marking all undocumented public API elements as
"internal" and "private" because most of these undocumented
elements should not be documented. The standard way to tell
QDoc not to warn about an undocumented elemewnt in the public
API is to give it a QDoc comment with the command "\internl"
in it. But it was decided this would require too much work
to eliminate all the warnings, because there are so many
undocumented elements in the Qt public API that we really
don't want to be documented. So we decided to just mark them
all as both internal and private in QDoc itself, and that
eliminated a great many useless QDoc warnings.

But it also meant that QDoc would no longer warn when a
public element was left undocumented by mistake. This is
most often seen in C++ classes that are in the public API
but are not documented. QFutureInterface is an example of
a class that is not documented but should be documented
because it is useful.

This change lets QDoc warn that a class in the public API
was not documented with a \class comment.

Special cases:
1. If the undocumented class has no members, don't warn that it was
not documented with a \class comment.
2. If the undocumented class's name contains the word "Private" it
is probably not meant to be in the public API, so don't warn that
it has no \class comment.
3. If the undocumented class has no function members, then don't
warn that it has no \class comment.
4. If the undocumented class is marked DontDocument, then don't
warn that it has no \class comment.

The other part of this change relates to item 4 above. To mark a
class or struct as DontDocument required adding a new topic
command to QDoc. The new topic command is \dontdocument. The
argument to this command is a list of class and struct names. The
list is enclosed in parentheses. For example:

\dontdocument (QMacAutoReleasePool QIncompatibleFlag
                ...
               QTextCodec::ConverterState QThreadStorageData)
QDoc looks up each name in the list and marks it DontDocument.
The documentation generator then sees the node is marked
DontDocument and ignores the node. This makes it a lot easier to
tell QDoc which public classes and structs should not generate
the warning about no documentation.

Task-number: QTBUG-57183
Change-Id: I7eee48de03ca7aeb72c63ae90ba373503d41612d
Reviewed-by: Topi Reiniö <topi.reinio@qt.io>

11 files changed, 115 insertions, 40 deletions

diff --git a/src/qdoc/codeparser.h b/src/qdoc/codeparser.h
index bde7bda9e..56183a2f8 100644
--- a/src/qdoc/codeparser.h
+++ b/src/qdoc/codeparser.h
@@ -99,6 +99,7 @@ private:
 #define COMMAND_COPYRHOLDER             Doc::alias(QLatin1String("copyrholder"))
 #define COMMAND_COPYRYEAR               Doc::alias(QLatin1String("copyryear"))
 #define COMMAND_DEPRECATED              Doc::alias(QLatin1String("deprecated")) // ### don't document
+#define COMMAND_DONTDOCUMENT            Doc::alias(QLatin1String("dontdocument"))
 #define COMMAND_DITAMAP                 Doc::alias(QLatin1String("ditamap"))
 #define COMMAND_ENUM                    Doc::alias(QLatin1String("enum"))
 #define COMMAND_EXAMPLE                 Doc::alias(QLatin1String("example"))
diff --git a/src/qdoc/cppcodemarker.cpp b/src/qdoc/cppcodemarker.cpp
index aad3c6dd7..2387fb22d 100644
--- a/src/qdoc/cppcodemarker.cpp
+++ b/src/qdoc/cppcodemarker.cpp
@@ -293,15 +293,12 @@ QString CppCodeMarker::markedUpSynopsis(const Node *node,
     }
 
     if (style == Section::Summary) {
-        if (node->status() == Node::Preliminary) {
+        if (node->isPreliminary())
             extra += "(preliminary) ";
-        }
-        else if (node->status() == Node::Deprecated) {
+        else if (node->isDeprecated())
             extra += "(deprecated) ";
-        }
-        else if (node->status() == Node::Obsolete) {
+        else if (node->isObsolete())
             extra += "(obsolete) ";
-        }
     }
 
     if (!extra.isEmpty()) {
@@ -358,15 +355,12 @@ QString CppCodeMarker::markedUpQmlItem(const Node* node, bool summary)
 
     QString extra;
     if (summary) {
-        if (node->status() == Node::Preliminary) {
+        if (node->isPreliminary())
             extra += " (preliminary)";
-        }
-        else if (node->status() == Node::Deprecated) {
+        else if (node->isDeprecated())
             extra += " (deprecated)";
-        }
-        else if (node->status() == Node::Obsolete) {
+        else if (node->isObsolete())
             extra += " (obsolete)";
-        }
     }
 
     if (!extra.isEmpty()) {
diff --git a/src/qdoc/cppcodeparser.cpp b/src/qdoc/cppcodeparser.cpp
index 05d177f73..f6f2eb5f2 100644
--- a/src/qdoc/cppcodeparser.cpp
+++ b/src/qdoc/cppcodeparser.cpp
@@ -60,6 +60,7 @@ CppCodeParser::CppCodeParser()
     if (topicCommands_.isEmpty()) {
         topicCommands_ << COMMAND_CLASS
                        << COMMAND_DITAMAP
+                       << COMMAND_DONTDOCUMENT
                        << COMMAND_ENUM
                        << COMMAND_EXAMPLE
                        << COMMAND_EXTERNALPAGE
@@ -970,6 +971,8 @@ void CppCodeParser::processTopicArgs(const Doc &doc, const QString &topic, NodeL
                 node = parseMacroArg(doc.location(), args[0].first);
             } else if (isQMLMethodTopic(topic) || isJSMethodTopic(topic)) {
                 node = parseOtherFuncArg(topic, doc.location(), args[0].first);
+            } else if (topic == COMMAND_DONTDOCUMENT) {
+                qdb_->primaryTree()->addToDontDocumentMap(args[0].first);
             } else {
                 node = processTopicCommand(doc, topic, args[0]);
             }
diff --git a/src/qdoc/generator.cpp b/src/qdoc/generator.cpp
index ac5ee56ef..ac6577c81 100644
--- a/src/qdoc/generator.cpp
+++ b/src/qdoc/generator.cpp
@@ -217,8 +217,7 @@ int Generator::appendSortedNames(Text& text, const ClassNode* cn, const QList<Re
     while (r != rc.constEnd()) {
         ClassNode* rcn = (*r).node_;
         if (rcn && rcn->access() == Node::Public &&
-              rcn->status() != Node::Internal &&
-              !rcn->doc().isEmpty()) {
+            !rcn->isInternal() && !rcn->doc().isEmpty()) {
             Text className;
             appendFullName(className, rcn, cn);
             classMap[className.toString().toLower()] = className;
@@ -690,7 +689,7 @@ QString Generator::fullDocumentLocation(const Node *node, bool useSubdir)
     }
 
     if (!node->isClassNode() && !node->isNamespace()) {
-        if (node->status() == Node::Obsolete)
+        if (node->isObsolete())
             parentName.replace(QLatin1Char('.') + currentGenerator()->fileExtension(),
                                "-obsolete." + currentGenerator()->fileExtension());
     }
@@ -1179,8 +1178,8 @@ void Generator::generateDocumentation(Node* node)
             generatePageNode(static_cast<PageNode*>(node), marker);
             endSubPage();
         } else if (node->isAggregate()) {
-            if (node->isClassNode() || node->isHeader() ||
-                (node->isNamespace() && node->docMustBeGenerated())) {
+            if ((node->isClassNode() || node->isHeader() || node->isNamespace()) &&
+                node->docMustBeGenerated()) {
                 beginSubPage(node, fileName(node));
                 generateCppReferencePage(static_cast<Aggregate*>(node), marker);
                 endSubPage();
diff --git a/src/qdoc/htmlgenerator.cpp b/src/qdoc/htmlgenerator.cpp
index 4ce0b9b69..f51e358a2 100644
--- a/src/qdoc/htmlgenerator.cpp
+++ b/src/qdoc/htmlgenerator.cpp
@@ -535,7 +535,7 @@ int HtmlGenerator::generateAtom(const Atom *atom, const Node *relative, CodeMark
                 if (autolinkErrors())
                     relative->doc().location().warning(tr("Can't autolink to '%1'").arg(atom->string()));
             }
-            else if (node && node->status() == Node::Obsolete) {
+            else if (node && node->isObsolete()) {
                 if ((relative->parent() != node) && !relative->isObsolete())
                     link.clear();
             }
@@ -988,7 +988,7 @@ int HtmlGenerator::generateAtom(const Atom *atom, const Node *relative, CodeMark
               no longer warn about linking to obsolete things?
              */
             node = nullptr;
-            if (node && node->status() == Node::Obsolete) {
+            if (node && node->isObsolete()) {
                 if ((relative->parent() != node) && !relative->isObsolete()) {
                     inObsoleteLink = true;
                     if (obsoleteLinks) {
@@ -1507,7 +1507,12 @@ void HtmlGenerator::generateCppReferencePage(Aggregate *aggregate, CodeMarker *m
     QString detailsRef = registerRef("details");
     out() << "<a name=\"" << detailsRef << "\"></a>" << divNavTop << '\n';
 
-    if (!aggregate->doc().isEmpty()) {
+    if (aggregate->doc().isEmpty()) {
+        QString command = "documentation";
+        if (aggregate->isClassNode())
+            command = "\'\\class\' comment";
+        aggregate->location().warning(tr("No %1 for '%2'").arg(command).arg(aggregate->plainSignature()));
+    } else {
         generateExtractionMark(aggregate, DetailedDescriptionMark);
         out() << "<div class=\"descr\">\n" // QTBUG-9504
               << "<h2 id=\"" << detailsRef << "\">" << "Detailed Description" << "</h2>\n";
@@ -2335,7 +2340,7 @@ void HtmlGenerator::generateRequisites(Aggregate *aggregate, CodeMarker *marker)
 
     if (aggregate->isClassNode()) {
         ClassNode* classe = static_cast<ClassNode*>(aggregate);
-        if (classe->qmlElement() != nullptr && classe->status() != Node::Internal) {
+        if (classe->qmlElement() != nullptr && !classe->isInternal()) {
             text.clear();
             text << Atom(Atom::LinkNode, CodeMarker::stringForNode(classe->qmlElement()))
                  << Atom(Atom::FormattingLeft, ATOM_FORMATTING_LINK)
@@ -2460,7 +2465,7 @@ void HtmlGenerator::generateQmlRequisites(QmlTypeNode *qcn, CodeMarker *marker)
 
     //add the instantiates to the map
     ClassNode* cn = qcn->classNode();
-    if (cn && (cn->status() != Node::Internal)) {
+    if (cn && !cn->isInternal()) {
         text.clear();
         text << Atom(Atom::LinkNode,CodeMarker::stringForNode(qcn));
         text << Atom(Atom::FormattingLeft, ATOM_FORMATTING_LINK);
@@ -2745,7 +2750,7 @@ 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]->status() == Node::Internal) {
+                if (nodes[j]->access() == Node::Private || nodes[j]->isInternal()) {
                     continue;
                 }
                 out() << "<li class=\"fn\">";
@@ -4017,7 +4022,7 @@ void HtmlGenerator::generateFullName(const Node *apparentNode, const Node *relat
     if (actualNode == nullptr)
         actualNode = apparentNode;
     out() << "<a href=\"" << linkForNode(actualNode, relative);
-    if (actualNode->status() == Node::Obsolete)
+    if (actualNode->isObsolete())
         out() << "\" class=\"obsolete";
     out() << "\">";
     out() << protectEnc(apparentNode->fullName(relative));
@@ -4213,7 +4218,7 @@ void HtmlGenerator::beginLink(const QString &link, const Node *node, const Node
     }
     else if (node == nullptr || (relative != nullptr && node->status() == relative->status()))
         out() << "<a href=\"" << link_ << "\">";
-    else if (node->status() == Node::Obsolete)
+    else if (node->isObsolete())
         out() << "<a href=\"" << link_ << "\" class=\"obsolete\">";
     else
         out() << "<a href=\"" << link_ << "\">";
@@ -4417,7 +4422,7 @@ void HtmlGenerator::generateQmlInherits(QmlTypeNode* qcn, CodeMarker* marker)
 void HtmlGenerator::generateQmlInstantiates(QmlTypeNode* qcn, CodeMarker* marker)
 {
     ClassNode* cn = qcn->classNode();
-    if (cn && (cn->status() != Node::Internal)) {
+    if (cn && !cn->isInternal()) {
         Text text;
         text << Atom::ParaLeft;
         text << Atom(Atom::LinkNode,CodeMarker::stringForNode(qcn));
@@ -4450,7 +4455,7 @@ void HtmlGenerator::generateQmlInstantiates(QmlTypeNode* qcn, CodeMarker* marker
  */
 void HtmlGenerator::generateInstantiatedBy(ClassNode* cn, CodeMarker* marker)
 {
-    if (cn &&  cn->status() != Node::Internal && cn->qmlElement() != nullptr) {
+    if (cn &&  !cn->isInternal() && cn->qmlElement() != nullptr) {
         const QmlTypeNode* qcn = cn->qmlElement();
         Text text;
         text << Atom::ParaLeft;
diff --git a/src/qdoc/node.cpp b/src/qdoc/node.cpp
index 26e042e25..d418190a3 100644
--- a/src/qdoc/node.cpp
+++ b/src/qdoc/node.cpp
@@ -1113,16 +1113,18 @@ QStringList Aggregate::primaryKeys()
 void Aggregate::markUndocumentedChildrenInternal()
 {
     foreach (Node *child, children_) {
-        if (!child->isSharingComment() && !child->hasDoc() && !child->docMustBeGenerated()) {
-            if (child->isFunction()) {
-                if (static_cast<FunctionNode*>(child)->hasAssociatedProperties())
-                    continue;
-            } else if (child->isTypedef()) {
-                if (static_cast<TypedefNode*>(child)->hasAssociatedEnum())
-                    continue;
+        if (!child->isSharingComment() && !child->hasDoc() && !child->isDontDocument()) {
+            if (!child->docMustBeGenerated()) {
+                if (child->isFunction()) {
+                    if (static_cast<FunctionNode*>(child)->hasAssociatedProperties())
+                        continue;
+                } else if (child->isTypedef()) {
+                    if (static_cast<TypedefNode*>(child)->hasAssociatedEnum())
+                        continue;
+                }
+                child->setAccess(Node::Private);
+                child->setStatus(Node::Internal);
             }
-            child->setAccess(Node::Private);
-            child->setStatus(Node::Internal);
         }
         if (child->isAggregate()) {
             static_cast<Aggregate*>(child)->markUndocumentedChildrenInternal();
@@ -1866,6 +1868,26 @@ FunctionNode* ClassNode::findOverriddenFunction(const FunctionNode* fn)
 }
 
 /*!
+  Returns true if the class or struct represented by this class
+  node must be documented. If this function returns true, then
+  qdoc must find a qdoc comment for this class. If it returns
+  false, then the class need not be documented.
+ */
+bool ClassNode::docMustBeGenerated() const
+{
+    if (isInternal() || isPrivate() || isDontDocument() ||
+        declLocation().fileName().endsWith(QLatin1String("_p.h")))
+        return false;
+    if (count() == 0)
+        return false;
+    if (name().contains(QLatin1String("Private")))
+        return false;
+    if (functionCount_ == 0)
+        return false;
+    return true;
+}
+
+/*!
   \class Headerode
   \brief This class represents a C++ header file.
  */
diff --git a/src/qdoc/node.h b/src/qdoc/node.h
index a40f4d766..168ff79a4 100644
--- a/src/qdoc/node.h
+++ b/src/qdoc/node.h
@@ -112,7 +112,8 @@ public:
         Deprecated,
         Preliminary,
         Active,
-        Internal
+        Internal,
+        DontDocument
     }; // don't reorder this enum
 
     enum ThreadSafeness : unsigned char {
@@ -171,6 +172,8 @@ public:
     bool isAnyType() const { return true; }
     bool isClass() const { return nodeType_ == Class; }
     bool isCppNode() const { return genus() == CPP; }
+    bool isDeprecated() const { return (status_ == Deprecated); }
+    bool isDontDocument() const { return (status_ == DontDocument); }
     bool isEnumType() const { return nodeType_ == Enum; }
     bool isExample() const { return nodeType_ == Example; }
     bool isExternalPage() const { return nodeType_ == ExternalPage; }
@@ -512,6 +515,8 @@ private:
     NodeList enumChildren_;
     NodeMap nonfunctionMap_;
     NodeList nonfunctionList_;
+
+ protected:
     int functionCount_;
     FunctionMap functionMap_;
 };
@@ -628,6 +633,7 @@ public:
     PropertyNode* findPropertyNode(const QString& name);
     QmlTypeNode* findQmlBaseNode();
     FunctionNode* findOverriddenFunction(const FunctionNode* fn);
+    bool docMustBeGenerated() const override;
 
 private:
     QList<RelatedClass> bases_;
diff --git a/src/qdoc/qdocdatabase.cpp b/src/qdoc/qdocdatabase.cpp
index d7bf16ea1..aefed5615 100644
--- a/src/qdoc/qdocdatabase.cpp
+++ b/src/qdoc/qdocdatabase.cpp
@@ -1124,6 +1124,7 @@ void QDocDatabase::resolveStuff() {
         primaryTreeRoot()->normalizeOverloads();
         primaryTree()->fixInheritance(primaryTreeRoot());
         primaryTree()->resolveProperties();
+        primaryTree()->markDontDocumentNodes();
         primaryTreeRoot()->markUndocumentedChildrenInternal();
         primaryTreeRoot()->resolveQmlInheritance();
         primaryTree()->resolveTargets(primaryTreeRoot());
diff --git a/src/qdoc/qdocdatabase.h b/src/qdoc/qdocdatabase.h
index e8d5153fe..c04918c69 100644
--- a/src/qdoc/qdocdatabase.h
+++ b/src/qdoc/qdocdatabase.h
@@ -417,10 +417,10 @@ class QDocDatabase
     QDocDatabase();
     QDocDatabase(QDocDatabase const& ) : showInternal_(false), forest_(this) { }
     QDocDatabase& operator=(QDocDatabase const& );
-    Tree* primaryTree() { return forest_.primaryTree(); }
 
  public:
     static bool             debug;
+    Tree* primaryTree() { return forest_.primaryTree(); }
 
  private:
     static QDocDatabase*    qdocDB_;
diff --git a/src/qdoc/tree.cpp b/src/qdoc/tree.cpp
index dff42d095..52ae4ca49 100644
--- a/src/qdoc/tree.cpp
+++ b/src/qdoc/tree.cpp
@@ -144,8 +144,7 @@ Node* Tree::findNodeForInclude(const QStringList& path) const
  */
 Aggregate *Tree::findAggregate(const QString &name)
 {
-    QStringList path;
-    path << name;
+    QStringList path = name.split(QLatin1String("::"));
     return static_cast<Aggregate*>(findNodeRecursive(path, 0, const_cast<NamespaceNode*>(root()), &Node::isFirstClassAggregate));
 }
 
@@ -1423,4 +1422,46 @@ FunctionNode *Tree::findMacroNode(const QString &t, const Aggregate *parent)
     return nullptr;
 }
 
+/*!
+  Add the class and struct names in \a arg to the \e {don't document}
+  map.
+ */
+void Tree::addToDontDocumentMap(QString &arg)
+{
+    arg.remove(QChar('('));
+    arg.remove(QChar(')'));
+    QString t = arg.simplified();
+    QStringList sl = t.split(QChar(' '));
+    if (sl.isEmpty())
+        return;
+    for (const QString& s : sl) {
+        if (!dontDocumentMap_.contains(s))
+            dontDocumentMap_.insert(s, nullptr);
+    }
+}
+
+/*!
+  The \e {don't document} map has been loaded with the names
+  of classes and structs in the current module that are not
+  documented and should not be documented. Now traverse the
+  map, and for each class or struct name, find the class node
+  that represents that class or struct and mark it with the
+  \C DontDocument status.
+
+  This results in a map of the class and struct nodes in the
+  module that are in the public API but are not meant to be
+  used by anyone. They are only used internally, but for one
+  reason or another, they must have public visibility.
+ */
+void Tree::markDontDocumentNodes()
+{
+    NodeMap::iterator i = dontDocumentMap_.begin();
+    while (i != dontDocumentMap_.end()) {
+        Aggregate* node = findAggregate(i.key());
+        if (node != nullptr)
+            node->setStatus(Node::DontDocument);
+        ++i;
+    }
+}
+
 QT_END_NAMESPACE
diff --git a/src/qdoc/tree.h b/src/qdoc/tree.h
index e8a8f261e..2586f5c01 100644
--- a/src/qdoc/tree.h
+++ b/src/qdoc/tree.h
@@ -110,6 +110,8 @@ class Tree
     void setIndexTitle(const QString &t) { indexTitle_ = t; }
     NodeList &proxies() { return proxies_; }
     void appendProxy(ProxyNode *t) { proxies_.append(t); }
+    void addToDontDocumentMap(QString &arg);
+    void markDontDocumentNodes();
 
  private: // The rest of the class is private.
     Aggregate *findAggregate(const QString &name);
@@ -245,6 +247,7 @@ private:
     ExampleNodeMap          exampleNodeMap_;
     TargetListMap*          targetListMap_;
     NodeList proxies_;
+    NodeMap                 dontDocumentMap_;
 };
 
 QT_END_NAMESPACE