loplugin:stringstatic also look for local statics

Add some API to O*StringLiteral, to make it easier
to use in some places that were using O*String

Change-Id: I1fb93bd47ac2065c9220d509aad3f4320326d99e
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/100270
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>

2 files changed, 66 insertions, 18 deletions

diff --git a/compilerplugins/clang/stringstatic.cxx b/compilerplugins/clang/stringstatic.cxx
index 69e6c427f90e..282144042f4e 100644
--- a/compilerplugins/clang/stringstatic.cxx
+++ b/compilerplugins/clang/stringstatic.cxx
@@ -9,13 +9,13 @@
 
 #ifndef LO_CLANG_SHARED_PLUGINS
 
-#include <set>
-
 #include "check.hxx"
 #include "compat.hxx"
 #include "plugin.hxx"
 
-/** Look for static OUString and OUString[], they can be more efficiently declared as:
+#include <unordered_set>
+
+/** Look for static O*String and O*String[], they can be more efficiently declared as:
 
         static const OUStringLiteral our_aLBEntryMap[] = {" ", ", "};
         static const OUStringLiteral sName("name");
@@ -37,9 +37,11 @@ public:
     void postRun() override;
     bool VisitVarDecl(VarDecl const*);
     bool VisitReturnStmt(ReturnStmt const*);
+    bool VisitDeclRefExpr(DeclRefExpr const*);
+
 private:
-    std::set<VarDecl const *> potentialVars;
-    std::set<VarDecl const *> excludeVars;
+    std::unordered_set<VarDecl const *> potentialVars;
+    std::unordered_set<VarDecl const *> excludeVars;
 };
 
 void StringStatic::run()
@@ -68,7 +70,7 @@ void StringStatic::postRun()
     }
     for (auto const & varDecl : potentialVars) {
         report(DiagnosticsEngine::Warning,
-                "rather declare this using OUStringLiteral or char[]",
+                "rather declare this using OUStringLiteral/OStringLiteral/char[]",
                 varDecl->getLocation())
             << varDecl->getSourceRange();
     }
@@ -76,22 +78,23 @@ void StringStatic::postRun()
 
 bool StringStatic::VisitVarDecl(VarDecl const* varDecl)
 {
-    if (ignoreLocation(varDecl)) {
+    if (ignoreLocation(varDecl))
         return true;
-    }
     QualType qt = varDecl->getType();
-    if (!varDecl->hasGlobalStorage()
-        || !varDecl->isThisDeclarationADefinition()
-        || !qt.isConstQualified()) {
+    if (!varDecl->hasGlobalStorage() && !varDecl->isStaticLocal())
         return true;
-    }
-    if (qt->isArrayType()) {
+    if (!varDecl->isThisDeclarationADefinition()
+        || !qt.isConstQualified())
+        return true;
+    if (qt->isArrayType())
         qt = qt->getAsArrayTypeUnsafe()->getElementType();
-    }
-    if (!loplugin::TypeCheck(qt).Class("OUString").Namespace("rtl").GlobalNamespace()) {
+
+    auto tc = loplugin::TypeCheck(qt);
+    if (!tc.Class("OUString").Namespace("rtl").GlobalNamespace()
+        && !tc.Class("OString").Namespace("rtl").GlobalNamespace())
         return true;
-    }
-    if (varDecl->hasInit()) {
+    if (varDecl->hasInit())
+    {
         Expr const * expr = varDecl->getInit();
         while (true) {
             if (ExprWithCleanups const * exprWithCleanups = dyn_cast<ExprWithCleanups>(expr)) {
@@ -107,7 +110,7 @@ bool StringStatic::VisitVarDecl(VarDecl const* varDecl)
                 expr = bindExpr->getSubExpr();
             }
             else if (CXXConstructExpr const * constructExpr = dyn_cast<CXXConstructExpr>(expr)) {
-                if (constructExpr->getNumArgs() != 1) {
+                if (constructExpr->getNumArgs() == 0) {
                     return true;
                 }
                 expr = constructExpr->getArg(0);
@@ -142,6 +145,27 @@ bool StringStatic::VisitReturnStmt(ReturnStmt const * returnStmt)
     return true;
 }
 
+bool StringStatic::VisitDeclRefExpr(DeclRefExpr const * declRef)
+{
+    if (ignoreLocation(declRef))
+        return true;
+    VarDecl const * varDecl = dyn_cast<VarDecl>(declRef->getDecl());
+    if (!varDecl)
+        return true;
+    if (potentialVars.count(varDecl) == 0)
+        return true;
+    // ignore globals that are used in CPPUNIT_ASSERT expressions, otherwise we can end up
+    // trying to compare an OUStringLiteral and an OUString, and CPPUNIT can't handle that
+    auto loc = compat::getBeginLoc(declRef);
+    if (compiler.getSourceManager().isMacroArgExpansion(loc))
+    {
+        StringRef name { Lexer::getImmediateMacroName(loc, compiler.getSourceManager(), compiler.getLangOpts()) };
+        if (name.startswith("CPPUNIT_ASSERT"))
+            excludeVars.insert(varDecl);
+    }
+    return true;
+}
+
 loplugin::Plugin::Registration<StringStatic> stringstatic("stringstatic");
 
 } // namespace
diff --git a/compilerplugins/clang/test/stringstatic.cxx b/compilerplugins/clang/test/stringstatic.cxx
new file mode 100644
index 000000000000..73fd38ec629a
--- /dev/null
+++ b/compilerplugins/clang/test/stringstatic.cxx
@@ -0,0 +1,24 @@
+/* -*- 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 <rtl/string.hxx>
+#include <rtl/ustring.hxx>
+
+// expected-error@+1 {{rather declare this using OUStringLiteral/OStringLiteral/char[] [loplugin:stringstatic]}}
+static const OUString TEST1 = "xxx";
+
+void test2()
+{
+    // expected-error@+1 {{rather declare this using OUStringLiteral/OStringLiteral/char[] [loplugin:stringstatic]}}
+    static const OUString XXX = "xxx";
+    // expected-error@+1 {{rather declare this using OUStringLiteral/OStringLiteral/char[] [loplugin:stringstatic]}}
+    static const OUString XXX2 = "xxx";
+    (void)XXX;
+    (void)XXX2;
+}