use crate::utils::sugg::Sugg;
use crate::utils::usage::is_unused;
use crate::utils::{
- expr_block, get_arg_name, 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,
+ 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, 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::lint::in_external_macro;
-use rustc::ty::{self, Ty};
use rustc_ast::ast::LitKind;
use rustc_errors::Applicability;
use rustc_hir::def::CtorKind;
use rustc_hir::{
- print, Arm, BindingAnnotation, Block, BorrowKind, Expr, ExprKind, Local, MatchSource, Mutability, Pat, PatKind,
+ 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;
/// ```rust
/// # fn bar(stool: &str) {}
/// # let x = Some("abc");
+ ///
+ /// // Bad
/// match x {
/// Some(ref foo) => bar(foo),
/// _ => (),
/// }
+ ///
+ /// // Good
+ /// if let Some(ref foo) = x {
+ /// bar(foo);
+ /// }
/// ```
pub SINGLE_MATCH,
style,
///
/// **Example:**
/// ```rust,ignore
+ /// // Bad
/// match x {
/// &A(ref y) => foo(y),
/// &B => bar(),
/// _ => frob(&x),
/// }
+ ///
+ /// // Good
+ /// match *x {
+ /// A(ref y) => foo(y),
+ /// B => bar(),
+ /// _ => frob(x),
+ /// }
/// ```
pub MATCH_REF_PATS,
style,
/// }
/// ```
pub MATCH_BOOL,
- style,
+ pedantic,
"a `match` on a boolean expression instead of an `if..else` block"
}
/// **What it does:** Checks for arm which matches all errors with `Err(_)`
/// and take drastic actions like `panic!`.
///
- /// **Why is this bad?** It is generally a bad practice, just like
+ /// **Why is this bad?** It is generally a bad practice, similar to
/// catching all exceptions in java with `catch(Exception)`
///
/// **Known problems:** None.
/// }
/// ```
pub MATCH_WILD_ERR_ARM,
- style,
+ pedantic,
"a `match` with `Err(_)` arm and take drastic actions"
}
/// **Example:**
/// ```rust
/// let x: Option<()> = None;
+ ///
+ /// // Bad
/// let r: Option<&()> = match x {
/// None => None,
/// Some(ref v) => Some(v),
/// };
+ ///
+ /// // Good
+ /// let r: Option<&()> = x.as_ref();
/// ```
pub MATCH_AS_REF,
complexity,
/// ```rust
/// # enum Foo { A(usize), B(usize) }
/// # let x = Foo::B(1);
+ ///
+ /// // Bad
/// match x {
- /// A => {},
+ /// Foo::A(_) => {},
/// _ => {},
/// }
+ ///
+ /// // Good
+ /// match x {
+ /// Foo::A(_) => {},
+ /// Foo::B(_) => {},
+ /// }
/// ```
pub WILDCARD_ENUM_MATCH_ARM,
restriction,
"a wildcard enum match arm using `_`"
}
+declare_clippy_lint! {
+ /// **What it does:** Checks for wildcard enum matches for a single variant.
+ ///
+ /// **Why is this bad?** New enum variants added by library updates can be missed.
+ ///
+ /// **Known problems:** Suggested replacements may not use correct path to enum
+ /// if it's not present in the current scope.
+ ///
+ /// **Example:**
+ ///
+ /// ```rust
+ /// # enum Foo { A, B, C }
+ /// # let x = Foo::B;
+ /// // Bad
+ /// match x {
+ /// Foo::A => {},
+ /// Foo::B => {},
+ /// _ => {},
+ /// }
+ ///
+ /// // Good
+ /// match x {
+ /// Foo::A => {},
+ /// Foo::B => {},
+ /// Foo::C => {},
+ /// }
+ /// ```
+ pub MATCH_WILDCARD_FOR_SINGLE_VARIANTS,
+ pedantic,
+ "a wildcard enum match for a single variant"
+}
+
declare_clippy_lint! {
/// **What it does:** Checks for wildcard pattern used with others patterns in same match arm.
///
///
/// **Example:**
/// ```rust
+ /// // Bad
/// match "foo" {
/// "a" => {},
/// "bar" | _ => {},
/// }
+ ///
+ /// // Good
+ /// match "foo" {
+ /// "a" => {},
+ /// _ => {},
+ /// }
/// ```
pub WILDCARD_IN_OR_PATTERNS,
complexity,
MATCH_WILD_ERR_ARM,
MATCH_AS_REF,
WILDCARD_ENUM_MATCH_ARM,
+ MATCH_WILDCARD_FOR_SINGLE_VARIANTS,
WILDCARD_IN_OR_PATTERNS,
MATCH_SINGLE_BINDING,
INFALLIBLE_DESTRUCTURING_MATCH,
fn check_local(&mut self, cx: &LateContext<'a, 'tcx>, local: &'tcx Local<'_>) {
if_chain! {
+ if !in_external_macro(cx.sess(), local.span);
+ if !in_macro(local.span);
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();
fn check_pat(&mut self, cx: &LateContext<'a, 'tcx>, pat: &'tcx Pat<'_>) {
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 Some(def_id) = path.res.opt_def_id();
REST_PAT_IN_FULLY_BOUND_STRUCTS,
pat.span,
"unnecessary use of `..` pattern in struct binding. All fields were already bound",
+ None,
"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].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
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,
};
MATCH_BOOL,
expr.span,
"you seem to be trying to match on a boolean expression",
- move |db| {
+ move |diag| {
if arms.len() == 2 {
// no guards
let exprs = if let PatKind::Lit(ref arm_bool) = arms[0].pat.kind {
};
if let Some(sugg) = sugg {
- db.span_suggestion(
+ diag.span_suggestion(
expr.span,
"consider using an `if`/`else` expression",
sugg,
MATCH_OVERLAPPING_ARM,
start.span,
"some ranges overlap",
- end.span,
+ Some(end.span),
"overlaps with this",
);
}
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) {
+ if is_type_diagnostic_item(cx, ex_ty, sym!(result_type)) {
for arm in arms {
if let PatKind::TupleStruct(ref path, ref inner, _) = arm.pat.kind {
- let path_str = print::to_string(print::NO_ANN, |s| s.print_qpath(path, false));
+ 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("_");
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",
+ None,
+ "match each error separately or use the error output, or use `.except(msg)` if the error case is unreachable",
);
}
}
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 {
+ } else if let PatKind::TupleStruct(ref path, ref patterns, ..) = arm.pat.kind {
if let QPath::Resolved(_, p) = path {
- missing_variants.retain(|e| e.ctor_def_id != Some(p.res.def_id()));
+ // 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<'_>| {
+ if let PatKind::Wild | PatKind::Binding(.., None) = pat.kind {
+ true
+ } else {
+ false
+ }
+ };
+ if patterns.iter().all(is_pattern_exhaustive) {
+ missing_variants.retain(|e| e.ctor_def_id != Some(p.res.def_id()));
+ }
}
}
}
}
}
+ if suggestion.len() == 1 {
+ // No need to check for non-exhaustive enum as in that case len would be greater than 1
+ span_lint_and_sugg(
+ cx,
+ MATCH_WILDCARD_FOR_SINGLE_VARIANTS,
+ wildcard_span,
+ message,
+ "try this",
+ suggestion[0].clone(),
+ Applicability::MaybeIncorrect,
+ )
+ };
+
span_lint_and_sugg(
cx,
WILDCARD_ENUM_MATCH_ARM,
message,
"try this",
suggestion.join(" | "),
- Applicability::MachineApplicable,
+ Applicability::MaybeIncorrect,
)
}
}
}
}));
- span_lint_and_then(cx, MATCH_REF_PATS, expr.span, title, |db| {
+ span_lint_and_then(cx, MATCH_REF_PATS, expr.span, title, |diag| {
if !expr.span.from_expansion() {
- multispan_sugg(db, msg.to_owned(), suggs);
+ multispan_sugg(diag, msg, suggs);
}
});
}
WILDCARD_IN_OR_PATTERNS,
arm.pat.span,
"wildcard pattern covers any other pattern as it will match anyway.",
+ None,
"Consider handling `_` separately.",
);
}
}
}
-fn check_match_single_binding(cx: &LateContext<'_, '_>, ex: &Expr<'_>, arms: &[Arm<'_>], expr: &Expr<'_>) {
+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 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,
- expr.span,
+ target_span,
"this match could be written as a `let` statement",
"consider using `let` statement",
- format!(
- "let {} = {};\n{}{}",
- 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_body,
- ),
+ sugg,
applicability,
);
},
}
}
+/// 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>,
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