[Libreoffice-commits] core.git: compilerplugins/clang editeng/source include/editeng

[Noel Grandin]

 compilerplugins/clang/sfxpoolitem.cxx |  147 ++++++++++++++++++++++++++++++++++
 editeng/source/items/paraitem.cxx     |    8 +
 include/editeng/pmdlitem.hxx          |    1 
 3 files changed, 156 insertions(+)

New commits:
commit 370294f6d1471b89a30c1f8b917cde3830d66b2f
Author: Noel Grandin <noel at peralex.com>
Date:   Tue Dec 8 14:17:07 2015 +0200

    new loplugin sfxpoolitem
    
    generates a warning about subclasses that add members but do not
    override operator==()
    
    Change-Id: If6df1a2cbd115f17bcca22f9b7995181dcf55c03
    Reviewed-on: https://gerrit.libreoffice.org/20468
    Tested-by: Jenkins <ci at libreoffice.org>
    Reviewed-by: Michael Stahl <mstahl at redhat.com>

diff --git a/compilerplugins/clang/sfxpoolitem.cxx b/compilerplugins/clang/sfxpoolitem.cxx
new file mode 100644
index 0000000..3248135
--- /dev/null
+++ b/compilerplugins/clang/sfxpoolitem.cxx
@@ -0,0 +1,147 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/*
+ * This file is part of the LibreOffice project.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+#include <string>
+#include <iostream>
+
+#include "plugin.hxx"
+#include "compat.hxx"
+#include "clang/AST/CXXInheritance.h"
+
+/*
+what might be more interesting is a warning about subclasses that add
+members but do not override virtual operator==() - that is easily
+forgotten and hard to notice.
+*/
+namespace {
+
+class SfxPoolItem:
+    public RecursiveASTVisitor<SfxPoolItem>, public loplugin::Plugin
+{
+public:
+    explicit SfxPoolItem(InstantiationData const & data): Plugin(data) {}
+
+    virtual void run() override { TraverseDecl(compiler.getASTContext().getTranslationUnitDecl()); }
+
+    bool VisitCXXRecordDecl( const CXXRecordDecl* );
+};
+
+bool BaseCheckNotSfxPoolItemSubclass(
+    const CXXRecordDecl *BaseDefinition
+#if __clang_major__ == 3 && __clang_minor__ <= 7
+    , void *
+#endif
+    )
+{
+    if (BaseDefinition && BaseDefinition->getQualifiedNameAsString() == "SfxPoolItem") {
+        return false;
+    }
+    return true;
+}
+
+bool isDerivedFromSfxPoolItem(const CXXRecordDecl *decl) {
+    if (!decl)
+        return false;
+    if (decl->getQualifiedNameAsString() == "SfxPoolItem")
+        return true;
+    if (!decl->hasDefinition()) {
+        return false;
+    }
+    if (// not sure what hasAnyDependentBases() does,
+        // but it avoids classes we don't want, e.g. WeakAggComponentImplHelper1
+        !decl->hasAnyDependentBases() &&
+        !compat::forallBases(*decl, BaseCheckNotSfxPoolItemSubclass, nullptr, true)) {
+        return true;
+    }
+    return false;
+}
+
+
+bool BaseCheckNotSwMsgPoolItemSubclass(
+    const CXXRecordDecl *BaseDefinition
+#if __clang_major__ == 3 && __clang_minor__ <= 7
+    , void *
+#endif
+    )
+{
+    if (BaseDefinition && BaseDefinition->getQualifiedNameAsString() == "SwMsgPoolItem") {
+        return false;
+    }
+    return true;
+}
+
+bool isDerivedFromSwMsgPoolItem(const CXXRecordDecl *decl) {
+    if (!decl)
+        return false;
+    if (decl->getQualifiedNameAsString() == "SwMsgPoolItem")
+        return true;
+    if (!decl->hasDefinition()) {
+        return false;
+    }
+    if (// not sure what hasAnyDependentBases() does,
+        // but it avoids classes we don't want, e.g. WeakAggComponentImplHelper1
+        !decl->hasAnyDependentBases() &&
+        !compat::forallBases(*decl, BaseCheckNotSwMsgPoolItemSubclass, nullptr, true)) {
+        return true;
+    }
+    return false;
+}
+
+bool endsWith(const string& a, const string& b) {
+    if (b.size() > a.size()) return false;
+    return std::equal(a.begin() + a.size() - b.size(), a.end(), b.begin());
+}
+
+bool SfxPoolItem::VisitCXXRecordDecl(const CXXRecordDecl* decl)
+{
+    if (ignoreLocation(decl)) {
+       return true;
+    }
+    if (!decl->hasDefinition()) {
+       return true;
+    }
+    // check if this class is derived from Window
+    if (!isDerivedFromSfxPoolItem(decl)) {
+        return true;
+    }
+    // the SwMsgPoolItem are some sort of hack to transport down-castable objects to SwClient::Modify(), they're not "real" items
+    if (isDerivedFromSwMsgPoolItem(decl)) {
+        return true;
+    }
+    if (decl->field_begin() == decl->field_end()) {
+        return true;
+    }
+    // the enum types do some weird stuff involving SfxEnumItemInterface
+    std::string sRecordName = decl->getQualifiedNameAsString();
+    if (sRecordName == "SfxEnumItem" || sRecordName == "SfxAllEnumItem")
+        return true;
+
+    // the new field is only used for reading and writing to storage
+    if (sRecordName == "SvxCharSetColorItem")
+        return true;
+
+    for (auto it = decl->method_begin(); it != decl->method_end(); ++it) {
+        if ( endsWith((*it)->getQualifiedNameAsString(), "::operator==") )
+            return true;
+    }
+    report(
+            DiagnosticsEngine::Warning,
+            "SfxPoolItem subclass %0 declares new fields, but does not overide operator==",
+            decl->getLocStart())
+        << decl->getQualifiedNameAsString() << decl->getSourceRange();
+    return true;
+}
+
+
+
+loplugin::Plugin::Registration< SfxPoolItem > X("sfxpoolitem");
+
+}
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/editeng/source/items/paraitem.cxx b/editeng/source/items/paraitem.cxx
index 061bf02..0b43f54 100644
--- a/editeng/source/items/paraitem.cxx
+++ b/editeng/source/items/paraitem.cxx
@@ -1273,6 +1273,14 @@ bool SvxPageModelItem::PutValue( const css::uno::Any& rVal, sal_uInt8 nMemberId
     return bRet;
 }
 
+bool SvxPageModelItem::operator==( const SfxPoolItem& rAttr ) const
+{
+    DBG_ASSERT( SfxPoolItem::operator==(rAttr), "unequal types" );
+
+    return SfxStringItem::operator==(rAttr) &&
+           bAuto == static_cast<const SvxPageModelItem&>( rAttr ).bAuto;
+}
+
 bool SvxPageModelItem::GetPresentation
 (
     SfxItemPresentation ePres,
diff --git a/include/editeng/pmdlitem.hxx b/include/editeng/pmdlitem.hxx
index 4a0e4ee..423a06b 100644
--- a/include/editeng/pmdlitem.hxx
+++ b/include/editeng/pmdlitem.hxx
@@ -51,6 +51,7 @@ public:
 
     virtual bool            QueryValue( css::uno::Any& rVal, sal_uInt8 nMemberId = 0 ) const override;
     virtual bool            PutValue( const css::uno::Any& rVal, sal_uInt8 nMemberId ) override;
+    virtual bool            operator==( const SfxPoolItem& ) const override;
 };
 
 inline SvxPageModelItem::SvxPageModelItem( sal_uInt16 nWh )