[clang-tidy] Enhance modernize-use-starts-ends-with to handle substr patterns

clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.cpp

@@ -30,6 +30,17 @@ struct NotLengthExprForStringNode {
                IntegerLiteralSizeNode->getValue().getZExtValue();
       }
 
+      if (const auto *DeclRefNode = Node.get<DeclRefExpr>()) {
+        if (const auto *VD = dyn_cast<VarDecl>(DeclRefNode->getDecl())) {
+          if (VD->hasInit() && VD->getType().isConstQualified()) {
+            if (const auto *Init = dyn_cast<IntegerLiteral>(VD->getInit())) {
+              return StringLiteralNode->getLength() !=
+                     Init->getValue().getZExtValue();
+            }
+          }
+        }
+      }
+
       if (const auto *StrlenNode = Node.get<CallExpr>()) {
         if (StrlenNode->getDirectCallee()->getName() != "strlen" ||
             StrlenNode->getNumArgs() != 1) {
@@ -171,10 +182,64 @@ void UseStartsEndsWithCheck::registerMatchers(MatchFinder *Finder) {
                              hasRHS(lengthExprForStringNode("needle")))))
           .bind("expr"),
       this);
+
+  Finder->addMatcher(
+      cxxOperatorCallExpr(
+          hasAnyOperatorName("==", "!="),
+          anyOf(
+              hasOperands(
+                  cxxMemberCallExpr(
+                      argumentCountIs(2), hasArgument(0, ZeroLiteral),
+                      hasArgument(1, lengthExprForStringNode("needle")),
+                      callee(
+                          cxxMethodDecl(hasName("substr"),
+                                        ofClass(OnClassWithStartsWithFunction))
+                              .bind("find_fun")))
+                      .bind("find_expr"),
+                  expr().bind("needle")),
+              hasOperands(expr().bind("needle"),
+                          cxxMemberCallExpr(
+                              argumentCountIs(2), hasArgument(0, ZeroLiteral),
+                              hasArgument(1, lengthExprForStringNode("needle")),
+                              callee(cxxMethodDecl(
+                                         hasName("substr"),
+                                         ofClass(OnClassWithStartsWithFunction))
+                                         .bind("find_fun")))
+                              .bind("find_expr"))))
+          .bind("expr"),
+      this);
+}
+
+bool UseStartsEndsWithCheck::isNegativeComparison(const Expr* ComparisonExpr) {
+  // Handle direct != operator
+  if (const auto *BO = llvm::dyn_cast<BinaryOperator>(ComparisonExpr)) {
+    return BO->getOpcode() == BO_NE;
+  }
+
+  // Handle operator!= call
+  if (const auto *Op = llvm::dyn_cast<CXXOperatorCallExpr>(ComparisonExpr)) {
+    return Op->getOperator() == OO_ExclaimEqual;
+  }
+
+  // Handle rewritten !(expr == expr)
+  if (const auto *UO = llvm::dyn_cast<UnaryOperator>(ComparisonExpr)) {
+    if (UO->getOpcode() == UO_LNot) {
+      if (const auto *InnerBO = 
+          llvm::dyn_cast<BinaryOperator>(UO->getSubExpr()->IgnoreParens())) {
+        return InnerBO->getOpcode() == BO_EQ;
+      }
+      if (const auto *InnerOp = 
+          llvm::dyn_cast<CXXOperatorCallExpr>(UO->getSubExpr()->IgnoreParens())) {
+        return InnerOp->getOperator() == OO_EqualEqual;
+      }
+    }
+  }
+
+  return false;
 }
 
 void UseStartsEndsWithCheck::check(const MatchFinder::MatchResult &Result) {
-  const auto *ComparisonExpr = Result.Nodes.getNodeAs<BinaryOperator>("expr");
+  const auto *ComparisonExpr = Result.Nodes.getNodeAs<Expr>("expr");
   const auto *FindExpr = Result.Nodes.getNodeAs<CXXMemberCallExpr>("find_expr");
   const auto *FindFun = Result.Nodes.getNodeAs<CXXMethodDecl>("find_fun");
   const auto *SearchExpr = Result.Nodes.getNodeAs<Expr>("needle");
@@ -183,40 +248,43 @@ void UseStartsEndsWithCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *EndsWithFunction =
       Result.Nodes.getNodeAs<CXXMethodDecl>("ends_with_fun");
   assert(bool(StartsWithFunction) != bool(EndsWithFunction));
+
   const CXXMethodDecl *ReplacementFunction =
       StartsWithFunction ? StartsWithFunction : EndsWithFunction;
 
   if (ComparisonExpr->getBeginLoc().isMacroID())
     return;
 
-  const bool Neg = ComparisonExpr->getOpcode() == BO_NE;
+  bool Neg = isNegativeComparison(ComparisonExpr);
 
-  auto Diagnostic =
-      diag(FindExpr->getExprLoc(), "use %0 instead of %1() %select{==|!=}2 0")
-      << ReplacementFunction->getName() << FindFun->getName() << Neg;
+  // Retrieve the source text of the search expression.
+  const auto SearchExprText = Lexer::getSourceText(
+      CharSourceRange::getTokenRange(SearchExpr->getSourceRange()),
+      *Result.SourceManager, Result.Context->getLangOpts());
 
-  // Remove possible arguments after search expression and ' [!=]= .+' suffix.
-  Diagnostic << FixItHint::CreateReplacement(
-      CharSourceRange::getTokenRange(
-          Lexer::getLocForEndOfToken(SearchExpr->getEndLoc(), 0,
-                                     *Result.SourceManager, getLangOpts()),
-          ComparisonExpr->getEndLoc()),
-      ")");
+  auto Diag = diag(ComparisonExpr->getBeginLoc(),
+                   "use %0 instead of %1 %select{==|!=}2 ")
+              << ReplacementFunction->getName() << FindFun->getNameAsString()
+              << Neg;
 
-  // Remove possible '.+ [!=]= ' prefix.
-  Diagnostic << FixItHint::CreateRemoval(CharSourceRange::getCharRange(
+  // Remove everything before the function call.
+  Diag << FixItHint::CreateRemoval(CharSourceRange::getCharRange(
       ComparisonExpr->getBeginLoc(), FindExpr->getBeginLoc()));
 
-  // Replace method name by '(starts|ends)_with'.
-  // Remove possible arguments before search expression.
-  Diagnostic << FixItHint::CreateReplacement(
-      CharSourceRange::getCharRange(FindExpr->getExprLoc(),
-                                    SearchExpr->getBeginLoc()),
-      (ReplacementFunction->getName() + "(").str());
+  // Rename the function to `starts_with` or `ends_with`.
+  Diag << FixItHint::CreateReplacement(FindExpr->getExprLoc(),
+                                       ReplacementFunction->getName());
 
-  // Add possible negation '!'.
-  if (Neg)
-    Diagnostic << FixItHint::CreateInsertion(FindExpr->getBeginLoc(), "!");
+  // Replace arguments and everything after the function call.
+  Diag << FixItHint::CreateReplacement(
+      CharSourceRange::getTokenRange(FindExpr->getArg(0)->getBeginLoc(),
+                                     ComparisonExpr->getEndLoc()),
+      (SearchExprText + ")").str());
+
+  // Add negation if necessary.
+  if (Neg) {
+    Diag << FixItHint::CreateInsertion(FindExpr->getBeginLoc(), "!");
+  }
 }
 
 } // namespace clang::tidy::modernize

clang-tools-extra/clang-tidy/modernize/UseStartsEndsWithCheck.h

@@ -25,6 +25,7 @@ class UseStartsEndsWithCheck : public ClangTidyCheck {
   UseStartsEndsWithCheck(StringRef Name, ClangTidyContext *Context);
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  bool isNegativeComparison(const Expr* ComparisonExpr);
   bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
     return LangOpts.CPlusPlus;
   }

clang-tools-extra/docs/ReleaseNotes.rst

@@ -244,7 +244,10 @@ Changes in existing checks
 
 - Improved :doc:`modernize-use-starts-ends-with
   <clang-tidy/checks/modernize/use-starts-ends-with>` check to handle two cases
-  that can be replaced with ``ends_with``
+  that can be replaced with ``ends_with`` and detect patterns using ``substr``
+  that can be replaced with ``starts_with``. Now handles cases like
+  ``str.substr(0, n) == "literal"``, with support for length determination through
+  integer literals, ``strlen()``, and ``size()``/``length()`` member functions.
 
 - Improved :doc:`modernize-use-std-format
   <clang-tidy/checks/modernize/use-std-format>` check to support replacing

clang-tools-extra/docs/clang-tidy/checks/modernize/use-starts-ends-with.rst

@@ -7,26 +7,16 @@ Checks for common roundabout ways to express ``starts_with`` and ``ends_with``
 and suggests replacing with the simpler method when it is available. Notably, 
 this will work with ``std::string`` and ``std::string_view``.
 
-.. code-block:: c++
+The check handles the following expressions:
 
-  std::string s = "...";
-  if (s.find("prefix") == 0) { /* do something */ }
-  if (s.rfind("prefix", 0) == 0) { /* do something */ }
-  if (s.compare(0, strlen("prefix"), "prefix") == 0) { /* do something */ }
-  if (s.compare(s.size() - strlen("suffix"), strlen("suffix"), "suffix") == 0) {
-    /* do something */
-  }
-  if (s.rfind("suffix") == (s.length() - 6)) {
-    /* do something */
-  }
-
-becomes
-
-.. code-block:: c++
-
-  std::string s = "...";
-  if (s.starts_with("prefix")) { /* do something */ }
-  if (s.starts_with("prefix")) { /* do something */ }
-  if (s.starts_with("prefix")) { /* do something */ }
-  if (s.ends_with("suffix")) { /* do something */ }
-  if (s.ends_with("suffix")) { /* do something */ }
+==================================================== =====================
+Expression                                           Replacement
+---------------------------------------------------- ---------------------
+``u.find(v) == 0``                                   ``u.starts_with(v)``
+``u.rfind(v, 0) != 0``                               ``!u.starts_with(v)``
+``u.compare(0, v.size(), v) == 0``                   ``u.starts_with(v)``
+``u.substr(0, v.size()) == v``                       ``u.starts_with(v)``
+``v != u.substr(0, v.size())``                       ``!u.starts_with(v)``
+``u.compare(u.size() - v.size(), v.size(), v) == 0`` ``u.ends_with(v)``
+``u.rfind(v) == u.size() - v.size()``                ``u.ends_with(v)``
+==================================================== =====================

clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string

@@ -60,6 +60,8 @@ struct basic_string {
   _Type& insert(size_type pos, const C* s);
   _Type& insert(size_type pos, const C* s, size_type n);
 
+  _Type substr(size_type pos = 0, size_type count = npos) const;
+
   constexpr bool starts_with(std::basic_string_view<C, T> sv) const noexcept;
   constexpr bool starts_with(C ch) const noexcept;
   constexpr bool starts_with(const C* s) const;