[clang-tools-extra] 9c8bc09 - [clang-tidy] Fix readability-redundant-string-cstr for smart pointer #576705

[Piotr Zegar via cfe-commits]

Author: Mike Crowe
Date: 2023-03-10T13:52:49Z
New Revision: 9c8bc090a0c291efb41cc4b5c51ea0c340961ad1

URL: https://github.com/llvm/llvm-project/commit/9c8bc090a0c291efb41cc4b5c51ea0c340961ad1
DIFF: https://github.com/llvm/llvm-project/commit/9c8bc090a0c291efb41cc4b5c51ea0c340961ad1.diff

LOG: [clang-tidy] Fix readability-redundant-string-cstr for smart pointer #576705

Fix the readability-redundant-string-cstr check to correctly replace
calls to c_str() via an overloaded operator-> (such as from an
iterator.)

Previously, the fix for `i->c_str()` would be `*i->`. Using consume_back
to remove any trailing `->` results in the correct `*i`.

Add some lit check test cases too.

Fixes: https://github.com/llvm/llvm-project/issues/56705

Reviewed By: PiotrZSL

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

Added: 
    

Modified: 
    clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
    clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
index 960d5abcc912e..1b21f70f0bf64 100644
--- a/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
@@ -52,6 +52,10 @@ formatDereference(const ast_matchers::MatchFinder::MatchResult &Result,
 
   if (Text.empty())
     return std::string();
+
+  // Remove remaining '->' from overloaded operator call
+  Text.consume_back("->");
+
   // Add leading '*'.
   if (needParensAfterUnaryOperator(ExprNode)) {
     return (llvm::Twine("*(") + Text + ")").str();

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp
index 2b6d290a4aff6..5dc138e81eeec 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp
@@ -1,6 +1,12 @@
 // RUN: %check_clang_tidy %s readability-redundant-string-cstr %t -- -- -isystem %clang_tidy_headers
 #include <string>
 
+template <typename T>
+struct iterator {
+  T *operator->();
+  T &operator*();
+};
+
 namespace llvm {
 struct StringRef {
   StringRef(const char *p);
@@ -202,6 +208,31 @@ void m1(std::string&&) {
   m1p2(s.c_str());  
 }
 
+// Test for overloaded operator->
+void it(iterator<std::string> i)
+{
+  std::string tmp;
+  tmp = i->c_str();
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+  // CHECK-FIXES: {{^  }}tmp = *i;{{$}}
+
+  // An unlikely situation and the outcome is not ideal, but at least the fix doesn't generate broken code.
+  tmp = i.operator->()->c_str();
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+  // CHECK-FIXES: {{^  }}tmp = *i.operator->();{{$}}
+
+  // The fix contains an unnecessary set of parentheses, but these have no effect.
+  iterator<std::string> *pi = &i;
+  tmp = (*pi)->c_str();
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+  // CHECK-FIXES: {{^  }}tmp = *(*pi);{{$}}
+
+  // An unlikely situation, but at least the fix doesn't generate broken code.
+  tmp = pi->operator->()->c_str();
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+  // CHECK-FIXES: {{^  }}tmp = *pi->operator->();{{$}}
+}
+
 namespace PR45286 {
 struct Foo {
   void func(const std::string &) {}