new clang plugin: staticmethods

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

Change-Id: Ie6684cc95d088e8750b300a028b49f763da00345

3 files changed, 152 insertions, 14 deletions

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: */