new clang plugin: staticmethods

Genius suggestion from Tor Lillqvist, write a clang plugin that finds
methods that can be static.

Change-Id: Ie6684cc95d088e8750b300a028b49f763da00345

12 files changed, 182 insertions, 40 deletions

diff --git a/comphelper/source/misc/componentbase.cxx b/comphelper/source/misc/componentbase.cxx
index aa23f9d5fda2..7b0b8201e490 100644
--- a/comphelper/source/misc/componentbase.cxx
+++ b/comphelper/source/misc/componentbase.cxx
@@ -51,7 +51,7 @@ namespace comphelper
     }
 
 
-    Reference< XInterface > ComponentBase::getComponent() const
+    Reference< XInterface > ComponentBase::getComponent()
     {
         return NULL;
     }
diff --git a/comphelper/source/property/MasterPropertySet.cxx b/comphelper/source/property/MasterPropertySet.cxx
index 8c5a220a0bec..f5e9cba48387 100644
--- a/comphelper/source/property/MasterPropertySet.cxx
+++ b/comphelper/source/property/MasterPropertySet.cxx
@@ -370,9 +370,13 @@ PropertyState SAL_CALL MasterPropertySet::getPropertyState( const OUString& Prop
         if (pSlave->mpMutex)
             xMutexGuard.reset( new osl::Guard< comphelper::SolarMutex >(pSlave->mpMutex) );
 
-        pSlave->_preGetPropertyState();
-        pSlave->_getPropertyState( *((*aIter).second->mpInfo), aState );
-        pSlave->_postGetPropertyState();
+        // FIXME: Each of these three methods does OSL_FAIL( "you have
+        // to implement this yourself!") and nothing else, so this
+        // can't make much sense. Is this whole else branch in fact
+        // dead code?
+        ChainablePropertySet::_preGetPropertyState();
+        ChainablePropertySet::_getPropertyState( *((*aIter).second->mpInfo), aState );
+        ChainablePropertySet::_postGetPropertyState();
     }
 
     return aState;
@@ -404,10 +408,10 @@ Sequence< PropertyState > SAL_CALL MasterPropertySet::getPropertyStates( const S
                 SlaveData * pSlave = maSlaveMap [ (*aIter).second->mnMapId ];
                 if (!pSlave->IsInit())
                 {
-                    pSlave->mpSlave->_preGetPropertyState();
+                    comphelper::ChainablePropertySet::_preGetPropertyState();
                     pSlave->SetInit ( true );
                 }
-                pSlave->mpSlave->_getPropertyState( *((*aIter).second->mpInfo), *pState );
+                comphelper::ChainablePropertySet::_getPropertyState( *((*aIter).second->mpInfo), *pState );
             }
         }
         _postGetPropertyState();
@@ -416,7 +420,7 @@ Sequence< PropertyState > SAL_CALL MasterPropertySet::getPropertyStates( const S
         {
             if ( (*aSlaveIter).second->IsInit())
             {
-                (*aSlaveIter).second->mpSlave->_postGetPropertyState();
+                comphelper::ChainablePropertySet::_postGetPropertyState();
                 (*aSlaveIter).second->SetInit ( false );
             }
             ++aSlaveIter;
diff --git a/compilerplugins/clang/compat.hxx b/compilerplugins/clang/compat.hxx
index 78acb54418f7..64e0b04659b9 100644
--- a/compilerplugins/clang/compat.hxx
+++ b/compilerplugins/clang/compat.hxx
@@ -224,6 +224,16 @@ inline bool isMacroBodyExpansion(clang::CompilerInstance& compiler, clang::Sourc
 
 }
 
+inline bool isMacroBodyExpansion(clang::CompilerInstance& compiler, clang::SourceLocation location)
+{
+#if (__clang_major__ == 3 && __clang_minor__ >= 3) || __clang_major__ > 3
+    return compiler.getSourceManager().isMacroBodyExpansion(location);
+#else
+    return location.isMacroID()
+        && !compiler.getSourceManager().isMacroArgExpansion(location);
+#endif
+}
+
 #endif
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/compilerplugins/clang/literaltoboolconversion.cxx b/compilerplugins/clang/literaltoboolconversion.cxx
index 65fe53cdcfba..00ecdd8045c1 100644
--- a/compilerplugins/clang/literaltoboolconversion.cxx
+++ b/compilerplugins/clang/literaltoboolconversion.cxx
@@ -29,8 +29,6 @@ public:
 
 private:
     bool isFromCIncludeFile(SourceLocation spellingLocation) const;
-
-    bool isMacroBodyExpansion(SourceLocation location) const;
 };
 
 bool LiteralToBoolConversion::VisitImplicitCastExpr(
@@ -64,7 +62,7 @@ bool LiteralToBoolConversion::VisitImplicitCastExpr(
         while (compiler.getSourceManager().isMacroArgExpansion(loc)) {
             loc = compiler.getSourceManager().getImmediateMacroCallerLoc(loc);
         }
-        if (isMacroBodyExpansion(loc)) {
+        if (isMacroBodyExpansion(compiler, loc)) {
             StringRef name { Lexer::getImmediateMacroName(
                 loc, compiler.getSourceManager(), compiler.getLangOpts()) };
             if (name == "sal_False" || name == "sal_True") {
@@ -166,17 +164,6 @@ bool LiteralToBoolConversion::isFromCIncludeFile(
             .endswith(".h"));
 }
 
-bool LiteralToBoolConversion::isMacroBodyExpansion(SourceLocation location)
-    const
-{
-#if (__clang_major__ == 3 && __clang_minor__ >= 3) || __clang_major__ > 3
-    return compiler.getSourceManager().isMacroBodyExpansion(location);
-#else
-    return location.isMacroID()
-        && !compiler.getSourceManager().isMacroArgExpansion(location);
-#endif
-}
-
 loplugin::Plugin::Registration<LiteralToBoolConversion> X(
     "literaltoboolconversion", true);
 
diff --git a/compilerplugins/clang/staticmethods.cxx b/compilerplugins/clang/staticmethods.cxx
new file mode 100644
index 000000000000..b5b2a9d79d80
--- /dev/null
+++ b/compilerplugins/clang/staticmethods.cxx
@@ -0,0 +1,141 @@
+/* -*- 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 "plugin.hxx"
+#include "compat.hxx"
+
+/*
+  Look for member functions that can be static
+*/
+namespace {
+
+class StaticMethods:
+    public RecursiveASTVisitor<StaticMethods>, public loplugin::Plugin
+{
+private:
+    bool bVisitedThis;
+public:
+    explicit StaticMethods(InstantiationData const & data): Plugin(data) {}
+
+    void run() override
+    { TraverseDecl(compiler.getASTContext().getTranslationUnitDecl()); }
+
+    bool TraverseCXXMethodDecl(const CXXMethodDecl * decl);
+
+    bool VisitCXXThisExpr(const CXXThisExpr *) { bVisitedThis = true; return true; }
+    // these two indicate that we hit something that makes our analysis unreliable
+    bool VisitUnresolvedMemberExpr(const UnresolvedMemberExpr *) { bVisitedThis = true; return true; }
+    bool VisitCXXDependentScopeMemberExpr(const CXXDependentScopeMemberExpr *) { bVisitedThis = true; return true; }
+private:
+    std::string getFilename(SourceLocation loc);
+};
+
+bool BaseCheckNotTestFixtureSubclass(const CXXRecordDecl *BaseDefinition, void *) {
+    if (BaseDefinition->getQualifiedNameAsString().compare("CppUnit::TestFixture") == 0) {
+        return false;
+    }
+    return true;
+}
+
+bool isDerivedFromTestFixture(const CXXRecordDecl *decl) {
+    if (!decl->hasDefinition())
+        return false;
+    if (// not sure what hasAnyDependentBases() does,
+        // but it avoids classes we don't want, e.g. WeakAggComponentImplHelper1
+        !decl->hasAnyDependentBases() &&
+        !decl->forallBases(BaseCheckNotTestFixtureSubclass, nullptr, true)) {
+        return true;
+    }
+    return false;
+}
+
+std::string StaticMethods::getFilename(SourceLocation loc)
+{
+    SourceLocation spellingLocation = compiler.getSourceManager().getSpellingLoc(loc);
+    return compiler.getSourceManager().getFilename(spellingLocation);
+}
+
+
+bool StaticMethods::TraverseCXXMethodDecl(const CXXMethodDecl * pCXXMethodDecl) {
+    if (ignoreLocation(pCXXMethodDecl)) {
+        return true;
+    }
+    if (!pCXXMethodDecl->isInstance() || pCXXMethodDecl->isVirtual() || !pCXXMethodDecl->hasBody()) {
+        return true;
+    }
+    if (pCXXMethodDecl->getOverloadedOperator() != OverloadedOperatorKind::OO_None || pCXXMethodDecl->hasAttr<OverrideAttr>()) {
+        return true;
+    }
+    if (isa<CXXConstructorDecl>(pCXXMethodDecl) || isa<CXXDestructorDecl>(pCXXMethodDecl) || isa<CXXConversionDecl>(pCXXMethodDecl)) {
+        return true;
+    }
+    if (isInUnoIncludeFile(compiler.getSourceManager().getSpellingLoc(pCXXMethodDecl->getCanonicalDecl()->getLocStart()))) {
+        return true;
+    }
+    if ( pCXXMethodDecl != pCXXMethodDecl->getCanonicalDecl() ) {
+        return true;
+    }
+
+    // leave these alone for now, it is possible to fix them, but I don't understand how
+    SourceLocation canonicalLoc = pCXXMethodDecl->getCanonicalDecl()->getLocStart();
+    if (isMacroBodyExpansion(compiler, canonicalLoc) ) {
+        StringRef name { Lexer::getImmediateMacroName(
+                canonicalLoc, compiler.getSourceManager(), compiler.getLangOpts()) };
+        if (name == "DECL_LINK") {
+            return true;
+        }
+    }
+    // the CppUnit stuff uses macros and methods that can't be changed
+    if (isDerivedFromTestFixture(pCXXMethodDecl->getParent())) {
+        return true;
+    }
+    // don't mess with the backwards compatibility stuff
+    if (getFilename(pCXXMethodDecl->getLocStart()) == SRCDIR "/cppuhelper/source/compat.cxx") {
+        return true;
+    }
+    // the DDE has a dummy implementation on Linux and a real one on Windows
+    if (getFilename(pCXXMethodDecl->getCanonicalDecl()->getLocStart()) == SRCDIR "/include/svl/svdde.hxx") {
+        return true;
+    }
+    std::string aParentName = pCXXMethodDecl->getParent()->getQualifiedNameAsString();
+    // special case having something to do with static initialisation
+    // sal/osl/all/utility.cxx
+    if (aParentName == "osl::OGlobalTimer") {
+        return true;
+    }
+    // can't change it because in debug mode it can't be static
+    // sal/cpprt/operators_new_delete.cxx
+    if (aParentName == "(anonymous namespace)::AllocatorTraits") {
+        return true;
+    }
+    // in this case, the code is taking the address of the member function
+    // shell/source/unix/sysshell/recently_used_file_handler.cxx
+    if (aParentName == "(anonymous namespace)::recently_used_item") {
+        return true;
+    }
+
+    bVisitedThis = false;
+    TraverseStmt(pCXXMethodDecl->getBody());
+    if (bVisitedThis) {
+        return true;
+    }
+
+    report(
+        DiagnosticsEngine::Warning,
+        "this method can be declared static " + aParentName,
+        pCXXMethodDecl->getCanonicalDecl()->getLocStart())
+      << pCXXMethodDecl->getCanonicalDecl()->getSourceRange();
+    return true;
+}
+
+loplugin::Plugin::Registration<StaticMethods> X("staticmethods", false);
+
+}
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/include/comphelper/ChainablePropertySet.hxx b/include/comphelper/ChainablePropertySet.hxx
index 31e1de7a5bbd..07210beaa516 100644
--- a/include/comphelper/ChainablePropertySet.hxx
+++ b/include/comphelper/ChainablePropertySet.hxx
@@ -96,16 +96,16 @@ namespace comphelper
         virtual void _postGetValues ()
             throw(::com::sun::star::beans::UnknownPropertyException, ::com::sun::star::beans::PropertyVetoException, ::com::sun::star::lang::IllegalArgumentException, ::com::sun::star::lang::WrappedTargetException ) = 0;
 
-        void _preGetPropertyState ()
+        static void _preGetPropertyState ()
             throw(::com::sun::star::beans::UnknownPropertyException, ::com::sun::star::beans::PropertyVetoException, ::com::sun::star::lang::IllegalArgumentException, ::com::sun::star::lang::WrappedTargetException );
-        void _getPropertyState( const comphelper::PropertyInfo& rInfo, ::com::sun::star::beans::PropertyState& rState )
+        static void _getPropertyState( const comphelper::PropertyInfo& rInfo, ::com::sun::star::beans::PropertyState& rState )
             throw(::com::sun::star::beans::UnknownPropertyException );
-        void _postGetPropertyState ()
+        static void _postGetPropertyState ()
             throw(::com::sun::star::beans::UnknownPropertyException, ::com::sun::star::beans::PropertyVetoException, ::com::sun::star::lang::IllegalArgumentException, ::com::sun::star::lang::WrappedTargetException );
 
-        void _setPropertyToDefault( const comphelper::PropertyInfo& rEntry )
+        static void _setPropertyToDefault( const comphelper::PropertyInfo& rEntry )
             throw(::com::sun::star::beans::UnknownPropertyException );
-        ::com::sun::star::uno::Any _getPropertyDefault( const comphelper::PropertyInfo& rEntry )
+        static ::com::sun::star::uno::Any _getPropertyDefault( const comphelper::PropertyInfo& rEntry )
             throw(::com::sun::star::beans::UnknownPropertyException, ::com::sun::star::lang::WrappedTargetException );
 
     public:
diff --git a/include/comphelper/MasterPropertySet.hxx b/include/comphelper/MasterPropertySet.hxx
index b2cb4d9a1067..bff7835bbba3 100644
--- a/include/comphelper/MasterPropertySet.hxx
+++ b/include/comphelper/MasterPropertySet.hxx
@@ -84,16 +84,16 @@ namespace comphelper
         virtual void _postGetValues ()
             throw(::com::sun::star::beans::UnknownPropertyException, ::com::sun::star::beans::PropertyVetoException, ::com::sun::star::lang::IllegalArgumentException, ::com::sun::star::lang::WrappedTargetException ) = 0;
 
-        void _preGetPropertyState ()
+        static void _preGetPropertyState ()
             throw(::com::sun::star::beans::UnknownPropertyException, ::com::sun::star::beans::PropertyVetoException, ::com::sun::star::lang::IllegalArgumentException, ::com::sun::star::lang::WrappedTargetException );
-        void _getPropertyState( const comphelper::PropertyInfo& rInfo, ::com::sun::star::beans::PropertyState& rState )
+        static void _getPropertyState( const comphelper::PropertyInfo& rInfo, ::com::sun::star::beans::PropertyState& rState )
             throw(::com::sun::star::beans::UnknownPropertyException );
-        void _postGetPropertyState ()
+        static void _postGetPropertyState ()
             throw(::com::sun::star::beans::UnknownPropertyException, ::com::sun::star::beans::PropertyVetoException, ::com::sun::star::lang::IllegalArgumentException, ::com::sun::star::lang::WrappedTargetException );
 
-        void _setPropertyToDefault( const comphelper::PropertyInfo& rEntry )
+        static void _setPropertyToDefault( const comphelper::PropertyInfo& rEntry )
             throw(::com::sun::star::beans::UnknownPropertyException );
-        ::com::sun::star::uno::Any _getPropertyDefault( const comphelper::PropertyInfo& rEntry )
+        static ::com::sun::star::uno::Any _getPropertyDefault( const comphelper::PropertyInfo& rEntry )
             throw(::com::sun::star::beans::UnknownPropertyException, ::com::sun::star::lang::WrappedTargetException );
 
     public:
diff --git a/include/comphelper/accessibletexthelper.hxx b/include/comphelper/accessibletexthelper.hxx
index 6edef5b11c2a..af4d69055a5d 100644
--- a/include/comphelper/accessibletexthelper.hxx
+++ b/include/comphelper/accessibletexthelper.hxx
@@ -52,9 +52,9 @@ namespace comphelper
 
         ::com::sun::star::uno::Reference < ::com::sun::star::i18n::XBreakIterator >             implGetBreakIterator();
         ::com::sun::star::uno::Reference < ::com::sun::star::i18n::XCharacterClassification >   implGetCharacterClassification();
-        bool                             implIsValidBoundary( ::com::sun::star::i18n::Boundary& rBoundary, sal_Int32 nLength );
-        bool                             implIsValidIndex( sal_Int32 nIndex, sal_Int32 nLength );
-        bool                             implIsValidRange( sal_Int32 nStartIndex, sal_Int32 nEndIndex, sal_Int32 nLength );
+        static bool                      implIsValidBoundary( ::com::sun::star::i18n::Boundary& rBoundary, sal_Int32 nLength );
+        static bool                      implIsValidIndex( sal_Int32 nIndex, sal_Int32 nLength );
+        static bool                      implIsValidRange( sal_Int32 nStartIndex, sal_Int32 nEndIndex, sal_Int32 nLength );
         virtual OUString                 implGetText() = 0;
         virtual ::com::sun::star::lang::Locale  implGetLocale() = 0;
         virtual void                     implGetSelection( sal_Int32& nStartIndex, sal_Int32& nEndIndex ) = 0;
diff --git a/include/comphelper/componentbase.hxx b/include/comphelper/componentbase.hxx
index 7881ae4de5e0..349e6c648641 100644
--- a/include/comphelper/componentbase.hxx
+++ b/include/comphelper/componentbase.hxx
@@ -100,8 +100,8 @@ namespace comphelper
 
             The default implementation returns <NULL/>.
         */
-        ::com::sun::star::uno::Reference< ::com::sun::star::uno::XInterface >
-                getComponent() const;
+        static ::com::sun::star::uno::Reference< ::com::sun::star::uno::XInterface >
+                getComponent();
 
     private:
         ::cppu::OBroadcastHelper&   m_rBHelper;
diff --git a/include/comphelper/numberedcollection.hxx b/include/comphelper/numberedcollection.hxx
index f1d6ccc3d1df..c231863f6d9b 100644
--- a/include/comphelper/numberedcollection.hxx
+++ b/include/comphelper/numberedcollection.hxx
@@ -152,8 +152,8 @@ class COMPHELPER_DLLPUBLIC NumberedCollection : private ::cppu::BaseMutex
          */
         ::sal_Int32 impl_searchFreeNumber ();
 
-        void impl_cleanUpDeadItems (      TNumberedItemHash& lItems    ,
-                                    const TDeadItemList&     lDeadItems);
+        static void impl_cleanUpDeadItems (      TNumberedItemHash& lItems    ,
+                                           const TDeadItemList&     lDeadItems);
 
 
     // member
diff --git a/include/comphelper/propertycontainer.hxx b/include/comphelper/propertycontainer.hxx
index 70ec84e0350f..1561b61e8165 100644
--- a/include/comphelper/propertycontainer.hxx
+++ b/include/comphelper/propertycontainer.hxx
@@ -49,7 +49,7 @@ protected:
     OPropertyContainer(::cppu::OBroadcastHelper& _rBHelper);
 
     /// for scripting : the types of the interfaces supported by this class
-    ::com::sun::star::uno::Sequence< ::com::sun::star::uno::Type > getBaseTypes() throw (::com::sun::star::uno::RuntimeException, std::exception);
+    static ::com::sun::star::uno::Sequence< ::com::sun::star::uno::Type > getBaseTypes() throw (::com::sun::star::uno::RuntimeException, std::exception);
 
 // OPropertySetHelper overridables
     virtual sal_Bool SAL_CALL convertFastPropertyValue(
diff --git a/include/comphelper/propstate.hxx b/include/comphelper/propstate.hxx
index a8029a6b849d..1bd91e2a9990 100644
--- a/include/comphelper/propstate.hxx
+++ b/include/comphelper/propstate.hxx
@@ -75,7 +75,7 @@ namespace comphelper
 
         void firePropertyChange(sal_Int32 nHandle, const ::com::sun::star::uno::Any& aNewValue, const ::com::sun::star::uno::Any& aOldValue);
 
-        css::uno::Sequence<css::uno::Type> getTypes();
+        static css::uno::Sequence<css::uno::Type> getTypes();
     };