feedback

Author: hjanuschka

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

@@ -30,17 +30,6 @@ 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) {
@@ -82,6 +71,18 @@ struct NotLengthExprForStringNode {
   ASTContext *Context;
 };
 
+static bool isNegativeComparison(const Expr *ComparisonExpr) {
+  // Handle direct != operator
+  if (const auto *BO = llvm::dyn_cast<BinaryOperator>(ComparisonExpr)) {
+    return BO->getOpcode() == BO_NE;
+  }
+
+  if (const auto *Op = llvm::dyn_cast<CXXOperatorCallExpr>(ComparisonExpr)) {
+    return Op->getOperator() == OO_ExclaimEqual;
+  }
+  return false;
+}
+
 AST_MATCHER_P(Expr, lengthExprForStringNode, std::string, ID) {
   return Builder->removeBindings(NotLengthExprForStringNode(
       ID, DynTypedNode::create(Node), &(Finder->getASTContext())));
@@ -186,58 +187,19 @@ void UseStartsEndsWithCheck::registerMatchers(MatchFinder *Finder) {
   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"))))
+          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<Expr>("expr");
   const auto *FindExpr = Result.Nodes.getNodeAs<CXXMemberCallExpr>("find_expr");
@@ -262,10 +224,11 @@ void UseStartsEndsWithCheck::check(const MatchFinder::MatchResult &Result) {
       CharSourceRange::getTokenRange(SearchExpr->getSourceRange()),
       *Result.SourceManager, Result.Context->getLangOpts());
 
-  auto Diag = diag(ComparisonExpr->getBeginLoc(),
-                   "use %0 instead of %1 %select{==|!=}2 ")
-              << ReplacementFunction->getName() << FindFun->getNameAsString()
-              << Neg;
+  auto Diag = diag(FindExpr->getExprLoc(),
+                   FindFun->getName() == "substr"
+                       ? "use %0 instead of %1() %select{==|!=}2"
+                       : "use %0 instead of %1() %select{==|!=}2 0")
+              << ReplacementFunction->getName() << FindFun->getName() << Neg;
 
   // Remove everything before the function call.
   Diag << FixItHint::CreateRemoval(CharSourceRange::getCharRange(

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

@@ -25,7 +25,6 @@ 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

@@ -243,11 +243,9 @@ Changes in existing checks
   ``NULL``/``__null`` (but not ``0``) when used with a templated type.
 
 - 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`` 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.
+	  <clang-tidy/checks/modernize/use-starts-ends-with>` check to handle two new
+	  cases from ``rfind`` and ``compare`` to ``ends_with``, and one new case from
+	  ``substr`` to ``starts_with``.
 
 - Improved :doc:`modernize-use-std-format
   <clang-tidy/checks/modernize/use-std-format>` check to support replacing