Skip to content

Commit fbe292e

Browse files
committed
Auto merge of rust-lang#10971 - Centri3:unnecessary_cast_fully_qual_fix, r=dswij
Check for fully qualified paths in `unnecessary_cast` Noticed this doesn't pick up `::std::primitive::u32` or the sort, now it does changelog: none
2 parents 0f31fff + 15b68c2 commit fbe292e

File tree

5 files changed

+104
-48
lines changed

5 files changed

+104
-48
lines changed

clippy_lints/src/casts/mod.rs

+8
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,14 @@ declare_clippy_lint! {
181181
/// ### Why is this bad?
182182
/// It's just unnecessary.
183183
///
184+
/// ### Known problems
185+
/// When the expression on the left is a function call, the lint considers the return type to be
186+
/// a type alias if it's aliased through a `use` statement
187+
/// (like `use std::io::Result as IoResult`). It will not lint such cases.
188+
///
189+
/// This check is also rather primitive. It will only work on primitive types without any
190+
/// intermediate references, raw pointers and trait objects may or may not work.
191+
///
184192
/// ### Example
185193
/// ```rust
186194
/// let _ = 2i32 as i32;

clippy_lints/src/casts/unnecessary_cast.rs

+12-10
Original file line numberDiff line numberDiff line change
@@ -85,11 +85,6 @@ pub(super) fn check<'tcx>(
8585
}
8686
}
8787

88-
// skip cast of fn call that returns type alias
89-
if let ExprKind::Cast(inner, ..) = expr.kind && is_cast_from_ty_alias(cx, inner, cast_from) {
90-
return false;
91-
}
92-
9388
// skip cast to non-primitive type
9489
if_chain! {
9590
if let ExprKind::Cast(_, cast_to) = expr.kind;
@@ -101,6 +96,11 @@ pub(super) fn check<'tcx>(
10196
}
10297
}
10398

99+
// skip cast of fn call that returns type alias
100+
if let ExprKind::Cast(inner, ..) = expr.kind && is_cast_from_ty_alias(cx, inner, cast_from) {
101+
return false;
102+
}
103+
104104
if let Some(lit) = get_numeric_literal(cast_expr) {
105105
let literal_str = &cast_str;
106106

@@ -254,14 +254,12 @@ fn is_cast_from_ty_alias<'tcx>(cx: &LateContext<'tcx>, expr: impl Visitable<'tcx
254254
// function's declaration snippet is exactly equal to the `Ty`. That way, we can
255255
// see whether it's a type alias.
256256
//
257-
// Will this work for more complex types? Probably not!
257+
// FIXME: This won't work if the type is given an alias through `use`, should we
258+
// consider this a type alias as well?
258259
if !snippet
259260
.split("->")
260261
.skip(1)
261-
.map(|s| {
262-
s.trim() == cast_from.to_string()
263-
|| s.split("where").any(|ty| ty.trim() == cast_from.to_string())
264-
})
262+
.map(|s| snippet_eq_ty(s, cast_from) || s.split("where").any(|ty| snippet_eq_ty(ty, cast_from)))
265263
.any(|a| a)
266264
{
267265
return ControlFlow::Break(());
@@ -288,3 +286,7 @@ fn is_cast_from_ty_alias<'tcx>(cx: &LateContext<'tcx>, expr: impl Visitable<'tcx
288286
})
289287
.is_some()
290288
}
289+
290+
fn snippet_eq_ty(snippet: &str, ty: Ty<'_>) -> bool {
291+
snippet.trim() == ty.to_string() || snippet.trim().contains(&format!("::{ty}"))
292+
}

tests/ui/unnecessary_cast.fixed

+17
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,16 @@ mod fake_libc {
3838
}
3939
}
4040

41+
fn aaa() -> ::std::primitive::u32 {
42+
0
43+
}
44+
45+
use std::primitive::u32 as UnsignedThirtyTwoBitInteger;
46+
47+
fn bbb() -> UnsignedThirtyTwoBitInteger {
48+
0
49+
}
50+
4151
#[rustfmt::skip]
4252
fn main() {
4353
// Test cast_unnecessary
@@ -105,6 +115,13 @@ fn main() {
105115
extern_fake_libc::getpid_SAFE_TRUTH() as i32;
106116
let pid = unsafe { fake_libc::getpid() };
107117
pid as i32;
118+
aaa();
119+
let x = aaa();
120+
aaa();
121+
// Will not lint currently.
122+
bbb() as u32;
123+
let x = bbb();
124+
bbb() as u32;
108125

109126
let i8_ptr: *const i8 = &1;
110127
let u8_ptr: *const u8 = &1;

tests/ui/unnecessary_cast.rs

+17
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,16 @@ mod fake_libc {
3838
}
3939
}
4040

41+
fn aaa() -> ::std::primitive::u32 {
42+
0
43+
}
44+
45+
use std::primitive::u32 as UnsignedThirtyTwoBitInteger;
46+
47+
fn bbb() -> UnsignedThirtyTwoBitInteger {
48+
0
49+
}
50+
4151
#[rustfmt::skip]
4252
fn main() {
4353
// Test cast_unnecessary
@@ -105,6 +115,13 @@ fn main() {
105115
extern_fake_libc::getpid_SAFE_TRUTH() as i32;
106116
let pid = unsafe { fake_libc::getpid() };
107117
pid as i32;
118+
aaa() as u32;
119+
let x = aaa();
120+
aaa() as u32;
121+
// Will not lint currently.
122+
bbb() as u32;
123+
let x = bbb();
124+
bbb() as u32;
108125

109126
let i8_ptr: *const i8 = &1;
110127
let u8_ptr: *const u8 = &1;

0 commit comments

Comments
 (0)