[PATCH] D27210: [clang-tidy] misc-string-compare. Adding a new check to clang-tidy

[Malcolm Parsons via Phabricator via cfe-commits]

malcolm.parsons added inline comments.


================
Comment at: clang-tidy/misc/StringCompareCheck.cpp:25
+      callee(cxxMethodDecl(hasName("compare"),
+                           ofClass(classTemplateSpecializationDecl(
+                               hasName("::std::basic_string"))))),
----------------
This needs to check that it's one of the single parameter overloads of compare.


================
Comment at: clang-tidy/misc/StringCompareCheck.cpp:27
+                               hasName("::std::basic_string"))))),
+      hasArgument(0, declRefExpr()), callee(memberExpr()));
+
----------------
I don't think you need to check what the first argument is.


================
Comment at: clang-tidy/misc/StringCompareCheck.cpp:39
+      binaryOperator(anyOf(hasOperatorName("=="), hasOperatorName("!=")),
+                                         hasEitherOperand(strCompare),
+                                         hasEitherOperand(integerLiteral(equals(0))))
----------------
Is this clang-formatted?


================
Comment at: test/clang-tidy/misc-string-compare.cpp:9
+
+  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]
----------------
malcolm.parsons wrote:
> Some other test ideas:
> 
> ```
> if (str1.compare("foo")) {}
> 
> return str1.compare(str2) == 0;
> 
> func(str1.compare(str2) != 0);
> 
> if (str2.empty() || str1.compare(str2) != 0) {}
> ```
There's still no test for the single parameter c-string overload.


https://reviews.llvm.org/D27210