chart2/qa/extras/chart2dump/chart2dump.cxx | 4 +-
compilerplugins/clang/stringconstant.cxx | 43 ++++++++++++++++++++++++++
compilerplugins/clang/test/stringconstant.cxx | 13 +++++++
desktop/qa/desktop_lib/test_desktop_lib.cxx | 4 +-
sal/qa/rtl/locale/rtl_locale.cxx | 12 +++----
5 files changed, 66 insertions(+), 10 deletions(-)
New commits:
commit 5d80385fd167e5e88bb0ce959c78568d7d817842
Author: Stephan Bergmann <sberg...@redhat.com>
AuthorDate: Fri Jul 21 22:42:35 2023 +0200
Commit: Stephan Bergmann <sberg...@redhat.com>
CommitDate: Mon Jul 31 23:24:52 2023 +0200
loplugin:stringconstant: Catch some O[U]String::getStr anti-patterns
Change-Id: I36bc86fcffc3c10fe44e60d779c9aa48eeed00f2
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/154749
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sberg...@redhat.com>
diff --git a/chart2/qa/extras/chart2dump/chart2dump.cxx
b/chart2/qa/extras/chart2dump/chart2dump.cxx
index 030a536f320a..339a69bb80bd 100644
--- a/chart2/qa/extras/chart2dump/chart2dump.cxx
+++ b/chart2/qa/extras/chart2dump/chart2dump.cxx
@@ -1100,7 +1100,7 @@ DECLARE_DUMP_TEST( PivotChartDataButtonTest,
Chart2DumpTest, false )
{
return
xShapeNode->getShapeType() == "com.sun.star.drawing.TextShape";
} );
- CPPUNIT_ASSERT_MESSAGE( OString( "Cannot find Data button shape"
).getStr(), xButton.is() );
+ CPPUNIT_ASSERT_MESSAGE( "Cannot find Data button shape", xButton.is() );
// Make sure that there is no arrow shape with the Data button
uno::Reference<drawing::XShape> xArrow = getShapeByName( xShapes,
"FieldButton.Row.8",
@@ -1108,7 +1108,7 @@ DECLARE_DUMP_TEST( PivotChartDataButtonTest,
Chart2DumpTest, false )
{
return
xShapeNode->getShapeType() == "com.sun.star.drawing.PolyPolygonShape";
} );
- CPPUNIT_ASSERT_MESSAGE( OString( "Arrow shape should not be present for
the Data button" ).getStr(), !xArrow.is() );
+ CPPUNIT_ASSERT_MESSAGE( "Arrow shape should not be present for the Data
button", !xArrow.is() );
// Assert the background color of the Data button
util::Color aButtonFillColor = 0;
diff --git a/compilerplugins/clang/stringconstant.cxx
b/compilerplugins/clang/stringconstant.cxx
index cf1eb6afedbf..c64c2c9d6589 100644
--- a/compilerplugins/clang/stringconstant.cxx
+++ b/compilerplugins/clang/stringconstant.cxx
@@ -191,6 +191,8 @@ public:
bool VisitCallExpr(CallExpr const * expr);
+ bool VisitCXXMemberCallExpr(CXXMemberCallExpr const * expr);
+
bool VisitCXXConstructExpr(CXXConstructExpr const * expr);
bool VisitReturnStmt(ReturnStmt const * stmt);
@@ -856,6 +858,47 @@ bool StringConstant::VisitCallExpr(CallExpr const * expr) {
return true;
}
+bool StringConstant::VisitCXXMemberCallExpr(CXXMemberCallExpr const * expr) {
+ if (ignoreLocation(expr)) {
+ return true;
+ }
+ FunctionDecl const * fdecl = expr->getDirectCallee();
+ if (fdecl == nullptr) {
+ return true;
+ }
+ auto const c = loplugin::DeclCheck(fdecl).Function("getStr");
+ if ((c.Class("OString").Namespace("rtl").GlobalNamespace()
+ || c.Class("OUString").Namespace("rtl").GlobalNamespace())
+ && fdecl->getNumParams() == 0)
+ {
+ auto const e1 =
expr->getImplicitObjectArgument()->IgnoreImplicit()->IgnoreParens();
+ if (auto const e2 = dyn_cast<CXXTemporaryObjectExpr>(e1)) {
+ if (e2->getNumArgs() != 0) {
+ return true;
+ }
+ report(
+ DiagnosticsEngine::Warning,
+ "in call of '%0', replace default-constructed %1 directly with
an empty %select{ordinary|UTF-16}2 string literal",
+ expr->getExprLoc())
+ << fdecl->getQualifiedNameAsString() << e2->getType() <<
bool(loplugin::TypeCheck(e2->getType()).Class("OUString")) <<
expr->getSourceRange();
+ return true;
+ }
+ if (auto const e2 = dyn_cast<CXXFunctionalCastExpr>(e1)) {
+ auto const e3 =
dyn_cast<clang::StringLiteral>(e2->getSubExprAsWritten()->IgnoreParens());
+ if (e3 == nullptr) {
+ return true;
+ }
+ report(
+ DiagnosticsEngine::Warning,
+ "in call of '%0', replace %1 constructed from a string literal
directly with %select{the|a UTF-16}2 string literal",
+ expr->getExprLoc())
+ << fdecl->getQualifiedNameAsString() << e2->getType() <<
(loplugin::TypeCheck(e2->getType()).Class("OUString") && !e3->isUTF16()) <<
expr->getSourceRange();
+ return true;
+ }
+ }
+ return true;
+}
+
bool StringConstant::VisitCXXConstructExpr(CXXConstructExpr const * expr) {
if (ignoreLocation(expr)) {
return true;
diff --git a/compilerplugins/clang/test/stringconstant.cxx
b/compilerplugins/clang/test/stringconstant.cxx
index 02f83b531068..ada901b877ce 100644
--- a/compilerplugins/clang/test/stringconstant.cxx
+++ b/compilerplugins/clang/test/stringconstant.cxx
@@ -128,6 +128,19 @@ int main() {
ub.append(static_cast<char const *>(sc2)); // at runtime: append "foo"
ub.append(static_cast<char const *>(sc3)); // at runtime: assert
ub.append(static_cast<char const *>(sc4)); // at runtime: UB
+
+ // expected-error-re@+1 {{in call of 'rtl::OString::getStr', replace
default-constructed '{{(rtl::)?}}OString' directly with an empty ordinary
string literal}}
+ OString().getStr();
+ // expected-error-re@+1 {{in call of 'rtl::OString::getStr', replace
'{{(rtl::)?}}OString' constructed from a string literal directly with the
string literal}}
+ OString("foo").getStr();
+ // expected-error-re@+1 {{in call of 'rtl::OString::getStr', replace
'{{(rtl::)?}}OString' constructed from a string literal directly with the
string literal}}
+ (OString(("foo"))).getStr();
+ // expected-error-re@+1 {{in call of 'rtl::OUString::getStr', replace
default-constructed '{{(rtl::)?}}OUString' directly with an empty UTF-16 string
literal}}
+ OUString().getStr();
+ // expected-error-re@+1 {{in call of 'rtl::OUString::getStr', replace
'{{(rtl::)?}}OUString' constructed from a string literal directly with a UTF-16
string literal}}
+ OUString("foo").getStr();
+ // expected-error-re@+1 {{in call of 'rtl::OUString::getStr', replace
'{{(rtl::)?}}OUString' constructed from a string literal directly with the
string literal}}
+ OUString(u"foo").getStr();
}
diff --git a/desktop/qa/desktop_lib/test_desktop_lib.cxx
b/desktop/qa/desktop_lib/test_desktop_lib.cxx
index b698cc5976b7..e93cf695068f 100644
--- a/desktop/qa/desktop_lib/test_desktop_lib.cxx
+++ b/desktop/qa/desktop_lib/test_desktop_lib.cxx
@@ -2638,10 +2638,10 @@ void DesktopLOKTest::testRunMacro()
bool bGoodMacro, bNonExistentMacro;
// Tools macros come pre-installed in system share/basic folder,
- bGoodMacro = aOffice.m_pOfficeClass->runMacro(&aOffice,
OString("macro:///Tools.Debug.ActivateReadOnlyFlag()").getStr());
+ bGoodMacro = aOffice.m_pOfficeClass->runMacro(&aOffice,
"macro:///Tools.Debug.ActivateReadOnlyFlag()");
CPPUNIT_ASSERT(bGoodMacro);
- bNonExistentMacro = aOffice.m_pOfficeClass->runMacro(&aOffice,
OString("macro:///I.Am.Not(There)").getStr());
+ bNonExistentMacro = aOffice.m_pOfficeClass->runMacro(&aOffice,
"macro:///I.Am.Not(There)");
CPPUNIT_ASSERT(!bNonExistentMacro);
}
diff --git a/sal/qa/rtl/locale/rtl_locale.cxx b/sal/qa/rtl/locale/rtl_locale.cxx
index 27e30db9f9d4..f1b4e12ee4b7 100644
--- a/sal/qa/rtl/locale/rtl_locale.cxx
+++ b/sal/qa/rtl/locale/rtl_locale.cxx
@@ -30,7 +30,7 @@ namespace rtl_locale
// default locale for test purpose
static void setDefaultLocale()
{
- rtl_locale_setDefault(OUString("de").getStr(),
OUString("DE").getStr(), /* OUString() */ OUString("hochdeutsch").getStr() );
+ rtl_locale_setDefault(u"de", u"DE", /* OUString() */ u"hochdeutsch" );
}
class getDefault : public CppUnit::TestFixture
@@ -76,7 +76,7 @@ public:
// insert your test code here.
void setDefault_001()
{
- rtl_locale_setDefault(OUString("en").getStr(),
OUString("US").getStr(), OUString().getStr());
+ rtl_locale_setDefault(u"en", u"US", u"");
rtl_Locale* pData = rtl_locale_getDefault();
CPPUNIT_ASSERT_MESSAGE("locale must not null", pData != nullptr);
@@ -241,8 +241,8 @@ public:
// insert your test code here.
void equals_001()
{
- rtl_Locale* pData1 = rtl_locale_register(OUString("en").getStr(),
OUString("US").getStr(), OUString().getStr());
- rtl_Locale* pData2 = rtl_locale_register(OUString("en").getStr(),
OUString("US").getStr(), OUString().getStr());
+ rtl_Locale* pData1 = rtl_locale_register(u"en", u"US", u"");
+ rtl_Locale* pData2 = rtl_locale_register(u"en", u"US", u"");
bool bLocaleAreEqual = (pData1 == pData2);
@@ -251,8 +251,8 @@ public:
void equals_002()
{
- rtl_Locale* pData1 = rtl_locale_register(OUString("en").getStr(),
OUString("US").getStr(), OUString().getStr());
- rtl_Locale* pData2 = rtl_locale_register(OUString("en").getStr(),
OUString("US").getStr(), OUString().getStr());
+ rtl_Locale* pData1 = rtl_locale_register(u"en", u"US", u"");
+ rtl_Locale* pData2 = rtl_locale_register(u"en", u"US", u"");
sal_Int32 nEqual = rtl_locale_equals(pData1, pData2);
CPPUNIT_ASSERT(nEqual != 0);
