Auto merge of #13608 - J-ZhengLi:issue12338-new, r=Alexendoo

[`infinite_loops`]: fix incorrect suggestions on async functions/closures closes: #12338 I intend to fix this in #12421 but got distracted by some other problems in the same lint, delaying the process of closing the actual issue. So here's a separated PR that only focus on the issue and nothing else. --- changelog: [`infinite_loops`]: fix suggestion error on async functions/closures
rust-lang · Nov 1, 2024 · e4dc892 · e4dc892
2 parents c782988 + c4815ae
commit e4dc892
Show file tree

Hide file tree

Showing 3 changed files with 95 additions and 29 deletions.
diff --git a/clippy_lints/src/loops/infinite_loop.rs b/clippy_lints/src/loops/infinite_loop.rs
@@ -1,12 +1,13 @@
 use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::{fn_def_id, is_from_proc_macro, is_lint_allowed};
 use hir::intravisit::{Visitor, walk_expr};
-use hir::{Expr, ExprKind, FnRetTy, FnSig, Node};
+use hir::{Expr, ExprKind, FnRetTy, FnSig, Node, TyKind};
 use rustc_ast::Label;
 use rustc_errors::Applicability;
 use rustc_hir as hir;
 use rustc_lint::{LateContext, LintContext};
 use rustc_middle::lint::in_external_macro;
+use rustc_span::sym;
 
 use super::INFINITE_LOOP;
 
@@ -25,13 +26,7 @@ pub(super) fn check<'tcx>(
         return;
     };
     // Or, its parent function is already returning `Never`
-    if matches!(
-        parent_fn_ret,
-        FnRetTy::Return(hir::Ty {
-            kind: hir::TyKind::Never,
-            ..
-        })
-    ) {
+    if is_never_return(parent_fn_ret) {
         return;
     }
 
@@ -69,6 +64,16 @@ pub(super) fn check<'tcx>(
 fn get_parent_fn_ret_ty<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>) -> Option<FnRetTy<'tcx>> {
     for (_, parent_node) in cx.tcx.hir().parent_iter(expr.hir_id) {
         match parent_node {
+            // Skip `Coroutine` closures, these are the body of `async fn`, not async closures.
+            // This is because we still need to backtrack one parent node to get the `OpaqueDef` ty.
+            Node::Expr(Expr {
+                kind:
+                    ExprKind::Closure(hir::Closure {
+                        kind: hir::ClosureKind::Coroutine(_),
+                        ..
+                    }),
+                ..
+            }) => (),
             Node::Item(hir::Item {
                 kind: hir::ItemKind::Fn(FnSig { decl, .. }, _, _),
                 ..
@@ -143,3 +148,44 @@ impl<'hir> Visitor<'hir> for LoopVisitor<'hir, '_> {
         }
     }
 }
+
+/// Return `true` if the given [`FnRetTy`] is never (!).
+///
+/// Note: This function also take care of return type of async fn,
+/// as the actual type is behind an [`OpaqueDef`](TyKind::OpaqueDef).
+fn is_never_return(ret_ty: FnRetTy<'_>) -> bool {
+    let FnRetTy::Return(hir_ty) = ret_ty else { return false };
+
+    match hir_ty.kind {
+        TyKind::Never => true,
+        TyKind::OpaqueDef(
+            hir::OpaqueTy {
+                origin: hir::OpaqueTyOrigin::AsyncFn { .. },
+                bounds,
+                ..
+            },
+            _,
+        ) => {
+            if let Some(trait_ref) = bounds.iter().find_map(|b| b.trait_ref())
+                && let Some(segment) = trait_ref
+                    .path
+                    .segments
+                    .iter()
+                    .find(|seg| seg.ident.name == sym::future_trait)
+                && let Some(args) = segment.args
+                && let Some(cst_kind) = args
+                    .constraints
+                    .iter()
+                    .find_map(|cst| (cst.ident.name == sym::Output).then_some(cst.kind))
+                && let hir::AssocItemConstraintKind::Equality {
+                    term: hir::Term::Ty(ty),
+                } = cst_kind
+            {
+                matches!(ty.kind, TyKind::Never)
+            } else {
+                false
+            }
+        },
+        _ => false,
+    }
+}
diff --git a/tests/ui/infinite_loops.rs b/tests/ui/infinite_loops.rs
@@ -3,6 +3,7 @@
 
 #![allow(clippy::never_loop)]
 #![warn(clippy::infinite_loop)]
+#![feature(async_closure)]
 
 extern crate proc_macros;
 use proc_macros::{external, with_span};
@@ -428,4 +429,23 @@ fn continue_outer() {
     }
 }
 
+// don't suggest adding `-> !` to async fn/closure that already returning `-> !`
+mod issue_12338 {
+    use super::do_something;
+
+    async fn foo() -> ! {
+        loop {
+            do_something();
+        }
+    }
+
+    fn bar() {
+        let _ = async || -> ! {
+            loop {
+                do_something();
+            }
+        };
+    }
+}
+
 fn main() {}
diff --git a/tests/ui/infinite_loops.stderr b/tests/ui/infinite_loops.stderr
@@ -1,5 +1,5 @@
 error: infinite loop detected
-  --> tests/ui/infinite_loops.rs:13:5
+  --> tests/ui/infinite_loops.rs:14:5
    |
 LL | /     loop {
 LL | |
@@ -15,7 +15,7 @@ LL | fn no_break() -> ! {
    |               ++++
 
 error: infinite loop detected
-  --> tests/ui/infinite_loops.rs:20:5
+  --> tests/ui/infinite_loops.rs:21:5
    |
 LL | /     loop {
 LL | |
@@ -32,7 +32,7 @@ LL | fn all_inf() -> ! {
    |              ++++
 
 error: infinite loop detected
-  --> tests/ui/infinite_loops.rs:22:9
+  --> tests/ui/infinite_loops.rs:23:9
    |
 LL | /         loop {
 LL | |
@@ -49,7 +49,7 @@ LL | fn all_inf() -> ! {
    |              ++++
 
 error: infinite loop detected
-  --> tests/ui/infinite_loops.rs:24:13
+  --> tests/ui/infinite_loops.rs:25:13
    |
 LL | /             loop {
 LL | |
@@ -63,7 +63,7 @@ LL | fn all_inf() -> ! {
    |              ++++
 
 error: infinite loop detected
-  --> tests/ui/infinite_loops.rs:38:5
+  --> tests/ui/infinite_loops.rs:39:5
    |
 LL | /     loop {
 LL | |
@@ -74,7 +74,7 @@ LL | |     }
    = help: if this is not intended, try adding a `break` or `return` condition in the loop
 
 error: infinite loop detected
-  --> tests/ui/infinite_loops.rs:51:5
+  --> tests/ui/infinite_loops.rs:52:5
    |
 LL | /     loop {
 LL | |         fn inner_fn() -> ! {
@@ -90,7 +90,7 @@ LL | fn no_break_never_ret_noise() -> ! {
    |                               ++++
 
 error: infinite loop detected
-  --> tests/ui/infinite_loops.rs:94:5
+  --> tests/ui/infinite_loops.rs:95:5
    |
 LL | /     loop {
 LL | |
@@ -107,7 +107,7 @@ LL | fn break_inner_but_not_outer_1(cond: bool) -> ! {
    |                                            ++++
 
 error: infinite loop detected
-  --> tests/ui/infinite_loops.rs:105:5
+  --> tests/ui/infinite_loops.rs:106:5
    |
 LL | /     loop {
 LL | |
@@ -124,7 +124,7 @@ LL | fn break_inner_but_not_outer_2(cond: bool) -> ! {
    |                                            ++++
 
 error: infinite loop detected
-  --> tests/ui/infinite_loops.rs:119:9
+  --> tests/ui/infinite_loops.rs:120:9
    |
 LL | /         loop {
 LL | |
@@ -138,7 +138,7 @@ LL | fn break_outer_but_not_inner() -> ! {
    |                                ++++
 
 error: infinite loop detected
-  --> tests/ui/infinite_loops.rs:142:9
+  --> tests/ui/infinite_loops.rs:143:9
    |
 LL | /         loop {
 LL | |
@@ -155,7 +155,7 @@ LL | fn break_wrong_loop(cond: bool) -> ! {
    |                                 ++++
 
 error: infinite loop detected
-  --> tests/ui/infinite_loops.rs:182:5
+  --> tests/ui/infinite_loops.rs:183:5
    |
 LL | /     loop {
 LL | |
@@ -172,7 +172,7 @@ LL | fn match_like() -> ! {
    |                 ++++
 
 error: infinite loop detected
-  --> tests/ui/infinite_loops.rs:223:5
+  --> tests/ui/infinite_loops.rs:224:5
    |
 LL | /     loop {
 LL | |
@@ -186,7 +186,7 @@ LL | fn match_like() -> ! {
    |                 ++++
 
 error: infinite loop detected
-  --> tests/ui/infinite_loops.rs:228:5
+  --> tests/ui/infinite_loops.rs:229:5
    |
 LL | /     loop {
 LL | |
@@ -203,7 +203,7 @@ LL | fn match_like() -> ! {
    |                 ++++
 
 error: infinite loop detected
-  --> tests/ui/infinite_loops.rs:333:9
+  --> tests/ui/infinite_loops.rs:334:9
    |
 LL | /         loop {
 LL | |
@@ -217,7 +217,7 @@ LL |     fn problematic_trait_method() -> ! {
    |                                   ++++
 
 error: infinite loop detected
-  --> tests/ui/infinite_loops.rs:343:9
+  --> tests/ui/infinite_loops.rs:344:9
    |
 LL | /         loop {
 LL | |
@@ -231,7 +231,7 @@ LL |     fn could_be_problematic() -> ! {
    |                               ++++
 
 error: infinite loop detected
-  --> tests/ui/infinite_loops.rs:352:9
+  --> tests/ui/infinite_loops.rs:353:9
    |
 LL | /         loop {
 LL | |
@@ -245,7 +245,7 @@ LL |     let _loop_forever = || -> ! {
    |                            ++++
 
 error: infinite loop detected
-  --> tests/ui/infinite_loops.rs:366:8
+  --> tests/ui/infinite_loops.rs:367:8
    |
 LL |       Ok(loop {
    |  ________^
@@ -256,7 +256,7 @@ LL | |     })
    = help: if this is not intended, try adding a `break` or `return` condition in the loop
 
 error: infinite loop detected
-  --> tests/ui/infinite_loops.rs:408:5
+  --> tests/ui/infinite_loops.rs:409:5
    |
 LL | /     'infinite: loop {
 LL | |
@@ -272,7 +272,7 @@ LL | fn continue_outer() -> ! {
    |                     ++++
 
 error: infinite loop detected
-  --> tests/ui/infinite_loops.rs:415:5
+  --> tests/ui/infinite_loops.rs:416:5
    |
 LL | /     loop {
 LL | |
@@ -289,7 +289,7 @@ LL | fn continue_outer() -> ! {
    |                     ++++
 
 error: infinite loop detected
-  --> tests/ui/infinite_loops.rs:417:9
+  --> tests/ui/infinite_loops.rs:418:9
    |
 LL | /         'inner: loop {
 LL | |             loop {
@@ -304,7 +304,7 @@ LL | fn continue_outer() -> ! {
    |                     ++++
 
 error: infinite loop detected
-  --> tests/ui/infinite_loops.rs:425:5
+  --> tests/ui/infinite_loops.rs:426:5
    |
 LL | /     loop {
 LL | |