Skip to content

Ignore more type aliases in unnecessary_cast #10942

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 68 additions & 3 deletions clippy_lints/src/casts/unnecessary_cast.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::numeric_literal::NumericLiteral;
use clippy_utils::source::snippet_opt;
use clippy_utils::{get_parent_expr, is_hir_ty_cfg_dependant, is_ty_alias, path_to_local};
use clippy_utils::visitors::{for_each_expr, Visitable};
use clippy_utils::{get_parent_expr, get_parent_node, is_hir_ty_cfg_dependant, is_ty_alias, path_to_local};
use if_chain::if_chain;
use rustc_ast::{LitFloatType, LitIntType, LitKind};
use rustc_errors::Applicability;
use rustc_hir::def::Res;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::{Expr, ExprKind, Lit, Node, Path, QPath, TyKind, UnOp};
use rustc_lint::{LateContext, LintContext};
use rustc_middle::lint::in_external_macro;
use rustc_middle::ty::{self, FloatTy, InferTy, Ty};
use std::ops::ControlFlow;

use super::UNNECESSARY_CAST;

Expand Down Expand Up @@ -59,7 +61,7 @@ pub(super) fn check<'tcx>(
}
}

// skip cast of local to type alias
// skip cast of local that is a type alias
if let ExprKind::Cast(inner, ..) = expr.kind
&& let ExprKind::Path(qpath) = inner.kind
&& let QPath::Resolved(None, Path { res, .. }) = qpath
Expand All @@ -83,6 +85,11 @@ pub(super) fn check<'tcx>(
}
}

// skip cast of fn call that returns type alias
if let ExprKind::Cast(inner, ..) = expr.kind && is_cast_from_ty_alias(cx, inner, cast_from) {
return false;
}

// skip cast to non-primitive type
if_chain! {
if let ExprKind::Cast(_, cast_to) = expr.kind;
Expand Down Expand Up @@ -223,3 +230,61 @@ fn fp_ty_mantissa_nbits(typ: Ty<'_>) -> u32 {
_ => 0,
}
}

/// Finds whether an `Expr` returns a type alias.
///
/// TODO: Maybe we should move this to `clippy_utils` so others won't need to go down this dark,
/// dark path reimplementing this (or something similar).
fn is_cast_from_ty_alias<'tcx>(cx: &LateContext<'tcx>, expr: impl Visitable<'tcx>, cast_from: Ty<'tcx>) -> bool {
for_each_expr(expr, |expr| {
// Calls are a `Path`, and usage of locals are a `Path`. So, this checks
// - call() as i32
// - local as i32
if let ExprKind::Path(qpath) = expr.kind {
let res = cx.qpath_res(&qpath, expr.hir_id);
// Function call
if let Res::Def(DefKind::Fn, def_id) = res {
let Some(snippet) = snippet_opt(cx, cx.tcx.def_span(def_id)) else {
return ControlFlow::Continue(());
};
// This is the worst part of this entire function. This is the only way I know of to
// check whether a function returns a type alias. Sure, you can get the return type
// from a function in the current crate as an hir ty, but how do you get it for
// external functions?? Simple: It's impossible. So, we check whether a part of the
// function's declaration snippet is exactly equal to the `Ty`. That way, we can
// see whether it's a type alias.
//
// Will this work for more complex types? Probably not!
if !snippet
.split("->")
.skip(0)
.map(|s| {
s.trim() == cast_from.to_string()
|| s.split("where").any(|ty| ty.trim() == cast_from.to_string())
})
.any(|a| a)
{
return ControlFlow::Break(());
}
// Local usage
} else if let Res::Local(hir_id) = res
&& let Some(parent) = get_parent_node(cx.tcx, hir_id)
&& let Node::Local(l) = parent
{
if let Some(e) = l.init && is_cast_from_ty_alias(cx, e, cast_from) {
return ControlFlow::Break::<()>(());
}

if let Some(ty) = l.ty
&& let TyKind::Path(qpath) = ty.kind
&& is_ty_alias(&qpath)
{
return ControlFlow::Break::<()>(());
}
}
}

ControlFlow::Continue(())
})
.is_some()
}
10 changes: 10 additions & 0 deletions tests/ui/auxiliary/extern_fake_libc.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
#![allow(nonstandard_style)]
#![allow(clippy::missing_safety_doc, unused)]

type pid_t = i32;
pub unsafe fn getpid() -> pid_t {
pid_t::from(0)
}
pub fn getpid_SAFE_TRUTH() -> pid_t {
unsafe { getpid() }
}
27 changes: 25 additions & 2 deletions tests/ui/unnecessary_cast.fixed
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
//@run-rustfix
//@aux-build:extern_fake_libc.rs
#![warn(clippy::unnecessary_cast)]
#![allow(
unused,
clippy::borrow_as_ptr,
clippy::no_effect,
clippy::nonstandard_macro_braces,
clippy::unnecessary_operation
clippy::unnecessary_operation,
nonstandard_style,
unused
)]

extern crate extern_fake_libc;

type PtrConstU8 = *const u8;
type PtrMutU8 = *mut u8;

Expand All @@ -19,6 +23,21 @@ fn uwu<T, U>(ptr: *const T) -> *const U {
ptr as *const U
}

mod fake_libc {
type pid_t = i32;
pub unsafe fn getpid() -> pid_t {
pid_t::from(0)
}
// Make sure a where clause does not break it
pub fn getpid_SAFE_TRUTH<T: Clone>(t: &T) -> pid_t
where
T: Clone,
{
t;
unsafe { getpid() }
}
}

#[rustfmt::skip]
fn main() {
// Test cast_unnecessary
Expand Down Expand Up @@ -82,6 +101,10 @@ fn main() {
// or from
let x: I32Alias = 1;
let y = x as u64;
fake_libc::getpid_SAFE_TRUTH(&0u32) as i32;
extern_fake_libc::getpid_SAFE_TRUTH() as i32;
let pid = unsafe { fake_libc::getpid() };
pid as i32;

let i8_ptr: *const i8 = &1;
let u8_ptr: *const u8 = &1;
Expand Down
27 changes: 25 additions & 2 deletions tests/ui/unnecessary_cast.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
//@run-rustfix
//@aux-build:extern_fake_libc.rs
#![warn(clippy::unnecessary_cast)]
#![allow(
unused,
clippy::borrow_as_ptr,
clippy::no_effect,
clippy::nonstandard_macro_braces,
clippy::unnecessary_operation
clippy::unnecessary_operation,
nonstandard_style,
unused
)]

extern crate extern_fake_libc;

type PtrConstU8 = *const u8;
type PtrMutU8 = *mut u8;

Expand All @@ -19,6 +23,21 @@ fn uwu<T, U>(ptr: *const T) -> *const U {
ptr as *const U
}

mod fake_libc {
type pid_t = i32;
pub unsafe fn getpid() -> pid_t {
pid_t::from(0)
}
// Make sure a where clause does not break it
pub fn getpid_SAFE_TRUTH<T: Clone>(t: &T) -> pid_t
where
T: Clone,
{
t;
unsafe { getpid() }
}
}

#[rustfmt::skip]
fn main() {
// Test cast_unnecessary
Expand Down Expand Up @@ -82,6 +101,10 @@ fn main() {
// or from
let x: I32Alias = 1;
let y = x as u64;
fake_libc::getpid_SAFE_TRUTH(&0u32) as i32;
extern_fake_libc::getpid_SAFE_TRUTH() as i32;
let pid = unsafe { fake_libc::getpid() };
pid as i32;

let i8_ptr: *const i8 = &1;
let u8_ptr: *const u8 = &1;
Expand Down
Loading