Introduce OStringChar

...similar to OUStringChar, to be used in string concatenation expressions.  And
enable the corresponding loplugin:stringadd check, and fix its findings.

Change-Id: I35ebb2253ba82bda6c98ae6ebd2ad4f27cf9abf9
Reviewed-on: https://gerrit.libreoffice.org/81456
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>

16 files changed, 58 insertions, 40 deletions

diff --git a/compilerplugins/clang/stringadd.cxx b/compilerplugins/clang/stringadd.cxx
index 67d3adc6c131..399af5138fd9 100644
--- a/compilerplugins/clang/stringadd.cxx
+++ b/compilerplugins/clang/stringadd.cxx
@@ -202,15 +202,6 @@ bool StringAdd::VisitCXXOperatorCallExpr(CXXOperatorCallExpr const* operatorCall
         if (!tc3.Class("OUString").Namespace("rtl").GlobalNamespace()
             && !tc3.Class("OString").Namespace("rtl").GlobalNamespace())
             return;
-        // we don't have OStringLiteral1, so char needs to generate a temporary
-        if (tc.Class("OString").Namespace("rtl").GlobalNamespace()
-            || tc.Struct("OStringConcat").Namespace("rtl").GlobalNamespace())
-            if (auto bindTemp = dyn_cast<CXXBindTemporaryExpr>(e->getSubExpr()))
-                if (auto cxxConstruct = dyn_cast<CXXConstructExpr>(bindTemp->getSubExpr()))
-                    if (loplugin::TypeCheck(
-                            cxxConstruct->getConstructor()->getParamDecl(0)->getType())
-                            .Char())
-                        return;
         report(DiagnosticsEngine::Warning,
                ("avoid constructing %0 from %1 on %select{L|R}2HS of + (where %select{R|L}2HS is of"
                 " type %3)"),
diff --git a/compilerplugins/clang/test/stringadd.cxx b/compilerplugins/clang/test/stringadd.cxx
index 27cc57f1395e..a8469026e509 100644
--- a/compilerplugins/clang/test/stringadd.cxx
+++ b/compilerplugins/clang/test/stringadd.cxx
@@ -191,7 +191,7 @@ void f2(char ch)
     OString s;
     // expected-error@+1 {{avoid constructing 'rtl::OString' from 'const char [4]' on RHS of + (where LHS is of type 'rtl::OString') [loplugin:stringadd]}}
     s = s + OString("xxx");
-    // no warning expected, no OStringLiteral1
+    // expected-error@+1 {{avoid constructing 'rtl::OString' from 'char' on RHS of + (where LHS is of type 'rtl::OString') [loplugin:stringadd]}}
     s = s + OString(ch);
 }
 }
diff --git a/desktop/source/deployment/registry/component/dp_component.cxx b/desktop/source/deployment/registry/component/dp_component.cxx
index 54ac54daa987..cfecf6927496 100644
--- a/desktop/source/deployment/registry/component/dp_component.cxx
+++ b/desktop/source/deployment/registry/component/dp_component.cxx
@@ -919,10 +919,10 @@ void BackendImpl::unorc_flush( Reference<XCommandEnvironment> const & xCmdEnv )
             OString buf2 =
                 "ORIGIN=" +
                 osOrigin +
-                OString(LF) +
+                OStringChar(LF) +
                 "UNO_SERVICES=?$ORIGIN/"  +
                 OUStringToOString( sNativeRDB, RTL_TEXTENCODING_ASCII_US ) +
-                OString(LF);
+                OStringChar(LF);
 
             const Reference<io::XInputStream> xData(
                 ::xmlscript::createInputStream(
diff --git a/idl/source/prj/database.cxx b/idl/source/prj/database.cxx
index b63205a0911b..2629c2d457cb 100644
--- a/idl/source/prj/database.cxx
+++ b/idl/source/prj/database.cxx
@@ -197,7 +197,7 @@ bool SvIdlDataBase::ReadIdFile( const OString& rOFileName )
                           || rTok.GetChar() == '^'
                           || rTok.GetChar() == '~' )
                         {
-                            throw SvParseException( "unknown operator '" + OString(rTok.GetChar()) + "'in define", rTok );
+                            throw SvParseException( "unknown operator '" + OStringChar(rTok.GetChar()) + "'in define", rTok );
                         }
                         if( rTok.GetChar() != '+'
                           && rTok.GetChar() != '('
diff --git a/idl/source/prj/parser.cxx b/idl/source/prj/parser.cxx
index 8bb45b10b273..be948e8ec31f 100644
--- a/idl/source/prj/parser.cxx
+++ b/idl/source/prj/parser.cxx
@@ -558,7 +558,7 @@ OString SvIdlParser::ReadString()
 void SvIdlParser::Read(char cChar)
 {
     if( !ReadIf(cChar) )
-        throw SvParseException(rInStm, "expected char '" + OString(cChar) + "'");
+        throw SvParseException(rInStm, "expected char '" + OStringChar(cChar) + "'");
 }
 
 bool SvIdlParser::ReadIf(char cChar)
diff --git a/include/rtl/string.hxx b/include/rtl/string.hxx
index 1826f42f46ac..a372a3428ca1 100644
--- a/include/rtl/string.hxx
+++ b/include/rtl/string.hxx
@@ -1971,6 +1971,7 @@ typedef rtlunittest::OString OString;
 
 #if defined LIBO_INTERNAL_ONLY && !defined RTL_STRING_UNITTEST
 using ::rtl::OString;
+using ::rtl::OStringChar;
 using ::rtl::OStringHash;
 using ::rtl::OStringLiteral;
 #endif
diff --git a/include/rtl/stringconcat.hxx b/include/rtl/stringconcat.hxx
index 588be8f0073e..f5c9a4d33a52 100644
--- a/include/rtl/stringconcat.hxx
+++ b/include/rtl/stringconcat.hxx
@@ -147,6 +147,16 @@ struct ToStringHelper< const char[ N ] >
     };
 
 template<>
+struct ToStringHelper<OStringChar>
+    {
+    static std::size_t length(OStringChar) { return 1; }
+    static char* addData(char* buffer, OStringChar data)
+    { return addDataHelper(buffer, &data.c, 1); }
+    static bool const allowOStringConcat = true;
+    static bool const allowOUStringConcat = false;
+    };
+
+template<>
 struct ToStringHelper< const sal_Unicode* >
     {
     static std::size_t length( const sal_Unicode* str ) {
diff --git a/include/rtl/stringutils.hxx b/include/rtl/stringutils.hxx
index 3ed4bfc8e950..b968845465ce 100644
--- a/include/rtl/stringutils.hxx
+++ b/include/rtl/stringutils.hxx
@@ -36,6 +36,18 @@ namespace rtl
 #if defined LIBO_INTERNAL_ONLY
 /// @cond INTERNAL
 
+// A simple wrapper around a single char.  Can be useful in string concatenation contexts, like in
+//
+//  OString s = ...;
+//  char c = ...;
+//  s += OStringChar(c);
+//
+struct SAL_WARN_UNUSED OStringChar {
+    constexpr OStringChar(char theC): c(theC) {}
+    template<typename T> OStringChar(T &&) = delete;
+    char const c;
+};
+
 /** A simple wrapper around a single sal_Unicode character.
 
     Can be useful to pass a sal_Unicode constant into an OUString-related
diff --git a/jvmfwk/plugins/sunmajor/pluginlib/sunjavaplugin.cxx b/jvmfwk/plugins/sunmajor/pluginlib/sunjavaplugin.cxx
index b9f43ac2fad5..128de9b352b0 100644
--- a/jvmfwk/plugins/sunmajor/pluginlib/sunjavaplugin.cxx
+++ b/jvmfwk/plugins/sunmajor/pluginlib/sunjavaplugin.cxx
@@ -697,7 +697,7 @@ javaPluginError jfw_plugin_startJavaVirtualMachine(
         {
             OString sAddPath = getPluginJarPath(pInfo->sVendor, pInfo->sLocation,pInfo->sVersion);
             if (!sAddPath.isEmpty())
-                opt += OString(SAL_PATHSEPARATOR) + sAddPath;
+                opt += OStringChar(SAL_PATHSEPARATOR) + sAddPath;
         }
 #endif
         if (opt == "-Xint") {
diff --git a/oox/source/vml/vmlinputstream.cxx b/oox/source/vml/vmlinputstream.cxx
index 8585facaacde..215eff35f621 100644
--- a/oox/source/vml/vmlinputstream.cxx
+++ b/oox/source/vml/vmlinputstream.cxx
@@ -352,7 +352,7 @@ void InputStream::updateBuffer()
         if( bHasOpeningBracket && !mxTextStrm->isEOF() )
         {
             // read the element text (add the leading opening bracket manually)
-            OString aElement = OString( '<' ) + readToElementEnd();
+            OString aElement = "<" + readToElementEnd();
             // check for CDATA part, starting with '<![CDATA['
             if( aElement.match( gaOpeningCData ) )
             {
diff --git a/sc/qa/unit/opencl-test.cxx b/sc/qa/unit/opencl-test.cxx
index 2cb7667751cf..abf87e6b33bd 100644
--- a/sc/qa/unit/opencl-test.cxx
+++ b/sc/qa/unit/opencl-test.cxx
@@ -738,10 +738,10 @@ void ScOpenCLTest::testSystematic()
             double fExcel = rDoc.GetValue(ScAddress(j, nBVertBegin + (i - nAVertBegin), 0));
 
             const OString sFailedMessage =
-                OString(static_cast<sal_Char>('A'+j)) +
+                OStringChar(static_cast<sal_Char>('A'+j)) +
                 OString::number(i+1) +
                 "!=" +
-                OString(static_cast<sal_Char>('A'+j)) +
+                OStringChar(static_cast<sal_Char>('A'+j)) +
                 OString::number(nBVertBegin+(i-nAVertBegin)+1);
             CPPUNIT_ASSERT_DOUBLES_EQUAL_MESSAGE(sFailedMessage.getStr(), fExcel, fLibre, 1e-10);
         }
diff --git a/sc/qa/unit/ucalc_sort.cxx b/sc/qa/unit/ucalc_sort.cxx
index a527fce73ac0..bc44ac2e9ddc 100644
--- a/sc/qa/unit/ucalc_sort.cxx
+++ b/sc/qa/unit/ucalc_sort.cxx
@@ -1147,7 +1147,7 @@ void Test::testSortRefUpdate4_Impl()
         {
             for (SCCOL nCol=0; nCol < 4; ++nCol)
             {
-                ASSERT_FORMULA_EQUAL(*m_pDoc, ScAddress(nCol,nRow+1,0), aCheck[nRow][nCol], OString("Wrong formula in " + OString('A'+nCol) + OString::number(nRow+2) + ".").getStr());
+                ASSERT_FORMULA_EQUAL(*m_pDoc, ScAddress(nCol,nRow+1,0), aCheck[nRow][nCol], OString("Wrong formula in " + OStringChar(char('A'+nCol)) + OString::number(nRow+2) + ".").getStr());
             }
         }
 
@@ -1218,7 +1218,7 @@ void Test::testSortRefUpdate4_Impl()
         {
             for (SCCOL nCol=0; nCol < 4; ++nCol)
             {
-                ASSERT_FORMULA_EQUAL(*m_pDoc, ScAddress(nCol,nRow+1,0), aCheck[nRow][nCol], OString("Wrong formula in " + OString('A'+nCol) + OString::number(nRow+2) + ".").getStr());
+                ASSERT_FORMULA_EQUAL(*m_pDoc, ScAddress(nCol,nRow+1,0), aCheck[nRow][nCol], OString("Wrong formula in " + OStringChar(char('A'+nCol)) + OString::number(nRow+2) + ".").getStr());
             }
         }
     }
diff --git a/sw/source/filter/html/htmlnumwriter.cxx b/sw/source/filter/html/htmlnumwriter.cxx
index 22c63e29ec78..cb21883db528 100644
--- a/sw/source/filter/html/htmlnumwriter.cxx
+++ b/sw/source/filter/html/htmlnumwriter.cxx
@@ -251,7 +251,7 @@ Writer& OutHTML_NumBulListStart( SwHTMLWriter& rWrt,
             }
             if( cType )
             {
-                sOut += " " OOO_STRING_SVTOOLS_HTML_O_type "=\"" + OString(cType) + "\"";
+                sOut += " " OOO_STRING_SVTOOLS_HTML_O_type "=\"" + OStringChar(cType) + "\"";
             }
 
             sal_uInt16 nStartVal = rNumFormat.GetStart();
diff --git a/sw/source/uibase/utlui/content.cxx b/sw/source/uibase/utlui/content.cxx
index 0381b2101281..5ecb48f02415 100644
--- a/sw/source/uibase/utlui/content.cxx
+++ b/sw/source/uibase/utlui/content.cxx
@@ -104,7 +104,11 @@ using namespace ::com::sun::star::text;
 using namespace ::com::sun::star::uno;
 using namespace ::com::sun::star::container;
 
-#define NAVI_BOOKMARK_DELIM     u'\x0001'
+namespace {
+
+constexpr char NAVI_BOOKMARK_DELIM = '\x01';
+
+}
 
 class SwContentArr
     : public o3tl::sorted_vector<std::unique_ptr<SwContent>, o3tl::less_uniqueptr_to<SwContent>,
@@ -3772,9 +3776,9 @@ void NaviContentBookmark::Copy( TransferDataContainer& rData ) const
 {
     rtl_TextEncoding eSysCSet = osl_getThreadTextEncoding();
 
-    OString sStrBuf(OUStringToOString(aUrl, eSysCSet) + OString(NAVI_BOOKMARK_DELIM) +
-                    OUStringToOString(aDescr, eSysCSet) + OString(NAVI_BOOKMARK_DELIM) +
-                    OString::number(static_cast<int>(nDefDrag)) + OString(NAVI_BOOKMARK_DELIM) +
+    OString sStrBuf(OUStringToOString(aUrl, eSysCSet) + OStringChar(NAVI_BOOKMARK_DELIM) +
+                    OUStringToOString(aDescr, eSysCSet) + OStringChar(NAVI_BOOKMARK_DELIM) +
+                    OString::number(static_cast<int>(nDefDrag)) + OStringChar(NAVI_BOOKMARK_DELIM) +
                     OString::number(nDocSh));
     rData.CopyByteString(SotClipboardFormatId::SONLK, sStrBuf);
 }
diff --git a/vcl/unx/generic/fontmanager/fontconfig.cxx b/vcl/unx/generic/fontmanager/fontconfig.cxx
index cdef675dd33a..6c6ea0c6563d 100644
--- a/vcl/unx/generic/fontmanager/fontconfig.cxx
+++ b/vcl/unx/generic/fontmanager/fontconfig.cxx
@@ -766,7 +766,7 @@ namespace
 
         if (!sRegion.isEmpty())
         {
-            sLangAttrib = sLang + OString('-') + sRegion;
+            sLangAttrib = sLang + "-" + sRegion;
             if (FcStrSetMember(xLangSet.get(), reinterpret_cast<const FcChar8*>(sLangAttrib.getStr())))
             {
                 return sLangAttrib;
diff --git a/xmlhelp/source/cxxhelp/provider/urlparameter.cxx b/xmlhelp/source/cxxhelp/provider/urlparameter.cxx
index 05b7f1dbb1e9..8e99fe22df03 100644
--- a/xmlhelp/source/cxxhelp/provider/urlparameter.cxx
+++ b/xmlhelp/source/cxxhelp/provider/urlparameter.cxx
@@ -712,35 +712,35 @@ InputStreamTransformer::InputStreamTransformer( URLParameter* urlParam,
 
         parString[last++] = "Program";
         OString aPureProgramm( urlParam->getByName( "Program" ) );
-        parString[last++] = OString('\'') + aPureProgramm + OString('\'');
+        parString[last++] = "'" + aPureProgramm + "'";
         parString[last++] = "Database";
-        parString[last++] = OString('\'') + urlParam->getByName( "DatabasePar" ) + OString('\'');
+        parString[last++] = "'" + urlParam->getByName( "DatabasePar" ) + "'";
         parString[last++] = "Id";
-        parString[last++] = OString('\'') + urlParam->getByName( "Id" ) + OString('\'');
+        parString[last++] = "'" + urlParam->getByName( "Id" ) + "'";
         parString[last++] = "Path";
         OString aPath( urlParam->getByName( "Path" ) );
-        parString[last++] = OString('\'') + aPath + OString('\'');
+        parString[last++] = "'" + aPath + "'";
 
         OString aPureLanguage = urlParam->getByName( "Language" );
         parString[last++] = "Language";
-        parString[last++] = OString('\'') + aPureLanguage + OString('\'');
+        parString[last++] = "'" + aPureLanguage + "'";
         parString[last++] = "System";
-        parString[last++] = OString('\'') + urlParam->getByName( "System" ) + OString('\'');
+        parString[last++] = "'" + urlParam->getByName( "System" ) + "'";
         parString[last++] = "productname";
-        parString[last++] = OString('\'') + OString(
+        parString[last++] = "'" + OString(
             pDatabases->getProductName().getStr(),
             pDatabases->getProductName().getLength(),
-            RTL_TEXTENCODING_UTF8 ) + OString('\'');
+            RTL_TEXTENCODING_UTF8 ) + "'";
         parString[last++] = "productversion";
-        parString[last++] = OString('\'') +
+        parString[last++] = "'" +
             OString(  pDatabases->getProductVersion().getStr(),
                           pDatabases->getProductVersion().getLength(),
-                          RTL_TEXTENCODING_UTF8 ) + OString('\'');
+                          RTL_TEXTENCODING_UTF8 ) + "'";
 
         parString[last++] = "imgtheme";
-        parString[last++] = OString('\'') + pDatabases->getImageTheme() + OString('\'');
+        parString[last++] = "'" + pDatabases->getImageTheme() + "'";
         parString[last++] = "hp";
-        parString[last++] = OString('\'') + urlParam->getByName( "HelpPrefix" ) + OString('\'');
+        parString[last++] = "'" + urlParam->getByName( "HelpPrefix" ) + "'";
 
         if( !parString[last-1].isEmpty() )
         {
@@ -801,7 +801,7 @@ InputStreamTransformer::InputStreamTransformer( URLParameter* urlParam,
             OString aExpandedExtensionPath = OUStringToOString( aOUExpandedExtensionPath, osl_getThreadTextEncoding() );
 
             parString[last++] = "ExtensionPath";
-            parString[last++] = OString('\'') + aExpandedExtensionPath + OString('\'');
+            parString[last++] = "'" + aExpandedExtensionPath + "'";
 
             // ExtensionId
             OString aPureExtensionId;
@@ -810,7 +810,7 @@ InputStreamTransformer::InputStreamTransformer( URLParameter* urlParam,
                 aPureExtensionId = aPath.copy( 0, iSlash );
 
             parString[last++] = "ExtensionId";
-            parString[last++] = OString('\'') + aPureExtensionId + OString('\'');
+            parString[last++] = "'" + aPureExtensionId + "'";
         }
 
         for( int i = 0; i < last; ++i )