Skip to content

Commit 4aa55ff

Browse files
committed
handle generic return types in control flow and skip unsafe blocks
1 parent 71768d3 commit 4aa55ff

File tree

3 files changed

+124
-5
lines changed

3 files changed

+124
-5
lines changed

clippy_lints/src/casts/needless_type_cast.rs

Lines changed: 50 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
use clippy_utils::diagnostics::span_lint_and_sugg;
2-
use clippy_utils::visitors::{for_each_expr, for_each_expr_without_closures};
2+
use clippy_utils::visitors::{Descend, for_each_expr, for_each_expr_without_closures};
33
use core::ops::ControlFlow;
44
use rustc_data_structures::fx::FxHashMap;
55
use rustc_errors::Applicability;
66
use rustc_hir::def::{DefKind, Res};
7-
use rustc_hir::{Body, Expr, ExprKind, HirId, LetStmt, PatKind, StmtKind};
7+
use rustc_hir::{BlockCheckMode, Body, Expr, ExprKind, HirId, LetStmt, PatKind, StmtKind, UnsafeSource};
88
use rustc_lint::LateContext;
99
use rustc_middle::ty::{Ty, TypeVisitableExt};
1010
use rustc_span::Span;
@@ -55,7 +55,11 @@ fn collect_binding_from_let<'a>(
5555
let_expr: &rustc_hir::LetExpr<'a>,
5656
bindings: &mut FxHashMap<HirId, BindingInfo<'a>>,
5757
) {
58-
if let_expr.ty.is_none() || let_expr.span.from_expansion() || has_generic_return_type(cx, let_expr.init) {
58+
if let_expr.ty.is_none()
59+
|| let_expr.span.from_expansion()
60+
|| has_generic_return_type(cx, let_expr.init)
61+
|| contains_unsafe(let_expr.init)
62+
{
5963
return;
6064
}
6165

@@ -82,7 +86,9 @@ fn collect_binding_from_local<'a>(
8286
) {
8387
if let_stmt.ty.is_none()
8488
|| let_stmt.span.from_expansion()
85-
|| let_stmt.init.is_some_and(|init| has_generic_return_type(cx, init))
89+
|| let_stmt
90+
.init
91+
.is_some_and(|init| has_generic_return_type(cx, init) || contains_unsafe(init))
8692
{
8793
return;
8894
}
@@ -103,8 +109,48 @@ fn collect_binding_from_local<'a>(
103109
}
104110
}
105111

112+
fn contains_unsafe(expr: &Expr<'_>) -> bool {
113+
for_each_expr_without_closures(expr, |e| {
114+
if let ExprKind::Block(block, _) = e.kind
115+
&& let BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided) = block.rules
116+
{
117+
return ControlFlow::Break(());
118+
}
119+
ControlFlow::Continue(())
120+
})
121+
.is_some()
122+
}
123+
106124
fn has_generic_return_type(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
107125
match &expr.kind {
126+
ExprKind::Block(block, _) => {
127+
if let Some(tail_expr) = block.expr {
128+
return has_generic_return_type(cx, tail_expr);
129+
}
130+
false
131+
},
132+
ExprKind::If(_, then_block, else_expr) => {
133+
has_generic_return_type(cx, then_block) || else_expr.is_some_and(|e| has_generic_return_type(cx, e))
134+
},
135+
ExprKind::Match(_, arms, _) => arms.iter().any(|arm| has_generic_return_type(cx, arm.body)),
136+
ExprKind::Loop(block, label, ..) => for_each_expr_without_closures(*block, |e| {
137+
match e.kind {
138+
ExprKind::Loop(..) => {
139+
// Unlabeled breaks inside nested loops target the inner loop, not ours
140+
return ControlFlow::Continue(Descend::No);
141+
},
142+
ExprKind::Break(dest, Some(break_expr)) => {
143+
let targets_this_loop =
144+
dest.label.is_none() || dest.label.map(|l| l.ident) == label.map(|l| l.ident);
145+
if targets_this_loop && has_generic_return_type(cx, break_expr) {
146+
return ControlFlow::Break(());
147+
}
148+
},
149+
_ => {},
150+
}
151+
ControlFlow::Continue(Descend::Yes)
152+
})
153+
.is_some(),
108154
ExprKind::MethodCall(..) => {
109155
if let Some(def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id) {
110156
let sig = cx.tcx.fn_sig(def_id).instantiate_identity();
@@ -158,7 +204,6 @@ fn is_cast_in_generic_context<'a>(cx: &LateContext<'a>, cast_expr: &Expr<'a>) ->
158204
match parent {
159205
rustc_hir::Node::Expr(parent_expr) => {
160206
match &parent_expr.kind {
161-
// Closure body is a separate type inference context
162207
ExprKind::Closure(_) => return false,
163208
ExprKind::Call(callee, _) => {
164209
if let ExprKind::Path(qpath) = &callee.kind {

tests/ui/needless_type_cast.fixed

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,3 +143,40 @@ fn test_generic_initializer() {
143143
let _ = a as i32;
144144
let _ = a as i32;
145145
}
146+
147+
fn test_unsafe_transmute() {
148+
// Should not lint: initializer contains unsafe block
149+
#[allow(clippy::useless_transmute)]
150+
let x: u32 = unsafe { std::mem::transmute(0u32) };
151+
let _ = x as u64;
152+
}
153+
154+
fn test_if_with_generic() {
155+
// Should not lint: one branch has generic return type
156+
let x: u8 = if true { generic(1) } else { 2 };
157+
let _ = x as i32;
158+
}
159+
160+
fn test_match_with_generic() {
161+
// Should not lint: one branch has generic return type
162+
let x: u8 = match 1 {
163+
1 => generic(1),
164+
_ => 2,
165+
};
166+
let _ = x as i32;
167+
}
168+
169+
fn test_default() {
170+
// Should not lint: Default::default() has generic return type
171+
let x: u8 = Default::default();
172+
let _ = x as i32;
173+
}
174+
175+
fn test_loop_with_generic() {
176+
// Should not lint: loop break has generic return type
177+
#[allow(clippy::never_loop)]
178+
let x: u8 = loop {
179+
break generic(1);
180+
};
181+
let _ = x as i32;
182+
}

tests/ui/needless_type_cast.rs

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,3 +143,40 @@ fn test_generic_initializer() {
143143
let _ = a as i32;
144144
let _ = a as i32;
145145
}
146+
147+
fn test_unsafe_transmute() {
148+
// Should not lint: initializer contains unsafe block
149+
#[allow(clippy::useless_transmute)]
150+
let x: u32 = unsafe { std::mem::transmute(0u32) };
151+
let _ = x as u64;
152+
}
153+
154+
fn test_if_with_generic() {
155+
// Should not lint: one branch has generic return type
156+
let x: u8 = if true { generic(1) } else { 2 };
157+
let _ = x as i32;
158+
}
159+
160+
fn test_match_with_generic() {
161+
// Should not lint: one branch has generic return type
162+
let x: u8 = match 1 {
163+
1 => generic(1),
164+
_ => 2,
165+
};
166+
let _ = x as i32;
167+
}
168+
169+
fn test_default() {
170+
// Should not lint: Default::default() has generic return type
171+
let x: u8 = Default::default();
172+
let _ = x as i32;
173+
}
174+
175+
fn test_loop_with_generic() {
176+
// Should not lint: loop break has generic return type
177+
#[allow(clippy::never_loop)]
178+
let x: u8 = loop {
179+
break generic(1);
180+
};
181+
let _ = x as i32;
182+
}

0 commit comments

Comments
 (0)