MartinThoma / flake8-simplify

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

[Adjust Rule] SIM904 false positive #157

Open navotarad opened 2 years ago

navotarad commented 2 years ago

Desired change

Explanation

The warning points at the wrong line in the code, and is unavoidable in some situations

Example 1

In this example the warning message is confusing as it is raised against the a = {'b': 'c'} line, which is what the rule recommends writing

def f(a=None):
    if a is None:
        a = {"b": "c"}
    else:
        a["b"] = "c"
scratch.py:3:9: SIM904 Initialize dictionary 'a' directly

The warning can be avoided by writing the logic as

def f(a=None):
    if a is None:
        a = {}
    a['b'] = 'c'

which could be argued is better but it does not solve the issue of the next example.

Example 2

Here the creation logic for the dictionary is distinct from the update logic, and it also triggers the warning

def f(a=None):
    if a is None:
        a = {"b": "x"}
    else:
        a["b"] = "c"
scratch.py:3:9: SIM904 Initialize dictionary 'a' directly

Again the code can be rewritten as

def f(a=None):
    if a is None:
        a = {}
        b = "x"
    else:
        b = "c"
    a["b"] = b

to avoid the warning, but I certainly don't think this is better