change is_call_max_min pattern to avoid usage of def_path_res

Author: Xaeroxe

clippy_lints/src/manual_clamp.rs

@@ -1,6 +1,7 @@
 use itertools::Itertools;
 use rustc_hir::{
-    def::Res, Arm, BinOpKind, Block, BlockCheckMode, Expr, ExprKind, Guard, HirId, PatKind, QPath, StmtKind,
+    def::Res, Arm, BinOpKind, Block, BlockCheckMode, Expr, ExprKind, Guard, HirId, PatKind, PathSegment, PrimTy, QPath,
+    StmtKind,
 };
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_middle::ty::Ty;
@@ -10,8 +11,8 @@ use rustc_span::{symbol::sym, Span};
 use std::ops::Deref;
 
 use clippy_utils::{
-    consts::constant, def_path_res, diagnostics::span_lint_and_then, eq_expr_value, get_trait_def_id, higher::If,
-    is_trait_method, meets_msrv, msrvs, path_to_local_id, paths, peel_blocks_with_stmt, sugg::Sugg,
+    consts::constant, diagnostics::span_lint_and_then, eq_expr_value, get_trait_def_id, higher::If, is_diag_trait_item,
+    is_trait_method, meets_msrv, msrvs, path_res, path_to_local_id, paths, peel_blocks_with_stmt, sugg::Sugg,
     ty::implements_trait,
 };
 use rustc_errors::Applicability;
@@ -278,101 +279,97 @@ fn is_max_min_pattern<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> O
 /// # ;
 /// ```
 fn is_call_max_min_pattern<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> Option<ClampSuggestion<'tcx>> {
-    fn for_methods<'tcx>(
+    fn segment<'tcx>(cx: &LateContext<'_>, func: &Expr<'tcx>) -> Option<FunctionType<'tcx>> {
+        match func.kind {
+            ExprKind::Path(QPath::Resolved(None, path)) => {
+                let id = path.res.opt_def_id()?;
+                match cx.tcx.get_diagnostic_name(id) {
+                    Some(sym::cmp_min) => Some(FunctionType::CmpMin),
+                    Some(sym::cmp_max) => Some(FunctionType::CmpMax),
+                    _ if is_diag_trait_item(cx, id, sym::Ord) => {
+                        Some(FunctionType::OrdOrFloat(path.segments.last().expect("infallible")))
+                    },
+                    _ => None,
+                }
+            },
+            ExprKind::Path(QPath::TypeRelative(ty, seg)) => {
+                matches!(path_res(cx, ty), Res::PrimTy(PrimTy::Float(_))).then(|| FunctionType::OrdOrFloat(seg))
+            },
+            _ => None,
+        }
+    }
+
+    enum FunctionType<'tcx> {
+        CmpMin,
+        CmpMax,
+        OrdOrFloat(&'tcx PathSegment<'tcx>),
+    }
+
+    fn extract_call_data(e: ExprRef<'_>, is_first: bool) -> Option<(ExprRef<'_>, ExprRef<'_>, ExprRef<'_>, bool)> {
+        if let ExprKind::Call(inner_fn, &[ref third, ref fourth]) = &e.kind {
+            Some((inner_fn, third, fourth, is_first))
+        } else {
+            None
+        }
+    }
+
+    fn check<'tcx>(
         cx: &LateContext<'tcx>,
-        expr: &'tcx Expr<'tcx>,
-        max_is_first: bool,
-        first_res: Res,
-        second_res: Res,
-        is_float: bool,
+        outer_fn: ExprRef<'tcx>,
+        first: ExprRef<'tcx>,
+        second: ExprRef<'tcx>,
+        span: Span,
     ) -> Option<ClampSuggestion<'tcx>> {
-        let mut min = None;
-        let mut max = None;
-        let mut input = None;
-        let (mut_one, mut_two) = if max_is_first {
-            (&mut min, &mut max)
-        } else {
-            (&mut max, &mut min)
-        };
-        let mut process_consts = |arg_third, arg_fourth| {
-            let third_is_const = constant(cx, cx.typeck_results(), simplify_kind(arg_third)).is_some();
-            let fourth_is_const = constant(cx, cx.typeck_results(), simplify_kind(arg_fourth)).is_some();
+        if let Ok((inner_fn, third, fourth, is_first)) =
+            extract_call_data(first, true).into_iter().chain(extract_call_data(second, false)).exactly_one()
+        && let Some(inner_seg) = segment(cx, inner_fn)
+        && let Some(outer_seg) = segment(cx, outer_fn) {
+            let third_is_const = constant(cx, cx.typeck_results(), simplify_kind(third)).is_some();
+            let fourth_is_const = constant(cx, cx.typeck_results(), simplify_kind(fourth)).is_some();
+            let input;
+            let new_arg;
             match (third_is_const, fourth_is_const) {
-                (true, false) => (input, *mut_two) = (Some(arg_fourth), Some(arg_third)),
-                (false, true) => (input, *mut_two) = (Some(arg_third), Some(arg_fourth)),
-                _ => {},
+                (true, false) => (input, new_arg) = (fourth, third),
+                (false, true) => (input, new_arg) = (third, fourth),
+                _ => return None,
             }
-        };
-        if let ExprKind::Call(
-            Expr { kind: ExprKind::Path(path), hir_id, .. }, &[ref arg_first, ref arg_second]
-        ) = &expr.kind
-        && cx.qpath_res(path, *hir_id) == first_res {
-            if let ExprKind::Call(
-                Expr { kind: ExprKind::Path(path_first), hir_id, .. }, &[ref arg_third, ref arg_fourth]
-            ) = &simplify_kind(arg_first).kind
-            && cx.qpath_res(path_first, *hir_id) == second_res {
-                *mut_one = Some(arg_second);
-                process_consts(arg_third, arg_fourth);
-            } else if let ExprKind::Call(
-                Expr { kind: ExprKind::Path(path_second), hir_id, .. }, &[ref arg_third, ref arg_fourth]
-            ) = &simplify_kind(arg_second).kind
-            && cx.qpath_res(path_second, *hir_id) == second_res {
-                *mut_one = Some(arg_first);
-                process_consts(arg_third, arg_fourth);
+            let other_new_arg = if is_first {
+                second
+            } else {
+                first
+            };
+            let is_float = cx.typeck_results().expr_ty_adjusted(input).is_floating_point();
+            let min_max_suggestion = Some(ClampSuggestion {
+                params: InputMinMax { input, min: other_new_arg, max: new_arg, is_float },
+                span,
+                make_assignment: None,
+            });
+            let max_min_suggestion = Some(ClampSuggestion {
+                params: InputMinMax { input, min: new_arg, max: other_new_arg, is_float },
+                span,
+                make_assignment: None,
+            });
+            match (inner_seg, outer_seg) {
+                (FunctionType::CmpMin, FunctionType::CmpMax) => min_max_suggestion,
+                (FunctionType::CmpMax, FunctionType::CmpMin) => max_min_suggestion,
+                (FunctionType::OrdOrFloat(first_segment), FunctionType::OrdOrFloat(second_segment)) => {
+                    match (first_segment.ident.as_str(), second_segment.ident.as_str()) {
+                        ("min", "max") => min_max_suggestion,
+                        ("max", "min") => max_min_suggestion,
+                        _ => None,
+                    }
+                }
+                _ => None,
             }
+        } else {
+            None
         }
-        let input = input?;
-        let min = min?;
-        let max = max?;
-        Some(ClampSuggestion {
-            params: InputMinMax {
-                input,
-                min,
-                max,
-                is_float,
-            },
-            span: expr.span,
-            make_assignment: None,
-        })
     }
-    // Lots of expensive queries coming up which we don't want to repeat for every expr, so short
-    // circuit this a bit.
-    let is_dual_call = |e| {
-        matches!(
-            &simplify_kind(e).kind,
-            ExprKind::Call(
-                Expr {
-                    kind: ExprKind::Path(_),
-                    ..
-                },
-                &[_, _]
-            )
-        )
-    };
-    let type_clampability = if let ExprKind::Call(
-        Expr {
-            kind: ExprKind::Path(_),
-            ..
-        },
-        &[ref arg_first, ref arg_second],
-    ) = &expr.kind
-    && (is_dual_call(arg_first) ^ is_dual_call(arg_second)) {
-        TypeClampability::is_clampable(cx, cx.typeck_results().expr_ty(arg_first))
+
+    if let ExprKind::Call(outer_fn, [first, second]) = &expr.kind {
+        check(cx, outer_fn, first, second, expr.span).or_else(|| check(cx, outer_fn, second, first, expr.span))
     } else {
         None
-    }?;
-    let is_float = type_clampability.is_float();
-    let for_each_combo_of = |min, max| {
-        for_methods(cx, expr, true, max, min, is_float).or_else(|| for_methods(cx, expr, false, min, max, is_float))
-    };
-    if is_float {
-        let (f32_min_res, f32_max_res) = for_min_max(|m| def_path_res(cx, &["f32", m]));
-        let (f64_min_res, f64_max_res) = for_min_max(|m| def_path_res(cx, &["f64", m]));
-        for_each_combo_of(f32_min_res, f32_max_res).or_else(|| for_each_combo_of(f64_min_res, f64_max_res))
-    } else {
-        let (core_min_res, core_max_res) = for_min_max(|m| def_path_res(cx, &["core", "cmp", m]));
-        let (core_ord_min_res, core_ord_max_res) = for_min_max(|m| def_path_res(cx, &["core", "cmp", "Ord", m]));
-        for_each_combo_of(core_min_res, core_max_res).or_else(|| for_each_combo_of(core_ord_min_res, core_ord_max_res))
     }
 }
 
@@ -720,10 +717,6 @@ fn ord_op_same_direction(a: BinOpKind, b: BinOpKind) -> bool {
     }
 }
 
-fn for_min_max<T>(mut f: impl FnMut(&str) -> T) -> (T, T) {
-    (f("min"), f("max"))
-}
-
 /// Really similar to Cow, but doesn't have a `Clone` requirement.
 #[derive(Debug)]
 enum MaybeBorrowedStmtKind<'a> {