<div dir="ltr">Hi Alexander,<div><br></div><div>I have now implemented your suggestions - all but the fixit one. If I have added bindings for str1 and str2 in ast matcher, how would I go about creating a replacement for the entire implicitCastExpr or binaryOperator? I can't find any example in the code for clang-tidy to suggest how I would build a new node for the AST. Or I am looking at it from a wrong direction? </div><div><br></div><div>Could you hint me in the right direction as to how I would make the fixit? </div><div><br></div><div>Best regards,</div><div>Mads Ravn</div></div><br><div class="gmail_quote"><div dir="ltr">On Thu, Dec 1, 2016 at 3:41 PM Alexander Kornienko via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">alexfh requested changes to this revision.<br class="gmail_msg">
alexfh added inline comments.<br class="gmail_msg">
This revision now requires changes to proceed.<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
================<br class="gmail_msg">
Comment at: clang-tidy/misc/StringCompareCheck.cpp:29<br class="gmail_msg">
+<br class="gmail_msg">
+  // First and second case: cast str.compare(str) to boolean<br class="gmail_msg">
+  Finder->addMatcher(<br class="gmail_msg">
----------------<br class="gmail_msg">
Please add trailing periods here and elsewhere.<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
================<br class="gmail_msg">
Comment at: clang-tidy/misc/StringCompareCheck.cpp:36-50<br class="gmail_msg">
+  // Third case: str.compare(str) == 0<br class="gmail_msg">
+  Finder->addMatcher(<br class="gmail_msg">
+      binaryOperator(hasOperatorName("=="),<br class="gmail_msg">
+                                         hasEitherOperand(strCompare),<br class="gmail_msg">
+                                         hasEitherOperand(integerLiteral(equals(0))))<br class="gmail_msg">
+          .bind("match"),<br class="gmail_msg">
+      this);<br class="gmail_msg">
----------------<br class="gmail_msg">
These two matchers can be merged to avoid repetition.<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
================<br class="gmail_msg">
Comment at: clang-tidy/misc/StringCompareCheck.cpp:55-57<br class="gmail_msg">
+    diag(Matched->getLocStart(),<br class="gmail_msg">
+         "do not use compare to test equality of strings; "<br class="gmail_msg">
+         "use the string equality operator instead");<br class="gmail_msg">
----------------<br class="gmail_msg">
It looks like it's relatively straightforward to implement fixit hints. WDYT?<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
================<br class="gmail_msg">
Comment at: test/clang-tidy/misc-string-compare.cpp:29<br class="gmail_msg">
+  if(!str1.compare(str2)) {}<br class="gmail_msg">
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: do not use compare to test equality of strings; use the string equality operator instead [misc-string-compare]<br class="gmail_msg">
+  if(str1.compare(str2) == 0) {}<br class="gmail_msg">
----------------<br class="gmail_msg">
Truncate all CHECK patterns after the first one after `of strings;`<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
<a href="https://reviews.llvm.org/D27210" rel="noreferrer" class="gmail_msg" target="_blank">https://reviews.llvm.org/D27210</a><br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
</blockquote></div>