Skip to content

restrict manual_clamp to const case, bring it out of nursery #12543

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Mar 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 34 additions & 4 deletions clippy_lints/src/manual_clamp.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use clippy_config::msrvs::{self, Msrv};
use clippy_utils::consts::{constant, Constant};
use clippy_utils::diagnostics::{span_lint_and_then, span_lint_hir_and_then};
use clippy_utils::higher::If;
use clippy_utils::sugg::Sugg;
Expand All @@ -17,6 +18,7 @@ use rustc_middle::ty::Ty;
use rustc_session::impl_lint_pass;
use rustc_span::symbol::sym;
use rustc_span::Span;
use std::cmp::Ordering;
use std::ops::Deref;

declare_clippy_lint! {
Expand All @@ -26,6 +28,11 @@ declare_clippy_lint! {
/// ### Why is this bad?
/// clamp is much shorter, easier to read, and doesn't use any control flow.
///
/// ### Limitations
///
/// This lint will only trigger if max and min are known at compile time, and max is
/// greater than min.
///
/// ### Known issue(s)
/// If the clamped variable is NaN this suggestion will cause the code to propagate NaN
/// rather than returning either `max` or `min`.
Expand Down Expand Up @@ -80,7 +87,7 @@ declare_clippy_lint! {
/// ```
#[clippy::version = "1.66.0"]
pub MANUAL_CLAMP,
nursery,
complexity,
"using a clamp pattern instead of the clamp function"
}
impl_lint_pass!(ManualClamp => [MANUAL_CLAMP]);
Expand All @@ -103,6 +110,26 @@ struct ClampSuggestion<'tcx> {
hir_with_ignore_attr: Option<HirId>,
}

impl<'tcx> ClampSuggestion<'tcx> {
/// This function will return true if and only if you can demonstrate at compile time that min
/// is less than max.
fn min_less_than_max(&self, cx: &LateContext<'tcx>) -> bool {
let max_type = cx.typeck_results().expr_ty(self.params.max);
let min_type = cx.typeck_results().expr_ty(self.params.min);
if max_type != min_type {
return false;
}
if let Some(max) = constant(cx, cx.typeck_results(), self.params.max)
&& let Some(min) = constant(cx, cx.typeck_results(), self.params.min)
&& let Some(ord) = Constant::partial_cmp(cx.tcx, max_type, &min, &max)
{
ord != Ordering::Greater
} else {
false
}
}
}

#[derive(Debug)]
struct InputMinMax<'tcx> {
input: &'tcx Expr<'tcx>,
Expand All @@ -123,7 +150,7 @@ impl<'tcx> LateLintPass<'tcx> for ManualClamp {
.or_else(|| is_match_pattern(cx, expr))
.or_else(|| is_if_elseif_pattern(cx, expr));
if let Some(suggestion) = suggestion {
emit_suggestion(cx, &suggestion);
maybe_emit_suggestion(cx, &suggestion);
}
}
}
Expand All @@ -133,13 +160,16 @@ impl<'tcx> LateLintPass<'tcx> for ManualClamp {
return;
}
for suggestion in is_two_if_pattern(cx, block) {
emit_suggestion(cx, &suggestion);
maybe_emit_suggestion(cx, &suggestion);
}
}
extract_msrv_attr!(LateContext);
}

fn emit_suggestion<'tcx>(cx: &LateContext<'tcx>, suggestion: &ClampSuggestion<'tcx>) {
fn maybe_emit_suggestion<'tcx>(cx: &LateContext<'tcx>, suggestion: &ClampSuggestion<'tcx>) {
if !suggestion.min_less_than_max(cx) {
return;
}
let ClampSuggestion {
params: InputMinMax {
input,
Expand Down
Loading