clang-tidy: readability-redundant-smartptr-get does not remove (#97964)

[akshaykumars614]

added a check to remove '->' if exists

added testcase and modified Release Notes

pull request in continue of #98757

[llvmbot]

@llvm/pr-subscribers-clang-tools-extra

@llvm/pr-subscribers-clang-tidy

Author: None (akshaykumars614)

Changes

added a check to remove '->' if exists

added testcase and modified Release Notes

pull request in continue of #98757

---

Full diff: https://github.com/llvm/llvm-project/pull/100177.diff

3 Files Affected:

• (modified) clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.cpp (+4)
• (modified) clang-tools-extra/docs/ReleaseNotes.rst (+4)
• (modified) clang-tools-extra/test/clang-tidy/checkers/readability/redundant-smartptr-get.cpp (+72)

diff --git a/clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.cpp
index 8837ac16e8828..be52af77ae0a5 100644
--- a/clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.cpp
@@ -164,6 +164,10 @@ void RedundantSmartptrGetCheck::check(const MatchFinder::MatchResult &Result) {
   StringRef SmartptrText = Lexer::getSourceText(
       CharSourceRange::getTokenRange(Smartptr->getSourceRange()),
       *Result.SourceManager, getLangOpts());
+  // Check if the last two characters are "->" and remove them
+  if (SmartptrText.ends_with("->")) {
+    SmartptrText = SmartptrText.drop_back(2);
+  }
   // Replace foo->get() with *foo, and foo.get() with foo.
   std::string Replacement = Twine(IsPtrToPtr ? "*" : "", SmartptrText).str();
   diag(GetCall->getBeginLoc(), "redundant get() call on smart pointer")
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index a23483e6df6d2..374e89b8226a6 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -498,6 +498,10 @@ Changes in existing checks
   false-positives when type of the member does not match the type of the
   initializer.
 
+- Improved :doc:`readability-redundant-smartptr-get
+  <clang-tidy/checks/readability/redundant-smartptr-get>` check to
+  remove '->' when reduntant get() is removed.
+
 - Improved :doc:`readability-static-accessed-through-instance
   <clang-tidy/checks/readability/static-accessed-through-instance>` check to
   support calls to overloaded operators as base expression and provide fixes to
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-smartptr-get.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-smartptr-get.cpp
index 01f12b6bfe6ea..e9ecc06d53959 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-smartptr-get.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-smartptr-get.cpp
@@ -20,6 +20,47 @@ struct shared_ptr {
   explicit operator bool() const noexcept;
 };
 
+template <typename T>
+struct vector {
+  vector();
+  bool operator==(const vector<T>& other) const;
+  bool operator!=(const vector<T>& other) const;
+  unsigned long size() const;
+  bool empty() const;
+
+  // Basic iterator implementation for testing
+  struct iterator {
+    T* ptr;
+    iterator(T* p) : ptr(p) {}
+    T& operator*() { return *ptr; }
+    T* operator->() { return ptr; }
+    iterator& operator++() {
+      ++ptr;
+      return *this;
+    }
+    bool operator!=(const iterator& other) const { return ptr != other.ptr; }
+  };
+
+  iterator begin();
+  iterator end();
+
+  T* data;
+  unsigned long sz;
+};
+
+template <typename T>
+vector<T>::vector() : data(nullptr), sz(0) {}
+
+template <typename T>
+typename vector<T>::iterator vector<T>::begin() {
+  return iterator(data);
+}
+
+template <typename T>
+typename vector<T>::iterator vector<T>::end() {
+  return iterator(data + sz);
+}
+
 }  // namespace std
 
 struct Bar {
@@ -235,3 +276,34 @@ void Negative() {
   if (MACRO(x) == nullptr)
     ;
 }
+
+void test_redundant_get() {
+  std::vector<std::shared_ptr<int>> v;
+  auto f = [](int) {};
+  for (auto i = v.begin(); i != v.end(); ++i) {
+    f(*i->get());
+    // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: redundant get() call
+    // CHECK-FIXES: f(**i);
+  }
+}
+
+struct Inner {
+  int a;
+  int *getValue()  { return &a; }
+};
+
+struct Example {
+  Inner inner;
+  Inner* get() { return &inner; }
+  int *getValue()  { return inner.getValue(); }
+};
+
+void test_redundant_get_with_member() {
+  std::vector<std::shared_ptr<Example>> v;
+  auto f = [](int) {};
+  for (auto i = v.begin(); i != v.end(); ++i) {
+    f(*(*i).get()->get()->getValue());
+    // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: redundant get() call
+    // CHECK-FIXES: f(**i->get()->getValue());
+ }
+}

[PiotrZSL]

Except release notes, looks fine.

[akshaykumars614]

I made the changes

[PiotrZSL]

still incorrectly rebased code, that stuff shoudn't be here
just rebase your changes on top of main branch, and then do force push, or cleanup this fill manually

[akshaykumars614]

got it. Changed accordingly.

[akshaykumars614]

Did I do mistake again...? I am really sorry if I did. I am new here. I am learning online and doing it. Please correct me @PiotrZSL