[Libreoffice-commits] core.git: compilerplugins/clang reportdesign/source sw/source
Noel Grandin (via logerrit)
logerrit at kemper.freedesktop.org
Sun Sep 20 15:00:42 UTC 2020
compilerplugins/clang/test/xmlimport.cxx | 36 ++++++
compilerplugins/clang/xmlimport.cxx | 152 ++++++++++++++++++---------
reportdesign/source/filter/xml/xmlfilter.cxx | 4
sw/source/filter/xml/xmlfmt.cxx | 5
4 files changed, 145 insertions(+), 52 deletions(-)
New commits:
commit 4487a14e7cce62e4fb6c1ebddac5f3e713c061de
Author: Noel Grandin <noelgrandin at gmail.com>
AuthorDate: Sun Sep 20 14:53:51 2020 +0200
Commit: Noel Grandin <noel.grandin at collabora.co.uk>
CommitDate: Sun Sep 20 17:00:04 2020 +0200
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 at collabora.co.uk>
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 at +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 at +1 {{overrides startElement, but looks like a fastparser context class, no constructor that takes slowparser args [loplugin:xmlimport]}}
+ virtual void EndElement() override;
+ // expected-error at +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;
}
diff --git a/reportdesign/source/filter/xml/xmlfilter.cxx b/reportdesign/source/filter/xml/xmlfilter.cxx
index e5d84148f61f..5c65664625f3 100644
--- a/reportdesign/source/filter/xml/xmlfilter.cxx
+++ b/reportdesign/source/filter/xml/xmlfilter.cxx
@@ -93,7 +93,7 @@ public:
RptMLMasterStylesContext_Impl(const RptMLMasterStylesContext_Impl&) = delete;
RptMLMasterStylesContext_Impl& operator=(const RptMLMasterStylesContext_Impl&) = delete;
- virtual void EndElement() override;
+ virtual void SAL_CALL endFastElement(sal_Int32 nElement) override;
};
}
@@ -103,7 +103,7 @@ RptMLMasterStylesContext_Impl::RptMLMasterStylesContext_Impl( ORptFilter& rImpor
{
}
-void RptMLMasterStylesContext_Impl::EndElement()
+void RptMLMasterStylesContext_Impl::endFastElement(sal_Int32 )
{
FinishStyles( true );
GetImport().FinishStyles();
diff --git a/sw/source/filter/xml/xmlfmt.cxx b/sw/source/filter/xml/xmlfmt.cxx
index e1aebad2ed0b..446b5fd13aa3 100644
--- a/sw/source/filter/xml/xmlfmt.cxx
+++ b/sw/source/filter/xml/xmlfmt.cxx
@@ -738,14 +738,13 @@ protected:
public:
-
SwXMLStylesContext_Impl(
SwXMLImport& rImport,
bool bAuto );
virtual bool InsertStyleFamily( XmlStyleFamily nFamily ) const override;
- virtual void EndElement() override;
+ virtual void SAL_CALL endFastElement(sal_Int32 nElement) override;
};
}
@@ -929,7 +928,7 @@ OUString SwXMLStylesContext_Impl::GetServiceName( XmlStyleFamily nFamily ) const
return SvXMLStylesContext::GetServiceName( nFamily );
}
-void SwXMLStylesContext_Impl::EndElement()
+void SwXMLStylesContext_Impl::endFastElement(sal_Int32 )
{
GetSwImport().InsertStyles( IsAutomaticStyle() );
}
More information about the Libreoffice-commits
mailing list