MartinThoma / flake8-simplify

❄ A flake8 plugin that helps you to simplify code
MIT License
185 stars 19 forks source link

[Adjust Rule]: SIM204, SIM205, SIM206, SIM207 on sets #116

Closed Nathanmalnoury closed 2 years ago

Nathanmalnoury commented 2 years ago

Hi, thanks a lot for this flake8 library, It's been helping us a lot with keeping a nice code quality 1

I think we've spotted a small issue when it comes to replacing not < with >= rules using sets.

Desired change

Using sets, saying not (a < b) is not equivalent to a >= b:

I am unsure what the adjustment should be, maybe omit these rules when comparing sets ?

Explanation

On sets, (not (a < b)) != (b >= a)

Example

This is an example where the mentioned rule(s) would currently be suboptimal:

a = {4, 5}
b = {1, 2, 3}

# this raises: SIM204 Use 'a >= b' instead of 'not (a < b)'
assert not (a < b)   # no elements of a are in b; would pass 

# when making the suggesting change:
assert a >= b  # all elements of b are in a -> AssertionError
MartinThoma commented 2 years ago

Thank you very much! I wasn't aware that sets could be compared.

Understanding the issue

If I get it right, then a < b is equivalent to a.issubset(b)?

And b < a is equivalent to b.issuperset(a)?

Fixing the issue

If that is the case, then those rule suddenly becomes WAY more complicated. The way flake8-simplify works is on the abstract syntax tree. At this level, I don't have any type information. I might need to disable those 4 rules to prevent false-positives :smiling_face_with_tear:

Nathanmalnoury commented 2 years ago

I guess you meant a > b in your second example:

(Here is the link to python's doc about them: https://docs.python.org/3/library/stdtypes.html#frozenset.issubset)

I think you might have to disable those 4 rules indeed 😬 Sorry about that