[clang-tidy] Abseil: faster strsplit delimiter check

This check is an abseil specific check that checks for code using single character string literals as delimiters and transforms the code into characters.

The check was developed internally and has been running at google, this is just
a move to open source the check. It was originally written by @sbenza.

Patch by Deanna Garcia!

llvm-svn: 340411

clang-tools-extra/clang-tidy/abseil/AbseilTidyModule.cpp

@@ -11,6 +11,7 @@
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
 #include "DurationDivisionCheck.h"
+#include "FasterStrsplitDelimiterCheck.h"
 #include "StringFindStartswithCheck.h"
 
 namespace clang {
@@ -22,6 +23,8 @@ public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<DurationDivisionCheck>(
         "abseil-duration-division");
+    CheckFactories.registerCheck<FasterStrsplitDelimiterCheck>(
+        "abseil-faster-strsplit-delimiter");
     CheckFactories.registerCheck<StringFindStartswithCheck>(
         "abseil-string-find-startswith");
   }

clang-tools-extra/clang-tidy/abseil/CMakeLists.txt

@@ -3,6 +3,7 @@ set(LLVM_LINK_COMPONENTS support)
 add_clang_library(clangTidyAbseilModule
   AbseilTidyModule.cpp
   DurationDivisionCheck.cpp
+  FasterStrsplitDelimiterCheck.cpp
   StringFindStartswithCheck.cpp
 
   LINK_LIBS

clang-tools-extra/clang-tidy/abseil/FasterStrsplitDelimiterCheck.cpp

@@ -0,0 +1,131 @@
+//===--- FasterStrsplitDelimiterCheck.cpp - clang-tidy---------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "FasterStrsplitDelimiterCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+namespace {
+
+AST_MATCHER(StringLiteral, lengthIsOne) { return Node.getLength() == 1; }
+
+::internal::Matcher<Expr>
+constructExprWithArg(llvm::StringRef ClassName,
+                     const ::internal::Matcher<Expr> &Arg) {
+  auto ConstrExpr = cxxConstructExpr(hasType(recordDecl(hasName(ClassName))),
+                                     hasArgument(0, ignoringParenCasts(Arg)));
+
+  return anyOf(ConstrExpr, cxxBindTemporaryExpr(has(ConstrExpr)));
+}
+
+::internal::Matcher<Expr>
+copyConstructExprWithArg(llvm::StringRef ClassName,
+                         const ::internal::Matcher<Expr> &Arg) {
+  return constructExprWithArg(ClassName, constructExprWithArg(ClassName, Arg));
+}
+
+llvm::Optional<std::string> makeCharacterLiteral(const StringLiteral *Literal) {
+  std::string Result;
+  {
+    llvm::raw_string_ostream Stream(Result);
+    Literal->outputString(Stream);
+  }
+
+  // Special case: If the string contains a single quote, we just need to return
+  // a character of the single quote. This is a special case because we need to
+  // escape it in the character literal.
+  if (Result == R"("'")")
+    return std::string(R"('\'')");
+
+  assert(Result.size() == 3 || (Result.size() == 4 && Result.substr(0, 2) == "\"\\"));
+
+  // Now replace the " with '.
+  auto Pos = Result.find_first_of('"');
+  if (Pos == Result.npos)
+    return llvm::None;
+  Result[Pos] = '\'';
+  Pos = Result.find_last_of('"');
+  if (Pos == Result.npos)
+    return llvm::None;
+  Result[Pos] = '\'';
+  return Result;
+}
+
+} // anonymous namespace
+
+void FasterStrsplitDelimiterCheck::registerMatchers(MatchFinder *Finder) {
+  if (!getLangOpts().CPlusPlus)
+    return;
+
+  // Binds to one character string literals.
+  const auto SingleChar =
+      expr(ignoringParenCasts(stringLiteral(lengthIsOne()).bind("Literal")));
+
+  // Binds to a string_view (either absl or std) that was passed by value and
+  // contructed from string literal.
+  auto StringViewArg =
+      copyConstructExprWithArg("::absl::string_view", SingleChar);
+
+  auto ByAnyCharArg =
+      expr(copyConstructExprWithArg("::absl::ByAnyChar", StringViewArg))
+          .bind("ByAnyChar");
+
+  // Find uses of absl::StrSplit(..., "x") and absl::StrSplit(...,
+  // absl::ByAnyChar("x")) to transform them into absl::StrSplit(..., 'x').
+  Finder->addMatcher(callExpr(callee(functionDecl(hasName("::absl::StrSplit"))),
+                              hasArgument(1, anyOf(ByAnyCharArg, SingleChar)),
+                              unless(isInTemplateInstantiation()))
+                         .bind("StrSplit"),
+                     this);
+
+  // Find uses of absl::MaxSplits("x", N) and
+  // absl::MaxSplits(absl::ByAnyChar("x"), N) to transform them into
+  // absl::MaxSplits('x', N).
+  Finder->addMatcher(
+      callExpr(
+          callee(functionDecl(hasName("::absl::MaxSplits"))),
+          hasArgument(0, anyOf(ByAnyCharArg, ignoringParenCasts(SingleChar))),
+          unless(isInTemplateInstantiation())),
+      this);
+}
+
+void FasterStrsplitDelimiterCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  const auto *Literal = Result.Nodes.getNodeAs<StringLiteral>("Literal");
+
+  if (Literal->getBeginLoc().isMacroID() || Literal->getEndLoc().isMacroID())
+    return;
+
+  llvm::Optional<std::string> Replacement = makeCharacterLiteral(Literal);
+  if (!Replacement)
+    return;
+  SourceRange Range = Literal->getSourceRange();
+
+  if (const auto *ByAnyChar = Result.Nodes.getNodeAs<Expr>("ByAnyChar"))
+    Range = ByAnyChar->getSourceRange();
+
+  diag(
+      Literal->getBeginLoc(),
+      "%select{absl::StrSplit()|absl::MaxSplits()}0 called with a string "
+      "literal "
+      "consisting of a single character; consider using the character overload")
+      << (Result.Nodes.getNodeAs<CallExpr>("StrSplit") ? 0 : 1)
+      << FixItHint::CreateReplacement(CharSourceRange::getTokenRange(Range),
+                                      *Replacement);
+}
+
+} // namespace abseil
+} // namespace tidy
+} // namespace clang

clang-tools-extra/clang-tidy/abseil/FasterStrsplitDelimiterCheck.h

@@ -0,0 +1,36 @@
+//===--- FasterStrsplitDelimiterCheck.h - clang-tidy-------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_FASTERSTRSPLITDELIMITERCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_FASTERSTRSPLITDELIMITERCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+/// Finds instances of absl::StrSplit() or absl::MaxSplits() where the delimiter
+/// is a single character string literal and replaces it with a character.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/abseil-faster-strsplit-delimiter.html
+class FasterStrsplitDelimiterCheck : public ClangTidyCheck {
+public:
+  FasterStrsplitDelimiterCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace abseil
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_FASTERSTRSPLITDELIMITERCHECK_H

clang-tools-extra/docs/ReleaseNotes.rst

@@ -64,6 +64,12 @@ Improvements to clang-tidy
   floating-point context, and recommends the use of a function that
   returns a floating-point value.
 
+- New :doc:`abseil-faster-strsplit-delimiter
+  <clang-tidy/checks/abseil-faster-strsplit-delimiter>` check.
+
+  Finds instances of ``absl::StrSplit()`` or ``absl::MaxSplits()`` where the
+  delimiter is a single character string literal and replaces with a character.
+
 - New :doc:`readability-magic-numbers
   <clang-tidy/checks/readability-magic-numbers>` check.
 

clang-tools-extra/docs/clang-tidy/checks/abseil-faster-strsplit-delimiter.rst

@@ -0,0 +1,41 @@
+.. title:: clang-tidy - abseil-faster-strsplit-delimiter
+
+abseil-faster-strsplit-delimiter
+================================
+
+Finds instances of ``absl::StrSplit()`` or ``absl::MaxSplits()`` where the
+delimiter is a single character string literal and replaces with a character.
+The check will offer a suggestion to change the string literal into a character.
+It will also catch code using ``absl::ByAnyChar()`` for just a single character
+and will transform that into a single character as well.
+
+These changes will give the same result, but using characters rather than
+single character string literals is more efficient and readable.
+
+Examples:
+
+.. code-block:: c++
+
+  // Original - the argument is a string literal.
+  for (auto piece : absl::StrSplit(str, "B")) {
+
+  // Suggested - the argument is a character, which causes the more efficient
+  // overload of absl::StrSplit() to be used.
+  for (auto piece : absl::StrSplit(str, 'B')) {
+
+
+  // Original - the argument is a string literal inside absl::ByAnyChar call.
+  for (auto piece : absl::StrSplit(str, absl::ByAnyChar("B"))) {
+
+  // Suggested - the argument is a character, which causes the more efficient
+  // overload of absl::StrSplit() to be used and we do not need absl::ByAnyChar
+  // anymore.
+  for (auto piece : absl::StrSplit(str, 'B')) {
+
+
+  // Original - the argument is a string literal inside absl::MaxSplits call.
+  for (auto piece : absl::StrSplit(str, absl::MaxSplits("B", 1))) {
+
+  // Suggested - the argument is a character, which causes the more efficient
+  // overload of absl::StrSplit() to be used.
+  for (auto piece : absl::StrSplit(str, absl::MaxSplits('B', 1))) {

clang-tools-extra/docs/clang-tidy/checks/list.rst

@@ -5,6 +5,7 @@ Clang-Tidy Checks
 
 .. toctree::
    abseil-duration-division
+   abseil-faster-strsplit-delimiter
    abseil-string-find-startswith
    android-cloexec-accept
    android-cloexec-accept4

clang-tools-extra/test/clang-tidy/abseil-faster-strsplit-delimiter.cpp

@@ -0,0 +1,99 @@
+// RUN: %check_clang_tidy %s abseil-faster-strsplit-delimiter %t
+
+namespace absl {
+
+class string_view {
+  public:
+    string_view();
+    string_view(const char *);
+};
+
+namespace strings_internal {
+struct Splitter {};
+struct MaxSplitsImpl {
+  MaxSplitsImpl();
+  ~MaxSplitsImpl();
+};
+} //namespace strings_internal
+
+template <typename Delim>
+strings_internal::Splitter StrSplit(absl::string_view, Delim) {
+  return {};
+}
+template <typename Delim, typename Pred>
+strings_internal::Splitter StrSplit(absl::string_view, Delim, Pred) {
+  return {};
+}
+
+class ByAnyChar {
+  public:
+    explicit ByAnyChar(absl::string_view);
+    ~ByAnyChar();
+};
+
+template <typename Delim>
+strings_internal::MaxSplitsImpl MaxSplits(Delim, int) {
+  return {};
+}
+
+} //namespace absl
+
+void SplitDelimiters() {
+  absl::StrSplit("ABC", "A");
+  // CHECK-MESSAGES: [[@LINE-1]]:25: warning: absl::StrSplit() called with a string literal consisting of a single character; consider using the character overload [abseil-faster-strsplit-delimiter]
+  // CHECK-FIXES: absl::StrSplit("ABC", 'A');
+
+  absl::StrSplit("ABC", absl::ByAnyChar("\n"));
+  // CHECK-MESSAGES: [[@LINE-1]]:41: warning: absl::StrSplit()
+  // CHECK-FIXES: absl::StrSplit("ABC", '\n');
+
+  // Works with predicate
+  absl::StrSplit("ABC", "A", [](absl::string_view) { return true; });
+  // CHECK-MESSAGES: [[@LINE-1]]:25: warning: absl::StrSplit()
+  // CHECK-FIXES: absl::StrSplit("ABC", 'A', [](absl::string_view) { return true; });
+
+  // Doesn't do anything with other strings lenghts.
+  absl::StrSplit("ABC", "AB");
+  absl::StrSplit("ABC", absl::ByAnyChar(""));
+  absl::StrSplit("ABC", absl::ByAnyChar(" \t"));
+
+  // Escapes a single quote in the resulting character literal.
+  absl::StrSplit("ABC", "'");
+  // CHECK-MESSAGES: [[@LINE-1]]:25: warning: absl::StrSplit()
+  // CHECK-FIXES: absl::StrSplit("ABC", '\'');
+
+  absl::StrSplit("ABC", "\"");
+  // CHECK-MESSAGES: [[@LINE-1]]:25: warning: absl::StrSplit()
+  // CHECK-FIXES: absl::StrSplit("ABC", '\"');
+
+  absl::StrSplit("ABC", absl::MaxSplits("\t", 1));
+  // CHECK-MESSAGES: [[@LINE-1]]:41: warning: absl::MaxSplits()
+  // CHECK-FIXES: absl::StrSplit("ABC", absl::MaxSplits('\t', 1));
+
+  auto delim = absl::MaxSplits(absl::ByAnyChar(" "), 1);
+  // CHECK-MESSAGES: [[@LINE-1]]:48: warning: absl::MaxSplits()
+  // CHECK-FIXES: auto delim = absl::MaxSplits(' ', 1);
+}
+
+#define MACRO(str) absl::StrSplit("ABC", str)
+
+void Macro() {
+  MACRO("A");
+}
+
+template <typename T>
+void FunctionTemplate() {
+  // This one should not warn because ByAnyChar is a dependent type.
+  absl::StrSplit("TTT", T("A"));
+
+  // This one will warn, but we are checking that we get a correct warning only
+  // once.
+  absl::StrSplit("TTT", "A");
+  // CHECK-MESSAGES: [[@LINE-1]]:25: warning: absl::StrSplit()
+  // CHECK-FIXES: absl::StrSplit("TTT", 'A');
+}
+
+void FunctionTemplateCaller() {
+  FunctionTemplate<absl::ByAnyChar>();
+  FunctionTemplate<absl::string_view>();
+}