madsravn updated this revision to Diff 79624.
madsravn added a comment.
Updated per comments
https://reviews.llvm.org/D27210
Files:
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/MiscTidyModule.cpp
clang-tidy/misc/StringCompareCheck.cpp
clang-tidy/misc/StringCompareCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/misc-string-compare.rst
test/clang-tidy/misc-string-compare.cpp
Index: test/clang-tidy/misc-string-compare.cpp
===================================================================
--- test/clang-tidy/misc-string-compare.cpp
+++ test/clang-tidy/misc-string-compare.cpp
@@ -0,0 +1,29 @@
+// RUN: %check_clang_tidy %s misc-string-compare %t -- -- -std=c++11
+
+#include <string>
+
+void Test() {
+ std::string str1{"a"};
+ std::string str2{"b"};
+
+ if(str1.compare(str2)) {}
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: do not use compare to test equality of strings; use the string equality operator instead [misc-string-compare]
+ if(!str1.compare(str2)) {}
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: do not use compare to test equality of strings; use the string equality operator instead [misc-string-compare]
+ if(str1.compare(str2) == 0) {}
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: do not use compare to test equality of strings; use the string equality operator instead [misc-string-compare]
+ if(str1.compare(str2) != 0) {}
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: do not use compare to test equality of strings; use the string equality operator instead [misc-string-compare]
+ if(0 == str1.compare(str2)) {}
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: do not use compare to test equality of strings; use the string equality operator instead [misc-string-compare]
+ if(0 != str1.compare(str2)) {}
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: do not use compare to test equality of strings; use the string equality operator instead [misc-string-compare]
+
+}
+
+void Valid() {
+ std::string str1{"a"};
+ std::string str2{"b"};
+ if(str1 == str2) {}
+ if(str1 != str2) {}
+}
Index: docs/clang-tidy/checks/misc-string-compare.rst
===================================================================
--- docs/clang-tidy/checks/misc-string-compare.rst
+++ docs/clang-tidy/checks/misc-string-compare.rst
@@ -0,0 +1,35 @@
+.. title:: clang-tidy - misc-string-compare
+
+misc-string-compare
+===================
+
+Finds string comparisons using the compare method.
+
+A common mistake is to use the string's ``compare`` method instead of using the
+equality or inequality operators. The compare method is intended for sorting
+functions and thus returns ``-1``, ``0`` or ``1`` depending on the lexicographical
+relationship between the strings compared. If an equality or inequality check
+can suffice, that is recommended.
+
+Examples:
+
+.. code-block:: c++
+
+ std::string str1{"a"};
+ std::string str2{"b"};
+
+ if(str1.compare(str2)) {} // use str1 != str2 instead
+
+ if(!str1.compare(str2)) {} // use str1 == str2 instead
+
+ if(str1.compare(str2) == 0) {} // use str1 == str2 instead
+
+ if(str1.compare(str2) != 0) {} // use str1 != str2 instead
+
+ if(0 == str1.compare(str2)) {} // use str1 == str2 instead
+
+ if(0 != str1.compare(str2)) {} // use str1 != str2 instead
+
+The above code examples shows the list of if-statements that this check will
+give a warning for. All of them uses ``compare`` to check if equality or
+inequality of two strings instead of using the correct operators.
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -80,6 +80,7 @@
misc-sizeof-container
misc-sizeof-expression
misc-static-assert
+ misc-string-compare
misc-string-constructor
misc-string-integer-assignment
misc-string-literal-with-embedded-nul
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -75,6 +75,11 @@
- `misc-pointer-and-integral-operation` check was removed.
+- New `misc-string-compare
+ <http://clang.llvm.org/extra/clang-tidy/checks/misc-string-compare.html>`_ check
+
+ Warns about using ``compare`` to test for string equality or ineqaulity.
+
- New `misc-use-after-move
<http://clang.llvm.org/extra/clang-tidy/checks/misc-use-after-move.html>`_ check
Index: clang-tidy/misc/StringCompareCheck.h
===================================================================
--- clang-tidy/misc/StringCompareCheck.h
+++ clang-tidy/misc/StringCompareCheck.h
@@ -0,0 +1,36 @@
+//===--- StringCompareCheck.h - clang-tidy-----------------------*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef STRING_COMPARE_CHECK_H
+#define STRING_COMPARE_CHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// This check flags all calls compare when used to check for string
+/// equality or inequality.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-string-compare-check.html
+class StringCompareCheck : public ClangTidyCheck {
+public:
+ StringCompareCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // STRING_COMPARE_CHECK_H
Index: clang-tidy/misc/StringCompareCheck.cpp
===================================================================
--- clang-tidy/misc/StringCompareCheck.cpp
+++ clang-tidy/misc/StringCompareCheck.cpp
@@ -0,0 +1,69 @@
+//===--- MiscStringCompare.cpp - clang-tidy--------------------------------===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "StringCompareCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+void StringCompareCheck::registerMatchers(MatchFinder *Finder) {
+ if (!getLangOpts().CPlusPlus)
+ return;
+ const auto strCompare = cxxMemberCallExpr(
+ callee(cxxMethodDecl(hasName("compare"),
+ ofClass(classTemplateSpecializationDecl(
+ hasName("::std::basic_string"))))),
+ hasArgument(0, declRefExpr()), callee(memberExpr()));
+
+ // First case: if(str.compare(str))
+ Finder->addMatcher(
+ ifStmt(hasCondition(implicitCastExpr(
+ hasImplicitDestinationType(isInteger()), has(strCompare))))
+ .bind("match"),
+ this);
+
+ // Second case: if(!str.compare(str))
+ Finder->addMatcher(ifStmt(hasCondition(unaryOperator(
+ hasOperatorName("!"),
+ hasUnaryOperand(implicitCastExpr(
+ hasImplicitDestinationType(isInteger()),
+ has(strCompare))))))
+ .bind("match"),
+ this);
+
+ // Third case: if(str.compare(str) == 0)
+ Finder->addMatcher(
+ ifStmt(hasCondition(binaryOperator(hasOperatorName("=="),
+ hasEitherOperand(strCompare))))
+ .bind("match"),
+ this);
+
+ // Fourth case: if(str.compare(str) != 0)
+ Finder->addMatcher(
+ ifStmt(hasCondition(binaryOperator(hasOperatorName("!="),
+ hasEitherOperand(strCompare))))
+ .bind("match"),
+ this);
+}
+
+void StringCompareCheck::check(const MatchFinder::MatchResult &Result) {
+ if (const auto *Matched = Result.Nodes.getNodeAs<Stmt>("match"))
+ diag(Matched->getLocStart(),
+ "do not use compare to test equality of strings; "
+ "use the string equality operator instead");
+}
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/misc/MiscTidyModule.cpp
===================================================================
--- clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tidy/misc/MiscTidyModule.cpp
@@ -35,6 +35,7 @@
#include "SizeofContainerCheck.h"
#include "SizeofExpressionCheck.h"
#include "StaticAssertCheck.h"
+#include "StringCompareCheck.h"
#include "StringConstructorCheck.h"
#include "StringIntegerAssignmentCheck.h"
#include "StringLiteralWithEmbeddedNulCheck.h"
@@ -105,6 +106,7 @@
CheckFactories.registerCheck<SizeofExpressionCheck>(
"misc-sizeof-expression");
CheckFactories.registerCheck<StaticAssertCheck>("misc-static-assert");
+ CheckFactories.registerCheck<StringCompareCheck>("misc-string-compare");
CheckFactories.registerCheck<StringConstructorCheck>(
"misc-string-constructor");
CheckFactories.registerCheck<StringIntegerAssignmentCheck>(
Index: clang-tidy/misc/CMakeLists.txt
===================================================================
--- clang-tidy/misc/CMakeLists.txt
+++ clang-tidy/misc/CMakeLists.txt
@@ -28,6 +28,7 @@
SizeofContainerCheck.cpp
SizeofExpressionCheck.cpp
StaticAssertCheck.cpp
+ StringCompareCheck.cpp
StringConstructorCheck.cpp
StringIntegerAssignmentCheck.cpp
StringLiteralWithEmbeddedNulCheck.cpp
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
