QtDocGenerator/QtXmlToSphinx: Fix see-also links

"See also" links may appear in the qdoc WebXML output as nested links:
   <see-also>QAbstractXmlReceiver<link raw="isValid()" href="qxmlquery.html#isValid" type="function">isValid()</link>
which was handled in handleLinkTag(), or as direct text:
  <see-also>rootIsDecorated()</see-also>
which was not handled, causing numerous warnings:
.../QAbstractXmlNodeModel.rst:448: WARNING: Content block expected for the "seealso" directive; none found.

Refactor and split QtXmlToSphinx::handleLinkTag() into several functions
to operate on a struct LinkContext and keep 2 instances for
nested link tags and directly embedded links.

Task-number: PYSIDE-363
Change-Id: I734884267209f3621bfc5db4bf4347b838eb0de6
Reviewed-by: Alexandru Croitor <alexandru.croitor@qt.io>

2 files changed, 166 insertions, 93 deletions

diff --git a/sources/shiboken2/generator/qtdoc/qtdocgenerator.cpp b/sources/shiboken2/generator/qtdoc/qtdocgenerator.cpp
index 4a60579a9..fb4de46d2 100644
--- a/sources/shiboken2/generator/qtdoc/qtdocgenerator.cpp
+++ b/sources/shiboken2/generator/qtdoc/qtdocgenerator.cpp
@@ -271,7 +271,7 @@ QString QtXmlToSphinx::popOutputBuffer()
     return strcpy;
 }
 
-QString QtXmlToSphinx::expandFunction(const QString& function)
+QString QtXmlToSphinx::expandFunction(const QString& function) const
 {
     const int firstDot = function.indexOf(QLatin1Char('.'));
     const AbstractMetaClass *metaClass = nullptr;
@@ -292,7 +292,7 @@ QString QtXmlToSphinx::expandFunction(const QString& function)
         : function;
 }
 
-QString QtXmlToSphinx::resolveContextForMethod(const QString& methodName)
+QString QtXmlToSphinx::resolveContextForMethod(const QString& methodName) const
 {
     const QStringRef currentClass = m_context.splitRef(QLatin1Char('.')).constLast();
 
@@ -523,13 +523,65 @@ void QtXmlToSphinx::handleArgumentTag(QXmlStreamReader& reader)
         m_output << reader.text().trimmed();
 }
 
+static inline QString functionLinkType() { return QStringLiteral("function"); }
+static inline QString classLinkType() { return QStringLiteral("class"); }
+
+static inline QString fixLinkType(const QStringRef &type)
+{
+    // TODO: create a flag PROPERTY-AS-FUNCTION to ask if the properties
+    // are recognized as such or not in the binding
+    if (type == QLatin1String("property"))
+        return functionLinkType();
+    if (type == QLatin1String("typedef"))
+        return classLinkType();
+    return type.toString();
+}
+
+static inline QString linkSourceAttribute(const QString &type)
+{
+    if (type == functionLinkType() || type == classLinkType())
+        return QLatin1String("raw");
+    return type == QLatin1String("enum") || type == QLatin1String("page")
+        ? type : QLatin1String("href");
+}
+
+// "See also" links may appear as nested links:
+//     <see-also>QAbstractXmlReceiver<link raw="isValid()" href="qxmlquery.html#isValid" type="function">isValid()</link>
+//     which is handled in handleLinkTag
+// or direct text:
+//     <see-also>rootIsDecorated()</see-also>
+//     which is handled here.
+
 void QtXmlToSphinx::handleSeeAlsoTag(QXmlStreamReader& reader)
 {
-    QXmlStreamReader::TokenType token = reader.tokenType();
-    if (token == QXmlStreamReader::StartElement)
+    switch (reader.tokenType()) {
+    case QXmlStreamReader::StartElement:
         m_output << INDENT << ".. seealso:: ";
-    else if (token == QXmlStreamReader::EndElement)
+        break;
+    case QXmlStreamReader::Characters: {
+        // Direct embedded link: <see-also>rootIsDecorated()</see-also>
+        const QStringRef textR = reader.text().trimmed();
+        if (!textR.isEmpty()) {
+            const QString text = textR.toString();
+            if (m_seeAlsoContext.isNull()) {
+                const QString type = text.endsWith(QLatin1String("()"))
+                    ? functionLinkType() : classLinkType();
+                m_seeAlsoContext.reset(handleLinkStart(type, text));
+            }
+            handleLinkText(m_seeAlsoContext.data(), text);
+        }
+    }
+        break;
+    case QXmlStreamReader::EndElement:
+        if (!m_seeAlsoContext.isNull()) { // direct, no nested </link> seen
+            handleLinkEnd(m_seeAlsoContext.data());
+            m_seeAlsoContext.reset();
+        }
         m_output << endl;
+        break;
+    default:
+        break;
+    }
 }
 
 static inline QString fallbackPathAttribute() { return QStringLiteral("path"); }
@@ -733,100 +785,103 @@ void QtXmlToSphinx::handleListTag(QXmlStreamReader& reader)
 
 void QtXmlToSphinx::handleLinkTag(QXmlStreamReader& reader)
 {
-    static QString l_linktag;
-    static QString l_linkref;
-    static QString l_linktext;
-    static QString l_linktagending;
-    static QString l_type;
-    QXmlStreamReader::TokenType token = reader.tokenType();
-    if (token == QXmlStreamReader::StartElement) {
-        l_linktagending = QLatin1String("` ");
-        if (m_insideBold) {
-            l_linktag.prepend(QLatin1String("**"));
-            l_linktagending.append(QLatin1String("**"));
-        } else if (m_insideItalic) {
-            l_linktag.prepend(QLatin1Char('*'));
-            l_linktagending.append(QLatin1Char('*'));
-        }
-        l_type = reader.attributes().value(QLatin1String("type")).toString();
-
-        // TODO: create a flag PROPERTY-AS-FUNCTION to ask if the properties
-        // are recognized as such or not in the binding
-        if (l_type == QLatin1String("property"))
-            l_type = QLatin1String("function");
-
-        if (l_type == QLatin1String("typedef"))
-            l_type = QLatin1String("class");
-
-        QString linkSource;
-        if (l_type == QLatin1String("function") || l_type == QLatin1String("class")) {
-            linkSource  = QLatin1String("raw");
-        } else if (l_type == QLatin1String("enum")) {
-            linkSource  = QLatin1String("enum");
-        } else if (l_type == QLatin1String("page")) {
-            linkSource  = QLatin1String("page");
+    switch (reader.tokenType()) {
+    case QXmlStreamReader::StartElement: {
+        // <link> embedded in <see-also> means the characters of <see-also> are no link.
+        m_seeAlsoContext.reset();
+        const QString type = fixLinkType(reader.attributes().value(QLatin1String("type")));
+        const QString ref = reader.attributes().value(linkSourceAttribute(type)).toString();
+        m_linkContext.reset(handleLinkStart(type, ref));
+    }
+        break;
+    case QXmlStreamReader::Characters:
+        Q_ASSERT(!m_linkContext.isNull());
+        handleLinkText(m_linkContext.data(), reader.text().toString());
+        break;
+    case QXmlStreamReader::EndElement:
+        Q_ASSERT(!m_linkContext.isNull());
+        handleLinkEnd(m_linkContext.data());
+        m_linkContext.reset();
+        break;
+    default:
+        break;
+    }
+}
+
+QtXmlToSphinx::LinkContext *QtXmlToSphinx::handleLinkStart(const QString &type, const QString &ref) const
+{
+    LinkContext *result = new LinkContext(ref, type);
+
+    result->linkTagEnding = QLatin1String("` ");
+    if (m_insideBold) {
+        result->linkTag.prepend(QLatin1String("**"));
+        result->linkTagEnding.append(QLatin1String("**"));
+    } else if (m_insideItalic) {
+        result->linkTag.prepend(QLatin1Char('*'));
+        result->linkTagEnding.append(QLatin1Char('*'));
+    }
+
+    result->linkRef.replace(QLatin1String("::"), QLatin1String("."));
+    result->linkRef.remove(QLatin1String("()"));
+
+    if (result->type == functionLinkType() && !m_context.isEmpty()) {
+        result->linkTag = QLatin1String(" :meth:`");
+        const QVector<QStringRef> rawlinklist = result->linkRef.splitRef(QLatin1Char('.'));
+        if (rawlinklist.size() == 1 || rawlinklist.constFirst() == m_context) {
+            QString context = resolveContextForMethod(rawlinklist.constLast().toString());
+            if (!result->linkRef.startsWith(context))
+                result->linkRef.prepend(context + QLatin1Char('.'));
         } else {
-            linkSource = QLatin1String("href");
+            result->linkRef = expandFunction(result->linkRef);
         }
-
-        l_linkref = reader.attributes().value(linkSource).toString();
-        l_linkref.replace(QLatin1String("::"), QLatin1String("."));
-        l_linkref.remove(QLatin1String("()"));
-
-        if (l_type == QLatin1String("function") && !m_context.isEmpty()) {
-            l_linktag = QLatin1String(" :meth:`");
-            const QVector<QStringRef> rawlinklist = l_linkref.splitRef(QLatin1Char('.'));
-            if (rawlinklist.size() == 1 || rawlinklist.constFirst() == m_context) {
-                QString context = resolveContextForMethod(rawlinklist.constLast().toString());
-                if (!l_linkref.startsWith(context))
-                    l_linkref.prepend(context + QLatin1Char('.'));
-            } else {
-                l_linkref = expandFunction(l_linkref);
+    } else if (result->type == functionLinkType() && m_context.isEmpty()) {
+        result->linkTag = QLatin1String(" :func:`");
+    } else if (result->type == classLinkType()) {
+        result->linkTag = QLatin1String(" :class:`");
+        if (const TypeEntry *type = TypeDatabase::instance()->findType(result->linkRef)) {
+            result->linkRef = type->qualifiedTargetLangName();
+        } else { // fall back to the old heuristic if the type wasn't found.
+            const QVector<QStringRef> rawlinklist = result->linkRef.splitRef(QLatin1Char('.'));
+            QStringList splittedContext = m_context.split(QLatin1Char('.'));
+            if (rawlinklist.size() == 1 || rawlinklist.constFirst() == splittedContext.constLast()) {
+                splittedContext.removeLast();
+                result->linkRef.prepend(QLatin1Char('~') + splittedContext.join(QLatin1Char('.'))
+                                                 + QLatin1Char('.'));
             }
-        } else if (l_type == QLatin1String("function") && m_context.isEmpty()) {
-            l_linktag = QLatin1String(" :func:`");
-        } else if (l_type == QLatin1String("class")) {
-            l_linktag = QLatin1String(" :class:`");
-            TypeEntry* type = TypeDatabase::instance()->findType(l_linkref);
-            if (type) {
-                l_linkref = type->qualifiedTargetLangName();
-            } else { // fall back to the old heuristic if the type wasn't found.
-                const QVector<QStringRef> rawlinklist = l_linkref.splitRef(QLatin1Char('.'));
-                QStringList splittedContext = m_context.split(QLatin1Char('.'));
-                if (rawlinklist.size() == 1 || rawlinklist.constFirst() == splittedContext.constLast()) {
-                    splittedContext.removeLast();
-                    l_linkref.prepend(QLatin1Char('~') + splittedContext.join(QLatin1Char('.'))
-                                      + QLatin1Char('.'));
-                }
-            }
-        } else if (l_type == QLatin1String("enum")) {
-            l_linktag = QLatin1String(" :attr:`");
-        } else if (l_type == QLatin1String("page") && l_linkref == m_generator->moduleName()) {
-            l_linktag = QLatin1String(" :mod:`");
-        } else {
-            l_linktag = QLatin1String(" :ref:`");
         }
+    } else if (result->type == QLatin1String("enum")) {
+        result->linkTag = QLatin1String(" :attr:`");
+    } else if (result->type == QLatin1String("page") && result->linkRef == m_generator->moduleName()) {
+        result->linkTag = QLatin1String(" :mod:`");
+    } else {
+        result->linkTag = QLatin1String(" :ref:`");
+    }
+    return result;
+}
 
-    } else if (token == QXmlStreamReader::Characters) {
-        QString linktext = reader.text().toString();
-        linktext.replace(QLatin1String("::"), QLatin1String("."));
-        const QStringRef item = l_linkref.splitRef(QLatin1Char('.')).constLast();
-        if (l_linkref == linktext
-            || (l_linkref + QLatin1String("()")) == linktext
-            || item == linktext
-            || (item + QLatin1String("()")) == linktext)
-            l_linktext.clear();
-        else
-            l_linktext = linktext + QLatin1Char('<');
-    } else if (token == QXmlStreamReader::EndElement) {
-        if (!l_linktext.isEmpty())
-            l_linktagending.prepend(QLatin1Char('>'));
-        m_output << l_linktag << l_linktext;
-        writeEscapedRstText(m_output, l_linkref);
-        m_output << l_linktagending;
+void QtXmlToSphinx::handleLinkText(LinkContext *linkContext, QString linktext) const
+{
+    linktext.replace(QLatin1String("::"), QLatin1String("."));
+    const QStringRef item = linkContext->linkRef.splitRef(QLatin1Char('.')).constLast();
+    if (linkContext->linkRef == linktext
+        || (linkContext->linkRef + QLatin1String("()")) == linktext
+        || item == linktext
+        || (item + QLatin1String("()")) == linktext) {
+        linkContext->linkText.clear();
+    } else {
+        linkContext->linkText = linktext + QLatin1Char('<');
     }
 }
 
+void QtXmlToSphinx::handleLinkEnd(LinkContext *linkContext)
+{
+    if (!linkContext->linkText.isEmpty())
+        linkContext->linkTagEnding.prepend(QLatin1Char('>'));
+    m_output << linkContext->linkTag << linkContext->linkText;
+    writeEscapedRstText(m_output, linkContext->linkRef);
+    m_output << linkContext->linkTagEnding;
+}
+
 // Copy images that are placed in a subdirectory "images" under the webxml files
 // by qdoc to a matching subdirectory under the "rst/PySide2/<module>" directory
 static bool copyImage(const QString &href, const QString &docDataDir,
diff --git a/sources/shiboken2/generator/qtdoc/qtdocgenerator.h b/sources/shiboken2/generator/qtdoc/qtdocgenerator.h
index 0a83fc8b3..1977f3019 100644
--- a/sources/shiboken2/generator/qtdoc/qtdocgenerator.h
+++ b/sources/shiboken2/generator/qtdoc/qtdocgenerator.h
@@ -30,6 +30,7 @@
 
 #include <QtCore/QStack>
 #include <QtCore/QHash>
+#include <QtCore/QScopedPointer>
 #include <QtCore/QTextStream>
 #include <QXmlStreamReader>
 #include "generator.h"
@@ -101,8 +102,19 @@ public:
     }
 
 private:
-    QString resolveContextForMethod(const QString& methodName);
-    QString expandFunction(const QString& function);
+    struct LinkContext
+    {
+        LinkContext(const QString &ref, const QString &lType) : linkRef(ref), type(lType) {}
+
+        QString linkTag;
+        QString linkRef;
+        QString linkText;
+        QString linkTagEnding;
+        QString type;
+    };
+
+    QString resolveContextForMethod(const QString& methodName) const;
+    QString expandFunction(const QString& function) const;
     QString transform(const QString& doc);
 
     void handleHeadingTag(QXmlStreamReader& reader);
@@ -133,6 +145,10 @@ private:
     void handleUselessTag(QXmlStreamReader& reader);
     void handleAnchorTag(QXmlStreamReader& reader);
 
+    LinkContext *handleLinkStart(const QString &type, const QString &ref) const;
+    void handleLinkText(LinkContext *linkContext, QString linktext) const;
+    void handleLinkEnd(LinkContext *linkContext);
+
     typedef void (QtXmlToSphinx::*TagHandler)(QXmlStreamReader&);
     QHash<QString, TagHandler> m_handlerMap;
     QStack<TagHandler> m_handlers;
@@ -143,6 +159,8 @@ private:
 
 
     Table m_currentTable;
+    QScopedPointer<LinkContext> m_linkContext; // for <link>
+    QScopedPointer<LinkContext> m_seeAlsoContext; // for <see-also>foo()</see-also>
     bool m_tableHasHeader;
     QString m_context;
     QtDocGenerator* m_generator;