[clang-tidy] Add more detection rules for redundant c_str calls.

Summary:
The string class contains methods which support receiving either a string literal or a string object.

For example, calls to append can receive either a char* or a string.
```
  string& append (const string& str);
  string& append (const char* s);
```

Which make these cases equivalent, and the .c_str() useless:
```
  std::string s = "123";
  str.append(s);
  str.append(s.c_str());
```

In these cases, removing .c_str()  doesn't provide any size or speed improvement.
It's only a readability issue.

If the string contains embedded NUL characters,  the string literal and the string
object won't produce the same semantic.

Reviewers: alexfh, sbenza

Subscribers: LegalizeAdulthood, aaron.ballman, chapuni, Eugene.Zelenko, cfe-commits

Differential Revision: http://reviews.llvm.org/D18475

llvm-svn: 266463

clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp

@@ -103,11 +103,74 @@ void RedundantStringCStrCheck::registerMatchers(
                         callee(cxxMethodDecl(hasName("c_str"))))
           .bind("call");
 
+  // Detect redundant 'c_str()' calls through a string constructor.
   Finder->addMatcher(
       cxxConstructExpr(StringConstructorExpr,
                        hasArgument(0, StringCStrCallExpr)),
       this);
 
+  // Detect: 's == str.c_str()'  ->  's == str'
+  Finder->addMatcher(
+      cxxOperatorCallExpr(
+          anyOf(hasOverloadedOperatorName("<"),
+                hasOverloadedOperatorName(">"),
+                hasOverloadedOperatorName(">="),
+                hasOverloadedOperatorName("<="),
+                hasOverloadedOperatorName("!="),
+                hasOverloadedOperatorName("=="),
+                hasOverloadedOperatorName("+")),
+          anyOf(allOf(hasArgument(0, StringExpr),
+                      hasArgument(1, StringCStrCallExpr)),
+                allOf(hasArgument(0, StringCStrCallExpr),
+                      hasArgument(1, StringExpr)))),
+      this);
+
+  // Detect: 'dst += str.c_str()'  ->  'dst += str'
+  // Detect: 's = str.c_str()'  ->  's = str'
+  Finder->addMatcher(
+      cxxOperatorCallExpr(
+          anyOf(hasOverloadedOperatorName("="),
+                hasOverloadedOperatorName("+=")),
+          hasArgument(0, StringExpr),
+          hasArgument(1, StringCStrCallExpr)),
+      this);
+
+  // Detect: 'dst.append(str.c_str())'  ->  'dst.append(str)'
+  Finder->addMatcher(
+      cxxMemberCallExpr(on(StringExpr),
+          callee(decl(cxxMethodDecl(
+              hasAnyName("append", "assign", "compare")))),
+          argumentCountIs(1),
+          hasArgument(0, StringCStrCallExpr)),
+      this);
+
+  // Detect: 'dst.compare(p, n, str.c_str())'  ->  'dst.compare(p, n, str)'
+  Finder->addMatcher(
+      cxxMemberCallExpr(on(StringExpr),
+          callee(decl(cxxMethodDecl(hasName("compare")))),
+          argumentCountIs(3),
+          hasArgument(2, StringCStrCallExpr)),
+      this);
+
+  // Detect: 'dst.find(str.c_str())'  ->  'dst.find(str)'
+  Finder->addMatcher(
+      cxxMemberCallExpr(on(StringExpr),
+          callee(decl(cxxMethodDecl(
+              hasAnyName("find", "find_first_not_of", "find_first_of",
+                         "find_last_not_of", "find_last_of", "rfind")))),
+          anyOf(argumentCountIs(1), argumentCountIs(2)),
+          hasArgument(0, StringCStrCallExpr)),
+      this);
+
+  // Detect: 'dst.insert(pos, str.c_str())'  ->  'dst.insert(pos, str)'
+  Finder->addMatcher(
+      cxxMemberCallExpr(on(StringExpr),
+          callee(decl(cxxMethodDecl(hasName("insert")))),
+          argumentCountIs(2),
+          hasArgument(1, StringCStrCallExpr)),
+      this);
+
+  // Detect redundant 'c_str()' calls through a StringRef constructor.
   Finder->addMatcher(
       cxxConstructExpr(
           // Implicit constructors of these classes are overloaded
@@ -115,8 +178,8 @@ void RedundantStringCStrCheck::registerMatchers(
           // referring to the argument.  Passing a string directly to
           // them is preferred to passing a char pointer.
           hasDeclaration(
-              cxxMethodDecl(anyOf(hasName("::llvm::StringRef::StringRef"),
-                                  hasName("::llvm::Twine::Twine")))),
+              cxxMethodDecl(hasAnyName("::llvm::StringRef::StringRef",
+                                       "::llvm::Twine::Twine"))),
           argumentCountIs(1),
           // The only argument must have the form x.c_str() or p->c_str()
           // where the method is string::c_str().  StringRef also has

clang-tools-extra/test/clang-tidy/readability-redundant-string-cstr.cpp

@@ -1,4 +1,8 @@
-// RUN: %check_clang_tidy %s readability-redundant-string-cstr %t -- -- -target x86_64-unknown -std=c++11
+// RUN: %check_clang_tidy %s readability-redundant-string-cstr %t -- -- -std=c++11
+
+typedef unsigned __INT16_TYPE__ char16;
+typedef unsigned __INT32_TYPE__ char32;
+typedef __SIZE_TYPE__ size;
 
 namespace std {
 template <typename T>
@@ -7,16 +11,50 @@ template <typename T>
 class char_traits {};
 template <typename C, typename T, typename A>
 struct basic_string {
+  typedef basic_string<C, T, A> _Type;
   basic_string();
   basic_string(const C *p, const A &a = A());
+
   const C *c_str() const;
+
+  _Type& append(const C *s);
+  _Type& append(const C *s, size n);
+  _Type& assign(const C *s);
+  _Type& assign(const C *s, size n);
+
+  int compare(const _Type&) const;
+  int compare(const C* s) const;
+  int compare(size pos, size len, const _Type&) const;
+  int compare(size pos, size len, const C* s) const;
+
+  size find(const _Type& str, size pos = 0) const;
+  size find(const C* s, size pos = 0) const;
+  size find(const C* s, size pos, size n) const;
+
+  _Type& insert(size pos, const _Type& str);
+  _Type& insert(size pos, const C* s);
+  _Type& insert(size pos, const C* s, size n);
+
+  _Type& operator+=(const _Type& str);
+  _Type& operator+=(const C* s);
+  _Type& operator=(const _Type& str);
+  _Type& operator=(const C* s);
 };
+
 typedef basic_string<char, std::char_traits<char>, std::allocator<char>> string;
 typedef basic_string<wchar_t, std::char_traits<wchar_t>, std::allocator<wchar_t>> wstring;
-typedef basic_string<char16_t, std::char_traits<char16_t>, std::allocator<char16_t>> u16string;
-typedef basic_string<char32_t, std::char_traits<char32_t>, std::allocator<char32_t>> u32string;
+typedef basic_string<char16, std::char_traits<char16>, std::allocator<char16>> u16string;
+typedef basic_string<char32, std::char_traits<char32>, std::allocator<char32>> u32string;
 }
 
+std::string operator+(const std::string&, const std::string&);
+std::string operator+(const std::string&, const char*);
+std::string operator+(const char*, const std::string&);
+
+bool operator==(const std::string&, const std::string&);
+bool operator==(const std::string&, const char*);
+bool operator==(const char*, const std::string&);
+
 namespace llvm {
 struct StringRef {
   StringRef(const char *p);
@@ -51,6 +89,80 @@ void f4(const std::string &s) {
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to `c_str()` [readability-redundant-string-cstr]
   // CHECK-FIXES: {{^  }}f1(*ptr);{{$}}
 }
+void f5(const std::string &s) {
+  std::string tmp;
+  tmp.append(s.c_str());
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: redundant call {{.*}}
+  // CHECK-FIXES: {{^  }}tmp.append(s);{{$}}
+  tmp.assign(s.c_str());
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: redundant call {{.*}}
+  // CHECK-FIXES: {{^  }}tmp.assign(s);{{$}}
+
+  if (tmp.compare(s.c_str()) == 0) return;
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: redundant call {{.*}}
+  // CHECK-FIXES: {{^  }}if (tmp.compare(s) == 0) return;{{$}}
+
+  if (tmp.compare(1, 2, s.c_str()) == 0) return;
+  // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: redundant call {{.*}}
+  // CHECK-FIXES: {{^  }}if (tmp.compare(1, 2, s) == 0) return;{{$}}
+
+  if (tmp.find(s.c_str()) == 0) return;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: redundant call {{.*}}
+  // CHECK-FIXES: {{^  }}if (tmp.find(s) == 0) return;{{$}}
+
+  if (tmp.find(s.c_str(), 2) == 0) return;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: redundant call {{.*}}
+  // CHECK-FIXES: {{^  }}if (tmp.find(s, 2) == 0) return;{{$}}
+
+  if (tmp.find(s.c_str(), 2) == 0) return;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: redundant call {{.*}}
+  // CHECK-FIXES: {{^  }}if (tmp.find(s, 2) == 0) return;{{$}}
+
+  tmp.insert(1, s.c_str());
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: redundant call {{.*}}
+  // CHECK-FIXES: {{^  }}tmp.insert(1, s);{{$}}
+
+  tmp = s.c_str();
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: redundant call {{.*}}
+  // CHECK-FIXES: {{^  }}tmp = s;{{$}}
+
+  tmp += s.c_str();
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: redundant call {{.*}}
+  // CHECK-FIXES: {{^  }}tmp += s;{{$}}
+
+  if (tmp == s.c_str()) return;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: redundant call {{.*}}
+  // CHECK-FIXES: {{^  }}if (tmp == s) return;{{$}}
+
+  tmp = s + s.c_str();
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: redundant call {{.*}}
+  // CHECK-FIXES: {{^  }}tmp = s + s;{{$}}
+
+  tmp = s.c_str() + s;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: redundant call {{.*}}
+  // CHECK-FIXES: {{^  }}tmp = s + s;{{$}}
+}
+void f6(const std::string &s) {
+  std::string tmp;
+  tmp.append(s.c_str(), 2);
+  tmp.assign(s.c_str(), 2);
+
+  if (tmp.compare(s) == 0) return;
+  if (tmp.compare(1, 2, s) == 0) return;
+
+  tmp = s;
+  tmp += s;
+
+  if (tmp == s)
+    return;
+
+  tmp = s + s;
+
+  if (tmp.find(s.c_str(), 2, 4) == 0) return;
+
+  tmp.insert(1, s);
+  tmp.insert(1, s.c_str(), 2);
+}
 
 // Tests for std::wstring.