Remove manual `await` detection (#5103)

We can just use `any_over_expr` instead.

crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_comprehension_any_all.rs

@@ -73,7 +73,7 @@ pub(crate) fn unnecessary_comprehension_any_all(
         let (Expr::ListComp(ast::ExprListComp { elt, .. } )| Expr::SetComp(ast::ExprSetComp { elt, .. })) = &args[0] else {
             return;
         };
-        if is_async_generator(elt) {
+        if contains_await(elt) {
             return;
         }
         if !checker.semantic().is_builtin(id) {
@@ -89,7 +89,7 @@ pub(crate) fn unnecessary_comprehension_any_all(
     }
 }
 
-/// Return `true` if the `Expr` contains an `await` expression.
+/// Return `true` if the [`Expr`] contains an `await` expression.
-fn is_async_generator(expr: &Expr) -> bool {
+fn contains_await(expr: &Expr) -> bool {
-    any_over_expr(expr, &|expr| matches!(expr, Expr::Await(_)))
+    any_over_expr(expr, &Expr::is_await_expr)
 }

crates/ruff/src/rules/flake8_simplify/rules/reimplemented_builtin.rs

@@ -56,6 +56,156 @@ impl Violation for ReimplementedBuiltin {
     }
 }
 
+/// SIM110, SIM111
+pub(crate) fn convert_for_loop_to_any_all(
+    checker: &mut Checker,
+    stmt: &Stmt,
+    sibling: Option<&Stmt>,
+) {
+    // There are two cases to consider:
+    // - `for` loop with an `else: return True` or `else: return False`.
+    // - `for` loop followed by `return True` or `return False`
+    if let Some(loop_info) = return_values_for_else(stmt)
+        .or_else(|| sibling.and_then(|sibling| return_values_for_siblings(stmt, sibling)))
+    {
+        // Check if loop_info.target, loop_info.iter, or loop_info.test contains `await`.
+        if contains_await(loop_info.target)
+            || contains_await(loop_info.iter)
+            || contains_await(loop_info.test)
+        {
+            return;
+        }
+        if loop_info.return_value && !loop_info.next_return_value {
+            if checker.enabled(Rule::ReimplementedBuiltin) {
+                let contents = return_stmt(
+                    "any",
+                    loop_info.test,
+                    loop_info.target,
+                    loop_info.iter,
+                    checker.generator(),
+                );
+
+                // Don't flag if the resulting expression would exceed the maximum line length.
+                let line_start = checker.locator.line_start(stmt.start());
+                if LineWidth::new(checker.settings.tab_size)
+                    .add_str(&checker.locator.contents()[TextRange::new(line_start, stmt.start())])
+                    .add_str(&contents)
+                    > checker.settings.line_length
+                {
+                    return;
+                }
+
+                let mut diagnostic = Diagnostic::new(
+                    ReimplementedBuiltin {
+                        repl: contents.clone(),
+                    },
+                    TextRange::new(stmt.start(), loop_info.terminal),
+                );
+                if checker.patch(diagnostic.kind.rule()) && checker.semantic().is_builtin("any") {
+                    #[allow(deprecated)]
+                    diagnostic.set_fix(Fix::unspecified(Edit::replacement(
+                        contents,
+                        stmt.start(),
+                        loop_info.terminal,
+                    )));
+                }
+                checker.diagnostics.push(diagnostic);
+            }
+        }
+
+        if !loop_info.return_value && loop_info.next_return_value {
+            if checker.enabled(Rule::ReimplementedBuiltin) {
+                // Invert the condition.
+                let test = {
+                    if let Expr::UnaryOp(ast::ExprUnaryOp {
+                        op: Unaryop::Not,
+                        operand,
+                        range: _,
+                    }) = &loop_info.test
+                    {
+                        *operand.clone()
+                    } else if let Expr::Compare(ast::ExprCompare {
+                        left,
+                        ops,
+                        comparators,
+                        range: _,
+                    }) = &loop_info.test
+                    {
+                        if let ([op], [comparator]) = (ops.as_slice(), comparators.as_slice()) {
+                            let op = match op {
+                                Cmpop::Eq => Cmpop::NotEq,
+                                Cmpop::NotEq => Cmpop::Eq,
+                                Cmpop::Lt => Cmpop::GtE,
+                                Cmpop::LtE => Cmpop::Gt,
+                                Cmpop::Gt => Cmpop::LtE,
+                                Cmpop::GtE => Cmpop::Lt,
+                                Cmpop::Is => Cmpop::IsNot,
+                                Cmpop::IsNot => Cmpop::Is,
+                                Cmpop::In => Cmpop::NotIn,
+                                Cmpop::NotIn => Cmpop::In,
+                            };
+                            let node = ast::ExprCompare {
+                                left: left.clone(),
+                                ops: vec![op],
+                                comparators: vec![comparator.clone()],
+                                range: TextRange::default(),
+                            };
+                            node.into()
+                        } else {
+                            let node = ast::ExprUnaryOp {
+                                op: Unaryop::Not,
+                                operand: Box::new(loop_info.test.clone()),
+                                range: TextRange::default(),
+                            };
+                            node.into()
+                        }
+                    } else {
+                        let node = ast::ExprUnaryOp {
+                            op: Unaryop::Not,
+                            operand: Box::new(loop_info.test.clone()),
+                            range: TextRange::default(),
+                        };
+                        node.into()
+                    }
+                };
+                let contents = return_stmt(
+                    "all",
+                    &test,
+                    loop_info.target,
+                    loop_info.iter,
+                    checker.generator(),
+                );
+
+                // Don't flag if the resulting expression would exceed the maximum line length.
+                let line_start = checker.locator.line_start(stmt.start());
+                if LineWidth::new(checker.settings.tab_size)
+                    .add_str(&checker.locator.contents()[TextRange::new(line_start, stmt.start())])
+                    .add_str(&contents)
+                    > checker.settings.line_length
+                {
+                    return;
+                }
+
+                let mut diagnostic = Diagnostic::new(
+                    ReimplementedBuiltin {
+                        repl: contents.clone(),
+                    },
+                    TextRange::new(stmt.start(), loop_info.terminal),
+                );
+                if checker.patch(diagnostic.kind.rule()) && checker.semantic().is_builtin("all") {
+                    #[allow(deprecated)]
+                    diagnostic.set_fix(Fix::unspecified(Edit::replacement(
+                        contents,
+                        stmt.start(),
+                        loop_info.terminal,
+                    )));
+                }
+                checker.diagnostics.push(diagnostic);
+            }
+        }
+    }
+}
+
 struct Loop<'a> {
     return_value: bool,
     next_return_value: bool,
@@ -225,157 +375,7 @@ fn return_stmt(id: &str, test: &Expr, target: &Expr, iter: &Expr, generator: Gen
     generator.stmt(&node3.into())
 }
 
-/// Return `true` if the `Expr` contains an `await` expression.
+/// Return `true` if the [`Expr`] contains an `await` expression.
 fn contains_await(expr: &Expr) -> bool {
-    any_over_expr(expr, &|expr| matches!(expr, Expr::Await(_)))
+    any_over_expr(expr, &Expr::is_await_expr)
-}
-
-/// SIM110, SIM111
-pub(crate) fn convert_for_loop_to_any_all(
-    checker: &mut Checker,
-    stmt: &Stmt,
-    sibling: Option<&Stmt>,
-) {
-    // There are two cases to consider:
-    // - `for` loop with an `else: return True` or `else: return False`.
-    // - `for` loop followed by `return True` or `return False`
-    if let Some(loop_info) = return_values_for_else(stmt)
-        .or_else(|| sibling.and_then(|sibling| return_values_for_siblings(stmt, sibling)))
-    {
-        // Check if loop_info.target, loop_info.iter, or loop_info.test contains `await`.
-        if contains_await(loop_info.target)
-            || contains_await(loop_info.iter)
-            || contains_await(loop_info.test)
-        {
-            return;
-        }
-        if loop_info.return_value && !loop_info.next_return_value {
-            if checker.enabled(Rule::ReimplementedBuiltin) {
-                let contents = return_stmt(
-                    "any",
-                    loop_info.test,
-                    loop_info.target,
-                    loop_info.iter,
-                    checker.generator(),
-                );
-
-                // Don't flag if the resulting expression would exceed the maximum line length.
-                let line_start = checker.locator.line_start(stmt.start());
-                if LineWidth::new(checker.settings.tab_size)
-                    .add_str(&checker.locator.contents()[TextRange::new(line_start, stmt.start())])
-                    .add_str(&contents)
-                    > checker.settings.line_length
-                {
-                    return;
-                }
-
-                let mut diagnostic = Diagnostic::new(
-                    ReimplementedBuiltin {
-                        repl: contents.clone(),
-                    },
-                    TextRange::new(stmt.start(), loop_info.terminal),
-                );
-                if checker.patch(diagnostic.kind.rule()) && checker.semantic().is_builtin("any") {
-                    #[allow(deprecated)]
-                    diagnostic.set_fix(Fix::unspecified(Edit::replacement(
-                        contents,
-                        stmt.start(),
-                        loop_info.terminal,
-                    )));
-                }
-                checker.diagnostics.push(diagnostic);
-            }
-        }
-
-        if !loop_info.return_value && loop_info.next_return_value {
-            if checker.enabled(Rule::ReimplementedBuiltin) {
-                // Invert the condition.
-                let test = {
-                    if let Expr::UnaryOp(ast::ExprUnaryOp {
-                        op: Unaryop::Not,
-                        operand,
-                        range: _,
-                    }) = &loop_info.test
-                    {
-                        *operand.clone()
-                    } else if let Expr::Compare(ast::ExprCompare {
-                        left,
-                        ops,
-                        comparators,
-                        range: _,
-                    }) = &loop_info.test
-                    {
-                        if let ([op], [comparator]) = (ops.as_slice(), comparators.as_slice()) {
-                            let op = match op {
-                                Cmpop::Eq => Cmpop::NotEq,
-                                Cmpop::NotEq => Cmpop::Eq,
-                                Cmpop::Lt => Cmpop::GtE,
-                                Cmpop::LtE => Cmpop::Gt,
-                                Cmpop::Gt => Cmpop::LtE,
-                                Cmpop::GtE => Cmpop::Lt,
-                                Cmpop::Is => Cmpop::IsNot,
-                                Cmpop::IsNot => Cmpop::Is,
-                                Cmpop::In => Cmpop::NotIn,
-                                Cmpop::NotIn => Cmpop::In,
-                            };
-                            let node = ast::ExprCompare {
-                                left: left.clone(),
-                                ops: vec![op],
-                                comparators: vec![comparator.clone()],
-                                range: TextRange::default(),
-                            };
-                            node.into()
-                        } else {
-                            let node = ast::ExprUnaryOp {
-                                op: Unaryop::Not,
-                                operand: Box::new(loop_info.test.clone()),
-                                range: TextRange::default(),
-                            };
-                            node.into()
-                        }
-                    } else {
-                        let node = ast::ExprUnaryOp {
-                            op: Unaryop::Not,
-                            operand: Box::new(loop_info.test.clone()),
-                            range: TextRange::default(),
-                        };
-                        node.into()
-                    }
-                };
-                let contents = return_stmt(
-                    "all",
-                    &test,
-                    loop_info.target,
-                    loop_info.iter,
-                    checker.generator(),
-                );
-
-                // Don't flag if the resulting expression would exceed the maximum line length.
-                let line_start = checker.locator.line_start(stmt.start());
-                if LineWidth::new(checker.settings.tab_size)
-                    .add_str(&checker.locator.contents()[TextRange::new(line_start, stmt.start())])
-                    .add_str(&contents)
-                    > checker.settings.line_length
-                {
-                    return;
-                }
-
-                let mut diagnostic = Diagnostic::new(
-                    ReimplementedBuiltin {
-                        repl: contents.clone(),
-                    },
-                    TextRange::new(stmt.start(), loop_info.terminal),
-                );
-                if checker.patch(diagnostic.kind.rule()) && checker.semantic().is_builtin("all") {
-                    #[allow(deprecated)]
-                    diagnostic.set_fix(Fix::unspecified(Edit::replacement(
-                        contents,
-                        stmt.start(),
-                        loop_info.terminal,
-                    )));
-                }
-                checker.diagnostics.push(diagnostic);
-            }
-        }
-    }
 }

crates/ruff/src/rules/pyupgrade/rules/unpacked_list_comprehension.rs

@@ -2,6 +2,7 @@ use rustpython_parser::ast::{self, Expr, Ranged};
 
 use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix};
 use ruff_macros::{derive_message_formats, violation};
+use ruff_python_ast::helpers::any_over_expr;
 
 use crate::checkers::ast::Checker;
 use crate::registry::AsRule;
@@ -20,126 +21,45 @@ impl AlwaysAutofixableViolation for UnpackedListComprehension {
     }
 }
 
-/// Returns `true` if `expr` contains an `Expr::Await`.
-fn contains_await(expr: &Expr) -> bool {
-    match expr {
-        Expr::Await(_) => true,
-        Expr::BoolOp(ast::ExprBoolOp { values, .. }) => values.iter().any(contains_await),
-        Expr::NamedExpr(ast::ExprNamedExpr {
-            target,
-            value,
-            range: _,
-        }) => contains_await(target) || contains_await(value),
-        Expr::BinOp(ast::ExprBinOp { left, right, .. }) => {
-            contains_await(left) || contains_await(right)
-        }
-        Expr::UnaryOp(ast::ExprUnaryOp { operand, .. }) => contains_await(operand),
-        Expr::Lambda(ast::ExprLambda { body, .. }) => contains_await(body),
-        Expr::IfExp(ast::ExprIfExp {
-            test,
-            body,
-            orelse,
-            range: _,
-        }) => contains_await(test) || contains_await(body) || contains_await(orelse),
-        Expr::Dict(ast::ExprDict {
-            keys,
-            values,
-            range: _,
-        }) => keys
-            .iter()
-            .flatten()
-            .chain(values.iter())
-            .any(contains_await),
-        Expr::Set(ast::ExprSet { elts, range: _ }) => elts.iter().any(contains_await),
-        Expr::ListComp(ast::ExprListComp { elt, .. }) => contains_await(elt),
-        Expr::SetComp(ast::ExprSetComp { elt, .. }) => contains_await(elt),
-        Expr::DictComp(ast::ExprDictComp { key, value, .. }) => {
-            contains_await(key) || contains_await(value)
-        }
-        Expr::GeneratorExp(ast::ExprGeneratorExp { elt, .. }) => contains_await(elt),
-        Expr::Yield(ast::ExprYield { value, range: _ }) => {
-            value.as_ref().map_or(false, |value| contains_await(value))
-        }
-        Expr::YieldFrom(ast::ExprYieldFrom { value, range: _ }) => contains_await(value),
-        Expr::Compare(ast::ExprCompare {
-            left, comparators, ..
-        }) => contains_await(left) || comparators.iter().any(contains_await),
-        Expr::Call(ast::ExprCall {
-            func,
-            args,
-            keywords,
-            range: _,
-        }) => {
-            contains_await(func)
-                || args.iter().any(contains_await)
-                || keywords
-                    .iter()
-                    .any(|keyword| contains_await(&keyword.value))
-        }
-        Expr::FormattedValue(ast::ExprFormattedValue {
-            value, format_spec, ..
-        }) => {
-            contains_await(value)
-                || format_spec
-                    .as_ref()
-                    .map_or(false, |value| contains_await(value))
-        }
-        Expr::JoinedStr(ast::ExprJoinedStr { values, range: _ }) => {
-            values.iter().any(contains_await)
-        }
-        Expr::Constant(_) => false,
-        Expr::Attribute(ast::ExprAttribute { value, .. }) => contains_await(value),
-        Expr::Subscript(ast::ExprSubscript { value, slice, .. }) => {
-            contains_await(value) || contains_await(slice)
-        }
-        Expr::Starred(ast::ExprStarred { value, .. }) => contains_await(value),
-        Expr::Name(_) => false,
-        Expr::List(ast::ExprList { elts, .. }) => elts.iter().any(contains_await),
-        Expr::Tuple(ast::ExprTuple { elts, .. }) => elts.iter().any(contains_await),
-        Expr::Slice(ast::ExprSlice {
-            lower,
-            upper,
-            step,
-            range: _,
-        }) => {
-            lower.as_ref().map_or(false, |value| contains_await(value))
-                || upper.as_ref().map_or(false, |value| contains_await(value))
-                || step.as_ref().map_or(false, |value| contains_await(value))
-        }
-    }
-}
-
 /// UP027
 pub(crate) fn unpacked_list_comprehension(checker: &mut Checker, targets: &[Expr], value: &Expr) {
     let Some(target) = targets.get(0) else {
         return;
     };
-    if let Expr::Tuple(_) = target {
-        if let Expr::ListComp(ast::ExprListComp {
-            elt,
-            generators,
-            range: _,
-        }) = value
-        {
-            if generators.iter().any(|generator| generator.is_async) || contains_await(elt) {
-                return;
-            }
 
-            let mut diagnostic = Diagnostic::new(UnpackedListComprehension, value.range());
+    if !target.is_tuple_expr() {
-            if checker.patch(diagnostic.kind.rule()) {
+        return;
-                let existing = checker.locator.slice(value.range());
-
-                let mut content = String::with_capacity(existing.len());
-                content.push('(');
-                content.push_str(&existing[1..existing.len() - 1]);
-                content.push(')');
-                #[allow(deprecated)]
-                diagnostic.set_fix(Fix::unspecified(Edit::range_replacement(
-                    content,
-                    value.range(),
-                )));
-            }
-            checker.diagnostics.push(diagnostic);
-        }
     }
+
+    let Expr::ListComp(ast::ExprListComp {
+        elt,
+        generators,
+        range: _,
+    }) = value else {
+        return;
+    };
+
+    if generators.iter().any(|generator| generator.is_async) || contains_await(elt) {
+        return;
+    }
+
+    let mut diagnostic = Diagnostic::new(UnpackedListComprehension, value.range());
+    if checker.patch(diagnostic.kind.rule()) {
+        let existing = checker.locator.slice(value.range());
+
+        let mut content = String::with_capacity(existing.len());
+        content.push('(');
+        content.push_str(&existing[1..existing.len() - 1]);
+        content.push(')');
+        diagnostic.set_fix(Fix::suggested(Edit::range_replacement(
+            content,
+            value.range(),
+        )));
+    }
+    checker.diagnostics.push(diagnostic);
+}
+
+/// Return `true` if the [`Expr`] contains an `await` expression.
+fn contains_await(expr: &Expr) -> bool {
+    any_over_expr(expr, &Expr::is_await_expr)
 }