loplugin:xmlimport check for bad conversions to fastparser

add a check for classes which have been partly converted to fastparser,
but not completedly.
This is to help me when I convert stuff.

and it uncovers a bug introduced with
commit 998308c363dfad03143591aa18256d2669b4da11
use more FastParser in SvXMLStylesContext

Change-Id: Ib50e7136da10a1a7a346102aa47efef2f543e2ac
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/102669
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>

2 files changed, 141 insertions, 47 deletions

diff --git a/compilerplugins/clang/test/xmlimport.cxx b/compilerplugins/clang/test/xmlimport.cxx
index 965d4936e41c..36230ffdb2d8 100644
--- a/compilerplugins/clang/test/xmlimport.cxx
+++ b/compilerplugins/clang/test/xmlimport.cxx
@@ -1,4 +1,4 @@
-/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4; fill-column: 100 -*- */
+/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 4; fill-column: 100 -*- */
 /*
  * This file is part of the LibreOffice project.
  *
@@ -12,6 +12,13 @@
 // Cannot include this, makes clang crash
 //#include "xmloff/xmlimp.hxx"
 
+#include <com/sun/star/uno/Reference.hxx>
+
+namespace com::sun::star::xml::sax
+{
+class XAttributeList;
+}
+
 class SvXMLImportContext
 {
 public:
@@ -20,6 +27,10 @@ public:
     virtual void createFastChildContext() {}
     virtual void startFastElement() {}
     virtual void endFastElement() {}
+
+    virtual void StartElement(const css::uno::Reference<css::xml::sax::XAttributeList>&) {}
+    virtual void EndElement() {}
+    virtual void Characters(const OUString&) {}
 };
 
 class Test1 : public SvXMLImportContext
@@ -51,4 +62,27 @@ public:
     virtual void endFastElement() override;
 };
 
+class Test5 : public SvXMLImportContext
+{
+public:
+    // expected-error@+1 {{overrides startElement, but looks like a fastparser context class, no constructor that takes slowparser args [loplugin:xmlimport]}}
+    virtual void
+    StartElement(const css::uno::Reference<css::xml::sax::XAttributeList>& xAttrList) override;
+    // expected-error@+1 {{overrides startElement, but looks like a fastparser context class, no constructor that takes slowparser args [loplugin:xmlimport]}}
+    virtual void EndElement() override;
+    // expected-error@+1 {{overrides startElement, but looks like a fastparser context class, no constructor that takes slowparser args [loplugin:xmlimport]}}
+    virtual void Characters(const OUString&) override;
+};
+
+// no warning expected
+class Test6 : public SvXMLImportContext
+{
+public:
+    Test6(sal_uInt16, const OUString&);
+    virtual void
+    StartElement(const css::uno::Reference<css::xml::sax::XAttributeList>& xAttrList) override;
+    virtual void EndElement() override;
+    virtual void Characters(const OUString&) override;
+};
+
 /* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
diff --git a/compilerplugins/clang/xmlimport.cxx b/compilerplugins/clang/xmlimport.cxx
index bdb41f616859..d6b1aa78e325 100644
--- a/compilerplugins/clang/xmlimport.cxx
+++ b/compilerplugins/clang/xmlimport.cxx
@@ -52,81 +52,141 @@ public:
     bool VisitCXXMethodDecl(const CXXMethodDecl*);
 };
 
-static bool containsStartFastElementMethod(const CXXRecordDecl* cxxRecordDecl,
-                                           bool& rbFoundImportContext)
+static bool containsStartFastElementMethod(const CXXRecordDecl* cxxRecordDecl)
 {
     auto dc = loplugin::DeclCheck(cxxRecordDecl);
-    if (dc.Class("SvXMLImportContext"))
-    {
-        rbFoundImportContext = true;
-        return false;
-    }
     if (dc.Class("XFastContextHandler"))
         return false;
     for (auto it = cxxRecordDecl->method_begin(); it != cxxRecordDecl->method_end(); ++it)
     {
         auto i = *it;
         if (i->getIdentifier() && i->getName() == "startFastElement")
-        {
-            //            i->dump();
             return true;
-        }
     }
     return false;
 }
+
 bool XmlImport::VisitCXXMethodDecl(const CXXMethodDecl* methodDecl)
 {
     auto beginLoc = compat::getBeginLoc(methodDecl);
     if (!beginLoc.isValid() || ignoreLocation(beginLoc))
         return true;
 
-    auto cxxRecordDecl = methodDecl->getParent();
-    if (!cxxRecordDecl || !cxxRecordDecl->getIdentifier())
-        return true;
-    auto className = cxxRecordDecl->getName();
-    if (className == "OOXMLFactory") // writerfilter
-        return true;
-    if (className == "SvXMLLegacyToFastDocHandler" || className == "ImportDocumentHandler"
-        || className == "ExportDocumentHandler") // reportdesign
-        return true;
-    if (className == "XMLEmbeddedObjectExportFilter" || className == "XMLBasicExportFilter"
-        || className == "XMLTransformerBase" || className == "SvXMLMetaExport") // xmloff
-        return true;
     if (!methodDecl->getIdentifier())
         return true;
-    if (!(methodDecl->getName() == "createFastChildContext" || methodDecl->getName() == "characters"
-          || methodDecl->getName() == "endFastElement"))
+
+    auto cxxRecordDecl = methodDecl->getParent();
+    if (!cxxRecordDecl || !cxxRecordDecl->getIdentifier())
         return true;
+
     if (loplugin::DeclCheck(cxxRecordDecl).Class("SvXMLImportContext"))
         return true;
 
-    bool foundImportContext = false;
-    if (containsStartFastElementMethod(cxxRecordDecl, foundImportContext))
-        return true;
+    if (methodDecl->getName() == "createFastChildContext" || methodDecl->getName() == "characters"
+        || methodDecl->getName() == "endFastElement")
+    {
+        auto className = cxxRecordDecl->getName();
+        if (className == "OOXMLFactory") // writerfilter
+            return true;
+        if (className == "SvXMLLegacyToFastDocHandler" || className == "ImportDocumentHandler"
+            || className == "ExportDocumentHandler") // reportdesign
+            return true;
+        if (className == "XMLEmbeddedObjectExportFilter" || className == "XMLBasicExportFilter"
+            || className == "XMLTransformerBase" || className == "SvXMLMetaExport") // xmloff
+            return true;
 
-    bool foundStartFastElement = false;
-    CXXBasePaths aPaths;
-    cxxRecordDecl->lookupInBases(
-        [&](const CXXBaseSpecifier* Specifier, CXXBasePath & /*Path*/) -> bool {
-            if (!Specifier->getType().getTypePtr())
-                return false;
-            const CXXRecordDecl* baseCXXRecordDecl = Specifier->getType()->getAsCXXRecordDecl();
-            if (!baseCXXRecordDecl)
+        if (containsStartFastElementMethod(cxxRecordDecl))
+            return true;
+
+        bool foundStartFastElement = false;
+        bool foundImportContext = false;
+
+        CXXBasePaths aPaths;
+        cxxRecordDecl->lookupInBases(
+            [&](const CXXBaseSpecifier* Specifier, CXXBasePath & /*Path*/) -> bool {
+                if (!Specifier->getType().getTypePtr())
+                    return false;
+                const CXXRecordDecl* baseCXXRecordDecl = Specifier->getType()->getAsCXXRecordDecl();
+                if (!baseCXXRecordDecl)
+                    return false;
+                if (baseCXXRecordDecl->isInvalidDecl())
+                    return false;
+                if (loplugin::DeclCheck(baseCXXRecordDecl).Class("SvXMLImportContext"))
+                    foundImportContext |= true;
+                else
+                    foundStartFastElement |= containsStartFastElementMethod(baseCXXRecordDecl);
                 return false;
-            if (baseCXXRecordDecl->isInvalidDecl())
+            },
+            aPaths);
+
+        if (foundImportContext && !foundStartFastElement)
+            report(DiagnosticsEngine::Warning, "must override startFastElement too",
+                   compat::getBeginLoc(methodDecl))
+                << methodDecl->getSourceRange();
+    }
+    else if (methodDecl->getName() == "StartElement" || methodDecl->getName() == "EndElement"
+             || methodDecl->getName() == "Characters")
+    {
+        if (loplugin::DeclCheck(cxxRecordDecl).Class("SchXMLAxisContext"))
+            return true;
+        if (loplugin::DeclCheck(cxxRecordDecl).Class("SchXMLChartContext"))
+            return true;
+        if (loplugin::DeclCheck(cxxRecordDecl).Class("SchXMLParagraphContext"))
+            return true;
+        if (loplugin::DeclCheck(cxxRecordDecl).Class("SchXMLLegendContext"))
+            return true;
+        if (loplugin::DeclCheck(cxxRecordDecl).Class("SchXMLPropertyMappingContext"))
+            return true;
+
+        bool foundImportContext = false;
+        CXXBasePaths aPaths;
+        cxxRecordDecl->lookupInBases(
+            [&](const CXXBaseSpecifier* Specifier, CXXBasePath & /*Path*/) -> bool {
+                if (!Specifier->getType().getTypePtr())
+                    return false;
+                const CXXRecordDecl* baseCXXRecordDecl = Specifier->getType()->getAsCXXRecordDecl();
+                if (!baseCXXRecordDecl)
+                    return false;
+                if (baseCXXRecordDecl->isInvalidDecl())
+                    return false;
+                if (loplugin::DeclCheck(baseCXXRecordDecl).Class("SvXMLImportContext"))
+                    foundImportContext |= true;
                 return false;
-            foundStartFastElement
-                |= containsStartFastElementMethod(baseCXXRecordDecl, foundImportContext);
-            return false;
-        },
-        aPaths);
+            },
+            aPaths);
 
-    if (foundStartFastElement || !foundImportContext)
-        return true;
+        if (!foundImportContext)
+            return true;
+
+        bool foundConstructor = false;
+        for (auto it = cxxRecordDecl->ctor_begin(); it != cxxRecordDecl->ctor_end(); ++it)
+        {
+            const CXXConstructorDecl* ctor = *it;
+            bool foundInt16 = false;
+            for (auto paramIt = ctor->param_begin(); paramIt != ctor->param_end(); ++paramIt)
+            {
+                const ParmVarDecl* pvd = *paramIt;
+                auto tc = loplugin::TypeCheck(pvd->getType());
+                if (tc.Typedef("sal_uInt16"))
+                    foundInt16 = true;
+                else if (tc.LvalueReference().Const().Class("OUString") && foundInt16)
+                    foundConstructor = true;
+                else
+                    foundInt16 = false;
+                if (tc.LvalueReference().Const().Class("OUString")
+                    && pvd->getName() == "rLocalName")
+                    foundConstructor = true;
+            }
+        }
+
+        if (!foundConstructor)
+            report(DiagnosticsEngine::Warning,
+                   "overrides startElement, but looks like a fastparser context class, no "
+                   "constructor that takes slowparser args",
+                   compat::getBeginLoc(methodDecl))
+                << methodDecl->getSourceRange();
+    }
 
-    report(DiagnosticsEngine::Warning, "must override startFastElement too",
-           compat::getBeginLoc(methodDecl))
-        << methodDecl->getSourceRange();
     return true;
 }