Introduce loplugin:mustoverride

...to find derived classes that do not directly override SfxPoolItem::ItemType.
Most of the cases that the plugin flagged were classes that are apparently not
meant to be instantiated themselves (so I added pure virtual overrides there),
but SdrMeasureFormatStringItem appears to genuinely have missed it
DECLARE_ITEM_TYPE_FUNCTION stanza.

Change-Id: Iec40080377f26cc4fa2c8cff735d5f0d3bd60509
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/181211
Reviewed-by: Armin Le Grand <Armin.Le.Grand@me.com>
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <stephan.bergmann@allotropia.de>

9 files changed, 146 insertions, 1 deletions

diff --git a/compilerplugins/clang/mustoverride.cxx b/compilerplugins/clang/mustoverride.cxx
new file mode 100644
index 000000000000..bd1fee629870
--- /dev/null
+++ b/compilerplugins/clang/mustoverride.cxx
@@ -0,0 +1,104 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4; fill-column: 100 -*- */
+/*
+ * 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/.
+ */
+
+#ifndef LO_CLANG_SHARED_PLUGINS
+
+// Find classes deriving from other classes with virtual member functions annotated with
+// SAL_LOPLUGIN_ANNOTATE("mustoverride"), but where the derived class fails to directly override
+// them.
+
+#include <set>
+
+#include "plugin.hxx"
+
+namespace
+{
+void findMustOverride(CXXRecordDecl const* decl, std::set<CXXMethodDecl const*>* mustOverride)
+{
+    for (auto const& base : decl->bases())
+    {
+        auto const d = base.getType()->getAsCXXRecordDecl();
+        if (d == nullptr)
+        {
+            continue;
+        }
+        findMustOverride(d, mustOverride);
+        for (auto const meth : d->methods())
+        {
+            if (!meth->isVirtual())
+            {
+                continue;
+            }
+            for (auto const attr : meth->specific_attrs<AnnotateAttr>())
+            {
+                if (attr->getAnnotation() == "loplugin:mustoverride")
+                {
+                    mustOverride->insert(meth);
+                    break;
+                }
+            }
+        }
+    }
+}
+
+class MustOverride : public loplugin::FilteringPlugin<MustOverride>
+{
+public:
+    explicit MustOverride(loplugin::InstantiationData const& data)
+        : FilteringPlugin(data)
+    {
+    }
+
+    bool VisitCXXRecordDecl(CXXRecordDecl const* decl)
+    {
+        if (ignoreLocation(decl))
+        {
+            return true;
+        }
+        if (!decl->isThisDeclarationADefinition())
+        {
+            return true;
+        }
+        if (!decl->isPolymorphic())
+        {
+            return true;
+        }
+        std::set<CXXMethodDecl const*> mustOverride;
+        findMustOverride(decl, &mustOverride);
+        for (auto const meth : mustOverride)
+        {
+            if (meth->getCorrespondingMethodDeclaredInClass(decl) != nullptr)
+            {
+                continue;
+            }
+            report(DiagnosticsEngine::Warning, "%0 does not override %1 from %2",
+                   decl->getLocation())
+                << decl << meth << meth->getParent() << decl->getSourceRange();
+            report(DiagnosticsEngine::Note, "declared here", meth->getLocation())
+                << meth->getSourceRange();
+        }
+        return true;
+    }
+
+private:
+    void run() override
+    {
+        if (compiler.getLangOpts().CPlusPlus)
+        {
+            TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
+        }
+    }
+};
+
+loplugin::Plugin::Registration<MustOverride> mustoverride("mustoverride");
+}
+
+#endif
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
diff --git a/compilerplugins/clang/test/mustoverride.cxx b/compilerplugins/clang/test/mustoverride.cxx
new file mode 100644
index 000000000000..1d9b550271e3
--- /dev/null
+++ b/compilerplugins/clang/test/mustoverride.cxx
@@ -0,0 +1,28 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4; fill-column: 100 -*- */
+/*
+ * 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 <sal/config.h>
+
+#include <sal/types.h>
+
+struct Base
+{
+    // expected-note@+1 {{declared here}}
+    SAL_LOPLUGIN_ANNOTATE("mustoverride") virtual void mustOverride();
+
+    SAL_LOPLUGIN_ANNOTATE("mustoverride") virtual void mustOverride2();
+};
+
+// expected-error@+1 {{'Derived' does not override 'mustOverride' from 'Base'}}
+struct Derived : Base
+{
+    void mustOverride2() override;
+};
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
diff --git a/include/svl/eitem.hxx b/include/svl/eitem.hxx
index 518ebe51db62..229979724059 100644
--- a/include/svl/eitem.hxx
+++ b/include/svl/eitem.hxx
@@ -37,6 +37,8 @@ protected:
 
     SfxEnumItem(const SfxEnumItem &) = default;
 
+    virtual SfxItemType ItemType() const override = 0;
+
 public:
 
     EnumT GetValue() const { return m_nValue; }
diff --git a/include/svl/metitem.hxx b/include/svl/metitem.hxx
index 30cf75b7f864..7b408fa60553 100644
--- a/include/svl/metitem.hxx
+++ b/include/svl/metitem.hxx
@@ -27,6 +27,8 @@ class SVL_DLLPUBLIC SfxMetricItem: public SfxInt32Item
 public:
     explicit                 SfxMetricItem( sal_uInt16 nWhich, sal_Int32 nValue );
 
+    virtual SfxItemType ItemType() const override = 0;
+
     virtual void             ScaleMetrics( tools::Long lMult, tools::Long lDiv ) override;
     virtual bool             HasMetrics() const override;
 
diff --git a/include/svl/poolitem.hxx b/include/svl/poolitem.hxx
index 440d152de9a0..f6f76dc5479d 100644
--- a/include/svl/poolitem.hxx
+++ b/include/svl/poolitem.hxx
@@ -208,6 +208,7 @@ enum class SfxItemType : sal_uInt16
     SdrMeasureBelowRefEdgeItemType,
     SdrMeasureDecimalPlacesItemType,
     SdrMeasureKindItemType,
+    SdrMeasureFormatStringItemType,
     SdrMeasureOverhangItemType,
     SdrMeasureScaleItemType,
     SdrMeasureTextAutoAngleItemType,
@@ -685,7 +686,7 @@ public:
                                  m_nWhich = nId;
                              }
     sal_uInt16 Which() const { return m_nWhich; }
-    virtual SfxItemType ItemType() const = 0;
+    SAL_LOPLUGIN_ANNOTATE("mustoverride") virtual SfxItemType ItemType() const = 0;
 
     // StaticWhichCast asserts if the TypedWhichId is not matching its type, otherwise it returns a reference.
     // You can use StaticWhichCast when you are sure about the type at compile time -- like a static_cast.
diff --git a/solenv/CompilerTest_compilerplugins_clang.mk b/solenv/CompilerTest_compilerplugins_clang.mk
index 5ae22fc2d634..eb74020d5ca6 100644
--- a/solenv/CompilerTest_compilerplugins_clang.mk
+++ b/solenv/CompilerTest_compilerplugins_clang.mk
@@ -50,6 +50,7 @@ $(eval $(call gb_CompilerTest_add_exception_objects,compilerplugins_clang, \
     compilerplugins/clang/test/makeshared \
     compilerplugins/clang/test/moveit \
     compilerplugins/clang/test/moveparam \
+    compilerplugins/clang/test/mustoverride \
     compilerplugins/clang/test/noexcept \
     compilerplugins/clang/test/noexceptmove \
     compilerplugins/clang/test/nullptr \
diff --git a/svx/inc/sxmfsitm.hxx b/svx/inc/sxmfsitm.hxx
index 56422bcc4018..3f670822c1e1 100644
--- a/svx/inc/sxmfsitm.hxx
+++ b/svx/inc/sxmfsitm.hxx
@@ -27,6 +27,7 @@ class SdrMeasureFormatStringItem final : public SfxStringItem {
 public:
     SdrMeasureFormatStringItem()
         : SfxStringItem(SDRATTR_MEASUREFORMATSTRING) {}
+    DECLARE_ITEM_TYPE_FUNCTION(SdrMeasureFormatStringItem)
     virtual SdrMeasureFormatStringItem* Clone(SfxItemPool*) const override
     {
         return new SdrMeasureFormatStringItem(*this);
diff --git a/sw/inc/fmtftntx.hxx b/sw/inc/fmtftntx.hxx
index 26106ef6aae9..601879239bb1 100644
--- a/sw/inc/fmtftntx.hxx
+++ b/sw/inc/fmtftntx.hxx
@@ -51,6 +51,8 @@ protected:
         : SfxEnumItem( nWhichL, ePos ), m_nOffset( 0 )
     {}
 
+    virtual SfxItemType ItemType() const override = 0;
+
 public:
     virtual sal_uInt16       GetValueCount() const override;
 
diff --git a/sw/inc/grfatr.hxx b/sw/inc/grfatr.hxx
index edc2b3a10d84..62f7585fec56 100644
--- a/sw/inc/grfatr.hxx
+++ b/sw/inc/grfatr.hxx
@@ -154,6 +154,8 @@ protected:
         : SfxInt16Item( nWhichL, nVal )
     {}
 
+    virtual SfxItemType ItemType() const override = 0;
+
 public:
     // pure virtual methods from SfxInt16Item
     virtual bool GetPresentation( SfxItemPresentation ePres,
@@ -265,6 +267,8 @@ class SwDrawModeGrf_Base: public SfxEnumItem<GraphicDrawMode> {
 protected:
     SwDrawModeGrf_Base(GraphicDrawMode nMode):
         SfxEnumItem(RES_GRFATR_DRAWMODE, nMode) {}
+
+    virtual SfxItemType ItemType() const override = 0;
 };
 
 class SW_DLLPUBLIC SwDrawModeGrf final : public SwDrawModeGrf_Base