From 22f57ff5842fde8acf78ea59528c149b0046b5fb Mon Sep 17 00:00:00 2001 From: bendn Date: Wed, 20 Mar 2024 23:33:16 +0700 Subject: [PATCH 1/3] fix `for x in y unsafe { }` --- clippy_lints/src/needless_for_each.rs | 5 +++-- tests/ui/needless_for_each_fixable.fixed | 4 ++++ tests/ui/needless_for_each_fixable.rs | 4 ++++ 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/needless_for_each.rs b/clippy_lints/src/needless_for_each.rs index 84a07df1bb0a..fda15f469f42 100644 --- a/clippy_lints/src/needless_for_each.rs +++ b/clippy_lints/src/needless_for_each.rs @@ -1,6 +1,6 @@ use rustc_errors::Applicability; use rustc_hir::intravisit::{walk_expr, Visitor}; -use rustc_hir::{Closure, Expr, ExprKind, Stmt, StmtKind}; +use rustc_hir::{Block, BlockCheckMode, Closure, Expr, ExprKind, Stmt, StmtKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::declare_lint_pass; use rustc_span::{sym, Span, Symbol}; @@ -68,7 +68,8 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessForEach { // e.g. `v.iter().for_each(f)` is simpler and clearer than using `for` loop. && let ExprKind::Closure(&Closure { body, .. }) = for_each_arg.kind && let body = cx.tcx.hir().body(body) - && let ExprKind::Block(..) = body.value.kind + // Skip the lint if the body is not safe. + && let ExprKind::Block(Block { rules: BlockCheckMode::DefaultBlock, .. }, ..) = body.value.kind { let mut ret_collector = RetCollector::default(); ret_collector.visit_expr(body.value); diff --git a/tests/ui/needless_for_each_fixable.fixed b/tests/ui/needless_for_each_fixable.fixed index 8c0e7ba76278..2362314290e6 100644 --- a/tests/ui/needless_for_each_fixable.fixed +++ b/tests/ui/needless_for_each_fixable.fixed @@ -113,6 +113,10 @@ fn should_not_lint() { let _ = v.iter().for_each(|elem| { acc += elem; }); + // `for_each` has a closure with an unsafe block. + v.iter().for_each(|elem| unsafe { + acc += elem; + }); } fn main() {} diff --git a/tests/ui/needless_for_each_fixable.rs b/tests/ui/needless_for_each_fixable.rs index cdc903a636c9..5b1186daa229 100644 --- a/tests/ui/needless_for_each_fixable.rs +++ b/tests/ui/needless_for_each_fixable.rs @@ -113,6 +113,10 @@ fn should_not_lint() { let _ = v.iter().for_each(|elem| { acc += elem; }); + // `for_each` has a closure with an unsafe block. + v.iter().for_each(|elem| unsafe { + acc += elem; + }); } fn main() {} From a356785065cd31d1501f533393f4b2bdbd04075e Mon Sep 17 00:00:00 2001 From: bendn Date: Thu, 21 Mar 2024 05:48:16 +0700 Subject: [PATCH 2/3] reasoning --- clippy_lints/src/needless_for_each.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/clippy_lints/src/needless_for_each.rs b/clippy_lints/src/needless_for_each.rs index fda15f469f42..3fe3d50197d2 100644 --- a/clippy_lints/src/needless_for_each.rs +++ b/clippy_lints/src/needless_for_each.rs @@ -68,7 +68,8 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessForEach { // e.g. `v.iter().for_each(f)` is simpler and clearer than using `for` loop. && let ExprKind::Closure(&Closure { body, .. }) = for_each_arg.kind && let body = cx.tcx.hir().body(body) - // Skip the lint if the body is not safe. + // Skip the lint if the body is not safe, so as not to suggest `for … in … unsafe {}` + // and suggesting `for … in … { unsafe { } }` is a little ugly. && let ExprKind::Block(Block { rules: BlockCheckMode::DefaultBlock, .. }, ..) = body.value.kind { let mut ret_collector = RetCollector::default(); From a3ef100fa6fdb7d07c52b35d2462fa74cbf7ccd3 Mon Sep 17 00:00:00 2001 From: bendn Date: Wed, 1 May 2024 07:45:15 +0700 Subject: [PATCH 3/3] warning --- clippy_lints/src/needless_for_each.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/clippy_lints/src/needless_for_each.rs b/clippy_lints/src/needless_for_each.rs index 3fe3d50197d2..630018238f4c 100644 --- a/clippy_lints/src/needless_for_each.rs +++ b/clippy_lints/src/needless_for_each.rs @@ -35,6 +35,16 @@ declare_clippy_lint! { /// println!("{}", elem); /// } /// ``` + /// + /// ### Known Problems + /// When doing things such as: + /// ```ignore + /// let v = vec![0, 1, 2]; + /// v.iter().for_each(|elem| unsafe { + /// libc::printf(c"%d\n".as_ptr(), elem); + /// }); + /// ``` + /// This lint will not trigger. #[clippy::version = "1.53.0"] pub NEEDLESS_FOR_EACH, pedantic,