qdoc: Clean up \keyword and \target processing

Extract recurring code in HelpProjectWriter::generateSection()
into a reusable lambda.

Move the warning about a 'bad keyword' to happen already
during parsing, to docparser.cpp. The only bad keyword
is an empty one, so modify the warning message accordingly.
Extend the same warning to apply to both \keyword and
\target commands.

Change-Id: Iec033e98517a959f0ed96dbdf859799347334874
Reviewed-by: Paul Wicking <[email protected]>
(cherry picked from commit 8460b4e)
Reviewed-by: Qt Cherry-pick Bot <[email protected]>

Author: toreinio

src/qdoc/qdoc/src/qdoc/docparser.cpp

@@ -1319,15 +1319,21 @@ QString DocParser::detailsUnknownCommand(const QSet<QString> &metaCommandSet, co
 /*!
     \internal
 
-    Issues a warning about the duplicate definition of a target or keyword in
-    at \a location. \a duplicateDefinition is the target being processed; the
-    already registered definition is \a previousDefinition.
+    Issues a warning about an empty or duplicate definition of either a
+    \\target or \\keyword command (determined by \a cmdString) at
+    \a location. \a duplicateDefinition is the target being processed;
+    the already registered definition is \a previousDefinition.
  */
-static void warnAboutPreexistingTarget(const Location &location, const QString &duplicateDefinition, const QString &previousDefinition)
+static void warnAboutEmptyOrPreexistingTarget(const Location &location, const QString &duplicateDefinition,
+                                              const QString &cmdString, const QString &previousDefinition)
 {
-    location.warning(
-            QStringLiteral("Duplicate target name '%1'. The previous occurrence is here: %2")
-                    .arg(duplicateDefinition, previousDefinition));
+    if (duplicateDefinition.isEmpty()) {
+        location.warning("Expected an argument for \\%1"_L1.arg(cmdString));
+    } else {
+        location.warning(
+                "Duplicate %3 name '%1'. The previous occurrence is here: %2"_L1
+                        .arg(duplicateDefinition, previousDefinition, cmdString));
+    }
 }
 
 /*!
@@ -1346,8 +1352,9 @@ static void warnAboutPreexistingTarget(const Location &location, const QString &
  */
 void DocParser::insertTarget(const QString &target)
 {
-    if (m_targetMap.contains(target))
-        return warnAboutPreexistingTarget(location(), target, m_targetMap[target].toString());
+    if (target.isEmpty() || m_targetMap.contains(target))
+        return warnAboutEmptyOrPreexistingTarget(location(), target,
+                s_utilities.cmdHash.key(CMD_TARGET), m_targetMap[target].toString());
 
     m_targetMap.insert(target, location());
     m_private->constructExtra();
@@ -1372,8 +1379,9 @@ void DocParser::insertTarget(const QString &target)
  */
 void DocParser::insertKeyword(const QString &keyword)
 {
-    if (m_targetMap.contains(keyword))
-        return warnAboutPreexistingTarget(location(), keyword, m_targetMap[keyword].toString());
+    if (keyword.isEmpty() || m_targetMap.contains(keyword))
+        return warnAboutEmptyOrPreexistingTarget(location(), keyword,
+                s_utilities.cmdHash.key(CMD_KEYWORD), m_targetMap[keyword].toString());
 
     m_targetMap.insert(keyword, location());
     m_private->constructExtra();

src/qdoc/qdoc/src/qdoc/helpprojectwriter.cpp

@@ -250,6 +250,14 @@ bool HelpProjectWriter::generateSection(HelpProject &project, QXmlStreamWriter &
         }
     }
 
+    auto appendDocKeywords = [&](const Node *n) {
+        for (const auto *kw : n->doc().keywords()) {
+                if (!kw->string().isEmpty()) {
+                    project.m_keywords.append(Keyword(kw->string(), kw->string(),
+                            m_gen->fullDocumentLocation(n)));
+                }
+            }
+    };
     // Unseen group nodes require no further processing as they have no documentation
     if (unseenGroup)
         return false;
@@ -263,19 +271,7 @@ bool HelpProjectWriter::generateSection(HelpProject &project, QXmlStreamWriter &
         break;
     case Node::QmlType:
     case Node::QmlValueType:
-        if (node->doc().hasKeywords()) {
-            const auto keywords = node->doc().keywords();
-            for (const Atom *keyword : keywords) {
-                if (!keyword->string().isEmpty()) {
-                    project.m_keywords.append(Keyword(keyword->string(), keyword->string(),
-                                                      m_gen->fullDocumentLocation(node)));
-                }
-                else
-                    node->doc().location().warning(
-                            QStringLiteral("Bad keyword in %1")
-                                    .arg(m_gen->fullDocumentLocation(node)));
-            }
-        }
+        appendDocKeywords(node);
         project.m_keywords.append(keywordDetails(node));
         break;
 
@@ -308,21 +304,8 @@ bool HelpProjectWriter::generateSection(HelpProject &project, QXmlStreamWriter &
     case Node::Group:
     case Node::Module:
     case Node::QmlModule: {
-        const auto *cn = static_cast<const CollectionNode *>(node);
-        if (!cn->fullTitle().isEmpty()) {
-            if (cn->doc().hasKeywords()) {
-                const auto keywords = cn->doc().keywords();
-                for (const Atom *keyword : keywords) {
-                    if (!keyword->string().isEmpty()) {
-                        project.m_keywords.append(
-                                Keyword(keyword->string(), keyword->string(),
-                                        m_gen->fullDocumentLocation(node)));
-                    } else
-                        cn->doc().location().warning(
-                                QStringLiteral("Bad keyword in %1")
-                                        .arg(m_gen->fullDocumentLocation(node)));
-                }
-            }
+        if (!node->fullTitle().isEmpty()) {
+            appendDocKeywords(node);
             project.m_keywords.append(keywordDetails(node));
         }
     } break;
@@ -379,21 +362,8 @@ bool HelpProjectWriter::generateSection(HelpProject &project, QXmlStreamWriter &
         // Page nodes (such as manual pages) contain subtypes, titles and other
         // attributes.
     case Node::Page: {
-        const auto *pn = static_cast<const PageNode *>(node);
-        if (!pn->fullTitle().isEmpty()) {
-            if (pn->doc().hasKeywords()) {
-                const auto keywords = pn->doc().keywords();
-                for (const Atom *keyword : keywords) {
-                    if (!keyword->string().isEmpty()) {
-                        project.m_keywords.append(
-                                Keyword(keyword->string(), keyword->string(),
-                                        m_gen->fullDocumentLocation(node)));
-                    } else {
-                        QString loc = m_gen->fullDocumentLocation(node);
-                        pn->doc().location().warning(QStringLiteral("Bad keyword in %1").arg(loc));
-                    }
-                }
-            }
+        if (!node->fullTitle().isEmpty()) {
+            appendDocKeywords(node);
             project.m_keywords.append(keywordDetails(node));
         }
         break;