Auto merge of #7705 - michaelsproul:fn_to_numeric_cast_any, r=camsteffen

Restriction lint for function pointer casts

The existing lints for function pointer casts cover the cases where a cast is non-portable or would result in truncation, however there's currently no way to forbid numeric function pointer casts entirely.

I've added a new lint `fn_to_numeric_cast_any`, which allows one to ban _all_ numeric function pointer casts, including to `usize`. This is useful if you're writing high-level Rust and want to opt-out of potentially surprising behaviour, avoiding silent bugs from forgotten parentheses, e.g.

```rust
fn foo() -> u32 {
    10
}

fn main() {
    let _ = foo as usize; // oops, forgot to call foo and got a random address instead!
}
```

~~I'm open to suggestions for the naming of the lint, because `fn_to_numeric_cast_any` is a bit clunky. Ideally I'd call this lint `fn_to_numeric_cast`, but that name is already taken for the more specific lint~~. We've stuck with `fn_to_numeric_cast_any` to avoid renaming the existing lint, or choosing a different name that's too generic (like `fn_cast`).

I'm also open to changing the suggestion behaviour, as adding parentheses is only one of many possible ways to fix the lint.

changelog: add ``[`fn_to_numeric_cast_any`]`` restriction lint

CHANGELOG.md

@@ -2730,6 +2730,7 @@ Released 2018-09-13
 [`fn_address_comparisons`]: https://rust-lang.github.io/rust-clippy/master/index.html#fn_address_comparisons
 [`fn_params_excessive_bools`]: https://rust-lang.github.io/rust-clippy/master/index.html#fn_params_excessive_bools
 [`fn_to_numeric_cast`]: https://rust-lang.github.io/rust-clippy/master/index.html#fn_to_numeric_cast
+[`fn_to_numeric_cast_any`]: https://rust-lang.github.io/rust-clippy/master/index.html#fn_to_numeric_cast_any
 [`fn_to_numeric_cast_with_truncation`]: https://rust-lang.github.io/rust-clippy/master/index.html#fn_to_numeric_cast_with_truncation
 [`for_kv_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#for_kv_map
 [`for_loops_over_fallibles`]: https://rust-lang.github.io/rust-clippy/master/index.html#for_loops_over_fallibles

clippy_lints/src/casts/fn_to_numeric_cast_any.rs

@@ -0,0 +1,34 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::source::snippet_with_applicability;
+use rustc_errors::Applicability;
+use rustc_hir::Expr;
+use rustc_lint::LateContext;
+use rustc_middle::ty::{self, Ty};
+
+use super::FN_TO_NUMERIC_CAST_ANY;
+
+pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_expr: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>) {
+    // We allow casts from any function type to any function type.
+    match cast_to.kind() {
+        ty::FnDef(..) | ty::FnPtr(..) => return,
+        _ => { /* continue to checks */ },
+    }
+
+    match cast_from.kind() {
+        ty::FnDef(..) | ty::FnPtr(_) => {
+            let mut applicability = Applicability::MaybeIncorrect;
+            let from_snippet = snippet_with_applicability(cx, cast_expr.span, "..", &mut applicability);
+
+            span_lint_and_sugg(
+                cx,
+                FN_TO_NUMERIC_CAST_ANY,
+                expr.span,
+                &format!("casting function pointer `{}` to `{}`", from_snippet, cast_to),
+                "did you mean to invoke the function?",
+                format!("{}() as {}", from_snippet, cast_to),
+                applicability,
+            );
+        },
+        _ => {},
+    }
+}

clippy_lints/src/casts/mod.rs

@@ -7,6 +7,7 @@ mod cast_ref_to_mut;
 mod cast_sign_loss;
 mod char_lit_as_u8;
 mod fn_to_numeric_cast;
+mod fn_to_numeric_cast_any;
 mod fn_to_numeric_cast_with_truncation;
 mod ptr_as_ptr;
 mod unnecessary_cast;
@@ -251,6 +252,42 @@ declare_clippy_lint! {
     "casting a function pointer to a numeric type not wide enough to store the address"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for casts of a function pointer to any integer type.
+    ///
+    /// ### Why is this bad?
+    /// Casting a function pointer to an integer can have surprising results and can occur
+    /// accidentally if parantheses are omitted from a function call. If you aren't doing anything
+    /// low-level with function pointers then you can opt-out of casting functions to integers in
+    /// order to avoid mistakes. Alternatively, you can use this lint to audit all uses of function
+    /// pointer casts in your code.
+    ///
+    /// ### Example
+    /// ```rust
+    /// // Bad: fn1 is cast as `usize`
+    /// fn fn1() -> u16 {
+    ///     1
+    /// };
+    /// let _ = fn1 as usize;
+    ///
+    /// // Good: maybe you intended to call the function?
+    /// fn fn2() -> u16 {
+    ///     1
+    /// };
+    /// let _ = fn2() as usize;
+    ///
+    /// // Good: maybe you intended to cast it to a function type?
+    /// fn fn3() -> u16 {
+    ///     1
+    /// }
+    /// let _ = fn3 as fn() -> u16;
+    /// ```
+    pub FN_TO_NUMERIC_CAST_ANY,
+    restriction,
+    "casting a function pointer to any integer type"
+}
+
 declare_clippy_lint! {
     /// ### What it does
     /// Checks for casts of `&T` to `&mut T` anywhere in the code.
@@ -360,6 +397,7 @@ impl_lint_pass!(Casts => [
     CAST_REF_TO_MUT,
     CAST_PTR_ALIGNMENT,
     UNNECESSARY_CAST,
+    FN_TO_NUMERIC_CAST_ANY,
     FN_TO_NUMERIC_CAST,
     FN_TO_NUMERIC_CAST_WITH_TRUNCATION,
     CHAR_LIT_AS_U8,
@@ -385,6 +423,7 @@ impl<'tcx> LateLintPass<'tcx> for Casts {
                 return;
             }
 
+            fn_to_numeric_cast_any::check(cx, expr, cast_expr, cast_from, cast_to);
             fn_to_numeric_cast::check(cx, expr, cast_expr, cast_from, cast_to);
             fn_to_numeric_cast_with_truncation::check(cx, expr, cast_expr, cast_from, cast_to);
             if cast_from.is_numeric() && cast_to.is_numeric() && !in_external_macro(cx.sess(), expr.span) {

clippy_lints/src/lib.register_lints.rs

@@ -67,6 +67,7 @@ store.register_lints(&[
     casts::CAST_SIGN_LOSS,
     casts::CHAR_LIT_AS_U8,
     casts::FN_TO_NUMERIC_CAST,
+    casts::FN_TO_NUMERIC_CAST_ANY,
     casts::FN_TO_NUMERIC_CAST_WITH_TRUNCATION,
     casts::PTR_AS_PTR,
     casts::UNNECESSARY_CAST,

clippy_lints/src/lib.register_restriction.rs

@@ -8,6 +8,7 @@ store.register_group(true, "clippy::restriction", Some("clippy_restriction"), ve
     LintId::of(as_conversions::AS_CONVERSIONS),
     LintId::of(asm_syntax::INLINE_ASM_X86_ATT_SYNTAX),
     LintId::of(asm_syntax::INLINE_ASM_X86_INTEL_SYNTAX),
+    LintId::of(casts::FN_TO_NUMERIC_CAST_ANY),
     LintId::of(create_dir::CREATE_DIR),
     LintId::of(dbg_macro::DBG_MACRO),
     LintId::of(default_numeric_fallback::DEFAULT_NUMERIC_FALLBACK),

tests/ui/fn_to_numeric_cast_any.rs

@@ -0,0 +1,76 @@
+#![warn(clippy::fn_to_numeric_cast_any)]
+#![allow(clippy::fn_to_numeric_cast, clippy::fn_to_numeric_cast_with_truncation)]
+
+fn foo() -> u8 {
+    0
+}
+
+fn generic_foo<T>(x: T) -> T {
+    x
+}
+
+trait Trait {
+    fn static_method() -> u32 {
+        2
+    }
+}
+
+struct Struct;
+
+impl Trait for Struct {}
+
+fn fn_pointer_to_integer() {
+    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;
+    let _ = foo as usize;
+}
+
+fn static_method_to_integer() {
+    let _ = Struct::static_method as usize;
+}
+
+fn fn_with_fn_arg(f: fn(i32) -> u32) -> usize {
+    f as usize
+}
+
+fn fn_with_generic_static_trait_method<T: Trait>() -> usize {
+    T::static_method as usize
+}
+
+fn closure_to_fn_to_integer() {
+    let clos = |x| x * 2_u32;
+
+    let _ = (clos as fn(u32) -> u32) as usize;
+}
+
+fn fn_to_raw_ptr() {
+    let _ = foo as *const ();
+}
+
+fn cast_fn_to_self() {
+    // Casting to the same function pointer type should be permitted.
+    let _ = foo as fn() -> u8;
+}
+
+fn cast_generic_to_concrete() {
+    // Casting to a more concrete function pointer type should be permitted.
+    let _ = generic_foo as fn(usize) -> usize;
+}
+
+fn cast_closure_to_fn() {
+    // Casting a closure to a function pointer should be permitted.
+    let id = |x| x;
+    let _ = id as fn(usize) -> usize;
+}
+
+fn main() {}

tests/ui/fn_to_numeric_cast_any.stderr

@@ -0,0 +1,106 @@
+error: casting function pointer `foo` to `i8`
+  --> $DIR/fn_to_numeric_cast_any.rs:23:13
+   |
+LL |     let _ = foo as i8;
+   |             ^^^^^^^^^ help: did you mean to invoke the function?: `foo() as i8`
+   |
+   = note: `-D clippy::fn-to-numeric-cast-any` implied by `-D warnings`
+
+error: casting function pointer `foo` to `i16`
+  --> $DIR/fn_to_numeric_cast_any.rs:24:13
+   |
+LL |     let _ = foo as i16;
+   |             ^^^^^^^^^^ help: did you mean to invoke the function?: `foo() as i16`
+
+error: casting function pointer `foo` to `i32`
+  --> $DIR/fn_to_numeric_cast_any.rs:25:13
+   |
+LL |     let _ = foo as i32;
+   |             ^^^^^^^^^^ help: did you mean to invoke the function?: `foo() as i32`
+
+error: casting function pointer `foo` to `i64`
+  --> $DIR/fn_to_numeric_cast_any.rs:26:13
+   |
+LL |     let _ = foo as i64;
+   |             ^^^^^^^^^^ help: did you mean to invoke the function?: `foo() as i64`
+
+error: casting function pointer `foo` to `i128`
+  --> $DIR/fn_to_numeric_cast_any.rs:27:13
+   |
+LL |     let _ = foo as i128;
+   |             ^^^^^^^^^^^ help: did you mean to invoke the function?: `foo() as i128`
+
+error: casting function pointer `foo` to `isize`
+  --> $DIR/fn_to_numeric_cast_any.rs:28:13
+   |
+LL |     let _ = foo as isize;
+   |             ^^^^^^^^^^^^ help: did you mean to invoke the function?: `foo() as isize`
+
+error: casting function pointer `foo` to `u8`
+  --> $DIR/fn_to_numeric_cast_any.rs:30:13
+   |
+LL |     let _ = foo as u8;
+   |             ^^^^^^^^^ help: did you mean to invoke the function?: `foo() as u8`
+
+error: casting function pointer `foo` to `u16`
+  --> $DIR/fn_to_numeric_cast_any.rs:31:13
+   |
+LL |     let _ = foo as u16;
+   |             ^^^^^^^^^^ help: did you mean to invoke the function?: `foo() as u16`
+
+error: casting function pointer `foo` to `u32`
+  --> $DIR/fn_to_numeric_cast_any.rs:32:13
+   |
+LL |     let _ = foo as u32;
+   |             ^^^^^^^^^^ help: did you mean to invoke the function?: `foo() as u32`
+
+error: casting function pointer `foo` to `u64`
+  --> $DIR/fn_to_numeric_cast_any.rs:33:13
+   |
+LL |     let _ = foo as u64;
+   |             ^^^^^^^^^^ help: did you mean to invoke the function?: `foo() as u64`
+
+error: casting function pointer `foo` to `u128`
+  --> $DIR/fn_to_numeric_cast_any.rs:34:13
+   |
+LL |     let _ = foo as u128;
+   |             ^^^^^^^^^^^ help: did you mean to invoke the function?: `foo() as u128`
+
+error: casting function pointer `foo` to `usize`
+  --> $DIR/fn_to_numeric_cast_any.rs:35:13
+   |
+LL |     let _ = foo as usize;
+   |             ^^^^^^^^^^^^ help: did you mean to invoke the function?: `foo() as usize`
+
+error: casting function pointer `Struct::static_method` to `usize`
+  --> $DIR/fn_to_numeric_cast_any.rs:39:13
+   |
+LL |     let _ = Struct::static_method as usize;
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: did you mean to invoke the function?: `Struct::static_method() as usize`
+
+error: casting function pointer `f` to `usize`
+  --> $DIR/fn_to_numeric_cast_any.rs:43:5
+   |
+LL |     f as usize
+   |     ^^^^^^^^^^ help: did you mean to invoke the function?: `f() as usize`
+
+error: casting function pointer `T::static_method` to `usize`
+  --> $DIR/fn_to_numeric_cast_any.rs:47:5
+   |
+LL |     T::static_method as usize
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^ help: did you mean to invoke the function?: `T::static_method() as usize`
+
+error: casting function pointer `(clos as fn(u32) -> u32)` to `usize`
+  --> $DIR/fn_to_numeric_cast_any.rs:53:13
+   |
+LL |     let _ = (clos as fn(u32) -> u32) as usize;
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: did you mean to invoke the function?: `(clos as fn(u32) -> u32)() as usize`
+
+error: casting function pointer `foo` to `*const ()`
+  --> $DIR/fn_to_numeric_cast_any.rs:57:13
+   |
+LL |     let _ = foo as *const ();
+   |             ^^^^^^^^^^^^^^^^ help: did you mean to invoke the function?: `foo() as *const ()`
+
+error: aborting due to 17 previous errors
+