[PATCH] D64151: Enhance abseil-faster-strsplit-delimiter to handle other non-printable characters.

[Xiaoyi Zhang via Phabricator via cfe-commits]

zhangxy988 created this revision.
zhangxy988 added a reviewer: gribozavr.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Currently it fails on cases like '\001'.

Note: Since `StringLiteral::outputString` dumps most nonprintable
characters in octal value, the exact string literal format isn't preserved,
e.g. `"\x01"` becomes `'\001'`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D64151

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


Index: clang-tools-extra/test/clang-tidy/abseil-faster-strsplit-delimiter.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/abseil-faster-strsplit-delimiter.cpp
+++ clang-tools-extra/test/clang-tidy/abseil-faster-strsplit-delimiter.cpp
@@ -44,6 +44,31 @@
   // 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", "\x01");
+  // 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", '\x01');
+
+  absl::StrSplit("ABC", "\001");
+  // 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", '\001');
+
+  absl::StrSplit("ABC", R"(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", 'B');
+
+  absl::StrSplit("ABC", R"(')");
+  // 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", '\'');
+
+  absl::StrSplit("ABC", R"(
+)");
+  // CHECK-MESSAGES: [[@LINE-2]]: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", '\n');
+
+  absl::StrSplit("ABC", R"delimiter(A)delimiter");
+  // 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');
Index: clang-tools-extra/clang-tidy/abseil/FasterStrsplitDelimiterCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/abseil/FasterStrsplitDelimiterCheck.cpp
+++ clang-tools-extra/clang-tidy/abseil/FasterStrsplitDelimiterCheck.cpp
@@ -7,8 +7,10 @@
 //===----------------------------------------------------------------------===//
 
 #include "FasterStrsplitDelimiterCheck.h"
+
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Tooling/FixIt.h"
 
 using namespace clang::ast_matchers;
 
@@ -35,23 +37,27 @@
   return constructExprWithArg(ClassName, constructExprWithArg(ClassName, Arg));
 }
 
-llvm::Optional<std::string> makeCharacterLiteral(const StringLiteral *Literal) {
-  std::string Result;
-  {
+llvm::Optional<std::string> makeCharacterLiteral(const StringLiteral *Literal,
+                                                 const ASTContext &Context) {
+  assert(Literal->getLength() == 1);
+  assert(Literal->getCharByteWidth() == 1); // no wide char
+  std::string Result = clang::tooling::fixit::getText(*Literal, Context).str();
+  bool IsRawStringLiteral = StringRef(Result).startswith(R"(R")");
+  // Since raw string literal might contain unescaped non-printable characters,
+  // we normalize them using `StringLiteral::outputString`.
+  if (IsRawStringLiteral) {
+    Result.clear();
     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('"');
+  std::string::size_type Pos = Result.find_first_of('"');
   if (Pos == Result.npos)
     return llvm::None;
   Result[Pos] = '\'';
@@ -107,7 +113,8 @@
   if (Literal->getBeginLoc().isMacroID() || Literal->getEndLoc().isMacroID())
     return;
 
-  llvm::Optional<std::string> Replacement = makeCharacterLiteral(Literal);
+  llvm::Optional<std::string> Replacement =
+      makeCharacterLiteral(Literal, *Result.Context);
   if (!Replacement)
     return;
   SourceRange Range = Literal->getSourceRange();


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D64151.207853.patch
Type: text/x-patch
Size: 4888 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190703/e2c12313/attachment-0001.bin>