]> git.lizzy.rs Git - rust.git/blobdiff - clippy_lints/src/misc.rs
Auto merge of #5334 - flip1995:backport_remerge, r=flip1995
[rust.git] / clippy_lints / src / misc.rs
index a761f80b7939f5b0f2d4a823b6ef6d84edbeb714..6dbc33d87299fcfe076322aa9c1eb133afb7fd82 100644 (file)
@@ -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! {
     /// ```
     pub ZERO_PTR,
     style,
-    "using 0 as *{const, mut} T"
+    "using `0 as *{const, mut} T`"
 }
 
 declare_clippy_lint! {
@@ -236,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,
     ) {
@@ -246,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,
@@ -261,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,
                             );
                         }
                     );
@@ -302,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 {} {{ {}; }}",
@@ -328,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;
@@ -337,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);
                 }
@@ -368,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, "..");
@@ -386,9 +389,9 @@ fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
                             ),
                             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");
                 }
             },
@@ -398,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('_') &&
@@ -438,22 +441,30 @@ fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
     }
 }
 
-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 {
@@ -461,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(),
@@ -470,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)
@@ -488,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, ".."))
@@ -502,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 {
@@ -533,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,
     };
 
@@ -577,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 {
@@ -590,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();
 
@@ -614,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);
         }
     }
 }