Skip to content

Commit

Permalink
upgrade equatable_if_let to style
Browse files Browse the repository at this point in the history
  • Loading branch information
HKalbasi committed Oct 30, 2021
1 parent 310ecb0 commit 298c8ed
Show file tree
Hide file tree
Showing 38 changed files with 875 additions and 179 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2843,6 +2843,7 @@ Released 2018-09-13
[`enum_variant_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#enum_variant_names
[`eq_op`]: https://rust-lang.github.io/rust-clippy/master/index.html#eq_op
[`equatable_if_let`]: https://rust-lang.github.io/rust-clippy/master/index.html#equatable_if_let
[`equatable_matches`]: https://rust-lang.github.io/rust-clippy/master/index.html#equatable_matches
[`erasing_op`]: https://rust-lang.github.io/rust-clippy/master/index.html#erasing_op
[`eval_order_dependence`]: https://rust-lang.github.io/rust-clippy/master/index.html#eval_order_dependence
[`excessive_precision`]: https://rust-lang.github.io/rust-clippy/master/index.html#excessive_precision
Expand Down
6 changes: 2 additions & 4 deletions clippy_lints/src/casts/cast_lossless.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,17 +65,15 @@ fn should_lint(cx: &LateContext<'_>, expr: &Expr<'_>, cast_from: Ty<'_>, cast_to

(true, false) => {
let from_nbits = utils::int_ty_to_nbits(cast_from, cx.tcx);
let to_nbits = if let ty::Float(FloatTy::F32) = cast_to.kind() {
let to_nbits = if cast_to.kind() == &ty::Float(FloatTy::F32) {
32
} else {
64
};
from_nbits < to_nbits
},

(_, _) => {
matches!(cast_from.kind(), ty::Float(FloatTy::F32)) && matches!(cast_to.kind(), ty::Float(FloatTy::F64))
},
(_, _) => cast_from.kind() == &ty::Float(FloatTy::F32) && cast_to.kind() == &ty::Float(FloatTy::F64),
}
}

Expand Down
4 changes: 1 addition & 3 deletions clippy_lints/src/casts/cast_possible_truncation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,7 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_expr: &Expr<'_>,
},

(_, _) => {
if matches!(cast_from.kind(), &ty::Float(FloatTy::F64))
&& matches!(cast_to.kind(), &ty::Float(FloatTy::F32))
{
if cast_from.kind() == &ty::Float(FloatTy::F64) && cast_to.kind() == &ty::Float(FloatTy::F32) {
"casting `f64` to `f32` may truncate the value".to_string()
} else {
return;
Expand Down
268 changes: 216 additions & 52 deletions clippy_lints/src/equatable_if_let.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,23 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::snippet_with_context;
use clippy_utils::source::{snippet_with_applicability, snippet_with_context};
use clippy_utils::ty::implements_trait;
use clippy_utils::{diagnostics::span_lint_and_sugg, higher::MatchesExpn};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::{Expr, ExprKind, Pat, PatKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty::Ty;
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_hir::{
def::{DefKind, Res},
Arm, Expr, ExprKind, Pat, PatKind, QPath,
};
use rustc_lint::{LateContext, LateLintPass, Lint};
use rustc_middle::ty::{Adt, Ty};
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::{Span, SyntaxContext};

use crate::utils::conf::EquatablePatternLevel;

declare_clippy_lint! {
/// ### What it does
/// Checks for pattern matchings that can be expressed using equality.
/// Checks for `if let <pat> = <expr>` (and `while let` and similars) that can be expressed
/// using `if <expr> == <pat>`.
///
/// ### Why is this bad?
///
Expand All @@ -33,68 +40,225 @@ declare_clippy_lint! {
/// }
/// ```
pub EQUATABLE_IF_LET,
nursery,
"using pattern matching instead of equality"
style,
"using if let instead of if with a equality condition"
}

declare_lint_pass!(PatternEquality => [EQUATABLE_IF_LET]);
declare_clippy_lint! {
/// ### What it does
/// Checks for `matches!(<expr>, <pat>)` that can be expressed
/// using `<expr> == <pat>`.
///
/// ### Why is this bad?
///
/// It is less concise and less clear.
///
/// ### Example
/// ```rust,ignore
/// let condition = matches!(x, Some(2));
/// ```
/// Should be written
/// ```rust,ignore
/// let condition = x == Some(2);
/// ```
pub EQUATABLE_MATCHES,
pedantic,
"using `matches!` instead of equality"
}

pub struct PatternEquality {
level: EquatablePatternLevel,
}

/// detects if pattern matches just one thing
fn unary_pattern(pat: &Pat<'_>) -> bool {
fn array_rec(pats: &[Pat<'_>]) -> bool {
pats.iter().all(unary_pattern)
impl PatternEquality {
pub fn new(level: EquatablePatternLevel) -> PatternEquality {
PatternEquality { level }
}
match &pat.kind {
PatKind::Slice(_, _, _) | PatKind::Range(_, _, _) | PatKind::Binding(..) | PatKind::Wild | PatKind::Or(_) => {
}

impl_lint_pass!(PatternEquality => [EQUATABLE_IF_LET, EQUATABLE_MATCHES]);

fn equatable_pattern(cx: &LateContext<'_>, pat: &Pat<'_>) -> bool {
fn array_rec(cx: &LateContext<'_>, pats: &[Pat<'_>]) -> bool {
pats.iter().all(|x| equatable_pattern(cx, x))
}
fn is_derived(cx: &LateContext<'_>, pat: &Pat<'_>) -> bool {
let ty = cx.typeck_results().pat_ty(pat);
if let Some(def_id) = cx.tcx.lang_items().structural_peq_trait() {
implements_trait(cx, ty, def_id, &[ty.into()])
} else {
false
}
}
match &pat.kind {
PatKind::Slice(a, None, []) => array_rec(cx, a),
PatKind::Struct(_, a, etc) => !etc && is_derived(cx, pat) && a.iter().all(|x| equatable_pattern(cx, x.pat)),
PatKind::Tuple(a, etc) => !etc.is_some() && array_rec(cx, a),
PatKind::TupleStruct(_, a, etc) => !etc.is_some() && is_derived(cx, pat) && array_rec(cx, a),
PatKind::Ref(x, _) | PatKind::Box(x) => equatable_pattern(cx, x),
PatKind::Path(QPath::Resolved(_, b)) => match b.res {
Res::Def(DefKind::Const, _) => true,
_ => is_derived(cx, pat),
},
PatKind::Struct(_, a, etc) => !etc && a.iter().all(|x| unary_pattern(x.pat)),
PatKind::Tuple(a, etc) | PatKind::TupleStruct(_, a, etc) => !etc.is_some() && array_rec(a),
PatKind::Ref(x, _) | PatKind::Box(x) => unary_pattern(x),
PatKind::Path(_) | PatKind::Lit(_) => true,
PatKind::Path(_) => is_derived(cx, pat),
PatKind::Lit(_) => true,
PatKind::Slice(..) | PatKind::Range(..) | PatKind::Binding(..) | PatKind::Wild | PatKind::Or(_) => false,
}
}

fn is_structural_partial_eq(cx: &LateContext<'tcx>, ty: Ty<'tcx>, other: Ty<'tcx>) -> bool {
fn is_partial_eq(cx: &LateContext<'tcx>, t1: Ty<'tcx>, t2: Ty<'tcx>) -> bool {
if let Some(def_id) = cx.tcx.lang_items().eq_trait() {
implements_trait(cx, ty, def_id, &[other.into()])
implements_trait(cx, t1, def_id, &[t2.into()])
} else {
false
}
}

fn pat_to_string(
cx: &LateContext<'tcx>,
app: &mut Applicability,
pat: &Pat<'_>,
goal: Ty<'_>,
ctxt: SyntaxContext,
) -> Option<String> {
fn inner(
cx: &LateContext<'tcx>,
app: &mut Applicability,
pat: &Pat<'_>,
goal: Ty<'_>,
r: &mut String,
ctxt: SyntaxContext,
) -> bool {
let ty = cx.typeck_results().pat_ty(pat);
if ty == goal {
match &pat.kind {
PatKind::TupleStruct(q, ..) | PatKind::Struct(q, ..) => {
let (adt_def, generic_args) = if let Adt(x, y) = ty.kind() {
(x, y)
} else {
return false; // shouldn't happen
};
let path = if let QPath::Resolved(.., p) = q {
p
} else {
return false; // give up
};
let var = adt_def.variant_of_res(path.res);
match &pat.kind {
PatKind::TupleStruct(_, params, _) => {
*r += &*snippet_with_applicability(cx, path.span, "..", app);
*r += "(";
for (i, (p, f)) in params.iter().zip(var.fields.iter()).enumerate() {
if i != 0 {
*r += ", ";
}
inner(cx, app, p, f.ty(cx.tcx, generic_args), r, ctxt);
}
*r += ")";
},
PatKind::Struct(_, fields, _) => {
*r += &*snippet_with_applicability(cx, path.span, "..", app);
*r += " { ";
for (i, p) in fields.iter().enumerate() {
if i != 0 {
*r += ", ";
}
*r += &*snippet_with_applicability(cx, p.ident.span, "..", app);
*r += ": ";
if let Some(x) = var.fields.iter().find(|f| f.ident == p.ident) {
inner(cx, app, p.pat, x.ty(cx.tcx, generic_args), r, ctxt);
} else {
return false; // won't happen
}
}
*r += " }";
},
_ => return false, // won't happen
}
},
_ => {
*r += &*snippet_with_context(cx, pat.span, ctxt, "..", app).0;
},
}
return true;
}
if goal.is_ref() {
if let Some(tam) = goal.builtin_deref(true) {
*r += "&";
return inner(cx, app, pat, tam.ty, r, ctxt);
}
}
false
}
let mut r = "".to_string();
if let PatKind::Struct(..) = pat.kind {
r += "(";
}
let success = inner(cx, app, pat, goal, &mut r, ctxt);
if let PatKind::Struct(..) = pat.kind {
r += ")";
}
if !success {
return None;
}
Some(r)
}

fn level_contains(level: EquatablePatternLevel, pat: &Pat<'_>) -> bool {
match level {
EquatablePatternLevel::Primitive => matches!(pat.kind, PatKind::Lit(_)),
EquatablePatternLevel::Simple => matches!(pat.kind, PatKind::Lit(_) | PatKind::Path(_)),
EquatablePatternLevel::All => true,
}
}

fn emit_lint(
cx: &LateContext<'tcx>,
pat: &Pat<'_>,
exp: &Expr<'_>,
ctxt: SyntaxContext,
span: Span,
lint: &'static Lint,
level: EquatablePatternLevel,
) {
if_chain! {
if equatable_pattern(cx, pat);
if level_contains(level, pat);
let exp_ty = cx.typeck_results().expr_ty(exp);
if is_partial_eq(cx, exp_ty, exp_ty);
let mut app = Applicability::MachineApplicable;
if let Some(pat_str) = pat_to_string(cx, &mut app, pat, exp_ty, ctxt);
then {
let exp_str = snippet_with_context(cx, exp.span, ctxt, "..", &mut app).0;
span_lint_and_sugg(
cx,
lint,
span,
"this pattern matching can be expressed using equality",
"try",
format!(
"{} == {}",
exp_str,
pat_str,
),
app,
);
}
}
}

impl<'tcx> LateLintPass<'tcx> for PatternEquality {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
if_chain! {
if let ExprKind::Let(pat, exp, _) = expr.kind;
if unary_pattern(pat);
let exp_ty = cx.typeck_results().expr_ty(exp);
let pat_ty = cx.typeck_results().pat_ty(pat);
if is_structural_partial_eq(cx, exp_ty, pat_ty);
then {

let mut applicability = Applicability::MachineApplicable;
let pat_str = match pat.kind {
PatKind::Struct(..) => format!(
"({})",
snippet_with_context(cx, pat.span, expr.span.ctxt(), "..", &mut applicability).0,
),
_ => snippet_with_context(cx, pat.span, expr.span.ctxt(), "..", &mut applicability).0.to_string(),
};
span_lint_and_sugg(
cx,
EQUATABLE_IF_LET,
expr.span,
"this pattern matching can be expressed using equality",
"try",
format!(
"{} == {}",
snippet_with_context(cx, exp.span, expr.span.ctxt(), "..", &mut applicability).0,
pat_str,
),
applicability,
);
}
if let ExprKind::Let(pat, exp, _) = expr.kind {
emit_lint(cx, pat, exp, expr.span.ctxt(), expr.span, EQUATABLE_IF_LET, self.level);
}
if let Some(MatchesExpn {
call_site,
arm: Arm { pat, guard: None, .. },
exp,
}) = MatchesExpn::parse(expr)
{
emit_lint(cx, pat, exp, expr.span.ctxt(), call_site, EQUATABLE_MATCHES, self.level);
}
}
}
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_all.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
LintId::of(enum_variants::MODULE_INCEPTION),
LintId::of(eq_op::EQ_OP),
LintId::of(eq_op::OP_REF),
LintId::of(equatable_if_let::EQUATABLE_IF_LET),
LintId::of(erasing_op::ERASING_OP),
LintId::of(escape::BOXED_LOCAL),
LintId::of(eta_reduction::REDUNDANT_CLOSURE),
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ store.register_lints(&[
eq_op::EQ_OP,
eq_op::OP_REF,
equatable_if_let::EQUATABLE_IF_LET,
equatable_if_let::EQUATABLE_MATCHES,
erasing_op::ERASING_OP,
escape::BOXED_LOCAL,
eta_reduction::REDUNDANT_CLOSURE,
Expand Down
1 change: 0 additions & 1 deletion clippy_lints/src/lib.register_nursery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ store.register_group(true, "clippy::nursery", Some("clippy_nursery"), vec![
LintId::of(copies::BRANCHES_SHARING_CODE),
LintId::of(disallowed_method::DISALLOWED_METHOD),
LintId::of(disallowed_type::DISALLOWED_TYPE),
LintId::of(equatable_if_let::EQUATABLE_IF_LET),
LintId::of(fallible_impl_from::FALLIBLE_IMPL_FROM),
LintId::of(floating_point_arithmetic::IMPRECISE_FLOPS),
LintId::of(floating_point_arithmetic::SUBOPTIMAL_FLOPS),
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_pedantic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ store.register_group(true, "clippy::pedantic", Some("clippy_pedantic"), vec![
LintId::of(doc::MISSING_PANICS_DOC),
LintId::of(empty_enum::EMPTY_ENUM),
LintId::of(enum_variants::MODULE_NAME_REPETITIONS),
LintId::of(equatable_if_let::EQUATABLE_MATCHES),
LintId::of(eta_reduction::REDUNDANT_CLOSURE_FOR_METHOD_CALLS),
LintId::of(excessive_bools::FN_PARAMS_EXCESSIVE_BOOLS),
LintId::of(excessive_bools::STRUCT_EXCESSIVE_BOOLS),
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_style.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ store.register_group(true, "clippy::style", Some("clippy_style"), vec![
LintId::of(enum_variants::ENUM_VARIANT_NAMES),
LintId::of(enum_variants::MODULE_INCEPTION),
LintId::of(eq_op::OP_REF),
LintId::of(equatable_if_let::EQUATABLE_IF_LET),
LintId::of(eta_reduction::REDUNDANT_CLOSURE),
LintId::of(float_literal::EXCESSIVE_PRECISION),
LintId::of(from_over_into::FROM_OVER_INTO),
Expand Down
3 changes: 2 additions & 1 deletion clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -722,7 +722,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|| Box::new(future_not_send::FutureNotSend));
store.register_late_pass(|| Box::new(if_let_mutex::IfLetMutex));
store.register_late_pass(|| Box::new(if_not_else::IfNotElse));
store.register_late_pass(|| Box::new(equatable_if_let::PatternEquality));
let equatable_pattern = conf.equatable_pattern;
store.register_late_pass(move || Box::new(equatable_if_let::PatternEquality::new(equatable_pattern)));
store.register_late_pass(|| Box::new(mut_mutex_lock::MutMutexLock));
store.register_late_pass(|| Box::new(match_on_vec_items::MatchOnVecItems));
store.register_late_pass(|| Box::new(manual_async_fn::ManualAsyncFn));
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/manual_async_fn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ fn captures_all_lifetimes(inputs: &[Ty<'_>], output_lifetimes: &[LifetimeName])
// - There's only one output lifetime bound using `+ '_`
// - All input lifetimes are explicitly bound to the output
input_lifetimes.is_empty()
|| (output_lifetimes.len() == 1 && matches!(output_lifetimes[0], LifetimeName::Underscore))
|| (output_lifetimes.len() == 1 && output_lifetimes[0] == LifetimeName::Underscore)
|| input_lifetimes
.iter()
.all(|in_lt| output_lifetimes.iter().any(|out_lt| in_lt == out_lt))
Expand Down
Loading

0 comments on commit 298c8ed

Please sign in to comment.