MartinThoma / flake8-simplify

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

[Adjust Rule] Remove SIM116 due to inefficiency/code evaluation problem #113

Closed Effervex closed 2 years ago

Effervex commented 2 years ago

Desired change

Explanation

This style guideline may result in better looking code, but it is inefficient and can even create errors. Each dict value is interpreted whenever the dict is evaluated.

Example

some_array = [0,1] if x else [0,1,2]
mapping = {
  "foo": "bar",
  "foo2": long_expensive_function()
  "foo3": three_arg_func(some_array[0], some_array[1], some_array[2])
}
return mapping.get("foo",42) # This will still evaluate "foo2", and if x is True, completely crash when evaluating "foo3"
MartinThoma commented 2 years ago

thank you for the explanation - I understand that function calls might be problematic. My plan now is to only show this message if the values of mapping are constants. If at least one argument is a function, SIM116 should not be shown.

Effervex commented 2 years ago

That's a nice solution! I didn't think that level of detail could be checked, but that's useful.

MartinThoma commented 2 years ago

This is an example of a false-positive:

if a == "foo":
    return "bar"
elif a == "bar":
    return baz() # <----- that makes it a false-positive
elif a == "boo":
    return "ooh"
else:
    return 42

Here is its AST:

$ python -m astpretty --no-show-offsets /dev/stdin <<< `cat example.py`
Module(
    body=[
        If(
            test=Compare(
                left=Name(id='a', ctx=Load()),
                ops=[Eq()],
                comparators=[Constant(value='foo', kind=None)],
            ),
            body=[
                Return(
                    value=Constant(value='bar', kind=None),
                ),
            ],
            orelse=[
                If(
                    test=Compare(
                        left=Name(id='a', ctx=Load()),
                        ops=[Eq()],
                        comparators=[Constant(value='bar', kind=None)],
                    ),
                    body=[
                        Return(
                            value=Call(
                                func=Name(id='baz', ctx=Load()),
                                args=[],
                                keywords=[],
                            ),
                        ),
                    ],
                    orelse=[
                        If(
                            test=Compare(
                                left=Name(id='a', ctx=Load()),
                                ops=[Eq()],
                                comparators=[Constant(value='boo', kind=None)],
                            ),
                            body=[
                                Return(
                                    value=Constant(value='ooh', kind=None),
                                ),
                            ],
                            orelse=[
                                Return(
                                    value=Constant(value=42, kind=None),
                                ),
                            ],
                        ),
                    ],
                ),
            ],
        ),
    ],
    type_ignores=[],
)