Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[clang-tidy] Add modernize-use-std-format check #90397

Merged
merged 16 commits into from May 13, 2024

Conversation

mikecrowe
Copy link
Contributor

Add a new clang-tidy check that converts absl::StrFormat (and similar functions) to std::format (and similar functions.)

Split the configuration of FormatStringConverter out to a separate Configuration class so that we don't risk confusion by passing two boolean configuration parameters into the constructor. Add AllowTrailingNewlineRemoval option since we never want to remove trailing newlines in this check.

Add a new clang-tidy check that converts absl::StrFormat (and similar
functions) to std::format (and similar functions.)

Split the configuration of FormatStringConverter out to a separate
Configuration class so that we don't risk confusion by passing two
boolean configuration parameters into the constructor. Add
AllowTrailingNewlineRemoval option since we never want to remove
trailing newlines in this check.

Differential Revision: https://reviews.llvm.org/D154287
@mikecrowe
Copy link
Contributor Author

This check was originally proposed and reviewed at https://reviews.llvm.org/D154287 last year but never landed since I did not (and still do not) have commit access. Near the end of that review, @PiotrZSL wrote:

To be honest, I do not see to much use case for this check. But that's fine.

which may mean that it should not land. I've created this pull request for discoverability by those looking for such a check since Phabricator is no longer used.

@llvmbot
Copy link
Collaborator

llvmbot commented Apr 28, 2024

@llvm/pr-subscribers-clang-tools-extra

Author: Mike Crowe (mikecrowe)

Changes

Add a new clang-tidy check that converts absl::StrFormat (and similar functions) to std::format (and similar functions.)

Split the configuration of FormatStringConverter out to a separate Configuration class so that we don't risk confusion by passing two boolean configuration parameters into the constructor. Add AllowTrailingNewlineRemoval option since we never want to remove trailing newlines in this check.


Patch is 28.81 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/90397.diff

13 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/modernize/CMakeLists.txt (+1)
  • (modified) clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp (+2)
  • (added) clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.cpp (+108)
  • (added) clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.h (+50)
  • (modified) clang-tools-extra/clang-tidy/modernize/UseStdPrintCheck.cpp (+4-1)
  • (modified) clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp (+6-4)
  • (modified) clang-tools-extra/clang-tidy/utils/FormatStringConverter.h (+8-1)
  • (modified) clang-tools-extra/docs/ReleaseNotes.rst (+9)
  • (modified) clang-tools-extra/docs/clang-tidy/checks/list.rst (+1)
  • (added) clang-tools-extra/docs/clang-tidy/checks/modernize/use-std-format.rst (+84)
  • (added) clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format-custom.cpp (+76)
  • (added) clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format-fmt.cpp (+37)
  • (added) clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format.cpp (+97)
diff --git a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
index 8005d6e91c060c..576805c4c7f181 100644
--- a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
@@ -41,6 +41,7 @@ add_clang_library(clangTidyModernizeModule
   UseNullptrCheck.cpp
   UseOverrideCheck.cpp
   UseStartsEndsWithCheck.cpp
+  UseStdFormatCheck.cpp
   UseStdNumbersCheck.cpp
   UseStdPrintCheck.cpp
   UseTrailingReturnTypeCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
index 776558433c5baa..b9c7a2dc383e88 100644
--- a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
@@ -42,6 +42,7 @@
 #include "UseNullptrCheck.h"
 #include "UseOverrideCheck.h"
 #include "UseStartsEndsWithCheck.h"
+#include "UseStdFormatCheck.h"
 #include "UseStdNumbersCheck.h"
 #include "UseStdPrintCheck.h"
 #include "UseTrailingReturnTypeCheck.h"
@@ -76,6 +77,7 @@ class ModernizeModule : public ClangTidyModule {
         "modernize-use-designated-initializers");
     CheckFactories.registerCheck<UseStartsEndsWithCheck>(
         "modernize-use-starts-ends-with");
+    CheckFactories.registerCheck<UseStdFormatCheck>("modernize-use-std-format");
     CheckFactories.registerCheck<UseStdNumbersCheck>(
         "modernize-use-std-numbers");
     CheckFactories.registerCheck<UseStdPrintCheck>("modernize-use-std-print");
diff --git a/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.cpp
new file mode 100644
index 00000000000000..d22ebe857cf415
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.cpp
@@ -0,0 +1,108 @@
+//===--- UseStdFormatCheck.cpp - clang-tidy -------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "UseStdFormatCheck.h"
+#include "../utils/FormatStringConverter.h"
+#include "../utils/Matchers.h"
+#include "../utils/OptionsUtils.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+#include "clang/Tooling/FixIt.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::modernize {
+
+namespace {
+AST_MATCHER(StringLiteral, isOrdinary) { return Node.isOrdinary(); }
+} // namespace
+
+UseStdFormatCheck::UseStdFormatCheck(StringRef Name, ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      StrictMode(Options.getLocalOrGlobal("StrictMode", false)),
+      StrFormatLikeFunctions(utils::options::parseStringList(
+          Options.get("StrFormatLikeFunctions", ""))),
+      ReplacementFormatFunction(
+          Options.get("ReplacementFormatFunction", "std::format")),
+      IncludeInserter(Options.getLocalOrGlobal("IncludeStyle",
+                                               utils::IncludeSorter::IS_LLVM),
+                      areDiagsSelfContained()),
+      MaybeHeaderToInclude(Options.get("FormatHeader")) {
+  if (StrFormatLikeFunctions.empty())
+    StrFormatLikeFunctions.push_back("absl::StrFormat");
+
+  if (!MaybeHeaderToInclude && ReplacementFormatFunction == "std::format")
+    MaybeHeaderToInclude = "<format>";
+}
+
+void UseStdFormatCheck::registerPPCallbacks(const SourceManager &SM,
+                                            Preprocessor *PP,
+                                            Preprocessor *ModuleExpanderPP) {
+  IncludeInserter.registerPreprocessor(PP);
+}
+
+void UseStdFormatCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+      callExpr(argumentCountAtLeast(1),
+               hasArgument(0, stringLiteral(isOrdinary())),
+               callee(functionDecl(unless(cxxMethodDecl()),
+                                   matchers::matchesAnyListedName(
+                                       StrFormatLikeFunctions))
+                          .bind("func_decl")))
+          .bind("strformat"),
+      this);
+}
+
+void UseStdFormatCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  using utils::options::serializeStringList;
+  Options.store(Opts, "StrictMode", StrictMode);
+  Options.store(Opts, "StrFormatLikeFunctions",
+                serializeStringList(StrFormatLikeFunctions));
+  Options.store(Opts, "ReplacementFormatFunction", ReplacementFormatFunction);
+  Options.store(Opts, "IncludeStyle", IncludeInserter.getStyle());
+  if (MaybeHeaderToInclude)
+    Options.store(Opts, "FormatHeader", *MaybeHeaderToInclude);
+}
+
+void UseStdFormatCheck::check(const MatchFinder::MatchResult &Result) {
+  const unsigned FormatArgOffset = 0;
+  const auto *OldFunction = Result.Nodes.getNodeAs<FunctionDecl>("func_decl");
+  const auto *StrFormat = Result.Nodes.getNodeAs<CallExpr>("strformat");
+
+  utils::FormatStringConverter::Configuration ConverterConfig;
+  ConverterConfig.StrictMode = StrictMode;
+  utils::FormatStringConverter Converter(Result.Context, StrFormat,
+                                         FormatArgOffset, ConverterConfig,
+                                         getLangOpts());
+  const Expr *StrFormatCall = StrFormat->getCallee();
+  if (!Converter.canApply()) {
+    DiagnosticBuilder Diag = diag(StrFormat->getBeginLoc(),
+                                  "unable to use '%0' instead of %1 because %2")
+                             << ReplacementFormatFunction
+                             << OldFunction->getIdentifier()
+                             << Converter.conversionNotPossibleReason();
+    return;
+  }
+
+  DiagnosticBuilder Diag =
+      diag(StrFormatCall->getBeginLoc(), "use '%0' instead of %1")
+      << ReplacementFormatFunction << OldFunction->getIdentifier();
+  Diag << FixItHint::CreateReplacement(
+      CharSourceRange::getTokenRange(StrFormatCall->getBeginLoc(),
+                                     StrFormatCall->getEndLoc()),
+      ReplacementFormatFunction);
+  Converter.applyFixes(Diag, *Result.SourceManager);
+
+  if (MaybeHeaderToInclude)
+    Diag << IncludeInserter.createIncludeInsertion(
+        Result.Context->getSourceManager().getFileID(
+            StrFormatCall->getBeginLoc()),
+        *MaybeHeaderToInclude);
+}
+
+} // namespace clang::tidy::modernize
diff --git a/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.h b/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.h
new file mode 100644
index 00000000000000..d2aeb1bd2802c2
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.h
@@ -0,0 +1,50 @@
+//===--- UseStdFormatCheck.h - clang-tidy -----------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USESTDFORMATCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USESTDFORMATCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+#include "../ClangTidyCheck.h"
+#include "../utils/IncludeInserter.h"
+
+namespace clang::tidy::modernize {
+
+/// Convert calls to absl::StrFormat-like functions to std::format.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/modernize/use-std-format.html
+class UseStdFormatCheck : public ClangTidyCheck {
+public:
+  UseStdFormatCheck(StringRef Name, ClangTidyContext *Context);
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+    if (ReplacementFormatFunction == "std::format")
+      return LangOpts.CPlusPlus20;
+    return LangOpts.CPlusPlus;
+  }
+  void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
+                           Preprocessor *ModuleExpanderPP) override;
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  std::optional<TraversalKind> getCheckTraversalKind() const override {
+    return TK_IgnoreUnlessSpelledInSource;
+  }
+
+private:
+  bool StrictMode;
+  std::vector<StringRef> StrFormatLikeFunctions;
+  StringRef ReplacementFormatFunction;
+  utils::IncludeInserter IncludeInserter;
+  std::optional<StringRef> MaybeHeaderToInclude;
+};
+
+} // namespace clang::tidy::modernize
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USESTDFORMATCHECK_H
diff --git a/clang-tools-extra/clang-tidy/modernize/UseStdPrintCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseStdPrintCheck.cpp
index 660996aba7b70d..a4d5fdb852d559 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseStdPrintCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseStdPrintCheck.cpp
@@ -129,8 +129,11 @@ void UseStdPrintCheck::check(const MatchFinder::MatchResult &Result) {
     FormatArgOffset = 1;
   }
 
+  utils::FormatStringConverter::Configuration ConverterConfig;
+  ConverterConfig.StrictMode = StrictMode;
+  ConverterConfig.AllowTrailingNewlineRemoval = true;
   utils::FormatStringConverter Converter(
-      Result.Context, Printf, FormatArgOffset, StrictMode, getLangOpts());
+      Result.Context, Printf, FormatArgOffset, ConverterConfig, getLangOpts());
   const Expr *PrintfCall = Printf->getCallee();
   const StringRef ReplacementFunction = Converter.usePrintNewlineFunction()
                                             ? ReplacementPrintlnFunction
diff --git a/clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp b/clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp
index ad10f745b6acfb..301300eafb5581 100644
--- a/clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp
+++ b/clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp
@@ -198,10 +198,11 @@ static bool castMismatchedIntegerTypes(const CallExpr *Call, bool StrictMode) {
 FormatStringConverter::FormatStringConverter(ASTContext *ContextIn,
                                              const CallExpr *Call,
                                              unsigned FormatArgOffset,
-                                             bool StrictMode,
+                                             const Configuration ConfigIn,
                                              const LangOptions &LO)
-    : Context(ContextIn),
-      CastMismatchedIntegerTypes(castMismatchedIntegerTypes(Call, StrictMode)),
+    : Context(ContextIn), Config(ConfigIn),
+      CastMismatchedIntegerTypes(
+          castMismatchedIntegerTypes(Call, ConfigIn.StrictMode)),
       Args(Call->getArgs()), NumArgs(Call->getNumArgs()),
       ArgsOffset(FormatArgOffset + 1), LangOpts(LO) {
   assert(ArgsOffset <= NumArgs);
@@ -627,7 +628,8 @@ void FormatStringConverter::finalizeFormatText() {
 
   // It's clearer to convert printf("Hello\r\n"); to std::print("Hello\r\n")
   // than to std::println("Hello\r");
-  if (StringRef(StandardFormatString).ends_with("\\n") &&
+  if (Config.AllowTrailingNewlineRemoval &&
+      StringRef(StandardFormatString).ends_with("\\n") &&
       !StringRef(StandardFormatString).ends_with("\\\\n") &&
       !StringRef(StandardFormatString).ends_with("\\r\\n")) {
     UsePrintNewlineFunction = true;
diff --git a/clang-tools-extra/clang-tidy/utils/FormatStringConverter.h b/clang-tools-extra/clang-tidy/utils/FormatStringConverter.h
index 1949870f62ed68..1109a0b602262f 100644
--- a/clang-tools-extra/clang-tidy/utils/FormatStringConverter.h
+++ b/clang-tools-extra/clang-tidy/utils/FormatStringConverter.h
@@ -32,8 +32,14 @@ class FormatStringConverter
 public:
   using ConversionSpecifier = clang::analyze_format_string::ConversionSpecifier;
   using PrintfSpecifier = analyze_printf::PrintfSpecifier;
+
+  struct Configuration {
+    bool StrictMode = false;
+    bool AllowTrailingNewlineRemoval = false;
+  };
+
   FormatStringConverter(ASTContext *Context, const CallExpr *Call,
-                        unsigned FormatArgOffset, bool StrictMode,
+                        unsigned FormatArgOffset, Configuration Config,
                         const LangOptions &LO);
 
   bool canApply() const { return ConversionNotPossibleReason.empty(); }
@@ -45,6 +51,7 @@ class FormatStringConverter
 
 private:
   ASTContext *Context;
+  const Configuration Config;
   const bool CastMismatchedIntegerTypes;
   const Expr *const *Args;
   const unsigned NumArgs;
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 3038d2b125f20d..c166801dfd6a60 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -145,6 +145,15 @@ New checks
   Finds initializer lists for aggregate types that could be
   written as designated initializers instead.
 
+- New :doc:`modernize-use-std-format
+  <clang-tidy/checks/modernize/use-std-format>` check.
+
+  Converts calls to ``absl::StrFormat``, or other functions via
+  configuration options, to C++20's ``std::format``, or another function
+  via a configuration option, modifying the format string appropriately and
+  removing now-unnecessary calls to ``std::string::c_str()`` and
+  ``std::string::data()``.
+
 - New :doc:`readability-enum-initial-value
   <clang-tidy/checks/readability/enum-initial-value>` check.
 
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 49747ff896ba5c..50cba5b1d21153 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -300,6 +300,7 @@ Clang-Tidy Checks
    :doc:`modernize-use-nullptr <modernize/use-nullptr>`, "Yes"
    :doc:`modernize-use-override <modernize/use-override>`, "Yes"
    :doc:`modernize-use-starts-ends-with <modernize/use-starts-ends-with>`, "Yes"
+   :doc:`modernize-use-std-format <modernize/use-std-format>`, "Yes"
    :doc:`modernize-use-std-numbers <modernize/use-std-numbers>`, "Yes"
    :doc:`modernize-use-std-print <modernize/use-std-print>`, "Yes"
    :doc:`modernize-use-trailing-return-type <modernize/use-trailing-return-type>`, "Yes"
diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-std-format.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-std-format.rst
new file mode 100644
index 00000000000000..60dc8ea5e3c46e
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-std-format.rst
@@ -0,0 +1,84 @@
+.. title:: clang-tidy - modernize-use-std-format
+
+modernize-use-std-format
+========================
+
+Converts calls to ``absl::StrFormat``, or other functions via
+configuration options, to C++20's ``std::format``, or another function
+via a configuration option, modifying the format string appropriately and
+removing now-unnecessary calls to ``std::string::c_str()`` and
+``std::string::data()``.
+
+In other words, it turns lines like:
+
+.. code-block:: c++
+
+  return absl::StrFormat("The %s is %3d\n", description.c_str(), value);
+
+into:
+
+.. code-block:: c++
+
+  return std::format("The {} is {:3}", description, value);
+
+The check uses the same format-string-conversion algorithm as
+`modernize-use-std-print <../modernize/use-std-print.html>`_ and its
+shortcomings are described in the documentation for that check.
+
+Options
+-------
+
+.. option:: StrictMode
+
+   When `true`, the check will add casts when converting from variadic
+   functions and printing signed or unsigned integer types (including
+   fixed-width integer types from ``<cstdint>``, ``ptrdiff_t``, ``size_t``
+   and ``ssize_t``) as the opposite signedness to ensure that the output
+   would matches that of a simple wrapper for ``std::sprintf`` that
+   accepted a C-style variable argument list. For example, with
+   `StrictMode` enabled:
+
+  .. code-block:: c++
+
+    extern std::string strprintf(const char *format, ...);
+    int i = -42;
+    unsigned int u = 0xffffffff;
+    return strprintf("%d %u\n", i, u);
+
+  would be converted to:
+
+  .. code-block:: c++
+
+    return std::format("{} {}\n", static_cast<unsigned int>(i), static_cast<int>(u));
+
+  to ensure that the output will continue to be the unsigned representation
+  of -42 and the signed representation of 0xffffffff (often 4294967254
+  and -1 respectively.) When `false` (which is the default), these casts
+  will not be added which may cause a change in the output. Note that this
+  option makes no difference for the default value of
+  `StrFormatLikeFunctions` since ``absl::StrFormat`` takes a function
+  parameter pack and is not a variadic function.
+
+.. option:: StrFormatLikeFunctions
+
+   A semicolon-separated list of (fully qualified) extra function names to
+   replace, with the requirement that the first parameter contains the
+   printf-style format string and the arguments to be formatted follow
+   immediately afterwards. The default value for this option is
+   `absl::StrFormat`.
+
+.. option:: ReplacementFormatFunction
+
+   The function that will be used to replace the function set by the
+   `StrFormatLikeFunctions` option rather than the default
+   `std::format`. It is expected that the function provides an interface
+   that is compatible with ``std::format``. A suitable candidate would be
+   `fmt::format`.
+
+.. option:: FormatHeader
+
+   The header that must be included for the declaration of
+   `ReplacementFormatFunction` so that a ``#include`` directive can be added if
+   required. If `ReplacementFormatFunction` is `std::format` then this option will
+   default to ``<format>``, otherwise this option will default to nothing
+   and no ``#include`` directive will be added.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format-custom.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format-custom.cpp
new file mode 100644
index 00000000000000..bcec9da5cf03a9
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format-custom.cpp
@@ -0,0 +1,76 @@
+// RUN: %check_clang_tidy -check-suffixes=,STRICT \
+// RUN:   -std=c++20 %s modernize-use-std-format %t --      \
+// RUN:   -config="{CheckOptions: \
+// RUN:             [ \
+// RUN:              { \
+// RUN:                key: StrictMode, value: true \
+// RUN:              }, \
+// RUN:              { \
+// RUN:               key: modernize-use-std-format.StrFormatLikeFunctions, \
+// RUN:               value: 'unqualified_strprintf;::strprintf; mynamespace::strprintf2' \
+// RUN:              }, \
+// RUN:              { \
+// RUN:               key: modernize-use-std-format.ReplacementFormatFunction, \
+// RUN:               value: 'fmt::format' \
+// RUN:              }, \
+// RUN:              { \
+// RUN:               key: modernize-use-std-format.FormatHeader, \
+// RUN:               value: '<fmt/core.h>' \
+// RUN:              } \
+// RUN:             ] \
+// RUN:            }" \
+// RUN:   -- -isystem %clang_tidy_headers
+// RUN: %check_clang_tidy -check-suffixes=,NOTSTRICT \
+// RUN:   -std=c++20 %s modernize-use-std-format %t --      \
+// RUN:   -config="{CheckOptions: \
+// RUN:             [ \
+// RUN:              { \
+// RUN:               key: modernize-use-std-format.StrFormatLikeFunctions, \
+// RUN:               value: '::strprintf; mynamespace::strprintf2' \
+// RUN:              }, \
+// RUN:              { \
+// RUN:               key: modernize-use-std-format.ReplacementFormatFunction, \
+// RUN:               value: 'fmt::format' \
+// RUN:              }, \
+// RUN:              { \
+// RUN:               key: modernize-use-std-format.FormatHeader, \
+// RUN:               value: '<fmt/core.h>' \
+// RUN:              } \
+// RUN:             ] \
+// RUN:            }" \
+// RUN:   -- -isystem %clang_tidy_headers
+
+#include <cstdio>
+#include <string>
+// CHECK-FIXES: #include <fmt/core.h>
+
+std::string strprintf(const char *, ...);
+
+namespace mynamespace {
+  std::string strprintf2(const char *, ...);
+}
+
+std::string strprintf_test(const std::string &name, double value) {
+  return strprintf("'%s'='%f'\n", name.c_str(), value);
+  // CHECK...
[truncated]

@llvmbot
Copy link
Collaborator

llvmbot commented Apr 28, 2024

@llvm/pr-subscribers-clang-tidy

Author: Mike Crowe (mikecrowe)

Changes

Add a new clang-tidy check that converts absl::StrFormat (and similar functions) to std::format (and similar functions.)

Split the configuration of FormatStringConverter out to a separate Configuration class so that we don't risk confusion by passing two boolean configuration parameters into the constructor. Add AllowTrailingNewlineRemoval option since we never want to remove trailing newlines in this check.


Patch is 28.81 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/90397.diff

13 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/modernize/CMakeLists.txt (+1)
  • (modified) clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp (+2)
  • (added) clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.cpp (+108)
  • (added) clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.h (+50)
  • (modified) clang-tools-extra/clang-tidy/modernize/UseStdPrintCheck.cpp (+4-1)
  • (modified) clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp (+6-4)
  • (modified) clang-tools-extra/clang-tidy/utils/FormatStringConverter.h (+8-1)
  • (modified) clang-tools-extra/docs/ReleaseNotes.rst (+9)
  • (modified) clang-tools-extra/docs/clang-tidy/checks/list.rst (+1)
  • (added) clang-tools-extra/docs/clang-tidy/checks/modernize/use-std-format.rst (+84)
  • (added) clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format-custom.cpp (+76)
  • (added) clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format-fmt.cpp (+37)
  • (added) clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format.cpp (+97)
diff --git a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
index 8005d6e91c060c..576805c4c7f181 100644
--- a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
@@ -41,6 +41,7 @@ add_clang_library(clangTidyModernizeModule
   UseNullptrCheck.cpp
   UseOverrideCheck.cpp
   UseStartsEndsWithCheck.cpp
+  UseStdFormatCheck.cpp
   UseStdNumbersCheck.cpp
   UseStdPrintCheck.cpp
   UseTrailingReturnTypeCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
index 776558433c5baa..b9c7a2dc383e88 100644
--- a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
@@ -42,6 +42,7 @@
 #include "UseNullptrCheck.h"
 #include "UseOverrideCheck.h"
 #include "UseStartsEndsWithCheck.h"
+#include "UseStdFormatCheck.h"
 #include "UseStdNumbersCheck.h"
 #include "UseStdPrintCheck.h"
 #include "UseTrailingReturnTypeCheck.h"
@@ -76,6 +77,7 @@ class ModernizeModule : public ClangTidyModule {
         "modernize-use-designated-initializers");
     CheckFactories.registerCheck<UseStartsEndsWithCheck>(
         "modernize-use-starts-ends-with");
+    CheckFactories.registerCheck<UseStdFormatCheck>("modernize-use-std-format");
     CheckFactories.registerCheck<UseStdNumbersCheck>(
         "modernize-use-std-numbers");
     CheckFactories.registerCheck<UseStdPrintCheck>("modernize-use-std-print");
diff --git a/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.cpp
new file mode 100644
index 00000000000000..d22ebe857cf415
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.cpp
@@ -0,0 +1,108 @@
+//===--- UseStdFormatCheck.cpp - clang-tidy -------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "UseStdFormatCheck.h"
+#include "../utils/FormatStringConverter.h"
+#include "../utils/Matchers.h"
+#include "../utils/OptionsUtils.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+#include "clang/Tooling/FixIt.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::modernize {
+
+namespace {
+AST_MATCHER(StringLiteral, isOrdinary) { return Node.isOrdinary(); }
+} // namespace
+
+UseStdFormatCheck::UseStdFormatCheck(StringRef Name, ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      StrictMode(Options.getLocalOrGlobal("StrictMode", false)),
+      StrFormatLikeFunctions(utils::options::parseStringList(
+          Options.get("StrFormatLikeFunctions", ""))),
+      ReplacementFormatFunction(
+          Options.get("ReplacementFormatFunction", "std::format")),
+      IncludeInserter(Options.getLocalOrGlobal("IncludeStyle",
+                                               utils::IncludeSorter::IS_LLVM),
+                      areDiagsSelfContained()),
+      MaybeHeaderToInclude(Options.get("FormatHeader")) {
+  if (StrFormatLikeFunctions.empty())
+    StrFormatLikeFunctions.push_back("absl::StrFormat");
+
+  if (!MaybeHeaderToInclude && ReplacementFormatFunction == "std::format")
+    MaybeHeaderToInclude = "<format>";
+}
+
+void UseStdFormatCheck::registerPPCallbacks(const SourceManager &SM,
+                                            Preprocessor *PP,
+                                            Preprocessor *ModuleExpanderPP) {
+  IncludeInserter.registerPreprocessor(PP);
+}
+
+void UseStdFormatCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+      callExpr(argumentCountAtLeast(1),
+               hasArgument(0, stringLiteral(isOrdinary())),
+               callee(functionDecl(unless(cxxMethodDecl()),
+                                   matchers::matchesAnyListedName(
+                                       StrFormatLikeFunctions))
+                          .bind("func_decl")))
+          .bind("strformat"),
+      this);
+}
+
+void UseStdFormatCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  using utils::options::serializeStringList;
+  Options.store(Opts, "StrictMode", StrictMode);
+  Options.store(Opts, "StrFormatLikeFunctions",
+                serializeStringList(StrFormatLikeFunctions));
+  Options.store(Opts, "ReplacementFormatFunction", ReplacementFormatFunction);
+  Options.store(Opts, "IncludeStyle", IncludeInserter.getStyle());
+  if (MaybeHeaderToInclude)
+    Options.store(Opts, "FormatHeader", *MaybeHeaderToInclude);
+}
+
+void UseStdFormatCheck::check(const MatchFinder::MatchResult &Result) {
+  const unsigned FormatArgOffset = 0;
+  const auto *OldFunction = Result.Nodes.getNodeAs<FunctionDecl>("func_decl");
+  const auto *StrFormat = Result.Nodes.getNodeAs<CallExpr>("strformat");
+
+  utils::FormatStringConverter::Configuration ConverterConfig;
+  ConverterConfig.StrictMode = StrictMode;
+  utils::FormatStringConverter Converter(Result.Context, StrFormat,
+                                         FormatArgOffset, ConverterConfig,
+                                         getLangOpts());
+  const Expr *StrFormatCall = StrFormat->getCallee();
+  if (!Converter.canApply()) {
+    DiagnosticBuilder Diag = diag(StrFormat->getBeginLoc(),
+                                  "unable to use '%0' instead of %1 because %2")
+                             << ReplacementFormatFunction
+                             << OldFunction->getIdentifier()
+                             << Converter.conversionNotPossibleReason();
+    return;
+  }
+
+  DiagnosticBuilder Diag =
+      diag(StrFormatCall->getBeginLoc(), "use '%0' instead of %1")
+      << ReplacementFormatFunction << OldFunction->getIdentifier();
+  Diag << FixItHint::CreateReplacement(
+      CharSourceRange::getTokenRange(StrFormatCall->getBeginLoc(),
+                                     StrFormatCall->getEndLoc()),
+      ReplacementFormatFunction);
+  Converter.applyFixes(Diag, *Result.SourceManager);
+
+  if (MaybeHeaderToInclude)
+    Diag << IncludeInserter.createIncludeInsertion(
+        Result.Context->getSourceManager().getFileID(
+            StrFormatCall->getBeginLoc()),
+        *MaybeHeaderToInclude);
+}
+
+} // namespace clang::tidy::modernize
diff --git a/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.h b/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.h
new file mode 100644
index 00000000000000..d2aeb1bd2802c2
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.h
@@ -0,0 +1,50 @@
+//===--- UseStdFormatCheck.h - clang-tidy -----------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USESTDFORMATCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USESTDFORMATCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+#include "../ClangTidyCheck.h"
+#include "../utils/IncludeInserter.h"
+
+namespace clang::tidy::modernize {
+
+/// Convert calls to absl::StrFormat-like functions to std::format.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/modernize/use-std-format.html
+class UseStdFormatCheck : public ClangTidyCheck {
+public:
+  UseStdFormatCheck(StringRef Name, ClangTidyContext *Context);
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+    if (ReplacementFormatFunction == "std::format")
+      return LangOpts.CPlusPlus20;
+    return LangOpts.CPlusPlus;
+  }
+  void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
+                           Preprocessor *ModuleExpanderPP) override;
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  std::optional<TraversalKind> getCheckTraversalKind() const override {
+    return TK_IgnoreUnlessSpelledInSource;
+  }
+
+private:
+  bool StrictMode;
+  std::vector<StringRef> StrFormatLikeFunctions;
+  StringRef ReplacementFormatFunction;
+  utils::IncludeInserter IncludeInserter;
+  std::optional<StringRef> MaybeHeaderToInclude;
+};
+
+} // namespace clang::tidy::modernize
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USESTDFORMATCHECK_H
diff --git a/clang-tools-extra/clang-tidy/modernize/UseStdPrintCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseStdPrintCheck.cpp
index 660996aba7b70d..a4d5fdb852d559 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseStdPrintCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseStdPrintCheck.cpp
@@ -129,8 +129,11 @@ void UseStdPrintCheck::check(const MatchFinder::MatchResult &Result) {
     FormatArgOffset = 1;
   }
 
+  utils::FormatStringConverter::Configuration ConverterConfig;
+  ConverterConfig.StrictMode = StrictMode;
+  ConverterConfig.AllowTrailingNewlineRemoval = true;
   utils::FormatStringConverter Converter(
-      Result.Context, Printf, FormatArgOffset, StrictMode, getLangOpts());
+      Result.Context, Printf, FormatArgOffset, ConverterConfig, getLangOpts());
   const Expr *PrintfCall = Printf->getCallee();
   const StringRef ReplacementFunction = Converter.usePrintNewlineFunction()
                                             ? ReplacementPrintlnFunction
diff --git a/clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp b/clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp
index ad10f745b6acfb..301300eafb5581 100644
--- a/clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp
+++ b/clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp
@@ -198,10 +198,11 @@ static bool castMismatchedIntegerTypes(const CallExpr *Call, bool StrictMode) {
 FormatStringConverter::FormatStringConverter(ASTContext *ContextIn,
                                              const CallExpr *Call,
                                              unsigned FormatArgOffset,
-                                             bool StrictMode,
+                                             const Configuration ConfigIn,
                                              const LangOptions &LO)
-    : Context(ContextIn),
-      CastMismatchedIntegerTypes(castMismatchedIntegerTypes(Call, StrictMode)),
+    : Context(ContextIn), Config(ConfigIn),
+      CastMismatchedIntegerTypes(
+          castMismatchedIntegerTypes(Call, ConfigIn.StrictMode)),
       Args(Call->getArgs()), NumArgs(Call->getNumArgs()),
       ArgsOffset(FormatArgOffset + 1), LangOpts(LO) {
   assert(ArgsOffset <= NumArgs);
@@ -627,7 +628,8 @@ void FormatStringConverter::finalizeFormatText() {
 
   // It's clearer to convert printf("Hello\r\n"); to std::print("Hello\r\n")
   // than to std::println("Hello\r");
-  if (StringRef(StandardFormatString).ends_with("\\n") &&
+  if (Config.AllowTrailingNewlineRemoval &&
+      StringRef(StandardFormatString).ends_with("\\n") &&
       !StringRef(StandardFormatString).ends_with("\\\\n") &&
       !StringRef(StandardFormatString).ends_with("\\r\\n")) {
     UsePrintNewlineFunction = true;
diff --git a/clang-tools-extra/clang-tidy/utils/FormatStringConverter.h b/clang-tools-extra/clang-tidy/utils/FormatStringConverter.h
index 1949870f62ed68..1109a0b602262f 100644
--- a/clang-tools-extra/clang-tidy/utils/FormatStringConverter.h
+++ b/clang-tools-extra/clang-tidy/utils/FormatStringConverter.h
@@ -32,8 +32,14 @@ class FormatStringConverter
 public:
   using ConversionSpecifier = clang::analyze_format_string::ConversionSpecifier;
   using PrintfSpecifier = analyze_printf::PrintfSpecifier;
+
+  struct Configuration {
+    bool StrictMode = false;
+    bool AllowTrailingNewlineRemoval = false;
+  };
+
   FormatStringConverter(ASTContext *Context, const CallExpr *Call,
-                        unsigned FormatArgOffset, bool StrictMode,
+                        unsigned FormatArgOffset, Configuration Config,
                         const LangOptions &LO);
 
   bool canApply() const { return ConversionNotPossibleReason.empty(); }
@@ -45,6 +51,7 @@ class FormatStringConverter
 
 private:
   ASTContext *Context;
+  const Configuration Config;
   const bool CastMismatchedIntegerTypes;
   const Expr *const *Args;
   const unsigned NumArgs;
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 3038d2b125f20d..c166801dfd6a60 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -145,6 +145,15 @@ New checks
   Finds initializer lists for aggregate types that could be
   written as designated initializers instead.
 
+- New :doc:`modernize-use-std-format
+  <clang-tidy/checks/modernize/use-std-format>` check.
+
+  Converts calls to ``absl::StrFormat``, or other functions via
+  configuration options, to C++20's ``std::format``, or another function
+  via a configuration option, modifying the format string appropriately and
+  removing now-unnecessary calls to ``std::string::c_str()`` and
+  ``std::string::data()``.
+
 - New :doc:`readability-enum-initial-value
   <clang-tidy/checks/readability/enum-initial-value>` check.
 
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 49747ff896ba5c..50cba5b1d21153 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -300,6 +300,7 @@ Clang-Tidy Checks
    :doc:`modernize-use-nullptr <modernize/use-nullptr>`, "Yes"
    :doc:`modernize-use-override <modernize/use-override>`, "Yes"
    :doc:`modernize-use-starts-ends-with <modernize/use-starts-ends-with>`, "Yes"
+   :doc:`modernize-use-std-format <modernize/use-std-format>`, "Yes"
    :doc:`modernize-use-std-numbers <modernize/use-std-numbers>`, "Yes"
    :doc:`modernize-use-std-print <modernize/use-std-print>`, "Yes"
    :doc:`modernize-use-trailing-return-type <modernize/use-trailing-return-type>`, "Yes"
diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-std-format.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-std-format.rst
new file mode 100644
index 00000000000000..60dc8ea5e3c46e
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-std-format.rst
@@ -0,0 +1,84 @@
+.. title:: clang-tidy - modernize-use-std-format
+
+modernize-use-std-format
+========================
+
+Converts calls to ``absl::StrFormat``, or other functions via
+configuration options, to C++20's ``std::format``, or another function
+via a configuration option, modifying the format string appropriately and
+removing now-unnecessary calls to ``std::string::c_str()`` and
+``std::string::data()``.
+
+In other words, it turns lines like:
+
+.. code-block:: c++
+
+  return absl::StrFormat("The %s is %3d\n", description.c_str(), value);
+
+into:
+
+.. code-block:: c++
+
+  return std::format("The {} is {:3}", description, value);
+
+The check uses the same format-string-conversion algorithm as
+`modernize-use-std-print <../modernize/use-std-print.html>`_ and its
+shortcomings are described in the documentation for that check.
+
+Options
+-------
+
+.. option:: StrictMode
+
+   When `true`, the check will add casts when converting from variadic
+   functions and printing signed or unsigned integer types (including
+   fixed-width integer types from ``<cstdint>``, ``ptrdiff_t``, ``size_t``
+   and ``ssize_t``) as the opposite signedness to ensure that the output
+   would matches that of a simple wrapper for ``std::sprintf`` that
+   accepted a C-style variable argument list. For example, with
+   `StrictMode` enabled:
+
+  .. code-block:: c++
+
+    extern std::string strprintf(const char *format, ...);
+    int i = -42;
+    unsigned int u = 0xffffffff;
+    return strprintf("%d %u\n", i, u);
+
+  would be converted to:
+
+  .. code-block:: c++
+
+    return std::format("{} {}\n", static_cast<unsigned int>(i), static_cast<int>(u));
+
+  to ensure that the output will continue to be the unsigned representation
+  of -42 and the signed representation of 0xffffffff (often 4294967254
+  and -1 respectively.) When `false` (which is the default), these casts
+  will not be added which may cause a change in the output. Note that this
+  option makes no difference for the default value of
+  `StrFormatLikeFunctions` since ``absl::StrFormat`` takes a function
+  parameter pack and is not a variadic function.
+
+.. option:: StrFormatLikeFunctions
+
+   A semicolon-separated list of (fully qualified) extra function names to
+   replace, with the requirement that the first parameter contains the
+   printf-style format string and the arguments to be formatted follow
+   immediately afterwards. The default value for this option is
+   `absl::StrFormat`.
+
+.. option:: ReplacementFormatFunction
+
+   The function that will be used to replace the function set by the
+   `StrFormatLikeFunctions` option rather than the default
+   `std::format`. It is expected that the function provides an interface
+   that is compatible with ``std::format``. A suitable candidate would be
+   `fmt::format`.
+
+.. option:: FormatHeader
+
+   The header that must be included for the declaration of
+   `ReplacementFormatFunction` so that a ``#include`` directive can be added if
+   required. If `ReplacementFormatFunction` is `std::format` then this option will
+   default to ``<format>``, otherwise this option will default to nothing
+   and no ``#include`` directive will be added.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format-custom.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format-custom.cpp
new file mode 100644
index 00000000000000..bcec9da5cf03a9
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format-custom.cpp
@@ -0,0 +1,76 @@
+// RUN: %check_clang_tidy -check-suffixes=,STRICT \
+// RUN:   -std=c++20 %s modernize-use-std-format %t --      \
+// RUN:   -config="{CheckOptions: \
+// RUN:             [ \
+// RUN:              { \
+// RUN:                key: StrictMode, value: true \
+// RUN:              }, \
+// RUN:              { \
+// RUN:               key: modernize-use-std-format.StrFormatLikeFunctions, \
+// RUN:               value: 'unqualified_strprintf;::strprintf; mynamespace::strprintf2' \
+// RUN:              }, \
+// RUN:              { \
+// RUN:               key: modernize-use-std-format.ReplacementFormatFunction, \
+// RUN:               value: 'fmt::format' \
+// RUN:              }, \
+// RUN:              { \
+// RUN:               key: modernize-use-std-format.FormatHeader, \
+// RUN:               value: '<fmt/core.h>' \
+// RUN:              } \
+// RUN:             ] \
+// RUN:            }" \
+// RUN:   -- -isystem %clang_tidy_headers
+// RUN: %check_clang_tidy -check-suffixes=,NOTSTRICT \
+// RUN:   -std=c++20 %s modernize-use-std-format %t --      \
+// RUN:   -config="{CheckOptions: \
+// RUN:             [ \
+// RUN:              { \
+// RUN:               key: modernize-use-std-format.StrFormatLikeFunctions, \
+// RUN:               value: '::strprintf; mynamespace::strprintf2' \
+// RUN:              }, \
+// RUN:              { \
+// RUN:               key: modernize-use-std-format.ReplacementFormatFunction, \
+// RUN:               value: 'fmt::format' \
+// RUN:              }, \
+// RUN:              { \
+// RUN:               key: modernize-use-std-format.FormatHeader, \
+// RUN:               value: '<fmt/core.h>' \
+// RUN:              } \
+// RUN:             ] \
+// RUN:            }" \
+// RUN:   -- -isystem %clang_tidy_headers
+
+#include <cstdio>
+#include <string>
+// CHECK-FIXES: #include <fmt/core.h>
+
+std::string strprintf(const char *, ...);
+
+namespace mynamespace {
+  std::string strprintf2(const char *, ...);
+}
+
+std::string strprintf_test(const std::string &name, double value) {
+  return strprintf("'%s'='%f'\n", name.c_str(), value);
+  // CHECK...
[truncated]

Remove duplicate inclusion of ClangTidyCheck.h.
Copy link
Member

@PiotrZSL PiotrZSL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be honest I'm not sure why it wasn't merged, there were some dependences on other changes and modernize-use-std-print check, and somehow it fall under radar.

Overall looks fine. Please fix pointed out nits that pop up.

Use getSourceRange rather than getBeginLoc()/getEndLoc().
Synchronise description of check in UseStdFormatCheck.h with the
more-detailed description in the release notes.
Add some tests that include macro expansions.
Avoid calling StringRef constructor multiple times.
Copy link
Member

@SimplyDanny SimplyDanny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only a few nits concerning non-functional code/text. Other than that LGTM.

mikecrowe and others added 3 commits April 30, 2024 17:34
…mat.rst

Co-authored-by: Danny Mösch <danny.moesch@icloud.com>
…mat.rst

Co-authored-by: Danny Mösch <danny.moesch@icloud.com>
…mat.rst

Co-authored-by: Danny Mösch <danny.moesch@icloud.com>
@mikecrowe mikecrowe requested a review from PiotrZSL May 1, 2024 17:14
Copy link
Member

@PiotrZSL PiotrZSL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Remove incorrect newline that won't be removed from the format string
during conversion.
Copy link
Contributor

@5chmidti 5chmidti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just nits, otherwise LGTM!

@mikecrowe
Copy link
Contributor Author

@PiotrZSL wrote:

Once build will pass, I will land this change, and fixes could be done in separate PR for both checks.

The build seems to be passing. Please can you land this if you are happy?

Thanks.

@PiotrZSL PiotrZSL merged commit af79372 into llvm:main May 13, 2024
5 checks passed
nhasabni pushed a commit to nhasabni/llvm-project that referenced this pull request May 14, 2024
Add a new clang-tidy check that converts absl::StrFormat (and similar
functions) to std::format (and similar functions.)

Split the configuration of FormatStringConverter out to a separate
Configuration class so that we don't risk confusion by passing two
boolean configuration parameters into the constructor. Add
AllowTrailingNewlineRemoval option since we never want to remove
trailing newlines in this check.
mub-at-arm pushed a commit to mub-at-arm/llvm-project that referenced this pull request May 16, 2024
Add a new clang-tidy check that converts absl::StrFormat (and similar
functions) to std::format (and similar functions.)

Split the configuration of FormatStringConverter out to a separate
Configuration class so that we don't risk confusion by passing two
boolean configuration parameters into the constructor. Add
AllowTrailingNewlineRemoval option since we never want to remove
trailing newlines in this check.
@kadircet
Copy link
Member

hi! this seems to be crashing for certain code patterns, #92896 has a minimal reproducer

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants