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

TRIO115 false positive with with sleep(var) where var starts as 0 #9935

Closed
mikenerone opened this issue Feb 11, 2024 · 6 comments
Closed

TRIO115 false positive with with sleep(var) where var starts as 0 #9935

mikenerone opened this issue Feb 11, 2024 · 6 comments
Assignees
Labels
bug Something isn't working

Comments

@mikenerone
Copy link

mikenerone commented Feb 11, 2024

Ruff 0.2.1

The following snippet illustrates a TRIO115 false positive when sleep() is passed a variable whose initial value was 0, but changes later in a loop construct. Similar to #9934, this is already fixed in flake8-trio. This seems to be strongly indicating that recent fixes to flake8-trio need to be ported in general.

import trio

async def main() -> None:
    sleep = 0
    for _ in range(2):
        # ↓↓↓↓↓ TRIO115 [*] Use `trio.lowlevel.checkpoint()` instead of `trio.sleep(0)`
        await trio.sleep(sleep)
        sleep = 10

trio.run(main)
@charliermarsh
Copy link
Member

charliermarsh commented Feb 11, 2024

Thanks. I don't think this has been "fixed" in flake8-trio -- rather, flake8-trio never flags with a non-constant, it only flags if you pass exactly 0. You can see the source here: https://github.com/Zac-HD/flake8-trio/blob/a652441fb3a9a92f14da3542bb2a0a1ba8f03f87/flake8_trio/visitors/visitors.py#L219.

@charliermarsh
Copy link
Member

We could consider doing the same, or just check if the variable is ever re-bound to something else.

@charliermarsh charliermarsh added the bug Something isn't working label Feb 11, 2024
@charliermarsh charliermarsh changed the title TRIO115 false positive with with sleep(var) where var starts as 0 (already fixed in flake8-trio - ruff is out of date) TRIO115 false positive with with sleep(var) where var starts as 0 Feb 11, 2024
@mikenerone
Copy link
Author

mikenerone commented Feb 11, 2024

Thanks. I don't think this has been "fixed" in flake8-trio -- rather, flake8-trio never flags with a non-constant, it only flags if you pass exactly 0.

We could consider doing the same, or just check if the variable is ever re-bound to something else.

@charliermarsh Oh, I see - you're correct. Not flagging on a non-constant sounds reasonable to me. With a variable, even if not re-bound, it seems like the developer is signaling that it's considered changeable in at least some sense.

@charliermarsh
Copy link
Member

Agreed.

@charliermarsh charliermarsh self-assigned this Feb 12, 2024
@jakkdl
Copy link

jakkdl commented Feb 12, 2024

flake8-trio dev here:
the "constant" in the source code is just how the AST labels literals, flake8-trio doesn't track the values of any variables (and has no straightforward way of doing so)

dhruvmanila pushed a commit that referenced this issue Mar 13, 2024
## Summary

Fix "TRIO115 false positive with with sleep(var) where var starts as 0"
#9935 based on the discussion in the issue.

## Test Plan

Issue code added to fixture
@robincaloudis
Copy link
Contributor

As #10376 is merged, it looks like this issue could be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants