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

[Xiaoyi Zhang via Phabricator via cfe-commits]

zhangxy988 updated this revision to Diff 208500.
zhangxy988 marked 2 inline comments as done.
zhangxy988 added a comment.

- Remove empty line and add assertion message.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64151/new/

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
@@ -9,6 +9,7 @@
 #include "FasterStrsplitDelimiterCheck.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Tooling/FixIt.h"
 
 using namespace clang::ast_matchers;
 
@@ -20,23 +21,29 @@
 
 AST_MATCHER(StringLiteral, lengthIsOne) { return Node.getLength() == 1; }
 
-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 &&
+         "Only single character string should be matched");
+  assert(Literal->getCharByteWidth() == 1 &&
+         "StrSplit doesn't support 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] = '\'';
@@ -98,7 +105,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.208500.patch
Type: text/x-patch
Size: 4889 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190708/c00c781a/attachment.bin>