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

FURB168 false negative and false positives #15776

Open
dscorbett opened this issue Jan 27, 2025 · 1 comment · May be fixed by #15779
Open

FURB168 false negative and false positives #15776

dscorbett opened this issue Jan 27, 2025 · 1 comment · May be fixed by #15779
Labels
bug Something isn't working rule Implementing or modifying a lint rule

Comments

@dscorbett
Copy link

Description

isinstance-type-none (FURB168) has a false negative and two false positives in Ruff 0.9.3.

The rule only triggers when the first argument to isinstance is an identifier. This is a false negative because any expression would work in that context.

$ cat >furb168_1.py <<'#EOF'
d = {"x": 1}
print(isinstance(d.get("x"), type(None)))
#EOF

$ python furb168_1.py
False

$ ruff --isolated check --preview --select FURB168 furb168_1.py
All checks passed!

If the expression consists purely of certain literals and operators, like 1 + 1, applying the FURB168 fix would produce a warning like SyntaxWarning: "is" with 'int' literal. Did you mean "=="?. That might be a reason the rule is currently so restricted, but I think it is better to apply the fix: the code is suspect either way but at least with the fix the user gets an explicit warning.

The first false positive is that the rule does not check whether type is the built-in type. (It does correctly check isinstance.)

$ cat >furb168_2.py <<'#EOF'
def type(x):
    return int
i = 1
print(isinstance(i, type(None)))
#EOF

$ python furb168_2.py
True

$ ruff --isolated check --preview --select FURB168 furb168_2.py --fix
Found 1 error (1 fixed, 0 remaining).

$ cat furb168_2.py
def type(x):
    return int
i = 1
print(i is None)

$ python furb168_2.py
False

The second false positive is that None | None is accepted as meaning type(None). That raises a TypeError which the fix suppresses.

$ cat >furb168_3.py <<'#EOF'
print(isinstance(abs, None | None))
#EOF

$ python furb168_3.py
Traceback (most recent call last):
  File "furb168_3.py", line 1, in <module>
    print(isinstance(abs, None | None))
                          ~~~~~^~~~~~
TypeError: unsupported operand type(s) for |: 'NoneType' and 'NoneType'

$ ruff --isolated check --preview --select FURB168 furb168_3.py --fix
Found 1 error (1 fixed, 0 remaining).

$ python furb168_3.py
False
@dylwil3
Copy link
Collaborator

dylwil3 commented Jan 27, 2025

Thank you! I'm more interested in resolving the false positives than the false negative.

@dylwil3 dylwil3 added bug Something isn't working rule Implementing or modifying a lint rule labels Jan 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working rule Implementing or modifying a lint rule
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants