diff options
46 files changed, 822 insertions, 537 deletions
diff --git a/clippy_lints/src/attrs.rs b/clippy_lints/src/attrs.rs index 3cef8d2a78b..0ce46e1c214 100644 --- a/clippy_lints/src/attrs.rs +++ b/clippy_lints/src/attrs.rs @@ -371,8 +371,8 @@ fn extract_clippy_lint(lint: &NestedMetaItem) -> Option<SymbolStr> { if meta_item.path.segments.len() > 1; if let tool_name = meta_item.path.segments[0].ident; if tool_name.name == sym::clippy; - let lint_name = meta_item.path.segments.last().unwrap().ident.name; then { + let lint_name = meta_item.path.segments.last().unwrap().ident.name; return Some(lint_name.as_str()); } } diff --git a/clippy_lints/src/bytecount.rs b/clippy_lints/src/bytecount.rs index 846ac08e93a..1546d823167 100644 --- a/clippy_lints/src/bytecount.rs +++ b/clippy_lints/src/bytecount.rs @@ -45,52 +45,48 @@ impl<'tcx> LateLintPass<'tcx> for ByteCount { if filter.ident.name == sym!(filter); if filter_args.len() == 2; if let ExprKind::Closure(_, _, body_id, _, _) = filter_args[1].kind; + let body = cx.tcx.hir().body(body_id); + if body.params.len() == 1; + if let Some(argname) = get_pat_name(&body.params[0].pat); + if let ExprKind::Binary(ref op, ref l, ref r) = body.value.kind; + if op.node == BinOpKind::Eq; + if match_type(cx, + cx.typeck_results().expr_ty(&filter_args[0]).peel_refs(), + &paths::SLICE_ITER); then { - let body = cx.tcx.hir().body(body_id); - if_chain! { - if body.params.len() == 1; - if let Some(argname) = get_pat_name(&body.params[0].pat); - if let ExprKind::Binary(ref op, ref l, ref r) = body.value.kind; - if op.node == BinOpKind::Eq; - if match_type(cx, - cx.typeck_results().expr_ty(&filter_args[0]).peel_refs(), - &paths::SLICE_ITER); - then { - let needle = match get_path_name(l) { - Some(name) if check_arg(name, argname, r) => r, - _ => match get_path_name(r) { - Some(name) if check_arg(name, argname, l) => l, - _ => { return; } - } - }; - if ty::Uint(UintTy::U8) != *cx.typeck_results().expr_ty(needle).peel_refs().kind() { - return; - } - let haystack = if let ExprKind::MethodCall(ref path, _, ref args, _) = - filter_args[0].kind { - let p = path.ident.name; - if (p == sym::iter || p == sym!(iter_mut)) && args.len() == 1 { - &args[0] - } else { - &filter_args[0] - } - } else { - &filter_args[0] - }; - let mut applicability = Applicability::MaybeIncorrect; - span_lint_and_sugg( - cx, - NAIVE_BYTECOUNT, - expr.span, - "you appear to be counting bytes the naive way", - "consider using the bytecount crate", - format!("bytecount::count({}, {})", - snippet_with_applicability(cx, haystack.span, "..", &mut applicability), - snippet_with_applicability(cx, needle.span, "..", &mut applicability)), - applicability, - ); + let needle = match get_path_name(l) { + Some(name) if check_arg(name, argname, r) => r, + _ => match get_path_name(r) { + Some(name) if check_arg(name, argname, l) => l, + _ => { return; } } }; + if ty::Uint(UintTy::U8) != *cx.typeck_results().expr_ty(needle).peel_refs().kind() { + return; + } + let haystack = if let ExprKind::MethodCall(ref path, _, ref args, _) = + filter_args[0].kind { + let p = path.ident.name; + if (p == sym::iter || p == sym!(iter_mut)) && args.len() == 1 { + &args[0] + } else { + &filter_args[0] + } + } else { + &filter_args[0] + }; + let mut applicability = Applicability::MaybeIncorrect; + span_lint_and_sugg( + cx, + NAIVE_BYTECOUNT, + expr.span, + "you appear to be counting bytes the naive way", + "consider using the bytecount crate", + format!("bytecount::count({}, {})", + snippet_with_applicability(cx, haystack.span, "..", &mut applicability), + snippet_with_applicability(cx, needle.span, "..", &mut applicability)), + applicability, + ); } }; } diff --git a/clippy_lints/src/checked_conversions.rs b/clippy_lints/src/checked_conversions.rs index ed46cac493a..b38c70bcc41 100644 --- a/clippy_lints/src/checked_conversions.rs +++ b/clippy_lints/src/checked_conversions.rs @@ -321,9 +321,8 @@ fn get_implementing_type<'a>(path: &QPath<'_>, candidates: &'a [&str], function: if path.ident.name.as_str() == function; if let TyKind::Path(QPath::Resolved(None, ref tp)) = &ty.kind; if let [int] = &*tp.segments; - let name = &int.ident.name.as_str(); - then { + let name = &int.ident.name.as_str(); candidates.iter().find(|c| name == *c).cloned() } else { None @@ -336,9 +335,8 @@ fn int_ty_to_sym<'tcx>(path: &QPath<'_>) -> Option<&'tcx str> { if_chain! { if let QPath::Resolved(_, ref path) = *path; if let [ty] = &*path.segments; - let name = &ty.ident.name.as_str(); - then { + let name = &ty.ident.name.as_str(); INTS.iter().find(|c| name == *c).cloned() } else { None diff --git a/clippy_lints/src/collapsible_match.rs b/clippy_lints/src/collapsible_match.rs index e2b3686ddf0..04fff237bb4 100644 --- a/clippy_lints/src/collapsible_match.rs +++ b/clippy_lints/src/collapsible_match.rs @@ -62,8 +62,8 @@ impl<'tcx> LateLintPass<'tcx> for CollapsibleMatch { } fn check_arm<'tcx>(arm: &Arm<'tcx>, wild_outer_arm: &Arm<'tcx>, cx: &LateContext<'tcx>) { + let expr = strip_singleton_blocks(arm.body); if_chain! { - let expr = strip_singleton_blocks(arm.body); if let ExprKind::Match(expr_in, arms_inner, _) = expr.kind; // the outer arm pattern and the inner match if expr_in.span.ctxt() == arm.pat.span.ctxt(); diff --git a/clippy_lints/src/default.rs b/clippy_lints/src/default.rs index 568a174445c..8a2146d93e8 100644 --- a/clippy_lints/src/default.rs +++ b/clippy_lints/src/default.rs @@ -84,22 +84,21 @@ impl LateLintPass<'_> for Default { if match_def_path(cx, def_id, &paths::DEFAULT_TRAIT_METHOD); // Detect and ignore <Foo as Default>::default() because these calls do explicitly name the type. if let QPath::Resolved(None, _path) = qpath; + let expr_ty = cx.typeck_results().expr_ty(expr); + if let ty::Adt(def, ..) = expr_ty.kind(); then { - let expr_ty = cx.typeck_results().expr_ty(expr); - if let ty::Adt(def, ..) = expr_ty.kind() { - // TODO: Work out a way to put "whatever the imported way of referencing - // this type in this file" rather than a fully-qualified type. - let replacement = format!("{}::default()", cx.tcx.def_path_str(def.did)); - span_lint_and_sugg( - cx, - DEFAULT_TRAIT_ACCESS, - expr.span, - &format!("calling `{}` is more clear than this expression", replacement), - "try", - replacement, - Applicability::Unspecified, // First resolve the TODO above - ); - } + // TODO: Work out a way to put "whatever the imported way of referencing + // this type in this file" rather than a fully-qualified type. + let replacement = format!("{}::default()", cx.tcx.def_path_str(def.did)); + span_lint_and_sugg( + cx, + DEFAULT_TRAIT_ACCESS, + expr.span, + &format!("calling `{}` is more clear than this expression", replacement), + "try", + replacement, + Applicability::Unspecified, // First resolve the TODO above + ); } } } @@ -202,14 +201,14 @@ impl LateLintPass<'_> for Default { let binding_type = if_chain! { if let ty::Adt(adt_def, substs) = binding_type.kind(); if !substs.is_empty(); - let adt_def_ty_name = cx.tcx.item_name(adt_def.did); - let generic_args = substs.iter().collect::<Vec<_>>(); - let tys_str = generic_args - .iter() - .map(ToString::to_string) - .collect::<Vec<_>>() - .join(", "); then { + let adt_def_ty_name = cx.tcx.item_name(adt_def.did); + let generic_args = substs.iter().collect::<Vec<_>>(); + let tys_str = generic_args + .iter() + .map(ToString::to_string) + .collect::<Vec<_>>() + .join(", "); format!("{}::<{}>", adt_def_ty_name, &tys_str) } else { binding_type.to_string() diff --git a/clippy_lints/src/default_numeric_fallback.rs b/clippy_lints/src/default_numeric_fallback.rs index d136db9373c..42bafc3442e 100644 --- a/clippy_lints/src/default_numeric_fallback.rs +++ b/clippy_lints/src/default_numeric_fallback.rs @@ -130,8 +130,8 @@ impl<'a, 'tcx> Visitor<'tcx> for NumericFallbackVisitor<'a, 'tcx> { }, ExprKind::Struct(_, fields, base) => { + let ty = self.cx.typeck_results().expr_ty(expr); if_chain! { - let ty = self.cx.typeck_results().expr_ty(expr); if let Some(adt_def) = ty.ty_adt_def(); if adt_def.is_struct(); if let Some(variant) = adt_def.variants.iter().next(); diff --git a/clippy_lints/src/exit.rs b/clippy_lints/src/exit.rs index 635b6d83eab..84fac998662 100644 --- a/clippy_lints/src/exit.rs +++ b/clippy_lints/src/exit.rs @@ -32,16 +32,14 @@ impl<'tcx> LateLintPass<'tcx> for Exit { if let ExprKind::Path(ref path) = path_expr.kind; if let Some(def_id) = cx.qpath_res(path, path_expr.hir_id).opt_def_id(); if match_def_path(cx, def_id, &paths::EXIT); + let parent = cx.tcx.hir().get_parent_item(e.hir_id); + if let Some(Node::Item(Item{kind: ItemKind::Fn(..), ..})) = cx.tcx.hir().find(parent); + // If the next item up is a function we check if it is an entry point + // and only then emit a linter warning + let def_id = cx.tcx.hir().local_def_id(parent); + if !is_entrypoint_fn(cx, def_id.to_def_id()); then { - let parent = cx.tcx.hir().get_parent_item(e.hir_id); - if let Some(Node::Item(Item{kind: ItemKind::Fn(..), ..})) = cx.tcx.hir().find(parent) { - // If the next item up is a function we check if it is an entry point - // and only then emit a linter warning - let def_id = cx.tcx.hir().local_def_id(parent); - if !is_entrypoint_fn(cx, def_id.to_def_id()) { - span_lint(cx, EXIT, e.span, "usage of `process::exit`"); - } - } + span_lint(cx, EXIT, e.span, "usage of `process::exit`"); } } } diff --git a/clippy_lints/src/float_literal.rs b/clippy_lints/src/float_literal.rs index 1ca5c685a75..7968e7b764d 100644 --- a/clippy_lints/src/float_literal.rs +++ b/clippy_lints/src/float_literal.rs @@ -61,8 +61,8 @@ declare_lint_pass!(FloatLiteral => [EXCESSIVE_PRECISION, LOSSY_FLOAT_LITERAL]); impl<'tcx> LateLintPass<'tcx> for FloatLiteral { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) { + let ty = cx.typeck_results().expr_ty(expr); if_chain! { - let ty = cx.typeck_results().expr_ty(expr); if let ty::Float(fty) = *ty.kind(); if let hir::ExprKind::Lit(ref lit) = expr.kind; if let LitKind::Float(sym, lit_float_ty) = lit.node; diff --git a/clippy_lints/src/formatting.rs b/clippy_lints/src/formatting.rs index b10e83c0ea8..48612befc68 100644 --- a/clippy_lints/src/formatting.rs +++ b/clippy_lints/src/formatting.rs @@ -217,9 +217,8 @@ fn check_else(cx: &EarlyContext<'_>, expr: &Expr) { if let Some(else_snippet) = snippet_opt(cx, else_span); if let Some(else_pos) = else_snippet.find("else"); if else_snippet[else_pos..].contains('\n'); - let else_desc = if is_if(else_) { "if" } else { "{..}" }; - then { + let else_desc = if is_if(else_) { "if" } else { "{..}" }; span_lint_and_note( cx, SUSPICIOUS_ELSE_FORMATTING, diff --git a/clippy_lints/src/if_let_mutex.rs b/clippy_lints/src/if_let_mutex.rs index 4aab43256bf..56dfcc6a506 100644 --- a/clippy_lints/src/if_let_mutex.rs +++ b/clippy_lints/src/if_let_mutex.rs @@ -94,13 +94,10 @@ impl<'tcx> Visitor<'tcx> for OppVisitor<'_, 'tcx> { type Map = Map<'tcx>; fn visit_expr(&mut self, expr: &'tcx Expr<'_>) { - if_chain! { - if let Some(mutex) = is_mutex_lock_call(self.cx, expr); - then { - self.found_mutex = Some(mutex); - self.mutex_lock_called = true; - return; - } + if let Some(mutex) = is_mutex_lock_call(self.cx, expr) { + self.found_mutex = Some(mutex); + self.mutex_lock_called = true; + return; } visit::walk_expr(self, expr); } @@ -121,13 +118,10 @@ impl<'tcx> Visitor<'tcx> for ArmVisitor<'_, 'tcx> { type Map = Map<'tcx>; fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) { - if_chain! { - if let Some(mutex) = is_mutex_lock_call(self.cx, expr); - then { - self.found_mutex = Some(mutex); - self.mutex_lock_called = true; - return; - } + if let Some(mutex) = is_mutex_lock_call(self.cx, expr) { + self.found_mutex = Some(mutex); + self.mutex_lock_called = true; + return; } visit::walk_expr(self, expr); } diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 3e0a3e3ef99..f5941977dd1 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -550,6 +550,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: #[cfg(feature = "internal-lints")] &utils::internal_lints::DEFAULT_LINT, #[cfg(feature = "internal-lints")] + &utils::internal_lints::IF_CHAIN_STYLE, + #[cfg(feature = "internal-lints")] &utils::internal_lints::INTERNING_DEFINED_SYMBOL, #[cfg(feature = "internal-lints")] &utils::internal_lints::INVALID_PATHS, @@ -1026,6 +1028,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| box utils::inspector::DeepCodeInspector); store.register_late_pass(|| box utils::internal_lints::CollapsibleCalls); store.register_late_pass(|| box utils::internal_lints::CompilerLintFunctions::new()); + store.register_late_pass(|| box utils::internal_lints::IfChainStyle); store.register_late_pass(|| box utils::internal_lints::InvalidPaths); store.register_late_pass(|| box utils::internal_lints::InterningDefinedSymbol::default()); store.register_late_pass(|| box utils::internal_lints::LintWithoutLintPass::default()); @@ -1442,6 +1445,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&utils::internal_lints::COLLAPSIBLE_SPAN_LINT_CALLS), LintId::of(&utils::internal_lints::COMPILER_LINT_FUNCTIONS), LintId::of(&utils::internal_lints::DEFAULT_LINT), + LintId::of(&utils::internal_lints::IF_CHAIN_STYLE), LintId::of(&utils::internal_lints::INTERNING_DEFINED_SYMBOL), LintId::of(&utils::internal_lints::INVALID_PATHS), LintId::of(&utils::internal_lints::LINT_WITHOUT_LINT_PASS), diff --git a/clippy_lints/src/loops/manual_flatten.rs b/clippy_lints/src/loops/manual_flatten.rs index 8d2b9cccba4..574ad8c0f29 100644 --- a/clippy_lints/src/loops/manual_flatten.rs +++ b/clippy_lints/src/loops/manual_flatten.rs @@ -46,9 +46,8 @@ pub(super) fn check<'tcx>( let some_ctor = is_some_ctor(cx, path.res); let ok_ctor = is_ok_ctor(cx, path.res); if some_ctor || ok_ctor; - let if_let_type = if some_ctor { "Some" } else { "Ok" }; - then { + let if_let_type = if some_ctor { "Some" } else { "Ok" }; // Prepare the error message let msg = format!("unnecessary `if let` since only the `{}` variant of the iterator element is used", if_let_type); diff --git a/clippy_lints/src/loops/manual_memcpy.rs b/clippy_lints/src/loops/manual_memcpy.rs index af6d56a89af..3c5abb7a3c4 100644 --- a/clippy_lints/src/loops/manual_memcpy.rs +++ b/clippy_lints/src/loops/manual_memcpy.rs @@ -61,8 +61,8 @@ pub(super) fn check<'tcx>( if_chain! { if let ExprKind::Index(base_left, idx_left) = lhs.kind; if let ExprKind::Index(base_right, idx_right) = rhs.kind; - if is_slice_like(cx, cx.typeck_results().expr_ty(base_left)) - && is_slice_like(cx, cx.typeck_results().expr_ty(base_right)); + if is_slice_like(cx, cx.typeck_results().expr_ty(base_left)); + if is_slice_like(cx, cx.typeck_results().expr_ty(base_right)); if let Some((start_left, offset_left)) = get_details_from_idx(cx, &idx_left, &starts); if let Some((start_right, offset_right)) = get_details_from_idx(cx, &idx_right, &starts); diff --git a/clippy_lints/src/loops/needless_collect.rs b/clippy_lints/src/loops/needless_collect.rs index 5594fc7b046..d23d3c508fa 100644 --- a/clippy_lints/src/loops/needless_collect.rs +++ b/clippy_lints/src/loops/needless_collect.rs @@ -26,60 +26,59 @@ fn check_needless_collect_direct_usage<'tcx>(expr: &'tcx Expr<'_>, cx: &LateCont if chain_method.ident.name == sym!(collect) && is_trait_method(cx, &args[0], sym::Iterator); if let Some(ref generic_args) = chain_method.args; if let Some(GenericArg::Type(ref ty)) = generic_args.args.get(0); + let ty = cx.typeck_results().node_type(ty.hir_id); + if is_type_diagnostic_item(cx, ty, sym::vec_type) + || is_type_diagnostic_item(cx, ty, sym::vecdeque_type) + || match_type(cx, ty, &paths::BTREEMAP) + || is_type_diagnostic_item(cx, ty, sym::hashmap_type); then { - let ty = cx.typeck_results().node_type(ty.hir_id); - if is_type_diagnostic_item(cx, ty, sym::vec_type) || - is_type_diagnostic_item(cx, ty, sym::vecdeque_type) || - match_type(cx, ty, &paths::BTREEMAP) || - is_type_diagnostic_item(cx, ty, sym::hashmap_type) { - if method.ident.name == sym!(len) { - let span = shorten_needless_collect_span(expr); - span_lint_and_sugg( - cx, - NEEDLESS_COLLECT, - span, - NEEDLESS_COLLECT_MSG, - "replace with", - "count()".to_string(), - Applicability::MachineApplicable, - ); - } - if method.ident.name == sym!(is_empty) { - let span = shorten_needless_collect_span(expr); - span_lint_and_sugg( - cx, - NEEDLESS_COLLECT, - span, - NEEDLESS_COLLECT_MSG, - "replace with", - "next().is_none()".to_string(), - Applicability::MachineApplicable, - ); - } - if method.ident.name == sym!(contains) { - let contains_arg = snippet(cx, args[1].span, "??"); - let span = shorten_needless_collect_span(expr); - span_lint_and_then( - cx, - NEEDLESS_COLLECT, - span, - NEEDLESS_COLLECT_MSG, - |diag| { - let (arg, pred) = contains_arg - .strip_prefix('&') - .map_or(("&x", &*contains_arg), |s| ("x", s)); - diag.span_suggestion( - span, - "replace with", - format!( - "any(|{}| x == {})", - arg, pred - ), - Applicability::MachineApplicable, - ); - } - ); - } + if method.ident.name == sym!(len) { + let span = shorten_needless_collect_span(expr); + span_lint_and_sugg( + cx, + NEEDLESS_COLLECT, + span, + NEEDLESS_COLLECT_MSG, + "replace with", + "count()".to_string(), + Applicability::MachineApplicable, + ); + } + if method.ident.name == sym!(is_empty) { + let span = shorten_needless_collect_span(expr); + span_lint_and_sugg( + cx, + NEEDLESS_COLLECT, + span, + NEEDLESS_COLLECT_MSG, + "replace with", + "next().is_none()".to_string(), + Applicability::MachineApplicable, + ); + } + if method.ident.name == sym!(contains) { + let contains_arg = snippet(cx, args[1].span, "??"); + let span = shorten_needless_collect_span(expr); + span_lint_and_then( + cx, + NEEDLESS_COLLECT, + span, + NEEDLESS_COLLECT_MSG, + |diag| { + let (arg, pred) = contains_arg + .strip_prefix('&') + .map_or(("&x", &*contains_arg), |s| ("x", s)); + diag.span_suggestion( + span, + "replace with", + format!( + "any(|{}| x == {})", + arg, pred + ), + Applicability::MachineApplicable, + ); + } + ); } } } diff --git a/clippy_lints/src/loops/needless_range_loop.rs b/clippy_lints/src/loops/needless_range_loop.rs index 3c40d54cb42..60afa449a45 100644 --- a/clippy_lints/src/loops/needless_range_loop.rs +++ b/clippy_lints/src/loops/needless_range_loop.rs @@ -256,49 +256,47 @@ impl<'a, 'tcx> VarVisitor<'a, 'tcx> { if let ExprKind::Path(ref seqpath) = seqexpr.kind; if let QPath::Resolved(None, ref seqvar) = *seqpath; if seqvar.segments.len() == 1; + let index_used_directly = path_to_local_id(idx, self.var); + let indexed_indirectly = { + let mut used_visitor = LocalUsedVisitor::new(self.cx, self.var); + walk_expr(&mut used_visitor, idx); + used_visitor.used + }; + if indexed_indirectly || index_used_directly; then { - let index_used_directly = path_to_local_id(idx, self.var); - let indexed_indirectly = { - let mut used_visitor = LocalUsedVisitor::new(self.cx, self.var); - walk_expr(&mut used_visitor, idx); - used_visitor.used - }; - - if indexed_indirectly || index_used_directly { - if self.prefer_mutable { - self.indexed_mut.insert(seqvar.segments[0].ident.name); + if self.prefer_mutable { + self.indexed_mut.insert(seqvar.segments[0].ident.name); + } + let res = self.cx.qpath_res(seqpath, seqexpr.hir_id); + match res { + Res::Local(hir_id) => { + let parent_id = self.cx.tcx.hir().get_parent_item(expr.hir_id); + let parent_def_id = self.cx.tcx.hir().local_def_id(parent_id); + let extent = self.cx.tcx.region_scope_tree(parent_def_id).var_scope(hir_id.local_id); + if indexed_indirectly { + self.indexed_indirectly.insert(seqvar.segments[0].ident.name, Some(extent)); + } + if index_used_directly { + self.indexed_directly.insert( + seqvar.segments[0].ident.name, + (Some(extent), self.cx.typeck_results().node_type(seqexpr.hir_id)), + ); + } + return false; // no need to walk further *on the variable* } - let res = self.cx.qpath_res(seqpath, seqexpr.hir_id); - match res { - Res::Local(hir_id) => { - let parent_id = self.cx.tcx.hir().get_parent_item(expr.hir_id); - let parent_def_id = self.cx.tcx.hir().local_def_id(parent_id); - let extent = self.cx.tcx.region_scope_tree(parent_def_id).var_scope(hir_id.local_id); - if indexed_indirectly { - self.indexed_indirectly.insert(seqvar.segments[0].ident.name, Some(extent)); - } - if index_used_directly { - self.indexed_directly.insert( - seqvar.segments[0].ident.name, - (Some(extent), self.cx.typeck_results().node_type(seqexpr.hir_id)), - ); - } - return false; // no need to walk further *on the variable* + Res::Def(DefKind::Static | DefKind::Const, ..) => { + if indexed_indirectly { + self.indexed_indirectly.insert(seqvar.segments[0].ident.name, None); } - Res::Def(DefKind::Static | DefKind::Const, ..) => { - if indexed_indirectly { - self.indexed_indirectly.insert(seqvar.segments[0].ident.name, None); - } - if index_used_directly { - self.indexed_directly.insert( - seqvar.segments[0].ident.name, - (None, self.cx.typeck_results().node_type(seqexpr.hir_id)), - ); - } - return false; // no need to walk further *on the variable* + if index_used_directly { + self.indexed_directly.insert( + seqvar.segments[0].ident.name, + (None, self.cx.typeck_results().node_type(seqexpr.hir_id)), + ); } - _ => (), + return false; // no need to walk further *on the variable* } + _ => (), } } } diff --git a/clippy_lints/src/loops/same_item_push.rs b/clippy_lints/src/loops/same_item_push.rs index 849d7ec718c..62c131968e7 100644 --- a/clippy_lints/src/loops/same_item_push.rs +++ b/clippy_lints/src/loops/same_item_push.rs @@ -63,8 +63,8 @@ pub(super) fn check<'tcx>( match cx.qpath_res(qpath, pushed_item.hir_id) { // immutable bindings that are initialized with literal or constant Res::Local(hir_id) => { + let node = cx.tcx.hir().get(hir_id); if_chain! { - let node = cx.tcx.hir().get(hir_id); if let Node::Binding(pat) = node; if let PatKind::Binding(bind_ann, ..) = pat.kind; if !matches!(bind_ann, BindingAnnotation::RefMut | BindingAnnotation::Mutable); diff --git a/clippy_lints/src/loops/while_immutable_condition.rs b/clippy_lints/src/loops/while_immutable_condition.rs index cad9ff8489a..7d697b2bf00 100644 --- a/clippy_lints/src/loops/while_immutable_condition.rs +++ b/clippy_lints/src/loops/while_immutable_condition.rs @@ -103,9 +103,8 @@ impl<'a, 'tcx> VarCollectorVisitor<'a, 'tcx> { if_chain! { if let ExprKind::Path(ref qpath) = ex.kind; if let QPath::Resolved(None, _) = *qpath; - let res = self.cx.qpath_res(qpath, ex.hir_id); then { - match res { + match self.cx.qpath_res(qpath, ex.hir_id) { Res::Local(hir_id) => { self.ids.insert(hir_id); }, diff --git a/clippy_lints/src/manual_non_exhaustive.rs b/clippy_lints/src/manual_non_exhaustive.rs index 95261580b8e..5a3f0f27bc6 100644 --- a/clippy_lints/src/manual_non_exhaustive.rs +++ b/clippy_lints/src/manual_non_exhaustive.rs @@ -113,8 +113,8 @@ fn check_manual_non_exhaustive_enum(cx: &EarlyContext<'_>, item: &Item, variants } } + let mut markers = variants.iter().filter(|v| is_non_exhaustive_marker(v)); if_chain! { - let mut markers = variants.iter().filter(|v| is_non_exhaustive_marker(v)); if let Some(marker) = markers.next(); if markers.count() == 0 && variants.len() > 1; then { diff --git a/clippy_lints/src/map_clone.rs b/clippy_lints/src/map_clone.rs index cfcc705eabc..9f1ab1f695d 100644 --- a/clippy_lints/src/map_clone.rs +++ b/clippy_lints/src/map_clone.rs @@ -58,9 +58,9 @@ impl<'tcx> LateLintPass<'tcx> for MapClone { let ty = cx.typeck_results().expr_ty(&args[0]); if is_type_diagnostic_item(cx, ty, sym::option_type) || is_trait_method(cx, e, sym::Iterator); if let hir::ExprKind::Closure(_, _, body_id, _, _) = args[1].kind; - let closure_body = cx.tcx.hir().body(body_id); - let closure_expr = remove_blocks(&closure_body.value); then { + let closure_body = cx.tcx.hir().body(body_id); + let closure_expr = remove_blocks(&closure_body.value); match closure_body.params[0].pat.kind { hir::PatKind::Ref(ref inner, hir::Mutability::Not) => if let hir::PatKind::Binding( hir::BindingAnnotation::Unannotated, .., name, None diff --git a/clippy_lints/src/map_unit_fn.rs b/clippy_lints/src/map_unit_fn.rs index d4764b5ccff..5cc16244a0d 100644 --- a/clippy_lints/src/map_unit_fn.rs +++ b/clippy_lints/src/map_unit_fn.rs @@ -168,17 +168,15 @@ fn unit_closure<'tcx>( cx: &LateContext<'tcx>, expr: &hir::Expr<'_>, ) -> Option<(&'tcx hir::Param<'tcx>, &'tcx hir::Expr<'tcx>)> { - if let hir::ExprKind::Closure(_, ref decl, inner_expr_id, _, _) = expr.kind { + if_chain! { + if let hir::ExprKind::Closure(_, ref decl, inner_expr_id, _, _) = expr.kind; let body = cx.tcx.hir().body(inner_expr_id); let body_expr = &body.value; - - if_chain! { - if decl.inputs.len() == 1; - if is_unit_expression(cx, body_expr); - if let Some(binding) = iter_input_pats(&decl, body).next(); - then { - return Some((binding, body_expr)); - } + if decl.inputs.len() == 1; + if is_unit_expression(cx, body_expr); + if let Some(binding) = iter_input_pats(&decl, body).next(); + then { + return Some((binding, body_expr)); } } None diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs index 8c6c30a1881..4a3be67dd75 100644 --- a/clippy_lints/src/matches.rs +++ b/clippy_lints/src/matches.rs @@ -732,8 +732,8 @@ fn report_single_match_single_pattern( format!(" else {}", expr_block(cx, els, None, "..", Some(expr.span))) }); + let (pat, pat_ref_count) = peel_hir_pat_refs(arms[0].pat); 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(); @@ -1480,8 +1480,8 @@ fn check_match_single_binding<'a>(cx: &LateContext<'a>, ex: &Expr<'a>, arms: &[A /// 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>> { + let map = &cx.tcx.hir(); 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 { diff --git a/clippy_lints/src/mem_replace.rs b/clippy_lints/src/mem_replace.rs index 426c108d89f..0418c616efa 100644 --- a/clippy_lints/src/mem_replace.rs +++ b/clippy_lints/src/mem_replace.rs @@ -225,34 +225,33 @@ fn is_default_equivalent_ctor(cx: &LateContext<'_>, def_id: DefId, path: &QPath< } fn check_replace_with_default(cx: &LateContext<'_>, src: &Expr<'_>, dest: &Expr<'_>, expr_span: Span) { - if let ExprKind::Call(ref repl_func, _) = src.kind { - if_chain! { - if !in_external_macro(cx.tcx.sess, expr_span); - if let ExprKind::Path(ref repl_func_qpath) = repl_func.kind; - if let Some(repl_def_id) = cx.qpath_res(repl_func_qpath, repl_func.hir_id).opt_def_id(); - if is_diagnostic_assoc_item(cx, repl_def_id, sym::Default) - || is_default_equivalent_ctor(cx, repl_def_id, repl_func_qpath); + if_chain! { + if let ExprKind::Call(ref repl_func, _) = src.kind; + if !in_external_macro(cx.tcx.sess, expr_span); + if let ExprKind::Path(ref repl_func_qpath) = repl_func.kind; + if let Some(repl_def_id) = cx.qpath_res(repl_func_qpath, repl_func.hir_id).opt_def_id(); + if is_diagnostic_assoc_item(cx, repl_def_id, sym::Default) + || is_default_equivalent_ctor(cx, repl_def_id, repl_func_qpath); - then { - span_lint_and_then( - cx, - MEM_REPLACE_WITH_DEFAULT, - expr_span, - "replacing a value of type `T` with `T::default()` is better expressed using `std::mem::take`", - |diag| { - if !in_macro(expr_span) { - let suggestion = format!("std::mem::take({})", snippet(cx, dest.span, "")); + then { + span_lint_and_then( + cx, + MEM_REPLACE_WITH_DEFAULT, + expr_span, + "replacing a value of type `T` with `T::default()` is better expressed using `std::mem::take`", + |diag| { + if !in_macro(expr_span) { + let suggestion = format!("std::mem::take({})", snippet(cx, dest.span, "")); - diag.span_suggestion( - expr_span, - "consider using", - suggestion, - Applicability::MachineApplicable - ); - } + diag.span_suggestion( + expr_span, + "consider using", + suggestion, + Applicability::MachineApplicable + ); } - ); - } + } + ); } } } diff --git a/clippy_lints/src/methods/bytes_nth.rs b/clippy_lints/src/methods/bytes_nth.rs index 77f140510b6..2ad3e673c57 100644 --- a/clippy_lints/src/methods/bytes_nth.rs +++ b/clippy_lints/src/methods/bytes_nth.rs @@ -1,7 +1,6 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::source::snippet_with_applicability; use clippy_utils::ty::is_type_diagnostic_item; -use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::Expr; use rustc_lint::LateContext; @@ -10,31 +9,26 @@ use rustc_span::sym; use super::BYTES_NTH; pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, recv: &'tcx Expr<'tcx>, n_arg: &'tcx Expr<'tcx>) { - if_chain! { - let ty = cx.typeck_results().expr_ty(recv).peel_refs(); - let caller_type = if is_type_diagnostic_item(cx, ty, sym::string_type) { - Some("String") - } else if ty.is_str() { - Some("str") - } else { - None - }; - if let Some(caller_type) = caller_type; - then { - let mut applicability = Applicability::MachineApplicable; - span_lint_and_sugg( - cx, - BYTES_NTH, - expr.span, - &format!("called `.byte().nth()` on a `{}`", caller_type), - "try", - format!( - "{}.as_bytes().get({})", - snippet_with_applicability(cx, recv.span, "..", &mut applicability), - snippet_with_applicability(cx, n_arg.span, "..", &mut applicability) - ), - applicability, - ); - } - } + let ty = cx.typeck_results().expr_ty(recv).peel_refs(); + let caller_type = if ty.is_str() { + "str" + } else if is_type_diagnostic_item(cx, ty, sym::string_type) { + "String" + } else { + return; + }; + let mut applicability = Applicability::MachineApplicable; + span_lint_and_sugg( + cx, + BYTES_NTH, + expr.span, + &format!("called `.byte().nth()` on a `{}`", caller_type), + "try", + format!( + "{}.as_bytes().get({})", + snippet_with_applicability(cx, recv.span, "..", &mut applicability), + snippet_with_applicability(cx, n_arg.span, "..", &mut applicability) + ), + applicability, + ); } diff --git a/clippy_lints/src/methods/from_iter_instead_of_collect.rs b/clippy_lints/src/methods/from_iter_instead_of_collect.rs index 15cf5674313..707c54f7a3c 100644 --- a/clippy_lints/src/methods/from_iter_instead_of_collect.rs +++ b/clippy_lints/src/methods/from_iter_instead_of_collect.rs @@ -40,8 +40,8 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, args: &[hir::Exp } fn extract_turbofish(cx: &LateContext<'_>, expr: &hir::Expr<'_>, ty: Ty<'tcx>) -> String { + let call_site = expr.span.source_callsite(); if_chain! { - let call_site = expr.span.source_callsite(); if let Ok(snippet) = cx.sess().source_map().span_to_snippet(call_site); let snippet_split = snippet.split("::").collect::<Vec<_>>(); if let Some((_, elements)) = snippet_split.split_last(); diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 9a95354db7a..70acf9f6e73 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -1882,11 +1882,10 @@ impl<'tcx> LateLintPass<'tcx> for Methods { if_chain! { if let TraitItemKind::Fn(ref sig, _) = item.kind; if let Some(first_arg_ty) = sig.decl.inputs.iter().next(); - let first_arg_span = first_arg_ty.span; - let first_arg_ty = hir_ty_to_ty(cx.tcx, first_arg_ty); - let self_ty = TraitRef::identity(cx.tcx, item.def_id.to_def_id()).self_ty(); - then { + let first_arg_span = first_arg_ty.span; + let first_arg_ty = hir_ty_to_ty(cx.tcx, first_arg_ty); + let self_ty = TraitRef::identity(cx.tcx, item.def_id.to_def_id()).self_ty(); wrong_self_convention::check( cx, &item.ident.name.as_str(), diff --git a/clippy_lints/src/methods/unnecessary_filter_map.rs b/clippy_lints/src/methods/unnecessary_filter_map.rs index dac7e039e37..8e637e12393 100644 --- a/clippy_lints/src/methods/unnecessary_filter_map.rs +++ b/clippy_lints/src/methods/unnecessary_filter_map.rs @@ -1,7 +1,6 @@ use clippy_utils::diagnostics::span_lint; use clippy_utils::usage::mutated_variables; use clippy_utils::{is_trait_method, match_qpath, path_to_local_id, paths}; -use if_chain::if_chain; use rustc_hir as hir; use rustc_hir::intravisit::{walk_expr, NestedVisitorMap, Visitor}; use rustc_lint::LateContext; @@ -54,18 +53,15 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, arg: &hir::Expr< fn check_expression<'tcx>(cx: &LateContext<'tcx>, arg_id: hir::HirId, expr: &'tcx hir::Expr<'_>) -> (bool, bool) { match &expr.kind { hir::ExprKind::Call(ref func, ref args) => { - if_chain! { - if let hir::ExprKind::Path(ref path) = func.kind; - then { - if match_qpath(path, &paths::OPTION_SOME) { - if path_to_local_id(&args[0], arg_id) { - return (false, false) - } - return (true, false); + if let hir::ExprKind::Path(ref path) = func.kind { + if match_qpath(path, &paths::OPTION_SOME) { + if path_to_local_id(&args[0], arg_id) { + return (false, false); } - // We don't know. It might do anything. - return (true, true); + return (true, false); } + // We don't know. It might do anything. + return (true, true); } (true, true) }, diff --git a/clippy_lints/src/misc.rs b/clippy_lints/src/misc.rs index 026ea50936a..c23643cb2f5 100644 --- a/clippy_lints/src/misc.rs +++ b/clippy_lints/src/misc.rs @@ -308,46 +308,45 @@ impl<'tcx> LateLintPass<'tcx> for MiscLints { if let PatKind::Binding(an, .., name, None) = local.pat.kind; if let Some(ref init) = local.init; if !higher::is_from_for_desugar(local); + if an == BindingAnnotation::Ref || an == BindingAnnotation::RefMut; then { - if an == BindingAnnotation::Ref || an == BindingAnnotation::RefMut { - // use the macro callsite when the init span (but not the whole local span) - // comes from an expansion like `vec![1, 2, 3]` in `let ref _ = vec![1, 2, 3];` - let sugg_init = if init.span.from_expansion() && !local.span.from_expansion() { - Sugg::hir_with_macro_callsite(cx, init, "..") - } else { - Sugg::hir(cx, init, "..") - }; - let (mutopt, initref) = if an == BindingAnnotation::RefMut { - ("mut ", sugg_init.mut_addr()) - } else { - ("", sugg_init.addr()) - }; - let tyopt = if let Some(ref ty) = local.ty { - format!(": &{mutopt}{ty}", mutopt=mutopt, ty=snippet(cx, ty.span, "..")) - } else { - String::new() - }; - span_lint_hir_and_then( - cx, - TOPLEVEL_REF_ARG, - init.hir_id, - local.pat.span, - "`ref` on an entire `let` pattern is discouraged, take a reference with `&` instead", - |diag| { - diag.span_suggestion( - stmt.span, - "try", - format!( - "let {name}{tyopt} = {initref};", - name=snippet(cx, name.span, ".."), - tyopt=tyopt, - initref=initref, - ), - Applicability::MachineApplicable, - ); - } - ); - } + // use the macro callsite when the init span (but not the whole local span) + // comes from an expansion like `vec![1, 2, 3]` in `let ref _ = vec![1, 2, 3];` + let sugg_init = if init.span.from_expansion() && !local.span.from_expansion() { + Sugg::hir_with_macro_callsite(cx, init, "..") + } else { + Sugg::hir(cx, init, "..") + }; + let (mutopt, initref) = if an == BindingAnnotation::RefMut { + ("mut ", sugg_init.mut_addr()) + } else { + ("", sugg_init.addr()) + }; + let tyopt = if let Some(ref ty) = local.ty { + format!(": &{mutopt}{ty}", mutopt=mutopt, ty=snippet(cx, ty.span, "..")) + } else { + String::new() + }; + span_lint_hir_and_then( + cx, + TOPLEVEL_REF_ARG, + init.hir_id, + local.pat.span, + "`ref` on an entire `let` pattern is discouraged, take a reference with `&` instead", + |diag| { + diag.span_suggestion( + stmt.span, + "try", + format!( + "let {name}{tyopt} = {initref};", + name=snippet(cx, name.span, ".."), + tyopt=tyopt, + initref=initref, + ), + Applicability::MachineApplicable, + ); + } + ); } }; if_chain! { @@ -462,21 +461,18 @@ fn check_nan(cx: &LateContext<'_>, expr: &Expr<'_>, cmp_expr: &Expr<'_>) { if_chain! { if !in_constant(cx, cmp_expr.hir_id); if let Some((value, _)) = constant(cx, cx.typeck_results(), expr); + if match value { + Constant::F32(num) => num.is_nan(), + Constant::F64(num) => num.is_nan(), + _ => false, + }; then { - let needs_lint = match value { - Constant::F32(num) => num.is_nan(), - Constant::F64(num) => num.is_nan(), - _ => false, - }; - - if needs_lint { - span_lint( - cx, - CMP_NAN, - cmp_expr.span, - "doomed comparison with `NAN`, use `{f32,f64}::is_nan()` instead", - ); - } + span_lint( + cx, + CMP_NAN, + cmp_expr.span, + "doomed comparison with `NAN`, use `{f32,f64}::is_nan()` instead", + ); } } } diff --git a/clippy_lints/src/new_without_default.rs b/clippy_lints/src/new_without_default.rs index 3789572ad43..a5f91eb035f 100644 --- a/clippy_lints/src/new_without_default.rs +++ b/clippy_lints/src/new_without_default.rs @@ -97,60 +97,60 @@ impl<'tcx> LateLintPass<'tcx> for NewWithoutDefault { // impl of `Default` return; } - if sig.decl.inputs.is_empty() && name == sym::new && cx.access_levels.is_reachable(id) { + if_chain! { + if sig.decl.inputs.is_empty(); + if name == sym::new; + if cx.access_levels.is_reachable(id); let self_def_id = cx.tcx.hir().local_def_id(cx.tcx.hir().get_parent_item(id)); let self_ty = cx.tcx.type_of(self_def_id); - if_chain! { - if TyS::same_type(self_ty, return_ty(cx, id)); - if let Some(default_trait_id) = get_trait_def_id(cx, &paths::DEFAULT_TRAIT); - then { - if self.impling_types.is_none() { - let mut impls = HirIdSet::default(); - cx.tcx.for_each_impl(default_trait_id, |d| { - if let Some(ty_def) = cx.tcx.type_of(d).ty_adt_def() { - if let Some(local_def_id) = ty_def.did.as_local() { - impls.insert(cx.tcx.hir().local_def_id_to_hir_id(local_def_id)); - } - } - }); - self.impling_types = Some(impls); - } - - // Check if a Default implementation exists for the Self type, regardless of - // generics - if_chain! { - if let Some(ref impling_types) = self.impling_types; - if let Some(self_def) = cx.tcx.type_of(self_def_id).ty_adt_def(); - if let Some(self_local_did) = self_def.did.as_local(); - then { - let self_id = cx.tcx.hir().local_def_id_to_hir_id(self_local_did); - if impling_types.contains(&self_id) { - return; + if TyS::same_type(self_ty, return_ty(cx, id)); + if let Some(default_trait_id) = get_trait_def_id(cx, &paths::DEFAULT_TRAIT); + then { + if self.impling_types.is_none() { + let mut impls = HirIdSet::default(); + cx.tcx.for_each_impl(default_trait_id, |d| { + if let Some(ty_def) = cx.tcx.type_of(d).ty_adt_def() { + if let Some(local_def_id) = ty_def.did.as_local() { + impls.insert(cx.tcx.hir().local_def_id_to_hir_id(local_def_id)); } } - } + }); + self.impling_types = Some(impls); + } - let generics_sugg = snippet(cx, generics.span, ""); - span_lint_hir_and_then( - cx, - NEW_WITHOUT_DEFAULT, - id, - impl_item.span, - &format!( - "you should consider adding a `Default` implementation for `{}`", - self_ty - ), - |diag| { - diag.suggest_prepend_item( - cx, - item.span, - "try this", - &create_new_without_default_suggest_msg(self_ty, &generics_sugg), - Applicability::MaybeIncorrect, - ); - }, - ); + // Check if a Default implementation exists for the Self type, regardless of + // generics + if_chain! { + if let Some(ref impling_types) = self.impling_types; + if let Some(self_def) = cx.tcx.type_of(self_def_id).ty_adt_def(); + if let Some(self_local_did) = self_def.did.as_local(); + let self_id = cx.tcx.hir().local_def_id_to_hir_id(self_local_did); + if impling_types.contains(&self_id); + then { + return; + } } + + let generics_sugg = snippet(cx, generics.span, ""); + span_lint_hir_and_then( + cx, + NEW_WITHOUT_DEFAULT, + id, + impl_item.span, + &format!( + "you should consider adding a `Default` implementation for `{}`", + self_ty + ), + |diag| { + diag.suggest_prepend_item( + cx, + item.span, + "try this", + &create_new_without_default_suggest_msg(self_ty, &generics_sugg), + Applicability::MaybeIncorrect, + ); + }, + ); } } } diff --git a/clippy_lints/src/non_copy_const.rs b/clippy_lints/src/non_copy_const.rs index aa1d8fbe300..d775cd7c7f7 100644 --- a/clippy_lints/src/non_copy_const.rs +++ b/clippy_lints/src/non_copy_const.rs @@ -307,19 +307,17 @@ impl<'tcx> LateLintPass<'tcx> for NonCopyConst { // we should use here as a frozen variant is a potential to be frozen // similar to unknown layouts. // e.g. `layout_of(...).is_err() || has_frozen_variant(...);` + let ty = hir_ty_to_ty(cx.tcx, hir_ty); + let normalized = cx.tcx.normalize_erasing_regions(cx.param_env, ty); + if is_unfrozen(cx, normalized); + if is_value_unfrozen_poly(cx, *body_id, normalized); then { - let ty = hir_ty_to_ty(cx.tcx, hir_ty); - let normalized = cx.tcx.normalize_erasing_regions(cx.param_env, ty); - if is_unfrozen(cx, normalized) - && is_value_unfrozen_poly(cx, *body_id, normalized) - { - lint( - cx, - Source::Assoc { - item: impl_item.span, - }, - ); - } + lint( + cx, + Source::Assoc { + item: impl_item.span, + }, + ); } } }, diff --git a/clippy_lints/src/panic_unimplemented.rs b/clippy_lints/src/panic_unimplemented.rs index d06e7f8fe1e..1e946858947 100644 --- a/clippy_lints/src/panic_unimplemented.rs +++ b/clippy_lints/src/panic_unimplemented.rs @@ -97,11 +97,10 @@ impl<'tcx> LateLintPass<'tcx> for PanicUnimplemented { fn get_outer_span(expr: &Expr<'_>) -> Span { if_chain! { if expr.span.from_expansion(); - let first = expr.span.ctxt().outer_expn_data(); - if first.call_site.from_expansion(); - let second = first.call_site.ctxt().outer_expn_data(); + let first = expr.span.ctxt().outer_expn_data().call_site; + if first.from_expansion(); then { - second.call_site + first.ctxt().outer_expn_data().call_site } else { expr.span } diff --git a/clippy_lints/src/ptr.rs b/clippy_lints/src/ptr.rs index be686b1b0cd..6e530d0ffb0 100644 --- a/clippy_lints/src/ptr.rs +++ b/clippy_lints/src/ptr.rs @@ -271,19 +271,18 @@ fn check_fn(cx: &LateContext<'_>, decl: &FnDecl<'_>, fn_id: HirId, opt_body_id: GenericArg::Type(ty) => Some(ty), _ => None, }); + let replacement = snippet_opt(cx, inner.span); + if let Some(r) = replacement; then { - let replacement = snippet_opt(cx, inner.span); - if let Some(r) = replacement { - span_lint_and_sugg( - cx, - PTR_ARG, - arg.span, - "using a reference to `Cow` is not recommended", - "change this to", - "&".to_owned() + &r, - Applicability::Unspecified, - ); - } + span_lint_and_sugg( + cx, + PTR_ARG, + arg.span, + "using a reference to `Cow` is not recommended", + "change this to", + "&".to_owned() + &r, + Applicability::Unspecified, + ); } } } diff --git a/clippy_lints/src/ranges.rs b/clippy_lints/src/ranges.rs index 95b21489eb5..79692abb6ac 100644 --- a/clippy_lints/src/ranges.rs +++ b/clippy_lints/src/ranges.rs @@ -320,32 +320,29 @@ fn match_ident(e: &Expr<'_>) -> Option<Ident> { } fn check_range_zip_with_len(cx: &LateContext<'_>, path: &PathSegment<'_>, args: &[Expr<'_>], span: Span) { - let name = path.ident.as_str(); - if name == "zip" && args.len() == 2 { - let iter = &args[0].kind; - let zip_arg = &args[1]; - if_chain! { - // `.iter()` call - if let ExprKind::MethodCall(ref iter_path, _, ref iter_args, _) = *iter; - if iter_path.ident.name == sym::iter; - // range expression in `.zip()` call: `0..x.len()` - if let Some(higher::Range { start: Some(start), end: Some(end), .. }) = higher::range(zip_arg); - if is_integer_const(cx, start, 0); - // `.len()` call - if let ExprKind::MethodCall(ref len_path, _, ref len_args, _) = end.kind; - if len_path.ident.name == sym!(len) && len_args.len() == 1; - // `.iter()` and `.len()` called on same `Path` - if let ExprKind::Path(QPath::Resolved(_, ref iter_path)) = iter_args[0].kind; - if let ExprKind::Path(QPath::Resolved(_, ref len_path)) = len_args[0].kind; - if SpanlessEq::new(cx).eq_path_segments(&iter_path.segments, &len_path.segments); - then { - span_lint(cx, - RANGE_ZIP_WITH_LEN, - span, - &format!("it is more idiomatic to use `{}.iter().enumerate()`", - snippet(cx, iter_args[0].span, "_")) - ); - } + if_chain! { + if path.ident.as_str() == "zip"; + if let [iter, zip_arg] = args; + // `.iter()` call + if let ExprKind::MethodCall(ref iter_path, _, ref iter_args, _) = iter.kind; + if iter_path.ident.name == sym::iter; + // range expression in `.zip()` call: `0..x.len()` + if let Some(higher::Range { start: Some(start), end: Some(end), .. }) = higher::range(zip_arg); + if is_integer_const(cx, start, 0); + // `.len()` call + if let ExprKind::MethodCall(ref len_path, _, ref len_args, _) = end.kind; + if len_path.ident.name == sym!(len) && len_args.len() == 1; + // `.iter()` and `.len()` called on same `Path` + if let ExprKind::Path(QPath::Resolved(_, ref iter_path)) = iter_args[0].kind; + if let ExprKind::Path(QPath::Resolved(_, ref len_path)) = len_args[0].kind; + if SpanlessEq::new(cx).eq_path_segments(&iter_path.segments, &len_path.segments); + then { + span_lint(cx, + RANGE_ZIP_WITH_LEN, + span, + &format!("it is more idiomatic to use `{}.iter().enumerate()`", + snippet(cx, iter_args[0].span, "_")) + ); } } } diff --git a/clippy_lints/src/redundant_closure_call.rs b/clippy_lints/src/redundant_closure_call.rs index 2977a108d14..5429d389610 100644 --- a/clippy_lints/src/redundant_closure_call.rs +++ b/clippy_lints/src/redundant_closure_call.rs @@ -113,8 +113,8 @@ impl<'tcx> LateLintPass<'tcx> for RedundantClosureCall { if_chain! { if let hir::ExprKind::Call(ref closure, _) = expr.kind; if let hir::ExprKind::Path(hir::QPath::Resolved(_, ref path)) = closure.kind; - if self.path.segments[0].ident == path.segments[0].ident - && self.path.res == path.res; + if self.path.segments[0].ident == path.segments[0].ident; + if self.path.res == path.res; then { self.count += 1; } diff --git a/clippy_lints/src/suspicious_trait_impl.rs b/clippy_lints/src/suspicious_trait_impl.rs index 99e3d818b79..512abde11a6 100644 --- a/clippy_lints/src/suspicious_trait_impl.rs +++ b/clippy_lints/src/suspicious_trait_impl.rs @@ -68,14 +68,13 @@ impl<'tcx> LateLintPass<'tcx> for SuspiciousImpl { // Check for more than one binary operation in the implemented function // Linting when multiple operations are involved can result in false positives + let parent_fn = cx.tcx.hir().get_parent_item(expr.hir_id); if_chain! { - let parent_fn = cx.tcx.hir().get_parent_item(expr.hir_id); if let hir::Node::ImplItem(impl_item) = cx.tcx.hir().get(parent_fn); if let hir::ImplItemKind::Fn(_, body_id) = impl_item.kind; - let body = cx.tcx.hir().body(body_id); - let mut visitor = BinaryExprVisitor { nb_binops: 0 }; - then { + let body = cx.tcx.hir().body(body_id); + let mut visitor = BinaryExprVisitor { nb_binops: 0 }; walk_expr(&mut visitor, &body.value); if visitor.nb_binops > 1 { return; diff --git a/clippy_lints/src/transmute/transmute_float_to_int.rs b/clippy_lints/src/transmute/transmute_float_to_int.rs index 72489f27cd3..1a6124e9ddb 100644 --- a/clippy_lints/src/transmute/transmute_float_to_int.rs +++ b/clippy_lints/src/transmute/transmute_float_to_int.rs @@ -36,10 +36,10 @@ pub(super) fn check<'tcx>( if_chain! { // if the expression is a float literal and it is unsuffixed then // add a suffix so the suggestion is valid and unambiguous - let op = format!("{}{}", arg, float_ty.name_str()).into(); if let ExprKind::Lit(lit) = &expr.kind; if let ast::LitKind::Float(_, ast::LitFloatType::Unsuffixed) = lit.node; then { + let op = format!("{}{}", arg, float_ty.name_str()).into(); match arg { sugg::Sugg::MaybeParen(_) => arg = sugg::Sugg::MaybeParen(op), _ => arg = sugg::Sugg::NonParen(op) diff --git a/clippy_lints/src/try_err.rs b/clippy_lints/src/try_err.rs index e61058c2749..e4799790d35 100644 --- a/clippy_lints/src/try_err.rs +++ b/clippy_lints/src/try_err.rs @@ -138,9 +138,8 @@ fn result_error_type<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Option<Ty<'t if_chain! { if let ty::Adt(_, subst) = ty.kind(); if is_type_diagnostic_item(cx, ty, sym::result_type); - let err_ty = subst.type_at(1); then { - Some(err_ty) + Some(subst.type_at(1)) } else { None } @@ -156,10 +155,8 @@ fn poll_result_error_type<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Option< if let ty::Adt(ready_def, ready_subst) = ready_ty.kind(); if cx.tcx.is_diagnostic_item(sym::result_type, ready_def.did); - let err_ty = ready_subst.type_at(1); - then { - Some(err_ty) + Some(ready_subst.type_at(1)) } else { None } @@ -179,10 +176,8 @@ fn poll_option_result_error_type<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> if let ty::Adt(some_def, some_subst) = some_ty.kind(); if cx.tcx.is_diagnostic_item(sym::result_type, some_def.did); - let err_ty = some_subst.type_at(1); - then { - Some(err_ty) + Some(some_subst.type_at(1)) } else { None } diff --git a/clippy_lints/src/unit_return_expecting_ord.rs b/clippy_lints/src/unit_return_expecting_ord.rs index bad3e488bb6..cdc65abe47c 100644 --- a/clippy_lints/src/unit_return_expecting_ord.rs +++ b/clippy_lints/src/unit_return_expecting_ord.rs @@ -116,8 +116,8 @@ fn check_arg<'tcx>(cx: &LateContext<'tcx>, arg: &'tcx Expr<'tcx>) -> Option<(Spa let ty = cx.tcx.erase_late_bound_regions(ret_ty); if ty.is_unit(); then { + let body = cx.tcx.hir().body(body_id); if_chain! { - let body = cx.tcx.hir().body(body_id); if let ExprKind::Block(block, _) = body.value.kind; if block.expr.is_none(); if let Some(stmt) = block.stmts.last(); diff --git a/clippy_lints/src/unit_types/unit_arg.rs b/clippy_lints/src/unit_types/unit_arg.rs index 925ab577099..f77d811c283 100644 --- a/clippy_lints/src/unit_types/unit_arg.rs +++ b/clippy_lints/src/unit_types/unit_arg.rs @@ -19,9 +19,9 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>) { if is_questionmark_desugar_marked_call(expr) { return; } + let map = &cx.tcx.hir(); + let opt_parent_node = map.find(map.get_parent_node(expr.hir_id)); if_chain! { - let map = &cx.tcx.hir(); - let opt_parent_node = map.find(map.get_parent_node(expr.hir_id)); if let Some(hir::Node::Expr(parent_expr)) = opt_parent_node; if is_questionmark_desugar_marked_call(parent_expr); then { diff --git a/clippy_lints/src/unnamed_address.rs b/clippy_lints/src/unnamed_address.rs index d5bc3de6698..9ceb2c3ffe2 100644 --- a/clippy_lints/src/unnamed_address.rs +++ b/clippy_lints/src/unnamed_address.rs @@ -116,8 +116,8 @@ impl LateLintPass<'_> for UnnamedAddress { if_chain! { if let ExprKind::Binary(binop, ref left, ref right) = expr.kind; if is_comparison(binop.node); - if cx.typeck_results().expr_ty_adjusted(left).is_fn_ptr() && - cx.typeck_results().expr_ty_adjusted(right).is_fn_ptr(); + if cx.typeck_results().expr_ty_adjusted(left).is_fn_ptr(); + if cx.typeck_results().expr_ty_adjusted(right).is_fn_ptr(); if is_fn_def(cx, left) || is_fn_def(cx, right); then { span_lint( diff --git a/clippy_lints/src/unused_self.rs b/clippy_lints/src/unused_self.rs index aef4ce75915..15343cf90f2 100644 --- a/clippy_lints/src/unused_self.rs +++ b/clippy_lints/src/unused_self.rs @@ -49,21 +49,18 @@ impl<'tcx> LateLintPass<'tcx> for UnusedSelf { if assoc_item.fn_has_self_parameter; if let ImplItemKind::Fn(.., body_id) = &impl_item.kind; let body = cx.tcx.hir().body(*body_id); - if !body.params.is_empty(); + if let [self_param, ..] = body.params; + let self_hir_id = self_param.pat.hir_id; + if !LocalUsedVisitor::new(cx, self_hir_id).check_body(body); then { - let self_param = &body.params[0]; - let self_hir_id = self_param.pat.hir_id; - if !LocalUsedVisitor::new(cx, self_hir_id).check_body(body) { - span_lint_and_help( - cx, - UNUSED_SELF, - self_param.span, - "unused `self` argument", - None, - "consider refactoring to a associated function", - ); - return; - } + span_lint_and_help( + cx, + UNUSED_SELF, + self_param.span, + "unused `self` argument", + None, + "consider refactoring to a associated function", + ); } } } diff --git a/clippy_lints/src/unwrap_in_result.rs b/clippy_lints/src/unwrap_in_result.rs index 0d745813beb..d17aa6d8424 100644 --- a/clippy_lints/src/unwrap_in_result.rs +++ b/clippy_lints/src/unwrap_in_result.rs @@ -110,31 +110,27 @@ impl<'a, 'tcx> Visitor<'tcx> for FindExpectUnwrap<'a, 'tcx> { } fn lint_impl_body<'tcx>(cx: &LateContext<'tcx>, impl_span: Span, impl_item: &'tcx hir::ImplItem<'_>) { - if_chain! { + if let ImplItemKind::Fn(_, body_id) = impl_item.kind { + let body = cx.tcx.hir().body(body_id); + let mut fpu = FindExpectUnwrap { + lcx: cx, + typeck_results: cx.tcx.typeck(impl_item.def_id), + result: Vec::new(), + }; + fpu.visit_expr(&body.value); - if let ImplItemKind::Fn(_, body_id) = impl_item.kind; - then { - let body = cx.tcx.hir().body(body_id); - let mut fpu = FindExpectUnwrap { - lcx: cx, - typeck_results: cx.tcx.typeck(impl_item.def_id), - result: Vec::new(), - }; - fpu.visit_expr(&body.value); - - // if we've found one, lint - if !fpu.result.is_empty() { - span_lint_and_then( - cx, - UNWRAP_IN_RESULT, - impl_span, - "used unwrap or expect in a function that returns result or option", - move |diag| { - diag.help( - "unwrap and expect should not be used in a function that returns result or option" ); - diag.span_note(fpu.result, "potential non-recoverable error(s)"); - }); - } + // if we've found one, lint + if !fpu.result.is_empty() { + span_lint_and_then( + cx, + UNWRAP_IN_RESULT, + impl_span, + "used unwrap or expect in a function that returns result or option", + move |diag| { + diag.help("unwrap and expect should not be used in a function that returns result or option"); + diag.span_note(fpu.result, "potential non-recoverable error(s)"); + }, + ); } } } diff --git a/clippy_lints/src/useless_conversion.rs b/clippy_lints/src/useless_conversion.rs index 3e1b69e676b..d893b271a20 100644 --- a/clippy_lints/src/useless_conversion.rs +++ b/clippy_lints/src/useless_conversion.rs @@ -103,25 +103,23 @@ impl<'tcx> LateLintPass<'tcx> for UselessConversion { ); } } - if match_trait_method(cx, e, &paths::TRY_INTO_TRAIT) && &*name.ident.as_str() == "try_into" { - if_chain! { - let a = cx.typeck_results().expr_ty(e); - let b = cx.typeck_results().expr_ty(&args[0]); - if is_type_diagnostic_item(cx, a, sym::result_type); - if let ty::Adt(_, substs) = a.kind(); - if let Some(a_type) = substs.types().next(); - if TyS::same_type(a_type, b); - - then { - span_lint_and_help( - cx, - USELESS_CONVERSION, - e.span, - &format!("useless conversion to the same type: `{}`", b), - None, - "consider removing `.try_into()`", - ); - } + if_chain! { + if match_trait_method(cx, e, &paths::TRY_INTO_TRAIT) && &*name.ident.as_str() == "try_into"; + let a = cx.typeck_results().expr_ty(e); + let b = cx.typeck_results().expr_ty(&args[0]); + if is_type_diagnostic_item(cx, a, sym::result_type); + if let ty::Adt(_, substs) = a.kind(); + if let Some(a_type) = substs.types().next(); + if TyS::same_type(a_type, b); + then { + span_lint_and_help( + cx, + USELESS_CONVERSION, + e.span, + &format!("useless conversion to the same type: `{}`", b), + None, + "consider removing `.try_into()`", + ); } } }, @@ -131,10 +129,9 @@ impl<'tcx> LateLintPass<'tcx> for UselessConversion { if args.len() == 1; if let ExprKind::Path(ref qpath) = path.kind; if let Some(def_id) = cx.qpath_res(qpath, path.hir_id).opt_def_id(); - let a = cx.typeck_results().expr_ty(e); - let b = cx.typeck_results().expr_ty(&args[0]); - then { + let a = cx.typeck_results().expr_ty(e); + let b = cx.typeck_results().expr_ty(&args[0]); if_chain! { if match_def_path(cx, def_id, &paths::TRY_FROM); if is_type_diagnostic_item(cx, a, sym::result_type); diff --git a/clippy_lints/src/utils/internal_lints.rs b/clippy_lints/src/utils/internal_lints.rs index c496ff1fb24..d5a13c135b1 100644 --- a/clippy_lints/src/utils/internal_lints.rs +++ b/clippy_lints/src/utils/internal_lints.rs @@ -1,8 +1,10 @@ use crate::consts::{constant_simple, Constant}; -use clippy_utils::diagnostics::{span_lint, span_lint_and_help, span_lint_and_sugg}; +use clippy_utils::diagnostics::{span_lint, span_lint_and_help, span_lint_and_sugg, span_lint_and_then}; use clippy_utils::source::snippet; use clippy_utils::ty::match_type; -use clippy_utils::{is_expn_of, match_def_path, match_qpath, method_calls, path_to_res, paths, run_lints, SpanlessEq}; +use clippy_utils::{ + is_else_clause, is_expn_of, match_def_path, match_qpath, method_calls, path_to_res, paths, run_lints, SpanlessEq, +}; use if_chain::if_chain; use rustc_ast::ast::{Crate as AstCrate, ItemKind, LitKind, ModKind, NodeId}; use rustc_ast::visit::FnKind; @@ -14,15 +16,17 @@ use rustc_hir::def_id::DefId; use rustc_hir::hir_id::CRATE_HIR_ID; use rustc_hir::intravisit::{NestedVisitorMap, Visitor}; use rustc_hir::{ - BinOpKind, Crate, Expr, ExprKind, HirId, Item, MutTy, Mutability, Node, Path, StmtKind, Ty, TyKind, UnOp, + BinOpKind, Block, Crate, Expr, ExprKind, HirId, Item, Local, MatchSource, MutTy, Mutability, Node, Path, Stmt, + StmtKind, Ty, TyKind, UnOp, }; -use rustc_lint::{EarlyContext, EarlyLintPass, LateContext, LateLintPass}; +use rustc_lint::{EarlyContext, EarlyLintPass, LateContext, LateLintPass, LintContext}; use rustc_middle::hir::map::Map; use rustc_middle::mir::interpret::ConstValue; use rustc_middle::ty; use rustc_session::{declare_lint_pass, declare_tool_lint, impl_lint_pass}; -use rustc_span::source_map::{Span, Spanned}; +use rustc_span::source_map::Spanned; use rustc_span::symbol::{Symbol, SymbolStr}; +use rustc_span::{BytePos, Span}; use rustc_typeck::hir_ty_to_ty; use std::borrow::{Borrow, Cow}; @@ -297,6 +301,13 @@ declare_clippy_lint! { "unnecessary conversion between Symbol and string" } +declare_clippy_lint! { + /// Finds unidiomatic usage of `if_chain!` + pub IF_CHAIN_STYLE, + internal, + "non-idiomatic `if_chain!` usage" +} + declare_lint_pass!(ClippyLintsInternal => [CLIPPY_LINTS_INTERNAL]); impl EarlyLintPass for ClippyLintsInternal { @@ -577,9 +588,9 @@ impl<'tcx> LateLintPass<'tcx> for CollapsibleCalls { if stmts.len() == 1 && block.expr.is_none(); if let StmtKind::Semi(only_expr) = &stmts[0].kind; if let ExprKind::MethodCall(ref ps, _, ref span_call_args, _) = &only_expr.kind; - let and_then_snippets = get_and_then_snippets(cx, and_then_args); - let mut sle = SpanlessEq::new(cx).deny_side_effects(); then { + let and_then_snippets = get_and_then_snippets(cx, and_then_args); + let mut sle = SpanlessEq::new(cx).deny_side_effects(); match &*ps.ident.as_str() { "span_suggestion" if sle.eq_expr(&and_then_args[2], &span_call_args[1]) => { suggest_suggestion(cx, expr, &and_then_snippets, &span_suggestion_snippets(cx, span_call_args)); @@ -1063,3 +1074,159 @@ impl<'tcx> SymbolStrExpr<'tcx> { } } } + +declare_lint_pass!(IfChainStyle => [IF_CHAIN_STYLE]); + +impl<'tcx> LateLintPass<'tcx> for IfChainStyle { + fn check_block(&mut self, cx: &LateContext<'tcx>, block: &'tcx hir::Block<'_>) { + let (local, after, if_chain_span) = if_chain! { + if let [Stmt { kind: StmtKind::Local(local), .. }, after @ ..] = block.stmts; + if let Some(if_chain_span) = is_expn_of(block.span, "if_chain"); + then { (local, after, if_chain_span) } else { return } + }; + if is_first_if_chain_expr(cx, block.hir_id, if_chain_span) { + span_lint( + cx, + IF_CHAIN_STYLE, + if_chain_local_span(cx, local, if_chain_span), + "`let` expression should be above the `if_chain!`", + ); + } else if local.span.ctxt() == block.span.ctxt() && is_if_chain_then(after, block.expr, if_chain_span) { + span_lint( + cx, + IF_CHAIN_STYLE, + if_chain_local_span(cx, local, if_chain_span), + "`let` expression should be inside `then { .. }`", + ) + } + } + + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) { + let (cond, then, els) = match expr.kind { + ExprKind::If(cond, then, els) => (Some(cond), then, els.is_some()), + ExprKind::Match( + _, + [arm, ..], + MatchSource::IfLetDesugar { + contains_else_clause: els, + }, + ) => (None, arm.body, els), + _ => return, + }; + let then_block = match then.kind { + ExprKind::Block(block, _) => block, + _ => return, + }; + let if_chain_span = is_expn_of(expr.span, "if_chain"); + if !els { + check_nested_if_chains(cx, expr, then_block, if_chain_span); + } + let if_chain_span = match if_chain_span { + None => return, + Some(span) => span, + }; + // check for `if a && b;` + if_chain! { + if let Some(cond) = cond; + if let ExprKind::Binary(op, _, _) = cond.kind; + if op.node == BinOpKind::And; + if cx.sess().source_map().is_multiline(cond.span); + then { + span_lint(cx, IF_CHAIN_STYLE, cond.span, "`if a && b;` should be `if a; if b;`"); + } + } + if is_first_if_chain_expr(cx, expr.hir_id, if_chain_span) + && is_if_chain_then(then_block.stmts, then_block.expr, if_chain_span) + { + span_lint(cx, IF_CHAIN_STYLE, expr.span, "`if_chain!` only has one `if`") + } + } +} + +fn check_nested_if_chains( + cx: &LateContext<'_>, + if_expr: &Expr<'_>, + then_block: &Block<'_>, + if_chain_span: Option<Span>, +) { + #[rustfmt::skip] + let (head, tail) = match *then_block { + Block { stmts, expr: Some(tail), .. } => (stmts, tail), + Block { + stmts: &[ + ref head @ .., + Stmt { kind: StmtKind::Expr(tail) | StmtKind::Semi(tail), .. } + ], + .. + } => (head, tail), + _ => return, + }; + if_chain! { + if matches!(tail.kind, + ExprKind::If(_, _, None) + | ExprKind::Match(.., MatchSource::IfLetDesugar { contains_else_clause: false })); + let sm = cx.sess().source_map(); + if head + .iter() + .all(|stmt| matches!(stmt.kind, StmtKind::Local(..)) && !sm.is_multiline(stmt.span)); + if if_chain_span.is_some() || !is_else_clause(cx.tcx, if_expr); + then {} else { return } + } + let (span, msg) = match (if_chain_span, is_expn_of(tail.span, "if_chain")) { + (None, Some(_)) => (if_expr.span, "this `if` can be part of the inner `if_chain!`"), + (Some(_), None) => (tail.span, "this `if` can be part of the outer `if_chain!`"), + (Some(a), Some(b)) if a != b => (b, "this `if_chain!` can be merged with the outer `if_chain!`"), + _ => return, + }; + span_lint_and_then(cx, IF_CHAIN_STYLE, span, msg, |diag| { + let (span, msg) = match head { + [] => return, + [stmt] => (stmt.span, "this `let` statement can also be in the `if_chain!`"), + [a, .., b] => ( + a.span.to(b.span), + "these `let` statements can also be in the `if_chain!`", + ), + }; + diag.span_help(span, msg); + }); +} + +fn is_first_if_chain_expr(cx: &LateContext<'_>, hir_id: HirId, if_chain_span: Span) -> bool { + cx.tcx + .hir() + .parent_iter(hir_id) + .find(|(_, node)| { + #[rustfmt::skip] + !matches!(node, Node::Expr(Expr { kind: ExprKind::Block(..), .. }) | Node::Stmt(_)) + }) + .map_or(false, |(id, _)| { + is_expn_of(cx.tcx.hir().span(id), "if_chain") != Some(if_chain_span) + }) +} + +/// Checks a trailing slice of statements and expression of a `Block` to see if they are part +/// of the `then {..}` portion of an `if_chain!` +fn is_if_chain_then(stmts: &[Stmt<'_>], expr: Option<&Expr<'_>>, if_chain_span: Span) -> bool { + let span = if let [stmt, ..] = stmts { + stmt.span + } else if let Some(expr) = expr { + expr.span + } else { + // empty `then {}` + return true; + }; + is_expn_of(span, "if_chain").map_or(true, |span| span != if_chain_span) +} + +/// Creates a `Span` for `let x = ..;` in an `if_chain!` call. +fn if_chain_local_span(cx: &LateContext<'_>, local: &Local<'_>, if_chain_span: Span) -> Span { + let mut span = local.pat.span; + if let Some(init) = local.init { + span = span.to(init.span); + } + span.adjust(if_chain_span.ctxt().outer_expn()); + let sm = cx.sess().source_map(); + let span = sm.span_extend_to_prev_str(span, "let", false); + let span = sm.span_extend_to_next_char(span, ';', false); + Span::new(span.lo() - BytePos(3), span.hi() + BytePos(1), span.ctxt()) +} diff --git a/clippy_lints/src/vec_init_then_push.rs b/clippy_lints/src/vec_init_then_push.rs index 8b696ed1c84..c7190e2f979 100644 --- a/clippy_lints/src/vec_init_then_push.rs +++ b/clippy_lints/src/vec_init_then_push.rs @@ -108,22 +108,21 @@ impl LateLintPass<'_> for VecInitThenPush { } fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - if self.searcher.is_none() { - if_chain! { - if !in_external_macro(cx.sess(), expr.span); - if let ExprKind::Assign(left, right, _) = expr.kind; - if let Some(id) = path_to_local(left); - if let Some(init_kind) = get_vec_init_kind(cx, right); - then { - self.searcher = Some(VecPushSearcher { - local_id: id, - init: init_kind, - lhs_is_local: false, - lhs_span: left.span, - err_span: expr.span, - found: 0, - }); - } + if_chain! { + if self.searcher.is_none(); + if !in_external_macro(cx.sess(), expr.span); + if let ExprKind::Assign(left, right, _) = expr.kind; + if let Some(id) = path_to_local(left); + if let Some(init_kind) = get_vec_init_kind(cx, right); + then { + self.searcher = Some(VecPushSearcher { + local_id: id, + init: init_kind, + lhs_is_local: false, + lhs_span: left.span, + err_span: expr.span, + found: 0, + }); } } } diff --git a/tests/ui-internal/if_chain_style.rs b/tests/ui-internal/if_chain_style.rs new file mode 100644 index 00000000000..8e871707aa8 --- /dev/null +++ b/tests/ui-internal/if_chain_style.rs @@ -0,0 +1,92 @@ +#![warn(clippy::if_chain_style)] +#![allow(clippy::no_effect)] + +extern crate if_chain; + +use if_chain::if_chain; + +fn main() { + if true { + let x = ""; + // `if_chain!` inside `if` + if_chain! { + if true; + if true; + then {} + } + } + if_chain! { + if true + // multi-line AND'ed conditions + && false; + if let Some(1) = Some(1); + // `let` before `then` + let x = ""; + then { + (); + } + } + if_chain! { + // single `if` condition + if true; + then { + let x = ""; + // nested if + if true {} + } + } + if_chain! { + // starts with `let ..` + let x = ""; + if let Some(1) = Some(1); + then { + let x = ""; + let x = ""; + // nested if_chain! + if_chain! { + if true; + if true; + then {} + } + } + } +} + +fn negative() { + if true { + (); + if_chain! { + if true; + if true; + then { (); } + } + } + if_chain! { + if true; + let x = ""; + if true; + then { (); } + } + if_chain! { + if true; + if true; + then { + if true { 1 } else { 2 } + } else { + 3 + } + }; + if true { + if_chain! { + if true; + if true; + then {} + } + } else if false { + if_chain! { + if true; + if false; + then {} + } + } +} diff --git a/tests/ui-internal/if_chain_style.stderr b/tests/ui-internal/if_chain_style.stderr new file mode 100644 index 00000000000..b53c3ea05da --- /dev/null +++ b/tests/ui-internal/if_chain_style.stderr @@ -0,0 +1,85 @@ +error: this `if` can be part of the inner `if_chain!` + --> $DIR/if_chain_style.rs:9:5 + | +LL | / if true { +LL | | let x = ""; +LL | | // `if_chain!` inside `if` +LL | | if_chain! { +... | +LL | | } +LL | | } + | |_____^ + | + = note: `-D clippy::if-chain-style` implied by `-D warnings` +help: this `let` statement can also be in the `if_chain!` + --> $DIR/if_chain_style.rs:10:9 + | +LL | let x = ""; + | ^^^^^^^^^^^ + +error: `if a && b;` should be `if a; if b;` + --> $DIR/if_chain_style.rs:19:12 + | +LL | if true + | ____________^ +LL | | // multi-line AND'ed conditions +LL | | && false; + | |____________________^ + +error: `let` expression should be inside `then { .. }` + --> $DIR/if_chain_style.rs:24:9 + | +LL | let x = ""; + | ^^^^^^^^^^^ + +error: this `if` can be part of the outer `if_chain!` + --> $DIR/if_chain_style.rs:35:13 + | +LL | if true {} + | ^^^^^^^^^^ + | +help: this `let` statement can also be in the `if_chain!` + --> $DIR/if_chain_style.rs:33:13 + | +LL | let x = ""; + | ^^^^^^^^^^^ + +error: `if_chain!` only has one `if` + --> $DIR/if_chain_style.rs:29:5 + | +LL | / if_chain! { +LL | | // single `if` condition +LL | | if true; +LL | | then { +... | +LL | | } +LL | | } + | |_____^ + | + = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) + +error: `let` expression should be above the `if_chain!` + --> $DIR/if_chain_style.rs:40:9 + | +LL | let x = ""; + | ^^^^^^^^^^^ + +error: this `if_chain!` can be merged with the outer `if_chain!` + --> $DIR/if_chain_style.rs:46:13 + | +LL | / if_chain! { +LL | | if true; +LL | | if true; +LL | | then {} +LL | | } + | |_____________^ + | +help: these `let` statements can also be in the `if_chain!` + --> $DIR/if_chain_style.rs:43:13 + | +LL | / let x = ""; +LL | | let x = ""; + | |_______________________^ + +error: aborting due to 7 previous errors + |