Skip to content

Commit 5d840b6

Browse files
feat: unnecessary_min_max lint
1 parent 95c62ff commit 5d840b6

9 files changed

+475
-87
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -5799,6 +5799,7 @@ Released 2018-09-13
57995799
[`unnecessary_lazy_evaluations`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_lazy_evaluations
58005800
[`unnecessary_literal_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_literal_unwrap
58015801
[`unnecessary_map_on_constructor`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_map_on_constructor
5802+
[`unnecessary_min_max`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_min_max
58025803
[`unnecessary_mut_passed`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_mut_passed
58035804
[`unnecessary_operation`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_operation
58045805
[`unnecessary_owned_empty_strings`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_owned_empty_strings

clippy_lints/src/declared_lints.rs

+1
Original file line numberDiff line numberDiff line change
@@ -466,6 +466,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
466466
crate::methods::UNNECESSARY_JOIN_INFO,
467467
crate::methods::UNNECESSARY_LAZY_EVALUATIONS_INFO,
468468
crate::methods::UNNECESSARY_LITERAL_UNWRAP_INFO,
469+
crate::methods::UNNECESSARY_MIN_MAX_INFO,
469470
crate::methods::UNNECESSARY_RESULT_MAP_OR_ELSE_INFO,
470471
crate::methods::UNNECESSARY_SORT_BY_INFO,
471472
crate::methods::UNNECESSARY_TO_OWNED_INFO,

clippy_lints/src/methods/mod.rs

+24
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ mod unnecessary_iter_cloned;
116116
mod unnecessary_join;
117117
mod unnecessary_lazy_eval;
118118
mod unnecessary_literal_unwrap;
119+
mod unnecessary_min_max;
119120
mod unnecessary_result_map_or_else;
120121
mod unnecessary_sort_by;
121122
mod unnecessary_to_owned;
@@ -3961,6 +3962,27 @@ declare_clippy_lint! {
39613962
"cloning an `Option` via `as_ref().cloned()`"
39623963
}
39633964

3965+
declare_clippy_lint! {
3966+
/// ### What it does
3967+
/// Checks for unnecessary calls to `min()` or `max()`
3968+
///
3969+
/// ### Why is this bad?
3970+
///
3971+
/// In these cases it is not necessary to call `min()`
3972+
/// ### Example
3973+
/// ```no_run
3974+
/// let _ = 0.min(7_u32);
3975+
/// ```
3976+
/// Use instead:
3977+
/// ```no_run
3978+
/// let _ = 0;
3979+
/// ```
3980+
#[clippy::version = "1.78.0"]
3981+
pub UNNECESSARY_MIN_MAX,
3982+
complexity,
3983+
"using 'min()/max()' when there is no need for it"
3984+
}
3985+
39643986
declare_clippy_lint! {
39653987
/// ### What it does
39663988
/// Checks for usage of `.map_or_else()` "map closure" for `Result` type.
@@ -4236,6 +4258,7 @@ impl_lint_pass!(Methods => [
42364258
UNNECESSARY_RESULT_MAP_OR_ELSE,
42374259
MANUAL_C_STR_LITERALS,
42384260
UNNECESSARY_GET_THEN_CHECK,
4261+
UNNECESSARY_MIN_MAX,
42394262
]);
42404263

42414264
/// Extracts a method call name, args, and `Span` of the method name.
@@ -4526,6 +4549,7 @@ impl Methods {
45264549
Some(("bytes", recv2, [], _, _)) => bytes_count_to_len::check(cx, expr, recv, recv2),
45274550
_ => {},
45284551
},
4552+
("min" | "max", [arg]) => unnecessary_min_max::check(cx, expr, name, recv, arg),
45294553
("drain", ..) => {
45304554
if let Node::Stmt(Stmt { hir_id: _, kind, .. }) = cx.tcx.parent_hir_node(expr.hir_id)
45314555
&& matches!(kind, StmtKind::Semi(_))
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
use std::cmp::Ordering;
2+
3+
use super::UNNECESSARY_MIN_MAX;
4+
use clippy_utils::diagnostics::span_lint_and_sugg;
5+
6+
use clippy_utils::consts::{constant, Constant};
7+
use clippy_utils::source::snippet;
8+
use clippy_utils::{clip, int_bits, unsext};
9+
use hir::Expr;
10+
11+
use rustc_errors::Applicability;
12+
use rustc_hir as hir;
13+
use rustc_lint::LateContext;
14+
15+
use rustc_middle::ty;
16+
use rustc_span::Span;
17+
18+
pub fn check<'tcx>(
19+
cx: &LateContext<'tcx>,
20+
expr: &'tcx Expr<'_>,
21+
name: &str,
22+
recv: &'tcx Expr<'_>,
23+
arg: &'tcx Expr<'_>,
24+
) {
25+
let typeck_results = cx.typeck_results();
26+
if let (Some(left), Some(right)) = (constant(cx, typeck_results, recv), constant(cx, typeck_results, arg)) {
27+
let Some(ord) = Constant::partial_cmp(cx.tcx, typeck_results.expr_ty(recv), &left, &right) else {
28+
return;
29+
};
30+
31+
lint(cx, expr, name, recv.span, arg.span, ord);
32+
} else if let Some(extrema) = detect_extrema(cx, recv) {
33+
let ord = match extrema {
34+
Extrema::Minimum => Ordering::Less,
35+
Extrema::Maximum => Ordering::Greater,
36+
};
37+
lint(cx, expr, name, recv.span, arg.span, ord);
38+
} else if let Some(extrema) = detect_extrema(cx, arg) {
39+
let ord = match extrema {
40+
Extrema::Minimum => Ordering::Greater,
41+
Extrema::Maximum => Ordering::Less,
42+
};
43+
lint(cx, expr, name, recv.span, arg.span, ord);
44+
}
45+
}
46+
47+
fn lint(cx: &LateContext<'_>, expr: &Expr<'_>, name: &str, lhs: Span, rhs: Span, order: Ordering) {
48+
let cmp_str = if (name == "min" && order.is_ge()) || (name == "max" && order.is_le()) {
49+
"smaller"
50+
} else {
51+
"greater"
52+
};
53+
54+
let suggested_value = if (name == "min" && order.is_ge()) || (name == "max" && order.is_le()) {
55+
snippet(cx, rhs, "..")
56+
} else {
57+
snippet(cx, lhs, "..")
58+
};
59+
60+
let msg = format!(
61+
"`{}` is never {} than `{}` and has therefore no effect",
62+
snippet(cx, lhs, ".."),
63+
cmp_str,
64+
snippet(cx, rhs, "..")
65+
);
66+
span_lint_and_sugg(
67+
cx,
68+
UNNECESSARY_MIN_MAX,
69+
expr.span,
70+
&msg,
71+
"try",
72+
suggested_value.to_string(),
73+
Applicability::MachineApplicable,
74+
);
75+
}
76+
77+
#[derive(Debug)]
78+
enum Extrema {
79+
Minimum,
80+
Maximum,
81+
}
82+
fn detect_extrema<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option<Extrema> {
83+
let ty = cx.typeck_results().expr_ty(expr);
84+
85+
let cv = constant(cx, cx.typeck_results(), expr)?;
86+
87+
match (ty.kind(), cv) {
88+
(&ty::Uint(_), Constant::Int(0)) => Some(Extrema::Minimum),
89+
(&ty::Int(ity), Constant::Int(i)) if i == unsext(cx.tcx, i128::MIN >> (128 - int_bits(cx.tcx, ity)), ity) => {
90+
Some(Extrema::Minimum)
91+
},
92+
93+
(&ty::Int(ity), Constant::Int(i)) if i == unsext(cx.tcx, i128::MAX >> (128 - int_bits(cx.tcx, ity)), ity) => {
94+
Some(Extrema::Maximum)
95+
},
96+
(&ty::Uint(uty), Constant::Int(i)) if i == clip(cx.tcx, u128::MAX, uty) => Some(Extrema::Maximum),
97+
98+
_ => None,
99+
}
100+
}

tests/ui/cast.rs

+1
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#![allow(
1313
clippy::cast_abs_to_unsigned,
1414
clippy::no_effect,
15+
clippy::unnecessary_min_max,
1516
clippy::unnecessary_operation,
1617
clippy::unnecessary_literal_unwrap
1718
)]

0 commit comments

Comments
 (0)