X-Git-Url: https://git.lizzy.rs/?a=blobdiff_plain;f=clippy_lints%2Fsrc%2Fmisc.rs;h=6dbc33d87299fcfe076322aa9c1eb133afb7fd82;hb=2ff568d746e4641b992c0b74bea046e43a637997;hp=b9c05bb2bd3342391537dc335f3ab2b7cd047684;hpb=f7600888205734d7c7b0ed2ec7db535f3d97873a;p=rust.git diff --git a/clippy_lints/src/misc.rs b/clippy_lints/src/misc.rs index b9c05bb2bd3..6dbc33d8729 100644 --- a/clippy_lints/src/misc.rs +++ b/clippy_lints/src/misc.rs @@ -1,20 +1,22 @@ use if_chain::if_chain; -use matches::matches; -use rustc::hir::intravisit::FnKind; -use rustc::hir::*; -use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; use rustc::ty; -use rustc::{declare_lint_pass, declare_tool_lint}; +use rustc_ast::ast::LitKind; use rustc_errors::Applicability; -use syntax::ast::LitKind; -use syntax::source_map::{ExpnKind, Span}; +use rustc_hir::intravisit::FnKind; +use rustc_hir::{ + def, BinOpKind, BindingAnnotation, Body, Expr, ExprKind, FnDecl, HirId, Mutability, PatKind, Stmt, StmtKind, Ty, + TyKind, UnOp, +}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::source_map::{ExpnKind, Span}; use crate::consts::{constant, Constant}; use crate::utils::sugg::Sugg; use crate::utils::{ - get_item_name, get_parent_expr, implements_trait, in_constant, is_integer_literal, iter_input_pats, - last_path_segment, match_qpath, match_trait_method, paths, snippet, span_lint, span_lint_and_then, - span_lint_hir_and_then, walk_ptrs_ty, SpanlessEq, + get_item_name, get_parent_expr, 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, walk_ptrs_ty, SpanlessEq, }; declare_clippy_lint! { @@ -62,7 +64,7 @@ /// ``` pub CMP_NAN, correctness, - "comparisons to NAN, which will always return false, probably not intended" + "comparisons to `NAN`, which will always return false, probably not intended" } declare_clippy_lint! { @@ -136,28 +138,6 @@ "taking a number modulo 1, which always returns 0" } -declare_clippy_lint! { - /// **What it does:** Checks for patterns in the form `name @ _`. - /// - /// **Why is this bad?** It's almost always more readable to just use direct - /// bindings. - /// - /// **Known problems:** None. - /// - /// **Example:** - /// ```rust - /// # let v = Some("abc"); - /// - /// match v { - /// Some(x) => (), - /// y @ _ => (), // easier written as `y`, - /// } - /// ``` - pub REDUNDANT_PATTERN, - style, - "using `name @ _` in a pattern" -} - declare_clippy_lint! { /// **What it does:** Checks for the use of bindings with a single leading /// underscore. @@ -215,7 +195,7 @@ /// ``` pub ZERO_PTR, style, - "using 0 as *{const, mut} T" + "using `0 as *{const, mut} T`" } declare_clippy_lint! { @@ -247,7 +227,6 @@ FLOAT_CMP, CMP_OWNED, MODULO_ONE, - REDUNDANT_PATTERN, USED_UNDERSCORE_BINDING, SHORT_CIRCUIT_STATEMENT, ZERO_PTR, @@ -259,8 +238,8 @@ fn check_fn( &mut self, cx: &LateContext<'a, 'tcx>, k: FnKind<'tcx>, - decl: &'tcx FnDecl, - body: &'tcx Body, + decl: &'tcx FnDecl<'_>, + body: &'tcx Body<'_>, _: Span, _: HirId, ) { @@ -269,7 +248,7 @@ fn check_fn( return; } for arg in iter_input_pats(decl, body) { - match arg.pat.node { + match arg.pat.kind { PatKind::Binding(BindingAnnotation::Ref, ..) | PatKind::Binding(BindingAnnotation::RefMut, ..) => { span_lint( cx, @@ -284,40 +263,45 @@ fn check_fn( } } - fn check_stmt(&mut self, cx: &LateContext<'a, 'tcx>, s: &'tcx Stmt) { + fn check_stmt(&mut self, cx: &LateContext<'a, 'tcx>, stmt: &'tcx Stmt<'_>) { if_chain! { - if let StmtKind::Local(ref l) = s.node; - if let PatKind::Binding(an, .., i, None) = l.pat.node; - if let Some(ref init) = l.init; + if let StmtKind::Local(ref local) = stmt.kind; + if let PatKind::Binding(an, .., name, None) = local.pat.kind; + if let Some(ref init) = local.init; then { if an == BindingAnnotation::Ref || an == BindingAnnotation::RefMut { - let sugg_init = Sugg::hir(cx, init, ".."); - let (mutopt,initref) = if an == BindingAnnotation::RefMut { + let sugg_init = if init.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) = l.ty { + 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, + span_lint_hir_and_then( + cx, TOPLEVEL_REF_ARG, init.hir_id, - l.pat.span, + local.pat.span, "`ref` on an entire `let` pattern is discouraged, take a reference with `&` instead", |db| { db.span_suggestion( - s.span, + stmt.span, "try", format!( "let {name}{tyopt} = {initref};", - name=snippet(cx, i.span, "_"), + name=snippet(cx, name.span, "_"), tyopt=tyopt, initref=initref, ), - Applicability::MachineApplicable, // snippet + Applicability::MachineApplicable, ); } ); @@ -325,19 +309,19 @@ fn check_stmt(&mut self, cx: &LateContext<'a, 'tcx>, s: &'tcx Stmt) { } }; if_chain! { - if let StmtKind::Semi(ref expr) = s.node; - if let ExprKind::Binary(ref binop, ref a, ref b) = expr.node; + if let StmtKind::Semi(ref expr) = stmt.kind; + if let ExprKind::Binary(ref binop, ref a, ref b) = expr.kind; if binop.node == BinOpKind::And || binop.node == BinOpKind::Or; if let Some(sugg) = Sugg::hir_opt(cx, a); then { span_lint_and_then(cx, SHORT_CIRCUIT_STATEMENT, - s.span, + stmt.span, "boolean short circuit operator in statement may be clearer using an explicit test", |db| { let sugg = if binop.node == BinOpKind::Or { !sugg } else { sugg }; db.span_suggestion( - s.span, + stmt.span, "replace it with", format!( "if {} {{ {}; }}", @@ -351,8 +335,8 @@ fn check_stmt(&mut self, cx: &LateContext<'a, 'tcx>, s: &'tcx Stmt) { }; } - fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) { - match expr.node { + fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) { + match expr.kind { ExprKind::Cast(ref e, ref ty) => { check_cast(cx, expr.span, e, ty); return; @@ -360,12 +344,8 @@ fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) { ExprKind::Binary(ref cmp, ref left, ref right) => { let op = cmp.node; if op.is_comparison() { - if let ExprKind::Path(QPath::Resolved(_, ref path)) = left.node { - check_nan(cx, path, expr); - } - if let ExprKind::Path(QPath::Resolved(_, ref path)) = right.node { - check_nan(cx, path, expr); - } + check_nan(cx, left, expr); + check_nan(cx, right, expr); check_to_owned(cx, left, right); check_to_owned(cx, right, left); } @@ -391,9 +371,9 @@ fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) { } } let (lint, msg) = if is_named_constant(cx, left) || is_named_constant(cx, right) { - (FLOAT_CMP_CONST, "strict comparison of f32 or f64 constant") + (FLOAT_CMP_CONST, "strict comparison of `f32` or `f64` constant") } else { - (FLOAT_CMP, "strict comparison of f32 or f64") + (FLOAT_CMP, "strict comparison of `f32` or `f64`") }; span_lint_and_then(cx, lint, expr.span, msg, |db| { let lhs = Sugg::hir(cx, left, ".."); @@ -407,11 +387,11 @@ fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) { lhs - rhs, if op == BinOpKind::Eq { '<' } else { '>' } ), - Applicability::MachineApplicable, // snippet + Applicability::HasPlaceholders, // snippet ); - db.span_note(expr.span, "std::f32::EPSILON and std::f64::EPSILON are available."); + db.span_note(expr.span, "`std::f32::EPSILON` and `std::f64::EPSILON` are available."); }); - } else if op == BinOpKind::Rem && is_integer_literal(right, 1) { + } else if op == BinOpKind::Rem && is_integer_const(cx, right, 1) { span_lint(cx, MODULO_ONE, expr.span, "any number modulo 1 will be 0"); } }, @@ -421,7 +401,7 @@ fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) { // Don't lint things expanded by #[derive(...)], etc return; } - let binding = match expr.node { + let binding = match expr.kind { ExprKind::Path(ref qpath) => { let binding = last_path_segment(qpath).ident.as_str(); if binding.starts_with('_') && @@ -459,40 +439,32 @@ fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) { ); } } - - fn check_pat(&mut self, cx: &LateContext<'a, 'tcx>, pat: &'tcx Pat) { - if let PatKind::Binding(.., ident, Some(ref right)) = pat.node { - if let PatKind::Wild = right.node { - span_lint( - cx, - REDUNDANT_PATTERN, - pat.span, - &format!( - "the `{} @ _` pattern can be written as just `{}`", - ident.name, ident.name - ), - ); - } - } - } } -fn check_nan(cx: &LateContext<'_, '_>, path: &Path, expr: &Expr) { - if !in_constant(cx, expr.hir_id) { - if let Some(seg) = path.segments.last() { - if seg.ident.name == sym!(NAN) { +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); + 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, - expr.span, - "doomed comparison with NAN, use `std::{f32,f64}::is_nan()` instead", + cmp_expr.span, + "doomed comparison with `NAN`, use `std::{f32,f64}::is_nan()` instead", ); } } } } -fn is_named_constant<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) -> bool { +fn is_named_constant<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) -> bool { if let Some((_, res)) = constant(cx, cx.tables, expr) { res } else { @@ -500,7 +472,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<'a, 'tcx>(cx: &LateContext<'a, '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(), @@ -509,14 +481,14 @@ fn is_allowed<'a, 'tcx>(cx: &LateContext<'a, '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 { +fn is_signum(cx: &LateContext<'_, '_>, expr: &Expr<'_>) -> bool { // The negation of a signum is still a signum - if let ExprKind::Unary(UnNeg, ref child_expr) = expr.node { + if let ExprKind::Unary(UnOp::UnNeg, ref child_expr) = expr.kind { return is_signum(cx, &child_expr); } if_chain! { - if let ExprKind::MethodCall(ref method_name, _, ref expressions) = expr.node; + if let ExprKind::MethodCall(ref method_name, _, ref 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) @@ -527,12 +499,12 @@ fn is_signum(cx: &LateContext<'_, '_>, expr: &Expr) -> bool { false } -fn is_float(cx: &LateContext<'_, '_>, expr: &Expr) -> bool { - matches!(walk_ptrs_ty(cx.tables.expr_ty(expr)).sty, ty::Float(_)) +fn is_float(cx: &LateContext<'_, '_>, expr: &Expr<'_>) -> bool { + matches!(walk_ptrs_ty(cx.tables.expr_ty(expr)).kind, ty::Float(_)) } -fn check_to_owned(cx: &LateContext<'_, '_>, expr: &Expr, other: &Expr) { - let (arg_ty, snip) = match expr.node { +fn check_to_owned(cx: &LateContext<'_, '_>, expr: &Expr<'_>, other: &Expr<'_>) { + 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, "..")) @@ -541,7 +513,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.node { + 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, "..")) } else { @@ -572,8 +544,8 @@ fn check_to_owned(cx: &LateContext<'_, '_>, expr: &Expr, other: &Expr) { return; } - let other_gets_derefed = match other.node { - ExprKind::Unary(UnDeref, _) => true, + let other_gets_derefed = match other.kind { + ExprKind::Unary(UnOp::UnDeref, _) => true, _ => false, }; @@ -616,10 +588,12 @@ 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 { +fn is_used(cx: &LateContext<'_, '_>, expr: &Expr<'_>) -> bool { if let Some(parent) = get_parent_expr(cx, expr) { - match parent.node { - ExprKind::Assign(_, ref rhs) | ExprKind::AssignOp(_, _, ref rhs) => SpanlessEq::new(cx).eq_expr(rhs, expr), + match parent.kind { + ExprKind::Assign(_, ref rhs, _) | ExprKind::AssignOp(_, _, ref rhs) => { + SpanlessEq::new(cx).eq_expr(rhs, expr) + }, _ => is_used(cx, parent), } } else { @@ -629,8 +603,8 @@ fn is_used(cx: &LateContext<'_, '_>, expr: &Expr) -> bool { /// Tests whether an expression is in a macro expansion (e.g., something /// generated by `#[derive(...)]` or the like). -fn in_attributes_expansion(expr: &Expr) -> bool { - use syntax::ext::hygiene::MacroKind; +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(); @@ -653,19 +627,27 @@ 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: &Ty<'_>) { if_chain! { - if let TyKind::Ptr(MutTy { mutbl, .. }) = ty.node; - if let ExprKind::Lit(ref lit) = e.node; - if let LitKind::Int(value, ..) = lit.node; - if value == 0; + if let TyKind::Ptr(ref mut_ty) = ty.kind; + if let ExprKind::Lit(ref lit) = e.kind; + if let LitKind::Int(0, _) = lit.node; if !in_constant(cx, e.hir_id); then { - let msg = match mutbl { - Mutability::MutMutable => "`0 as *mut _` detected. Consider using `ptr::null_mut()`", - Mutability::MutImmutable => "`0 as *const _` detected. Consider using `ptr::null()`", + let (msg, sugg_fn) = match mut_ty.mutbl { + Mutability::Mut => ("`0 as *mut _` detected", "std::ptr::null_mut"), + Mutability::Not => ("`0 as *const _` detected", "std::ptr::null"), + }; + + let (sugg, appl) = if let TyKind::Infer = mut_ty.ty.kind { + (format!("{}()", sugg_fn), Applicability::MachineApplicable) + } else if let Some(mut_ty_snip) = snippet_opt(cx, mut_ty.ty.span) { + (format!("{}::<{}>()", sugg_fn, mut_ty_snip), Applicability::MachineApplicable) + } else { + // `MaybeIncorrect` as type inference may not work with the suggested code + (format!("{}()", sugg_fn), Applicability::MaybeIncorrect) }; - span_lint(cx, ZERO_PTR, span, msg); + span_lint_and_sugg(cx, ZERO_PTR, span, msg, "try", sugg, appl); } } }