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 <vvd170...@gmail.com>
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 <eugene.zele...@gmail.com>
---
.../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 &Opts) {
+ 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 <vector>
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 &LangOpts) const override {
return LangOpts.CPlusPlus;
}
+
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+
+private:
+ const std::vector<StringRef> 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 :doc:`readability-string-compare
+ <clang-tidy/checks/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.
+
Removed checks
^^^^^^^^^^^^^^
diff --git
a/clang-tools-extra/docs/clang-tidy/checks/readability/string-compare.rst
b/clang-tools-extra/docs/clang-tidy/checks/readability/string-compare.rst
index 268632eee61a27..4be2473bed2d74 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/readability/string-compare.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/readability/string-compare.rst
@@ -14,10 +14,12 @@ recommended to avoid the risk of incorrect interpretation
of the return value
and to simplify the code. The string equality and inequality operators can
also be faster than the ``compare`` method due to early termination.
-Examples:
+Example
+-------
.. code-block:: c++
+ // The same rules apply to std::string_view.
std::string str1{"a"};
std::string str2{"b"};
@@ -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
+replacing ``compare`` with equality operator.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string
b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string
index d031f27beb9dfe..0c160bc182b6eb 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string
+++ b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string
@@ -108,6 +108,8 @@ struct basic_string_view {
constexpr bool starts_with(C ch) const noexcept;
constexpr bool starts_with(const C* s) const;
+ constexpr int compare(basic_string_view sv) const noexcept;
+
static constexpr size_t npos = -1;
};
@@ -132,6 +134,14 @@ bool operator==(const std::wstring&, const std::wstring&);
bool operator==(const std::wstring&, const wchar_t*);
bool operator==(const wchar_t*, const std::wstring&);
+bool operator==(const std::string_view&, const std::string_view&);
+bool operator==(const std::string_view&, const char*);
+bool operator==(const char*, const std::string_view&);
+
+bool operator!=(const std::string_view&, const std::string_view&);
+bool operator!=(const std::string_view&, const char*);
+bool operator!=(const char*, const std::string_view&);
+
size_t strlen(const char* str);
}
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/readability/string-compare-custom-string-classes.cpp
b/clang-tools-extra/test/clang-tidy/checkers/readability/string-compare-custom-string-classes.cpp
new file mode 100644
index 00000000000000..faf135833ee15d
--- /dev/null
+++
b/clang-tools-extra/test/clang-tidy/checkers/readability/string-compare-custom-string-classes.cpp
@@ -0,0 +1,35 @@
+// RUN: %check_clang_tidy %s readability-string-compare %t --
-config='{CheckOptions: {readability-string-compare.StringLikeClasses:
"CustomStringTemplateBase;CustomStringNonTemplateBase"}}' -- -isystem
%clang_tidy_headers
+#include <string>
+
+struct CustomStringNonTemplateBase {
+ int compare(const CustomStringNonTemplateBase& Other) const {
+ return 123; // value is not important for check
+ }
+};
+
+template <typename T>
+struct CustomStringTemplateBase {
+ int compare(const CustomStringTemplateBase& Other) const {
+ return 123;
+ }
+};
+
+struct CustomString1 : CustomStringNonTemplateBase {};
+struct CustomString2 : CustomStringTemplateBase<char> {};
+
+void CustomStringClasses() {
+ std::string_view sv1("a");
+ std::string_view sv2("b");
+ if (sv1.compare(sv2)) { // No warning - if a std class is not listed in
StringLikeClasses, it won't be checked.
+ }
+
+ CustomString1 custom1;
+ if (custom1.compare(custom1)) {
+ }
+ // CHECK-MESSAGES: [[@LINE-2]]:7: warning: do not use 'compare' to test
equality of strings; use the string equality operator instead
[readability-string-compare]
+
+ CustomString2 custom2;
+ if (custom2.compare(custom2)) {
+ }
+ // CHECK-MESSAGES: [[@LINE-2]]:7: warning: do not use 'compare' to test
equality of strings; use the string equality operator instead
[readability-string-compare]
+}
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/readability/string-compare.cpp
b/clang-tools-extra/test/clang-tidy/checkers/readability/string-compare.cpp
index 2c08b86cf72fa0..c4fea4341617b8 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability/string-compare.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/string-compare.cpp
@@ -67,11 +67,27 @@ void Test() {
if (str1.compare(comp())) {
}
// CHECK-MESSAGES: [[@LINE-2]]:7: warning: do not use 'compare' to test
equality of strings;
+
+ std::string_view sv1("a");
+ std::string_view sv2("b");
+ if (sv1.compare(sv2)) {
+ }
+ // CHECK-MESSAGES: [[@LINE-2]]:7: warning: do not use 'compare' to test
equality of strings; use the string equality operator instead
[readability-string-compare]
+}
+
+struct DerivedFromStdString : std::string {};
+
+void TestDerivedClass() {
+ DerivedFromStdString derived;
+ if (derived.compare(derived)) {
+ }
+ // CHECK-MESSAGES: [[@LINE-2]]:7: warning: do not use 'compare' to test
equality of strings; use the string equality operator instead
[readability-string-compare]
}
void Valid() {
std::string str1("a", 1);
std::string str2("b", 1);
+
if (str1 == str2) {
}
if (str1 != str2) {
@@ -96,4 +112,11 @@ void Valid() {
}
if (str1.compare(str2) == -1) {
}
+
+ std::string_view sv1("a");
+ std::string_view sv2("b");
+ if (sv1 == sv2) {
+ }
+ if (sv1.compare(sv2) > 0) {
+ }
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
