Expand asyncio-dangling-task (RUF006) to include new_event_loop
#9976
Conversation
|
charliermarsh
changed the title
asyncio-dangling-task (RUF006) to include new_event_loop
charliermarsh
added
rule
charliermarsh
force-pushed
the
improve-ruf006
branch
from
586666e
to
19ec2be
Compare
|
@charliermarsh What do you think of my comments? |
|
@tyilo - Which comments specifically? Sorry if I missed something. |
| expr.range(), | ||
| )); | ||
| } | ||
|
|
||
| // Ex) `loop = asyncio.get_running_loop(); loop.create_task(...)` | ||
| // Ex) `loop = ...; loop.create_task(...)` | ||
| if let Expr::Attribute(ast::ExprAttribute { attr, value, .. }) = func.as_ref() { | ||
| if attr == "create_task" { | ||
| if typing::resolve_assignment(value, semantic).is_some_and(|call_path| { |
There was a problem hiding this comment.
I don't know if ruff's typing analysis is good enough for this, but it would be better to check that the type of value is asyncio.AbstractEventLoop, so that the following would also be flagged:
import asyncio
def create_event_loop() -> asyncio.AbstractEventLoop:
return asyncio.new_event_loop()
loop = create_event_loop()
loop.create_task(asyncio.sleep(1))There was a problem hiding this comment.
Yeah we don't quite support that yet unfortunately.
| }) { | ||
| return Some(Diagnostic::new( | ||
| AsyncioDanglingTask { | ||
| expr: compose_call_path(value).unwrap_or_else(|| "asyncio".to_string()), |
There was a problem hiding this comment.
I think loop would be a better fallback:
| expr: compose_call_path(value).unwrap_or_else(|| "asyncio".to_string()), | |
| expr: compose_call_path(value).unwrap_or_else(|| "loop".to_string()), |
There was a problem hiding this comment.
Will change, though I think in practice this should never happen. I wish it were encoded in the type system, but e.g. value here has to be a Name, and compose_call_path always returns Some given a name.
Ah, sorry I haven't used GitHub's review functionality before, so my comments weren't published 😅 They should be visible now... |
…astral-sh#9976) ## Summary Fixes astral-sh#9974 ## Test Plan I added some new test cases.
Summary
Fixes #9974
Test Plan
I added some new test cases.