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

Ruff (and flake8) doesn't report a situation where a variable _may_ be unbound #2493

Closed
DMRobertson opened this issue Feb 2, 2023 · 7 comments

Comments

@DMRobertson
Copy link

Consider the following snippet:

from random import random


def coin_toss():
    return random.randint(0, 2)


def maybe_runtime_error():
    if coin_toss():
        x = []

    # runtime error if coin_toss returned False, but not detected by the linter
    x = list(x)
    print(x)


def runtime_error():
    # always a runtime error, detected by linter
    y = list(y)
    print(y)

There are two problems with this snippet.

  1. In maybe_runtime_error, the name x may not be bound to a variable when we come to evaluate list(x). This means we might get a runtime UnboundLocalError.
  2. In runtime_error, the name y is never bound to a variable when we come to evaluate list(y). We'll always get a runtime UnboundLocalError when calling this function.

Problem (2) is spotted by ruff, but problem (1) is not.

$ ruff --isolated temp.py
temp.py:19:14: F821 Undefined name `y`
Found 1 error.

I was a little surprised by this! But flake8 has the same output:

$ flake8 --version && flake8 temp.py
6.0.0 (mccabe: 0.7.0, pycodestyle: 2.10.0, pyflakes: 3.0.1) CPython 3.10.9 on Linux
temp.py:19:14: F821 undefined name 'y'

I'm not sure if it's fair game for ruff to spot the problem with x---perhaps that's more the remit of a typechecker like mypy. (Though mypy didn't warn about this either when I tried it!) It'd be great if it was feasible to spot this at lint-time.

Configuration
[tool.ruff]
line-length = 88

# See https://github.com/charliermarsh/ruff/#pycodestyle
# for error codes. The ones we ignore are:
#  E731: do not assign a lambda expression, use a def
#  E501: Line too long (black enforces this for us)
#
# flake8-bugbear compatible checks. Its error codes are described at
# https://github.com/charliermarsh/ruff/#flake8-bugbear
#  B019: Use of functools.lru_cache or functools.cache on methods can lead to memory leaks
#  B023: Functions defined inside a loop must not use variables redefined in the loop
#  B024: Abstract base class with no abstract method.
ignore = [
    "B019",
    "B023",
    "B024",
    "E501",
    "E731",
]
select = [
    # pycodestyle checks.
    "E",
    "W",
    # pyflakes checks.
    "F",
    # flake8-bugbear checks.
    "B0",
    # flake8-comprehensions checks.
    "C4",
]

And as for the version:

$ ruff --version
ruff 0.0.240
@charliermarsh
Copy link
Member

Appreciate the clear issue! Right now, (1) is outside of our capabilities -- although I'm surprised that Mypy doesn't flag this... Pyright does:

❯ pyright foo.py
/Users/crmarsh/workspace/ruff/foo.py
  /Users/crmarsh/workspace/ruff/foo.py:13:14 - error: "x" is possibly unbound (reportUnboundVariable)
1 error, 0 warnings, 0 informations
Completed in 0.467sec

We could potentially include some kind of "possibly unbound" code for those cases. I'd have to think on it 🤔

@DMRobertson
Copy link
Author

@charliermarsh
Copy link
Member

I'm gonna close for now just because it's not super actionable. I'd like Ruff to be able to handle cases like this eventually, but it'd be a fairly big project and probably outside of the scope of a bug fix. Hope that intent is clear. I appreciate the issue!

@frankdilo
Copy link

just wanted to drop in and say that I would find this extremely useful

it's really easy in long Python functions and code blocks when refactoring and moving variables inside nested conditionals or inline functions to nest a variable used somewhere else and only discover at runtime via a Sentry notification days later

@sascharo
Copy link

Is this still unsupported?

@clemente0731
Copy link

STILL ~
Ruff cannot detect the problem of "referenced before assignment" and shows all passes. In an if-else scenario, for example, a variable is declared in the else block, but it is not declared in the if block. Then, this corresponding variable is used again in a subsequent inevitable path.

@Garrett-R
Copy link

Garrett-R commented Oct 30, 2024

Note: this is the subject of this issue: #6902

STILL ~

It sounds like a pretty tough one, but I'm sure Ruff maintainers would welcome a PR for it.

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

6 participants