[clang-tidy] Handle data() in readability-redundant-string-cstr

Summary:
std::string::data() and std::string::c_str() are equivalent.
Enhance the readability-redundant-string-cstr check to also handle
calls to data().

Reviewers: etienneb, alexfh, aaron.ballman

Subscribers: cfe-commits

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

llvm-svn: 285901

5 files changed, 24 insertions, 10 deletions

diff --git a/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
index ecd173ad964..5b0e4ca265e 100644
--- a/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
@@ -99,7 +99,7 @@ void RedundantStringCStrCheck::registerMatchers(
   const auto StringCStrCallExpr =
       cxxMemberCallExpr(on(StringExpr.bind("arg")),
                         callee(memberExpr().bind("member")),
-                        callee(cxxMethodDecl(hasName("c_str"))))
+                        callee(cxxMethodDecl(hasAnyName("c_str", "data"))))
           .bind("call");
 
   // Detect redundant 'c_str()' calls through a string constructor.
@@ -192,7 +192,8 @@ void RedundantStringCStrCheck::registerMatchers(
 void RedundantStringCStrCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *Call = Result.Nodes.getStmtAs<CallExpr>("call");
   const auto *Arg = Result.Nodes.getStmtAs<Expr>("arg");
-  bool Arrow = Result.Nodes.getStmtAs<MemberExpr>("member")->isArrow();
+  const auto *Member = Result.Nodes.getStmtAs<MemberExpr>("member");
+  bool Arrow = Member->isArrow();
   // Replace the "call" node with the "arg" node, prefixed with '*'
   // if the call was using '->' rather than '.'.
   std::string ArgText =
@@ -200,7 +201,8 @@ void RedundantStringCStrCheck::check(const MatchFinder::MatchResult &Result) {
   if (ArgText.empty())
     return;
 
-  diag(Call->getLocStart(), "redundant call to `c_str()`")
+  diag(Call->getLocStart(), "redundant call to %0")
+      << Member->getMemberDecl()
       << FixItHint::CreateReplacement(Call->getSourceRange(), ArgText);
 }
 
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 2de0975b157..4c54d877216 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -134,6 +134,10 @@ Improvements to clang-tidy
   Flags member initializations that are unnecessary because the same default
   constructor would be called if they were not present.
 
+- The `readability-redundant-string-cstr
+  <http://clang.llvm.org/extra/clang-tidy/checks/readability-redundant-string-cstr.html>`_ check
+  now warns about redundant calls to data() too.
+
 Fixed bugs:
 
 - `modernize-make-unique
diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability-redundant-string-cstr.rst b/clang-tools-extra/docs/clang-tidy/checks/readability-redundant-string-cstr.rst
index 4614c53a227..e6760a41ca3 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/readability-redundant-string-cstr.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/readability-redundant-string-cstr.rst
@@ -4,4 +4,4 @@ readability-redundant-string-cstr
 =================================
 
 
-Finds unnecessary calls to ``std::string::c_str()``.
+Finds unnecessary calls to ``std::string::c_str()`` and ``std::string::data()``.
diff --git a/clang-tools-extra/test/clang-tidy/readability-redundant-string-cstr-msvc.cpp b/clang-tools-extra/test/clang-tidy/readability-redundant-string-cstr-msvc.cpp
index 1c31118dbc6..a6b6b20728c 100644
--- a/clang-tools-extra/test/clang-tidy/readability-redundant-string-cstr-msvc.cpp
+++ b/clang-tools-extra/test/clang-tidy/readability-redundant-string-cstr-msvc.cpp
@@ -12,6 +12,7 @@ struct basic_string {
   basic_string(const C *p);
   basic_string(const C *p, const A &a);
   const C *c_str() const;
+  const C *data() const;
 };
 typedef basic_string<char, std::char_traits<char>, std::allocator<char>> string;
 }
@@ -24,7 +25,10 @@ struct StringRef {
 
 void f1(const std::string &s) {
   f1(s.c_str());
-  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to `c_str()` [readability-redundant-string-cstr]
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+  // CHECK-FIXES: {{^  }}f1(s);{{$}}
+  f1(s.data());
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to 'data' [readability-redundant-string-cstr]
   // CHECK-FIXES: {{^  }}f1(s);{{$}}
 }
 void f2(const llvm::StringRef r) {
diff --git a/clang-tools-extra/test/clang-tidy/readability-redundant-string-cstr.cpp b/clang-tools-extra/test/clang-tidy/readability-redundant-string-cstr.cpp
index 009168f1550..976a54f7f3a 100644
--- a/clang-tools-extra/test/clang-tidy/readability-redundant-string-cstr.cpp
+++ b/clang-tools-extra/test/clang-tidy/readability-redundant-string-cstr.cpp
@@ -16,6 +16,7 @@ struct basic_string {
   basic_string(const C *p, const A &a = A());
 
   const C *c_str() const;
+  const C *data() const;
 
   _Type& append(const C *s);
   _Type& append(const C *s, size n);
@@ -66,7 +67,10 @@ struct StringRef {
 
 void f1(const std::string &s) {
   f1(s.c_str());
-  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to `c_str()` [readability-redundant-string-cstr]
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+  // CHECK-FIXES: {{^  }}f1(s);{{$}}
+  f1(s.data());
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to 'data' [readability-redundant-string-cstr]
   // CHECK-FIXES: {{^  }}f1(s);{{$}}
 }
 void f2(const llvm::StringRef r) {
@@ -86,7 +90,7 @@ void f3(const llvm::StringRef &r) {
 void f4(const std::string &s) {
   const std::string* ptr = &s;
   f1(ptr->c_str());
-  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to `c_str()` [readability-redundant-string-cstr]
+  // 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) {
@@ -168,7 +172,7 @@ void f6(const std::string &s) {
 
 void g1(const std::wstring &s) {
   g1(s.c_str());
-  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to `c_str()` [readability-redundant-string-cstr]
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
   // CHECK-FIXES: {{^  }}g1(s);{{$}}
 }
 
@@ -176,7 +180,7 @@ void g1(const std::wstring &s) {
 
 void h1(const std::u16string &s) {
   h1(s.c_str());
-  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to `c_str()` [readability-redundant-string-cstr]
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
   // CHECK-FIXES: {{^  }}h1(s);{{$}}
 }
 
@@ -184,7 +188,7 @@ void h1(const std::u16string &s) {
 
 void k1(const std::u32string &s) {
   k1(s.c_str());
-  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to `c_str()` [readability-redundant-string-cstr]
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
   // CHECK-FIXES: {{^  }}k1(s);{{$}}
 }