Skip to content
Open
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
68 changes: 37 additions & 31 deletions clippy_lints/src/equatable_if_let.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::source::snippet_with_context;
use clippy_utils::ty::implements_trait;
use rustc_errors::Applicability;
Expand Down Expand Up @@ -40,9 +40,9 @@ declare_clippy_lint! {
declare_lint_pass!(PatternEquality => [EQUATABLE_IF_LET]);

/// detects if pattern matches just one thing
fn unary_pattern(pat: &Pat<'_>) -> bool {
fn is_unary_pattern(pat: &Pat<'_>) -> bool {
fn array_rec(pats: &[Pat<'_>]) -> bool {
pats.iter().all(unary_pattern)
pats.iter().all(is_unary_pattern)
}
match &pat.kind {
PatKind::Missing => unreachable!(),
Expand All @@ -53,9 +53,9 @@ fn unary_pattern(pat: &Pat<'_>) -> bool {
| PatKind::Never
| PatKind::Or(_)
| PatKind::Err(_) => false,
PatKind::Struct(_, a, etc) => etc.is_none() && a.iter().all(|x| unary_pattern(x.pat)),
PatKind::Struct(_, a, etc) => etc.is_none() && a.iter().all(|x| is_unary_pattern(x.pat)),
PatKind::Tuple(a, etc) | PatKind::TupleStruct(_, a, etc) => etc.as_opt_usize().is_none() && array_rec(a),
PatKind::Ref(x, _) | PatKind::Box(x) | PatKind::Deref(x) | PatKind::Guard(x, _) => unary_pattern(x),
PatKind::Ref(x, _) | PatKind::Box(x) | PatKind::Deref(x) | PatKind::Guard(x, _) => is_unary_pattern(x),
PatKind::Expr(_) => true,
}
}
Expand Down Expand Up @@ -103,48 +103,54 @@ fn contains_type_mismatch(cx: &LateContext<'_>, pat: &Pat<'_>) -> bool {
impl<'tcx> LateLintPass<'tcx> for PatternEquality {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
if let ExprKind::Let(let_expr) = expr.kind
&& unary_pattern(let_expr.pat)
&& is_unary_pattern(let_expr.pat)
&& !expr.span.in_external_macro(cx.sess().source_map())
&& !let_expr.pat.span.from_expansion()
&& !let_expr.init.span.from_expansion()
{
let exp_ty = cx.typeck_results().expr_ty(let_expr.init);
let pat_ty = cx.typeck_results().pat_ty(let_expr.pat);
let mut applicability = Applicability::MachineApplicable;

let mut app = Applicability::MachineApplicable;
let ctxt = expr.span.ctxt();

if is_structural_partial_eq(cx, exp_ty, pat_ty) && !contains_type_mismatch(cx, let_expr.pat) {
let pat_str = match let_expr.pat.kind {
PatKind::Struct(..) => format!(
"({})",
snippet_with_context(cx, let_expr.pat.span, expr.span.ctxt(), "..", &mut applicability).0,
),
_ => snippet_with_context(cx, let_expr.pat.span, expr.span.ctxt(), "..", &mut applicability)
.0
.to_string(),
};
span_lint_and_sugg(
span_lint_and_then(
cx,
EQUATABLE_IF_LET,
expr.span,
"this pattern matching can be expressed using equality",
"try",
format!(
"{} == {pat_str}",
snippet_with_context(cx, let_expr.init.span, expr.span.ctxt(), "..", &mut applicability).0,
),
applicability,
|diag| {
let pat_str = {
let str = snippet_with_context(cx, let_expr.pat.span, ctxt, "..", &mut app).0;
if let PatKind::Struct(..) = let_expr.pat.kind {
format!("({str})").into()
} else {
str
}
};

let sugg = format!(
"{} == {pat_str}",
snippet_with_context(cx, let_expr.init.span, ctxt, "..", &mut app).0,
);
diag.span_suggestion(expr.span, "try", sugg, app);
},
);
} else {
span_lint_and_sugg(
span_lint_and_then(
cx,
EQUATABLE_IF_LET,
expr.span,
"this pattern matching can be expressed using `matches!`",
"try",
format!(
"matches!({}, {})",
snippet_with_context(cx, let_expr.init.span, expr.span.ctxt(), "..", &mut applicability).0,
snippet_with_context(cx, let_expr.pat.span, expr.span.ctxt(), "..", &mut applicability).0,
),
applicability,
|diag| {
let sugg = format!(
"matches!({}, {})",
snippet_with_context(cx, let_expr.init.span, ctxt, "..", &mut app).0,
snippet_with_context(cx, let_expr.pat.span, ctxt, "..", &mut app).0,
);
diag.span_suggestion(expr.span, "try", sugg, app);
},
);
}
}
Expand Down
86 changes: 69 additions & 17 deletions tests/ui/equatable_if_let.fixed
Original file line number Diff line number Diff line change
@@ -1,14 +1,6 @@
//@aux-build:proc_macros.rs

#![allow(
unused_variables,
dead_code,
clippy::derive_partial_eq_without_eq,
clippy::needless_ifs
)]
#![allow(clippy::derive_partial_eq_without_eq, clippy::needless_ifs)]
#![warn(clippy::equatable_if_let)]

extern crate proc_macros;
use proc_macros::{external, inline_macros};

use std::cmp::Ordering;
Expand Down Expand Up @@ -48,7 +40,6 @@ impl PartialEq for NotStructuralEq {
}
}

#[inline_macros]
fn main() {
let a = 2;
let b = 3;
Expand Down Expand Up @@ -95,13 +86,6 @@ fn main() {
//~^ equatable_if_let
if matches!(h, NoPartialEqStruct { a: 2, b: false }) {}
//~^ equatable_if_let

if "abc" == inline!("abc") {
//~^ equatable_if_let
println!("OK");
}

external!({ if let 2 = $a {} });
}

mod issue8710 {
Expand Down Expand Up @@ -139,3 +123,71 @@ mod issue8710 {
}
}
}

#[inline_macros]
fn issue14548() {
if let inline!("abc") = "abc" {
println!("OK");
}

let a = 2;
external!({ if let 2 = $a {} });

// Don't lint: `==`/`matches!` might be correct for a particular `$($font)|*`, but not in general
macro_rules! m1 {
($($font:pat_param)|*) => {
if let $($font)|* = "from_expansion" {}
}
}
m1!("foo");
m1!("Sans" | "Serif" | "Sans Mono");
m1!(inline!("foo"));

// Don't lint: the suggestion might be correct for a particular `$from_root_ctxt`, but not in
// general
macro_rules! m2 {
($from_root_ctxt:pat) => {
if let $from_root_ctxt = "from_expansion" {}
};
}
m2!("foo");
m2!("Sans" | "Serif" | "Sans Mono");
m2!(inline!("foo"));

// Don't lint: the suggestion might be correct for a particular `$from_root_ctxt`, but not in
// general
macro_rules! m3 {
($from_root_ctxt:expr) => {
if let "from_expansion" = $from_root_ctxt {}
};
}
m3!("foo");
m3!("foo");
m3!(inline!("foo"));

// Don't lint: the suggestion might be correct for a particular `$from_root_ctxt`, but not in
// general. Don't get confused by the scrutinee coming from macro invocation
macro_rules! m4 {
($from_root_ctxt:pat) => {
if let $from_root_ctxt = inline!("from_expansion") {}
};
}
m4!("foo");
m4!("Sans" | "Serif" | "Sans Mono");
m4!(inline!("foo"));

// Don't lint: the suggestion might be correct for a particular `$from_root_ctxt`, but not in
// general. Don't get confused by the scrutinee coming from macro invocation
macro_rules! m5 {
($from_root_ctxt:expr) => {
if let inline!("from_expansion") = $from_root_ctxt {}
};
}
m5!("foo");
m5!("foo");
m5!(inline!("foo"));

// (Would be nice to) lint: both sides are macro _invocations_, so the suggestion is correct in
// general
if let inline!("foo") = inline!("bar") {}
}
86 changes: 69 additions & 17 deletions tests/ui/equatable_if_let.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,6 @@
//@aux-build:proc_macros.rs

#![allow(
unused_variables,
dead_code,
clippy::derive_partial_eq_without_eq,
clippy::needless_ifs
)]
#![allow(clippy::derive_partial_eq_without_eq, clippy::needless_ifs)]
#![warn(clippy::equatable_if_let)]

extern crate proc_macros;
use proc_macros::{external, inline_macros};

use std::cmp::Ordering;
Expand Down Expand Up @@ -48,7 +40,6 @@ impl PartialEq for NotStructuralEq {
}
}

#[inline_macros]
fn main() {
let a = 2;
let b = 3;
Expand Down Expand Up @@ -95,13 +86,6 @@ fn main() {
//~^ equatable_if_let
if let NoPartialEqStruct { a: 2, b: false } = h {}
//~^ equatable_if_let

if let inline!("abc") = "abc" {
//~^ equatable_if_let
println!("OK");
}

external!({ if let 2 = $a {} });
}

mod issue8710 {
Expand Down Expand Up @@ -139,3 +123,71 @@ mod issue8710 {
}
}
}

#[inline_macros]
fn issue14548() {
if let inline!("abc") = "abc" {
println!("OK");
}

let a = 2;
external!({ if let 2 = $a {} });

// Don't lint: `==`/`matches!` might be correct for a particular `$($font)|*`, but not in general
macro_rules! m1 {
($($font:pat_param)|*) => {
if let $($font)|* = "from_expansion" {}
}
}
m1!("foo");
m1!("Sans" | "Serif" | "Sans Mono");
m1!(inline!("foo"));

// Don't lint: the suggestion might be correct for a particular `$from_root_ctxt`, but not in
// general
macro_rules! m2 {
($from_root_ctxt:pat) => {
if let $from_root_ctxt = "from_expansion" {}
};
}
m2!("foo");
m2!("Sans" | "Serif" | "Sans Mono");
m2!(inline!("foo"));

// Don't lint: the suggestion might be correct for a particular `$from_root_ctxt`, but not in
// general
macro_rules! m3 {
($from_root_ctxt:expr) => {
if let "from_expansion" = $from_root_ctxt {}
};
}
m3!("foo");
m3!("foo");
m3!(inline!("foo"));

// Don't lint: the suggestion might be correct for a particular `$from_root_ctxt`, but not in
// general. Don't get confused by the scrutinee coming from macro invocation
macro_rules! m4 {
($from_root_ctxt:pat) => {
if let $from_root_ctxt = inline!("from_expansion") {}
};
}
m4!("foo");
m4!("Sans" | "Serif" | "Sans Mono");
m4!(inline!("foo"));

// Don't lint: the suggestion might be correct for a particular `$from_root_ctxt`, but not in
// general. Don't get confused by the scrutinee coming from macro invocation
macro_rules! m5 {
($from_root_ctxt:expr) => {
if let inline!("from_expansion") = $from_root_ctxt {}
};
}
m5!("foo");
m5!("foo");
m5!(inline!("foo"));

// (Would be nice to) lint: both sides are macro _invocations_, so the suggestion is correct in
// general
if let inline!("foo") = inline!("bar") {}
}
Loading