From f42272102a97e2f57e4d02c3a5a1defa3c800970 Mon Sep 17 00:00:00 2001 From: Philipp Hansch Date: Wed, 3 Oct 2018 12:02:06 +0200 Subject: [PATCH] Reimplement the `fn_to_numeric_cast` lint --- CHANGELOG.md | 1 + README.md | 2 +- clippy_lints/src/lib.rs | 2 ++ clippy_lints/src/types.rs | 45 +++++++++++++++++++++++++++++++ tests/ui/fn_to_numeric_cast.rs | 49 ++++++++++++++++++++++++++++++++++ 5 files changed, 98 insertions(+), 1 deletion(-) create mode 100644 tests/ui/fn_to_numeric_cast.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 013a508cc76..2373cb87eed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -688,6 +688,7 @@ All notable changes to this project will be documented in this file. [`float_arithmetic`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#float_arithmetic [`float_cmp`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#float_cmp [`float_cmp_const`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#float_cmp_const +[`fn_to_numeric_cast`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#fn_to_numeric_cast [`for_kv_map`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#for_kv_map [`for_loop_over_option`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#for_loop_over_option [`for_loop_over_result`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#for_loop_over_result diff --git a/README.md b/README.md index a2b61703a82..f332a3f645d 100644 --- a/README.md +++ b/README.md @@ -9,7 +9,7 @@ We are currently in the process of discussing Clippy 1.0 via the RFC process in A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code. -[There are 277 lints included in this crate!](https://rust-lang-nursery.github.io/rust-clippy/master/index.html) +[There are 278 lints included in this crate!](https://rust-lang-nursery.github.io/rust-clippy/master/index.html) We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you: diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 8b1b626be44..b181a4c17ca 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -697,6 +697,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { types::CAST_LOSSLESS, types::CAST_PTR_ALIGNMENT, types::CHAR_LIT_AS_U8, + types::FN_TO_NUMERIC_CAST, types::IMPLICIT_HASHER, types::LET_UNIT_VALUE, types::OPTION_OPTION, @@ -789,6 +790,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { returns::LET_AND_RETURN, returns::NEEDLESS_RETURN, strings::STRING_LIT_AS_BYTES, + types::FN_TO_NUMERIC_CAST, types::IMPLICIT_HASHER, types::LET_UNIT_VALUE, unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME, diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index b98a0f88242..5d26e477c82 100644 --- a/clippy_lints/src/types.rs +++ b/clippy_lints/src/types.rs @@ -719,6 +719,30 @@ fn is_unit_literal(expr: &Expr) -> bool { "cast from a pointer to a more-strictly-aligned pointer" } +/// **What it does:** Checks for casts of function pointers to something other than usize +/// +/// **Why is this bad?** +/// Depending on the system architechture, casting a function pointer to something other than +/// `usize` will result in incorrect pointer addresses. +/// `usize` will always be able to store the function pointer on the given architechture. +/// +/// **Example** +/// +/// ```rust +/// // Bad +/// fn fun() -> i32 {} +/// let a = fun as i64; +/// +/// // Good +/// fn fun2() -> i32 {} +/// let a = fun2 as usize; +/// ``` +declare_clippy_lint! { + pub FN_TO_NUMERIC_CAST, + style, + "casting a function pointer to a numeric type other than usize" +} + /// Returns the size in bits of an integral type. /// Will return 0 if the type is not an int or uint variant fn int_ty_to_nbits(typ: Ty<'_>, tcx: TyCtxt<'_, '_, '_>) -> u64 { @@ -913,6 +937,7 @@ fn get_lints(&self) -> LintArray { CAST_LOSSLESS, UNNECESSARY_CAST, CAST_PTR_ALIGNMENT, + FN_TO_NUMERIC_CAST ) } } @@ -921,6 +946,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for CastPass { fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) { if let ExprKind::Cast(ref ex, _) = expr.node { let (cast_from, cast_to) = (cx.tables.expr_ty(ex), cx.tables.expr_ty(expr)); + lint_fn_to_numeric_cast(cx, expr, ex, cast_from, cast_to); if let ExprKind::Lit(ref lit) = ex.node { use crate::syntax::ast::{LitIntType, LitKind}; match lit.node { @@ -1021,6 +1047,25 @@ fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) { } } +fn lint_fn_to_numeric_cast(cx: &LateContext<'_, '_>, expr: &Expr, cast_expr: &Expr, cast_from: Ty, cast_to: Ty) { + match cast_from.sty { + ty::FnDef(..) | ty::FnPtr(_) => { + let from_snippet = snippet(cx, cast_expr.span, "x"); + if cast_to.sty != ty::Uint(UintTy::Usize) { + span_lint_and_sugg( + cx, + FN_TO_NUMERIC_CAST, + expr.span, + &format!("casting function pointer `{}` to `{}`", from_snippet, cast_to), + "try", + format!("{} as usize", from_snippet) + ); + } + }, + _ => {} + } +} + /// **What it does:** Checks for types used in structs, parameters and `let` /// declarations above a certain complexity threshold. /// diff --git a/tests/ui/fn_to_numeric_cast.rs b/tests/ui/fn_to_numeric_cast.rs new file mode 100644 index 00000000000..d250af61847 --- /dev/null +++ b/tests/ui/fn_to_numeric_cast.rs @@ -0,0 +1,49 @@ +#![feature(tool_lints)] + +#[warn(clippy::fn_to_numeric_cast)] + +fn foo() -> String { String::new() } + +fn test_function_to_numeric_cast() { + let _ = foo as i8; + let _ = foo as i16; + let _ = foo as i32; + let _ = foo as i64; + let _ = foo as i128; + let _ = foo as isize; + + let _ = foo as u8; + let _ = foo as u16; + let _ = foo as u32; + let _ = foo as u64; + let _ = foo as u128; + + // Casting to usize is OK and should not warn + let _ = foo as usize; +} + +fn test_function_var_to_numeric_cast() { + let abc: fn() -> String = foo; + + let _ = abc as i8; + let _ = abc as i16; + let _ = abc as i32; + let _ = abc as i64; + let _ = abc as i128; + let _ = abc as isize; + + let _ = abc as u8; + let _ = abc as u16; + let _ = abc as u32; + let _ = abc as u64; + let _ = abc as u128; + + // Casting to usize is OK and should not warn + let _ = abc as usize; +} + +fn fn_with_fn_args(f: fn(i32) -> i32) -> i32 { + f as i32 +} + +fn main() {}