use crate::consts::{constant, miri_to_const, Constant};
use crate::utils::sugg::Sugg;
-use crate::utils::usage::is_unused;
+use crate::utils::visitors::LocalUsedVisitor;
use crate::utils::{
- expr_block, get_arg_name, get_parent_expr, in_macro, indent_of, is_allowed, is_expn_of, is_refutable,
- is_type_diagnostic_item, is_wild, match_qpath, match_type, match_var, meets_msrv, multispan_sugg, remove_blocks,
- snippet, snippet_block, snippet_with_applicability, span_lint_and_help, span_lint_and_note, span_lint_and_sugg,
- span_lint_and_then,
+ expr_block, get_parent_expr, implements_trait, in_macro, indent_of, is_allowed, is_expn_of, is_refutable,
+ is_type_diagnostic_item, is_wild, match_qpath, match_type, meets_msrv, multispan_sugg, path_to_local_id,
+ peel_hir_pat_refs, peel_mid_ty_refs, peel_n_hir_expr_refs, remove_blocks, snippet, snippet_block, snippet_opt,
+ snippet_with_applicability, span_lint_and_help, span_lint_and_note, span_lint_and_sugg, span_lint_and_then,
+ strip_pat_refs,
};
use crate::utils::{paths, search_same, SpanlessEq, SpanlessHash};
use if_chain::if_chain;
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::lint::in_external_macro;
use rustc_middle::ty::{self, Ty, TyS};
+use rustc_semver::RustcVersion;
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::source_map::{Span, Spanned};
use rustc_span::{sym, Symbol};
-use semver::{Version, VersionReq};
use std::cmp::Ordering;
use std::collections::hash_map::Entry;
use std::collections::Bound;
}
declare_clippy_lint! {
- /// **What it does:** Lint for redundant pattern matching over `Result`, `Option` or
- /// `std::task::Poll`
+ /// **What it does:** Lint for redundant pattern matching over `Result`, `Option`,
+ /// `std::task::Poll` or `std::net::IpAddr`
///
/// **Why is this bad?** It's more concise and clear to just use the proper
/// utility function
///
/// ```rust
/// # use std::task::Poll;
+ /// # use std::net::{IpAddr, Ipv4Addr, Ipv6Addr};
/// if let Ok(_) = Ok::<i32, i32>(42) {}
/// if let Err(_) = Err::<i32, i32>(42) {}
/// if let None = None::<()> {}
/// if let Some(_) = Some(42) {}
/// if let Poll::Pending = Poll::Pending::<()> {}
/// if let Poll::Ready(_) = Poll::Ready(42) {}
+ /// if let IpAddr::V4(_) = IpAddr::V4(Ipv4Addr::LOCALHOST) {}
+ /// if let IpAddr::V6(_) = IpAddr::V6(Ipv6Addr::LOCALHOST) {}
/// match Ok::<i32, i32>(42) {
/// Ok(_) => true,
/// Err(_) => false,
///
/// ```rust
/// # use std::task::Poll;
+ /// # use std::net::{IpAddr, Ipv4Addr, Ipv6Addr};
/// if Ok::<i32, i32>(42).is_ok() {}
/// if Err::<i32, i32>(42).is_err() {}
/// if None::<()>.is_none() {}
/// if Some(42).is_some() {}
/// if Poll::Pending::<()>.is_pending() {}
/// if Poll::Ready(42).is_ready() {}
+ /// if IpAddr::V4(Ipv4Addr::LOCALHOST).is_ipv4() {}
+ /// if IpAddr::V6(Ipv6Addr::LOCALHOST).is_ipv6() {}
/// Ok::<i32, i32>(42).is_ok();
/// ```
pub REDUNDANT_PATTERN_MATCHING,
/// **Why is this bad?** Readability and needless complexity.
///
/// **Known problems:** This lint falsely triggers, if there are arms with
- /// `cfg` attributes that remove an arm evaluating to `false`.
+ /// `cfg` attributes that remove an arm evaluating to `false`.
///
/// **Example:**
/// ```rust
#[derive(Default)]
pub struct Matches {
- msrv: Option<VersionReq>,
+ msrv: Option<RustcVersion>,
infallible_destructuring_match_linted: bool,
}
impl Matches {
#[must_use]
- pub fn new(msrv: Option<VersionReq>) -> Self {
+ pub fn new(msrv: Option<RustcVersion>) -> Self {
Self {
msrv,
..Matches::default()
MATCH_SAME_ARMS,
]);
-const MATCH_LIKE_MATCHES_MACRO_MSRV: Version = Version {
- major: 1,
- minor: 42,
- patch: 0,
- pre: Vec::new(),
- build: Vec::new(),
-};
+const MATCH_LIKE_MATCHES_MACRO_MSRV: RustcVersion = RustcVersion::new(1, 42, 0);
impl<'tcx> LateLintPass<'tcx> for Matches {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if let PatKind::TupleStruct(
QPath::Resolved(None, ref variant_name), ref args, _) = arms[0].pat.kind;
if args.len() == 1;
- if let Some(arg) = get_arg_name(&args[0]);
+ if let PatKind::Binding(_, arg, ..) = strip_pat_refs(&args[0]).kind;
let body = remove_blocks(&arms[0].body);
- if match_var(body, arg);
+ if path_to_local_id(body, arg);
then {
let mut applicability = Applicability::MachineApplicable;
if_chain! {
if !in_external_macro(cx.sess(), pat.span);
if !in_macro(pat.span);
- if let PatKind::Struct(ref qpath, fields, true) = pat.kind;
- if let QPath::Resolved(_, ref path) = qpath;
+ if let PatKind::Struct(QPath::Resolved(_, ref path), fields, true) = pat.kind;
if let Some(def_id) = path.res.opt_def_id();
let ty = cx.tcx.type_of(def_id);
if let ty::Adt(def, _) = ty.kind();
if stmts.len() == 1 && block_expr.is_none() || stmts.is_empty() && block_expr.is_some() {
// single statement/expr "else" block, don't lint
return;
- } else {
- // block with 2+ statements or 1 expr and 1+ statement
- Some(els)
}
+ // block with 2+ statements or 1 expr and 1+ statement
+ Some(els)
} else {
// not a block, don't lint
return;
let els_str = els.map_or(String::new(), |els| {
format!(" else {}", expr_block(cx, els, None, "..", Some(expr.span)))
});
+
+ let (msg, sugg) = if_chain! {
+ let (pat, pat_ref_count) = peel_hir_pat_refs(arms[0].pat);
+ if let PatKind::Path(_) | PatKind::Lit(_) = pat.kind;
+ let (ty, ty_ref_count) = peel_mid_ty_refs(cx.typeck_results().expr_ty(ex));
+ if let Some(trait_id) = cx.tcx.lang_items().structural_peq_trait();
+ if ty.is_integral() || ty.is_char() || ty.is_str() || implements_trait(cx, ty, trait_id, &[]);
+ then {
+ // scrutinee derives PartialEq and the pattern is a constant.
+ let pat_ref_count = match pat.kind {
+ // string literals are already a reference.
+ PatKind::Lit(Expr { kind: ExprKind::Lit(lit), .. }) if lit.node.is_str() => pat_ref_count + 1,
+ _ => pat_ref_count,
+ };
+ // References are only implicitly added to the pattern, so no overflow here.
+ // e.g. will work: match &Some(_) { Some(_) => () }
+ // will not: match Some(_) { &Some(_) => () }
+ let ref_count_diff = ty_ref_count - pat_ref_count;
+
+ // Try to remove address of expressions first.
+ let (ex, removed) = peel_n_hir_expr_refs(ex, ref_count_diff);
+ let ref_count_diff = ref_count_diff - removed;
+
+ let msg = "you seem to be trying to use `match` for an equality check. Consider using `if`";
+ let sugg = format!(
+ "if {} == {}{} {}{}",
+ snippet(cx, ex.span, ".."),
+ // PartialEq for different reference counts may not exist.
+ "&".repeat(ref_count_diff),
+ snippet(cx, arms[0].pat.span, ".."),
+ expr_block(cx, &arms[0].body, None, "..", Some(expr.span)),
+ els_str,
+ );
+ (msg, sugg)
+ } else {
+ let msg = "you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let`";
+ let sugg = format!(
+ "if let {} = {} {}{}",
+ snippet(cx, arms[0].pat.span, ".."),
+ snippet(cx, ex.span, ".."),
+ expr_block(cx, &arms[0].body, None, "..", Some(expr.span)),
+ els_str,
+ );
+ (msg, sugg)
+ }
+ };
+
span_lint_and_sugg(
cx,
lint,
expr.span,
- "you seem to be trying to use match for destructuring a single pattern. Consider using `if \
- let`",
+ msg,
"try this",
- format!(
- "if let {} = {} {}{}",
- snippet(cx, arms[0].pat.span, ".."),
- snippet(cx, ex.span, ".."),
- expr_block(cx, &arms[0].body, None, "..", Some(expr.span)),
- els_str,
- ),
+ sugg,
Applicability::HasPlaceholders,
);
}
}
}
-fn check_wild_err_arm(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>]) {
+fn check_wild_err_arm<'tcx>(cx: &LateContext<'tcx>, ex: &Expr<'tcx>, arms: &[Arm<'tcx>]) {
let ex_ty = cx.typeck_results().expr_ty(ex).peel_refs();
if is_type_diagnostic_item(cx, ex_ty, sym::result_type) {
for arm in arms {
if !matching_wild {
// Looking for unused bindings (i.e.: `_e`)
inner.iter().for_each(|pat| {
- if let PatKind::Binding(.., ident, None) = &pat.kind {
- if ident.as_str().starts_with('_') && is_unused(ident, arm.body) {
+ if let PatKind::Binding(_, id, ident, None) = pat.kind {
+ if ident.as_str().starts_with('_')
+ && !LocalUsedVisitor::new(cx, id).check_expr(arm.body)
+ {
ident_bind_name = (&ident.name.as_str()).to_string();
matching_wild = true;
}
if let QPath::Resolved(_, p) = path {
missing_variants.retain(|e| e.ctor_def_id != Some(p.res.def_id()));
}
- } else if let PatKind::TupleStruct(ref path, ref patterns, ..) = arm.pat.kind {
- if let QPath::Resolved(_, p) = path {
- // Some simple checks for exhaustive patterns.
- // There is a room for improvements to detect more cases,
- // but it can be more expensive to do so.
- let is_pattern_exhaustive =
- |pat: &&Pat<'_>| matches!(pat.kind, PatKind::Wild | PatKind::Binding(.., None));
- if patterns.iter().all(is_pattern_exhaustive) {
- missing_variants.retain(|e| e.ctor_def_id != Some(p.res.def_id()));
- }
+ } else if let PatKind::TupleStruct(QPath::Resolved(_, p), ref patterns, ..) = arm.pat.kind {
+ // Some simple checks for exhaustive patterns.
+ // There is a room for improvements to detect more cases,
+ // but it can be more expensive to do so.
+ let is_pattern_exhaustive =
+ |pat: &&Pat<'_>| matches!(pat.kind, PatKind::Wild | PatKind::Binding(.., None));
+ if patterns.iter().all(is_pattern_exhaustive) {
+ missing_variants.retain(|e| e.ctor_def_id != Some(p.res.def_id()));
}
}
}
} else {
pat
};
+
+ // strip potential borrows (#6503), but only if the type is a reference
+ let mut ex_new = ex;
+ if let ExprKind::AddrOf(BorrowKind::Ref, .., ex_inner) = ex.kind {
+ if let ty::Ref(..) = cx.typeck_results().expr_ty(&ex_inner).kind() {
+ ex_new = ex_inner;
+ }
+ };
span_lint_and_sugg(
cx,
MATCH_LIKE_MATCHES_MACRO,
format!(
"{}matches!({}, {})",
if b0 { "" } else { "!" },
- snippet_with_applicability(cx, ex.span, "..", &mut applicability),
+ snippet_with_applicability(cx, ex_new.span, "..", &mut applicability),
pat_and_guard,
),
applicability,
if in_macro(expr.span) || arms.len() != 1 || is_refutable(cx, arms[0].pat) {
return;
}
+
+ // HACK:
+ // This is a hack to deal with arms that are excluded by macros like `#[cfg]`. It is only used here
+ // to prevent false positives as there is currently no better way to detect if code was excluded by
+ // a macro. See PR #6435
+ if_chain! {
+ if let Some(match_snippet) = snippet_opt(cx, expr.span);
+ if let Some(arm_snippet) = snippet_opt(cx, arms[0].span);
+ if let Some(ex_snippet) = snippet_opt(cx, ex.span);
+ let rest_snippet = match_snippet.replace(&arm_snippet, "").replace(&ex_snippet, "");
+ if rest_snippet.contains("=>");
+ then {
+ // The code it self contains another thick arrow "=>"
+ // -> Either another arm or a comment
+ return;
+ }
+ }
+
let matched_vars = ex.span;
let bind_names = arms[0].pat.span;
let match_body = remove_blocks(&arms[0].body);
if let ExprKind::Call(ref e, ref args) = remove_blocks(&arm.body).kind;
if let ExprKind::Path(ref some_path) = e.kind;
if match_qpath(some_path, &paths::OPTION_SOME) && args.len() == 1;
- if let ExprKind::Path(ref qpath) = args[0].kind;
- if let &QPath::Resolved(_, ref path2) = qpath;
+ if let ExprKind::Path(QPath::Resolved(_, ref path2)) = args[0].kind;
if path2.segments.len() == 1 && ident.name == path2.segments[0].ident.name;
then {
return Some(rb)
}
},
(&Kind::End(a, _), &Kind::Start(b, _)) if a != Bound::Included(b) => (),
- _ => return Some((a.range(), b.range())),
+ _ => {
+ // skip if the range `a` is completely included into the range `b`
+ if let Ordering::Equal | Ordering::Less = a.cmp(&b) {
+ let kind_a = Kind::End(a.range().node.1, a.range());
+ let kind_b = Kind::End(b.range().node.1, b.range());
+ if let Ordering::Equal | Ordering::Greater = kind_a.cmp(&kind_b) {
+ return None;
+ }
+ }
+ return Some((a.range(), b.range()));
+ },
}
}
"is_some()"
} else if match_qpath(path, &paths::POLL_READY) {
"is_ready()"
+ } else if match_qpath(path, &paths::IPADDR_V4) {
+ "is_ipv4()"
+ } else if match_qpath(path, &paths::IPADDR_V6) {
+ "is_ipv6()"
} else {
return;
}
"is_ok()",
"is_err()",
)
+ .or_else(|| {
+ find_good_method_for_match(
+ arms,
+ path_left,
+ path_right,
+ &paths::IPADDR_V4,
+ &paths::IPADDR_V6,
+ "is_ipv4()",
+ "is_ipv6()",
+ )
+ })
} else {
None
}
projects / rust.git / blobdiff