Insert blank lines before comments in E305

## Summary

Addresses #8451 by implementing rule 116 to add an unsafe fix when sleep
is used with a >24 hour interval to instead consider sleeping forever.

This rule is added as async instead as I my understanding was that these
trio rules would be moved to async anyway.

There are a couple of TODOs, which address further extending the rule by
adding support for lookups and evaluations, and also supporting `anyio`.

README.md

@@ -433,6 +433,7 @@ Ruff is used by a number of major open-source projects and companies, including:
 - Modern Treasury ([Python SDK](https://github.com/Modern-Treasury/modern-treasury-python))
 - Mozilla ([Firefox](https://github.com/mozilla/gecko-dev))
 - [Mypy](https://github.com/python/mypy)
+- [Nautobot](https://github.com/nautobot/nautobot)
 - Netflix ([Dispatch](https://github.com/Netflix/dispatch))
 - [Neon](https://github.com/neondatabase/neon)
 - [Nokia](https://nokia.com/)

crates/ruff_linter/resources/test/fixtures/flake8_async/ASYNC116.py

@@ -0,0 +1,57 @@
+# type: ignore
+# ASYNCIO_NO_ERROR - no asyncio.sleep_forever, so check intentionally doesn't trigger.
+import math
+from math import inf
+
+
+async def import_trio():
+    import trio
+
+    # These examples are probably not meant to ever wake up:
+    await trio.sleep(100000)  # error: 116, "async"
+
+    # 'inf literal' overflow trick
+    await trio.sleep(1e999)  # error: 116, "async"
+
+    await trio.sleep(86399)
+    await trio.sleep(86400)
+    await trio.sleep(86400.01)  # error: 116, "async"
+    await trio.sleep(86401)  # error: 116, "async"
+
+    await trio.sleep(-1)  # will raise a runtime error
+    await trio.sleep(0)  # handled by different check
+
+    # these ones _definitely_ never wake up (TODO)
+    await trio.sleep(float("inf"))
+    await trio.sleep(math.inf)
+    await trio.sleep(inf)
+
+    # don't require inf to be in math (TODO)
+    await trio.sleep(np.inf)
+
+    # don't evaluate expressions (TODO)
+    one_day = 86401
+    await trio.sleep(86400 + 1)
+    await trio.sleep(60 * 60 * 24 + 1)
+    await trio.sleep(foo())
+    await trio.sleep(one_day)
+    await trio.sleep(86400 + foo())
+    await trio.sleep(86400 + ...)
+    await trio.sleep("hello")
+    await trio.sleep(...)
+
+
+def not_async_fun():
+    import trio
+
+    # does not require the call to be awaited, nor in an async fun
+    trio.sleep(86401)  # error: 116, "async"
+    # also checks that we don't break visit_Call
+    trio.run(trio.sleep(86401))  # error: 116, "async"
+
+
+async def import_from_trio():
+    from trio import sleep
+
+    # catch from import
+    await sleep(86401)  # error: 116, "async"

crates/ruff_linter/src/checkers/ast/analyze/expression.rs

@@ -508,6 +508,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
             if checker.enabled(Rule::BlockingOsCallInAsyncFunction) {
                 flake8_async::rules::blocking_os_call(checker, call);
             }
+            if checker.enabled(Rule::SleepForeverCall) {
+                flake8_async::rules::sleep_forever_call(checker, call);
+            }
             if checker.any_enabled(&[Rule::Print, Rule::PPrint]) {
                 flake8_print::rules::print_call(checker, call);
             }

crates/ruff_linter/src/codes.rs

@@ -334,6 +334,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
         (Flake8Async, "100") => (RuleGroup::Stable, rules::flake8_async::rules::BlockingHttpCallInAsyncFunction),
         (Flake8Async, "101") => (RuleGroup::Stable, rules::flake8_async::rules::OpenSleepOrSubprocessInAsyncFunction),
         (Flake8Async, "102") => (RuleGroup::Stable, rules::flake8_async::rules::BlockingOsCallInAsyncFunction),
+        (Flake8Async, "116") => (RuleGroup::Preview, rules::flake8_async::rules::SleepForeverCall),
 
         // flake8-trio
         (Flake8Trio, "100") => (RuleGroup::Stable, rules::flake8_trio::rules::TrioTimeoutWithoutAwait),

crates/ruff_linter/src/rules/flake8_async/mod.rs

@@ -16,6 +16,7 @@ mod tests {
     #[test_case(Rule::BlockingHttpCallInAsyncFunction, Path::new("ASYNC100.py"))]
     #[test_case(Rule::OpenSleepOrSubprocessInAsyncFunction, Path::new("ASYNC101.py"))]
     #[test_case(Rule::BlockingOsCallInAsyncFunction, Path::new("ASYNC102.py"))]
+    #[test_case(Rule::SleepForeverCall, Path::new("ASYNC116.py"))]
     fn rules(rule_code: Rule, path: &Path) -> Result<()> {
         let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
         let diagnostics = test_path(

crates/ruff_linter/src/rules/flake8_async/rules/mod.rs

@@ -1,7 +1,9 @@
 pub(crate) use blocking_http_call::*;
 pub(crate) use blocking_os_call::*;
 pub(crate) use open_sleep_or_subprocess_call::*;
+pub(crate) use sleep_forever_call::*;
 
 mod blocking_http_call;
 mod blocking_os_call;
 mod open_sleep_or_subprocess_call;
+mod sleep_forever_call;

crates/ruff_linter/src/rules/flake8_async/rules/sleep_forever_call.rs

@@ -0,0 +1,110 @@
+use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation};
+use ruff_macros::{derive_message_formats, violation};
+use ruff_python_ast::{Expr, ExprCall, ExprNumberLiteral, Number};
+use ruff_python_semantic::Modules;
+use ruff_text_size::Ranged;
+
+use crate::{checkers::ast::Checker, importer::ImportRequest};
+
+/// ## What it does
+/// Checks for uses of `trio.sleep()` with an interval greater than 24 hours.
+///
+/// ## Why is this bad?
+/// `trio.sleep()` with an interval greater than 24 hours is usually intended
+/// to sleep indefinitely. Instead of using a large interval,
+/// `trio.sleep_forever()` better conveys the intent.
+///
+///
+/// ## Example
+/// ```python
+/// import trio
+///
+///
+/// async def func():
+///     await trio.sleep(86401)
+/// ```
+///
+/// Use instead:
+/// ```python
+/// import trio
+///
+///
+/// async def func():
+///     await trio.sleep_forever()
+/// ```
+#[violation]
+pub struct SleepForeverCall;
+
+impl Violation for SleepForeverCall {
+    const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        format!("`trio.sleep()` with >24 hour interval should usually be `trio.sleep_forever()`")
+    }
+
+    fn fix_title(&self) -> Option<String> {
+        Some(format!("Replace with `trio.sleep_forever()`"))
+    }
+}
+
+/// ASYNC116
+pub(crate) fn sleep_forever_call(checker: &mut Checker, call: &ExprCall) {
+    if !checker.semantic().seen_module(Modules::TRIO) {
+        return;
+    }
+
+    if call.arguments.len() != 1 {
+        return;
+    }
+
+    let Some(arg) = call.arguments.find_argument("seconds", 0) else {
+        return;
+    };
+
+    if !checker
+        .semantic()
+        .resolve_qualified_name(call.func.as_ref())
+        .is_some_and(|qualified_name| matches!(qualified_name.segments(), ["trio", "sleep"]))
+    {
+        return;
+    }
+
+    let Expr::NumberLiteral(ExprNumberLiteral { value, .. }) = arg else {
+        return;
+    };
+
+    // TODO(ekohilas): Replace with Duration::from_days(1).as_secs(); when available.
+    let one_day_in_secs = 60 * 60 * 24;
+    match value {
+        Number::Int(int_value) => {
+            let Some(int_value) = int_value.as_u64() else {
+                return;
+            };
+            if int_value <= one_day_in_secs {
+                return;
+            }
+        }
+        Number::Float(float_value) =>
+        {
+            #[allow(clippy::cast_precision_loss)]
+            if *float_value <= one_day_in_secs as f64 {
+                return;
+            }
+        }
+        Number::Complex { .. } => return,
+    }
+
+    let mut diagnostic = Diagnostic::new(SleepForeverCall, call.range());
+    let replacement_function = "sleep_forever";
+    diagnostic.try_set_fix(|| {
+        let (import_edit, binding) = checker.importer().get_or_import_symbol(
+            &ImportRequest::import_from("trio", replacement_function),
+            call.func.start(),
+            checker.semantic(),
+        )?;
+        let reference_edit = Edit::range_replacement(binding, call.func.range());
+        let arg_edit = Edit::range_replacement("()".to_string(), call.arguments.range());
+        Ok(Fix::unsafe_edits(import_edit, [reference_edit, arg_edit]))
+    });
+    checker.diagnostics.push(diagnostic);
+}

crates/ruff_linter/src/rules/flake8_async/snapshots/ruff_linter__rules__flake8_async__tests__ASYNC116_ASYNC116.py.snap

@@ -0,0 +1,145 @@
+---
+source: crates/ruff_linter/src/rules/flake8_async/mod.rs
+---
+ASYNC116.py:11:11: ASYNC116 [*] `trio.sleep()` with >24 hour interval should usually be `trio.sleep_forever()`
+   |
+10 |     # These examples are probably not meant to ever wake up:
+11 |     await trio.sleep(100000)  # error: 116, "async"
+   |           ^^^^^^^^^^^^^^^^^^ ASYNC116
+12 | 
+13 |     # 'inf literal' overflow trick
+   |
+   = help: Replace with `trio.sleep_forever()`
+
+ℹ Unsafe fix
+8  8  |     import trio
+9  9  | 
+10 10 |     # These examples are probably not meant to ever wake up:
+11    |-    await trio.sleep(100000)  # error: 116, "async"
+   11 |+    await trio.sleep_forever()  # error: 116, "async"
+12 12 | 
+13 13 |     # 'inf literal' overflow trick
+14 14 |     await trio.sleep(1e999)  # error: 116, "async"
+
+ASYNC116.py:14:11: ASYNC116 [*] `trio.sleep()` with >24 hour interval should usually be `trio.sleep_forever()`
+   |
+13 |     # 'inf literal' overflow trick
+14 |     await trio.sleep(1e999)  # error: 116, "async"
+   |           ^^^^^^^^^^^^^^^^^ ASYNC116
+15 | 
+16 |     await trio.sleep(86399)
+   |
+   = help: Replace with `trio.sleep_forever()`
+
+ℹ Unsafe fix
+11 11 |     await trio.sleep(100000)  # error: 116, "async"
+12 12 | 
+13 13 |     # 'inf literal' overflow trick
+14    |-    await trio.sleep(1e999)  # error: 116, "async"
+   14 |+    await trio.sleep_forever()  # error: 116, "async"
+15 15 | 
+16 16 |     await trio.sleep(86399)
+17 17 |     await trio.sleep(86400)
+
+ASYNC116.py:18:11: ASYNC116 [*] `trio.sleep()` with >24 hour interval should usually be `trio.sleep_forever()`
+   |
+16 |     await trio.sleep(86399)
+17 |     await trio.sleep(86400)
+18 |     await trio.sleep(86400.01)  # error: 116, "async"
+   |           ^^^^^^^^^^^^^^^^^^^^ ASYNC116
+19 |     await trio.sleep(86401)  # error: 116, "async"
+   |
+   = help: Replace with `trio.sleep_forever()`
+
+ℹ Unsafe fix
+15 15 | 
+16 16 |     await trio.sleep(86399)
+17 17 |     await trio.sleep(86400)
+18    |-    await trio.sleep(86400.01)  # error: 116, "async"
+   18 |+    await trio.sleep_forever()  # error: 116, "async"
+19 19 |     await trio.sleep(86401)  # error: 116, "async"
+20 20 | 
+21 21 |     await trio.sleep(-1)  # will raise a runtime error
+
+ASYNC116.py:19:11: ASYNC116 [*] `trio.sleep()` with >24 hour interval should usually be `trio.sleep_forever()`
+   |
+17 |     await trio.sleep(86400)
+18 |     await trio.sleep(86400.01)  # error: 116, "async"
+19 |     await trio.sleep(86401)  # error: 116, "async"
+   |           ^^^^^^^^^^^^^^^^^ ASYNC116
+20 | 
+21 |     await trio.sleep(-1)  # will raise a runtime error
+   |
+   = help: Replace with `trio.sleep_forever()`
+
+ℹ Unsafe fix
+16 16 |     await trio.sleep(86399)
+17 17 |     await trio.sleep(86400)
+18 18 |     await trio.sleep(86400.01)  # error: 116, "async"
+19    |-    await trio.sleep(86401)  # error: 116, "async"
+   19 |+    await trio.sleep_forever()  # error: 116, "async"
+20 20 | 
+21 21 |     await trio.sleep(-1)  # will raise a runtime error
+22 22 |     await trio.sleep(0)  # handled by different check
+
+ASYNC116.py:48:5: ASYNC116 [*] `trio.sleep()` with >24 hour interval should usually be `trio.sleep_forever()`
+   |
+47 |     # does not require the call to be awaited, nor in an async fun
+48 |     trio.sleep(86401)  # error: 116, "async"
+   |     ^^^^^^^^^^^^^^^^^ ASYNC116
+49 |     # also checks that we don't break visit_Call
+50 |     trio.run(trio.sleep(86401))  # error: 116, "async"
+   |
+   = help: Replace with `trio.sleep_forever()`
+
+ℹ Unsafe fix
+45 45 |     import trio
+46 46 | 
+47 47 |     # does not require the call to be awaited, nor in an async fun
+48    |-    trio.sleep(86401)  # error: 116, "async"
+   48 |+    trio.sleep_forever()  # error: 116, "async"
+49 49 |     # also checks that we don't break visit_Call
+50 50 |     trio.run(trio.sleep(86401))  # error: 116, "async"
+51 51 | 
+
+ASYNC116.py:50:14: ASYNC116 [*] `trio.sleep()` with >24 hour interval should usually be `trio.sleep_forever()`
+   |
+48 |     trio.sleep(86401)  # error: 116, "async"
+49 |     # also checks that we don't break visit_Call
+50 |     trio.run(trio.sleep(86401))  # error: 116, "async"
+   |              ^^^^^^^^^^^^^^^^^ ASYNC116
+   |
+   = help: Replace with `trio.sleep_forever()`
+
+ℹ Unsafe fix
+47 47 |     # does not require the call to be awaited, nor in an async fun
+48 48 |     trio.sleep(86401)  # error: 116, "async"
+49 49 |     # also checks that we don't break visit_Call
+50    |-    trio.run(trio.sleep(86401))  # error: 116, "async"
+   50 |+    trio.run(trio.sleep_forever())  # error: 116, "async"
+51 51 | 
+52 52 | 
+53 53 | async def import_from_trio():
+
+ASYNC116.py:57:11: ASYNC116 [*] `trio.sleep()` with >24 hour interval should usually be `trio.sleep_forever()`
+   |
+56 |     # catch from import
+57 |     await sleep(86401)  # error: 116, "async"
+   |           ^^^^^^^^^^^^ ASYNC116
+   |
+   = help: Replace with `trio.sleep_forever()`
+
+ℹ Unsafe fix
+2  2  | # ASYNCIO_NO_ERROR - no asyncio.sleep_forever, so check intentionally doesn't trigger.
+3  3  | import math
+4  4  | from math import inf
+   5  |+from trio import sleep_forever
+5  6  | 
+6  7  | 
+7  8  | async def import_trio():
+--------------------------------------------------------------------------------
+54 55 |     from trio import sleep
+55 56 | 
+56 57 |     # catch from import
+57    |-    await sleep(86401)  # error: 116, "async"
+   58 |+    await sleep_forever()  # error: 116, "async"

crates/ruff_linter/src/rules/flake8_future_annotations/rules/future_required_type_annotation.rs

@@ -7,7 +7,6 @@ use ruff_python_ast::Expr;
 use ruff_text_size::{Ranged, TextSize};
 
 use crate::checkers::ast::Checker;
-use crate::importer::Importer;
 
 /// ## What it does
 /// Checks for uses of PEP 585- and PEP 604-style type annotations in Python
@@ -87,13 +86,11 @@ impl AlwaysFixableViolation for FutureRequiredTypeAnnotation {
 /// FA102
 pub(crate) fn future_required_type_annotation(checker: &mut Checker, expr: &Expr, reason: Reason) {
     let mut diagnostic = Diagnostic::new(FutureRequiredTypeAnnotation { reason }, expr.range());
-    if let Some(python_ast) = checker.semantic().definitions.python_ast() {
-        let required_import =
-            AnyImport::ImportFrom(ImportFrom::member("__future__", "annotations"));
-        diagnostic.set_fix(Fix::unsafe_edit(
-            Importer::new(python_ast, checker.locator(), checker.stylist())
-                .add_import(&required_import, TextSize::default()),
-        ));
-    }
+    let required_import = AnyImport::ImportFrom(ImportFrom::member("__future__", "annotations"));
+    diagnostic.set_fix(Fix::unsafe_edit(
+        checker
+            .importer()
+            .add_import(&required_import, TextSize::default()),
+    ));
     checker.diagnostics.push(diagnostic);
 }

crates/ruff_linter/src/rules/pycodestyle/rules/blank_lines.rs

@@ -1009,10 +1009,10 @@ impl<'a> BlankLinesChecker<'a> {
                 )));
             } else {
                 diagnostic.set_fix(Fix::safe_edit(Edit::insertion(
-                    self.stylist
-                        .line_ending()
-                        .repeat(BLANK_LINES_TOP_LEVEL as usize),
-                    self.locator.line_start(line.first_token_range.start()),
+                    self.stylist.line_ending().repeat(
+                        (BLANK_LINES_TOP_LEVEL - line.preceding_blank_lines.count()) as usize,
+                    ),
+                    self.locator.line_start(state.last_non_comment_line_end),
                 )));
             }
 

crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__E305_E30.py.snap

@@ -11,14 +11,13 @@ E30.py:806:1: E305 [*] Expected 2 blank lines after class or function definition
     = help: Add missing blank line(s)
 
 ℹ Safe fix
-803 803 |     # comment
-804 804 | 
-805 805 |     # another comment
-    806 |+
-    807 |+
-806 808 | fn()
-807 809 | # end
-808 810 | 
+800 800 | def fn():
+801 801 |     print()
+802 802 | 
+    803 |+
+803 804 |     # comment
+804 805 | 
+805 806 |     # another comment
 
 E30.py:817:1: E305 [*] Expected 2 blank lines after class or function definition, found (1)
     |
@@ -30,14 +29,13 @@ E30.py:817:1: E305 [*] Expected 2 blank lines after class or function definition
     = help: Add missing blank line(s)
 
 ℹ Safe fix
-814 814 |     # comment
-815 815 | 
-816 816 |     # another comment
-    817 |+
-    818 |+
-817 819 | a = 1
-818 820 | # end
-819 821 | 
+811 811 | class Class():
+812 812 |     pass
+813 813 | 
+    814 |+
+814 815 |     # comment
+815 816 | 
+816 817 |     # another comment
 
 E30.py:829:1: E305 [*] Expected 2 blank lines after class or function definition, found (1)
     |
@@ -70,14 +68,13 @@ E30.py:841:1: E305 [*] Expected 2 blank lines after class or function definition
     = help: Add missing blank line(s)
 
 ℹ Safe fix
+837 837 | def a():
 838 838 |     print()
 839 839 | 
-840 840 | # Two spaces before comments, too.
-    841 |+
-    842 |+
-841 843 | if a():
-842 844 |     a()
-843 845 | # end
+    840 |+
+840 841 | # Two spaces before comments, too.
+841 842 | if a():
+842 843 |     a()
 
 E30.py:854:1: E305 [*] Expected 2 blank lines after class or function definition, found (1)
     |

crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__blank_lines_E305_notebook.snap

@@ -12,11 +12,10 @@ E30.ipynb:55:1: E305 [*] Expected 2 blank lines after class or function definiti
    = help: Add missing blank line(s)
 
 ℹ Safe fix
-52 52 |     # comment
-53 53 | 
-54 54 |     # another comment
-   55 |+
-   56 |+
-55 57 | fn()
-56 58 | # end
-57 59 | # E306:3:5
+49 49 | def fn():
+50 50 |     print()
+51 51 | 
+   52 |+
+52 53 |     # comment
+53 54 | 
+54 55 |     # another comment

ruff.schema.json

@@ -2685,6 +2685,8 @@
         "ASYNC100",
         "ASYNC101",
         "ASYNC102",
+        "ASYNC11",
+        "ASYNC116",
         "B",
         "B0",
         "B00",