diff --git a/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.h b/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.h --- a/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.h +++ b/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.h @@ -16,13 +16,15 @@ /// 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 StringParameterFunctions; }; } // namespace clang::tidy::readability diff --git a/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp --- 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 @@ } // 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 @@ 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/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -149,7 +149,8 @@ - Improved :doc:`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 --- 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 @@ 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 --- /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 + +namespace fmt { + inline namespace v8 { + template + void print(const char *, Args &&...); + template + std::string format(const char *, Args &&...); + } +} + +namespace notfmt { + inline namespace v8 { + template + void print(const char *, Args &&...); + template + 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 + void operator()(const char *fmt, Args &&...args) { + } + + template + 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 + void operator()(const char *fmt, Args &&...args) { + } + + template + 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()); +}