Improve loplugin:stringview detection of unnecessary O[U]String construction

Change-Id: Ia45119e11377e916a1e1deb5648ed9033c417d77
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/107228
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>

6 files changed, 126 insertions, 11 deletions

diff --git a/compilerplugins/clang/check.cxx b/compilerplugins/clang/check.cxx
index 2ae58504fe3b..003224a21ba1 100644
--- a/compilerplugins/clang/check.cxx
+++ b/compilerplugins/clang/check.cxx
@@ -113,6 +113,16 @@ TypeCheck TypeCheck::LvalueReference() const {
     return TypeCheck();
 }
 
+TypeCheck TypeCheck::RvalueReference() const {
+    if (!type_.isNull()) {
+        auto const t = type_->getAs<clang::RValueReferenceType>();
+        if (t != nullptr) {
+            return TypeCheck(t->getPointeeType());
+        }
+    }
+    return TypeCheck();
+}
+
 TypeCheck TypeCheck::Pointer() const {
     if (!type_.isNull()) {
         auto const t = type_->getAs<clang::PointerType>();
diff --git a/compilerplugins/clang/check.hxx b/compilerplugins/clang/check.hxx
index 03c5ab1eb1e4..9c35acff7b5e 100644
--- a/compilerplugins/clang/check.hxx
+++ b/compilerplugins/clang/check.hxx
@@ -64,6 +64,8 @@ public:
 
     TypeCheck LvalueReference() const;
 
+    TypeCheck RvalueReference() const;
+
     inline ContextCheck Class(llvm::StringRef id) const;
 
     inline ContextCheck Struct(llvm::StringRef id) const;
diff --git a/compilerplugins/clang/stringview.cxx b/compilerplugins/clang/stringview.cxx
index 06b2fb8fdf02..26af817659be 100644
--- a/compilerplugins/clang/stringview.cxx
+++ b/compilerplugins/clang/stringview.cxx
@@ -122,21 +122,85 @@ bool StringView::VisitImplicitCastExpr(ImplicitCastExpr const* expr)
     {
         handleCXXConstructExpr(e1);
     }
-    else if (auto const e2 = dyn_cast<CXXMemberCallExpr>(e))
+    else if (auto const e2 = dyn_cast<CXXFunctionalCastExpr>(e))
     {
-        handleCXXMemberCallExpr(e2);
+        auto e3 = e2->getSubExpr();
+        if (auto const e4 = dyn_cast<CXXBindTemporaryExpr>(e3))
+        {
+            e3 = e4->getSubExpr();
+        }
+        if (auto const e4 = dyn_cast<CXXConstructExpr>(e3))
+        {
+            handleCXXConstructExpr(e4);
+        }
+    }
+    else if (auto const e3 = dyn_cast<CXXMemberCallExpr>(e))
+    {
+        handleCXXMemberCallExpr(e3);
     }
     return true;
 }
 
 void StringView::handleCXXConstructExpr(CXXConstructExpr const* expr)
 {
-    if (expr->getNumArgs() != 0)
+    auto const d = expr->getConstructor();
+    switch (d->getNumParams())
     {
-        return;
+        case 0:
+            break;
+        case 1:
+        {
+            auto const t = d->getParamDecl(0)->getType();
+            if (t->isAnyCharacterType())
+            {
+                break;
+            }
+            loplugin::TypeCheck tc(t);
+            if (tc.LvalueReference()
+                    .Const()
+                    .Class("OStringLiteral")
+                    .Namespace("rtl")
+                    .GlobalNamespace()
+                || tc.LvalueReference()
+                       .Const()
+                       .Class("OUStringLiteral")
+                       .Namespace("rtl")
+                       .GlobalNamespace()
+                || tc.RvalueReference().Struct("OStringNumber").Namespace("rtl").GlobalNamespace()
+                || tc.RvalueReference().Struct("OUStringNumber").Namespace("rtl").GlobalNamespace()
+                || tc.ClassOrStruct("basic_string_view").StdNamespace())
+            {
+                break;
+            }
+            return;
+        }
+        case 2:
+        {
+            auto const t0 = d->getParamDecl(0)->getType();
+            if (t0->isPointerType() && t0->getPointeeType()->isAnyCharacterType())
+            {
+                auto const t = d->getParamDecl(1)->getType();
+                if (t->isIntegralType(compiler.getASTContext())
+                    && !(t->isBooleanType() || t->isAnyCharacterType()))
+                {
+                    break;
+                }
+            }
+            if (loplugin::TypeCheck(d->getParamDecl(1)->getType())
+                    .Struct("Dummy")
+                    .Namespace("libreoffice_internal")
+                    .Namespace("rtl")
+                    .GlobalNamespace())
+            {
+                break;
+            }
+            return;
+        }
+        default:
+            return;
     }
     report(DiagnosticsEngine::Warning,
-           "instead of an empty %0, pass an empty '%select{std::string_view|std::u16string_view}1'",
+           "instead of an %0, pass a '%select{std::string_view|std::u16string_view}1'",
            expr->getExprLoc())
         << expr->getType()
         << (loplugin::TypeCheck(expr->getType()).Class("OString").Namespace("rtl").GlobalNamespace()
diff --git a/compilerplugins/clang/test/stringview.cxx b/compilerplugins/clang/test/stringview.cxx
index 3c15d9cc4437..651e49e7bc27 100644
--- a/compilerplugins/clang/test/stringview.cxx
+++ b/compilerplugins/clang/test/stringview.cxx
@@ -7,10 +7,15 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/.
  */
 
+#include <sal/config.h>
+
+#include <string_view>
+
 #include <rtl/strbuf.hxx>
 #include <rtl/string.hxx>
 #include <rtl/ustrbuf.hxx>
 #include <rtl/ustring.hxx>
+#include <sal/types.h>
 
 void call_view(std::u16string_view) {}
 void call_view(std::string_view) {}
@@ -93,12 +98,44 @@ void f4(OUString s1, OUString s2)
 }
 }
 
-void f5()
+void f5(char const* s1, sal_Int32 n1, char16_t const* s2, sal_Int32 n2)
 {
-    // expected-error@+1 {{instead of an empty 'rtl::OString', pass an empty 'std::string_view' [loplugin:stringview]}}
+    // expected-error@+1 {{instead of an 'rtl::OString', pass a 'std::string_view' [loplugin:stringview]}}
     call_view(OString());
-    // expected-error@+1 {{instead of an empty 'rtl::OUString', pass an empty 'std::u16string_view' [loplugin:stringview]}}
+    // expected-error@+1 {{instead of an 'rtl::OString', pass a 'std::string_view' [loplugin:stringview]}}
+    call_view(OString("foo"));
+    // expected-error@+1 {{instead of an 'rtl::OString', pass a 'std::string_view' [loplugin:stringview]}}
+    call_view(OString(*s1));
+    // expected-error@+1 {{instead of an 'rtl::OString', pass a 'std::string_view' [loplugin:stringview]}}
+    call_view(OString(s1));
+    // expected-error@+1 {{instead of an 'rtl::OString', pass a 'std::string_view' [loplugin:stringview]}}
+    call_view(OString(s1, n1));
+    constexpr OStringLiteral l1("foo");
+    // expected-error@+1 {{instead of an 'rtl::OString', pass a 'std::string_view' [loplugin:stringview]}}
+    call_view(OString(l1));
+    // expected-error@+1 {{instead of an 'rtl::OString', pass a 'std::string_view' [loplugin:stringview]}}
+    call_view(OString(std::string_view("foo")));
+    // expected-error@+1 {{instead of an 'rtl::OString', pass a 'std::string_view' [loplugin:stringview]}}
+    call_view(OString(OString::number(0)));
+    // expected-error@+1 {{instead of an 'rtl::OUString', pass a 'std::u16string_view' [loplugin:stringview]}}
     call_view(OUString());
+    // expected-error@+1 {{instead of an 'rtl::OUString', pass a 'std::u16string_view' [loplugin:stringview]}}
+    call_view(OUString("foo"));
+    // expected-error@+1 {{instead of an 'rtl::OUString', pass a 'std::u16string_view' [loplugin:stringview]}}
+    call_view(OUString(u"foo"));
+    // expected-error@+1 {{instead of an 'rtl::OUString', pass a 'std::u16string_view' [loplugin:stringview]}}
+    call_view(OUString(*s2));
+    // expected-error@+1 {{instead of an 'rtl::OUString', pass a 'std::u16string_view' [loplugin:stringview]}}
+    call_view(OUString(s2));
+    // expected-error@+1 {{instead of an 'rtl::OUString', pass a 'std::u16string_view' [loplugin:stringview]}}
+    call_view(OUString(s2, n2));
+    constexpr OUStringLiteral l2(u"foo");
+    // expected-error@+1 {{instead of an 'rtl::OUString', pass a 'std::u16string_view' [loplugin:stringview]}}
+    call_view(OUString(l2));
+    // expected-error@+1 {{instead of an 'rtl::OUString', pass a 'std::u16string_view' [loplugin:stringview]}}
+    call_view(OUString(std::u16string_view(u"foo")));
+    // expected-error@+1 {{instead of an 'rtl::OUString', pass a 'std::u16string_view' [loplugin:stringview]}}
+    call_view(OUString(OUString::number(0)));
 }
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
diff --git a/sc/source/ui/docshell/impex.cxx b/sc/source/ui/docshell/impex.cxx
index 4db43588af50..0dfd3e67840a 100644
--- a/sc/source/ui/docshell/impex.cxx
+++ b/sc/source/ui/docshell/impex.cxx
@@ -62,6 +62,8 @@
 #include <editeng/editobj.hxx>
 
 #include <memory>
+#include <string_view>
+
 #include <osl/endian.h>
 
 // We don't want to end up with 2GB read in one line just because of malformed
@@ -1542,7 +1544,7 @@ void ScImportExport::EmbeddedNullTreatment( OUString & rStr )
     sal_Unicode cNull = 0;
     if (rStr.indexOf( cNull) >= 0)
     {
-        rStr = rStr.replaceAll( OUString( &cNull, 1), "");
+        rStr = rStr.replaceAll( std::u16string_view( &cNull, 1), "");
     }
 }
 
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport2.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport2.cxx
index 2f1e60fd8c9c..857f27ee636d 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport2.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport2.cxx
@@ -282,8 +282,8 @@ DECLARE_OOXMLEXPORT_TEST(testFdo51034, "fdo51034.odt")
 // Replace symbol - (i.e. U+2212) with ASCII - , LO does this change and it shouldn't matter.
 static void CHECK_FORMULA(OUString const & expected, OUString const & actual) {
     CPPUNIT_ASSERT_EQUAL(
-        expected.replaceAll( " ", "" ).replaceAll( OUString(u"\u2212"), "-" ),
-        actual.replaceAll( " ", "" ).replaceAll( OUString(u"\u2212"), "-" ));
+        expected.replaceAll( " ", "" ).replaceAll( u"\u2212", "-" ),
+        actual.replaceAll( " ", "" ).replaceAll( u"\u2212", "-" ));
 }
 
 DECLARE_OOXMLEXPORT_TEST(testMathAccents, "math-accents.docx")