Skip to content

Commit

Permalink
Stabilize ASYNC100, ASYNC109, ASYNC110, ASYNC115 and `ASYNC11…
Browse files Browse the repository at this point in the history
…6` behavior changes (#12844)

Closes #12268
  • Loading branch information
MichaReiser committed Aug 14, 2024
1 parent c97d157 commit 13702e1
Show file tree
Hide file tree
Showing 18 changed files with 449 additions and 895 deletions.
38 changes: 0 additions & 38 deletions crates/ruff_linter/src/rules/flake8_async/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,44 +115,6 @@ impl MethodName {
| MethodName::TrioCancelScope
)
}

/// Returns associated module
pub(super) fn module(self) -> AsyncModule {
match self {
MethodName::AsyncIoTimeout | MethodName::AsyncIoTimeoutAt => AsyncModule::AsyncIo,
MethodName::AnyIoMoveOnAfter
| MethodName::AnyIoFailAfter
| MethodName::AnyIoCancelScope => AsyncModule::AnyIo,
MethodName::TrioAcloseForcefully
| MethodName::TrioCancelScope
| MethodName::TrioCancelShieldedCheckpoint
| MethodName::TrioCheckpoint
| MethodName::TrioCheckpointIfCancelled
| MethodName::TrioFailAfter
| MethodName::TrioFailAt
| MethodName::TrioMoveOnAfter
| MethodName::TrioMoveOnAt
| MethodName::TrioOpenFile
| MethodName::TrioOpenProcess
| MethodName::TrioOpenSslOverTcpListeners
| MethodName::TrioOpenSslOverTcpStream
| MethodName::TrioOpenTcpListeners
| MethodName::TrioOpenTcpStream
| MethodName::TrioOpenUnixSocket
| MethodName::TrioPermanentlyDetachCoroutineObject
| MethodName::TrioReattachDetachedCoroutineObject
| MethodName::TrioRunProcess
| MethodName::TrioServeListeners
| MethodName::TrioServeSslOverTcp
| MethodName::TrioServeTcp
| MethodName::TrioSleep
| MethodName::TrioSleepForever
| MethodName::TrioTemporarilyDetachCoroutineObject
| MethodName::TrioWaitReadable
| MethodName::TrioWaitTaskRescheduled
| MethodName::TrioWaitWritable => AsyncModule::Trio,
}
}
}

impl MethodName {
Expand Down
26 changes: 1 addition & 25 deletions crates/ruff_linter/src/rules/flake8_async/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,10 @@ mod tests {
use anyhow::Result;
use test_case::test_case;

use crate::assert_messages;
use crate::registry::Rule;
use crate::settings::types::PreviewMode;
use crate::settings::LinterSettings;
use crate::test::test_path;
use crate::{assert_messages, settings};

#[test_case(Rule::CancelScopeNoCheckpoint, Path::new("ASYNC100.py"))]
#[test_case(Rule::TrioSyncCall, Path::new("ASYNC105.py"))]
Expand All @@ -37,27 +36,4 @@ mod tests {
assert_messages!(snapshot, diagnostics);
Ok(())
}

#[test_case(Rule::CancelScopeNoCheckpoint, Path::new("ASYNC100.py"))]
#[test_case(Rule::AsyncFunctionWithTimeout, Path::new("ASYNC109_0.py"))]
#[test_case(Rule::AsyncFunctionWithTimeout, Path::new("ASYNC109_1.py"))]
#[test_case(Rule::AsyncBusyWait, Path::new("ASYNC110.py"))]
#[test_case(Rule::AsyncZeroSleep, Path::new("ASYNC115.py"))]
#[test_case(Rule::LongSleepNotForever, Path::new("ASYNC116.py"))]
fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!(
"preview__{}_{}",
rule_code.noqa_code(),
path.to_string_lossy()
);
let diagnostics = test_path(
Path::new("flake8_async").join(path).as_path(),
&settings::LinterSettings {
preview: PreviewMode::Enabled,
..settings::LinterSettings::for_rule(rule_code)
},
)?;
assert_messages!(snapshot, diagnostics);
Ok(())
}
}
32 changes: 10 additions & 22 deletions crates/ruff_linter/src/rules/flake8_async/rules/async_busy_wait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use ruff_text_size::Ranged;

use crate::checkers::ast::Checker;
use crate::rules::flake8_async::helpers::AsyncModule;
use crate::settings::types::PreviewMode;

/// ## What it does
/// Checks for the use of an async sleep function in a `while` loop.
Expand Down Expand Up @@ -71,26 +70,15 @@ pub(crate) fn async_busy_wait(checker: &mut Checker, while_stmt: &ast::StmtWhile
return;
};

if matches!(checker.settings.preview, PreviewMode::Disabled) {
if matches!(qualified_name.segments(), ["trio", "sleep" | "sleep_until"]) {
checker.diagnostics.push(Diagnostic::new(
AsyncBusyWait {
module: AsyncModule::Trio,
},
while_stmt.range(),
));
}
} else {
if matches!(
qualified_name.segments(),
["trio" | "anyio", "sleep" | "sleep_until"] | ["asyncio", "sleep"]
) {
checker.diagnostics.push(Diagnostic::new(
AsyncBusyWait {
module: AsyncModule::try_from(&qualified_name).unwrap(),
},
while_stmt.range(),
));
}
if matches!(
qualified_name.segments(),
["trio" | "anyio", "sleep" | "sleep_until"] | ["asyncio", "sleep"]
) {
checker.diagnostics.push(Diagnostic::new(
AsyncBusyWait {
module: AsyncModule::try_from(&qualified_name).unwrap(),
},
while_stmt.range(),
));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ use ruff_text_size::Ranged;

use crate::checkers::ast::Checker;
use crate::rules::flake8_async::helpers::AsyncModule;
use crate::settings::types::PreviewMode;

/// ## What it does
/// Checks for `async` functions with a `timeout` argument.
Expand Down Expand Up @@ -87,17 +86,8 @@ pub(crate) fn async_function_with_timeout(
AsyncModule::AsyncIo
};

if matches!(checker.settings.preview, PreviewMode::Disabled) {
if matches!(module, AsyncModule::Trio) {
checker.diagnostics.push(Diagnostic::new(
AsyncFunctionWithTimeout { module },
timeout.range(),
));
}
} else {
checker.diagnostics.push(Diagnostic::new(
AsyncFunctionWithTimeout { module },
timeout.range(),
));
}
checker.diagnostics.push(Diagnostic::new(
AsyncFunctionWithTimeout { module },
timeout.range(),
));
}
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,7 @@ pub(crate) fn async_zero_sleep(checker: &mut Checker, call: &ExprCall) {
};

if let Some(module) = AsyncModule::try_from(&qualified_name) {
let is_relevant_module = if checker.settings.preview.is_enabled() {
matches!(module, AsyncModule::Trio | AsyncModule::AnyIo)
} else {
matches!(module, AsyncModule::Trio)
};
let is_relevant_module = matches!(module, AsyncModule::Trio | AsyncModule::AnyIo);

let is_sleep = is_relevant_module && matches!(qualified_name.segments(), [_, "sleep"]);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@ use ruff_python_ast::visitor::Visitor;
use ruff_python_ast::{StmtWith, WithItem};

use crate::checkers::ast::Checker;
use crate::rules::flake8_async::helpers::{AsyncModule, MethodName};
use crate::settings::types::PreviewMode;
use crate::rules::flake8_async::helpers::MethodName;

/// ## What it does
/// Checks for timeout context managers which do not contain a checkpoint.
Expand Down Expand Up @@ -88,17 +87,8 @@ pub(crate) fn cancel_scope_no_checkpoint(
return;
}

if matches!(checker.settings.preview, PreviewMode::Disabled) {
if matches!(method_name.module(), AsyncModule::Trio) {
checker.diagnostics.push(Diagnostic::new(
CancelScopeNoCheckpoint { method_name },
with_stmt.range,
));
}
} else {
checker.diagnostics.push(Diagnostic::new(
CancelScopeNoCheckpoint { method_name },
with_stmt.range,
));
}
checker.diagnostics.push(Diagnostic::new(
CancelScopeNoCheckpoint { method_name },
with_stmt.range,
));
}
Original file line number Diff line number Diff line change
Expand Up @@ -107,11 +107,7 @@ pub(crate) fn long_sleep_not_forever(checker: &mut Checker, call: &ExprCall) {
return;
};

let is_relevant_module = if checker.settings.preview.is_enabled() {
matches!(module, AsyncModule::AnyIo | AsyncModule::Trio)
} else {
matches!(module, AsyncModule::Trio)
};
let is_relevant_module = matches!(module, AsyncModule::AnyIo | AsyncModule::Trio);

let is_sleep = is_relevant_module && matches!(qualified_name.segments(), [_, "sleep"]);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,84 @@ ASYNC100.py:18:5: ASYNC100 A `with trio.move_on_after(...):` context does not co
19 | | ...
| |___________^ ASYNC100
|

ASYNC100.py:40:5: ASYNC100 A `with anyio.move_on_after(...):` context does not contain any `await` statements. This makes it pointless, as the timeout can only be triggered by a checkpoint.
|
39 | async def func():
40 | with anyio.move_on_after(delay=0.2):
| _____^
41 | | ...
| |___________^ ASYNC100
|

ASYNC100.py:45:5: ASYNC100 A `with anyio.fail_after(...):` context does not contain any `await` statements. This makes it pointless, as the timeout can only be triggered by a checkpoint.
|
44 | async def func():
45 | with anyio.fail_after():
| _____^
46 | | ...
| |___________^ ASYNC100
|

ASYNC100.py:50:5: ASYNC100 A `with anyio.CancelScope(...):` context does not contain any `await` statements. This makes it pointless, as the timeout can only be triggered by a checkpoint.
|
49 | async def func():
50 | with anyio.CancelScope():
| _____^
51 | | ...
| |___________^ ASYNC100
|

ASYNC100.py:55:5: ASYNC100 A `with anyio.CancelScope(...):` context does not contain any `await` statements. This makes it pointless, as the timeout can only be triggered by a checkpoint.
|
54 | async def func():
55 | with anyio.CancelScope(), nullcontext():
| _____^
56 | | ...
| |___________^ ASYNC100
|

ASYNC100.py:60:5: ASYNC100 A `with anyio.CancelScope(...):` context does not contain any `await` statements. This makes it pointless, as the timeout can only be triggered by a checkpoint.
|
59 | async def func():
60 | with nullcontext(), anyio.CancelScope():
| _____^
61 | | ...
| |___________^ ASYNC100
|

ASYNC100.py:65:5: ASYNC100 A `with asyncio.timeout(...):` context does not contain any `await` statements. This makes it pointless, as the timeout can only be triggered by a checkpoint.
|
64 | async def func():
65 | async with asyncio.timeout(delay=0.2):
| _____^
66 | | ...
| |___________^ ASYNC100
|

ASYNC100.py:70:5: ASYNC100 A `with asyncio.timeout_at(...):` context does not contain any `await` statements. This makes it pointless, as the timeout can only be triggered by a checkpoint.
|
69 | async def func():
70 | async with asyncio.timeout_at(when=0.2):
| _____^
71 | | ...
| |___________^ ASYNC100
|

ASYNC100.py:80:5: ASYNC100 A `with asyncio.timeout(...):` context does not contain any `await` statements. This makes it pointless, as the timeout can only be triggered by a checkpoint.
|
79 | async def func():
80 | async with asyncio.timeout(delay=0.2), asyncio.TaskGroup(), asyncio.timeout(delay=0.2):
| _____^
81 | | ...
| |___________^ ASYNC100
|

ASYNC100.py:90:5: ASYNC100 A `with asyncio.timeout(...):` context does not contain any `await` statements. This makes it pointless, as the timeout can only be triggered by a checkpoint.
|
89 | async def func():
90 | async with asyncio.timeout(delay=0.2), asyncio.timeout(delay=0.2):
| _____^
91 | | ...
| |___________^ ASYNC100
|
Original file line number Diff line number Diff line change
@@ -1,4 +1,18 @@
---
source: crates/ruff_linter/src/rules/flake8_async/mod.rs
---
ASYNC109_1.py:5:16: ASYNC109 Async function definition with a `timeout` parameter
|
5 | async def func(timeout):
| ^^^^^^^ ASYNC109
6 | ...
|
= help: Use `asyncio.timeout` instead

ASYNC109_1.py:9:16: ASYNC109 Async function definition with a `timeout` parameter
|
9 | async def func(timeout=10):
| ^^^^^^^^^^ ASYNC109
10 | ...
|
= help: Use `asyncio.timeout` instead
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,30 @@ ASYNC110.py:12:5: ASYNC110 Use `trio.Event` instead of awaiting `trio.sleep` in
13 | | await trio.sleep_until(10)
| |__________________________________^ ASYNC110
|

ASYNC110.py:22:5: ASYNC110 Use `asyncio.Event` instead of awaiting `asyncio.sleep` in a `while` loop
|
21 | async def func():
22 | while True:
| _____^
23 | | await anyio.sleep(10)
| |_____________________________^ ASYNC110
|

ASYNC110.py:27:5: ASYNC110 Use `asyncio.Event` instead of awaiting `asyncio.sleep` in a `while` loop
|
26 | async def func():
27 | while True:
| _____^
28 | | await anyio.sleep_until(10)
| |___________________________________^ ASYNC110
|

ASYNC110.py:37:5: ASYNC110 Use `anyio.Event` instead of awaiting `anyio.sleep` in a `while` loop
|
36 | async def func():
37 | while True:
| _____^
38 | | await asyncio.sleep(10)
| |_______________________________^ ASYNC110
|
Loading

0 comments on commit 13702e1

Please sign in to comment.