X-Git-Url: https://git.lizzy.rs/?a=blobdiff_plain;f=clippy_lints%2Fsrc%2Fswap.rs;h=f47945f0a14692956c8a9d12e917998be9164497;hb=e5a5b0a0774625eebbe7b29c67b49dc6431544d1;hp=11ab5b8762821f11eb57ca54f2e55093238664e0;hpb=abfa8a952c74a409ee0d3cc80d85d90cc9de70ae;p=rust.git diff --git a/clippy_lints/src/swap.rs b/clippy_lints/src/swap.rs index 11ab5b87628..f47945f0a14 100644 --- a/clippy_lints/src/swap.rs +++ b/clippy_lints/src/swap.rs @@ -1,14 +1,17 @@ use crate::utils::sugg::Sugg; use crate::utils::{ - differing_macro_contexts, match_type, paths, snippet, span_lint_and_then, walk_ptrs_ty, SpanlessEq, + differing_macro_contexts, is_type_diagnostic_item, match_type, paths, snippet_with_applicability, + span_lint_and_then, walk_ptrs_ty, SpanlessEq, }; use if_chain::if_chain; use matches::matches; +use rustc::declare_lint_pass; use rustc::hir::*; use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; use rustc::ty; -use rustc::{declare_lint_pass, declare_tool_lint}; use rustc_errors::Applicability; +use rustc_session::declare_tool_lint; +use syntax_pos::Symbol; declare_clippy_lint! { /// **What it does:** Checks for manual swapping. @@ -52,6 +55,12 @@ /// a = b; /// b = a; /// ``` + /// Could be written as: + /// ```rust + /// # let mut a = 1; + /// # let mut b = 2; + /// std::mem::swap(&mut a, &mut b); + /// ``` pub ALMOST_SWAPPED, correctness, "`foo = bar; bar = foo` sequence" @@ -71,99 +80,150 @@ fn check_manual_swap(cx: &LateContext<'_, '_>, block: &Block) { for w in block.stmts.windows(3) { if_chain! { // let t = foo(); - if let StmtKind::Local(ref tmp) = w[0].node; + if let StmtKind::Local(ref tmp) = w[0].kind; if let Some(ref tmp_init) = tmp.init; - if let PatKind::Binding(.., ident, None) = tmp.pat.node; + if let PatKind::Binding(.., ident, None) = tmp.pat.kind; // foo() = bar(); - if let StmtKind::Semi(ref first) = w[1].node; - if let ExprKind::Assign(ref lhs1, ref rhs1) = first.node; + if let StmtKind::Semi(ref first) = w[1].kind; + if let ExprKind::Assign(ref lhs1, ref rhs1) = first.kind; // bar() = t; - if let StmtKind::Semi(ref second) = w[2].node; - if let ExprKind::Assign(ref lhs2, ref rhs2) = second.node; - if let ExprKind::Path(QPath::Resolved(None, ref rhs2)) = rhs2.node; + if let StmtKind::Semi(ref second) = w[2].kind; + if let ExprKind::Assign(ref lhs2, ref rhs2) = second.kind; + if let ExprKind::Path(QPath::Resolved(None, ref rhs2)) = rhs2.kind; if rhs2.segments.len() == 1; if ident.as_str() == rhs2.segments[0].ident.as_str(); if SpanlessEq::new(cx).ignore_fn().eq_expr(tmp_init, lhs1); if SpanlessEq::new(cx).ignore_fn().eq_expr(rhs1, lhs2); then { - fn check_for_slice<'a>( - cx: &LateContext<'_, '_>, - lhs1: &'a Expr, - lhs2: &'a Expr, - ) -> Option<(&'a Expr, &'a Expr, &'a Expr)> { - if let ExprKind::Index(ref lhs1, ref idx1) = lhs1.node { - if let ExprKind::Index(ref lhs2, ref idx2) = lhs2.node { - if SpanlessEq::new(cx).ignore_fn().eq_expr(lhs1, lhs2) { - let ty = walk_ptrs_ty(cx.tables.expr_ty(lhs1)); - - if matches!(ty.sty, ty::Slice(_)) || - matches!(ty.sty, ty::Array(_, _)) || - match_type(cx, ty, &paths::VEC) || - match_type(cx, ty, &paths::VEC_DEQUE) { - return Some((lhs1, idx1, idx2)); - } - } + if let ExprKind::Field(ref lhs1, _) = lhs1.kind { + if let ExprKind::Field(ref lhs2, _) = lhs2.kind { + if lhs1.hir_id.owner_def_id() == lhs2.hir_id.owner_def_id() { + return; } } - - None } - let (replace, what, sugg) = if let Some((slice, idx1, idx2)) = check_for_slice(cx, lhs1, lhs2) { + let mut applicability = Applicability::MachineApplicable; + + let slice = check_for_slice(cx, lhs1, lhs2); + let (replace, what, sugg) = if let Slice::NotSwappable = slice { + return; + } else if let Slice::Swappable(slice, idx1, idx2) = slice { if let Some(slice) = Sugg::hir_opt(cx, slice) { - (false, - format!(" elements of `{}`", slice), - format!("{}.swap({}, {})", - slice.maybe_par(), - snippet(cx, idx1.span, ".."), - snippet(cx, idx2.span, ".."))) + ( + false, + format!(" elements of `{}`", slice), + format!( + "{}.swap({}, {})", + slice.maybe_par(), + snippet_with_applicability(cx, idx1.span, "..", &mut applicability), + snippet_with_applicability(cx, idx2.span, "..", &mut applicability), + ), + ) } else { (false, String::new(), String::new()) } } else if let (Some(first), Some(second)) = (Sugg::hir_opt(cx, lhs1), Sugg::hir_opt(cx, rhs1)) { - (true, format!(" `{}` and `{}`", first, second), - format!("std::mem::swap({}, {})", first.mut_addr(), second.mut_addr())) + ( + true, + format!(" `{}` and `{}`", first, second), + format!("std::mem::swap({}, {})", first.mut_addr(), second.mut_addr()), + ) } else { (true, String::new(), String::new()) }; let span = w[0].span.to(second.span); - span_lint_and_then(cx, - MANUAL_SWAP, - span, - &format!("this looks like you are swapping{} manually", what), - |db| { - if !sugg.is_empty() { - db.span_suggestion( - span, - "try", - sugg, - Applicability::Unspecified, - ); + span_lint_and_then( + cx, + MANUAL_SWAP, + span, + &format!("this looks like you are swapping{} manually", what), + |db| { + if !sugg.is_empty() { + db.span_suggestion( + span, + "try", + sugg, + applicability, + ); - if replace { - db.note("or maybe you should use `std::mem::replace`?"); - } - } - }); + if replace { + db.note("or maybe you should use `std::mem::replace`?"); + } + } + } + ); + } + } + } +} + +enum Slice<'a> { + /// `slice.swap(idx1, idx2)` can be used + /// + /// ## Example + /// + /// ```rust + /// # let mut a = vec![0, 1]; + /// let t = a[1]; + /// a[1] = a[0]; + /// a[0] = t; + /// // can be written as + /// a.swap(0, 1); + /// ``` + Swappable(&'a Expr, &'a Expr, &'a Expr), + /// The `swap` function cannot be used. + /// + /// ## Example + /// + /// ```rust + /// # let mut a = [vec![1, 2], vec![3, 4]]; + /// let t = a[0][1]; + /// a[0][1] = a[1][0]; + /// a[1][0] = t; + /// ``` + NotSwappable, + /// Not a slice + None, +} + +/// Checks if both expressions are index operations into "slice-like" types. +fn check_for_slice<'a>(cx: &LateContext<'_, '_>, lhs1: &'a Expr, lhs2: &'a Expr) -> Slice<'a> { + if let ExprKind::Index(ref lhs1, ref idx1) = lhs1.kind { + if let ExprKind::Index(ref lhs2, ref idx2) = lhs2.kind { + if SpanlessEq::new(cx).ignore_fn().eq_expr(lhs1, lhs2) { + let ty = walk_ptrs_ty(cx.tables.expr_ty(lhs1)); + + if matches!(ty.kind, ty::Slice(_)) + || matches!(ty.kind, ty::Array(_, _)) + || is_type_diagnostic_item(cx, ty, Symbol::intern("vec_type")) + || match_type(cx, ty, &paths::VEC_DEQUE) + { + return Slice::Swappable(lhs1, idx1, idx2); + } + } else { + return Slice::NotSwappable; } } } + + Slice::None } /// Implementation of the `ALMOST_SWAPPED` lint. fn check_suspicious_swap(cx: &LateContext<'_, '_>, block: &Block) { for w in block.stmts.windows(2) { if_chain! { - if let StmtKind::Semi(ref first) = w[0].node; - if let StmtKind::Semi(ref second) = w[1].node; + if let StmtKind::Semi(ref first) = w[0].kind; + if let StmtKind::Semi(ref second) = w[1].kind; if !differing_macro_contexts(first.span, second.span); - if let ExprKind::Assign(ref lhs0, ref rhs0) = first.node; - if let ExprKind::Assign(ref lhs1, ref rhs1) = second.node; + if let ExprKind::Assign(ref lhs0, ref rhs0) = first.kind; + if let ExprKind::Assign(ref lhs1, ref rhs1) = second.kind; if SpanlessEq::new(cx).ignore_fn().eq_expr(lhs0, rhs1); if SpanlessEq::new(cx).ignore_fn().eq_expr(lhs1, rhs0); then {