X-Git-Url: https://git.lizzy.rs/?a=blobdiff_plain;f=src%2Ftools%2Fclippy%2Fclippy_lints%2Fsrc%2Fmisc.rs;h=400f4b609af7f577833d183d1bf3486ef2224a88;hb=efad203144c25ba6f34e8e53d4c1a7417fb03c23;hp=99cd864cae4e35b62955871bdc6d924a3c19671d;hpb=7926c64642d070fe400dda1a53b4183f415d8445;p=rust.git diff --git a/src/tools/clippy/clippy_lints/src/misc.rs b/src/tools/clippy/clippy_lints/src/misc.rs index 99cd864cae4..400f4b609af 100644 --- a/src/tools/clippy/clippy_lints/src/misc.rs +++ b/src/tools/clippy/clippy_lints/src/misc.rs @@ -3,11 +3,11 @@ use rustc_errors::Applicability; use rustc_hir::intravisit::FnKind; use rustc_hir::{ - def, BinOpKind, BindingAnnotation, Body, Expr, ExprKind, FnDecl, HirId, Mutability, PatKind, Stmt, StmtKind, Ty, - TyKind, UnOp, + self as hir, def, BinOpKind, BindingAnnotation, Body, Expr, ExprKind, FnDecl, HirId, Mutability, PatKind, Stmt, + StmtKind, TyKind, UnOp, }; use rustc_lint::{LateContext, LateLintPass}; -use rustc_middle::ty; +use rustc_middle::ty::{self, Ty}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::hygiene::DesugaringKind; use rustc_span::source_map::{ExpnKind, Span}; @@ -260,10 +260,10 @@ FLOAT_CMP_CONST ]); -impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MiscLints { +impl<'tcx> LateLintPass<'tcx> for MiscLints { fn check_fn( &mut self, - cx: &LateContext<'a, 'tcx>, + cx: &LateContext<'tcx>, k: FnKind<'tcx>, decl: &'tcx FnDecl<'_>, body: &'tcx Body<'_>, @@ -287,7 +287,7 @@ fn check_fn( } } - fn check_stmt(&mut self, cx: &LateContext<'a, 'tcx>, stmt: &'tcx Stmt<'_>) { + fn check_stmt(&mut self, cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'_>) { if_chain! { if let StmtKind::Local(ref local) = stmt.kind; if let PatKind::Binding(an, .., name, None) = local.pat.kind; @@ -360,7 +360,7 @@ fn check_stmt(&mut self, cx: &LateContext<'a, 'tcx>, stmt: &'tcx Stmt<'_>) { }; } - fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { match expr.kind { ExprKind::Cast(ref e, ref ty) => { check_cast(cx, expr.span, e, ty); @@ -371,8 +371,8 @@ fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) { if op.is_comparison() { check_nan(cx, left, expr); check_nan(cx, right, expr); - check_to_owned(cx, left, right); - check_to_owned(cx, right, left); + check_to_owned(cx, left, right, true); + check_to_owned(cx, right, left, false); } if (op == BinOpKind::Eq || op == BinOpKind::Ne) && (is_float(cx, left) || is_float(cx, right)) { if is_allowed(cx, left) || is_allowed(cx, right) { @@ -436,7 +436,7 @@ fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) { binding != "_result" && // FIXME: #944 is_used(cx, expr) && // don't lint if the declaration is in a macro - non_macro_local(cx, cx.tables().qpath_res(qpath, expr.hir_id)) + non_macro_local(cx, cx.qpath_res(qpath, expr.hir_id)) { Some(binding) } else { @@ -493,7 +493,7 @@ fn get_lint_and_message( } } -fn check_nan(cx: &LateContext<'_, '_>, expr: &Expr<'_>, cmp_expr: &Expr<'_>) { +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.tables(), expr); @@ -516,7 +516,7 @@ fn check_nan(cx: &LateContext<'_, '_>, expr: &Expr<'_>, cmp_expr: &Expr<'_>) { } } -fn is_named_constant<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) -> bool { +fn is_named_constant<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> bool { if let Some((_, res)) = constant(cx, cx.tables(), expr) { res } else { @@ -524,7 +524,7 @@ fn is_named_constant<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) } } -fn is_allowed<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) -> bool { +fn is_allowed<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> bool { match constant(cx, cx.tables(), expr) { Some((Constant::F32(f), _)) => f == 0.0 || f.is_infinite(), Some((Constant::F64(f), _)) => f == 0.0 || f.is_infinite(), @@ -538,7 +538,7 @@ fn is_allowed<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) -> boo } // Return true if `expr` is the result of `signum()` invoked on a float value. -fn is_signum(cx: &LateContext<'_, '_>, expr: &Expr<'_>) -> bool { +fn is_signum(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { // The negation of a signum is still a signum if let ExprKind::Unary(UnOp::UnNeg, ref child_expr) = expr.kind { return is_signum(cx, &child_expr); @@ -556,7 +556,7 @@ fn is_signum(cx: &LateContext<'_, '_>, expr: &Expr<'_>) -> bool { false } -fn is_float(cx: &LateContext<'_, '_>, expr: &Expr<'_>) -> bool { +fn is_float(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { let value = &walk_ptrs_ty(cx.tables().expr_ty(expr)).kind; if let ty::Array(arr_ty, _) = value { @@ -566,15 +566,34 @@ fn is_float(cx: &LateContext<'_, '_>, expr: &Expr<'_>) -> bool { matches!(value, ty::Float(_)) } -fn is_array(cx: &LateContext<'_, '_>, expr: &Expr<'_>) -> bool { +fn is_array(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { matches!(&walk_ptrs_ty(cx.tables().expr_ty(expr)).kind, ty::Array(_, _)) } -fn check_to_owned(cx: &LateContext<'_, '_>, expr: &Expr<'_>, other: &Expr<'_>) { +fn check_to_owned(cx: &LateContext<'_>, expr: &Expr<'_>, other: &Expr<'_>, left: bool) { + #[derive(Default)] + struct EqImpl { + ty_eq_other: bool, + other_eq_ty: bool, + } + + impl EqImpl { + fn is_implemented(&self) -> bool { + self.ty_eq_other || self.other_eq_ty + } + } + + fn symmetric_partial_eq<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, other: Ty<'tcx>) -> Option { + cx.tcx.lang_items().eq_trait().map(|def_id| EqImpl { + ty_eq_other: implements_trait(cx, ty, def_id, &[other.into()]), + other_eq_ty: implements_trait(cx, other, def_id, &[ty.into()]), + }) + } + let (arg_ty, snip) = match expr.kind { ExprKind::MethodCall(.., ref args, _) if args.len() == 1 => { if match_trait_method(cx, expr, &paths::TO_STRING) || match_trait_method(cx, expr, &paths::TO_OWNED) { - (cx.tables().expr_ty_adjusted(&args[0]), snippet(cx, args[0].span, "..")) + (cx.tables().expr_ty(&args[0]), snippet(cx, args[0].span, "..")) } else { return; } @@ -582,7 +601,7 @@ fn check_to_owned(cx: &LateContext<'_, '_>, expr: &Expr<'_>, other: &Expr<'_>) { ExprKind::Call(ref path, ref v) if v.len() == 1 => { if let ExprKind::Path(ref path) = path.kind { if match_qpath(path, &["String", "from_str"]) || match_qpath(path, &["String", "from"]) { - (cx.tables().expr_ty_adjusted(&v[0]), snippet(cx, v[0].span, "..")) + (cx.tables().expr_ty(&v[0]), snippet(cx, v[0].span, "..")) } else { return; } @@ -593,28 +612,19 @@ fn check_to_owned(cx: &LateContext<'_, '_>, expr: &Expr<'_>, other: &Expr<'_>) { _ => return, }; - let other_ty = cx.tables().expr_ty_adjusted(other); - let partial_eq_trait_id = match cx.tcx.lang_items().eq_trait() { - Some(id) => id, - None => return, - }; + let other_ty = cx.tables().expr_ty(other); - let deref_arg_impl_partial_eq_other = arg_ty.builtin_deref(true).map_or(false, |tam| { - implements_trait(cx, tam.ty, partial_eq_trait_id, &[other_ty.into()]) - }); - let arg_impl_partial_eq_deref_other = other_ty.builtin_deref(true).map_or(false, |tam| { - implements_trait(cx, arg_ty, partial_eq_trait_id, &[tam.ty.into()]) - }); - let arg_impl_partial_eq_other = implements_trait(cx, arg_ty, partial_eq_trait_id, &[other_ty.into()]); + let without_deref = symmetric_partial_eq(cx, arg_ty, other_ty).unwrap_or_default(); + let with_deref = arg_ty + .builtin_deref(true) + .and_then(|tam| symmetric_partial_eq(cx, tam.ty, other_ty)) + .unwrap_or_default(); - if !deref_arg_impl_partial_eq_other && !arg_impl_partial_eq_deref_other && !arg_impl_partial_eq_other { + if !with_deref.is_implemented() && !without_deref.is_implemented() { return; } - let other_gets_derefed = match other.kind { - ExprKind::Unary(UnOp::UnDeref, _) => true, - _ => false, - }; + let other_gets_derefed = matches!(other.kind, ExprKind::Unary(UnOp::UnDeref, _)); let lint_span = if other_gets_derefed { expr.span.to(other.span) @@ -634,18 +644,34 @@ fn check_to_owned(cx: &LateContext<'_, '_>, expr: &Expr<'_>, other: &Expr<'_>) { return; } - let try_hint = if deref_arg_impl_partial_eq_other { - // suggest deref on the left - format!("*{}", snip) + let expr_snip; + let eq_impl; + if with_deref.is_implemented() { + expr_snip = format!("*{}", snip); + eq_impl = with_deref; } else { - // suggest dropping the to_owned on the left - snip.to_string() + expr_snip = snip.to_string(); + eq_impl = without_deref; }; + let span; + let hint; + if (eq_impl.ty_eq_other && left) || (eq_impl.other_eq_ty && !left) { + span = expr.span; + hint = expr_snip; + } else { + span = expr.span.to(other.span); + if eq_impl.ty_eq_other { + hint = format!("{} == {}", expr_snip, snippet(cx, other.span, "..")); + } else { + hint = format!("{} == {}", snippet(cx, other.span, ".."), expr_snip); + } + } + diag.span_suggestion( - lint_span, + span, "try", - try_hint, + hint, Applicability::MachineApplicable, // snippet ); }, @@ -655,17 +681,11 @@ fn check_to_owned(cx: &LateContext<'_, '_>, expr: &Expr<'_>, other: &Expr<'_>) { /// Heuristic to see if an expression is used. Should be compatible with /// `unused_variables`'s idea /// of what it means for an expression to be "used". -fn is_used(cx: &LateContext<'_, '_>, expr: &Expr<'_>) -> bool { - if let Some(parent) = get_parent_expr(cx, expr) { - match parent.kind { - ExprKind::Assign(_, ref rhs, _) | ExprKind::AssignOp(_, _, ref rhs) => { - SpanlessEq::new(cx).eq_expr(rhs, expr) - }, - _ => is_used(cx, parent), - } - } else { - true - } +fn is_used(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { + get_parent_expr(cx, expr).map_or(true, |parent| match parent.kind { + ExprKind::Assign(_, ref rhs, _) | ExprKind::AssignOp(_, _, ref rhs) => SpanlessEq::new(cx).eq_expr(rhs, expr), + _ => is_used(cx, parent), + }) } /// Tests whether an expression is in a macro expansion (e.g., something @@ -674,19 +694,14 @@ fn in_attributes_expansion(expr: &Expr<'_>) -> bool { use rustc_span::hygiene::MacroKind; if expr.span.from_expansion() { let data = expr.span.ctxt().outer_expn_data(); - - if let ExpnKind::Macro(MacroKind::Attr, _) = data.kind { - true - } else { - false - } + matches!(data.kind, ExpnKind::Macro(MacroKind::Attr, _)) } else { false } } /// Tests whether `res` is a variable defined outside a macro. -fn non_macro_local(cx: &LateContext<'_, '_>, res: def::Res) -> bool { +fn non_macro_local(cx: &LateContext<'_>, res: def::Res) -> bool { if let def::Res::Local(id) = res { !cx.tcx.hir().span(id).from_expansion() } else { @@ -694,7 +709,7 @@ fn non_macro_local(cx: &LateContext<'_, '_>, res: def::Res) -> bool { } } -fn check_cast(cx: &LateContext<'_, '_>, span: Span, e: &Expr<'_>, ty: &Ty<'_>) { +fn check_cast(cx: &LateContext<'_>, span: Span, e: &Expr<'_>, ty: &hir::Ty<'_>) { if_chain! { if let TyKind::Ptr(ref mut_ty) = ty.kind; if let ExprKind::Lit(ref lit) = e.kind;