[clang-tools-extra] 4e901cd - [clang-tidy] Support readability-redundant-string-cstr.StringParameterFunctions option
Piotr Zegar via cfe-commits
cfe-commits at lists.llvm.org
Mon Mar 13 11:03:46 PDT 2023
Author: Mike Crowe
Date: 2023-03-13T18:03:09Z
New Revision: 4e901cda725b5adbdfeb495607fda9a2f84128c8
URL: https://github.com/llvm/llvm-project/commit/4e901cda725b5adbdfeb495607fda9a2f84128c8
DIFF: https://github.com/llvm/llvm-project/commit/4e901cda725b5adbdfeb495607fda9a2f84128c8.diff
LOG: [clang-tidy] Support readability-redundant-string-cstr.StringParameterFunctions option
Add StringParameterFunctions option to allow the
readability-redundant-string-cstr check to work with library functions
such as fmt::format and spdlog::logger:info that are able to support
std::string arguments in addition to const char * ones.
Depends on D143342
Reviewed By: PiotrZSL
Differential Revision: https://reviews.llvm.org/D145885
Added:
clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr-function.cpp
Modified:
clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/readability/redundant-string-cstr.rst
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
index d1fdf8341f710..dfcc82c270540 100644
--- a/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
@@ -11,6 +11,8 @@
//===----------------------------------------------------------------------===//
#include "RedundantStringCStrCheck.h"
+#include "../utils/Matchers.h"
+#include "../utils/OptionsUtils.h"
#include "clang/Lex/Lexer.h"
#include "clang/Tooling/FixIt.h"
@@ -69,6 +71,17 @@ AST_MATCHER(MaterializeTemporaryExpr, isBoundToLValue) {
} // end namespace
+RedundantStringCStrCheck::RedundantStringCStrCheck(StringRef Name,
+ ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ StringParameterFunctions(utils::options::parseStringList(
+ Options.get("StringParameterFunctions", ""))) {
+ if (getLangOpts().CPlusPlus20)
+ StringParameterFunctions.push_back("::std::format");
+ if (getLangOpts().CPlusPlus2b)
+ StringParameterFunctions.push_back("::std::print");
+}
+
void RedundantStringCStrCheck::registerMatchers(
ast_matchers::MatchFinder *Finder) {
// Match expressions of type 'string' or 'string*'.
@@ -183,15 +196,13 @@ void RedundantStringCStrCheck::registerMatchers(
hasArgument(0, StringCStrCallExpr))),
this);
- if (getLangOpts().CPlusPlus20) {
+ if (!StringParameterFunctions.empty()) {
// Detect redundant 'c_str()' calls in parameters passed to std::format in
// C++20 onwards and std::print in C++23 onwards.
Finder->addMatcher(
traverse(TK_AsIs,
- callExpr(callee(functionDecl(
- getLangOpts().CPlusPlus2b
- ? hasAnyName("::std::print", "::std::format")
- : hasName("::std::format"))),
+ callExpr(callee(functionDecl(matchers::matchesAnyListedName(
+ StringParameterFunctions))),
forEachArgumentWithParam(StringCStrCallExpr,
parmVarDecl()))),
this);
diff --git a/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.h b/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.h
index 662fb955fc653..e2e6ab1fd939c 100644
--- a/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.h
+++ b/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.h
@@ -16,13 +16,15 @@ namespace clang::tidy::readability {
/// Finds unnecessary calls to `std::string::c_str()`.
class RedundantStringCStrCheck : public ClangTidyCheck {
public:
- RedundantStringCStrCheck(StringRef Name, ClangTidyContext *Context)
- : ClangTidyCheck(Name, Context) {}
+ RedundantStringCStrCheck(StringRef Name, ClangTidyContext *Context);
bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
return LangOpts.CPlusPlus;
}
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+ std::vector<StringRef> StringParameterFunctions;
};
} // namespace clang::tidy::readability
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 6d48ff7bf7eb1..2be8bfc51d675 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -149,7 +149,8 @@ Changes in existing checks
- Improved :doc:`readability-redundant-string-cstr
<clang-tidy/checks/readability/redundant-string-cstr>` check to recognise
unnecessary ``std::string::c_str()`` and ``std::string::data()`` calls in
- arguments to ``std::print`` and ``std::format``.
+ arguments to ``std::print``, ``std::format`` or other functions listed in
+ the ``StringParameterFunction`` check option.
- Deprecated check-local options `HeaderFileExtensions`
in :doc:`bugprone-dynamic-static-initializers
diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/redundant-string-cstr.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/redundant-string-cstr.rst
index e6760a41ca33c..2789f9c096ccf 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/readability/redundant-string-cstr.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/readability/redundant-string-cstr.rst
@@ -5,3 +5,16 @@ readability-redundant-string-cstr
Finds unnecessary calls to ``std::string::c_str()`` and ``std::string::data()``.
+
+Options
+-------
+
+.. option:: StringParameterFunctions
+
+ A semicolon-separated list of (fully qualified) function/method/operator
+ names, with the requirement that any parameter currently accepting a
+ ``const char*`` input should also be able to accept ``std::string``
+ inputs, or proper overload candidates that can do so should exist. This
+ can be used to configure functions such as ``fmt::format``,
+ ``spdlog::logger::info``, or wrappers around these and similar
+ functions. The default value is the empty string.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr-function.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr-function.cpp
new file mode 100644
index 0000000000000..04e035cdbb626
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr-function.cpp
@@ -0,0 +1,148 @@
+// RUN: %check_clang_tidy %s readability-redundant-string-cstr %t -- \
+// RUN: -config="{CheckOptions: \
+// RUN: [{key: readability-redundant-string-cstr.StringParameterFunctions, \
+// RUN: value: '::fmt::format; ::fmt::print; ::BaseLogger::operator(); ::BaseLogger::Log'}] \
+// RUN: }" \
+// RUN: -- -isystem %clang_tidy_headers
+#include <string>
+
+namespace fmt {
+ inline namespace v8 {
+ template<typename ...Args>
+ void print(const char *, Args &&...);
+ template<typename ...Args>
+ std::string format(const char *, Args &&...);
+ }
+}
+
+namespace notfmt {
+ inline namespace v8 {
+ template<typename ...Args>
+ void print(const char *, Args &&...);
+ template<typename ...Args>
+ std::string format(const char *, Args &&...);
+ }
+}
+
+void fmt_print(const std::string &s1, const std::string &s2, const std::string &s3) {
+ fmt::print("One:{}\n", s1.c_str());
+ // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+ // CHECK-FIXES: {{^ }}fmt::print("One:{}\n", s1);
+
+ fmt::print("One:{} Two:{} Three:{}\n", s1.c_str(), s2, s3.c_str());
+ // CHECK-MESSAGES: :[[@LINE-1]]:42: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+ // CHECK-MESSAGES: :[[@LINE-2]]:58: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+ // CHECK-FIXES: {{^ }}fmt::print("One:{} Two:{} Three:{}\n", s1, s2, s3);
+}
+
+// There's no c_str() call here, so it shouldn't be touched
+void fmt_print_no_cstr(const std::string &s1, const std::string &s2) {
+ fmt::print("One: {}, Two: {}\n", s1, s2);
+}
+
+// This isn't fmt::print, so it shouldn't be fixed.
+void not_fmt_print(const std::string &s1) {
+ notfmt::print("One: {}\n", s1.c_str());
+}
+
+void fmt_format(const std::string &s1, const std::string &s2, const std::string &s3) {
+ auto r1 = fmt::format("One:{}\n", s1.c_str());
+ // CHECK-MESSAGES: :[[@LINE-1]]:37: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+ // CHECK-FIXES: {{^ }}auto r1 = fmt::format("One:{}\n", s1);
+
+ auto r2 = fmt::format("One:{} Two:{} Three:{}\n", s1.c_str(), s2, s3.c_str());
+ // CHECK-MESSAGES: :[[@LINE-1]]:53: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+ // CHECK-MESSAGES: :[[@LINE-2]]:69: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+ // CHECK-FIXES: {{^ }}auto r2 = fmt::format("One:{} Two:{} Three:{}\n", s1, s2, s3);
+}
+
+// There's are c_str() calls here, so it shouldn't be touched
+void fmt_format_no_cstr(const std::string &s1, const std::string &s2) {
+ fmt::format("One: {}, Two: {}\n", s1, s2);
+}
+
+// This is not fmt::format, so it shouldn't be fixed
+std::string not_fmt_format(const std::string &s1) {
+ return notfmt::format("One: {}\n", s1.c_str());
+}
+
+class BaseLogger {
+public:
+ template <typename... Args>
+ void operator()(const char *fmt, Args &&...args) {
+ }
+
+ template <typename... Args>
+ void Log(const char *fmt, Args &&...args) {
+ }
+};
+
+class DerivedLogger : public BaseLogger {};
+class DoubleDerivedLogger : public DerivedLogger {};
+typedef DerivedLogger TypedefDerivedLogger;
+
+void logger1(const std::string &s1, const std::string &s2, const std::string &s3) {
+ BaseLogger LOGGER;
+
+ LOGGER("%s\n", s1.c_str(), s2, s3.c_str());
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+ // CHECK-MESSAGES: :[[@LINE-2]]:34: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+ // CHECK-FIXES: {{^ }}LOGGER("%s\n", s1, s2, s3);
+
+ DerivedLogger LOGGER2;
+ LOGGER2("%d %s\n", 42, s1.c_str(), s2.c_str(), s3);
+ // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+ // CHECK-MESSAGES: :[[@LINE-2]]:38: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+ // CHECK-FIXES: {{^ }}LOGGER2("%d %s\n", 42, s1, s2, s3);
+
+ DoubleDerivedLogger LOGGERD;
+ LOGGERD("%d %s\n", 42, s1.c_str(), s2, s3.c_str());
+ // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+ // CHECK-MESSAGES: :[[@LINE-2]]:42: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+ // CHECK-FIXES: {{^ }}LOGGERD("%d %s\n", 42, s1, s2, s3);
+
+ TypedefDerivedLogger LOGGERT;
+ LOGGERT("%d %s\n", 42, s1.c_str(), s2, s3.c_str());
+ // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+ // CHECK-MESSAGES: :[[@LINE-2]]:42: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+ // CHECK-FIXES: {{^ }}LOGGERT("%d %s\n", 42, s1, s2, s3);
+}
+
+void logger2(const std::string &s1, const std::string &s2) {
+ BaseLogger LOGGER3;
+
+ LOGGER3.Log("%s\n", s1.c_str(), s2.c_str());
+ // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+ // CHECK-MESSAGES: :[[@LINE-2]]:35: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+ // CHECK-FIXES: {{^ }}LOGGER3.Log("%s\n", s1, s2);
+
+ DerivedLogger LOGGER4;
+ LOGGER4.Log("%d %s\n", 42, s1.c_str(), s2.c_str());
+ // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+ // CHECK-MESSAGES: :[[@LINE-2]]:42: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+ // CHECK-FIXES: {{^ }}LOGGER4.Log("%d %s\n", 42, s1, s2);
+}
+
+class NotLogger {
+public:
+ template <typename... Args>
+ void operator()(const char *fmt, Args &&...args) {
+ }
+
+ template <typename... Args>
+ void Log(const char *fmt, Args &&...args) {
+ }
+};
+
+void Log(const char *fmt, ...);
+
+void logger3(const std::string &s1)
+{
+ // Not BaseLogger or something derived from it
+ NotLogger LOGGER;
+ LOGGER("%s\n", s1.c_str());
+ LOGGER.Log("%s\n", s1.c_str());
+
+ // Free function not in StringParameterFunctions list
+ Log("%s\n", s1.c_str());
+}
More information about the cfe-commits
mailing list