<div dir="ltr">Firstly, please respond in phabricator if it is possible. When you send email it doesn't appear in phabricator, it's probably a bug.<br><div class="gmail_extra"><br><div class="gmail_quote">2016-12-19 8:00 GMT+01:00 Mads Ravn <span dir="ltr"><<a href="mailto:madsravn@gmail.com" target="_blank">madsravn@gmail.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Hi Piotr,<div><br></div><div>Thank you for your detailed comments :) </div><div><br></div><div>I would love some help with the other fixit. I have some notes on it at home. But my main catch is that is an implicit cast to boolean from str.compare(str) with maybe an ! in front of it. And I need to fix that to str.compare(str) == 0 or str.compare(str) != 0.</div><div><br></div></div></blockquote><div>Why fix it to something, that you will want to fix it again to str == str and str != str?</div><div>I guess you just have to match parent of this expr is negation or anything, bind negation to some name and then check if you matched to the negation or not.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div></div><div>Where should I put the warning in a <span style="color:rgb(33,33,33)">static const global variable? Is it still in StringCompare.cpp or do we have a  joint files for these? </span></div><div><span style="color:rgb(33,33,33)"><br></span></div></div></blockquote><div>Yep, StringCompare.cpp, just like in other checks. </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><span style="color:rgb(33,33,33)"></span></div><div><span style="color:rgb(33,33,33)">Best regards,</span></div><div><span style="color:rgb(33,33,33)">Mads Ravn</span></div></div><div class="HOEnZb"><div class="h5"><br><div class="gmail_quote"><div dir="ltr">On Sun, Dec 18, 2016 at 11:26 PM Piotr Padlewski via Phabricator <<a href="mailto:reviews@reviews.llvm.org" target="_blank">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">Prazek added a comment.<br class="m_-7269145279221876740gmail_msg">
<br class="m_-7269145279221876740gmail_msg">
Do you need some help with implementing the other fixit, or you just need some extra time? It seems to be almost the same as your second fixit<br class="m_-7269145279221876740gmail_msg">
<br class="m_-7269145279221876740gmail_msg">
<br class="m_-7269145279221876740gmail_msg">
<br class="m_-7269145279221876740gmail_msg">
================<br class="m_-7269145279221876740gmail_msg">
Comment at: clang-tidy/misc/<wbr>StringCompareCheck.cpp:69-70<br class="m_-7269145279221876740gmail_msg">
+    diag(Matched->getLocStart(),<br class="m_-7269145279221876740gmail_msg">
+         "do not use 'compare' to test equality of strings; "<br class="m_-7269145279221876740gmail_msg">
+         "use the string equality operator instead");<br class="m_-7269145279221876740gmail_msg">
+<br class="m_-7269145279221876740gmail_msg">
----------------<br class="m_-7269145279221876740gmail_msg">
Take this warning to some static const global variable<br class="m_-7269145279221876740gmail_msg">
<br class="m_-7269145279221876740gmail_msg">
<br class="m_-7269145279221876740gmail_msg">
================<br class="m_-7269145279221876740gmail_msg">
Comment at: clang-tidy/misc/<wbr>StringCompareCheck.cpp:71<br class="m_-7269145279221876740gmail_msg">
+         "use the string equality operator instead");<br class="m_-7269145279221876740gmail_msg">
+<br class="m_-7269145279221876740gmail_msg">
+  if (const auto *Matched = Result.Nodes.getNodeAs<Stmt>("<wbr>match2")) {<br class="m_-7269145279221876740gmail_msg">
----------------<br class="m_-7269145279221876740gmail_msg">
match1 and match2 are in different matchers (passed to register matchers)?<br class="m_-7269145279221876740gmail_msg">
<br class="m_-7269145279221876740gmail_msg">
If so put return here after diag to finish control flow for this case.<br class="m_-7269145279221876740gmail_msg">
<br class="m_-7269145279221876740gmail_msg">
<br class="m_-7269145279221876740gmail_msg">
================<br class="m_-7269145279221876740gmail_msg">
Comment at: clang-tidy/misc/<wbr>StringCompareCheck.cpp:81<br class="m_-7269145279221876740gmail_msg">
+      auto Diag = diag(Matched->getLocStart(),<br class="m_-7269145279221876740gmail_msg">
+                       "do not use 'compare' to test equality of strings; "<br class="m_-7269145279221876740gmail_msg">
+                       "use the string equality operator instead");<br class="m_-7269145279221876740gmail_msg">
----------------<br class="m_-7269145279221876740gmail_msg">
and use it here<br class="m_-7269145279221876740gmail_msg">
<br class="m_-7269145279221876740gmail_msg">
<br class="m_-7269145279221876740gmail_msg">
================<br class="m_-7269145279221876740gmail_msg">
Comment at: clang-tidy/misc/<wbr>StringCompareCheck.h:10-11<br class="m_-7269145279221876740gmail_msg">
+<br class="m_-7269145279221876740gmail_msg">
+#ifndef STRING_COMPARE_CHECK_H<br class="m_-7269145279221876740gmail_msg">
+#define STRING_COMPARE_CHECK_H<br class="m_-7269145279221876740gmail_msg">
+<br class="m_-7269145279221876740gmail_msg">
----------------<br class="m_-7269145279221876740gmail_msg">
This should be much longer string. Do you know about ./add_new_check?<br class="m_-7269145279221876740gmail_msg">
<br class="m_-7269145279221876740gmail_msg">
Please make one similar to other checks<br class="m_-7269145279221876740gmail_msg">
<br class="m_-7269145279221876740gmail_msg">
<br class="m_-7269145279221876740gmail_msg">
================<br class="m_-7269145279221876740gmail_msg">
Comment at: clang-tidy/misc/<wbr>StringCompareCheck.h:36<br class="m_-7269145279221876740gmail_msg">
+<br class="m_-7269145279221876740gmail_msg">
+#endif // STRING_COMPARE_CHECK_H<br class="m_-7269145279221876740gmail_msg">
----------------<br class="m_-7269145279221876740gmail_msg">
DITTO<br class="m_-7269145279221876740gmail_msg">
<br class="m_-7269145279221876740gmail_msg">
<br class="m_-7269145279221876740gmail_msg">
================<br class="m_-7269145279221876740gmail_msg">
Comment at: test/clang-tidy/misc-string-<wbr>compare.cpp:35-40<br class="m_-7269145279221876740gmail_msg">
+  if (str1.compare(str2)) {<br class="m_-7269145279221876740gmail_msg">
+  }<br class="m_-7269145279221876740gmail_msg">
+  // CHECK-MESSAGES: [[@LINE-2]]:7: warning: do not use 'compare' to test equality of strings; use the string equality operator instead [misc-string-compare]<br class="m_-7269145279221876740gmail_msg">
+  if (!str1.compare(str2)) {<br class="m_-7269145279221876740gmail_msg">
+  }<br class="m_-7269145279221876740gmail_msg">
+  // CHECK-MESSAGES: [[@LINE-2]]:8: warning: do not use 'compare' to test equality of strings; use the string equality operator instead [misc-string-compare]<br class="m_-7269145279221876740gmail_msg">
----------------<br class="m_-7269145279221876740gmail_msg">
Why this one doesn't have fixit hint?<br class="m_-7269145279221876740gmail_msg">
<br class="m_-7269145279221876740gmail_msg">
<br class="m_-7269145279221876740gmail_msg">
================<br class="m_-7269145279221876740gmail_msg">
Comment at: test/clang-tidy/misc-string-<wbr>compare.cpp:70<br class="m_-7269145279221876740gmail_msg">
+  }<br class="m_-7269145279221876740gmail_msg">
+  // CHECK-MESSAGES: [[@LINE-2]]:7: warning: do not use 'compare' to test equality of strings;<br class="m_-7269145279221876740gmail_msg">
+  if (str3->compare(str2) == 0) {<br class="m_-7269145279221876740gmail_msg">
----------------<br class="m_-7269145279221876740gmail_msg">
no fixit?<br class="m_-7269145279221876740gmail_msg">
<br class="m_-7269145279221876740gmail_msg">
<br class="m_-7269145279221876740gmail_msg">
<a href="https://reviews.llvm.org/D27210" rel="noreferrer" class="m_-7269145279221876740gmail_msg" target="_blank">https://reviews.llvm.org/<wbr>D27210</a><br class="m_-7269145279221876740gmail_msg">
<br class="m_-7269145279221876740gmail_msg">
<br class="m_-7269145279221876740gmail_msg">
<br class="m_-7269145279221876740gmail_msg">
</blockquote></div>
</div></div></blockquote></div><br></div></div>