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

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh requested changes to this revision.
alexfh added inline comments.
This revision now requires changes to proceed.


================
Comment at: clang-tidy/misc/StringCompareCheck.cpp:29
+
+  // First and second case: cast str.compare(str) to boolean
+  Finder->addMatcher(
----------------
Please add trailing periods here and elsewhere.


================
Comment at: clang-tidy/misc/StringCompareCheck.cpp:36-50
+  // Third case: str.compare(str) == 0
+  Finder->addMatcher(
+      binaryOperator(hasOperatorName("=="),
+                                         hasEitherOperand(strCompare),
+                                         hasEitherOperand(integerLiteral(equals(0))))
+          .bind("match"),
+      this);
----------------
These two matchers can be merged to avoid repetition.


================
Comment at: clang-tidy/misc/StringCompareCheck.cpp:55-57
+    diag(Matched->getLocStart(),
+         "do not use compare to test equality of strings; "
+         "use the string equality operator instead");
----------------
It looks like it's relatively straightforward to implement fixit hints. WDYT?


================
Comment at: test/clang-tidy/misc-string-compare.cpp:29
+  if(!str1.compare(str2)) {}
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: do not use compare to test equality of strings; use the string equality operator instead [misc-string-compare]
+  if(str1.compare(str2) == 0) {}
----------------
Truncate all CHECK patterns after the first one after `of strings;`


https://reviews.llvm.org/D27210