Proposed Check: Flag Improper Use of string.find()

[gsexton]

Title: Flag Improper Use of string.find()

Description The string.find() method is easy to mis-use. A common error is:

mystring='abc'
if mystring.find('def'):
  print("def is contained in {}".format(mystring)

The issue is that if string.find fails, it returns -1, which is "truthy". If string.find() locates the output at the beginning of the line, it returns 0 which is "false".

mystring='abc'
if mystring.find('a'):
  print("a is in {}".format(mystring))
else:
  print('a is not in {}'.format(mystring))

It would be awesome if flake8-bugbear could flag any logical test involving string.find() that does not compare to a numerical value.

[cooperlees]

Hi,

Thanks for the suggestion. Yes, the find API is not very pythonic, but I understand why it is the way it is.

As for the check, I think we would need to tighten it a little more cause your suggestion feels a little noisy. There will be many developers who know this and are using it correctly and we would fire the lint error. I can't think of a clean way how we could AST parse python to always only notify when this is legit wrong usage. So we would need to think about this more before I feel this could be considered.

[gsexton]

@cooperlees

Let's review the "truth table" for the case:

mystring='abc'

mystring.find("a") (False)
mystring.find("d") (True)
mystring.find('b') (True)

The only case where found/not-found have different outcomes is when the string under test starts with the test value, but then the "truthiness" is inverted.

In short, you can only treat the output as a boolean value if you're looking at the beginning of the string.

mystring='abc def'
if not mystring.find('abc'):
  print("String starts with abc")

if mystring.find("def"):
  print("String does NOT start with def and we can't say if it's in mystring")

In both cases, the logic is inverted and confusing. Honestly, if I were doing a code review and saw this, I would insist it be re-written as:

if mystring.find("abc")==0:
  print("String starts with abc")

if mystring.find("abc")!=0:
  print("String does not start with abc")

So, to wrap it all up, my feeling is that yes, it's possible in a very narrow circumstance to correctly use it as a boolean, but those cases are so few, and the required code is so confusing, that it's a bad idea.

[cooperlees]

Ok, so you would like to enforce always using the explicit int comparison of mystring.find("foo") == 0 and !=0 to ensure it behaves as most people assume.

If that's the case, I guess this is narrow enough and a subtle bug that would be nice. I'd accept a check for ensuring people are being explicit and checking the returned int, not blind "truthyness". Please add tests for all scenarios discussed here please in the PR.

• Also ensure README.rst is updated with explanation too