[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]

https://github.com/PiotrZSL closed 
https://github.com/llvm/llvm-project/pull/88636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Vadim D. via cfe-commits]

vvd170501 wrote:

@EugeneZelenko, can you approve this, please? Or are 2 approves enough to merge?

https://github.com/llvm/llvm-project/pull/88636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Congcong Cai via cfe-commits]

https://github.com/HerrCai0907 approved this pull request.


https://github.com/llvm/llvm-project/pull/88636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Vadim D. via cfe-commits]

vvd170501 wrote:

@PiotrZSL, can you merge this, please?
I've fixed merge conflicts caused by ef5906989ae2004100ff56dc5ab59be2be9d5c99 
(changes in the string header for tests), now everything should be ok

https://github.com/llvm/llvm-project/pull/88636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Vadim D. via cfe-commits]

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

>From 54b68bdb6cdc6bdd81178e2abd78d211b6b7181a Mon Sep 17 00:00:00 2001
From: Vadim Dudkin 
Date: Sat, 13 Apr 2024 23:36:12 +0300
Subject: [PATCH] readability-string-compare: check std::string_view, allow
 custom string-like classes

Co-authored-by: EugeneZelenko 
---
 .../readability/StringCompareCheck.cpp| 27 --
 .../readability/StringCompareCheck.h  | 10 -
 clang-tools-extra/docs/ReleaseNotes.rst   |  5 +++
 .../checks/readability/string-compare.rst | 37 ++-
 .../clang-tidy/checkers/Inputs/Headers/string | 10 +
 .../string-compare-custom-string-classes.cpp  | 35 ++
 .../checkers/readability/string-compare.cpp   | 23 
 7 files changed, 139 insertions(+), 8 deletions(-)
 create mode 100644 
clang-tools-extra/test/clang-tidy/checkers/readability/string-compare-custom-string-classes.cpp

diff --git a/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp
index 3b5d89c8c64719..7c0bbef3ca0878 100644
--- a/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp
@@ -7,12 +7,15 @@
 
//===--===//
 
 #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"
 
 using namespace clang::ast_matchers;
+namespace optutils = clang::tidy::utils::options;
 
 namespace clang::tidy::readability {
 
@@ -20,11 +23,27 @@ static const StringRef CompareMessage = "do not use 
'compare' to test equality "
 "of strings; use the string equality "
 "operator instead";
 
+static const StringRef DefaultStringLikeClasses = "::std::basic_string;"
+  "::std::basic_string_view";
+
+StringCompareCheck::StringCompareCheck(StringRef Name,
+   ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  StringLikeClasses(optutils::parseStringList(
+  Options.get("StringLikeClasses", DefaultStringLikeClasses))) {}
+
+void StringCompareCheck::storeOptions(ClangTidyOptions::OptionMap ) {
+  Options.store(Opts, "StringLikeClasses",
+optutils::serializeStringList(StringLikeClasses));
+}
+
 void StringCompareCheck::registerMatchers(MatchFinder *Finder) {
+  if (StringLikeClasses.empty()) {
+return;
+  }
   const auto StrCompare = cxxMemberCallExpr(
-  callee(cxxMethodDecl(hasName("compare"),
-   ofClass(classTemplateSpecializationDecl(
-   hasName("::std::basic_string"),
+  callee(cxxMethodDecl(hasName("compare"), 
ofClass(cxxRecordDecl(hasAnyName(
+   StringLikeClasses),
   hasArgument(0, expr().bind("str2")), argumentCountIs(1),
   callee(memberExpr().bind("str1")));
 
diff --git a/clang-tools-extra/clang-tidy/readability/StringCompareCheck.h 
b/clang-tools-extra/clang-tidy/readability/StringCompareCheck.h
index 812736d806b71d..150090901a6e97 100644
--- a/clang-tools-extra/clang-tidy/readability/StringCompareCheck.h
+++ b/clang-tools-extra/clang-tidy/readability/StringCompareCheck.h
@@ -10,6 +10,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_STRINGCOMPARECHECK_H
 
 #include "../ClangTidyCheck.h"
+#include 
 
 namespace clang::tidy::readability {
 
@@ -20,13 +21,18 @@ namespace clang::tidy::readability {
 /// 
http://clang.llvm.org/extra/clang-tidy/checks/readability/string-compare.html
 class StringCompareCheck : public ClangTidyCheck {
 public:
-  StringCompareCheck(StringRef Name, ClangTidyContext *Context)
-  : ClangTidyCheck(Name, Context) {}
+  StringCompareCheck(StringRef Name, ClangTidyContext *Context);
+
   bool isLanguageVersionSupported(const LangOptions ) const override {
 return LangOpts.CPlusPlus;
   }
+
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult ) override;
+  void storeOptions(ClangTidyOptions::OptionMap ) override;
+
+private:
+  const std::vector StringLikeClasses;
 };
 
 } // namespace clang::tidy::readability
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst 
b/clang-tools-extra/docs/ReleaseNotes.rst
index 5b1feffb89ea06..aa8b4a63b60946 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -314,6 +314,11 @@ Changes in existing checks
   check by resolving fix-it overlaps in template code by disregarding implicit
   instances.

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

[Vadim D. via cfe-commits]

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

>From ab87ad179f385934e2ec17517b47da33d29eb256 Mon Sep 17 00:00:00 2001
From: Vadim Dudkin 
Date: Sat, 13 Apr 2024 23:36:12 +0300
Subject: [PATCH] readability-string-compare: check std::string_view, allow
 custom string-like classes

---
 .../readability/StringCompareCheck.cpp| 27 --
 .../readability/StringCompareCheck.h  | 10 -
 clang-tools-extra/docs/ReleaseNotes.rst   |  5 +++
 .../checks/readability/string-compare.rst | 37 ++-
 .../clang-tidy/checkers/Inputs/Headers/string | 10 +
 .../string-compare-custom-string-classes.cpp  | 35 ++
 .../checkers/readability/string-compare.cpp   | 23 
 7 files changed, 139 insertions(+), 8 deletions(-)
 create mode 100644 
clang-tools-extra/test/clang-tidy/checkers/readability/string-compare-custom-string-classes.cpp

diff --git a/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp
index 3b5d89c8c64719..7c0bbef3ca0878 100644
--- a/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp
@@ -7,12 +7,15 @@
 
//===--===//
 
 #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"
 
 using namespace clang::ast_matchers;
+namespace optutils = clang::tidy::utils::options;
 
 namespace clang::tidy::readability {
 
@@ -20,11 +23,27 @@ static const StringRef CompareMessage = "do not use 
'compare' to test equality "
 "of strings; use the string equality "
 "operator instead";
 
+static const StringRef DefaultStringLikeClasses = "::std::basic_string;"
+  "::std::basic_string_view";
+
+StringCompareCheck::StringCompareCheck(StringRef Name,
+   ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  StringLikeClasses(optutils::parseStringList(
+  Options.get("StringLikeClasses", DefaultStringLikeClasses))) {}
+
+void StringCompareCheck::storeOptions(ClangTidyOptions::OptionMap ) {
+  Options.store(Opts, "StringLikeClasses",
+optutils::serializeStringList(StringLikeClasses));
+}
+
 void StringCompareCheck::registerMatchers(MatchFinder *Finder) {
+  if (StringLikeClasses.empty()) {
+return;
+  }
   const auto StrCompare = cxxMemberCallExpr(
-  callee(cxxMethodDecl(hasName("compare"),
-   ofClass(classTemplateSpecializationDecl(
-   hasName("::std::basic_string"),
+  callee(cxxMethodDecl(hasName("compare"), 
ofClass(cxxRecordDecl(hasAnyName(
+   StringLikeClasses),
   hasArgument(0, expr().bind("str2")), argumentCountIs(1),
   callee(memberExpr().bind("str1")));
 
diff --git a/clang-tools-extra/clang-tidy/readability/StringCompareCheck.h 
b/clang-tools-extra/clang-tidy/readability/StringCompareCheck.h
index 812736d806b71d..150090901a6e97 100644
--- a/clang-tools-extra/clang-tidy/readability/StringCompareCheck.h
+++ b/clang-tools-extra/clang-tidy/readability/StringCompareCheck.h
@@ -10,6 +10,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_STRINGCOMPARECHECK_H
 
 #include "../ClangTidyCheck.h"
+#include 
 
 namespace clang::tidy::readability {
 
@@ -20,13 +21,18 @@ namespace clang::tidy::readability {
 /// 
http://clang.llvm.org/extra/clang-tidy/checks/readability/string-compare.html
 class StringCompareCheck : public ClangTidyCheck {
 public:
-  StringCompareCheck(StringRef Name, ClangTidyContext *Context)
-  : ClangTidyCheck(Name, Context) {}
+  StringCompareCheck(StringRef Name, ClangTidyContext *Context);
+
   bool isLanguageVersionSupported(const LangOptions ) const override {
 return LangOpts.CPlusPlus;
   }
+
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult ) override;
+  void storeOptions(ClangTidyOptions::OptionMap ) override;
+
+private:
+  const std::vector StringLikeClasses;
 };
 
 } // namespace clang::tidy::readability
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst 
b/clang-tools-extra/docs/ReleaseNotes.rst
index 5b1feffb89ea06..aa8b4a63b60946 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -314,6 +314,11 @@ Changes in existing checks
   check by resolving fix-it overlaps in template code by disregarding implicit
   instances.
 
+- Improved 

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

[Vadim D. via cfe-commits]

https://github.com/vvd170501 edited 
https://github.com/llvm/llvm-project/pull/88636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Vadim D. via cfe-commits]

@@ -50,5 +52,36 @@ Examples:
   }
 
 The above code examples show the list of if-statements that this check will
-give a warning for. All of them uses ``compare`` to check if equality or
+give a warning for. All of them use ``compare`` to check equality or
 inequality of two strings instead of using the correct operators.
+
+Options
+---
+
+.. option:: StringLikeClasses
+
+   A string containing semicolon-separated names of string-like classes.
+   By default contains only ``::std::basic_string``
+   and ``::std::basic_string_view``. If a class from this list has
+   a ``compare`` method similar to that of ``std::string``, it will be checked
+   in the same way.
+
+Example
+^^^
+
+.. code-block:: c++
+
+  struct CustomString {
+  public:
+int compare (const CustomString& other) const;
+  }
+
+  CustomString str1;
+  CustomString str2;
+
+  // use str1 != str2 instead.
+  if (str1.compare(str2)) {
+  }
+
+If `StringLikeClasses` contains ``CustomString``, the check will suggest

vvd170501 wrote:

Shouldn't `CustomString` be rendered as code here?

https://github.com/llvm/llvm-project/pull/88636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Vadim D. via cfe-commits]

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

>From 0db24a6806e9429b5e7b9cd9d0777315b3e6d87e Mon Sep 17 00:00:00 2001
From: Vadim Dudkin 
Date: Sat, 13 Apr 2024 23:36:12 +0300
Subject: [PATCH 1/8] readability-string-compare: check std::string_view, allow
 custom string-like classes

---
 .../readability/StringCompareCheck.cpp| 74 +--
 .../readability/StringCompareCheck.h  |  9 ++-
 clang-tools-extra/docs/ReleaseNotes.rst   |  5 ++
 .../checks/readability/string-compare.rst | 33 -
 .../clang-tidy/checkers/Inputs/Headers/string | 10 +++
 .../string-compare-custom-string-classes.cpp  | 38 ++
 .../checkers/readability/string-compare.cpp   | 23 ++
 7 files changed, 165 insertions(+), 27 deletions(-)
 create mode 100644 
clang-tools-extra/test/clang-tidy/checkers/readability/string-compare-custom-string-classes.cpp

diff --git a/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp
index 3b5d89c8c64719..905e5b156ef864 100644
--- a/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp
@@ -7,12 +7,16 @@
 
//===--===//
 
 #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 
 
 using namespace clang::ast_matchers;
+namespace optutils = clang::tidy::utils::options;
 
 namespace clang::tidy::readability {
 
@@ -20,29 +24,53 @@ static const StringRef CompareMessage = "do not use 
'compare' to test equality "
 "of strings; use the string equality "
 "operator instead";
 
+static const std::vector 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 ) {
+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 PossiblyTemplateClasses = StringClasses;
+PossiblyTemplateClasses.insert(PossiblyTemplateClasses.end(),
+   StringLikeClasses.begin(),
+   StringLikeClasses.end());
+RegisterForClasses(anyOf(
+

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

[Vadim D. via cfe-commits]

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

>From 0db24a6806e9429b5e7b9cd9d0777315b3e6d87e Mon Sep 17 00:00:00 2001
From: Vadim Dudkin 
Date: Sat, 13 Apr 2024 23:36:12 +0300
Subject: [PATCH 1/7] readability-string-compare: check std::string_view, allow
 custom string-like classes

---
 .../readability/StringCompareCheck.cpp| 74 +--
 .../readability/StringCompareCheck.h  |  9 ++-
 clang-tools-extra/docs/ReleaseNotes.rst   |  5 ++
 .../checks/readability/string-compare.rst | 33 -
 .../clang-tidy/checkers/Inputs/Headers/string | 10 +++
 .../string-compare-custom-string-classes.cpp  | 38 ++
 .../checkers/readability/string-compare.cpp   | 23 ++
 7 files changed, 165 insertions(+), 27 deletions(-)
 create mode 100644 
clang-tools-extra/test/clang-tidy/checkers/readability/string-compare-custom-string-classes.cpp

diff --git a/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp
index 3b5d89c8c64719..905e5b156ef864 100644
--- a/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp
@@ -7,12 +7,16 @@
 
//===--===//
 
 #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 
 
 using namespace clang::ast_matchers;
+namespace optutils = clang::tidy::utils::options;
 
 namespace clang::tidy::readability {
 
@@ -20,29 +24,53 @@ static const StringRef CompareMessage = "do not use 
'compare' to test equality "
 "of strings; use the string equality "
 "operator instead";
 
+static const std::vector 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 ) {
+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 PossiblyTemplateClasses = StringClasses;
+PossiblyTemplateClasses.insert(PossiblyTemplateClasses.end(),
+   StringLikeClasses.begin(),
+   StringLikeClasses.end());
+RegisterForClasses(anyOf(
+

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

[via cfe-commits]

@@ -50,5 +52,36 @@ Examples:
   }
 
 The above code examples show the list of if-statements that this check will
-give a warning for. All of them uses ``compare`` to check if equality or
+give a warning for. All of them use ``compare`` to check equality or
 inequality of two strings instead of using the correct operators.
+
+Options
+---
+
+.. option:: StringLikeClasses
+
+   A string containing semicolon-separated names of string-like classes.
+   By default contains only ``::std::basic_string``
+   and ``::std::basic_string_view``. If a class from this list has
+   a ``compare`` method similar to that of ``std::string``, it will be checked
+   in the same way.
+
+Example
+^^^
+
+.. code-block:: c++
+
+  struct CustomString {
+  public:
+int compare (const CustomString& other) const;
+  }
+
+  CustomString str1;
+  CustomString str2;
+
+  // use str1 != str2 instead.
+  if (str1.compare(str2)) {
+  }
+
+If `StringLikeClasses` contains ``CustomString``, the check will suggest

EugeneZelenko wrote:

```suggestion
If `StringLikeClasses` contains `CustomString`, the check will suggest
```

https://github.com/llvm/llvm-project/pull/88636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[via cfe-commits]

@@ -286,6 +286,11 @@ Changes in existing checks
   check by resolving fix-it overlaps in template code by disregarding implicit
   instances.
 
+- Improved :doc:`readability-string-compare
+  ` check to also detect
+  usages of `std::string_view::compare`. Added a `StringLikeClasses` option

EugeneZelenko wrote:

```suggestion
  usages of ``std::string_view::compare``. Added a `StringLikeClasses` option
```

https://github.com/llvm/llvm-project/pull/88636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[via cfe-commits]

@@ -286,6 +286,11 @@ Changes in existing checks
   check by resolving fix-it overlaps in template code by disregarding implicit
   instances.
 
+- Improved :doc:`readability-string-compare
+  ` check to also detect
+  usages of `std::string_view::compare`. Added a `StringLikeClasses` option
+  to detect usages of `compare` method in custom string-like classes.

EugeneZelenko wrote:

```suggestion
  to detect usages of ``compare`` method in custom string-like classes.
```

https://github.com/llvm/llvm-project/pull/88636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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]

@@ -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 
 
 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 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 ) {
+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 PossiblyTemplateClasses = StringClasses;
+PossiblyTemplateClasses.insert(PossiblyTemplateClasses.end(),
+   StringLikeClasses.begin(),
+   StringLikeClasses.end());
+RegisterForClasses(anyOf(
+classTemplateSpecializationDecl(hasAnyName(PossiblyTemplateClasses)),
+cxxRecordDecl(hasAnyName(StringLikeClasses;

PiotrZSL wrote:

OK, personally I would prefer regexp, but for now can be like this.

https://github.com/llvm/llvm-project/pull/88636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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]

https://github.com/PiotrZSL approved this pull request.

Looks fine for me.

https://github.com/llvm/llvm-project/pull/88636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Vadim D. via cfe-commits]

vvd170501 wrote:

Ping.
@PiotrZSL, I've added the changes you requested (except 
`matchesAnyListedName`), waiting for your feedback

https://github.com/llvm/llvm-project/pull/88636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Vadim D. via cfe-commits]

https://github.com/vvd170501 edited 
https://github.com/llvm/llvm-project/pull/88636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Vadim D. via cfe-commits]

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

>From 0db24a6806e9429b5e7b9cd9d0777315b3e6d87e Mon Sep 17 00:00:00 2001
From: Vadim Dudkin 
Date: Sat, 13 Apr 2024 23:36:12 +0300
Subject: [PATCH 1/6] readability-string-compare: check std::string_view, allow
 custom string-like classes

---
 .../readability/StringCompareCheck.cpp| 74 +--
 .../readability/StringCompareCheck.h  |  9 ++-
 clang-tools-extra/docs/ReleaseNotes.rst   |  5 ++
 .../checks/readability/string-compare.rst | 33 -
 .../clang-tidy/checkers/Inputs/Headers/string | 10 +++
 .../string-compare-custom-string-classes.cpp  | 38 ++
 .../checkers/readability/string-compare.cpp   | 23 ++
 7 files changed, 165 insertions(+), 27 deletions(-)
 create mode 100644 
clang-tools-extra/test/clang-tidy/checkers/readability/string-compare-custom-string-classes.cpp

diff --git a/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp
index 3b5d89c8c64719..905e5b156ef864 100644
--- a/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp
@@ -7,12 +7,16 @@
 
//===--===//
 
 #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 
 
 using namespace clang::ast_matchers;
+namespace optutils = clang::tidy::utils::options;
 
 namespace clang::tidy::readability {
 
@@ -20,29 +24,53 @@ static const StringRef CompareMessage = "do not use 
'compare' to test equality "
 "of strings; use the string equality "
 "operator instead";
 
+static const std::vector 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 ) {
+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 PossiblyTemplateClasses = StringClasses;
+PossiblyTemplateClasses.insert(PossiblyTemplateClasses.end(),
+   StringLikeClasses.begin(),
+   StringLikeClasses.end());
+RegisterForClasses(anyOf(
+

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

[Vadim D. via cfe-commits]

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

>From 0db24a6806e9429b5e7b9cd9d0777315b3e6d87e Mon Sep 17 00:00:00 2001
From: Vadim Dudkin 
Date: Sat, 13 Apr 2024 23:36:12 +0300
Subject: [PATCH 1/5] readability-string-compare: check std::string_view, allow
 custom string-like classes

---
 .../readability/StringCompareCheck.cpp| 74 +--
 .../readability/StringCompareCheck.h  |  9 ++-
 clang-tools-extra/docs/ReleaseNotes.rst   |  5 ++
 .../checks/readability/string-compare.rst | 33 -
 .../clang-tidy/checkers/Inputs/Headers/string | 10 +++
 .../string-compare-custom-string-classes.cpp  | 38 ++
 .../checkers/readability/string-compare.cpp   | 23 ++
 7 files changed, 165 insertions(+), 27 deletions(-)
 create mode 100644 
clang-tools-extra/test/clang-tidy/checkers/readability/string-compare-custom-string-classes.cpp

diff --git a/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp
index 3b5d89c8c64719..905e5b156ef864 100644
--- a/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp
@@ -7,12 +7,16 @@
 
//===--===//
 
 #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 
 
 using namespace clang::ast_matchers;
+namespace optutils = clang::tidy::utils::options;
 
 namespace clang::tidy::readability {
 
@@ -20,29 +24,53 @@ static const StringRef CompareMessage = "do not use 
'compare' to test equality "
 "of strings; use the string equality "
 "operator instead";
 
+static const std::vector 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 ) {
+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 PossiblyTemplateClasses = StringClasses;
+PossiblyTemplateClasses.insert(PossiblyTemplateClasses.end(),
+   StringLikeClasses.begin(),
+   StringLikeClasses.end());
+RegisterForClasses(anyOf(
+

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

[Vadim D. via cfe-commits]

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

>From 0db24a6806e9429b5e7b9cd9d0777315b3e6d87e Mon Sep 17 00:00:00 2001
From: Vadim Dudkin 
Date: Sat, 13 Apr 2024 23:36:12 +0300
Subject: [PATCH 1/4] readability-string-compare: check std::string_view, allow
 custom string-like classes

---
 .../readability/StringCompareCheck.cpp| 74 +--
 .../readability/StringCompareCheck.h  |  9 ++-
 clang-tools-extra/docs/ReleaseNotes.rst   |  5 ++
 .../checks/readability/string-compare.rst | 33 -
 .../clang-tidy/checkers/Inputs/Headers/string | 10 +++
 .../string-compare-custom-string-classes.cpp  | 38 ++
 .../checkers/readability/string-compare.cpp   | 23 ++
 7 files changed, 165 insertions(+), 27 deletions(-)
 create mode 100644 
clang-tools-extra/test/clang-tidy/checkers/readability/string-compare-custom-string-classes.cpp

diff --git a/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp
index 3b5d89c8c64719..905e5b156ef864 100644
--- a/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp
@@ -7,12 +7,16 @@
 
//===--===//
 
 #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 
 
 using namespace clang::ast_matchers;
+namespace optutils = clang::tidy::utils::options;
 
 namespace clang::tidy::readability {
 
@@ -20,29 +24,53 @@ static const StringRef CompareMessage = "do not use 
'compare' to test equality "
 "of strings; use the string equality "
 "operator instead";
 
+static const std::vector 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 ) {
+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 PossiblyTemplateClasses = StringClasses;
+PossiblyTemplateClasses.insert(PossiblyTemplateClasses.end(),
+   StringLikeClasses.begin(),
+   StringLikeClasses.end());
+RegisterForClasses(anyOf(
+

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

[Vadim D. via cfe-commits]

https://github.com/vvd170501 edited 
https://github.com/llvm/llvm-project/pull/88636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Vadim D. via cfe-commits]

https://github.com/vvd170501 edited 
https://github.com/llvm/llvm-project/pull/88636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Vadim D. via cfe-commits]

@@ -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 
 
 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 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 ) {
+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 PossiblyTemplateClasses = StringClasses;
+PossiblyTemplateClasses.insert(PossiblyTemplateClasses.end(),
+   StringLikeClasses.begin(),
+   StringLikeClasses.end());
+RegisterForClasses(anyOf(
+classTemplateSpecializationDecl(hasAnyName(PossiblyTemplateClasses)),
+cxxRecordDecl(hasAnyName(StringLikeClasses;

vvd170501 wrote:

But would regex be useful here?
- Usually there would be a few (1-2) custom string-like classes, if any, and it 
shouldn't be hard to just put their names into the list.
- The defaults would be less readable (`^::std::string$;^::std::string_view$;` 
or `^::std::string(_view)?$` vs `::std::string;::std::string_view`)

Also, existing checks with a `StringLikeClasses` use `hasAnyName`

https://github.com/llvm/llvm-project/pull/88636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Vadim D. via cfe-commits]

https://github.com/vvd170501 edited 
https://github.com/llvm/llvm-project/pull/88636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Vadim D. via cfe-commits]

https://github.com/vvd170501 edited 
https://github.com/llvm/llvm-project/pull/88636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Vadim D. via cfe-commits]

https://github.com/vvd170501 edited 
https://github.com/llvm/llvm-project/pull/88636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Vadim D. via cfe-commits]

@@ -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 
 
 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 StringClasses = {
+"::std::basic_string", "::std::basic_string_view"};
+
+StringCompareCheck::StringCompareCheck(StringRef Name,
+   ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  StringLikeClasses(
+  optutils::parseStringList(Options.get("StringLikeClasses", ""))) {}

vvd170501 wrote:

> put StringClasses as default

I thought about implementing the option this way, but it seems to me it'd be a 
bit harder to use than a list of additional classes.
if the user wanted to add custom classes, they'd need to also include 
`std::string` and `std::string_view` in the list to keep the check enabled for 
these classes.
So, I decided to append custom classes to the default list instead of 
overwriting it.

> Or call it like "AdditionalStringLikeClasses"

Ok.

> but still have common handling

Could you elaborate? I'm not sure what you mean by "common handling". Removing 
the lambda?

https://github.com/llvm/llvm-project/pull/88636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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]

@@ -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 
 
 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 StringClasses = {
+"::std::basic_string", "::std::basic_string_view"};
+
+StringCompareCheck::StringCompareCheck(StringRef Name,
+   ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  StringLikeClasses(
+  optutils::parseStringList(Options.get("StringLikeClasses", ""))) {}

PiotrZSL wrote:

Or call it like "AdditionalStringLikeClasses", but still have common handling.

https://github.com/llvm/llvm-project/pull/88636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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]

@@ -20,13 +21,17 @@ namespace clang::tidy::readability {
 /// 
http://clang.llvm.org/extra/clang-tidy/checks/readability/string-compare.html
 class StringCompareCheck : public ClangTidyCheck {
 public:
-  StringCompareCheck(StringRef Name, ClangTidyContext *Context)
-  : ClangTidyCheck(Name, Context) {}
+  StringCompareCheck(StringRef Name, ClangTidyContext *Context);
+
   bool isLanguageVersionSupported(const LangOptions ) const override {
 return LangOpts.CPlusPlus;
   }
+

PiotrZSL wrote:

missing method storeOptions

https://github.com/llvm/llvm-project/pull/88636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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]

https://github.com/PiotrZSL requested changes to this pull request.

- Missing storeOptions
- Some other tiny nits.
Would be nice to get rid of lambda RegisterForClasses as it's not needed, and 
have common handling.

https://github.com/llvm/llvm-project/pull/88636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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]

@@ -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 
 
 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 StringClasses = {
+"::std::basic_string", "::std::basic_string_view"};
+
+StringCompareCheck::StringCompareCheck(StringRef Name,
+   ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  StringLikeClasses(
+  optutils::parseStringList(Options.get("StringLikeClasses", ""))) {}

PiotrZSL wrote:

put StringClasses  as default

https://github.com/llvm/llvm-project/pull/88636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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]

@@ -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 
 
 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 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 ) {
+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 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
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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]

https://github.com/PiotrZSL edited 
https://github.com/llvm/llvm-project/pull/88636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Vadim D. via cfe-commits]

https://github.com/vvd170501 edited 
https://github.com/llvm/llvm-project/pull/88636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Vadim D. via cfe-commits]

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

>From 0db24a6806e9429b5e7b9cd9d0777315b3e6d87e Mon Sep 17 00:00:00 2001
From: Vadim Dudkin 
Date: Sat, 13 Apr 2024 23:36:12 +0300
Subject: [PATCH 1/2] readability-string-compare: check std::string_view, allow
 custom string-like classes

---
 .../readability/StringCompareCheck.cpp| 74 +--
 .../readability/StringCompareCheck.h  |  9 ++-
 clang-tools-extra/docs/ReleaseNotes.rst   |  5 ++
 .../checks/readability/string-compare.rst | 33 -
 .../clang-tidy/checkers/Inputs/Headers/string | 10 +++
 .../string-compare-custom-string-classes.cpp  | 38 ++
 .../checkers/readability/string-compare.cpp   | 23 ++
 7 files changed, 165 insertions(+), 27 deletions(-)
 create mode 100644 
clang-tools-extra/test/clang-tidy/checkers/readability/string-compare-custom-string-classes.cpp

diff --git a/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp
index 3b5d89c8c64719..905e5b156ef864 100644
--- a/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp
@@ -7,12 +7,16 @@
 
//===--===//
 
 #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 
 
 using namespace clang::ast_matchers;
+namespace optutils = clang::tidy::utils::options;
 
 namespace clang::tidy::readability {
 
@@ -20,29 +24,53 @@ static const StringRef CompareMessage = "do not use 
'compare' to test equality "
 "of strings; use the string equality "
 "operator instead";
 
+static const std::vector 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 ) {
+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 PossiblyTemplateClasses = StringClasses;
+PossiblyTemplateClasses.insert(PossiblyTemplateClasses.end(),
+   StringLikeClasses.begin(),
+   StringLikeClasses.end());
+RegisterForClasses(anyOf(
+

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

[Vadim D. via cfe-commits]

@@ -50,5 +52,32 @@ Examples:
   }
 
 The above code examples show the list of if-statements that this check will
-give a warning for. All of them uses ``compare`` to check if equality or
+give a warning for. All of them use ``compare`` to check equality or
 inequality of two strings instead of using the correct operators.
+
+Options
+---
+
+.. option:: StringLikeClasses
+
+   A string containing comma-separated names of string-like classes. Default 
is an empty string.
+   If a class from this list has a ``compare`` method similar to 
``std::string``, it will be checked in the same way.
+
+Example
+^^^
+
+.. code-block:: c++
+
+  struct CustomString {
+  public:
+int compare (const CustomString& other) const;
+  }
+
+  CustomString str1;
+  CustomString str2;
+
+  // use str1 != str2 instead.
+  if (str1.compare(str2)) {
+  }
+
+If `StringLikeClasses` contains ``CustomString``, the check will suggest 
replacing ``compare`` with equality operator.

vvd170501 wrote:

Default value of what? If you mean `StringLikeClasses`, it is empty by default 
(see line 63)

https://github.com/llvm/llvm-project/pull/88636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[via cfe-commits]

@@ -50,5 +52,32 @@ Examples:
   }
 
 The above code examples show the list of if-statements that this check will
-give a warning for. All of them uses ``compare`` to check if equality or
+give a warning for. All of them use ``compare`` to check equality or
 inequality of two strings instead of using the correct operators.
+
+Options
+---
+
+.. option:: StringLikeClasses
+
+   A string containing comma-separated names of string-like classes. Default 
is an empty string.

EugeneZelenko wrote:

Please follow 80 characters limit.

https://github.com/llvm/llvm-project/pull/88636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[via cfe-commits]

@@ -50,5 +52,32 @@ Examples:
   }
 
 The above code examples show the list of if-statements that this check will
-give a warning for. All of them uses ``compare`` to check if equality or
+give a warning for. All of them use ``compare`` to check equality or
 inequality of two strings instead of using the correct operators.
+
+Options
+---
+
+.. option:: StringLikeClasses
+
+   A string containing comma-separated names of string-like classes. Default 
is an empty string.
+   If a class from this list has a ``compare`` method similar to 
``std::string``, it will be checked in the same way.
+
+Example
+^^^
+
+.. code-block:: c++
+
+  struct CustomString {
+  public:
+int compare (const CustomString& other) const;
+  }
+
+  CustomString str1;
+  CustomString str2;
+
+  // use str1 != str2 instead.
+  if (str1.compare(str2)) {
+  }
+
+If `StringLikeClasses` contains ``CustomString``, the check will suggest 
replacing ``compare`` with equality operator.

EugeneZelenko wrote:

What is default value?

https://github.com/llvm/llvm-project/pull/88636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[via cfe-commits]

llvmbot wrote:




@llvm/pr-subscribers-clang-tools-extra

Author: Vadim D. (vvd170501)


Changes

This PR aims to expand the list of classes that are considered to be "strings" 
by `readability-string-compare` check.

1. Currently only `std::string;:compare` is checked, but  `std::string_view` 
has a similar `compare` method. This PR enables checking of 
`std::string_view::compare` by default.
2. Some codebases use custom string-like classes that have public interfaces 
similar to `std::string` or `std::string_view`. Example: 
[TStringBase](https://github.com/yandex/yatool/blob/main/util/generic/strbase.h#L38),
 A new option, `readability-string-compare.StringClassNames`, is added to allow 
specifying a custom list of string-like classes.

Related to, but does not solve #28396 (only adds support for custom 
string-like classes, not custom functions)

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


7 Files Affected:

- (modified) clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp 
(+51-23) 
- (modified) clang-tools-extra/clang-tidy/readability/StringCompareCheck.h 
(+7-2) 
- (modified) clang-tools-extra/docs/ReleaseNotes.rst (+5) 
- (modified) 
clang-tools-extra/docs/clang-tidy/checks/readability/string-compare.rst (+31-2) 
- (modified) clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string 
(+10) 
- (added) 
clang-tools-extra/test/clang-tidy/checkers/readability/string-compare-custom-string-classes.cpp
 (+38) 
- (modified) 
clang-tools-extra/test/clang-tidy/checkers/readability/string-compare.cpp (+23) 


``diff
diff --git a/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp
index 3b5d89c8c64719..905e5b156ef864 100644
--- a/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp
@@ -7,12 +7,16 @@
 
//===--===//
 
 #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 
 
 using namespace clang::ast_matchers;
+namespace optutils = clang::tidy::utils::options;
 
 namespace clang::tidy::readability {
 
@@ -20,29 +24,53 @@ static const StringRef CompareMessage = "do not use 
'compare' to test equality "
 "of strings; use the string equality "
 "operator instead";
 
+static const std::vector 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 ) {
+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"),
+   

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

[via cfe-commits]

llvmbot wrote:




@llvm/pr-subscribers-clang-tidy

Author: Vadim D. (vvd170501)


Changes

This PR aims to expand the list of classes that are considered to be "strings" 
by `readability-string-compare` check.

1. Currently only `std::string;:compare` is checked, but  `std::string_view` 
has a similar `compare` method. This PR enables checking of 
`std::string_view::compare` by default.
2. Some codebases use custom string-like classes that have public interfaces 
similar to `std::string` or `std::string_view`. Example: 
[TStringBase](https://github.com/yandex/yatool/blob/main/util/generic/strbase.h#L38),
 A new option, `readability-string-compare.StringClassNames`, is added to allow 
specifying a custom list of string-like classes.

Related to, but does not solve #28396 (only adds support for custom 
string-like classes, not custom functions)

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


7 Files Affected:

- (modified) clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp 
(+51-23) 
- (modified) clang-tools-extra/clang-tidy/readability/StringCompareCheck.h 
(+7-2) 
- (modified) clang-tools-extra/docs/ReleaseNotes.rst (+5) 
- (modified) 
clang-tools-extra/docs/clang-tidy/checks/readability/string-compare.rst (+31-2) 
- (modified) clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string 
(+10) 
- (added) 
clang-tools-extra/test/clang-tidy/checkers/readability/string-compare-custom-string-classes.cpp
 (+38) 
- (modified) 
clang-tools-extra/test/clang-tidy/checkers/readability/string-compare.cpp (+23) 


``diff
diff --git a/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp
index 3b5d89c8c64719..905e5b156ef864 100644
--- a/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp
@@ -7,12 +7,16 @@
 
//===--===//
 
 #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 
 
 using namespace clang::ast_matchers;
+namespace optutils = clang::tidy::utils::options;
 
 namespace clang::tidy::readability {
 
@@ -20,29 +24,53 @@ static const StringRef CompareMessage = "do not use 
'compare' to test equality "
 "of strings; use the string equality "
 "operator instead";
 
+static const std::vector 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 ) {
+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"),
+   

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

[Vadim D. via cfe-commits]

https://github.com/vvd170501 ready_for_review 
https://github.com/llvm/llvm-project/pull/88636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Vadim D. via cfe-commits]

@@ -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 
 
 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 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 ) {

vvd170501 wrote:

Type of `StringClassMatcher` dependinds on whether the check is used with or 
without `StringLikeClasses`, so a template lambda is needed.

It's possible to use `anyOf` and `cxxRecordDecl` in both cases and remove the 
lambda, but this will probably slow the check down in default case.

https://github.com/llvm/llvm-project/pull/88636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Vadim D. via cfe-commits]

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

>From 0db24a6806e9429b5e7b9cd9d0777315b3e6d87e Mon Sep 17 00:00:00 2001
From: Vadim Dudkin 
Date: Sat, 13 Apr 2024 23:36:12 +0300
Subject: [PATCH] readability-string-compare: check std::string_view, allow
 custom string-like classes

---
 .../readability/StringCompareCheck.cpp| 74 +--
 .../readability/StringCompareCheck.h  |  9 ++-
 clang-tools-extra/docs/ReleaseNotes.rst   |  5 ++
 .../checks/readability/string-compare.rst | 33 -
 .../clang-tidy/checkers/Inputs/Headers/string | 10 +++
 .../string-compare-custom-string-classes.cpp  | 38 ++
 .../checkers/readability/string-compare.cpp   | 23 ++
 7 files changed, 165 insertions(+), 27 deletions(-)
 create mode 100644 
clang-tools-extra/test/clang-tidy/checkers/readability/string-compare-custom-string-classes.cpp

diff --git a/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp
index 3b5d89c8c64719..905e5b156ef864 100644
--- a/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp
@@ -7,12 +7,16 @@
 
//===--===//
 
 #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 
 
 using namespace clang::ast_matchers;
+namespace optutils = clang::tidy::utils::options;
 
 namespace clang::tidy::readability {
 
@@ -20,29 +24,53 @@ static const StringRef CompareMessage = "do not use 
'compare' to test equality "
 "of strings; use the string equality "
 "operator instead";
 
+static const std::vector 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 ) {
+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 PossiblyTemplateClasses = StringClasses;
+PossiblyTemplateClasses.insert(PossiblyTemplateClasses.end(),
+   StringLikeClasses.begin(),
+   StringLikeClasses.end());
+RegisterForClasses(anyOf(
+

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

[Vadim D. via cfe-commits]

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

>From b63dd11ea87cd504c8972de6ed29330d6702abca Mon Sep 17 00:00:00 2001
From: Vadim Dudkin 
Date: Sat, 13 Apr 2024 23:36:12 +0300
Subject: [PATCH] readability-string-compare: check std::string_view, allow
 custom string-like classes

---
 .../readability/StringCompareCheck.cpp| 74 +--
 .../readability/StringCompareCheck.h  |  9 ++-
 clang-tools-extra/docs/ReleaseNotes.rst   |  5 ++
 .../checks/readability/string-compare.rst | 33 -
 .../clang-tidy/checkers/Inputs/Headers/string | 10 +++
 .../string-compare-custom-string-classes.cpp  | 36 +
 .../checkers/readability/string-compare.cpp   | 23 ++
 7 files changed, 163 insertions(+), 27 deletions(-)
 create mode 100644 
clang-tools-extra/test/clang-tidy/checkers/readability/string-compare-custom-string-classes.cpp

diff --git a/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp
index 3b5d89c8c64719..905e5b156ef864 100644
--- a/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp
@@ -7,12 +7,16 @@
 
//===--===//
 
 #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 
 
 using namespace clang::ast_matchers;
+namespace optutils = clang::tidy::utils::options;
 
 namespace clang::tidy::readability {
 
@@ -20,29 +24,53 @@ static const StringRef CompareMessage = "do not use 
'compare' to test equality "
 "of strings; use the string equality "
 "operator instead";
 
+static const std::vector 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 ) {
+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 PossiblyTemplateClasses = StringClasses;
+PossiblyTemplateClasses.insert(PossiblyTemplateClasses.end(),
+   StringLikeClasses.begin(),
+   StringLikeClasses.end());
+RegisterForClasses(anyOf(
+

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

[Vadim D. via cfe-commits]

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

>From 91eccdd291fa4f0753aa8e9970fa146516823e22 Mon Sep 17 00:00:00 2001
From: Vadim Dudkin 
Date: Sat, 13 Apr 2024 23:36:12 +0300
Subject: [PATCH] readability-string-compare: check std::string_view, allow
 custom string-like classes

---
 .../readability/StringCompareCheck.cpp| 73 +--
 .../readability/StringCompareCheck.h  |  9 ++-
 clang-tools-extra/docs/ReleaseNotes.rst   |  5 ++
 .../checks/readability/string-compare.rst | 33 -
 .../clang-tidy/checkers/Inputs/Headers/string | 10 +++
 .../string-compare-custom-string-classes.cpp  | 36 +
 .../checkers/readability/string-compare.cpp   | 23 ++
 7 files changed, 162 insertions(+), 27 deletions(-)
 create mode 100644 
clang-tools-extra/test/clang-tidy/checkers/readability/string-compare-custom-string-classes.cpp

diff --git a/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp
index 3b5d89c8c64719..20218975ec121b 100644
--- a/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp
@@ -7,12 +7,16 @@
 
//===--===//
 
 #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 
 
 using namespace clang::ast_matchers;
+namespace optutils = clang::tidy::utils::options;
 
 namespace clang::tidy::readability {
 
@@ -20,29 +24,52 @@ static const StringRef CompareMessage = "do not use 
'compare' to test equality "
 "of strings; use the string equality "
 "operator instead";
 
+static const std::vector 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 ) {
+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.
+std::vector PossiblyTemplateClasses = StringClasses;
+PossiblyTemplateClasses.insert(PossiblyTemplateClasses.end(),
+   StringLikeClasses.begin(),
+   StringLikeClasses.end());
+RegisterForClasses(anyOf(
+classTemplateSpecializationDecl(hasAnyName(PossiblyTemplateClasses)),
+   

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

[Vadim D. via cfe-commits]

https://github.com/vvd170501 edited 
https://github.com/llvm/llvm-project/pull/88636
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Vadim D. via cfe-commits]

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

This PR aims to expand the list of classes that are considered to be "strings" 
by `readability-string-compare` check.

1. Currently `readability-string-compare` only checks `std::string;:compare`, 
but  `std::string_view` has a similar `compare` method, which also should not 
be used to check equality of two strings.
2. Some codebases use custom string-like classes that have public interfaces 
similar to `std::string` or `std::string_view`. Example: 
[TStringBase](https://github.com/yandex/yatool/blob/main/util/generic/strbase.h#L38),

Related to, but does not solve #28396 (only adds support for custom string-like 
classes, not custom functions)

>From eb5279fd83ba114b8eb094099d5993d6bfb003e3 Mon Sep 17 00:00:00 2001
From: Vadim Dudkin 
Date: Sat, 13 Apr 2024 23:36:12 +0300
Subject: [PATCH] readability-string-compare: check std::string_view, allow
 custom string-like classes

---
 .../readability/StringCompareCheck.cpp| 75 +--
 .../readability/StringCompareCheck.h  | 10 ++-
 .../clang-tidy/checkers/Inputs/Headers/string | 10 +++
 .../string-compare-custom-string-classes.cpp  | 41 ++
 .../checkers/readability/string-compare.cpp   | 14 
 5 files changed, 125 insertions(+), 25 deletions(-)
 create mode 100644 
clang-tools-extra/test/clang-tidy/checkers/readability/string-compare-custom-string-classes.cpp

diff --git a/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp
index 3b5d89c8c64719..30bde1f8b75b61 100644
--- a/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp
@@ -7,12 +7,15 @@
 
//===--===//
 
 #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"
 
 using namespace clang::ast_matchers;
+namespace optutils = clang::tidy::utils::options;
 
 namespace clang::tidy::readability {
 
@@ -20,29 +23,55 @@ static const StringRef CompareMessage = "do not use 
'compare' to test equality "
 "of strings; use the string equality "
 "operator instead";
 
+static const StringRef DefaultStringClassNames = "::std::basic_string;"
+ "::std::basic_string_view";
+
+StringCompareCheck::StringCompareCheck(StringRef Name,
+   ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  StringClassNames(optutils::parseStringList(
+  Options.get("StringClassNames", DefaultStringClassNames))),
+  // Both std::string and std::string_view are templates, so this check 
only
+  // needs to match template classes by default.
+  // Custom `StringClassNames` may contain non-template classes, and
+  // it's impossible to tell them apart from templates just by name.
+  CheckNonTemplateClasses(Options.get("StringClassNames").has_value()) {}
+
 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);
+  if (StringClassNames.empty()) {
+return;
+  }
+  const auto RegisterForClasses = [&, this](const auto ) {
+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()),
+