[PATCH] D80023: [clang-tidy] Add abseil-string-find-str-contains checker.

Wed May 20 09:16:11 PDT 2020

ymandel added a comment.

LGTM, but leaving for one of the other reviewers to give you the official "Accept Revision".

================
Comment at: clang-tools-extra/clang-tidy/abseil/StringFindStrContainsCheck.cpp:72
+           binaryOperator(hasOperatorName("=="),
+                          hasEitherOperand(ignoringParenImpCasts(StringNpos)),
+                          hasEitherOperand(ignoringParenImpCasts(StringFind))),
----------------
tdl-g wrote:
> njames93 wrote:
> > ymandel wrote:
> > > Would `hasOperands` replace these two separate calls to `hasEitherOperand`? Same below lines 79-80 (I think it's a new matcher, fwiw).
> > Just added, I added it to remove calls of multiple hasEitherOperand calls for the reason of preventing the matchers matching the same operand. I just got a little lost in my previous comment
> Switched to hasOperands.  I agree that expresses the intent better than two independent calls of hasEitherOperand.
> 
> Note that it was working fine before--the test cases confirmed that it wasn't matching string::npos == string::npos or s.find(a) == s.find(a).  I'm guessing that's because the matchers inside hasEitherOperand have bindings, and if the matcher matched but one of the bindings was missing, the rewriterule suppressed itself?  Is this a plausible theory?
> 
> (The question moot since hasOperands is better overall, but I'd like to understand why it was working before.)
I was wondering the same when I noticed this. I think its something like this: one of the arguments falls into bucket 1, one of the arguments falls into bucket 2.  Since a single argument cannot simultaneously fall into two buckets (the predicates are mutually exlusive), it must be the case that the arguments are different.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80023/new/

https://reviews.llvm.org/D80023