[clang-tools-extra] r365463 - Enhance abseil-faster-strsplit-delimiter to handle other non-printable characters.

[Dmitri Gribenko via cfe-commits]

Author: gribozavr
Date: Tue Jul  9 04:04:04 2019
New Revision: 365463

URL: http://llvm.org/viewvc/llvm-project?rev=365463&view=rev
Log:
Enhance abseil-faster-strsplit-delimiter to handle other non-printable characters.

Summary:
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'`.

Reviewers: gribozavr

Reviewed By: gribozavr

Subscribers: lebedev.ri, Eugene.Zelenko, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D64151

Patch by Xiaoyi Zhang.

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

Modified: clang-tools-extra/trunk/clang-tidy/abseil/FasterStrsplitDelimiterCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/FasterStrsplitDelimiterCheck.cpp?rev=365463&r1=365462&r2=365463&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/abseil/FasterStrsplitDelimiterCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/abseil/FasterStrsplitDelimiterCheck.cpp Tue Jul  9 04:04:04 2019
@@ -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 @@ namespace {
 
 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 @@ void FasterStrsplitDelimiterCheck::check
   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();

Modified: clang-tools-extra/trunk/test/clang-tidy/abseil-faster-strsplit-delimiter.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/abseil-faster-strsplit-delimiter.cpp?rev=365463&r1=365462&r2=365463&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/abseil-faster-strsplit-delimiter.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/abseil-faster-strsplit-delimiter.cpp Tue Jul  9 04:04:04 2019
@@ -44,6 +44,31 @@ void SplitDelimiters() {
   // 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", 'A');
+
+  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');