MartinThoma / flake8-simplify

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

[Adjust Rule] SIM401 False positive, rule too greedy #177

Open joaoe opened 1 year ago

joaoe commented 1 year ago

Desired change

Example

This testcase shows the false posivie

container_1 = unrelated_container, expensive_side_effect = None
key = "value"
if key in container_1 :
    retval = unrelated_container[key]
else:
    retval  = expensive_side_effect()

Output

sim401_bug.py:3:1: SIM401 Use 'retval = container_1.get(key, expensive_side_effect())' instead of an if-block

Issues with the rule

  1. container_1 is not a dict, could be any generic container
  2. the variable accessed inside the if block is not the same one in the condition
  3. the else block is non-trival code with side-effects

Suggestion

The rule should only trigger in the very strict case

if key in container:
    retval = container[key]
else:
    retval  = "value"

Where

  1. container must be accessed in both the condition with the form key in container and then the if block container[key]
  2. key must be reused both in the condition and if block
  3. "value" must be a literal or simple variable name e.g. "value" or value. Else, stuff like value.property or value() can have side-effects which change the state of the application.