[clang-tidy] Allow specifying pipe syntax for use-ranges checks (#98696)

Summary:
Add `UseReversePipe` option to (boost|modernize)-use-ranges checks. This
controls whether to create a reverse view using function syntax
(`reverse(Range)`) or pipe syntax (`Range | reverse`)

Test Plan: 

Reviewers: 

Subscribers: 

Tasks: 

Tags: 


Differential Revision: https://phabricator.intern.facebook.com/D60251675

Author: njames93

clang-tools-extra/clang-tidy/boost/UseRangesCheck.cpp

@@ -331,12 +331,15 @@ utils::UseRangesCheck::ReplacerMap UseRangesCheck::getReplacerMap() const {
 
 UseRangesCheck::UseRangesCheck(StringRef Name, ClangTidyContext *Context)
     : utils::UseRangesCheck(Name, Context),
-      IncludeBoostSystem(Options.get("IncludeBoostSystem", true)) {}
+      IncludeBoostSystem(Options.get("IncludeBoostSystem", true)),
+      UseReversePipe(Options.get("UseReversePipe", false)) {}
 
 void UseRangesCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   utils::UseRangesCheck::storeOptions(Opts);
   Options.store(Opts, "IncludeBoostSystem", IncludeBoostSystem);
+  Options.store(Opts, "UseReversePipe", UseReversePipe);
 }
+
 DiagnosticBuilder UseRangesCheck::createDiag(const CallExpr &Call) {
   DiagnosticBuilder D =
       diag(Call.getBeginLoc(), "use a %0 version of this algorithm");
@@ -362,10 +365,10 @@ UseRangesCheck::getReverseDescriptor() const {
       {"::boost::rbegin", "::boost::rend"},
       {"::boost::const_rbegin", "::boost::const_rend"},
   };
-  return ReverseIteratorDescriptor{"boost::adaptors::reverse",
-                                   IncludeBoostSystem
-                                       ? "<boost/range/adaptor/reversed.hpp>"
-                                       : "boost/range/adaptor/reversed.hpp",
-                                   Refs};
+  return ReverseIteratorDescriptor{
+      UseReversePipe ? "boost::adaptors::reversed" : "boost::adaptors::reverse",
+      IncludeBoostSystem ? "<boost/range/adaptor/reversed.hpp>"
+                         : "boost/range/adaptor/reversed.hpp",
+      Refs, UseReversePipe};
 }
 } // namespace clang::tidy::boost

clang-tools-extra/clang-tidy/boost/UseRangesCheck.h

@@ -36,6 +36,7 @@ class UseRangesCheck : public utils::UseRangesCheck {
 
 private:
   bool IncludeBoostSystem;
+  bool UseReversePipe;
 };
 
 } // namespace clang::tidy::boost

clang-tools-extra/clang-tidy/modernize/UseRangesCheck.cpp

@@ -166,6 +166,15 @@ utils::UseRangesCheck::ReplacerMap UseRangesCheck::getReplacerMap() const {
   return Result;
 }
 
+UseRangesCheck::UseRangesCheck(StringRef Name, ClangTidyContext *Context)
+    : utils::UseRangesCheck(Name, Context),
+      UseReversePipe(Options.get("UseReversePipe", false)) {}
+
+void UseRangesCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  utils::UseRangesCheck::storeOptions(Opts);
+  Options.store(Opts, "UseReversePipe", UseReversePipe);
+}
+
 bool UseRangesCheck::isLanguageVersionSupported(
     const LangOptions &LangOpts) const {
   return LangOpts.CPlusPlus20;
@@ -180,6 +189,8 @@ std::optional<UseRangesCheck::ReverseIteratorDescriptor>
 UseRangesCheck::getReverseDescriptor() const {
   static const std::pair<StringRef, StringRef> Refs[] = {
       {"::std::rbegin", "::std::rend"}, {"::std::crbegin", "::std::crend"}};
-  return ReverseIteratorDescriptor{"std::views::reverse", "<ranges>", Refs};
+  return ReverseIteratorDescriptor{UseReversePipe ? "std::views::reverse"
+                                                  : "std::ranges::reverse_view",
+                                   "<ranges>", Refs, UseReversePipe};
 }
 } // namespace clang::tidy::modernize

clang-tools-extra/clang-tidy/modernize/UseRangesCheck.h

@@ -20,7 +20,9 @@ namespace clang::tidy::modernize {
 /// http://clang.llvm.org/extra/clang-tidy/checks/modernize/use-ranges.html
 class UseRangesCheck : public utils::UseRangesCheck {
 public:
-  using utils::UseRangesCheck::UseRangesCheck;
+  UseRangesCheck(StringRef CheckName, ClangTidyContext *Context);
+
+  void storeOptions(ClangTidyOptions::OptionMap &Options) override;
 
   ReplacerMap getReplacerMap() const override;
 
@@ -31,6 +33,9 @@ class UseRangesCheck : public utils::UseRangesCheck {
   getReverseDescriptor() const override;
 
   bool isLanguageVersionSupported(const LangOptions &LangOpts) const override;
+
+private:
+  bool UseReversePipe;
 };
 
 } // namespace clang::tidy::modernize

clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp

@@ -242,16 +242,20 @@ void UseRangesCheck::check(const MatchFinder::MatchResult &Result) {
           Diag << Inserter.createIncludeInsertion(
               Result.SourceManager->getFileID(Call->getBeginLoc()),
               *ReverseDescriptor->ReverseHeader);
+        StringRef ArgText = Lexer::getSourceText(
+            CharSourceRange::getTokenRange(ArgExpr->getSourceRange()),
+            Result.Context->getSourceManager(), Result.Context->getLangOpts());
+        SmallString<128> ReplaceText;
+        if (ReverseDescriptor->IsPipeSyntax)
+          ReplaceText.assign(
+              {ArgText, " | ", ReverseDescriptor->ReverseAdaptorName});
+        else
+          ReplaceText.assign(
+              {ReverseDescriptor->ReverseAdaptorName, "(", ArgText, ")"});
         Diag << FixItHint::CreateReplacement(
             Call->getArg(Replace == Indexes::Second ? Second : First)
                 ->getSourceRange(),
-            SmallString<128>{
-                ReverseDescriptor->ReverseAdaptorName, "(",
-                Lexer::getSourceText(
-                    CharSourceRange::getTokenRange(ArgExpr->getSourceRange()),
-                    Result.Context->getSourceManager(),
-                    Result.Context->getLangOpts()),
-                ")"});
+            ReplaceText);
       }
       ToRemove.push_back(Replace == Indexes::Second ? First : Second);
     }

clang-tools-extra/clang-tidy/utils/UseRangesCheck.h

@@ -38,6 +38,7 @@ class UseRangesCheck : public ClangTidyCheck {
     StringRef ReverseAdaptorName;
     std::optional<StringRef> ReverseHeader;
     ArrayRef<std::pair<StringRef, StringRef>> FreeReverseNames;
+    bool IsPipeSyntax = false;
   };
 
   class Replacer : public llvm::RefCountedBase<Replacer> {

clang-tools-extra/docs/clang-tidy/checks/boost/use-ranges.rst

@@ -154,8 +154,8 @@ Transforms to:
 
 .. code-block:: c++
 
-  auto AreSame = std::equal(boost::adaptors::reverse(Items1),
-                            boost::adaptors::reverse(Items2));
+  auto AreSame = boost::range::equal(boost::adaptors::reverse(Items1),
+                                     boost::adaptors::reverse(Items2));
 
 Options
 -------
@@ -170,3 +170,18 @@ Options
    If `true` (default value) the boost headers are included as system headers
    with angle brackets (`#include <boost.hpp>`), otherwise quotes are used
    (`#include "boost.hpp"`).
+
+.. option:: UseReversePipe
+
+  When `true` (default `false`), fixes which involve reverse ranges will use the
+  pipe adaptor syntax instead of the function syntax.
+
+  .. code-block:: c++
+
+    std::find(Items.rbegin(), Items.rend(), 0);
+
+  Transforms to:
+
+  .. code-block:: c++
+
+    boost::range::find(Items | boost::adaptors::reversed, 0);

clang-tools-extra/docs/clang-tidy/checks/modernize/use-ranges.rst

@@ -116,8 +116,8 @@ Transforms to:
 
 .. code-block:: c++
 
-  auto AreSame = std::equal(std::views::reverse(Items1),
-                            std::views::reverse(Items2));
+  auto AreSame = std::ranges::equal(std::ranges::reverse_view(Items1),
+                                    std::ranges::reverse_view(Items2));
 
 Options
 -------
@@ -127,3 +127,17 @@ Options
    A string specifying which include-style is used, `llvm` or `google`. Default
    is `llvm`.
 
+.. option:: UseReversePipe
+
+  When `true` (default `false`), fixes which involve reverse ranges will use the
+  pipe adaptor syntax instead of the function syntax.
+
+  .. code-block:: c++
+
+    std::find(Items.rbegin(), Items.rend(), 0);
+
+  Transforms to:
+
+  .. code-block:: c++
+
+    std::ranges::find(Items | std::views::reverse, 0);

clang-tools-extra/test/clang-tidy/checkers/boost/Inputs/use-ranges/fake_boost.h

@@ -0,0 +1,29 @@
+#ifndef USE_RANGES_FAKE_BOOST_H
+#define USE_RANGES_FAKE_BOOST_H
+
+namespace boost {
+namespace range_adl_barrier {
+
+template <typename T> void *begin(T &);
+template <typename T> void *end(T &);
+template <typename T> void *const_begin(const T &);
+template <typename T> void *const_end(const T &);
+} // namespace range_adl_barrier
+
+using namespace range_adl_barrier;
+
+template <typename T> void *rbegin(T &);
+template <typename T> void *rend(T &);
+
+template <typename T> void *const_rbegin(T &);
+template <typename T> void *const_rend(T &);
+namespace algorithm {
+
+template <class InputIterator, class T, class BinaryOperation>
+T reduce(InputIterator first, InputIterator last, T init, BinaryOperation bOp) {
+  return init;
+}
+} // namespace algorithm
+} // namespace boost
+
+#endif // USE_RANGES_FAKE_BOOST_H

clang-tools-extra/test/clang-tidy/checkers/boost/Inputs/use-ranges/fake_std.h

@@ -0,0 +1,99 @@
+#ifndef USE_RANGES_FAKE_STD_H
+#define USE_RANGES_FAKE_STD_H
+namespace std {
+
+template <typename T> class vector {
+public:
+  using iterator = T *;
+  using const_iterator = const T *;
+  using reverse_iterator = T*;
+  using reverse_const_iterator = const T*;
+
+  constexpr const_iterator begin() const;
+  constexpr const_iterator end() const;
+  constexpr const_iterator cbegin() const;
+  constexpr const_iterator cend() const;
+  constexpr iterator begin();
+  constexpr iterator end();
+  constexpr reverse_const_iterator rbegin() const;
+  constexpr reverse_const_iterator rend() const;
+  constexpr reverse_const_iterator crbegin() const;
+  constexpr reverse_const_iterator crend() const;
+  constexpr reverse_iterator rbegin();
+  constexpr reverse_iterator rend();
+};
+
+template <typename Container> constexpr auto begin(const Container &Cont) {
+  return Cont.begin();
+}
+
+template <typename Container> constexpr auto begin(Container &Cont) {
+  return Cont.begin();
+}
+
+template <typename Container> constexpr auto end(const Container &Cont) {
+  return Cont.end();
+}
+
+template <typename Container> constexpr auto end(Container &Cont) {
+  return Cont.end();
+}
+
+template <typename Container> constexpr auto cbegin(const Container &Cont) {
+  return Cont.cbegin();
+}
+
+template <typename Container> constexpr auto cend(const Container &Cont) {
+  return Cont.cend();
+}
+// Find
+template< class InputIt, class T >
+InputIt find(InputIt first, InputIt last, const T& value);
+
+template <typename Iter> void reverse(Iter begin, Iter end);
+
+template <class InputIt1, class InputIt2>
+bool includes(InputIt1 first1, InputIt1 last1, InputIt2 first2, InputIt2 last2);
+
+template <class ForwardIt1, class ForwardIt2>
+bool is_permutation(ForwardIt1 first1, ForwardIt1 last1, ForwardIt2 first2,
+                    ForwardIt2 last2);
+
+template <class BidirIt>
+bool next_permutation(BidirIt first, BidirIt last);
+
+inline namespace inline_test{
+
+template <class ForwardIt1, class ForwardIt2>
+bool equal(ForwardIt1 first1, ForwardIt1 last1,
+           ForwardIt2 first2, ForwardIt2 last2);
+
+template <class RandomIt>
+void push_heap(RandomIt first, RandomIt last);
+
+template <class InputIt, class OutputIt, class UnaryPred>
+OutputIt copy_if(InputIt first, InputIt last, OutputIt d_first, UnaryPred pred);
+
+template <class ForwardIt>
+ForwardIt is_sorted_until(ForwardIt first, ForwardIt last);
+
+template <class InputIt>
+void reduce(InputIt first, InputIt last);
+
+template <class InputIt, class T>
+T reduce(InputIt first, InputIt last, T init);
+
+template <class InputIt, class T, class BinaryOp>
+T reduce(InputIt first, InputIt last, T init, BinaryOp op) {
+  // Need a definition to suppress undefined_internal_type when invoked with lambda
+  return init;
+}
+
+template <class InputIt, class T>
+T accumulate(InputIt first, InputIt last, T init);
+
+} // namespace inline_test
+
+} // namespace std
+
+#endif // USE_RANGES_FAKE_STD_H