MartinThoma / flake8-simplify

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

[SIM908] False-positives for strings #126

Closed jonyscathe closed 2 years ago

jonyscathe commented 2 years ago

Hey,

I have some code to modify a string. The string will be a number with a letter at the end. If the number has a decimal point in it, it swaps the letter to where the dot is. For example '10k' will stay as '10k', but '4.7k' becomes '4k7' There is probably a simpler way to do this, but currently the code for this is:

    if '.' in resistance:
        # Swap '.' with suffix
        resistance = resistance.replace('.', resistance[-1])[:-1]

The if statement above is flagging SIM908 which I really think it shouldn't be

MartinThoma commented 2 years ago

SIM908: Use dict.get(key) is triggered by if-statements which have a subscript (and a bit more ... source).

Example

if "." in resistance:
    # Swap '.' with suffix
    resistance = resistance.replace(".", resistance[-1])[:-1]

AST

$ python -m astpretty --no-show-offsets /dev/stdin <<< `cat example.py`
Module(
    body=[
        If(
            test=Compare(
                left=Constant(value='.', kind=None),
                ops=[In()],
                comparators=[Name(id='resistance', ctx=Load())],
            ),
            body=[
                Assign(
                    targets=[Name(id='resistance', ctx=Store())],
                    value=Subscript(
                        value=Call(
                            func=Attribute(
                                value=Name(id='resistance', ctx=Load()),
                                attr='replace',
                                ctx=Load(),
                            ),
                            args=[
                                Constant(value='.', kind=None),
                                Subscript(
                                    value=Name(id='resistance', ctx=Load()),
                                    slice=UnaryOp(
                                        op=USub(),
                                        operand=Constant(value=1, kind=None),
                                    ),
                                    ctx=Load(),
                                ),
                            ],
                            keywords=[],
                        ),
                        slice=Slice(
                            lower=None,
                            upper=UnaryOp(
                                op=USub(),
                                operand=Constant(value=1, kind=None),
                            ),
                            step=None,
                        ),
                        ctx=Load(),
                    ),
                    type_comment=None,
                ),
            ],
            orelse=[],
        ),
    ],
    type_ignores=[],
)
MartinThoma commented 2 years ago

@jonyscathe Please give me you output for those two commands:

python --version
flake8 --version

It should look like this:

$ python --version
Python 3.10.2

$ flake8 --version
4.0.1 (flake8-bugbear: 22.1.11, flake8-comprehensions: 3.8.0, flake8-eradicate: 1.2.0, flake8-executable: 2.1.1, flake8-pytest-style: 1.6.0, flake8-raise: 0.0.5, flake8-string-format: 0.3.0, flake8_assert_msg.flake8_assert_msg: (1, 1, 1), flake8_builtins: 1.5.2, flake8_implicit_str_concat: 0.2.0, flake8_isort:
4.1.1, flake8_simplify: 0.18.2, mccabe: 0.6.1, naming: 0.12.1, pycodestyle: 2.8.0, pyflakes: 2.4.0, radon: 4.0.0) CPython 3.10.2 on Linux

When I execute flake8 on your example, I get:

$ flake8 example.py 
example.py:1:11: F821 undefined name 'resistance'
example.py:3:18: F821 undefined name 'resistance'
example.py:3:42: F821 undefined name 'resistance'

There is no SIM error message. That could be due to a Python/flake8 version, that has a different AST and is not affected by this issue.

MartinThoma commented 2 years ago

Nevermind, I can reproduce it: https://github.com/MartinThoma/flake8-simplify/pull/128 :-)

MartinThoma commented 2 years ago

The issue should be fixed in flake8-simplify==0.19.1. Thank you for your help!

MartinThoma commented 2 years ago

I'm curious: Do you write a game? "resistance" sounds a bit like it :-)

MartinThoma commented 2 years ago

Also, why do you have so weird repository names like https://github.com/jonyscathe/sadasdddddddddddd ?

jonyscathe commented 2 years ago

Thanks for fixing this so quickly.

I didn't realise I had any public repositories on github... not too sure what that is doing there! I have written an electronics part management library. resistance is just for the resistance value of a resistor. And I need to be able to change strings between how they are stored in a database, as say '4.7 kΩ' to how they are typically printed on schematics '4k7'. So nothing quite as exciting as a game!

MartinThoma commented 2 years ago

Oh, I understand 💡

Is it anything public that you wrote? Or something you would like to put on pypi but you need support to do so?

jonyscathe commented 2 years ago

It is a private project at this point - I don't own the IP for it myself.

MartinThoma commented 2 years ago

I understand. If it becomes free software at some point and you need help in packing it, you can ping me :-)