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>
diff --git a/compilerplugins/clang/test/xmlimport.cxx b/compilerplugins/clang/test/xmlimport.cxx
index 965d493..36230ff 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 bdb41f6..d6b1aa7 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;
if (!methodDecl->getIdentifier())
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"))
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)
return false;
if (baseCXXRecordDecl->isInvalidDecl())
return false;
foundStartFastElement
|= containsStartFastElementMethod(baseCXXRecordDecl, foundImportContext);
return false;
},
aPaths);
if (containsStartFastElementMethod(cxxRecordDecl))
return true;
if (foundStartFastElement || !foundImportContext)
return true;
bool foundStartFastElement = false;
bool foundImportContext = false;
report(DiagnosticsEngine::Warning, "must override startFastElement too",
compat::getBeginLoc(methodDecl))
<< methodDecl->getSourceRange();
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;
},
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;
},
aPaths);
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();
}
return true;
}
diff --git a/reportdesign/source/filter/xml/xmlfilter.cxx b/reportdesign/source/filter/xml/xmlfilter.cxx
index e5d84148..5c65664 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 e1aebad..446b5fd 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() );
}
