-use crate::consts::{constant, Constant};
+use crate::consts::{constant, miri_to_const, Constant};
use crate::utils::paths;
use crate::utils::sugg::Sugg;
+use crate::utils::usage::is_unused;
use crate::utils::{
- expr_block, in_macro_or_desugar, is_allowed, is_expn_of, match_qpath, match_type, multispan_sugg, remove_blocks,
- snippet, snippet_with_applicability, span_lint_and_sugg, span_lint_and_then, span_note_and_lint, walk_ptrs_ty,
+ expr_block, get_arg_name, get_parent_expr, in_macro, indent_of, is_allowed, is_expn_of, is_refutable, is_wild,
+ match_qpath, match_type, match_var, 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,
+ walk_ptrs_ty,
};
use if_chain::if_chain;
-use rustc::hir::def::CtorKind;
-use rustc::hir::*;
-use rustc::lint::{in_external_macro, LateContext, LateLintPass, LintArray, LintContext, LintPass};
-use rustc::ty::{self, Ty};
-use rustc::{declare_lint_pass, declare_tool_lint};
+use rustc_ast::ast::LitKind;
use rustc_errors::Applicability;
+use rustc_hir::def::CtorKind;
+use rustc_hir::{
+ Arm, BindingAnnotation, Block, BorrowKind, Expr, ExprKind, Local, MatchSource, Mutability, Node, Pat, PatKind,
+ QPath, RangeEnd,
+};
+use rustc_lint::{LateContext, LateLintPass, LintContext};
+use rustc_middle::lint::in_external_macro;
+use rustc_middle::ty::{self, Ty};
+use rustc_session::{declare_tool_lint, impl_lint_pass};
+use rustc_span::source_map::Span;
use std::cmp::Ordering;
use std::collections::Bound;
-use std::ops::Deref;
-use syntax::ast::LitKind;
-use syntax::source_map::Span;
declare_clippy_lint! {
/// **What it does:** Checks for matches with a single arm where an `if let`
/// ```
pub SINGLE_MATCH,
style,
- "a match statement with a single nontrivial arm (i.e., where the other arm is `_ => {}`) instead of `if let`"
+ "a `match` statement with a single nontrivial arm (i.e., where the other arm is `_ => {}`) instead of `if let`"
}
declare_clippy_lint! {
- /// **What it does:** Checks for matches with a two arms where an `if let else` will
+ /// **What it does:** Checks for matches with two arms where an `if let else` will
/// usually suffice.
///
/// **Why is this bad?** Just readability – `if let` nests less than a `match`.
/// Using `match`:
///
/// ```rust
+ /// # fn bar(foo: &usize) {}
+ /// # let other_ref: usize = 1;
+ /// # let x: Option<&usize> = Some(&1);
/// match x {
/// Some(ref foo) => bar(foo),
- /// _ => bar(other_ref),
+ /// _ => bar(&other_ref),
/// }
/// ```
///
/// Using `if let` with `else`:
///
/// ```rust
+ /// # fn bar(foo: &usize) {}
+ /// # let other_ref: usize = 1;
+ /// # let x: Option<&usize> = Some(&1);
/// if let Some(ref foo) = x {
/// bar(foo);
/// } else {
- /// bar(other_ref);
+ /// bar(&other_ref);
/// }
/// ```
pub SINGLE_MATCH_ELSE,
pedantic,
- "a match statement with a two arms where the second arm's pattern is a placeholder instead of a specific match pattern"
+ "a `match` statement with two arms where the second arm's pattern is a placeholder instead of a specific match pattern"
}
declare_clippy_lint! {
/// ```
pub MATCH_REF_PATS,
style,
- "a match or `if let` with all arms prefixed with `&` instead of deref-ing the match expression"
+ "a `match` or `if let` with all arms prefixed with `&` instead of deref-ing the match expression"
}
declare_clippy_lint! {
/// ```
pub MATCH_BOOL,
style,
- "a match on a boolean expression instead of an `if..else` block"
+ "a `match` on a boolean expression instead of an `if..else` block"
}
declare_clippy_lint! {
/// ```
pub MATCH_OVERLAPPING_ARM,
style,
- "a match with overlapping arms"
+ "a `match` with overlapping arms"
}
declare_clippy_lint! {
/// ```
pub MATCH_WILD_ERR_ARM,
style,
- "a match with `Err(_)` arm and take drastic actions"
+ "a `match` with `Err(_)` arm and take drastic actions"
}
declare_clippy_lint! {
/// ```
pub MATCH_AS_REF,
complexity,
- "a match on an Option value instead of using `as_ref()` or `as_mut`"
+ "a `match` on an Option value instead of using `as_ref()` or `as_mut`"
}
declare_clippy_lint! {
///
/// **Example:**
/// ```rust
+ /// # enum Foo { A(usize), B(usize) }
+ /// # let x = Foo::B(1);
/// match x {
/// A => {},
/// _ => {},
"a wildcard enum match arm using `_`"
}
-declare_lint_pass!(Matches => [
+declare_clippy_lint! {
+ /// **What it does:** Checks for wildcard pattern used with others patterns in same match arm.
+ ///
+ /// **Why is this bad?** Wildcard pattern already covers any other pattern as it will match anyway.
+ /// It makes the code less readable, especially to spot wildcard pattern use in match arm.
+ ///
+ /// **Known problems:** None.
+ ///
+ /// **Example:**
+ /// ```rust
+ /// match "foo" {
+ /// "a" => {},
+ /// "bar" | _ => {},
+ /// }
+ /// ```
+ pub WILDCARD_IN_OR_PATTERNS,
+ complexity,
+ "a wildcard pattern used with others patterns in same match arm"
+}
+
+declare_clippy_lint! {
+ /// **What it does:** Checks for matches being used to destructure a single-variant enum
+ /// or tuple struct where a `let` will suffice.
+ ///
+ /// **Why is this bad?** Just readability – `let` doesn't nest, whereas a `match` does.
+ ///
+ /// **Known problems:** None.
+ ///
+ /// **Example:**
+ /// ```rust
+ /// enum Wrapper {
+ /// Data(i32),
+ /// }
+ ///
+ /// let wrapper = Wrapper::Data(42);
+ ///
+ /// let data = match wrapper {
+ /// Wrapper::Data(i) => i,
+ /// };
+ /// ```
+ ///
+ /// The correct use would be:
+ /// ```rust
+ /// enum Wrapper {
+ /// Data(i32),
+ /// }
+ ///
+ /// let wrapper = Wrapper::Data(42);
+ /// let Wrapper::Data(data) = wrapper;
+ /// ```
+ pub INFALLIBLE_DESTRUCTURING_MATCH,
+ style,
+ "a `match` statement with a single infallible arm instead of a `let`"
+}
+
+declare_clippy_lint! {
+ /// **What it does:** Checks for useless match that binds to only one value.
+ ///
+ /// **Why is this bad?** Readability and needless complexity.
+ ///
+ /// **Known problems:** Suggested replacements may be incorrect when `match`
+ /// is actually binding temporary value, bringing a 'dropped while borrowed' error.
+ ///
+ /// **Example:**
+ /// ```rust
+ /// # let a = 1;
+ /// # let b = 2;
+ ///
+ /// // Bad
+ /// match (a, b) {
+ /// (c, d) => {
+ /// // useless match
+ /// }
+ /// }
+ ///
+ /// // Good
+ /// let (c, d) = (a, b);
+ /// ```
+ pub MATCH_SINGLE_BINDING,
+ complexity,
+ "a match with a single binding instead of using `let` statement"
+}
+
+declare_clippy_lint! {
+ /// **What it does:** Checks for unnecessary '..' pattern binding on struct when all fields are explicitly matched.
+ ///
+ /// **Why is this bad?** Correctness and readability. It's like having a wildcard pattern after
+ /// matching all enum variants explicitly.
+ ///
+ /// **Known problems:** None.
+ ///
+ /// **Example:**
+ /// ```rust
+ /// # struct A { a: i32 }
+ /// let a = A { a: 5 };
+ ///
+ /// // Bad
+ /// match a {
+ /// A { a: 5, .. } => {},
+ /// _ => {},
+ /// }
+ ///
+ /// // Good
+ /// match a {
+ /// A { a: 5 } => {},
+ /// _ => {},
+ /// }
+ /// ```
+ pub REST_PAT_IN_FULLY_BOUND_STRUCTS,
+ restriction,
+ "a match on a struct that binds all fields but still uses the wildcard pattern"
+}
+
+#[derive(Default)]
+pub struct Matches {
+ infallible_destructuring_match_linted: bool,
+}
+
+impl_lint_pass!(Matches => [
SINGLE_MATCH,
MATCH_REF_PATS,
MATCH_BOOL,
MATCH_OVERLAPPING_ARM,
MATCH_WILD_ERR_ARM,
MATCH_AS_REF,
- WILDCARD_ENUM_MATCH_ARM
+ WILDCARD_ENUM_MATCH_ARM,
+ WILDCARD_IN_OR_PATTERNS,
+ MATCH_SINGLE_BINDING,
+ INFALLIBLE_DESTRUCTURING_MATCH,
+ REST_PAT_IN_FULLY_BOUND_STRUCTS
]);
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Matches {
- fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
+ fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) {
if in_external_macro(cx.sess(), expr.span) {
return;
}
- if let ExprKind::Match(ref ex, ref arms, MatchSource::Normal) = expr.node {
+ if let ExprKind::Match(ref ex, ref arms, MatchSource::Normal) = expr.kind {
check_single_match(cx, ex, arms, expr);
check_match_bool(cx, ex, arms, expr);
check_overlapping_arms(cx, ex, arms);
check_wild_err_arm(cx, ex, arms);
check_wild_enum_match(cx, ex, arms);
check_match_as_ref(cx, ex, arms, expr);
+ check_wild_in_or_pats(cx, arms);
+
+ if self.infallible_destructuring_match_linted {
+ self.infallible_destructuring_match_linted = false;
+ } else {
+ check_match_single_binding(cx, ex, arms, expr);
+ }
}
- if let ExprKind::Match(ref ex, ref arms, _) = expr.node {
+ if let ExprKind::Match(ref ex, ref arms, _) = expr.kind {
check_match_ref_pats(cx, ex, arms, expr);
}
}
+
+ fn check_local(&mut self, cx: &LateContext<'a, 'tcx>, local: &'tcx Local<'_>) {
+ if_chain! {
+ if let Some(ref expr) = local.init;
+ if let ExprKind::Match(ref target, ref arms, MatchSource::Normal) = expr.kind;
+ if arms.len() == 1 && arms[0].guard.is_none();
+ 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]);
+ let body = remove_blocks(&arms[0].body);
+ if match_var(body, arg);
+
+ then {
+ let mut applicability = Applicability::MachineApplicable;
+ self.infallible_destructuring_match_linted = true;
+ span_lint_and_sugg(
+ cx,
+ INFALLIBLE_DESTRUCTURING_MATCH,
+ local.span,
+ "you seem to be trying to use `match` to destructure a single infallible pattern. \
+ Consider using `let`",
+ "try this",
+ format!(
+ "let {}({}) = {};",
+ snippet_with_applicability(cx, variant_name.span, "..", &mut applicability),
+ snippet_with_applicability(cx, local.pat.span, "..", &mut applicability),
+ snippet_with_applicability(cx, target.span, "..", &mut applicability),
+ ),
+ applicability,
+ );
+ }
+ }
+ }
+
+ fn check_pat(&mut self, cx: &LateContext<'a, 'tcx>, pat: &'tcx Pat<'_>) {
+ if_chain! {
+ if let PatKind::Struct(ref qpath, fields, true) = pat.kind;
+ if let QPath::Resolved(_, ref path) = qpath;
+ 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 def.is_struct() || def.is_union();
+ if fields.len() == def.non_enum_variant().fields.len();
+
+ then {
+ span_lint_and_help(
+ cx,
+ REST_PAT_IN_FULLY_BOUND_STRUCTS,
+ pat.span,
+ "unnecessary use of `..` pattern in struct binding. All fields were already bound",
+ "consider removing `..` from this binding",
+ );
+ }
+ }
+ }
}
#[rustfmt::skip]
-fn check_single_match(cx: &LateContext<'_, '_>, ex: &Expr, arms: &[Arm], expr: &Expr) {
- if arms.len() == 2 &&
- arms[0].pats.len() == 1 && arms[0].guard.is_none() &&
- arms[1].pats.len() == 1 && arms[1].guard.is_none() {
+fn check_single_match(cx: &LateContext<'_, '_>, ex: &Expr<'_>, arms: &[Arm<'_>], expr: &Expr<'_>) {
+ if arms.len() == 2 && arms[0].guard.is_none() && arms[1].guard.is_none() {
+ if in_macro(expr.span) {
+ // Don't lint match expressions present in
+ // macro_rules! block
+ return;
+ }
+ if let PatKind::Or(..) = arms[0].pat.kind {
+ // don't lint for or patterns for now, this makes
+ // the lint noisy in unnecessary situations
+ return;
+ }
let els = remove_blocks(&arms[1].body);
let els = if is_unit_expr(els) {
None
- } else if let ExprKind::Block(_, _) = els.node {
+ } else if let ExprKind::Block(_, _) = els.kind {
// matches with blocks that contain statements are prettier as `if let + else`
Some(els)
} else {
return;
};
let ty = cx.tables.expr_ty(ex);
- if ty.sty != ty::Bool || is_allowed(cx, MATCH_BOOL, ex.hir_id) {
+ if ty.kind != ty::Bool || is_allowed(cx, MATCH_BOOL, ex.hir_id) {
check_single_match_single_pattern(cx, ex, arms, expr, els);
check_single_match_opt_like(cx, ex, arms, expr, ty, els);
}
fn check_single_match_single_pattern(
cx: &LateContext<'_, '_>,
- ex: &Expr,
- arms: &[Arm],
- expr: &Expr,
- els: Option<&Expr>,
+ ex: &Expr<'_>,
+ arms: &[Arm<'_>],
+ expr: &Expr<'_>,
+ els: Option<&Expr<'_>>,
) {
- if is_wild(&arms[1].pats[0]) {
+ if is_wild(&arms[1].pat) {
report_single_match_single_pattern(cx, ex, arms, expr, els);
}
}
fn report_single_match_single_pattern(
cx: &LateContext<'_, '_>,
- ex: &Expr,
- arms: &[Arm],
- expr: &Expr,
- els: Option<&Expr>,
+ ex: &Expr<'_>,
+ arms: &[Arm<'_>],
+ expr: &Expr<'_>,
+ els: Option<&Expr<'_>>,
) {
let lint = if els.is_some() { SINGLE_MATCH_ELSE } else { SINGLE_MATCH };
let els_str = els.map_or(String::new(), |els| {
- format!(" else {}", expr_block(cx, els, None, ".."))
+ format!(" else {}", expr_block(cx, els, None, "..", Some(expr.span)))
});
span_lint_and_sugg(
cx,
"try this",
format!(
"if let {} = {} {}{}",
- snippet(cx, arms[0].pats[0].span, ".."),
+ snippet(cx, arms[0].pat.span, ".."),
snippet(cx, ex.span, ".."),
- expr_block(cx, &arms[0].body, None, ".."),
+ expr_block(cx, &arms[0].body, None, "..", Some(expr.span)),
els_str,
),
Applicability::HasPlaceholders,
fn check_single_match_opt_like(
cx: &LateContext<'_, '_>,
- ex: &Expr,
- arms: &[Arm],
- expr: &Expr,
+ ex: &Expr<'_>,
+ arms: &[Arm<'_>],
+ expr: &Expr<'_>,
ty: Ty<'_>,
- els: Option<&Expr>,
+ els: Option<&Expr<'_>>,
) {
// list of candidate `Enum`s we know will never get any more members
let candidates = &[
(&paths::RESULT, "Ok"),
];
- let path = match arms[1].pats[0].node {
+ let path = match arms[1].pat.kind {
PatKind::TupleStruct(ref path, ref inner, _) => {
// Contains any non wildcard patterns (e.g., `Err(err)`)?
if !inner.iter().all(is_wild) {
return;
}
- print::to_string(print::NO_ANN, |s| s.print_qpath(path, false))
+ rustc_hir_pretty::to_string(rustc_hir_pretty::NO_ANN, |s| s.print_qpath(path, false))
},
PatKind::Binding(BindingAnnotation::Unannotated, .., ident, None) => ident.to_string(),
- PatKind::Path(ref path) => print::to_string(print::NO_ANN, |s| s.print_qpath(path, false)),
+ PatKind::Path(ref path) => {
+ rustc_hir_pretty::to_string(rustc_hir_pretty::NO_ANN, |s| s.print_qpath(path, false))
+ },
_ => return,
};
}
}
-fn check_match_bool(cx: &LateContext<'_, '_>, ex: &Expr, arms: &[Arm], expr: &Expr) {
+fn check_match_bool(cx: &LateContext<'_, '_>, ex: &Expr<'_>, arms: &[Arm<'_>], expr: &Expr<'_>) {
// Type of expression is `bool`.
- if cx.tables.expr_ty(ex).sty == ty::Bool {
+ if cx.tables.expr_ty(ex).kind == ty::Bool {
span_lint_and_then(
cx,
MATCH_BOOL,
expr.span,
"you seem to be trying to match on a boolean expression",
move |db| {
- if arms.len() == 2 && arms[0].pats.len() == 1 {
+ if arms.len() == 2 {
// no guards
- let exprs = if let PatKind::Lit(ref arm_bool) = arms[0].pats[0].node {
- if let ExprKind::Lit(ref lit) = arm_bool.node {
+ let exprs = if let PatKind::Lit(ref arm_bool) = arms[0].pat.kind {
+ if let ExprKind::Lit(ref lit) = arm_bool.kind {
match lit.node {
LitKind::Bool(true) => Some((&*arms[0].body, &*arms[1].body)),
LitKind::Bool(false) => Some((&*arms[1].body, &*arms[0].body)),
(false, false) => Some(format!(
"if {} {} else {}",
snippet(cx, ex.span, "b"),
- expr_block(cx, true_expr, None, ".."),
- expr_block(cx, false_expr, None, "..")
+ expr_block(cx, true_expr, None, "..", Some(expr.span)),
+ expr_block(cx, false_expr, None, "..", Some(expr.span))
)),
(false, true) => Some(format!(
"if {} {}",
snippet(cx, ex.span, "b"),
- expr_block(cx, true_expr, None, "..")
+ expr_block(cx, true_expr, None, "..", Some(expr.span))
)),
(true, false) => {
let test = Sugg::hir(cx, ex, "..");
- Some(format!("if {} {}", !test, expr_block(cx, false_expr, None, "..")))
+ Some(format!(
+ "if {} {}",
+ !test,
+ expr_block(cx, false_expr, None, "..", Some(expr.span))
+ ))
},
(true, true) => None,
};
if let Some(sugg) = sugg {
db.span_suggestion(
expr.span,
- "consider using an if/else expression",
+ "consider using an `if`/`else` expression",
sugg,
Applicability::HasPlaceholders,
);
}
}
-fn check_overlapping_arms<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, ex: &'tcx Expr, arms: &'tcx [Arm]) {
+fn check_overlapping_arms<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, ex: &'tcx Expr<'_>, arms: &'tcx [Arm<'_>]) {
if arms.len() >= 2 && cx.tables.expr_ty(ex).is_integral() {
- let ranges = all_ranges(cx, arms);
+ let ranges = all_ranges(cx, arms, cx.tables.expr_ty(ex));
let type_ranges = type_ranges(&ranges);
if !type_ranges.is_empty() {
if let Some((start, end)) = overlapping(&type_ranges) {
- span_note_and_lint(
+ span_lint_and_note(
cx,
MATCH_OVERLAPPING_ARM,
start.span,
}
}
-fn is_wild(pat: &impl std::ops::Deref<Target = Pat>) -> bool {
- match pat.node {
- PatKind::Wild => true,
- _ => false,
- }
-}
-
-fn check_wild_err_arm(cx: &LateContext<'_, '_>, ex: &Expr, arms: &[Arm]) {
+fn check_wild_err_arm(cx: &LateContext<'_, '_>, ex: &Expr<'_>, arms: &[Arm<'_>]) {
let ex_ty = walk_ptrs_ty(cx.tables.expr_ty(ex));
if match_type(cx, ex_ty, &paths::RESULT) {
for arm in arms {
- if let PatKind::TupleStruct(ref path, ref inner, _) = arm.pats[0].node {
- let path_str = print::to_string(print::NO_ANN, |s| s.print_qpath(path, false));
- if_chain! {
- if path_str == "Err";
- if inner.iter().any(is_wild);
- if let ExprKind::Block(ref block, _) = arm.body.node;
- if is_panic_block(block);
- then {
- // `Err(_)` arm with `panic!` found
- span_note_and_lint(cx,
- MATCH_WILD_ERR_ARM,
- arm.pats[0].span,
- "Err(_) will match all errors, maybe not a good idea",
- arm.pats[0].span,
- "to remove this warning, match each error separately \
- or use unreachable macro");
+ if let PatKind::TupleStruct(ref path, ref inner, _) = arm.pat.kind {
+ let path_str = rustc_hir_pretty::to_string(rustc_hir_pretty::NO_ANN, |s| s.print_qpath(path, false));
+ if path_str == "Err" {
+ let mut matching_wild = inner.iter().any(is_wild);
+ let mut ident_bind_name = String::from("_");
+ 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) {
+ ident_bind_name = (&ident.name.as_str()).to_string();
+ matching_wild = true;
+ }
+ }
+ });
+ }
+ if_chain! {
+ if matching_wild;
+ if let ExprKind::Block(ref block, _) = arm.body.kind;
+ if is_panic_block(block);
+ then {
+ // `Err(_)` or `Err(_e)` arm with `panic!` found
+ span_lint_and_note(cx,
+ MATCH_WILD_ERR_ARM,
+ arm.pat.span,
+ &format!("`Err({})` matches all errors", &ident_bind_name),
+ arm.pat.span,
+ "match each error separately or use the error output",
+ );
+ }
}
}
}
}
}
-fn check_wild_enum_match(cx: &LateContext<'_, '_>, ex: &Expr, arms: &[Arm]) {
+fn check_wild_enum_match(cx: &LateContext<'_, '_>, ex: &Expr<'_>, arms: &[Arm<'_>]) {
let ty = cx.tables.expr_ty(ex);
if !ty.is_enum() {
// If there isn't a nice closed set of possible values that can be conveniently enumerated,
let mut wildcard_span = None;
let mut wildcard_ident = None;
for arm in arms {
- for pat in &arm.pats {
- if let PatKind::Wild = pat.node {
- wildcard_span = Some(pat.span);
- } else if let PatKind::Binding(_, _, ident, None) = pat.node {
- wildcard_span = Some(pat.span);
- wildcard_ident = Some(ident);
- }
+ if let PatKind::Wild = arm.pat.kind {
+ wildcard_span = Some(arm.pat.span);
+ } else if let PatKind::Binding(_, _, ident, None) = arm.pat.kind {
+ wildcard_span = Some(arm.pat.span);
+ wildcard_ident = Some(ident);
}
}
// already covered.
let mut missing_variants = vec![];
- if let ty::Adt(def, _) = ty.sty {
+ if let ty::Adt(def, _) = ty.kind {
for variant in &def.variants {
missing_variants.push(variant);
}
// covered by the set of guards that cover it, but that's really hard to do.
continue;
}
- for pat in &arm.pats {
- if let PatKind::Path(ref path) = pat.deref().node {
- 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, ..) = pat.deref().node {
- if let QPath::Resolved(_, p) = path {
- missing_variants.retain(|e| e.ctor_def_id != Some(p.res.def_id()));
- }
+ if let PatKind::Path(ref path) = arm.pat.kind {
+ 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, ..) = arm.pat.kind {
+ if let QPath::Resolved(_, p) = path {
+ missing_variants.retain(|e| e.ctor_def_id != Some(p.res.def_id()));
}
}
}
- let suggestion: Vec<String> = missing_variants
+ let mut suggestion: Vec<String> = missing_variants
.iter()
.map(|v| {
let suffix = match v.ctor_kind {
return;
}
+ let mut message = "wildcard match will miss any future added variants";
+
+ if let ty::Adt(def, _) = ty.kind {
+ if def.is_variant_list_non_exhaustive() {
+ message = "match on non-exhaustive enum doesn't explicitly match all known variants";
+ suggestion.push(String::from("_"));
+ }
+ }
+
span_lint_and_sugg(
cx,
WILDCARD_ENUM_MATCH_ARM,
wildcard_span,
- "wildcard match will miss any future added variants.",
+ message,
"try this",
suggestion.join(" | "),
Applicability::MachineApplicable,
}
// If the block contains only a `panic!` macro (as expression or statement)
-fn is_panic_block(block: &Block) -> bool {
+fn is_panic_block(block: &Block<'_>) -> bool {
match (&block.expr, block.stmts.len(), block.stmts.first()) {
(&Some(ref exp), 0, _) => {
is_expn_of(exp.span, "panic").is_some() && is_expn_of(exp.span, "unreachable").is_none()
}
}
-fn check_match_ref_pats(cx: &LateContext<'_, '_>, ex: &Expr, arms: &[Arm], expr: &Expr) {
+fn check_match_ref_pats(cx: &LateContext<'_, '_>, ex: &Expr<'_>, arms: &[Arm<'_>], expr: &Expr<'_>) {
if has_only_ref_pats(arms) {
- let mut suggs = Vec::new();
- let (title, msg) = if let ExprKind::AddrOf(Mutability::MutImmutable, ref inner) = ex.node {
+ let mut suggs = Vec::with_capacity(arms.len() + 1);
+ let (title, msg) = if let ExprKind::AddrOf(BorrowKind::Ref, Mutability::Not, ref inner) = ex.kind {
let span = ex.span.source_callsite();
suggs.push((span, Sugg::hir_with_macro_callsite(cx, inner, "..").to_string()));
(
)
};
- suggs.extend(arms.iter().flat_map(|a| &a.pats).filter_map(|p| {
- if let PatKind::Ref(ref refp, _) = p.node {
- Some((p.span, snippet(cx, refp.span, "..").to_string()))
+ suggs.extend(arms.iter().filter_map(|a| {
+ if let PatKind::Ref(ref refp, _) = a.pat.kind {
+ Some((a.pat.span, snippet(cx, refp.span, "..").to_string()))
} else {
None
}
}));
span_lint_and_then(cx, MATCH_REF_PATS, expr.span, title, |db| {
- if !in_macro_or_desugar(expr.span) {
+ if !expr.span.from_expansion() {
multispan_sugg(db, msg.to_owned(), suggs);
}
});
}
}
-fn check_match_as_ref(cx: &LateContext<'_, '_>, ex: &Expr, arms: &[Arm], expr: &Expr) {
- if arms.len() == 2
- && arms[0].pats.len() == 1
- && arms[0].guard.is_none()
- && arms[1].pats.len() == 1
- && arms[1].guard.is_none()
- {
+fn check_match_as_ref(cx: &LateContext<'_, '_>, ex: &Expr<'_>, arms: &[Arm<'_>], expr: &Expr<'_>) {
+ if arms.len() == 2 && arms[0].guard.is_none() && arms[1].guard.is_none() {
let arm_ref: Option<BindingAnnotation> = if is_none_arm(&arms[0]) {
is_ref_some_arm(&arms[1])
} else if is_none_arm(&arms[1]) {
} else {
"as_mut"
};
+
+ let output_ty = cx.tables.expr_ty(expr);
+ let input_ty = cx.tables.expr_ty(ex);
+
+ let cast = if_chain! {
+ if let ty::Adt(_, substs) = input_ty.kind;
+ let input_ty = substs.type_at(0);
+ if let ty::Adt(_, substs) = output_ty.kind;
+ let output_ty = substs.type_at(0);
+ if let ty::Ref(_, output_ty, _) = output_ty.kind;
+ if input_ty != output_ty;
+ then {
+ ".map(|x| x as _)"
+ } else {
+ ""
+ }
+ };
+
let mut applicability = Applicability::MachineApplicable;
span_lint_and_sugg(
cx,
MATCH_AS_REF,
expr.span,
- &format!("use {}() instead", suggestion),
+ &format!("use `{}()` instead", suggestion),
"try this",
format!(
- "{}.{}()",
+ "{}.{}(){}",
snippet_with_applicability(cx, ex.span, "_", &mut applicability),
- suggestion
+ suggestion,
+ cast,
),
applicability,
)
}
}
+fn check_wild_in_or_pats(cx: &LateContext<'_, '_>, arms: &[Arm<'_>]) {
+ for arm in arms {
+ if let PatKind::Or(ref fields) = arm.pat.kind {
+ // look for multiple fields in this arm that contains at least one Wild pattern
+ if fields.len() > 1 && fields.iter().any(is_wild) {
+ span_lint_and_help(
+ cx,
+ WILDCARD_IN_OR_PATTERNS,
+ arm.pat.span,
+ "wildcard pattern covers any other pattern as it will match anyway.",
+ "Consider handling `_` separately.",
+ );
+ }
+ }
+ }
+}
+
+fn check_match_single_binding<'a>(cx: &LateContext<'_, 'a>, ex: &Expr<'a>, arms: &[Arm<'_>], expr: &Expr<'_>) {
+ if in_macro(expr.span) || arms.len() != 1 || is_refutable(cx, arms[0].pat) {
+ return;
+ }
+ let matched_vars = ex.span;
+ let bind_names = arms[0].pat.span;
+ let match_body = remove_blocks(&arms[0].body);
+ let mut snippet_body = if match_body.span.from_expansion() {
+ Sugg::hir_with_macro_callsite(cx, match_body, "..").to_string()
+ } else {
+ snippet_block(cx, match_body.span, "..", Some(expr.span)).to_string()
+ };
+
+ // Do we need to add ';' to suggestion ?
+ match match_body.kind {
+ ExprKind::Block(block, _) => {
+ // macro + expr_ty(body) == ()
+ if block.span.from_expansion() && cx.tables.expr_ty(&match_body).is_unit() {
+ snippet_body.push(';');
+ }
+ },
+ _ => {
+ // expr_ty(body) == ()
+ if cx.tables.expr_ty(&match_body).is_unit() {
+ snippet_body.push(';');
+ }
+ },
+ }
+
+ let mut applicability = Applicability::MaybeIncorrect;
+ match arms[0].pat.kind {
+ PatKind::Binding(..) | PatKind::Tuple(_, _) | PatKind::Struct(..) => {
+ // If this match is in a local (`let`) stmt
+ let (target_span, sugg) = if let Some(parent_let_node) = opt_parent_let(cx, ex) {
+ (
+ parent_let_node.span,
+ format!(
+ "let {} = {};\n{}let {} = {};",
+ snippet_with_applicability(cx, bind_names, "..", &mut applicability),
+ snippet_with_applicability(cx, matched_vars, "..", &mut applicability),
+ " ".repeat(indent_of(cx, expr.span).unwrap_or(0)),
+ snippet_with_applicability(cx, parent_let_node.pat.span, "..", &mut applicability),
+ snippet_body
+ ),
+ )
+ } else {
+ // If we are in closure, we need curly braces around suggestion
+ let mut indent = " ".repeat(indent_of(cx, ex.span).unwrap_or(0));
+ let (mut cbrace_start, mut cbrace_end) = ("".to_string(), "".to_string());
+ if let Some(parent_expr) = get_parent_expr(cx, expr) {
+ if let ExprKind::Closure(..) = parent_expr.kind {
+ cbrace_end = format!("\n{}}}", indent);
+ // Fix body indent due to the closure
+ indent = " ".repeat(indent_of(cx, bind_names).unwrap_or(0));
+ cbrace_start = format!("{{\n{}", indent);
+ }
+ };
+ (
+ expr.span,
+ format!(
+ "{}let {} = {};\n{}{}{}",
+ cbrace_start,
+ snippet_with_applicability(cx, bind_names, "..", &mut applicability),
+ snippet_with_applicability(cx, matched_vars, "..", &mut applicability),
+ indent,
+ snippet_body,
+ cbrace_end
+ ),
+ )
+ };
+ span_lint_and_sugg(
+ cx,
+ MATCH_SINGLE_BINDING,
+ target_span,
+ "this match could be written as a `let` statement",
+ "consider using `let` statement",
+ sugg,
+ applicability,
+ );
+ },
+ PatKind::Wild => {
+ span_lint_and_sugg(
+ cx,
+ MATCH_SINGLE_BINDING,
+ expr.span,
+ "this match could be replaced by its body itself",
+ "consider using the match body instead",
+ snippet_body,
+ Applicability::MachineApplicable,
+ );
+ },
+ _ => (),
+ }
+}
+
+/// Returns true if the `ex` match expression is in a local (`let`) statement
+fn opt_parent_let<'a>(cx: &LateContext<'_, 'a>, ex: &Expr<'a>) -> Option<&'a Local<'a>> {
+ if_chain! {
+ let map = &cx.tcx.hir();
+ if let Some(Node::Expr(parent_arm_expr)) = map.find(map.get_parent_node(ex.hir_id));
+ if let Some(Node::Local(parent_let_expr)) = map.find(map.get_parent_node(parent_arm_expr.hir_id));
+ then {
+ return Some(parent_let_expr);
+ }
+ }
+ None
+}
+
/// Gets all arms that are unbounded `PatRange`s.
-fn all_ranges<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, arms: &'tcx [Arm]) -> Vec<SpannedRange<Constant>> {
+fn all_ranges<'a, 'tcx>(
+ cx: &LateContext<'a, 'tcx>,
+ arms: &'tcx [Arm<'_>],
+ ty: Ty<'tcx>,
+) -> Vec<SpannedRange<Constant>> {
arms.iter()
.flat_map(|arm| {
if let Arm {
- ref pats, guard: None, ..
+ ref pat, guard: None, ..
} = *arm
{
- pats.iter()
- } else {
- [].iter()
- }
- .filter_map(|pat| {
- if let PatKind::Range(ref lhs, ref rhs, ref range_end) = pat.node {
- let lhs = constant(cx, cx.tables, lhs)?.0;
- let rhs = constant(cx, cx.tables, rhs)?.0;
- let rhs = match *range_end {
+ if let PatKind::Range(ref lhs, ref rhs, range_end) = pat.kind {
+ let lhs = match lhs {
+ Some(lhs) => constant(cx, cx.tables, lhs)?.0,
+ None => miri_to_const(ty.numeric_min_val(cx.tcx)?)?,
+ };
+ let rhs = match rhs {
+ Some(rhs) => constant(cx, cx.tables, rhs)?.0,
+ None => miri_to_const(ty.numeric_max_val(cx.tcx)?)?,
+ };
+ let rhs = match range_end {
RangeEnd::Included => Bound::Included(rhs),
RangeEnd::Excluded => Bound::Excluded(rhs),
};
});
}
- if let PatKind::Lit(ref value) = pat.node {
+ if let PatKind::Lit(ref value) = pat.kind {
let value = constant(cx, cx.tables, value)?.0;
return Some(SpannedRange {
span: pat.span,
node: (value.clone(), Bound::Included(value)),
});
}
-
- None
- })
+ }
+ None
})
.collect()
}
.collect()
}
-fn is_unit_expr(expr: &Expr) -> bool {
- match expr.node {
+fn is_unit_expr(expr: &Expr<'_>) -> bool {
+ match expr.kind {
ExprKind::Tup(ref v) if v.is_empty() => true,
ExprKind::Block(ref b, _) if b.stmts.is_empty() && b.expr.is_none() => true,
_ => false,
}
// Checks if arm has the form `None => None`
-fn is_none_arm(arm: &Arm) -> bool {
- match arm.pats[0].node {
+fn is_none_arm(arm: &Arm<'_>) -> bool {
+ match arm.pat.kind {
PatKind::Path(ref path) if match_qpath(path, &paths::OPTION_NONE) => true,
_ => false,
}
}
// Checks if arm has the form `Some(ref v) => Some(v)` (checks for `ref` and `ref mut`)
-fn is_ref_some_arm(arm: &Arm) -> Option<BindingAnnotation> {
+fn is_ref_some_arm(arm: &Arm<'_>) -> Option<BindingAnnotation> {
if_chain! {
- if let PatKind::TupleStruct(ref path, ref pats, _) = arm.pats[0].node;
+ if let PatKind::TupleStruct(ref path, ref pats, _) = arm.pat.kind;
if pats.len() == 1 && match_qpath(path, &paths::OPTION_SOME);
- if let PatKind::Binding(rb, .., ident, _) = pats[0].node;
+ if let PatKind::Binding(rb, .., ident, _) = pats[0].kind;
if rb == BindingAnnotation::Ref || rb == BindingAnnotation::RefMut;
- if let ExprKind::Call(ref e, ref args) = remove_blocks(&arm.body).node;
- if let ExprKind::Path(ref some_path) = e.node;
+ 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].node;
+ if let ExprKind::Path(ref qpath) = args[0].kind;
if let &QPath::Resolved(_, ref path2) = qpath;
if path2.segments.len() == 1 && ident.name == path2.segments[0].ident.name;
then {
None
}
-fn has_only_ref_pats(arms: &[Arm]) -> bool {
+fn has_only_ref_pats(arms: &[Arm<'_>]) -> bool {
let mapped = arms
.iter()
- .flat_map(|a| &a.pats)
- .map(|p| {
- match p.node {
+ .map(|a| {
+ match a.pat.kind {
PatKind::Ref(..) => Some(true), // &-patterns
PatKind::Wild => Some(false), // an "anything" wildcard is also fine
_ => None, // any other pattern is not fine
None
}
+
+#[test]
+fn test_overlapping() {
+ use rustc_span::source_map::DUMMY_SP;
+
+ let sp = |s, e| SpannedRange {
+ span: DUMMY_SP,
+ node: (s, e),
+ };
+
+ assert_eq!(None, overlapping::<u8>(&[]));
+ assert_eq!(None, overlapping(&[sp(1, Bound::Included(4))]));
+ assert_eq!(
+ None,
+ overlapping(&[sp(1, Bound::Included(4)), sp(5, Bound::Included(6))])
+ );
+ assert_eq!(
+ None,
+ overlapping(&[
+ sp(1, Bound::Included(4)),
+ sp(5, Bound::Included(6)),
+ sp(10, Bound::Included(11))
+ ],)
+ );
+ assert_eq!(
+ Some((&sp(1, Bound::Included(4)), &sp(3, Bound::Included(6)))),
+ overlapping(&[sp(1, Bound::Included(4)), sp(3, Bound::Included(6))])
+ );
+ assert_eq!(
+ Some((&sp(5, Bound::Included(6)), &sp(6, Bound::Included(11)))),
+ overlapping(&[
+ sp(1, Bound::Included(4)),
+ sp(5, Bound::Included(6)),
+ sp(6, Bound::Included(11))
+ ],)
+ );
+}
projects / rust.git / blobdiff