diff --git a/clippy_lints/src/minmax.rs b/clippy_lints/src/minmax.rs index dae39aaf5e21..1f798fd11209 100644 --- a/clippy_lints/src/minmax.rs +++ b/clippy_lints/src/minmax.rs @@ -18,6 +18,10 @@ declare_clippy_lint! { /// ```ignore /// min(0, max(100, x)) /// ``` + /// or + /// ```ignore + /// x.max(100).min(0) + /// ``` /// It will always be equal to `0`. Probably the author meant to clamp the value /// between 0 and 100, but has erroneously swapped `min` and `max`. pub MIN_MAX, @@ -60,25 +64,35 @@ enum MinMax { } fn min_max<'a>(cx: &LateContext<'_>, expr: &'a Expr<'a>) -> Option<(MinMax, Constant, &'a Expr<'a>)> { - if let ExprKind::Call(ref path, ref args) = expr.kind { - if let ExprKind::Path(ref qpath) = path.kind { - cx.typeck_results() - .qpath_res(qpath, path.hir_id) - .opt_def_id() - .and_then(|def_id| { - if match_def_path(cx, def_id, &paths::CMP_MIN) { - fetch_const(cx, args, MinMax::Min) - } else if match_def_path(cx, def_id, &paths::CMP_MAX) { - fetch_const(cx, args, MinMax::Max) - } else { - None - } - }) - } else { - None - } - } else { - None + match expr.kind { + ExprKind::Call(ref path, ref args) => { + if let ExprKind::Path(ref qpath) = path.kind { + cx.typeck_results() + .qpath_res(qpath, path.hir_id) + .opt_def_id() + .and_then(|def_id| { + if match_def_path(cx, def_id, &paths::CMP_MIN) { + fetch_const(cx, args, MinMax::Min) + } else if match_def_path(cx, def_id, &paths::CMP_MAX) { + fetch_const(cx, args, MinMax::Max) + } else { + None + } + }) + } else { + None + } + }, + ExprKind::MethodCall(ref path, _, ref args, _) => { + if path.ident.as_str() == sym!(max).as_str() { + fetch_const(cx, args, MinMax::Max) + } else if path.ident.as_str() == sym!(min).as_str() { + fetch_const(cx, args, MinMax::Min) + } else { + None + } + }, + _ => None, } } diff --git a/tests/ui/min_max.rs b/tests/ui/min_max.rs index 8307d4b3019f..90ec5676493a 100644 --- a/tests/ui/min_max.rs +++ b/tests/ui/min_max.rs @@ -30,4 +30,18 @@ fn main() { max(min(s, "Apple"), "Zoo"); max("Apple", min(s, "Zoo")); // ok + + x.min(1).max(3); + x.max(3).min(1); + + x.max(1).min(3); // ok + x.min(3).max(1); // ok + + max(x.min(1), 3); + min(x.max(1), 3); // ok + + s.max("Zoo").min("Apple"); + s.min("Apple").max("Zoo"); + + s.min("Zoo").max("Apple"); // ok } diff --git a/tests/ui/min_max.stderr b/tests/ui/min_max.stderr index b552c137f7c7..653946dc025f 100644 --- a/tests/ui/min_max.stderr +++ b/tests/ui/min_max.stderr @@ -42,5 +42,35 @@ error: this `min`/`max` combination leads to constant result LL | max(min(s, "Apple"), "Zoo"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 7 previous errors +error: this `min`/`max` combination leads to constant result + --> $DIR/min_max.rs:34:5 + | +LL | x.min(1).max(3); + | ^^^^^^^^^^^^^^^ + +error: this `min`/`max` combination leads to constant result + --> $DIR/min_max.rs:35:5 + | +LL | x.max(3).min(1); + | ^^^^^^^^^^^^^^^ + +error: this `min`/`max` combination leads to constant result + --> $DIR/min_max.rs:40:5 + | +LL | max(x.min(1), 3); + | ^^^^^^^^^^^^^^^^ + +error: this `min`/`max` combination leads to constant result + --> $DIR/min_max.rs:43:5 + | +LL | s.max("Zoo").min("Apple"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: this `min`/`max` combination leads to constant result + --> $DIR/min_max.rs:44:5 + | +LL | s.min("Apple").max("Zoo"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 12 previous errors