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

SIM108 false negative when variable has type annotation #15554

Open
nickdrozd opened this issue Jan 17, 2025 · 3 comments · May be fixed by #15665
Open

SIM108 false negative when variable has type annotation #15554

nickdrozd opened this issue Jan 17, 2025 · 3 comments · May be fixed by #15665
Labels
rule Implementing or modifying a lint rule

Comments

@nickdrozd
Copy link
Contributor

Assignment with if/else gets flagged with SIM108, suggesting ternary operator. Good. But if the variable is annotated, not flagged:

from random import randint

if randint(0, 3):
    x: int = 4  # not flagged
else:
    x = 5

Could be if var annotated or else var, not flagged. Also not flagged if both vars have annotation, even if annotation is the same.

@MichaReiser MichaReiser added the rule Implementing or modifying a lint rule label Jan 17, 2025
@MichaReiser
Copy link
Member

That makes sense to me but is slightly more complicated because that only works if both branches infer to the same type, unless the fix removes the type annotation.

@nickdrozd
Copy link
Contributor Author

If the variable is declared with different types in the branches, Mypy flags it as a redefinition:

if randint(0, 3):
    x: int = 4
else:
    x: str = 'q'  # mypy flags as redefined

(It's not even clear whether there is one variable x defined in two places or two separate variables each called x. Yet another reason not to define a variable in branches.)

In this case it would be fine to add a new annotation that is the union of types in the branches:

x: int | str = 4 if randint(0, 3) else 'q'

@MichaReiser
Copy link
Member

The problem I see is more about:

if randint(0, 3):
    x: int = 4
else:
    x = 'q'  # mypy flags as redefined

Mypy might also flag this as redefinition but I'd have to check if this is true for every type checker (including red knot). But I could still see us flagging this but we may not want to provide a fix for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rule Implementing or modifying a lint rule
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants