Skip to content

Commit 783b5f9

Browse files
committed
lint bindings defined with one type but always cast to another
1 parent 3efb2ac commit 783b5f9

File tree

6 files changed

+375
-0
lines changed

6 files changed

+375
-0
lines changed

clippy_lints/src/casts/mod.rs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ mod fn_to_numeric_cast;
1919
mod fn_to_numeric_cast_any;
2020
mod fn_to_numeric_cast_with_truncation;
2121
mod manual_dangling_ptr;
22+
mod needless_type_cast;
2223
mod ptr_as_ptr;
2324
mod ptr_cast_constness;
2425
mod ref_as_ptr;
@@ -813,6 +814,32 @@ declare_clippy_lint! {
813814
"casting a primitive method pointer to any integer type"
814815
}
815816

817+
declare_clippy_lint! {
818+
/// ### What it does
819+
/// Checks for bindings (constants, statics, or let bindings) that are defined
820+
/// with one numeric type but are consistently cast to a different type in all usages.
821+
///
822+
/// ### Why is this bad?
823+
/// If a binding is always cast to a different type when used, it would be clearer
824+
/// and more efficient to define it with the target type from the start.
825+
///
826+
/// ### Example
827+
/// ```no_run
828+
/// const SIZE: u16 = 15;
829+
/// let arr: [u8; SIZE as usize] = [0; SIZE as usize];
830+
/// ```
831+
///
832+
/// Use instead:
833+
/// ```no_run
834+
/// const SIZE: usize = 15;
835+
/// let arr: [u8; SIZE] = [0; SIZE];
836+
/// ```
837+
#[clippy::version = "1.85.0"]
838+
pub NEEDLESS_TYPE_CAST,
839+
pedantic,
840+
"binding defined with one type but always cast to another"
841+
}
842+
816843
pub struct Casts {
817844
msrv: Msrv,
818845
}
@@ -851,6 +878,7 @@ impl_lint_pass!(Casts => [
851878
AS_POINTER_UNDERSCORE,
852879
MANUAL_DANGLING_PTR,
853880
CONFUSING_METHOD_TO_NUMERIC_CAST,
881+
NEEDLESS_TYPE_CAST,
854882
]);
855883

856884
impl<'tcx> LateLintPass<'tcx> for Casts {
@@ -920,4 +948,8 @@ impl<'tcx> LateLintPass<'tcx> for Casts {
920948
cast_slice_different_sizes::check(cx, expr, self.msrv);
921949
ptr_cast_constness::check_null_ptr_cast_method(cx, expr);
922950
}
951+
952+
fn check_body(&mut self, cx: &LateContext<'tcx>, body: &rustc_hir::Body<'tcx>) {
953+
needless_type_cast::check(cx, body);
954+
}
923955
}
Lines changed: 197 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,197 @@
1+
use clippy_utils::diagnostics::span_lint_and_sugg;
2+
use clippy_utils::source::snippet;
3+
use clippy_utils::visitors::for_each_expr_without_closures;
4+
use core::ops::ControlFlow;
5+
use rustc_data_structures::fx::FxHashMap;
6+
use rustc_errors::Applicability;
7+
use rustc_hir::def::Res;
8+
use rustc_hir::{Block, Body, ExprKind, HirId, LetStmt, PatKind, StmtKind};
9+
use rustc_lint::LateContext;
10+
use rustc_middle::ty::Ty;
11+
use rustc_span::Span;
12+
13+
use super::NEEDLESS_TYPE_CAST;
14+
15+
struct BindingInfo<'a> {
16+
source_ty: Ty<'a>,
17+
ty_span: Option<Span>,
18+
pat_span: Span,
19+
}
20+
21+
struct UsageInfo<'a> {
22+
is_cast: bool,
23+
cast_to: Option<Ty<'a>>,
24+
}
25+
26+
pub(super) fn check<'a>(cx: &LateContext<'a>, body: &Body<'a>) {
27+
let mut bindings: FxHashMap<HirId, BindingInfo<'a>> = FxHashMap::default();
28+
29+
collect_bindings_from_block(cx, body.value, &mut bindings);
30+
31+
for_each_expr_without_closures(body.value, |expr| {
32+
if let ExprKind::Let(let_expr) = expr.kind {
33+
collect_binding_from_let(cx, let_expr, &mut bindings);
34+
}
35+
ControlFlow::<()>::Continue(())
36+
});
37+
38+
#[allow(rustc::potential_query_instability)]
39+
let mut binding_vec: Vec<_> = bindings.into_iter().collect();
40+
binding_vec.sort_by_key(|(_, info)| info.pat_span.lo());
41+
42+
for (hir_id, binding_info) in binding_vec {
43+
check_binding_usages(cx, body, hir_id, &binding_info);
44+
}
45+
}
46+
47+
fn collect_bindings_from_block<'a>(
48+
cx: &LateContext<'a>,
49+
expr: &rustc_hir::Expr<'a>,
50+
bindings: &mut FxHashMap<HirId, BindingInfo<'a>>,
51+
) {
52+
if let ExprKind::Block(block, _) = expr.kind {
53+
collect_bindings_from_block_inner(cx, block, bindings);
54+
}
55+
}
56+
57+
fn collect_bindings_from_block_inner<'a>(
58+
cx: &LateContext<'a>,
59+
block: &Block<'a>,
60+
bindings: &mut FxHashMap<HirId, BindingInfo<'a>>,
61+
) {
62+
for stmt in block.stmts {
63+
if let StmtKind::Let(let_stmt) = stmt.kind {
64+
collect_binding_from_local(cx, let_stmt, bindings);
65+
}
66+
}
67+
}
68+
69+
fn collect_binding_from_let<'a>(
70+
cx: &LateContext<'a>,
71+
let_expr: &rustc_hir::LetExpr<'a>,
72+
bindings: &mut FxHashMap<HirId, BindingInfo<'a>>,
73+
) {
74+
if let PatKind::Binding(_, hir_id, _, _) = let_expr.pat.kind {
75+
let ty = cx.typeck_results().pat_ty(let_expr.pat);
76+
if ty.is_numeric() {
77+
bindings.insert(
78+
hir_id,
79+
BindingInfo {
80+
source_ty: ty,
81+
ty_span: let_expr.ty.map(|t| t.span),
82+
pat_span: let_expr.pat.span,
83+
},
84+
);
85+
}
86+
}
87+
}
88+
89+
fn collect_binding_from_local<'a>(
90+
cx: &LateContext<'a>,
91+
let_stmt: &LetStmt<'a>,
92+
bindings: &mut FxHashMap<HirId, BindingInfo<'a>>,
93+
) {
94+
// Only check bindings with explicit type annotations
95+
// Otherwise, the suggestion to change the type may not be valid
96+
// (e.g., `let x = 42u8;` cannot just change to `let x: i64 = 42u8;`)
97+
if let_stmt.ty.is_none() {
98+
return;
99+
}
100+
101+
if let PatKind::Binding(_, hir_id, _, _) = let_stmt.pat.kind {
102+
let ty = cx.typeck_results().pat_ty(let_stmt.pat);
103+
if ty.is_numeric() {
104+
bindings.insert(
105+
hir_id,
106+
BindingInfo {
107+
source_ty: ty,
108+
ty_span: let_stmt.ty.map(|t| t.span),
109+
pat_span: let_stmt.pat.span,
110+
},
111+
);
112+
}
113+
}
114+
}
115+
116+
fn check_binding_usages<'a>(cx: &LateContext<'a>, body: &Body<'a>, hir_id: HirId, binding_info: &BindingInfo<'a>) {
117+
let mut usages: Vec<UsageInfo<'a>> = Vec::new();
118+
119+
for_each_expr_without_closures(body.value, |expr| {
120+
if let ExprKind::Path(ref qpath) = expr.kind {
121+
if let Res::Local(id) = cx.qpath_res(qpath, expr.hir_id) {
122+
if id == hir_id {
123+
let parent_id = cx.tcx.parent_hir_id(expr.hir_id);
124+
let parent = cx.tcx.hir_node(parent_id);
125+
126+
if let rustc_hir::Node::Expr(parent_expr) = parent {
127+
if let ExprKind::Cast(_, _) = parent_expr.kind {
128+
let target_ty = cx.typeck_results().expr_ty(parent_expr);
129+
usages.push(UsageInfo {
130+
is_cast: true,
131+
cast_to: Some(target_ty),
132+
});
133+
} else {
134+
usages.push(UsageInfo {
135+
is_cast: false,
136+
cast_to: None,
137+
});
138+
}
139+
} else {
140+
usages.push(UsageInfo {
141+
is_cast: false,
142+
cast_to: None,
143+
});
144+
}
145+
}
146+
}
147+
}
148+
ControlFlow::<()>::Continue(())
149+
});
150+
151+
if usages.is_empty() {
152+
return;
153+
}
154+
155+
if !usages.iter().all(|u| u.is_cast) {
156+
return;
157+
}
158+
159+
let first_target = match usages.first().and_then(|u| u.cast_to) {
160+
Some(ty) => ty,
161+
None => return,
162+
};
163+
164+
if !usages.iter().all(|u| u.cast_to == Some(first_target)) {
165+
return;
166+
}
167+
168+
if first_target == binding_info.source_ty {
169+
return;
170+
}
171+
172+
let suggestion = if binding_info.ty_span.is_some() {
173+
format!("{first_target}")
174+
} else {
175+
format!(": {first_target}")
176+
};
177+
178+
let span = binding_info.ty_span.unwrap_or(binding_info.pat_span);
179+
let current_snippet = snippet(cx, span, "_");
180+
181+
span_lint_and_sugg(
182+
cx,
183+
NEEDLESS_TYPE_CAST,
184+
span,
185+
format!(
186+
"this binding is defined as `{}` but is always cast to `{}`",
187+
binding_info.source_ty, first_target
188+
),
189+
"consider defining it as",
190+
if binding_info.ty_span.is_some() {
191+
suggestion
192+
} else {
193+
format!("{current_snippet}{suggestion}")
194+
},
195+
Applicability::MaybeIncorrect,
196+
);
197+
}

clippy_lints/src/declared_lints.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
7070
crate::casts::FN_TO_NUMERIC_CAST_ANY_INFO,
7171
crate::casts::FN_TO_NUMERIC_CAST_WITH_TRUNCATION_INFO,
7272
crate::casts::MANUAL_DANGLING_PTR_INFO,
73+
crate::casts::NEEDLESS_TYPE_CAST_INFO,
7374
crate::casts::PTR_AS_PTR_INFO,
7475
crate::casts::PTR_CAST_CONSTNESS_INFO,
7576
crate::casts::REF_AS_PTR_INFO,

tests/ui/needless_type_cast.fixed

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
#![warn(clippy::needless_type_cast)]
2+
#![allow(clippy::no_effect, clippy::unnecessary_cast, unused)]
3+
4+
fn main() {
5+
// Should lint: let binding always cast to i32
6+
let count: i32 = 10;
7+
//~^ needless_type_cast
8+
let _a = count as i32 + 5;
9+
let _b = count as i32 * 2;
10+
11+
// Should lint: let binding always cast to u64
12+
let value: u64 = 20;
13+
//~^ needless_type_cast
14+
let _x = value as u64;
15+
let _y = value as u64 + 100;
16+
17+
// Should NOT lint: binding used without cast
18+
let mixed_use: u16 = 30;
19+
let _p = mixed_use;
20+
let _q = mixed_use as u32;
21+
22+
// Should NOT lint: binding cast to different types
23+
let different: u8 = 5;
24+
let _m = different as u16;
25+
let _n = different as u32;
26+
27+
// Should NOT lint: binding not cast at all
28+
let plain: i32 = 100;
29+
let _r = plain + 1;
30+
let _s = plain * 2;
31+
32+
// Should NOT lint: inferred type with literal suffix (suggestion doesn't fix the literal)
33+
let inferred = 42u8;
34+
let _t = inferred as i64;
35+
let _u = inferred as i64 + 10;
36+
37+
// Should lint: single usage that is a cast
38+
let single: usize = 1;
39+
//~^ needless_type_cast
40+
let _v = single as usize;
41+
}
42+
43+
fn test_multiple_bindings() {
44+
// Should lint: both bindings always cast
45+
let width: u32 = 10;
46+
//~^ needless_type_cast
47+
let height: u32 = 20;
48+
//~^ needless_type_cast
49+
let _area = (width as u32) * (height as u32);
50+
}
51+
52+
fn test_no_usage() {
53+
// Should NOT lint: binding never used
54+
let _unused: u16 = 30;
55+
}

tests/ui/needless_type_cast.rs

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
#![warn(clippy::needless_type_cast)]
2+
#![allow(clippy::no_effect, clippy::unnecessary_cast, unused)]
3+
4+
fn main() {
5+
// Should lint: let binding always cast to i32
6+
let count: u8 = 10;
7+
//~^ needless_type_cast
8+
let _a = count as i32 + 5;
9+
let _b = count as i32 * 2;
10+
11+
// Should lint: let binding always cast to u64
12+
let value: u16 = 20;
13+
//~^ needless_type_cast
14+
let _x = value as u64;
15+
let _y = value as u64 + 100;
16+
17+
// Should NOT lint: binding used without cast
18+
let mixed_use: u16 = 30;
19+
let _p = mixed_use;
20+
let _q = mixed_use as u32;
21+
22+
// Should NOT lint: binding cast to different types
23+
let different: u8 = 5;
24+
let _m = different as u16;
25+
let _n = different as u32;
26+
27+
// Should NOT lint: binding not cast at all
28+
let plain: i32 = 100;
29+
let _r = plain + 1;
30+
let _s = plain * 2;
31+
32+
// Should NOT lint: inferred type with literal suffix (suggestion doesn't fix the literal)
33+
let inferred = 42u8;
34+
let _t = inferred as i64;
35+
let _u = inferred as i64 + 10;
36+
37+
// Should lint: single usage that is a cast
38+
let single: u8 = 1;
39+
//~^ needless_type_cast
40+
let _v = single as usize;
41+
}
42+
43+
fn test_multiple_bindings() {
44+
// Should lint: both bindings always cast
45+
let width: u8 = 10;
46+
//~^ needless_type_cast
47+
let height: u8 = 20;
48+
//~^ needless_type_cast
49+
let _area = (width as u32) * (height as u32);
50+
}
51+
52+
fn test_no_usage() {
53+
// Should NOT lint: binding never used
54+
let _unused: u16 = 30;
55+
}

0 commit comments

Comments
 (0)