[clang-tools-extra] [clang-tidy] check `std::string_view` and custom string-like classes in `readability-string-compare` (PR #88636)

Piotr Zegar via cfe-commits cfe-commits at lists.llvm.org
Sun Apr 14 08:36:51 PDT 2024


================
@@ -7,42 +7,70 @@
 //===----------------------------------------------------------------------===//
 
 #include "StringCompareCheck.h"
-#include "../utils/FixItHintUtils.h"
+#include "../utils/OptionsUtils.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Tooling/FixIt.h"
+#include "llvm/ADT/StringRef.h"
+#include <vector>
 
 using namespace clang::ast_matchers;
+namespace optutils = clang::tidy::utils::options;
 
 namespace clang::tidy::readability {
 
 static const StringRef CompareMessage = "do not use 'compare' to test equality "
                                         "of strings; use the string equality "
                                         "operator instead";
 
+static const std::vector<StringRef> StringClasses = {
+    "::std::basic_string", "::std::basic_string_view"};
+
+StringCompareCheck::StringCompareCheck(StringRef Name,
+                                       ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      StringLikeClasses(
+          optutils::parseStringList(Options.get("StringLikeClasses", ""))) {}
+
 void StringCompareCheck::registerMatchers(MatchFinder *Finder) {
-  const auto StrCompare = cxxMemberCallExpr(
-      callee(cxxMethodDecl(hasName("compare"),
-                           ofClass(classTemplateSpecializationDecl(
-                               hasName("::std::basic_string"))))),
-      hasArgument(0, expr().bind("str2")), argumentCountIs(1),
-      callee(memberExpr().bind("str1")));
-
-  // First and second case: cast str.compare(str) to boolean.
-  Finder->addMatcher(
-      traverse(TK_AsIs,
-               implicitCastExpr(hasImplicitDestinationType(booleanType()),
-                                has(StrCompare))
-                   .bind("match1")),
-      this);
-
-  // Third and fourth case: str.compare(str) == 0 and str.compare(str) != 0.
-  Finder->addMatcher(
-      binaryOperator(hasAnyOperatorName("==", "!="),
-                     hasOperands(StrCompare.bind("compare"),
-                                 integerLiteral(equals(0)).bind("zero")))
-          .bind("match2"),
-      this);
+  const auto RegisterForClasses = [&, this](const auto &StringClassMatcher) {
+    const auto StrCompare = cxxMemberCallExpr(
+        callee(cxxMethodDecl(hasName("compare"), ofClass(StringClassMatcher))),
+        hasArgument(0, expr().bind("str2")), argumentCountIs(1),
+        callee(memberExpr().bind("str1")));
+
+    // First and second case: cast str.compare(str) to boolean.
+    Finder->addMatcher(
+        traverse(TK_AsIs,
+                 implicitCastExpr(hasImplicitDestinationType(booleanType()),
+                                  has(StrCompare))
+                     .bind("match1")),
+        this);
+
+    // Third and fourth case: str.compare(str) == 0
+    // and str.compare(str) !=  0.
+    Finder->addMatcher(
+        binaryOperator(hasAnyOperatorName("==", "!="),
+                       hasOperands(StrCompare.bind("compare"),
+                                   integerLiteral(equals(0)).bind("zero")))
+            .bind("match2"),
+        this);
+  };
+  if (StringLikeClasses.empty()) {
+    RegisterForClasses(
+        classTemplateSpecializationDecl(hasAnyName(StringClasses)));
+  } else {
+    // StringLikeClasses may or may not be templates, so we need to match both
+    // template and non-template classes.
+    std::vector<StringRef> PossiblyTemplateClasses = StringClasses;
+    PossiblyTemplateClasses.insert(PossiblyTemplateClasses.end(),
+                                   StringLikeClasses.begin(),
+                                   StringLikeClasses.end());
+    RegisterForClasses(anyOf(
+        classTemplateSpecializationDecl(hasAnyName(PossiblyTemplateClasses)),
+        cxxRecordDecl(hasAnyName(StringLikeClasses))));
----------------
PiotrZSL wrote:

consider using matchesAnyListedName instead of hasAnyName, to support regex.

https://github.com/llvm/llvm-project/pull/88636


More information about the cfe-commits mailing list