Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SIM908] False-positives for strings #126

Closed
jonyscathe opened this issue Mar 28, 2022 · 10 comments
Closed

[SIM908] False-positives for strings #126

jonyscathe opened this issue Mar 28, 2022 · 10 comments

Comments

@jonyscathe
Copy link

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
Copy link
Owner

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
Copy link
Owner

@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
Copy link
Owner

Nevermind, I can reproduce it: #128 :-)

@MartinThoma MartinThoma changed the title False positive for SIM908 [SIM908] False-positives for strings Mar 29, 2022
MartinThoma added a commit that referenced this issue Mar 29, 2022
MartinThoma added a commit that referenced this issue Mar 29, 2022
Ensure that the assigned name is equal to the name in the if.test

See #126
MartinThoma added a commit that referenced this issue Mar 29, 2022
Ensure that the assigned name is equal to the name in the if.test

See #126
@MartinThoma
Copy link
Owner

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

@MartinThoma
Copy link
Owner

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

@MartinThoma
Copy link
Owner

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

@jonyscathe
Copy link
Author

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
Copy link
Owner

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
Copy link
Author

jonyscathe commented Mar 29, 2022

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

@MartinThoma
Copy link
Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants