X-Git-Url: https://git.lizzy.rs/?a=blobdiff_plain;f=clippy_lints%2Fsrc%2Fmisc.rs;h=d156c1d5bf47fac784021160f5956ccd02233796;hb=76bd5d232c767dbdab63005eca07a9563982e9dd;hp=f16feb9b1ba2d977f2303d39cd20c07ffe949660;hpb=82a7068007a1490b43a2eb4e70e0f70de384a9ae;p=rust.git diff --git a/clippy_lints/src/misc.rs b/clippy_lints/src/misc.rs index f16feb9b1ba..d156c1d5bf4 100644 --- a/clippy_lints/src/misc.rs +++ b/clippy_lints/src/misc.rs @@ -1,3 +1,6 @@ +use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg, span_lint_and_then, span_lint_hir_and_then}; +use clippy_utils::source::{snippet, snippet_opt}; +use clippy_utils::ty::implements_trait; use if_chain::if_chain; use rustc_ast::ast::LitKind; use rustc_errors::Applicability; @@ -12,13 +15,13 @@ use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::hygiene::DesugaringKind; use rustc_span::source_map::{ExpnKind, Span}; +use rustc_span::symbol::sym; use crate::consts::{constant, Constant}; -use crate::utils::sugg::Sugg; -use crate::utils::{ - get_item_name, get_parent_expr, higher, implements_trait, in_constant, is_integer_const, iter_input_pats, - last_path_segment, match_qpath, match_trait_method, paths, snippet, snippet_opt, span_lint, span_lint_and_sugg, - span_lint_and_then, span_lint_hir_and_then, SpanlessEq, unsext, +use clippy_utils::sugg::Sugg; +use clippy_utils::{ + get_item_name, get_parent_expr, higher, in_constant, is_diag_trait_item, is_integer_const, iter_input_pats, + last_path_segment, match_qpath, unsext, SpanlessEq, }; declare_clippy_lint! { @@ -278,7 +281,7 @@ fn check_fn( span: Span, _: HirId, ) { - if let FnKind::Closure(_) = k { + if let FnKind::Closure = k { // Does not apply to closures return; } @@ -292,7 +295,7 @@ fn check_fn( TOPLEVEL_REF_ARG, arg.pat.span, "`ref` directly on a function argument is ignored. \ - Consider using a reference type instead.", + Consider using a reference type instead", ); } } @@ -301,55 +304,54 @@ fn check_fn( fn check_stmt(&mut self, cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'_>) { if_chain! { if !in_external_macro(cx.tcx.sess, stmt.span); - if let StmtKind::Local(ref local) = stmt.kind; + if let StmtKind::Local(local) = stmt.kind; if let PatKind::Binding(an, .., name, None) = local.pat.kind; - if let Some(ref init) = local.init; + if let Some(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(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! { - if let StmtKind::Semi(ref expr) = stmt.kind; - if let ExprKind::Binary(ref binop, ref a, ref b) = expr.kind; + if let StmtKind::Semi(expr) = stmt.kind; + if let ExprKind::Binary(ref binop, a, b) = expr.kind; if binop.node == BinOpKind::And || binop.node == BinOpKind::Or; if let Some(sugg) = Sugg::hir_opt(cx, a); then { @@ -376,74 +378,13 @@ fn check_stmt(&mut self, cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'_>) { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { match expr.kind { - ExprKind::Cast(ref e, ref ty) => { + ExprKind::Cast(e, ty) => { check_cast(cx, expr.span, e, ty); return; }, - ExprKind::Binary(ref cmp, ref left, ref right) => { - let op = cmp.node; - if op.is_comparison() { - check_nan(cx, left, expr); - check_nan(cx, right, expr); - 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) { - return; - } - - // Allow comparing the results of signum() - if is_signum(cx, left) && is_signum(cx, right) { - return; - } - - if let Some(name) = get_item_name(cx, expr) { - let name = name.as_str(); - if name == "eq" - || name == "ne" - || name == "is_nan" - || name.starts_with("eq_") - || name.ends_with("_eq") - { - return; - } - } - let is_comparing_arrays = is_array(cx, left) || is_array(cx, right); - let (lint, msg) = get_lint_and_message( - is_named_constant(cx, left) || is_named_constant(cx, right), - is_comparing_arrays, - ); - span_lint_and_then(cx, lint, expr.span, msg, |diag| { - let lhs = Sugg::hir(cx, left, ".."); - let rhs = Sugg::hir(cx, right, ".."); - - if !is_comparing_arrays { - diag.span_suggestion( - expr.span, - "consider comparing them within some margin of error", - format!( - "({}).abs() {} error_margin", - lhs - rhs, - if op == BinOpKind::Eq { '<' } else { '>' } - ), - Applicability::HasPlaceholders, // snippet - ); - } - diag.note("`f32::EPSILON` and `f64::EPSILON` are available for the `error_margin`"); - }); - } else if op == BinOpKind::Rem { - if is_integer_const(cx, right, 1) { - span_lint(cx, MODULO_ONE, expr.span, "any number modulo 1 will be 0"); - } - - if let ty::Int(ity) = cx.typeck_results().expr_ty(right).kind() { - if is_integer_const(cx, right, unsext(cx.tcx, -1, *ity)) { - span_lint(cx, MODULO_ONE, expr.span, - "any number modulo -1 will panic/overflow or result in 0"); - } - }; - } + ExprKind::Binary(ref cmp, left, right) => { + check_binary(cx, expr, cmp, left, right); + return; }, _ => {}, } @@ -483,7 +424,7 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { expr.span, &format!( "used binding `{}` which is prefixed with an underscore. A leading \ - underscore signals that a binding will not be used.", + underscore signals that a binding will not be used", binding ), ); @@ -520,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", + ); } } } @@ -563,12 +501,12 @@ fn is_allowed<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> bool { // Return true if `expr` is the result of `signum()` invoked on a float value. 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); + if let ExprKind::Unary(UnOp::Neg, child_expr) = expr.kind { + return is_signum(cx, child_expr); } if_chain! { - if let ExprKind::MethodCall(ref method_name, _, ref expressions, _) = expr.kind; + if let ExprKind::MethodCall(method_name, _, expressions, _) = expr.kind; if sym!(signum) == method_name.ident.name; // Check that the receiver of the signum() is a float (expressions[0] is the receiver of // the method call) @@ -614,14 +552,19 @@ fn symmetric_partial_eq<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, other: Ty<'t } 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.typeck_results().expr_ty(&args[0]), snippet(cx, args[0].span, "..")) - } else { - return; - } + ExprKind::MethodCall(.., args, _) if args.len() == 1 => { + if_chain!( + if let Some(expr_def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id); + if is_diag_trait_item(cx, expr_def_id, sym::ToString) + || is_diag_trait_item(cx, expr_def_id, sym::ToOwned); + then { + (cx.typeck_results().expr_ty(&args[0]), snippet(cx, args[0].span, "..")) + } else { + return; + } + ) }, - ExprKind::Call(ref path, ref v) if v.len() == 1 => { + ExprKind::Call(path, 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.typeck_results().expr_ty(&v[0]), snippet(cx, v[0].span, "..")) @@ -647,7 +590,7 @@ fn symmetric_partial_eq<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, other: Ty<'t return; } - let other_gets_derefed = matches!(other.kind, ExprKind::Unary(UnOp::UnDeref, _)); + let other_gets_derefed = matches!(other.kind, ExprKind::Unary(UnOp::Deref, _)); let lint_span = if other_gets_derefed { expr.span.to(other.span) @@ -706,7 +649,7 @@ fn symmetric_partial_eq<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, other: Ty<'t /// of what it means for an expression to be "used". 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), + ExprKind::Assign(_, rhs, _) | ExprKind::AssignOp(_, _, rhs) => SpanlessEq::new(cx).eq_expr(rhs, expr), _ => is_used(cx, parent), }) } @@ -756,3 +699,74 @@ fn check_cast(cx: &LateContext<'_>, span: Span, e: &Expr<'_>, ty: &hir::Ty<'_>) } } } + +fn check_binary( + cx: &LateContext<'a>, + expr: &Expr<'_>, + cmp: &rustc_span::source_map::Spanned, + left: &'a Expr<'_>, + right: &'a Expr<'_>, +) { + let op = cmp.node; + if op.is_comparison() { + check_nan(cx, left, expr); + check_nan(cx, right, expr); + 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) { + return; + } + + // Allow comparing the results of signum() + if is_signum(cx, left) && is_signum(cx, right) { + return; + } + + if let Some(name) = get_item_name(cx, expr) { + let name = name.as_str(); + if name == "eq" || name == "ne" || name == "is_nan" || name.starts_with("eq_") || name.ends_with("_eq") { + return; + } + } + let is_comparing_arrays = is_array(cx, left) || is_array(cx, right); + let (lint, msg) = get_lint_and_message( + is_named_constant(cx, left) || is_named_constant(cx, right), + is_comparing_arrays, + ); + span_lint_and_then(cx, lint, expr.span, msg, |diag| { + let lhs = Sugg::hir(cx, left, ".."); + let rhs = Sugg::hir(cx, right, ".."); + + if !is_comparing_arrays { + diag.span_suggestion( + expr.span, + "consider comparing them within some margin of error", + format!( + "({}).abs() {} error_margin", + lhs - rhs, + if op == BinOpKind::Eq { '<' } else { '>' } + ), + Applicability::HasPlaceholders, // snippet + ); + } + diag.note("`f32::EPSILON` and `f64::EPSILON` are available for the `error_margin`"); + }); + } else if op == BinOpKind::Rem { + if is_integer_const(cx, right, 1) { + span_lint(cx, MODULO_ONE, expr.span, "any number modulo 1 will be 0"); + } + + if let ty::Int(ity) = cx.typeck_results().expr_ty(right).kind() { + if is_integer_const(cx, right, unsext(cx.tcx, -1, *ity)) { + span_lint( + cx, + MODULO_ONE, + expr.span, + "any number modulo -1 will panic/overflow or result in 0", + ); + } + }; + } +}