Add new confusing_method_to_numeric_cast lint (#13979)

Fixes rust-lang/rust-clippy#13973.

I don't think we can make `fn_to_numeric_cast_any` to be emitted in some
special cases. Its category cannot be changed at runtime.

I think in this case, the best might be a specialized new lint so we can
target exactly what we want.

----

changelog: Add new `confusing_method_to_numeric_cast` lint

Author: llogiq

CHANGELOG.md

@@ -5692,6 +5692,7 @@ Released 2018-09-13
 [`collection_is_never_read`]: https://rust-lang.github.io/rust-clippy/master/index.html#collection_is_never_read
 [`comparison_chain`]: https://rust-lang.github.io/rust-clippy/master/index.html#comparison_chain
 [`comparison_to_empty`]: https://rust-lang.github.io/rust-clippy/master/index.html#comparison_to_empty
+[`confusing_method_to_numeric_cast`]: https://rust-lang.github.io/rust-clippy/master/index.html#confusing_method_to_numeric_cast
 [`const_is_empty`]: https://rust-lang.github.io/rust-clippy/master/index.html#const_is_empty
 [`const_static_lifetime`]: https://rust-lang.github.io/rust-clippy/master/index.html#const_static_lifetime
 [`copy_iterator`]: https://rust-lang.github.io/rust-clippy/master/index.html#copy_iterator

clippy_lints/src/casts/confusing_method_to_numeric_cast.rs

@@ -0,0 +1,82 @@
+use clippy_utils::diagnostics::span_lint_and_then;
+use clippy_utils::source::snippet_with_applicability;
+use rustc_errors::Applicability;
+use rustc_hir::Expr;
+use rustc_lint::LateContext;
+use rustc_middle::ty::{self, GenericArg, Ty};
+use rustc_span::def_id::DefId;
+use rustc_span::{Symbol, sym};
+
+use super::CONFUSING_METHOD_TO_NUMERIC_CAST;
+
+fn get_primitive_ty_name(ty: Ty<'_>) -> Option<&'static str> {
+    match ty.kind() {
+        ty::Char => Some("char"),
+        ty::Int(int) => Some(int.name_str()),
+        ty::Uint(uint) => Some(uint.name_str()),
+        ty::Float(float) => Some(float.name_str()),
+        _ => None,
+    }
+}
+
+fn get_const_name_and_ty_name(
+    cx: &LateContext<'_>,
+    method_name: Symbol,
+    method_def_id: DefId,
+    generics: &[GenericArg<'_>],
+) -> Option<(&'static str, &'static str)> {
+    let method_name = method_name.as_str();
+    let diagnostic_name = cx.tcx.get_diagnostic_name(method_def_id);
+
+    let ty_name = if diagnostic_name.is_some_and(|diag| diag == sym::cmp_ord_min || diag == sym::cmp_ord_max) {
+        // We get the type on which the `min`/`max` method of the `Ord` trait is implemented.
+        if let [ty] = generics
+            && let Some(ty) = ty.as_type()
+        {
+            get_primitive_ty_name(ty)?
+        } else {
+            return None;
+        }
+    } else if let Some(impl_id) = cx.tcx.impl_of_method(method_def_id)
+        && let Some(ty_name) = get_primitive_ty_name(cx.tcx.type_of(impl_id).instantiate_identity())
+        && ["min", "max", "minimum", "maximum", "min_value", "max_value"].contains(&method_name)
+    {
+        ty_name
+    } else {
+        return None;
+    };
+
+    let const_name = if method_name.starts_with("max") { "MAX" } else { "MIN" };
+    Some((const_name, ty_name))
+}
+
+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 */ },
+    }
+
+    if let ty::FnDef(def_id, generics) = cast_from.kind()
+        && let Some(method_name) = cx.tcx.opt_item_name(*def_id)
+        && let Some((const_name, ty_name)) = get_const_name_and_ty_name(cx, method_name, *def_id, generics.as_slice())
+    {
+        let mut applicability = Applicability::MaybeIncorrect;
+        let from_snippet = snippet_with_applicability(cx, cast_expr.span, "..", &mut applicability);
+
+        span_lint_and_then(
+            cx,
+            CONFUSING_METHOD_TO_NUMERIC_CAST,
+            expr.span,
+            format!("casting function pointer `{from_snippet}` to `{cast_to}`"),
+            |diag| {
+                diag.span_suggestion_verbose(
+                    expr.span,
+                    "did you mean to use the associated constant?",
+                    format!("{ty_name}::{const_name} as {cast_to}"),
+                    applicability,
+                );
+            },
+        );
+    }
+}

clippy_lints/src/casts/mod.rs

@@ -14,6 +14,7 @@ mod cast_sign_loss;
 mod cast_slice_different_sizes;
 mod cast_slice_from_raw_parts;
 mod char_lit_as_u8;
+mod confusing_method_to_numeric_cast;
 mod fn_to_numeric_cast;
 mod fn_to_numeric_cast_any;
 mod fn_to_numeric_cast_with_truncation;
@@ -786,6 +787,32 @@ declare_clippy_lint! {
     "casting small constant literals to pointers to create dangling pointers"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for casts of a primitive method pointer like `max`/`min` to any integer type.
+    ///
+    /// ### Why restrict this?
+    /// Casting a function pointer to an integer can have surprising results and can occur
+    /// accidentally if parentheses 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
+    /// ```no_run
+    /// let _ = u16::max as usize;
+    /// ```
+    ///
+    /// Use instead:
+    /// ```no_run
+    /// let _ = u16::MAX as usize;
+    /// ```
+    #[clippy::version = "1.86.0"]
+    pub CONFUSING_METHOD_TO_NUMERIC_CAST,
+    suspicious,
+    "casting a primitive method pointer to any integer type"
+}
+
 pub struct Casts {
     msrv: Msrv,
 }
@@ -823,6 +850,7 @@ impl_lint_pass!(Casts => [
     REF_AS_PTR,
     AS_POINTER_UNDERSCORE,
     MANUAL_DANGLING_PTR,
+    CONFUSING_METHOD_TO_NUMERIC_CAST,
 ]);
 
 impl<'tcx> LateLintPass<'tcx> for Casts {
@@ -847,6 +875,7 @@ impl<'tcx> LateLintPass<'tcx> for Casts {
             ptr_cast_constness::check(cx, expr, cast_from_expr, cast_from, cast_to, self.msrv);
             as_ptr_cast_mut::check(cx, expr, cast_from_expr, cast_to);
             fn_to_numeric_cast_any::check(cx, expr, cast_from_expr, cast_from, cast_to);
+            confusing_method_to_numeric_cast::check(cx, expr, cast_from_expr, cast_from, cast_to);
             fn_to_numeric_cast::check(cx, expr, cast_from_expr, cast_from, cast_to);
             fn_to_numeric_cast_with_truncation::check(cx, expr, cast_from_expr, cast_from, cast_to);
             zero_ptr::check(cx, expr, cast_from_expr, cast_to_hir);

clippy_lints/src/declared_lints.rs

@@ -64,6 +64,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
     crate::casts::CAST_SLICE_DIFFERENT_SIZES_INFO,
     crate::casts::CAST_SLICE_FROM_RAW_PARTS_INFO,
     crate::casts::CHAR_LIT_AS_U8_INFO,
+    crate::casts::CONFUSING_METHOD_TO_NUMERIC_CAST_INFO,
     crate::casts::FN_TO_NUMERIC_CAST_INFO,
     crate::casts::FN_TO_NUMERIC_CAST_ANY_INFO,
     crate::casts::FN_TO_NUMERIC_CAST_WITH_TRUNCATION_INFO,

tests/ui/confusing_method_to_numeric_cast.fixed

@@ -0,0 +1,14 @@
+#![feature(float_minimum_maximum)]
+#![warn(clippy::confusing_method_to_numeric_cast)]
+
+fn main() {
+    let _ = u16::MAX as usize; //~ confusing_method_to_numeric_cast
+    let _ = u16::MIN as usize; //~ confusing_method_to_numeric_cast
+    let _ = u16::MAX as usize; //~ confusing_method_to_numeric_cast
+    let _ = u16::MIN as usize; //~ confusing_method_to_numeric_cast
+
+    let _ = f32::MAX as usize; //~ confusing_method_to_numeric_cast
+    let _ = f32::MAX as usize; //~ confusing_method_to_numeric_cast
+    let _ = f32::MIN as usize; //~ confusing_method_to_numeric_cast
+    let _ = f32::MIN as usize; //~ confusing_method_to_numeric_cast
+}

tests/ui/confusing_method_to_numeric_cast.rs

@@ -0,0 +1,14 @@
+#![feature(float_minimum_maximum)]
+#![warn(clippy::confusing_method_to_numeric_cast)]
+
+fn main() {
+    let _ = u16::max as usize; //~ confusing_method_to_numeric_cast
+    let _ = u16::min as usize; //~ confusing_method_to_numeric_cast
+    let _ = u16::max_value as usize; //~ confusing_method_to_numeric_cast
+    let _ = u16::min_value as usize; //~ confusing_method_to_numeric_cast
+
+    let _ = f32::maximum as usize; //~ confusing_method_to_numeric_cast
+    let _ = f32::max as usize; //~ confusing_method_to_numeric_cast
+    let _ = f32::minimum as usize; //~ confusing_method_to_numeric_cast
+    let _ = f32::min as usize; //~ confusing_method_to_numeric_cast
+}

tests/ui/confusing_method_to_numeric_cast.stderr

@@ -0,0 +1,100 @@
+error: casting function pointer `u16::max` to `usize`
+  --> tests/ui/confusing_method_to_numeric_cast.rs:5:13
+   |
+LL |     let _ = u16::max as usize;
+   |             ^^^^^^^^^^^^^^^^^
+   |
+   = note: `-D clippy::confusing-method-to-numeric-cast` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::confusing_method_to_numeric_cast)]`
+help: did you mean to use the associated constant?
+   |
+LL -     let _ = u16::max as usize;
+LL +     let _ = u16::MAX as usize;
+   |
+
+error: casting function pointer `u16::min` to `usize`
+  --> tests/ui/confusing_method_to_numeric_cast.rs:6:13
+   |
+LL |     let _ = u16::min as usize;
+   |             ^^^^^^^^^^^^^^^^^
+   |
+help: did you mean to use the associated constant?
+   |
+LL -     let _ = u16::min as usize;
+LL +     let _ = u16::MIN as usize;
+   |
+
+error: casting function pointer `u16::max_value` to `usize`
+  --> tests/ui/confusing_method_to_numeric_cast.rs:7:13
+   |
+LL |     let _ = u16::max_value as usize;
+   |             ^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: did you mean to use the associated constant?
+   |
+LL -     let _ = u16::max_value as usize;
+LL +     let _ = u16::MAX as usize;
+   |
+
+error: casting function pointer `u16::min_value` to `usize`
+  --> tests/ui/confusing_method_to_numeric_cast.rs:8:13
+   |
+LL |     let _ = u16::min_value as usize;
+   |             ^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: did you mean to use the associated constant?
+   |
+LL -     let _ = u16::min_value as usize;
+LL +     let _ = u16::MIN as usize;
+   |
+
+error: casting function pointer `f32::maximum` to `usize`
+  --> tests/ui/confusing_method_to_numeric_cast.rs:10:13
+   |
+LL |     let _ = f32::maximum as usize;
+   |             ^^^^^^^^^^^^^^^^^^^^^
+   |
+help: did you mean to use the associated constant?
+   |
+LL -     let _ = f32::maximum as usize;
+LL +     let _ = f32::MAX as usize;
+   |
+
+error: casting function pointer `f32::max` to `usize`
+  --> tests/ui/confusing_method_to_numeric_cast.rs:11:13
+   |
+LL |     let _ = f32::max as usize;
+   |             ^^^^^^^^^^^^^^^^^
+   |
+help: did you mean to use the associated constant?
+   |
+LL -     let _ = f32::max as usize;
+LL +     let _ = f32::MAX as usize;
+   |
+
+error: casting function pointer `f32::minimum` to `usize`
+  --> tests/ui/confusing_method_to_numeric_cast.rs:12:13
+   |
+LL |     let _ = f32::minimum as usize;
+   |             ^^^^^^^^^^^^^^^^^^^^^
+   |
+help: did you mean to use the associated constant?
+   |
+LL -     let _ = f32::minimum as usize;
+LL +     let _ = f32::MIN as usize;
+   |
+
+error: casting function pointer `f32::min` to `usize`
+  --> tests/ui/confusing_method_to_numeric_cast.rs:13:13
+   |
+LL |     let _ = f32::min as usize;
+   |             ^^^^^^^^^^^^^^^^^
+   |
+help: did you mean to use the associated constant?
+   |
+LL -     let _ = f32::min as usize;
+LL +     let _ = f32::MIN as usize;
+   |
+
+error: aborting due to 8 previous errors
+