feedback

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

@@ -108,7 +108,7 @@ void StringViewSubstrCheck::check(const MatchFinder::MatchResult &Result) {
         if (const auto *LengthCall = dyn_cast<CXXMemberCallExpr>(LHS)) {
           if (const auto *LengthMethod =
                   dyn_cast<CXXMethodDecl>(LengthCall->getDirectCallee())) {
-            if (LengthMethod->getName() == "length") {
+            if (LengthMethod->getName() == "length" || LengthMethod->getName() == "size") {
               const Expr *LengthObject =
                   LengthCall->getImplicitObjectArgument();
               const auto *LengthDRE =
@@ -151,10 +151,10 @@ void StringViewSubstrCheck::check(const MatchFinder::MatchResult &Result) {
       }
     } else if (const auto *LengthCall =
                    dyn_cast<CXXMemberCallExpr>(LengthArg)) {
-      // Handle direct length() call
+      // Handle direct length() or size() call
       if (const auto *LengthMethod =
               dyn_cast<CXXMethodDecl>(LengthCall->getDirectCallee())) {
-        if (LengthMethod->getName() == "length") {
+        if (LengthMethod->getName() == "length" || LengthMethod->getName() == "size") {
           const Expr *LengthObject = LengthCall->getImplicitObjectArgument();
           const auto *LengthDRE =
               dyn_cast<DeclRefExpr>(LengthObject->IgnoreParenImpCasts());

clang-tools-extra/clang-tidy/readability/StringViewSubstrCheck.h

@@ -23,6 +23,8 @@ namespace clang::tidy::readability {
 /// The check matches two patterns:
 ///   sv = sv.substr(N) -> sv.remove_prefix(N)
 ///   sv = sv.substr(0, sv.length() - N) -> sv.remove_suffix(N)
+///   sv = sv.substr(0, sv.length()) -> // Remove redundant self-copy
+///   sv1 = sv2.substr(0, sv2.length()) -> sv1 = sv2
 ///
 /// These replacements make the intent clearer and are more efficient as they
 /// modify the string_view in place rather than creating a new one.
@@ -32,6 +34,9 @@ class StringViewSubstrCheck : public ClangTidyCheck {
       : ClangTidyCheck(Name, Context) {}
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+    return LangOpts.CPlusPlus17;
+  }
 };
 
 } // namespace clang::tidy::readability

clang-tools-extra/test/clang-tidy/checkers/readability/stringview_substr.cpp

@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s readability-stringview-substr %t
+// RUN: %check_clang_tidy %s readability-stringview-substr -std=c++17 %t
 
 namespace std {
 template <typename T>
@@ -12,6 +12,7 @@ class basic_string_view {
   void remove_prefix(size_type n) {}
   void remove_suffix(size_type n) {}
   size_type length() const { return 0; }
+  size_type size() const { return 0; }
   basic_string_view& operator=(const basic_string_view&) { return *this; }
 };
 
@@ -39,6 +40,28 @@ void test_basic() {
   // CHECK-FIXES: sv.remove_suffix((3 + 2))
 }
 
+void test_size_method() {
+  std::string_view sv("test");
+  std::string_view sv2("test");
+
+  // Should match: remove_suffix with size()
+  sv = sv.substr(0, sv.size() - 3);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer 'remove_suffix' over 'substr' for removing characters from the end [readability-stringview-substr]
+  // CHECK-FIXES: sv.remove_suffix(3)
+
+  // Should match: remove redundant self copies
+  sv = sv.substr(0, sv.size());
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: redundant self-copy [readability-stringview-substr]
+
+  sv = sv.substr(0, sv.size() - 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: redundant self-copy [readability-stringview-substr]
+
+  // Should match: simplify copies between different variables 
+  sv2 = sv.substr(0, sv.size());
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer direct copy over substr [readability-stringview-substr]
+  // CHECK-FIXES: sv2 = sv
+}
+
 void test_copies() {
   std::string_view sv("test");
   std::string_view sv1("test");