revert part of "improve loplugin:stringconstant"

Revert part of
    commit dd8d5e5795358d732a9f7a8af7c35f662321e332
    Date:   Mon Apr 29 11:18:21 2019 +0200
    improve loplugin:stringconstant

sberg's original gerrit comment:
but there can also be other problematic overloads for parameters like
`void const *` or `std::string_view`.  I'm not sure this change is worth
the potential false positives.

and continuing IRC discussion:
<noelgrandin> I'll revert the compilerplugins/ part
<sberg> noelgrandin, my main concern is that /if/ somebody eventually
runs into such an overload situation, it's really hard to get the
warnings/errors fixed for those people, short of going into the plugin
itself

Change-Id: I4916ce8943c4319d7ef9084e22d6a0eeb430b15c

2 files changed, 45 insertions, 76 deletions

diff --git a/compilerplugins/clang/stringconstant.cxx b/compilerplugins/clang/stringconstant.cxx
index 6a009e510297..05cfa03ff711 100644
--- a/compilerplugins/clang/stringconstant.cxx
+++ b/compilerplugins/clang/stringconstant.cxx
@@ -58,19 +58,7 @@ bool isLhsOfAssignment(FunctionDecl const * decl, unsigned parameter) {
         || (oo >= OO_PlusEqual && oo <= OO_GreaterGreaterEqual);
 }
 
-bool typecheckIsOUStringParam(const clang::QualType t) {
-    return bool(loplugin::TypeCheck(t).NotSubstTemplateTypeParmType()
-            .LvalueReference().Const().NotSubstTemplateTypeParmType()
-            .Class("OUString").Namespace("rtl").GlobalNamespace());
-}
-
-bool typecheckIsOStringParam(const clang::QualType t) {
-    return bool(loplugin::TypeCheck(t).NotSubstTemplateTypeParmType()
-            .LvalueReference().Const().NotSubstTemplateTypeParmType()
-            .Class("OString").Namespace("rtl").GlobalNamespace());
-}
-
-bool hasOverloads(FunctionDecl const * decl, unsigned arguments, unsigned paramIndex) {
+bool hasOverloads(FunctionDecl const * decl, unsigned arguments) {
     int n = 0;
     auto ctx = decl->getDeclContext();
     if (ctx->getDeclKind() == Decl::LinkageSpec) {
@@ -79,32 +67,17 @@ bool hasOverloads(FunctionDecl const * decl, unsigned arguments, unsigned paramI
     auto res = ctx->lookup(decl->getDeclName());
     for (auto d = res.begin(); d != res.end(); ++d) {
         FunctionDecl const * f = dyn_cast<FunctionDecl>(*d);
-        if (f == nullptr || f->getMinRequiredArguments() > arguments
-            || f->getNumParams() < arguments) {
-            continue;
-        }
-        auto consDecl = dyn_cast<CXXConstructorDecl>(f);
-        if (consDecl && consDecl->isCopyOrMoveConstructor()) {
-            continue;
-        }
-        // Deleted stuff like in ORowSetValueDecorator in connectivity can cause
-        // trouble.
-        if (consDecl && consDecl->isDeleted()) {
-            return true;
-        }
-        if (paramIndex >= f->getNumParams()) {
-            continue;
-        }
-        auto t = f->getParamDecl(paramIndex)->getType();
-        // bool because 'const char *' converts to bool
-        if (!typecheckIsOUStringParam(t) && !typecheckIsOStringParam(t)
-            && !loplugin::TypeCheck(t).Pointer().Const().Char()
-            && !loplugin::TypeCheck(t).AnyBoolean()) {
-            continue;
-        }
-        ++n;
-        if (n == 2) {
-            return true;
+        if (f != nullptr && f->getMinRequiredArguments() <= arguments
+            && f->getNumParams() >= arguments)
+        {
+            auto consDecl = dyn_cast<CXXConstructorDecl>(f);
+            if (consDecl && consDecl->isCopyOrMoveConstructor()) {
+                continue;
+            }
+            ++n;
+            if (n == 2) {
+                return true;
+            }
         }
     }
     return false;
@@ -296,6 +269,29 @@ bool StringConstant::VisitCallExpr(CallExpr const * expr) {
     if (fdecl == nullptr) {
         return true;
     }
+    for (unsigned i = 0; i != fdecl->getNumParams(); ++i) {
+        auto t = fdecl->getParamDecl(i)->getType();
+        if (loplugin::TypeCheck(t).NotSubstTemplateTypeParmType()
+            .LvalueReference().Const().NotSubstTemplateTypeParmType()
+            .Class("OUString").Namespace("rtl").GlobalNamespace())
+        {
+            if (!(isLhsOfAssignment(fdecl, i)
+                  || hasOverloads(fdecl, expr->getNumArgs())))
+            {
+                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")
     //TODO: u.compareToIgnoreAsciiCaseAscii("foo") -> u.???("foo")
@@ -777,25 +773,6 @@ bool StringConstant::VisitCallExpr(CallExpr const * expr) {
         }
         return true;
     }
-    for (unsigned i = 0; i != fdecl->getNumParams(); ++i) {
-        auto t = fdecl->getParamDecl(i)->getType();
-        if (typecheckIsOUStringParam(t))
-        {
-            if (!(isLhsOfAssignment(fdecl, i)
-                  || hasOverloads(fdecl, expr->getNumArgs(), i)))
-            {
-                handleOUStringCtor(expr, i, fdecl, true);
-            }
-        }
-        if (typecheckIsOStringParam(t))
-        {
-            if (!(isLhsOfAssignment(fdecl, i)
-                  || hasOverloads(fdecl, expr->getNumArgs(), i)))
-            {
-                handleOStringCtor(expr, i, fdecl, true);
-            }
-        }
-    }
     return true;
 }
 
@@ -1199,23 +1176,27 @@ bool StringConstant::VisitCXXConstructExpr(CXXConstructExpr const * expr) {
     auto consDecl = expr->getConstructor();
     for (unsigned i = 0; i != consDecl->getNumParams(); ++i) {
         auto t = consDecl->getParamDecl(i)->getType();
-        if (typecheckIsOUStringParam(t))
+        if (loplugin::TypeCheck(t).NotSubstTemplateTypeParmType()
+            .LvalueReference().Const().NotSubstTemplateTypeParmType()
+            .Class("OUString").Namespace("rtl").GlobalNamespace())
         {
             auto argExpr = expr->getArg(i);
             if (argExpr && i <= consDecl->getNumParams())
             {
-                if (!hasOverloads(consDecl, expr->getNumArgs(), i))
+                if (!hasOverloads(consDecl, expr->getNumArgs()))
                 {
                     handleOUStringCtor(expr, argExpr, consDecl, true);
                 }
             }
         }
-        if (typecheckIsOStringParam(t))
+        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(), i))
+                if (!hasOverloads(consDecl, expr->getNumArgs()))
                 {
                     handleOStringCtor(expr, argExpr, consDecl, true);
                 }
diff --git a/compilerplugins/clang/test/stringconstant.cxx b/compilerplugins/clang/test/stringconstant.cxx
index 1eda24580ab2..49ae3b68d035 100644
--- a/compilerplugins/clang/test/stringconstant.cxx
+++ b/compilerplugins/clang/test/stringconstant.cxx
@@ -17,7 +17,6 @@
 extern void foo(OUString const &);
 
 struct Foo {
-    Foo(int, const OUString &) {}
     Foo(OUString const &, int) {}
     Foo(OUString const &) {}
     void foo(OUString const &) const {}
@@ -29,11 +28,6 @@ struct Foo2 {
     void foo(OString const &) const {}
 };
 
-struct NegativeFoo {
-    NegativeFoo(const OString&, const OString& ) {}
-    NegativeFoo(const OString&, const OUString& ) {}
-};
-
 int main() {
     char const s1[] = "foo";
     char const * const s2 = "foo";
@@ -73,15 +67,9 @@ int main() {
     (void)aFoo;
     Foo aFoo2(OUString("xxx")); // expected-error {{in call of 'Foo::Foo', replace 'OUString' constructed from a string literal directly with the string literal}}
     aFoo2.foo(OUString("xxx")); // expected-error {{in call of 'Foo::foo', replace 'OUString' constructed from a string literal directly with the string literal}}
-    Foo aFoo3(1, OUString("xxx")); // expected-error {{in call of 'Foo::Foo', replace 'OUString' constructed from a string literal directly with the string literal}}
-    (void)aFoo3;
-
-    Foo2 aFoo4(OString("xxx")); // expected-error {{in call of 'Foo2::Foo2', replace 'OUString' constructed from a string literal directly with the string literal}}
-    aFoo4.foo(OString("xxx")); // expected-error {{in call of 'Foo2::foo', replace 'OUString' constructed from a string literal directly with the string literal}}
 
-    // no warning expected
-    NegativeFoo aNegativeFoo(OString("xxx"), OUString("1"));
-    (void)aNegativeFoo;
+    Foo2 aFoo3(OString("xxx")); // expected-error {{in call of 'Foo2::Foo2', replace 'OUString' constructed from a string literal directly with the string literal}}
+    aFoo3.foo(OString("xxx")); // expected-error {{in call of 'Foo2::foo', replace 'OUString' constructed from a string literal directly with the string literal}}
 
     (void) OUString("xxx", 3, RTL_TEXTENCODING_ASCII_US); // expected-error {{simplify construction of 'OUString' with string constant argument [loplugin:stringconstant]}}
     (void) OUString("xxx", 3, RTL_TEXTENCODING_ISO_8859_1); // expected-error {{suspicious 'rtl::OUString' constructor with text encoding 12 but plain ASCII content; use 'RTL_TEXTENCODING_ASCII_US' instead [loplugin:stringconstant]}}