loplugin:stringconstant look for unnecessary OString constructor use

and tweak the methods in check.hxx to make them more flexible when
called with
dc.Class(xxx ? "foo" : "bar")

Change-Id: I881fe628f22121ced4d8849715d6b1c92b092da1
Reviewed-on: https://gerrit.libreoffice.org/64207
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>

1 files changed, 59 insertions, 2 deletions

diff --git a/compilerplugins/clang/stringconstant.cxx b/compilerplugins/clang/stringconstant.cxx
index dd4eeff3763a..83a62a617d06 100644
--- a/compilerplugins/clang/stringconstant.cxx
+++ b/compilerplugins/clang/stringconstant.cxx
@@ -163,10 +163,23 @@ private:
         CallExpr const * expr, unsigned arg, FunctionDecl const * callee,
         bool explicitFunctionalCastNotation);
 
+    void handleOStringCtor(
+        CallExpr const * expr, unsigned arg, FunctionDecl const * callee,
+        bool explicitFunctionalCastNotation);
+
     void handleOUStringCtor(
         Expr const * expr, Expr const * argExpr, FunctionDecl const * callee,
         bool explicitFunctionalCastNotation);
 
+    void handleOStringCtor(
+        Expr const * expr, Expr const * argExpr, FunctionDecl const * callee,
+        bool explicitFunctionalCastNotation);
+
+    enum class StringKind { Unicode, Char };
+    void handleStringCtor(
+        Expr const * expr, Expr const * argExpr, FunctionDecl const * callee,
+        bool explicitFunctionalCastNotation, StringKind stringKind);
+
     void handleFunArgOstring(
         CallExpr const * expr, unsigned arg, FunctionDecl const * callee);
 
@@ -268,6 +281,16 @@ bool StringConstant::VisitCallExpr(CallExpr const * expr) {
                 handleOUStringCtor(expr, i, fdecl, true);
             }
         }
+        if (loplugin::TypeCheck(t).NotSubstTemplateTypeParmType()
+            .LvalueReference().Const().NotSubstTemplateTypeParmType()
+            .Class("OString").Namespace("rtl").GlobalNamespace())
+        {
+            if (!(isLhsOfAssignment(fdecl, i)
+                  || hasOverloads(fdecl, expr->getNumArgs())))
+            {
+                handleOStringCtor(expr, i, fdecl, true);
+            }
+        }
     }
     loplugin::DeclCheck dc(fdecl);
     //TODO: u.compareToAscii("foo") -> u.???("foo")
@@ -1166,6 +1189,19 @@ bool StringConstant::VisitCXXConstructExpr(CXXConstructExpr const * expr) {
                 }
             }
         }
+        if (loplugin::TypeCheck(t).NotSubstTemplateTypeParmType()
+            .LvalueReference().Const().NotSubstTemplateTypeParmType()
+            .Class("OString").Namespace("rtl").GlobalNamespace())
+        {
+            auto argExpr = expr->getArg(i);
+            if (argExpr && i <= consDecl->getNumParams())
+            {
+                if (!hasOverloads(consDecl, expr->getNumArgs()))
+                {
+                    handleOStringCtor(expr, argExpr, consDecl, true);
+                }
+            }
+        }
     }
 
     return true;
@@ -1785,10 +1821,31 @@ void StringConstant::handleOUStringCtor(
     handleOUStringCtor(expr, expr->getArg(arg), callee, explicitFunctionalCastNotation);
 }
 
+void StringConstant::handleOStringCtor(
+    CallExpr const * expr, unsigned arg, FunctionDecl const * callee,
+    bool explicitFunctionalCastNotation)
+{
+    handleOStringCtor(expr, expr->getArg(arg), callee, explicitFunctionalCastNotation);
+}
+
 void StringConstant::handleOUStringCtor(
     Expr const * expr, Expr const * argExpr, FunctionDecl const * callee,
     bool explicitFunctionalCastNotation)
 {
+    handleStringCtor(expr, argExpr, callee, explicitFunctionalCastNotation, StringKind::Unicode);
+}
+
+void StringConstant::handleOStringCtor(
+    Expr const * expr, Expr const * argExpr, FunctionDecl const * callee,
+    bool explicitFunctionalCastNotation)
+{
+    handleStringCtor(expr, argExpr, callee, explicitFunctionalCastNotation, StringKind::Char);
+}
+
+void StringConstant::handleStringCtor(
+    Expr const * expr, Expr const * argExpr, FunctionDecl const * callee,
+    bool explicitFunctionalCastNotation, StringKind stringKind)
+{
     auto e0 = argExpr->IgnoreParenImpCasts();
     auto e1 = dyn_cast<CXXFunctionalCastExpr>(e0);
     if (e1 == nullptr) {
@@ -1808,7 +1865,7 @@ void StringConstant::handleOUStringCtor(
         return;
     }
     if (!loplugin::DeclCheck(e3->getConstructor()).MemberFunction()
-        .Class("OUString").Namespace("rtl").GlobalNamespace())
+        .Class(stringKind == StringKind::Unicode ? "OUString" : "OString").Namespace("rtl").GlobalNamespace())
     {
         return;
     }
@@ -1825,7 +1882,7 @@ void StringConstant::handleOUStringCtor(
         && e3->getConstructor()->getNumParams() == 1
         && (loplugin::TypeCheck(
                 e3->getConstructor()->getParamDecl(0)->getType())
-            .Typedef("sal_Unicode").GlobalNamespace()))
+            .Typedef(stringKind == StringKind::Unicode ? "sal_Unicode" : "char").GlobalNamespace()))
     {
         // It may not be easy to rewrite OUString(c), esp. given there is no
         // OUString ctor taking an OUStringLiteral1 arg, so don't warn there: