chart2/source/controller/main/ChartController_TextEdit.cxx | 2 compilerplugins/clang/stringplusequal.cxx | 103 +++++++++++++ compilerplugins/clang/test/stringplusequal.cxx | 32 ++++ emfio/source/reader/emfreader.cxx | 4 oox/source/export/shapes.cxx | 2 sfx2/source/commandpopup/CommandPopup.cxx | 3 solenv/CompilerTest_compilerplugins_clang.mk | 1 svx/source/svdraw/svdmrkv.cxx | 6 sw/source/core/edit/acorrect.cxx | 2 unoxml/source/rdf/CURI.cxx | 2 vcl/jsdialog/jsdialogregister.cxx | 2 11 files changed, 146 insertions(+), 13 deletions(-)
New commits: commit c2a0e06b637f798b508de408f820f496a5419d9f Author: Noel Grandin <[email protected]> AuthorDate: Wed Nov 19 11:55:39 2025 +0200 Commit: Noel Grandin <[email protected]> CommitDate: Thu Nov 20 07:26:33 2025 +0100 new loplugin:stringplusequal which simplifies O[U]String expressions like A = A + foo into A += foo mostly because then they get spotted by other string plugins. Change-Id: I2b81b701144acdf18743b52cb0befa7f90fd9528 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/194191 Tested-by: Jenkins Reviewed-by: Noel Grandin <[email protected]> diff --git a/chart2/source/controller/main/ChartController_TextEdit.cxx b/chart2/source/controller/main/ChartController_TextEdit.cxx index 0cd974176672..cb5252629f9b 100644 --- a/chart2/source/controller/main/ChartController_TextEdit.cxx +++ b/chart2/source/controller/main/ChartController_TextEdit.cxx @@ -189,7 +189,7 @@ uno::Sequence< uno::Reference< chart2::XFormattedString > > ChartController::Get uno::Reference< chart2::XFormattedString2 > xFmtStr = chart2::FormattedString::create(m_xCC); if (bNextPara) - aNewString = aNewString + OUStringChar(' '); + aNewString += OUStringChar(' '); xFmtStr->setString(aNewString); aNewStrings.emplace_back(xFmtStr); diff --git a/compilerplugins/clang/stringplusequal.cxx b/compilerplugins/clang/stringplusequal.cxx new file mode 100644 index 000000000000..957ce7df095b --- /dev/null +++ b/compilerplugins/clang/stringplusequal.cxx @@ -0,0 +1,103 @@ +/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */ +/* + * This file is part of the LibreOffice project. + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ + +#include "check.hxx" +#include "plugin.hxx" +#include "config_clang.h" +#include <vector> + +/** Look for OUString/OString being appended to using "foo = foo +" + */ +namespace +{ +class StringPlusEqual : public clang::RecursiveASTVisitor<StringPlusEqual>, public loplugin::Plugin +{ +public: + explicit StringPlusEqual(loplugin::InstantiationData const& rData) + : Plugin(rData) + { + } + + void run() override; + bool VisitCallExpr(CallExpr const*); +}; + +void StringPlusEqual::run() { TraverseDecl(compiler.getASTContext().getTranslationUnitDecl()); } + +bool StringPlusEqual::VisitCallExpr(CallExpr const* callExpr) +{ + if (ignoreLocation(callExpr)) + return true; + auto operatorCallExpr = dyn_cast<CXXOperatorCallExpr>(callExpr); + if (!operatorCallExpr) + return true; + if (operatorCallExpr->getOperator() != OO_Equal) + return true; + + if (auto memberExpr = dyn_cast<MemberExpr>(callExpr->getArg(0))) + { + auto tc = loplugin::TypeCheck(memberExpr->getType()); + if (!tc.Class("OUString").Namespace("rtl").GlobalNamespace() + && !tc.Class("OString").Namespace("rtl").GlobalNamespace()) + return true; + auto fieldDecl = dyn_cast<FieldDecl>(memberExpr->getMemberDecl()); + if (isInUnoIncludeFile( + compiler.getSourceManager().getSpellingLoc(fieldDecl->getLocation()))) + return true; + if (ignoreLocation(compiler.getSourceManager().getSpellingLoc(fieldDecl->getLocation()))) + return true; + auto rhs = dyn_cast<CXXConstructExpr>(callExpr->getArg(1)->IgnoreImplicit()); + if (!rhs || rhs->getNumArgs() == 0) + return true; + auto rhs2 = dyn_cast<CXXOperatorCallExpr>(rhs->getArg(0)->IgnoreImplicit()); + if (!rhs2) + return true; + if (rhs2->getOperator() != OO_Plus) + return true; + auto rhsMemberExpr = dyn_cast<MemberExpr>(rhs2->getArg(0)->IgnoreImplicit()); + if (!rhsMemberExpr) + return true; + if (rhsMemberExpr->getMemberDecl() != memberExpr->getMemberDecl()) + return true; + report(DiagnosticsEngine::Warning, "rather use +=", operatorCallExpr->getBeginLoc()) + << operatorCallExpr->getSourceRange(); + } + else if (auto declRefExpr = dyn_cast<DeclRefExpr>(callExpr->getArg(0))) + { + auto varDecl = dyn_cast<VarDecl>(declRefExpr->getDecl()); + if (!varDecl) + return true; + auto tc = loplugin::TypeCheck(varDecl->getType()); + if (!tc.Class("OUString").Namespace("rtl").GlobalNamespace() + && !tc.Class("OString").Namespace("rtl").GlobalNamespace()) + return true; + auto rhs = dyn_cast<CXXConstructExpr>(callExpr->getArg(1)->IgnoreImplicit()); + if (!rhs || rhs->getNumArgs() == 0) + return true; + auto rhs2 = dyn_cast<CXXOperatorCallExpr>(rhs->getArg(0)->IgnoreImplicit()); + if (!rhs2) + return true; + if (rhs2->getOperator() != OO_Plus) + return true; + auto rhsDeclRef = dyn_cast<DeclRefExpr>(rhs2->getArg(0)->IgnoreImplicit()); + if (!rhsDeclRef) + return true; + if (rhsDeclRef->getDecl() != declRefExpr->getDecl()) + return true; + report(DiagnosticsEngine::Warning, "rather use +=", operatorCallExpr->getBeginLoc()) + << operatorCallExpr->getSourceRange(); + } + return true; +} + +loplugin::Plugin::Registration<StringPlusEqual> X("stringplusequal", true); + +} // namespace + +/* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/compilerplugins/clang/test/stringplusequal.cxx b/compilerplugins/clang/test/stringplusequal.cxx new file mode 100644 index 000000000000..6f8c4ec542bc --- /dev/null +++ b/compilerplugins/clang/test/stringplusequal.cxx @@ -0,0 +1,32 @@ +/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4; fill-column: 100 -*- */ +/* + * This file is part of the LibreOffice project. + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ + +#include "sal/config.h" +#include "rtl/string.hxx" +#include "rtl/ustring.hxx" + +#pragma clang diagnostic ignored "-Wunknown-warning-option" // for Clang < 13 +#pragma clang diagnostic ignored "-Wunused-but-set-variable" + +struct Foo +{ + OUString m_field; + void func1() + { + m_field = m_field + "xxx"; // expected-error {{rather use += [loplugin:stringplusequal]}} + } +}; + +void func2() +{ + OUString s2; + s2 = s2 + "xxx"; // expected-error {{rather use += [loplugin:stringplusequal]}} +} + +/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */ diff --git a/emfio/source/reader/emfreader.cxx b/emfio/source/reader/emfreader.cxx index 739b49ab9b40..4a73b5cb3338 100644 --- a/emfio/source/reader/emfreader.cxx +++ b/emfio/source/reader/emfreader.cxx @@ -451,7 +451,7 @@ namespace emfio break; sal_Unicode cUniChar = static_cast<sal_Unicode>(cChar); - aDesc = aDesc + OUStringChar(cUniChar); + aDesc += OUStringChar(cUniChar); } SAL_INFO("emfio", " Description: " << aDesc); @@ -2193,7 +2193,7 @@ namespace emfio mpInputStream->ReadUChar(cChar); if (cChar == 0) break; - aDesc = aDesc + OUStringChar(static_cast<sal_Unicode>(cChar)); + aDesc += OUStringChar(static_cast<sal_Unicode>(cChar)); } // if it's the standard color space name, no need to do anything if (aDesc != "COSP") diff --git a/oox/source/export/shapes.cxx b/oox/source/export/shapes.cxx index 0a2c6799fb6d..8382d9789d83 100644 --- a/oox/source/export/shapes.cxx +++ b/oox/source/export/shapes.cxx @@ -1934,7 +1934,7 @@ ShapeExport& ShapeExport::WriteConnectorShape( const Reference< XShape >& xShape lcl_Rotate(nAngle, center, aEndPoint); nAngle *= 60000; } - sGeometry = sGeometry + OUString::number(aAdjustValueList.size() + 2); + sGeometry += OUString::number(aAdjustValueList.size() + 2); } } diff --git a/sfx2/source/commandpopup/CommandPopup.cxx b/sfx2/source/commandpopup/CommandPopup.cxx index d1ff9826ae09..8a7a05fff0dc 100644 --- a/sfx2/source/commandpopup/CommandPopup.cxx +++ b/sfx2/source/commandpopup/CommandPopup.cxx @@ -86,8 +86,7 @@ void MenuContentHandler::gatherMenuContent( aNewContent.m_aMenuLabel = vcl::CommandInfoProvider::GetLabelForCommand(aCommandProperties); if (!rMenuContent.m_aFullLabelWithPath.isEmpty()) - aNewContent.m_aFullLabelWithPath - = rMenuContent.m_aFullLabelWithPath + aMenuLabelSeparator; + aNewContent.m_aFullLabelWithPath += aMenuLabelSeparator; aNewContent.m_aFullLabelWithPath += aNewContent.m_aMenuLabel; aNewContent.m_aSearchableMenuLabel = toLower(aNewContent.m_aFullLabelWithPath); diff --git a/solenv/CompilerTest_compilerplugins_clang.mk b/solenv/CompilerTest_compilerplugins_clang.mk index 75433f14bdef..540badf3ed98 100644 --- a/solenv/CompilerTest_compilerplugins_clang.mk +++ b/solenv/CompilerTest_compilerplugins_clang.mk @@ -94,6 +94,7 @@ $(eval $(call gb_CompilerTest_add_exception_objects,compilerplugins_clang, \ compilerplugins/clang/test/stringconcatliterals \ compilerplugins/clang/test/stringconstant \ compilerplugins/clang/test/stringliteralvar \ + compilerplugins/clang/test/stringplusequal \ compilerplugins/clang/test/stringstatic \ compilerplugins/clang/test/stringview \ compilerplugins/clang/test/stringviewdangle \ diff --git a/svx/source/svdraw/svdmrkv.cxx b/svx/source/svdraw/svdmrkv.cxx index 6b1be04536af..19e8b92d1a48 100644 --- a/svx/source/svdraw/svdmrkv.cxx +++ b/svx/source/svdraw/svdmrkv.cxx @@ -1228,14 +1228,12 @@ void SdrMarkView::SetMarkHandlesForLOKit(tools::Rectangle const & rRect, const S responseJSON.add_child("kinds", nodes); std::stringstream aStream; boost::property_tree::write_json(aStream, responseJSON, /*pretty=*/ false); - handleArrayStr = ", \"handles\":"_ostr; - handleArrayStr = handleArrayStr + aStream.str().c_str(); + handleArrayStr = ", \"handles\":"_ostr + aStream.str().c_str(); if (bConnectorSelection) { aStream.str(""); boost::property_tree::write_json(aStream, aGluePointsTree, /*pretty=*/ false); - handleArrayStr = handleArrayStr + ", \"GluePoints\":"; - handleArrayStr = handleArrayStr + aStream.str().c_str(); + handleArrayStr += ", \"GluePoints\":"_ostr + aStream.str().c_str(); } } diff --git a/sw/source/core/edit/acorrect.cxx b/sw/source/core/edit/acorrect.cxx index 98a0022f0979..0730062f0dd3 100644 --- a/sw/source/core/edit/acorrect.cxx +++ b/sw/source/core/edit/acorrect.cxx @@ -587,7 +587,7 @@ bool SwAutoCorrDoc::TransliterateRTLWord( sal_Int32& rSttPos, sal_Int32 nEndPos, if (pFormatter->GetPreviewString(sPrefix, 0, sConverted, &pColor, LANGUAGE_USER_HUNGARIAN_ROVAS)) { if ( bHasBracket ) - sConverted = sConverted + "]"; + sConverted += "]"; bRet = true; } } diff --git a/unoxml/source/rdf/CURI.cxx b/unoxml/source/rdf/CURI.cxx index 8554a9a9fd5a..549de5bdf884 100644 --- a/unoxml/source/rdf/CURI.cxx +++ b/unoxml/source/rdf/CURI.cxx @@ -740,7 +740,7 @@ void SAL_CALL CURI::initialize(const css::uno::Sequence< css::uno::Any > & aArgu u"CURI::initialize: argument must be string"_ustr, *this, 1); } // just append the parameters and then split them again; seems simplest - arg0 = arg0 + arg1; + arg0 += arg1; arg1.clear(); } diff --git a/vcl/jsdialog/jsdialogregister.cxx b/vcl/jsdialog/jsdialogregister.cxx index 1342d649d8b6..0a71a2252df1 100644 --- a/vcl/jsdialog/jsdialogregister.cxx +++ b/vcl/jsdialog/jsdialogregister.cxx @@ -43,7 +43,7 @@ void JSInstanceBuilder::RememberWidget(OUString sId, weld::Widget* pWidget) { unsigned long long int nIndex = nNotRepeatIndex++; // found duplicated it -> add some number to the id and apply to the widget - sId = sId + OUString::number(nIndex); + sId += OUString::number(nIndex); SalInstanceWidget* pSalWidget = dynamic_cast<SalInstanceWidget*>(pWidget); assert(pSalWidget && "can only be a SalInstanceWidget"); vcl::Window* pVclWidget = pSalWidget->getWidget();
