Make loplugin:stringadd slightly more aggressive

...by assuming that all const member functions are side-effect free.  (This
presumably means that some of the special cases in
StringAdd::isSideEffectFree are obsoleted by this more general case, but any
such removal is postponed to later clean-up.)

(Came across this when idly wondering why
8b7f948d9d79393bc6c1b11d239706666fd5d7de "sc, VmlFormControlExporter: avoid
OStringBuffer style" had not been found by the plugin before.)

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

12 files changed, 71 insertions, 34 deletions

diff --git a/compilerplugins/clang/stringadd.cxx b/compilerplugins/clang/stringadd.cxx
index c25992928f1e..394139dbb354 100644
--- a/compilerplugins/clang/stringadd.cxx
+++ b/compilerplugins/clang/stringadd.cxx
@@ -378,6 +378,36 @@ bool StringAdd::isSideEffectFree(Expr const* expr)
                         }
                     }
                 }
+                // Aggressively assume that calls to const member functions are side effect free (if
+                // all of the call's sub-expressions are):
+                if (calleeMethodDecl->isConst())
+                {
+                    auto sef = true;
+                    // Other options besides CXXMemberCallExpr are e.g. CXXOperatorCallExpr which
+                    // does not have such a target expression:
+                    if (auto const mce = dyn_cast<CXXMemberCallExpr>(callExpr))
+                    {
+                        if (!isSideEffectFree(mce->getImplicitObjectArgument()))
+                        {
+                            sef = false;
+                        }
+                    }
+                    if (sef)
+                    {
+                        for (unsigned i = 0; i != callExpr->getNumArgs(); ++i)
+                        {
+                            if (!isSideEffectFree(callExpr->getArg(i)))
+                            {
+                                sef = false;
+                                break;
+                            }
+                        }
+                    }
+                    if (sef)
+                    {
+                        return true;
+                    }
+                }
             }
         if (auto calleeFunctionDecl = dyn_cast_or_null<FunctionDecl>(callExpr->getCalleeDecl()))
             if (calleeFunctionDecl && calleeFunctionDecl->getIdentifier())
diff --git a/compilerplugins/clang/test/stringadd.cxx b/compilerplugins/clang/test/stringadd.cxx
index fb805ce519b9..a20b64698433 100644
--- a/compilerplugins/clang/test/stringadd.cxx
+++ b/compilerplugins/clang/test/stringadd.cxx
@@ -235,4 +235,28 @@ void f2(char ch)
     s = s + OString(ch);
 }
 }
+
+namespace test10
+{
+struct C
+{
+    OString constStringFunction(int) const;
+    OString nonConstStringFunction();
+    int constIntFunction() const;
+    int nonConstIntFunction();
+};
+
+C getC();
+
+void f1(C c)
+{
+    OString s;
+    // expected-error@+1 {{simplify by merging with the preceding assignment [loplugin:stringadd]}}
+    s += c.constStringFunction(c.constIntFunction());
+    s += c.constStringFunction(c.nonConstIntFunction());
+    s += c.nonConstStringFunction();
+    s += getC().constStringFunction(c.constIntFunction());
+}
+}
+
 /* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
diff --git a/extensions/source/bibliography/datman.cxx b/extensions/source/bibliography/datman.cxx
index a492c839712e..875938b7924f 100644
--- a/extensions/source/bibliography/datman.cxx
+++ b/extensions/source/bibliography/datman.cxx
@@ -848,9 +848,7 @@ void BibDataManager::startQueryWith(const OUString& rQuery)
     OUString aQueryString;
     if(!rQuery.isEmpty())
     {
-        aQueryString=aQuoteChar;
-        aQueryString+=getQueryField();
-        aQueryString+=aQuoteChar + " like '";
+        aQueryString=aQuoteChar + getQueryField() + aQuoteChar + " like '";
         OUString sQuery = rQuery.replaceAll("?","_").replaceAll("*","%");
         aQueryString += sQuery + "%'";
     }
diff --git a/i18nlangtag/qa/cppunit/test_languagetag.cxx b/i18nlangtag/qa/cppunit/test_languagetag.cxx
index bd6f1b27bf22..4fcc05785f2e 100644
--- a/i18nlangtag/qa/cppunit/test_languagetag.cxx
+++ b/i18nlangtag/qa/cppunit/test_languagetag.cxx
@@ -760,8 +760,7 @@ void TestLanguageTag::testAllIsoLangEntries()
         LanguageTag aTagID( elem.mnLang);
         if (!checkMapping( elem.maBcp47, aTagString.getBcp47()))
         {
-            OString aMessage( OUStringToOString( elem.maBcp47, RTL_TEXTENCODING_ASCII_US));
-            aMessage += " -> " + OUStringToOString( aTagString.getBcp47(), RTL_TEXTENCODING_ASCII_US);
+            OString aMessage( OUStringToOString( elem.maBcp47, RTL_TEXTENCODING_ASCII_US) + " -> " + OUStringToOString( aTagString.getBcp47(), RTL_TEXTENCODING_ASCII_US) );
             CPPUNIT_ASSERT_EQUAL_MESSAGE( aMessage.getStr(), aTagString.getBcp47(), elem.maBcp47 );
         }
         if (elem.maBcp47 != aTagID.getBcp47())
diff --git a/sc/source/ui/miscdlgs/solveroptions.cxx b/sc/source/ui/miscdlgs/solveroptions.cxx
index 07d86f5a9ee5..b82654b5f5d5 100644
--- a/sc/source/ui/miscdlgs/solveroptions.cxx
+++ b/sc/source/ui/miscdlgs/solveroptions.cxx
@@ -286,8 +286,8 @@ void ScSolverOptionsDialog::EditOption()
             {
                 pStringItem->SetIntValue(m_xIntDialog->GetValue());
 
-                OUString sTxt(pStringItem->GetText() + ": ");
-                sTxt += OUString::number(pStringItem->GetIntValue());
+                OUString sTxt(
+                    pStringItem->GetText() + ": " + OUString::number(pStringItem->GetIntValue()));
 
                 m_xLbSettings->set_text(nEntry, sTxt, 0);
             }
diff --git a/svl/qa/unit/lockfiles/test_lockfiles.cxx b/svl/qa/unit/lockfiles/test_lockfiles.cxx
index 78e17f2ca5d1..b2bba0f9ce3c 100644
--- a/svl/qa/unit/lockfiles/test_lockfiles.cxx
+++ b/svl/qa/unit/lockfiles/test_lockfiles.cxx
@@ -129,9 +129,7 @@ void LockfileTest::testLOLockFileContent()
     // User name
     sal_Int32 nFirstChar = 0;
     sal_Int32 nNextComma = sLockFileContent.indexOf(',', nFirstChar);
-    OUString sUserName;
-    sUserName += aUserOpt.GetFirstName() + " ";
-    sUserName += aUserOpt.GetLastName();
+    OUString sUserName = aUserOpt.GetFirstName() + " " + aUserOpt.GetLastName();
     CPPUNIT_ASSERT_EQUAL(sUserName, sLockFileContent.copy(nFirstChar, nNextComma - nFirstChar));
 
     // System user name
@@ -403,9 +401,7 @@ void LockfileTest::testWordLockFileContent()
     aLockFile.RemoveFileDirectly();
 
     // First character is the size of the user name
-    OUString sUserName;
-    sUserName += aUserOpt.GetFirstName() + " ";
-    sUserName += aUserOpt.GetLastName();
+    OUString sUserName = aUserOpt.GetFirstName() + " " + aUserOpt.GetLastName();
     int nIndex = 0;
     CPPUNIT_ASSERT_EQUAL(sUserName.getLength(), static_cast<sal_Int32>(sLockFileContent[nIndex]));
 
@@ -457,9 +453,7 @@ void LockfileTest::testExcelLockFileContent()
     aLockFile.RemoveFileDirectly();
 
     // First character is the size of the user name
-    OUString sUserName;
-    sUserName += aUserOpt.GetFirstName() + " ";
-    sUserName += aUserOpt.GetLastName();
+    OUString sUserName = aUserOpt.GetFirstName() + " " + aUserOpt.GetLastName();
     int nIndex = 0;
     CPPUNIT_ASSERT_EQUAL(sUserName.getLength(), static_cast<sal_Int32>(sLockFileContent[nIndex]));
 
@@ -516,9 +510,7 @@ void LockfileTest::testPowerPointLockFileContent()
     aLockFile.RemoveFileDirectly();
 
     // First character is the size of the user name
-    OUString sUserName;
-    sUserName += aUserOpt.GetFirstName() + " ";
-    sUserName += aUserOpt.GetLastName();
+    OUString sUserName = aUserOpt.GetFirstName() + " " + aUserOpt.GetLastName();
     int nIndex = 0;
     CPPUNIT_ASSERT_EQUAL(sUserName.getLength(), static_cast<sal_Int32>(sLockFileContent[nIndex]));
 
diff --git a/sw/source/core/doc/doc.cxx b/sw/source/core/doc/doc.cxx
index e1929ca2978a..6539cc5fd1db 100644
--- a/sw/source/core/doc/doc.cxx
+++ b/sw/source/core/doc/doc.cxx
@@ -566,8 +566,7 @@ static void lcl_FormatPostIt(
             " ";
     }
     aStr += SwViewShell::GetShellRes()->aPostItAuthor;
-    aStr += sTmp;
-    aStr += pField->GetPar1() + " ";
+    aStr += sTmp + pField->GetPar1() + " ";
     SvtSysLocale aSysLocale;
     aStr += /*(LocaleDataWrapper&)*/aSysLocale.GetLocaleData().getDate( pField->GetDate() );
     if(pField->GetResolved())
diff --git a/sw/source/core/tox/ToxTextGenerator.cxx b/sw/source/core/tox/ToxTextGenerator.cxx
index 199e30256475..b1c3cd013d8a 100644
--- a/sw/source/core/tox/ToxTextGenerator.cxx
+++ b/sw/source/core/tox/ToxTextGenerator.cxx
@@ -154,8 +154,7 @@ ToxTextGenerator::GenerateTextForChapterToken(const SwFormToken& chapterToken, c
         retval += aField.GetNumber(pLayout); // get the string number without pre/postfix
     }
     else if (CF_NUMBER_NOPREPST == chapterToken.nChapterFormat || CF_NUM_TITLE == chapterToken.nChapterFormat) {
-        retval += aField.GetNumber(pLayout) + " ";
-        retval += aField.GetTitle(pLayout);
+        retval += aField.GetNumber(pLayout) + " " + aField.GetTitle(pLayout);
     } else if (CF_TITLE == chapterToken.nChapterFormat) {
         retval += aField.GetTitle(pLayout);
     }
diff --git a/sw/source/filter/ww8/rtfattributeoutput.cxx b/sw/source/filter/ww8/rtfattributeoutput.cxx
index 34e6cabd0591..5759c4f60c89 100644
--- a/sw/source/filter/ww8/rtfattributeoutput.cxx
+++ b/sw/source/filter/ww8/rtfattributeoutput.cxx
@@ -518,8 +518,8 @@ void RtfAttributeOutput::StartRuby(const SwTextNode& rNode, sal_Int32 /*nPos*/,
 {
     WW8Ruby aWW8Ruby(rNode, rRuby, GetExport());
     OUString aStr(FieldString(ww::eEQ) + "\\* jc");
-    aStr += OUString::number(aWW8Ruby.GetJC()) + " \\* \"Font:";
-    aStr += aWW8Ruby.GetFontFamily() + "\" \\* hps";
+    aStr += OUString::number(aWW8Ruby.GetJC()) + " \\* \"Font:" + aWW8Ruby.GetFontFamily()
+            + "\" \\* hps";
     aStr += OUString::number((aWW8Ruby.GetRubyHeight() + 5) / 10) + " \\o";
     if (aWW8Ruby.GetDirective())
     {
diff --git a/sw/source/filter/ww8/wrtw8nds.cxx b/sw/source/filter/ww8/wrtw8nds.cxx
index da5253f723b7..e40c7eeb5138 100644
--- a/sw/source/filter/ww8/wrtw8nds.cxx
+++ b/sw/source/filter/ww8/wrtw8nds.cxx
@@ -903,8 +903,8 @@ void WW8AttributeOutput::StartRuby( const SwTextNode& rNode, sal_Int32 /*nPos*/,
 {
     WW8Ruby aWW8Ruby(rNode, rRuby, GetExport());
     OUString aStr( FieldString( ww::eEQ ) + "\\* jc" );
-    aStr += OUString::number(aWW8Ruby.GetJC()) + " \\* \"Font:";
-    aStr += aWW8Ruby.GetFontFamily() + "\" \\* hps";
+    aStr += OUString::number(aWW8Ruby.GetJC()) + " \\* \"Font:" + aWW8Ruby.GetFontFamily()
+        + "\" \\* hps";
     aStr += OUString::number((aWW8Ruby.GetRubyHeight() + 5) / 10) + " \\o";
     if (aWW8Ruby.GetDirective())
     {
diff --git a/sw/source/uibase/sidebar/PageMarginControl.cxx b/sw/source/uibase/sidebar/PageMarginControl.cxx
index 3dfd61a1f6fa..19da13693b6a 100644
--- a/sw/source/uibase/sidebar/PageMarginControl.cxx
+++ b/sw/source/uibase/sidebar/PageMarginControl.cxx
@@ -330,11 +330,9 @@ void PageMarginControl::FillHelpText( const bool bUserCustomValuesAvailable )
         aHelpText += m_xWidthHeightField->get_text();
         aHelpText += m_bUserCustomMirrored ? aOuter : aRight;
         SetMetricValue( *m_xWidthHeightField, m_nUserCustomPageRightMargin, m_eUnit );
-        aHelpText += m_xWidthHeightField->get_text();
-        aHelpText += aTop;
+        aHelpText += m_xWidthHeightField->get_text() + aTop;
         SetMetricValue( *m_xWidthHeightField, m_nUserCustomPageTopMargin, m_eUnit );
-        aHelpText += m_xWidthHeightField->get_text();
-        aHelpText += aBottom;
+        aHelpText += m_xWidthHeightField->get_text() + aBottom;
         SetMetricValue( *m_xWidthHeightField, m_nUserCustomPageBottomMargin, m_eUnit );
         aHelpText += m_xWidthHeightField->get_text();
     }
diff --git a/ucb/source/ucp/hierarchy/hierarchycontent.cxx b/ucb/source/ucp/hierarchy/hierarchycontent.cxx
index 76391b4bfc51..c92264fc4c16 100644
--- a/ucb/source/ucp/hierarchy/hierarchycontent.cxx
+++ b/ucb/source/ucp/hierarchy/hierarchycontent.cxx
@@ -1365,9 +1365,7 @@ void HierarchyContent::insert( sal_Int32 nNameClashResolve,
                 }
                 else
                 {
-                    OUString aNewTitle( m_aProps.getTitle() );
-                    aNewTitle += "_" +
-                        OUString::number( nTry );
+                    OUString aNewTitle( m_aProps.getTitle() + "_" + OUString::number( nTry ) );
                     m_aProps.setTitle( aNewTitle );
                 }
             }