[clang-tidy] Add readability-string-view-substr check

clang-tools-extra/clang-tidy/readability/CMakeLists.txt

@@ -53,6 +53,7 @@ add_clang_library(clangTidyReadabilityModule STATIC
   StaticAccessedThroughInstanceCheck.cpp
   StaticDefinitionInAnonymousNamespaceCheck.cpp
   StringCompareCheck.cpp
+  StringViewSubstrCheck.cpp
   SuspiciousCallArgumentCheck.cpp
   UniqueptrDeleteReleaseCheck.cpp
   UppercaseLiteralSuffixCheck.cpp

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

@@ -56,6 +56,7 @@
 #include "StaticAccessedThroughInstanceCheck.h"
 #include "StaticDefinitionInAnonymousNamespaceCheck.h"
 #include "StringCompareCheck.h"
+#include "StringViewSubstrCheck.h"
 #include "SuspiciousCallArgumentCheck.h"
 #include "UniqueptrDeleteReleaseCheck.h"
 #include "UppercaseLiteralSuffixCheck.h"
@@ -146,6 +147,8 @@ class ReadabilityModule : public ClangTidyModule {
         "readability-static-definition-in-anonymous-namespace");
     CheckFactories.registerCheck<StringCompareCheck>(
         "readability-string-compare");
+    CheckFactories.registerCheck<StringViewSubstrCheck>(
+        "readability-stringview-substr");
     CheckFactories.registerCheck<readability::NamedParameterCheck>(
         "readability-named-parameter");
     CheckFactories.registerCheck<NonConstParameterCheck>(

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

@@ -0,0 +1,191 @@
+//===--- StringViewSubstrCheck.cpp - clang-tidy------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "StringViewSubstrCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+void StringViewSubstrCheck::registerMatchers(MatchFinder *Finder) {
+  // Match string_view type
+  const auto StringViewDecl = recordDecl(hasName("::std::basic_string_view"));
+  const auto IsStringView = qualType(
+      hasUnqualifiedDesugaredType(recordType(hasDeclaration(StringViewDecl))));
+
+  // Length/size matcher reused in multiple places
+  const auto LengthMatcher = cxxMemberCallExpr(callee(memberExpr(hasDeclaration(
+      cxxMethodDecl(anyOf(hasName("length"), hasName("size")))))));
+
+  // Match various forms of zero
+  const auto IsZero =
+      expr(anyOf(ignoringParenImpCasts(integerLiteral(equals(0))),
+                 ignoringParenImpCasts(declRefExpr(
+                     to(varDecl(hasInitializer(integerLiteral(equals(0)))))))));
+
+  // Match substr() call patterns
+  const auto SubstrCall =
+      cxxMemberCallExpr(
+          callee(memberExpr(hasDeclaration(cxxMethodDecl(hasName("substr"))))),
+          on(expr(hasType(IsStringView)).bind("source")),
+          // substr always has 2 args (second one may be defaulted)
+          argumentCountIs(2),
+          anyOf(
+              // Case 1: sv.substr(n, npos) -> remove_prefix
+              allOf(hasArgument(0, expr().bind("prefix_n")),
+                    hasArgument(1, cxxDefaultArgExpr())),
+
+              // Case 2: sv.substr(0, sv.length()) or sv.substr(0, sv.length() -
+              // 0) -> redundant self-copy
+              allOf(hasArgument(0, IsZero.bind("zero")),
+                    hasArgument(
+                        1, anyOf(LengthMatcher.bind("full_length"),
+                                 binaryOperator(
+                                     hasOperatorName("-"),
+                                     hasLHS(LengthMatcher.bind("full_length")),
+                                     hasRHS(IsZero))))),
+
+              // Case 3: sv.substr(0, sv.length() - n) -> remove_suffix
+              allOf(hasArgument(0, IsZero),
+                    hasArgument(
+                        1, binaryOperator(
+                               hasOperatorName("-"),
+                               hasLHS(LengthMatcher.bind("length_call")),
+                               hasRHS(expr(unless(IsZero)).bind("suffix_n")))
+                               .bind("length_minus_n")))))
+          .bind("substr_call");
+
+  // Only match assignments not part of larger expressions
+  Finder->addMatcher(
+      stmt(cxxOperatorCallExpr(
+               unless(isInTemplateInstantiation()),
+               hasOverloadedOperatorName("="),
+               hasArgument(0, expr(hasType(IsStringView)).bind("target")),
+               hasArgument(1, SubstrCall))
+               .bind("assignment"),
+           unless(anyOf(hasAncestor(varDecl()), hasAncestor(callExpr())))),
+      this);
+}
+
+void StringViewSubstrCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *Assignment =
+      Result.Nodes.getNodeAs<CXXOperatorCallExpr>("assignment");
+  const auto *Target = Result.Nodes.getNodeAs<Expr>("target");
+  const auto *Source = Result.Nodes.getNodeAs<Expr>("source");
+  const auto *SubstrCall =
+      Result.Nodes.getNodeAs<CXXMemberCallExpr>("substr_call");
+  const auto *PrefixN = Result.Nodes.getNodeAs<Expr>("prefix_n");
+  const auto *FullLength = Result.Nodes.getNodeAs<Expr>("full_length");
+  const auto *LengthCall = Result.Nodes.getNodeAs<Expr>("length_call");
+  const auto *SuffixN = Result.Nodes.getNodeAs<Expr>("suffix_n");
+
+  if (!Assignment || !Target || !Source || !SubstrCall)
+    return;
+
+  const auto *TargetDRE = dyn_cast<DeclRefExpr>(Target->IgnoreParenImpCasts());
+  const auto *SourceDRE = dyn_cast<DeclRefExpr>(Source->IgnoreParenImpCasts());
+
+  if (!TargetDRE || !SourceDRE)
+    return;
+
+  const bool IsSameVar = (TargetDRE->getDecl() == SourceDRE->getDecl());
+  const std::string TargetName = TargetDRE->getNameInfo().getAsString();
+  const std::string SourceName = SourceDRE->getNameInfo().getAsString();
+
+  // Case 1: remove_prefix
+  if (PrefixN) {
+    if (!IsSameVar)
+      return;
+
+    std::string PrefixText =
+        Lexer::getSourceText(
+            CharSourceRange::getTokenRange(PrefixN->getSourceRange()),
+            *Result.SourceManager, Result.Context->getLangOpts())
+            .str();
+
+    std::string Replacement = TargetName + ".remove_prefix(" + PrefixText + ")";
+    diag(Assignment->getBeginLoc(), "prefer 'remove_prefix' over 'substr' for "
+                                    "removing characters from the start")
+        << FixItHint::CreateReplacement(Assignment->getSourceRange(),
+                                        Replacement);
+    return;
+  }
+
+  // Case 2: redundant full copy
+  if (FullLength) {
+    const auto *LengthObject =
+        cast<CXXMemberCallExpr>(FullLength)->getImplicitObjectArgument();
+    const auto *LengthDRE =
+        dyn_cast<DeclRefExpr>(LengthObject->IgnoreParenImpCasts());
+
+    if (!LengthDRE || LengthDRE->getDecl() != SourceDRE->getDecl())
+      return;
+
+    if (IsSameVar) {
+      // Remove redundant self copy including trailing semicolon
+      SourceLocation EndLoc = Lexer::findLocationAfterToken(
+          Assignment->getEndLoc(), tok::semi, *Result.SourceManager,
+          Result.Context->getLangOpts(), false);
+
+      if (EndLoc.isValid()) {
+        diag(Assignment->getBeginLoc(), "redundant self-copy")
+            << FixItHint::CreateRemoval(CharSourceRange::getCharRange(
+                   Assignment->getBeginLoc(), EndLoc));
+      }
+    } else {
+      // Direct copy between different variables
+      std::string Replacement = TargetName + " = " + SourceName;
+      diag(Assignment->getBeginLoc(), "prefer direct copy over substr")
+          << FixItHint::CreateReplacement(Assignment->getSourceRange(),
+                                          Replacement);
+    }
+    return;
+  }
+
+  // Case 3: remove_suffix
+  if (LengthCall && SuffixN) {
+    const auto *LengthObject =
+        cast<CXXMemberCallExpr>(LengthCall)->getImplicitObjectArgument();
+    const auto *LengthDRE =
+        dyn_cast<DeclRefExpr>(LengthObject->IgnoreParenImpCasts());
+
+    if (!LengthDRE || LengthDRE->getDecl() != SourceDRE->getDecl())
+      return;
+
+    std::string SuffixText =
+        Lexer::getSourceText(
+            CharSourceRange::getTokenRange(SuffixN->getSourceRange()),
+            *Result.SourceManager, Result.Context->getLangOpts())
+            .str();
+
+    if (IsSameVar) {
+      std::string Replacement =
+          TargetName + ".remove_suffix(" + SuffixText + ")";
+      diag(Assignment->getBeginLoc(), "prefer 'remove_suffix' over 'substr' "
+                                      "for removing characters from the end")
+          << FixItHint::CreateReplacement(Assignment->getSourceRange(),
+                                          Replacement);
+    } else {
+      std::string Replacement = TargetName + " = " + SourceName + ";\n" + "  " +
+                                TargetName + ".remove_suffix(" + SuffixText +
+                                ")";
+
+      diag(Assignment->getBeginLoc(),
+           "prefer assignment and remove_suffix over substr")
+          << FixItHint::CreateReplacement(Assignment->getSourceRange(),
+                                          Replacement);
+    }
+
+    return;
+  }
+}
+
+} // namespace clang::tidy::readability

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

@@ -0,0 +1,45 @@
+//===--- StringViewSubstrCheck.h - clang-tidy---------------------*- C++
+//-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_STRINGVIEWSUBSTRCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_STRINGVIEWSUBSTRCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::readability {
+
+/// Finds string_view substr() calls that can be replaced with remove_prefix()
+/// or remove_suffix().
+///
+/// For the user-facing documentation see:
+/// https://clang.llvm.org/extra/clang-tidy/checks/readability/string-view-substr.html
+///
+/// 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
+///   sv1 = sv2.substr(0, sv2.length() - N) -> sv1 = sv2; sv1.remove_suffix(N)
+///
+/// These replacements make the intent clearer and are more efficient as they
+/// modify the string_view in place rather than creating a new one.
+class StringViewSubstrCheck : public ClangTidyCheck {
+public:
+  StringViewSubstrCheck(StringRef Name, ClangTidyContext *Context)
+      : 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
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_STRINGVIEWSUBSTRCHECK_H

clang-tools-extra/docs/ReleaseNotes.rst

@@ -136,6 +136,15 @@ New checks
   Gives warnings for tagged unions, where the number of tags is
   different from the number of data members inside the union.
 
+- New :doc:`readability-stringview-substr 
+  <clang-tidy/checks/readability/stringview-substr>` check.
+
+  Finds ``std::string_view::substr()`` calls that can be replaced with clearer 
+  alternatives using ``remove_prefix()``, ``remove_suffix()``, or direct assignment.
+  The suggested transformations make the intent clearer and are more efficient as they 
+  either modify the string_view in place or perform a direct assignment instead of 
+  creating an intermediate substring.
+
 - New :doc:`modernize-use-integer-sign-comparison
   <clang-tidy/checks/modernize/use-integer-sign-comparison>` check.
 

clang-tools-extra/docs/clang-tidy/checks/readability/stringview-substr.rst

@@ -0,0 +1,19 @@
+.. title:: clang-tidy - readability-stringview-substr
+
+readability-stringview-substr
+==============================
+
+Finds ``string_view substr()`` calls that can be replaced with clearer alternatives
+using ``remove_prefix()`` or ``remove_suffix()``.
+
+The check suggests the following transformations:
+
+===========================================  =======================================
+Expression                                   Replacement
+===========================================  =======================================
+``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())``           Redundant self-copy
+``sv1 = sv.substr(0, sv.length())``          ``sv1 = sv``
+``sv1 = sv2.substr(0, sv2.length()-n)``      ``sv1 = sv2;`` ``sv1.remove_suffix(n)``
+===========================================  =======================================