QDoc: refactor and document CppCodeParser::hasTooManyTopics

* Inverse a conditional check and make the condition more expressive.
  This allows an early return, as well as dedenting the remaining code
  in the method, and the removal of a later Q_ASSERT that could be
  surprising for readers that don't keep in mind that
  QString::lastIndexOf may return -1.
* Use std::all_of instead of a for loop on a negative condition,
  to make the code more expressive about the intention. Remove an
  internal bool that was used to indicate whether the error condition
  was met or not as it is superfluous after this change.
* Rename a variable internal to the method for accuracy.
* Add internal documentation for the method.
* Remove adding of punctuation in the warning message in favor of
  reusing convenience methods provided by Utilies. Build the warning
  message using std::accumulate.
* Be const correct, because it's nice.

Change-Id: I418362a9464575e1aeae2c93bd20e9a114ace3bc
Reviewed-by: Luca Di Sera <luca.disera@qt.io>

1 files changed, 30 insertions, 21 deletions

diff --git a/src/qdoc/cppcodeparser.cpp b/src/qdoc/cppcodeparser.cpp
index 82753cb61..01f044b8e 100644
--- a/src/qdoc/cppcodeparser.cpp
+++ b/src/qdoc/cppcodeparser.cpp
@@ -966,30 +966,39 @@ void CppCodeParser::processMetaCommands(NodeList &nodes, DocList &docs)
     }
 }
 
+/*!
+  * \internal
+  * \brief Checks if there are too many topic commands in \a doc.
+  *
+  * This method compares the commands used in \a doc with the set of topic
+  * commands. If zero or one topic command is found, or if all found topic
+  * commands are {\\qml*}-commands, the method returns \c false.
+  *
+  * If more than one topic command is found, QDoc issues a warning and the list
+  * of topic commands used in \a doc, and the method returns \c true.
+  */
 bool CppCodeParser::hasTooManyTopics(const Doc &doc) const
 {
     const QSet<QString> topicCommandsUsed = topicCommands() & doc.metaCommandsUsed();
-    if (topicCommandsUsed.size() > 1) {
-        bool ok = true;
-        for (const auto &t : topicCommandsUsed) {
-            if (!t.startsWith(QLatin1String("qml")))
-                ok = false;
-        }
-        if (ok)
-            return false;
-        QString topicList;
-        for (const auto &t : topicCommandsUsed)
-            topicList += QLatin1String(" \\") + t + QLatin1Char(',');
-        topicList[topicList.lastIndexOf(',')] = '.';
-        qsizetype i = topicList.lastIndexOf(',');
-        Q_ASSERT(i >= 0); // we had at least two commas
-        topicList[i] = ' ';
-        topicList.insert(i + 1, "and");
-        doc.location().warning(
-                QStringLiteral("Multiple topic commands found in comment:%1").arg(topicList));
-        return true;
-    }
-    return false;
+
+    if (topicCommandsUsed.empty() || topicCommandsUsed.size() == 1)
+        return false;
+    if (std::all_of(topicCommandsUsed.cbegin(), topicCommandsUsed.cend(),
+                    [](const auto &cmd) { return cmd.startsWith(QLatin1String("qml")); }))
+        return false;
+
+    const QStringList commands = topicCommandsUsed.values();
+    const QString topicCommands{ std::accumulate(
+            commands.cbegin(), commands.cend(), QString{},
+            [index = qsizetype{ 0 }, numberOfCommands = commands.size()](
+                    const QString &accumulator, const QString &topic) mutable -> QString {
+                return accumulator + QLatin1String("\\") + topic
+                        + Utilities::separator(index++, numberOfCommands);
+            }) };
+
+    doc.location().warning(
+            QStringLiteral("Multiple topic commands found in comment: %1").arg(topicCommands));
+    return true;
 }
 
 QT_END_NAMESPACE