Skip to content

Commit 6110c80

Browse files
authored
Fix needless_type_cast suggesting invalid code for non-literal initializers (#16248)
When the initializer is a function call or suffixed literal (e.g., `size_of::<T>()` or `10u8`), suggest casting the initializer too, since changing only the type annotation would fail to compile. Fixes #16240 ``` changelog: [`needless_type_cast`]: Fix suggesting invalid code for non-literal initializers ```
2 parents 37330eb + 0f17de7 commit 6110c80

File tree

4 files changed

+330
-17
lines changed

4 files changed

+330
-17
lines changed

clippy_lints/src/casts/needless_type_cast.rs

Lines changed: 54 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
1-
use clippy_utils::diagnostics::span_lint_and_sugg;
1+
use clippy_utils::diagnostics::span_lint_and_then;
2+
use clippy_utils::sugg::Sugg;
23
use clippy_utils::visitors::{Descend, for_each_expr, for_each_expr_without_closures};
34
use core::ops::ControlFlow;
5+
use rustc_ast::ast::{LitFloatType, LitIntType, LitKind};
46
use rustc_data_structures::fx::FxHashMap;
57
use rustc_errors::Applicability;
68
use rustc_hir::def::{DefKind, Res};
@@ -14,6 +16,7 @@ use super::NEEDLESS_TYPE_CAST;
1416
struct BindingInfo<'a> {
1517
source_ty: Ty<'a>,
1618
ty_span: Span,
19+
init: Option<&'a Expr<'a>>,
1720
}
1821

1922
struct UsageInfo<'a> {
@@ -73,6 +76,7 @@ fn collect_binding_from_let<'a>(
7376
BindingInfo {
7477
source_ty: ty,
7578
ty_span: ty_hir.span,
79+
init: Some(let_expr.init),
7680
},
7781
);
7882
}
@@ -103,6 +107,7 @@ fn collect_binding_from_local<'a>(
103107
BindingInfo {
104108
source_ty: ty,
105109
ty_span: ty_hir.span,
110+
init: let_stmt.init,
106111
},
107112
);
108113
}
@@ -229,6 +234,18 @@ fn is_cast_in_generic_context<'a>(cx: &LateContext<'a>, cast_expr: &Expr<'a>) ->
229234
}
230235
}
231236

237+
fn can_coerce_to_target_type(expr: &Expr<'_>) -> bool {
238+
match expr.kind {
239+
ExprKind::Lit(lit) => matches!(
240+
lit.node,
241+
LitKind::Int(_, LitIntType::Unsuffixed) | LitKind::Float(_, LitFloatType::Unsuffixed)
242+
),
243+
ExprKind::Unary(rustc_hir::UnOp::Neg, inner) => can_coerce_to_target_type(inner),
244+
ExprKind::Binary(_, lhs, rhs) => can_coerce_to_target_type(lhs) && can_coerce_to_target_type(rhs),
245+
_ => false,
246+
}
247+
}
248+
232249
fn check_binding_usages<'a>(cx: &LateContext<'a>, body: &Body<'a>, hir_id: HirId, binding_info: &BindingInfo<'a>) {
233250
let mut usages = Vec::new();
234251

@@ -269,16 +286,48 @@ fn check_binding_usages<'a>(cx: &LateContext<'a>, body: &Body<'a>, hir_id: HirId
269286
return;
270287
};
271288

272-
span_lint_and_sugg(
289+
// Don't lint if there's exactly one use and the initializer cannot be coerced to the
290+
// target type (i.e., would require an explicit cast). In such cases, the fix would add
291+
// a cast to the initializer rather than eliminating one - the cast isn't truly "needless."
292+
// See: https://github.com/rust-lang/rust-clippy/issues/16240
293+
if usages.len() == 1
294+
&& binding_info
295+
.init
296+
.is_some_and(|init| !can_coerce_to_target_type(init) && !init.span.from_expansion())
297+
{
298+
return;
299+
}
300+
301+
span_lint_and_then(
273302
cx,
274303
NEEDLESS_TYPE_CAST,
275304
binding_info.ty_span,
276305
format!(
277306
"this binding is defined as `{}` but is always cast to `{}`",
278307
binding_info.source_ty, first_target
279308
),
280-
"consider defining it as",
281-
first_target.to_string(),
282-
Applicability::MaybeIncorrect,
309+
|diag| {
310+
if let Some(init) = binding_info
311+
.init
312+
.filter(|i| !can_coerce_to_target_type(i) && !i.span.from_expansion())
313+
{
314+
let sugg = Sugg::hir(cx, init, "..").as_ty(first_target);
315+
diag.multipart_suggestion(
316+
format!("consider defining it as `{first_target}` and casting the initializer"),
317+
vec![
318+
(binding_info.ty_span, first_target.to_string()),
319+
(init.span, sugg.to_string()),
320+
],
321+
Applicability::MachineApplicable,
322+
);
323+
} else {
324+
diag.span_suggestion(
325+
binding_info.ty_span,
326+
"consider defining it as",
327+
first_target.to_string(),
328+
Applicability::MachineApplicable,
329+
);
330+
}
331+
},
283332
);
284333
}

tests/ui/needless_type_cast.fixed

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@ fn generic<T>(x: T) -> T {
99
x
1010
}
1111

12+
fn returns_u8() -> u8 {
13+
10
14+
}
15+
1216
fn main() {
1317
let a: i32 = 10;
1418
//~^ needless_type_cast
@@ -180,3 +184,86 @@ fn test_loop_with_generic() {
180184
};
181185
let _ = x as i32;
182186
}
187+
188+
fn test_size_of_cast() {
189+
use std::mem::size_of;
190+
// Should lint: suggest casting the initializer
191+
let size: u64 = size_of::<u32>() as u64;
192+
//~^ needless_type_cast
193+
let _ = size as u64;
194+
let _ = size as u64;
195+
}
196+
197+
fn test_suffixed_literal_cast() {
198+
// Should lint: suggest casting the initializer
199+
let a: i32 = 10u8 as i32;
200+
//~^ needless_type_cast
201+
let _ = a as i32;
202+
let _ = a as i32;
203+
}
204+
205+
fn test_negative_literal() {
206+
// Negative literal - should just change type, not add cast
207+
let a: i32 = -10;
208+
//~^ needless_type_cast
209+
let _ = a as i32;
210+
let _ = a as i32;
211+
}
212+
213+
fn test_suffixed_negative_literal() {
214+
// Suffixed negative - needs cast
215+
let a: i32 = -10i8 as i32;
216+
//~^ needless_type_cast
217+
let _ = a as i32;
218+
let _ = a as i32;
219+
}
220+
221+
fn test_binary_op() {
222+
// Binary op needs parens in cast
223+
let a: i32 = 10 + 5;
224+
//~^ needless_type_cast
225+
let _ = a as i32;
226+
let _ = a as i32;
227+
}
228+
229+
fn test_fn_return_as_init() {
230+
let a: i32 = returns_u8() as i32;
231+
//~^ needless_type_cast
232+
let _ = a as i32;
233+
let _ = a as i32;
234+
}
235+
236+
fn test_method_as_init() {
237+
let a: i32 = 2u8.saturating_add(3) as i32;
238+
//~^ needless_type_cast
239+
let _ = a as i32;
240+
let _ = a as i32;
241+
}
242+
243+
fn test_const_as_init() {
244+
const X: u8 = 10;
245+
let a: i32 = X as i32;
246+
//~^ needless_type_cast
247+
let _ = a as i32;
248+
let _ = a as i32;
249+
}
250+
251+
fn test_single_use_fn_call() {
252+
// Should not lint: only one use, and fixing would just move the cast
253+
// to the initializer rather than eliminating it
254+
let a: u8 = returns_u8();
255+
let _ = a as i32;
256+
}
257+
258+
fn test_single_use_suffixed_literal() {
259+
// Should not lint: only one use with a suffixed literal
260+
let a: u8 = 10u8;
261+
let _ = a as i32;
262+
}
263+
264+
fn test_single_use_binary_op() {
265+
// Should lint: binary op of unsuffixed literals can be coerced
266+
let a: i32 = 10 + 5;
267+
//~^ needless_type_cast
268+
let _ = a as i32;
269+
}

tests/ui/needless_type_cast.rs

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@ fn generic<T>(x: T) -> T {
99
x
1010
}
1111

12+
fn returns_u8() -> u8 {
13+
10
14+
}
15+
1216
fn main() {
1317
let a: u8 = 10;
1418
//~^ needless_type_cast
@@ -180,3 +184,86 @@ fn test_loop_with_generic() {
180184
};
181185
let _ = x as i32;
182186
}
187+
188+
fn test_size_of_cast() {
189+
use std::mem::size_of;
190+
// Should lint: suggest casting the initializer
191+
let size: usize = size_of::<u32>();
192+
//~^ needless_type_cast
193+
let _ = size as u64;
194+
let _ = size as u64;
195+
}
196+
197+
fn test_suffixed_literal_cast() {
198+
// Should lint: suggest casting the initializer
199+
let a: u8 = 10u8;
200+
//~^ needless_type_cast
201+
let _ = a as i32;
202+
let _ = a as i32;
203+
}
204+
205+
fn test_negative_literal() {
206+
// Negative literal - should just change type, not add cast
207+
let a: i8 = -10;
208+
//~^ needless_type_cast
209+
let _ = a as i32;
210+
let _ = a as i32;
211+
}
212+
213+
fn test_suffixed_negative_literal() {
214+
// Suffixed negative - needs cast
215+
let a: i8 = -10i8;
216+
//~^ needless_type_cast
217+
let _ = a as i32;
218+
let _ = a as i32;
219+
}
220+
221+
fn test_binary_op() {
222+
// Binary op needs parens in cast
223+
let a: u8 = 10 + 5;
224+
//~^ needless_type_cast
225+
let _ = a as i32;
226+
let _ = a as i32;
227+
}
228+
229+
fn test_fn_return_as_init() {
230+
let a: u8 = returns_u8();
231+
//~^ needless_type_cast
232+
let _ = a as i32;
233+
let _ = a as i32;
234+
}
235+
236+
fn test_method_as_init() {
237+
let a: u8 = 2u8.saturating_add(3);
238+
//~^ needless_type_cast
239+
let _ = a as i32;
240+
let _ = a as i32;
241+
}
242+
243+
fn test_const_as_init() {
244+
const X: u8 = 10;
245+
let a: u8 = X;
246+
//~^ needless_type_cast
247+
let _ = a as i32;
248+
let _ = a as i32;
249+
}
250+
251+
fn test_single_use_fn_call() {
252+
// Should not lint: only one use, and fixing would just move the cast
253+
// to the initializer rather than eliminating it
254+
let a: u8 = returns_u8();
255+
let _ = a as i32;
256+
}
257+
258+
fn test_single_use_suffixed_literal() {
259+
// Should not lint: only one use with a suffixed literal
260+
let a: u8 = 10u8;
261+
let _ = a as i32;
262+
}
263+
264+
fn test_single_use_binary_op() {
265+
// Should lint: binary op of unsuffixed literals can be coerced
266+
let a: u8 = 10 + 5;
267+
//~^ needless_type_cast
268+
let _ = a as i32;
269+
}

0 commit comments

Comments
 (0)