diff --git a/README.md b/README.md index 403087a..f4093bc 100644 --- a/README.md +++ b/README.md @@ -53,6 +53,7 @@ All rules are **enabled by default**. Disable per-project via `pyproject.toml`. | `no-broad-except` | `except Exception:` / `except BaseException:` — too broad, catch specific types | | `no-pass-except` | `except` blocks containing only `pass` — silently swallows exceptions | | `no-nested-try` | Nested `try` blocks — extract the inner try into a separate function | +| `no-async-from-sync` | `asyncio.run()` / `get_event_loop()` / `run_until_complete()` / `ensure_future()` / `create_task()` inside a sync function — make the caller `async def` or move the sync/async boundary to an entrypoint | | `no-print` | `print()` calls — use structured logging | | `no-todo-comment` | `TODO`, `FIXME`, `HACK`, `XXX` comments — resolve or track in an issue | | `no-assert` | `assert` in production code — use `if not ...: raise ValueError(...)` instead | diff --git a/src/rules/mod.rs b/src/rules/mod.rs index 00a70fe..5daab9b 100644 --- a/src/rules/mod.rs +++ b/src/rules/mod.rs @@ -1,6 +1,7 @@ pub mod guarded_function_import; pub mod max_function_params; pub mod no_assert; +pub mod no_async_from_sync; pub mod no_bare_except; pub mod no_boolean_positional; pub mod no_broad_except; @@ -63,6 +64,7 @@ pub fn all_rules_with_config(config: &Config) -> Vec> { Box::new(no_str_empty_default::NoStrEmptyDefault), Box::new(no_typing_any::NoTypingAny), Box::new(no_assert::NoAssert), + Box::new(no_async_from_sync::NoAsyncFromSync), Box::new(no_nested_try::NoNestedTry), Box::new(no_pass_except::NoPassExcept), Box::new(max_function_params::MaxFunctionParams { max }), diff --git a/src/rules/no_async_from_sync.rs b/src/rules/no_async_from_sync.rs new file mode 100644 index 0000000..2010c2f --- /dev/null +++ b/src/rules/no_async_from_sync.rs @@ -0,0 +1,105 @@ +use crate::diagnostic::Diagnostic; +use crate::rules::Rule; + +// Attribute names that are unambiguously asyncio loop-dispatch. +// Matching on attribute alone keeps the rule resilient to `import asyncio as aio`. +const ALWAYS_BANNED_ATTRS: &[&str] = &[ + "get_event_loop", + "new_event_loop", + "run_until_complete", + "ensure_future", +]; + +// Attribute names that are common outside asyncio (`app.run`, `scheduler.create_task`). +// Only flag when the object is literally `asyncio`. +const ASYNCIO_ONLY_ATTRS: &[&str] = &["run", "create_task"]; + +pub struct NoAsyncFromSync; + +impl Rule for NoAsyncFromSync { + fn name(&self) -> &'static str { + "no-async-from-sync" + } + + fn help(&self) -> &'static str { + "Dispatching an async coroutine from a sync function via \ + `asyncio.get_event_loop()`, `asyncio.run()`, `loop.run_until_complete()`, \ + `asyncio.ensure_future()`, or `asyncio.create_task()` either deadlocks \ + (when a loop is already running), silently creates a new loop that \ + never runs tasks scheduled elsewhere, or raises `RuntimeError`. \ + Make the caller `async def` and `await` the coroutine, or move the \ + sync/async boundary to a single well-defined entrypoint \ + (`asyncio.run(main())` at `if __name__ == \"__main__\"`). If this is \ + a deliberate sync/async bridge (e.g. `nest_asyncio`, Jupyter, Django \ + ORM adapter), suppress with `# slopcop: ignore[no-async-from-sync]`." + } + + fn node_kinds(&self) -> &'static [&'static str] { + &["call"] + } + + fn check( + &self, + node: &tree_sitter::Node, + source: &[u8], + ancestors: &[tree_sitter::Node], + diagnostics: &mut Vec, + ) { + let Some(func) = node.child_by_field_name("function") else { + return; + }; + if func.kind() != "attribute" { + return; + } + let Some(attr) = func.child_by_field_name("attribute") else { + return; + }; + let attr_name = attr.utf8_text(source).unwrap_or(""); + + let matched_name = if ALWAYS_BANNED_ATTRS.contains(&attr_name) { + attr_name + } else if ASYNCIO_ONLY_ATTRS.contains(&attr_name) { + let Some(obj) = func.child_by_field_name("object") else { + return; + }; + if obj.kind() != "identifier" || obj.utf8_text(source).unwrap_or("") != "asyncio" { + return; + } + attr_name + } else { + return; + }; + + // Find the nearest enclosing function_definition. No enclosing function → + // module-level call (fine: the `if __name__ == "__main__": asyncio.run(...)` + // entrypoint pattern). + let Some(func_def) = ancestors + .iter() + .rev() + .find(|a| a.kind() == "function_definition") + else { + return; + }; + + // If the enclosing def is `async def`, the user has proper options + // (`await`, `asyncio.get_running_loop`) — this rule doesn't apply. + let is_async = func_def + .child(0) + .map(|c| c.kind() == "async") + .unwrap_or(false); + if is_async { + return; + } + + diagnostics.push(Diagnostic { + path: String::new(), + line: node.start_position().row + 1, + col: node.start_position().column, + rule_id: "no-async-from-sync", + severity: crate::rules::Severity::Error, + message: format!( + "Avoid `{matched_name}()` inside a sync function; make the caller `async def` or move the sync/async boundary to an entrypoint" + ), + }); + } +} diff --git a/tests/no_async_from_sync.rs b/tests/no_async_from_sync.rs new file mode 100644 index 0000000..fbde29e --- /dev/null +++ b/tests/no_async_from_sync.rs @@ -0,0 +1,125 @@ +mod helpers; +use helpers::lint_with_rule; + +const RULE: &str = "no-async-from-sync"; + +#[test] +fn get_event_loop_in_sync_def() { + let source = "def foo():\n loop = asyncio.get_event_loop()\n loop.run_until_complete(bar())\n"; + let d = lint_with_rule(source, RULE); + // Both `asyncio.get_event_loop()` and `loop.run_until_complete(...)` should flag. + assert_eq!(d.len(), 2); + assert_eq!(d[0].line, 2); + assert_eq!(d[1].line, 3); +} + +#[test] +fn get_event_loop_in_async_def_ok() { + let source = "async def foo():\n loop = asyncio.get_event_loop()\n"; + let d = lint_with_rule(source, RULE); + assert_eq!(d.len(), 0); +} + +#[test] +fn asyncio_run_in_sync_def() { + let source = "def foo():\n asyncio.run(bar())\n"; + let d = lint_with_rule(source, RULE); + assert_eq!(d.len(), 1); + assert_eq!(d[0].line, 2); +} + +#[test] +fn asyncio_run_at_module_level_ok() { + // Standard entrypoint pattern. + let source = "if __name__ == \"__main__\":\n asyncio.run(main())\n"; + let d = lint_with_rule(source, RULE); + assert_eq!(d.len(), 0); +} + +#[test] +fn run_until_complete_in_sync_def() { + let source = "def foo():\n loop.run_until_complete(bar())\n"; + let d = lint_with_rule(source, RULE); + assert_eq!(d.len(), 1); + assert_eq!(d[0].line, 2); +} + +#[test] +fn new_event_loop_in_sync_def() { + let source = "def foo():\n loop = asyncio.new_event_loop()\n"; + let d = lint_with_rule(source, RULE); + assert_eq!(d.len(), 1); + assert_eq!(d[0].line, 2); +} + +#[test] +fn ensure_future_in_sync_def() { + let source = "def foo():\n asyncio.ensure_future(bar())\n"; + let d = lint_with_rule(source, RULE); + assert_eq!(d.len(), 1); +} + +#[test] +fn create_task_asyncio_in_sync_def() { + let source = "def foo():\n asyncio.create_task(bar())\n"; + let d = lint_with_rule(source, RULE); + assert_eq!(d.len(), 1); +} + +#[test] +fn create_task_non_asyncio_object_ok() { + // `scheduler.create_task(...)` is not asyncio — too ambiguous to flag. + let source = "def foo():\n scheduler.create_task(job)\n"; + let d = lint_with_rule(source, RULE); + assert_eq!(d.len(), 0); +} + +#[test] +fn obj_run_non_asyncio_ok() { + // Bare `.run()` is common (threads, click, flask, subprocess). Don't flag. + let source = "def foo():\n thread.run()\n app.run(host=\"0.0.0.0\")\n"; + let d = lint_with_rule(source, RULE); + assert_eq!(d.len(), 0); +} + +#[test] +fn sync_nested_in_async_flagged() { + // Nearest enclosing function_definition wins. Sync helper defined inside + // an async function is still sync — dispatching the loop from it is bad. + let source = "async def outer():\n def inner():\n asyncio.run(bar())\n"; + let d = lint_with_rule(source, RULE); + assert_eq!(d.len(), 1); + assert_eq!(d[0].line, 3); +} + +#[test] +fn async_nested_in_sync_ok() { + // Inverse: async helper defined inside a sync function. The async def + // is the nearest scope — dispatch is fine there. + let source = "def outer():\n async def inner():\n loop = asyncio.get_event_loop()\n"; + let d = lint_with_rule(source, RULE); + assert_eq!(d.len(), 0); +} + +#[test] +fn pure_sync_code_ok() { + let source = "def foo():\n x = 1\n return x + 2\n"; + let d = lint_with_rule(source, RULE); + assert_eq!(d.len(), 0); +} + +#[test] +fn in_comment_ok() { + let source = "# asyncio.run(main())\n# loop.run_until_complete(x)\n"; + let d = lint_with_rule(source, RULE); + assert_eq!(d.len(), 0); +} + +#[test] +fn method_def_in_sync_class() { + // Sync method on a class — same rules apply. + let source = "class Service:\n def handle(self):\n asyncio.run(self._work())\n"; + let d = lint_with_rule(source, RULE); + assert_eq!(d.len(), 1); + assert_eq!(d[0].line, 3); +}