Skip to content

Commit

Permalink
feat: unnecessary_min_max lint
Browse files Browse the repository at this point in the history
  • Loading branch information
vohoanglong0107 committed Mar 25, 2024
1 parent 95c62ff commit 5d840b6
Show file tree
Hide file tree
Showing 9 changed files with 475 additions and 87 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5799,6 +5799,7 @@ Released 2018-09-13
[`unnecessary_lazy_evaluations`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_lazy_evaluations
[`unnecessary_literal_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_literal_unwrap
[`unnecessary_map_on_constructor`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_map_on_constructor
[`unnecessary_min_max`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_min_max
[`unnecessary_mut_passed`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_mut_passed
[`unnecessary_operation`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_operation
[`unnecessary_owned_empty_strings`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_owned_empty_strings
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::methods::UNNECESSARY_JOIN_INFO,
crate::methods::UNNECESSARY_LAZY_EVALUATIONS_INFO,
crate::methods::UNNECESSARY_LITERAL_UNWRAP_INFO,
crate::methods::UNNECESSARY_MIN_MAX_INFO,
crate::methods::UNNECESSARY_RESULT_MAP_OR_ELSE_INFO,
crate::methods::UNNECESSARY_SORT_BY_INFO,
crate::methods::UNNECESSARY_TO_OWNED_INFO,
Expand Down
24 changes: 24 additions & 0 deletions clippy_lints/src/methods/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ mod unnecessary_iter_cloned;
mod unnecessary_join;
mod unnecessary_lazy_eval;
mod unnecessary_literal_unwrap;
mod unnecessary_min_max;
mod unnecessary_result_map_or_else;
mod unnecessary_sort_by;
mod unnecessary_to_owned;
Expand Down Expand Up @@ -3961,6 +3962,27 @@ declare_clippy_lint! {
"cloning an `Option` via `as_ref().cloned()`"
}

declare_clippy_lint! {
/// ### What it does
/// Checks for unnecessary calls to `min()` or `max()`
///
/// ### Why is this bad?
///
/// In these cases it is not necessary to call `min()`
/// ### Example
/// ```no_run
/// let _ = 0.min(7_u32);
/// ```
/// Use instead:
/// ```no_run
/// let _ = 0;
/// ```
#[clippy::version = "1.78.0"]
pub UNNECESSARY_MIN_MAX,
complexity,
"using 'min()/max()' when there is no need for it"
}

declare_clippy_lint! {
/// ### What it does
/// Checks for usage of `.map_or_else()` "map closure" for `Result` type.
Expand Down Expand Up @@ -4236,6 +4258,7 @@ impl_lint_pass!(Methods => [
UNNECESSARY_RESULT_MAP_OR_ELSE,
MANUAL_C_STR_LITERALS,
UNNECESSARY_GET_THEN_CHECK,
UNNECESSARY_MIN_MAX,
]);

/// Extracts a method call name, args, and `Span` of the method name.
Expand Down Expand Up @@ -4526,6 +4549,7 @@ impl Methods {
Some(("bytes", recv2, [], _, _)) => bytes_count_to_len::check(cx, expr, recv, recv2),
_ => {},
},
("min" | "max", [arg]) => unnecessary_min_max::check(cx, expr, name, recv, arg),
("drain", ..) => {
if let Node::Stmt(Stmt { hir_id: _, kind, .. }) = cx.tcx.parent_hir_node(expr.hir_id)
&& matches!(kind, StmtKind::Semi(_))
Expand Down
100 changes: 100 additions & 0 deletions clippy_lints/src/methods/unnecessary_min_max.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
use std::cmp::Ordering;

use super::UNNECESSARY_MIN_MAX;
use clippy_utils::diagnostics::span_lint_and_sugg;

use clippy_utils::consts::{constant, Constant};
use clippy_utils::source::snippet;
use clippy_utils::{clip, int_bits, unsext};
use hir::Expr;

use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_lint::LateContext;

use rustc_middle::ty;
use rustc_span::Span;

pub fn check<'tcx>(
cx: &LateContext<'tcx>,
expr: &'tcx Expr<'_>,
name: &str,
recv: &'tcx Expr<'_>,
arg: &'tcx Expr<'_>,
) {
let typeck_results = cx.typeck_results();
if let (Some(left), Some(right)) = (constant(cx, typeck_results, recv), constant(cx, typeck_results, arg)) {
let Some(ord) = Constant::partial_cmp(cx.tcx, typeck_results.expr_ty(recv), &left, &right) else {
return;
};

lint(cx, expr, name, recv.span, arg.span, ord);
} else if let Some(extrema) = detect_extrema(cx, recv) {
let ord = match extrema {
Extrema::Minimum => Ordering::Less,
Extrema::Maximum => Ordering::Greater,
};
lint(cx, expr, name, recv.span, arg.span, ord);
} else if let Some(extrema) = detect_extrema(cx, arg) {
let ord = match extrema {
Extrema::Minimum => Ordering::Greater,
Extrema::Maximum => Ordering::Less,
};
lint(cx, expr, name, recv.span, arg.span, ord);
}
}

fn lint(cx: &LateContext<'_>, expr: &Expr<'_>, name: &str, lhs: Span, rhs: Span, order: Ordering) {
let cmp_str = if (name == "min" && order.is_ge()) || (name == "max" && order.is_le()) {
"smaller"
} else {
"greater"
};

let suggested_value = if (name == "min" && order.is_ge()) || (name == "max" && order.is_le()) {
snippet(cx, rhs, "..")
} else {
snippet(cx, lhs, "..")
};

let msg = format!(
"`{}` is never {} than `{}` and has therefore no effect",
snippet(cx, lhs, ".."),
cmp_str,
snippet(cx, rhs, "..")
);
span_lint_and_sugg(
cx,
UNNECESSARY_MIN_MAX,
expr.span,
&msg,
"try",
suggested_value.to_string(),
Applicability::MachineApplicable,
);
}

#[derive(Debug)]
enum Extrema {
Minimum,
Maximum,
}
fn detect_extrema<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option<Extrema> {
let ty = cx.typeck_results().expr_ty(expr);

let cv = constant(cx, cx.typeck_results(), expr)?;

match (ty.kind(), cv) {
(&ty::Uint(_), Constant::Int(0)) => Some(Extrema::Minimum),
(&ty::Int(ity), Constant::Int(i)) if i == unsext(cx.tcx, i128::MIN >> (128 - int_bits(cx.tcx, ity)), ity) => {
Some(Extrema::Minimum)
},

(&ty::Int(ity), Constant::Int(i)) if i == unsext(cx.tcx, i128::MAX >> (128 - int_bits(cx.tcx, ity)), ity) => {
Some(Extrema::Maximum)
},
(&ty::Uint(uty), Constant::Int(i)) if i == clip(cx.tcx, u128::MAX, uty) => Some(Extrema::Maximum),

_ => None,
}
}
1 change: 1 addition & 0 deletions tests/ui/cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#![allow(
clippy::cast_abs_to_unsigned,
clippy::no_effect,
clippy::unnecessary_min_max,
clippy::unnecessary_operation,
clippy::unnecessary_literal_unwrap
)]
Expand Down
Loading

0 comments on commit 5d840b6

Please sign in to comment.