clippy_lints/src/misc.rs

   1 use reexport::*;
   2 use rustc::hir::*;
   3 use rustc::hir::intravisit::FnKind;
   4 use rustc::lint::*;
   5 use rustc::middle::const_val::ConstVal;
   6 use rustc::ty;
   7 use rustc::ty::subst::Substs;
   8 use rustc_const_eval::ConstContext;
   9 use rustc_const_math::ConstFloat;
  10 use syntax::codemap::{ExpnFormat, Span};
  11 use utils::{get_item_name, get_parent_expr, implements_trait, in_constant, in_macro, is_integer_literal,
  12             iter_input_pats, last_path_segment, match_qpath, match_trait_method, paths, snippet, span_lint,
  13             span_lint_and_then, walk_ptrs_ty};
  14 use utils::sugg::Sugg;
  15 use syntax::ast::{FloatTy, LitKind, CRATE_NODE_ID};
  16
  17 /// **What it does:** Checks for function arguments and let bindings denoted as
  18 /// `ref`.
  19 ///
  20 /// **Why is this bad?** The `ref` declaration makes the function take an owned
  21 /// value, but turns the argument into a reference (which means that the value
  22 /// is destroyed when exiting the function). This adds not much value: either
  23 /// take a reference type, or take an owned value and create references in the
  24 /// body.
  25 ///
  26 /// For let bindings, `let x = &foo;` is preferred over `let ref x = foo`. The
  27 /// type of `x` is more obvious with the former.
  28 ///
  29 /// **Known problems:** If the argument is dereferenced within the function,
  30 /// removing the `ref` will lead to errors. This can be fixed by removing the
  31 /// dereferences, e.g. changing `*x` to `x` within the function.
  32 ///
  33 /// **Example:**
  34 /// ```rust
  35 /// fn foo(ref x: u8) -> bool { .. }
  36 /// ```
  37 declare_lint! {
  38     pub TOPLEVEL_REF_ARG,
  39     Warn,
  40     "an entire binding declared as `ref`, in a function argument or a `let` statement"
  41 }
  42
  43 /// **What it does:** Checks for comparisons to NaN.
  44 ///
  45 /// **Why is this bad?** NaN does not compare meaningfully to anything – not
  46 /// even itself – so those comparisons are simply wrong.
  47 ///
  48 /// **Known problems:** None.
  49 ///
  50 /// **Example:**
  51 /// ```rust
  52 /// x == NAN
  53 /// ```
  54 declare_lint! {
  55     pub CMP_NAN,
  56     Deny,
  57     "comparisons to NAN, which will always return false, probably not intended"
  58 }
  59
  60 /// **What it does:** Checks for (in-)equality comparisons on floating-point
  61 /// values (apart from zero), except in functions called `*eq*` (which probably
  62 /// implement equality for a type involving floats).
  63 ///
  64 /// **Why is this bad?** Floating point calculations are usually imprecise, so
  65 /// asking if two values are *exactly* equal is asking for trouble. For a good
  66 /// guide on what to do, see [the floating point
  67 /// guide](http://www.floating-point-gui.de/errors/comparison).
  68 ///
  69 /// **Known problems:** None.
  70 ///
  71 /// **Example:**
  72 /// ```rust
  73 /// y == 1.23f64
  74 /// y != x  // where both are floats
  75 /// ```
  76 declare_lint! {
  77     pub FLOAT_CMP,
  78     Warn,
  79     "using `==` or `!=` on float values instead of comparing difference with an epsilon"
  80 }
  81
  82 /// **What it does:** Checks for conversions to owned values just for the sake
  83 /// of a comparison.
  84 ///
  85 /// **Why is this bad?** The comparison can operate on a reference, so creating
  86 /// an owned value effectively throws it away directly afterwards, which is
  87 /// needlessly consuming code and heap space.
  88 ///
  89 /// **Known problems:** None.
  90 ///
  91 /// **Example:**
  92 /// ```rust
  93 /// x.to_owned() == y
  94 /// ```
  95 declare_lint! {
  96     pub CMP_OWNED,
  97     Warn,
  98     "creating owned instances for comparing with others, e.g. `x == \"foo\".to_string()`"
  99 }
 100
 101 /// **What it does:** Checks for getting the remainder of a division by one.
 102 ///
 103 /// **Why is this bad?** The result can only ever be zero. No one will write
 104 /// such code deliberately, unless trying to win an Underhanded Rust
 105 /// Contest. Even for that contest, it's probably a bad idea. Use something more
 106 /// underhanded.
 107 ///
 108 /// **Known problems:** None.
 109 ///
 110 /// **Example:**
 111 /// ```rust
 112 /// x % 1
 113 /// ```
 114 declare_lint! {
 115     pub MODULO_ONE,
 116     Warn,
 117     "taking a number modulo 1, which always returns 0"
 118 }
 119
 120 /// **What it does:** Checks for patterns in the form `name @ _`.
 121 ///
 122 /// **Why is this bad?** It's almost always more readable to just use direct
 123 /// bindings.
 124 ///
 125 /// **Known problems:** None.
 126 ///
 127 /// **Example:**
 128 /// ```rust
 129 /// match v {
 130 ///     Some(x) => (),
 131 ///     y @ _   => (), // easier written as `y`,
 132 /// }
 133 /// ```
 134 declare_lint! {
 135     pub REDUNDANT_PATTERN,
 136     Warn,
 137     "using `name @ _` in a pattern"
 138 }
 139
 140 /// **What it does:** Checks for the use of bindings with a single leading
 141 /// underscore.
 142 ///
 143 /// **Why is this bad?** A single leading underscore is usually used to indicate
 144 /// that a binding will not be used. Using such a binding breaks this
 145 /// expectation.
 146 ///
 147 /// **Known problems:** The lint does not work properly with desugaring and
 148 /// macro, it has been allowed in the mean time.
 149 ///
 150 /// **Example:**
 151 /// ```rust
 152 /// let _x = 0;
 153 /// let y = _x + 1; // Here we are using `_x`, even though it has a leading
 154 /// underscore.
 155 ///                 // We should rename `_x` to `x`
 156 /// ```
 157 declare_lint! {
 158     pub USED_UNDERSCORE_BINDING,
 159     Allow,
 160     "using a binding which is prefixed with an underscore"
 161 }
 162
 163 /// **What it does:** Checks for the use of short circuit boolean conditions as
 164 /// a
 165 /// statement.
 166 ///
 167 /// **Why is this bad?** Using a short circuit boolean condition as a statement
 168 /// may
 169 /// hide the fact that the second part is executed or not depending on the
 170 /// outcome of
 171 /// the first part.
 172 ///
 173 /// **Known problems:** None.
 174 ///
 175 /// **Example:**
 176 /// ```rust
 177 /// f() && g();  // We should write `if f() { g(); }`.
 178 /// ```
 179 declare_lint! {
 180     pub SHORT_CIRCUIT_STATEMENT,
 181     Warn,
 182     "using a short circuit boolean condition as a statement"
 183 }
 184
 185 /// **What it does:** Catch casts from `0` to some pointer type
 186 ///
 187 /// **Why is this bad?** This generally means `null` and is better expressed as
 188 /// {`std`, `core`}`::ptr::`{`null`, `null_mut`}.
 189 ///
 190 /// **Known problems:** None.
 191 ///
 192 /// **Example:**
 193 ///
 194 /// ```rust
 195 /// 0 as *const u32
 196 /// ```
 197 declare_lint! {
 198     pub ZERO_PTR,
 199     Warn,
 200     "using 0 as *{const, mut} T"
 201 }
 202
 203 #[derive(Copy, Clone)]
 204 pub struct Pass;
 205
 206 impl LintPass for Pass {
 207     fn get_lints(&self) -> LintArray {
 208         lint_array!(
 209             TOPLEVEL_REF_ARG,
 210             CMP_NAN,
 211             FLOAT_CMP,
 212             CMP_OWNED,
 213             MODULO_ONE,
 214             REDUNDANT_PATTERN,
 215             USED_UNDERSCORE_BINDING,
 216             SHORT_CIRCUIT_STATEMENT,
 217             ZERO_PTR
 218         )
 219     }
 220 }
 221
 222 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
 223     fn check_fn(
 224         &mut self,
 225         cx: &LateContext<'a, 'tcx>,
 226         k: FnKind<'tcx>,
 227         decl: &'tcx FnDecl,
 228         body: &'tcx Body,
 229         _: Span,
 230         _: NodeId,
 231     ) {
 232         if let FnKind::Closure(_) = k {
 233             // Does not apply to closures
 234             return;
 235         }
 236         for arg in iter_input_pats(decl, body) {
 237             match arg.pat.node {
 238                 PatKind::Binding(BindingAnnotation::Ref, _, _, _) |
 239                 PatKind::Binding(BindingAnnotation::RefMut, _, _, _) => {
 240                     span_lint(
 241                         cx,
 242                         TOPLEVEL_REF_ARG,
 243                         arg.pat.span,
 244                         "`ref` directly on a function argument is ignored. Consider using a reference type \
 245                          instead.",
 246                     );
 247                 },
 248                 _ => {},
 249             }
 250         }
 251     }
 252
 253     fn check_stmt(&mut self, cx: &LateContext<'a, 'tcx>, s: &'tcx Stmt) {
 254         if_chain! {
 255             if let StmtDecl(ref d, _) = s.node;
 256             if let DeclLocal(ref l) = d.node;
 257             if let PatKind::Binding(an, _, i, None) = l.pat.node;
 258             if let Some(ref init) = l.init;
 259             then {
 260                 if an == BindingAnnotation::Ref || an == BindingAnnotation::RefMut {
 261                     let init = Sugg::hir(cx, init, "..");
 262                     let (mutopt,initref) = if an == BindingAnnotation::RefMut {
 263                         ("mut ", init.mut_addr())
 264                     } else {
 265                         ("", init.addr())
 266                     };
 267                     let tyopt = if let Some(ref ty) = l.ty {
 268                         format!(": &{mutopt}{ty}", mutopt=mutopt, ty=snippet(cx, ty.span, "_"))
 269                     } else {
 270                         "".to_owned()
 271                     };
 272                     span_lint_and_then(cx,
 273                         TOPLEVEL_REF_ARG,
 274                         l.pat.span,
 275                         "`ref` on an entire `let` pattern is discouraged, take a reference with `&` instead",
 276                         |db| {
 277                             db.span_suggestion(s.span,
 278                                                "try",
 279                                                format!("let {name}{tyopt} = {initref};",
 280                                                        name=snippet(cx, i.span, "_"),
 281                                                        tyopt=tyopt,
 282                                                        initref=initref));
 283                         }
 284                     );
 285                 }
 286             }
 287         };
 288         if_chain! {
 289             if let StmtSemi(ref expr, _) = s.node;
 290             if let Expr_::ExprBinary(ref binop, ref a, ref b) = expr.node;
 291             if binop.node == BiAnd || binop.node == BiOr;
 292             if let Some(sugg) = Sugg::hir_opt(cx, a);
 293             then {
 294                 span_lint_and_then(cx,
 295                     SHORT_CIRCUIT_STATEMENT,
 296                     s.span,
 297                     "boolean short circuit operator in statement may be clearer using an explicit test",
 298                     |db| {
 299                         let sugg = if binop.node == BiOr { !sugg } else { sugg };
 300                         db.span_suggestion(s.span, "replace it with",
 301                                            format!("if {} {{ {}; }}", sugg, &snippet(cx, b.span, "..")));
 302                     });
 303             }
 304         };
 305     }
 306
 307     fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
 308         match expr.node {
 309             ExprCast(ref e, ref ty) => {
 310                 check_cast(cx, expr.span, e, ty);
 311                 return;
 312             },
 313             ExprBinary(ref cmp, ref left, ref right) => {
 314                 let op = cmp.node;
 315                 if op.is_comparison() {
 316                     if let ExprPath(QPath::Resolved(_, ref path)) = left.node {
 317                         check_nan(cx, path, expr);
 318                     }
 319                     if let ExprPath(QPath::Resolved(_, ref path)) = right.node {
 320                         check_nan(cx, path, expr);
 321                     }
 322                     check_to_owned(cx, left, right);
 323                     check_to_owned(cx, right, left);
 324                 }
 325                 if (op == BiEq || op == BiNe) && (is_float(cx, left) || is_float(cx, right)) {
 326                     if is_allowed(cx, left) || is_allowed(cx, right) {
 327                         return;
 328                     }
 329                     if let Some(name) = get_item_name(cx, expr) {
 330                         let name = name.as_str();
 331                         if name == "eq" || name == "ne" || name == "is_nan" || name.starts_with("eq_")
 332                             || name.ends_with("_eq")
 333                         {
 334                             return;
 335                         }
 336                     }
 337                     span_lint_and_then(cx, FLOAT_CMP, expr.span, "strict comparison of f32 or f64", |db| {
 338                         let lhs = Sugg::hir(cx, left, "..");
 339                         let rhs = Sugg::hir(cx, right, "..");
 340
 341                         db.span_suggestion(
 342                             expr.span,
 343                             "consider comparing them within some error",
 344                             format!("({}).abs() < error", lhs - rhs),
 345                         );
 346                         db.span_note(expr.span, "std::f32::EPSILON and std::f64::EPSILON are available.");
 347                     });
 348                 } else if op == BiRem && is_integer_literal(right, 1) {
 349                     span_lint(cx, MODULO_ONE, expr.span, "any number modulo 1 will be 0");
 350                 }
 351             },
 352             _ => {},
 353         }
 354         if in_attributes_expansion(expr) {
 355             // Don't lint things expanded by #[derive(...)], etc
 356             return;
 357         }
 358         let binding = match expr.node {
 359             ExprPath(ref qpath) => {
 360                 let binding = last_path_segment(qpath).name.as_str();
 361                 if binding.starts_with('_') &&
 362                     !binding.starts_with("__") &&
 363                     binding != "_result" && // FIXME: #944
 364                     is_used(cx, expr) &&
 365                     // don't lint if the declaration is in a macro
 366                     non_macro_local(cx, &cx.tables.qpath_def(qpath, expr.hir_id))
 367                 {
 368                     Some(binding)
 369                 } else {
 370                     None
 371                 }
 372             },
 373             ExprField(_, spanned) => {
 374                 let name = spanned.node.as_str();
 375                 if name.starts_with('_') && !name.starts_with("__") {
 376                     Some(name)
 377                 } else {
 378                     None
 379                 }
 380             },
 381             _ => None,
 382         };
 383         if let Some(binding) = binding {
 384             span_lint(
 385                 cx,
 386                 USED_UNDERSCORE_BINDING,
 387                 expr.span,
 388                 &format!(
 389                     "used binding `{}` which is prefixed with an underscore. A leading \
 390                      underscore signals that a binding will not be used.",
 391                     binding
 392                 ),
 393             );
 394         }
 395     }
 396
 397     fn check_pat(&mut self, cx: &LateContext<'a, 'tcx>, pat: &'tcx Pat) {
 398         if let PatKind::Binding(_, _, ref ident, Some(ref right)) = pat.node {
 399             if right.node == PatKind::Wild {
 400                 span_lint(
 401                     cx,
 402                     REDUNDANT_PATTERN,
 403                     pat.span,
 404                     &format!("the `{} @ _` pattern can be written as just `{}`", ident.node, ident.node),
 405                 );
 406             }
 407         }
 408     }
 409 }
 410
 411 fn check_nan(cx: &LateContext, path: &Path, expr: &Expr) {
 412     if !in_constant(cx, expr.id) {
 413         path.segments.last().map(|seg| {
 414             if seg.name == "NAN" {
 415                 span_lint(
 416                     cx,
 417                     CMP_NAN,
 418                     expr.span,
 419                     "doomed comparison with NAN, use `std::{f32,f64}::is_nan()` instead",
 420                 );
 421             }
 422         });
 423     }
 424 }
 425
 426 fn is_allowed<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) -> bool {
 427     let parent_item = cx.tcx.hir.get_parent(expr.id);
 428     let parent_def_id = cx.tcx.hir.local_def_id(parent_item);
 429     let substs = Substs::identity_for_item(cx.tcx, parent_def_id);
 430     let res = ConstContext::new(cx.tcx, cx.param_env.and(substs), cx.tables).eval(expr);
 431     if let Ok(&ty::Const {
 432         val: ConstVal::Float(val),
 433         ..
 434     }) = res
 435     {
 436         use std::cmp::Ordering;
 437         match val.ty {
 438             FloatTy::F32 => {
 439                 let zero = ConstFloat {
 440                     ty: FloatTy::F32,
 441                     bits: u128::from(0.0_f32.to_bits()),
 442                 };
 443
 444                 let infinity = ConstFloat {
 445                     ty: FloatTy::F32,
 446                     bits: u128::from(::std::f32::INFINITY.to_bits()),
 447                 };
 448
 449                 let neg_infinity = ConstFloat {
 450                     ty: FloatTy::F32,
 451                     bits: u128::from(::std::f32::NEG_INFINITY.to_bits()),
 452                 };
 453
 454                 val.try_cmp(zero) == Ok(Ordering::Equal) || val.try_cmp(infinity) == Ok(Ordering::Equal)
 455                     || val.try_cmp(neg_infinity) == Ok(Ordering::Equal)
 456             },
 457             FloatTy::F64 => {
 458                 let zero = ConstFloat {
 459                     ty: FloatTy::F64,
 460                     bits: u128::from(0.0_f64.to_bits()),
 461                 };
 462
 463                 let infinity = ConstFloat {
 464                     ty: FloatTy::F64,
 465                     bits: u128::from(::std::f64::INFINITY.to_bits()),
 466                 };
 467
 468                 let neg_infinity = ConstFloat {
 469                     ty: FloatTy::F64,
 470                     bits: u128::from(::std::f64::NEG_INFINITY.to_bits()),
 471                 };
 472
 473                 val.try_cmp(zero) == Ok(Ordering::Equal) || val.try_cmp(infinity) == Ok(Ordering::Equal)
 474                     || val.try_cmp(neg_infinity) == Ok(Ordering::Equal)
 475             },
 476         }
 477     } else {
 478         false
 479     }
 480 }
 481
 482 fn is_float(cx: &LateContext, expr: &Expr) -> bool {
 483     matches!(walk_ptrs_ty(cx.tables.expr_ty(expr)).sty, ty::TyFloat(_))
 484 }
 485
 486 fn check_to_owned(cx: &LateContext, expr: &Expr, other: &Expr) {
 487     let (arg_ty, snip) = match expr.node {
 488         ExprMethodCall(.., ref args) if args.len() == 1 => {
 489             if match_trait_method(cx, expr, &paths::TO_STRING) || match_trait_method(cx, expr, &paths::TO_OWNED) {
 490                 (cx.tables.expr_ty_adjusted(&args[0]), snippet(cx, args[0].span, ".."))
 491             } else {
 492                 return;
 493             }
 494         },
 495         ExprCall(ref path, ref v) if v.len() == 1 => if let ExprPath(ref path) = path.node {
 496             if match_qpath(path, &["String", "from_str"]) || match_qpath(path, &["String", "from"]) {
 497                 (cx.tables.expr_ty_adjusted(&v[0]), snippet(cx, v[0].span, ".."))
 498             } else {
 499                 return;
 500             }
 501         } else {
 502             return;
 503         },
 504         _ => return,
 505     };
 506
 507     let other_ty = cx.tables.expr_ty_adjusted(other);
 508     let partial_eq_trait_id = match cx.tcx.lang_items().eq_trait() {
 509         Some(id) => id,
 510         None => return,
 511     };
 512
 513     // *arg impls PartialEq<other>
 514     if !arg_ty
 515         .builtin_deref(true, ty::LvaluePreference::NoPreference)
 516         .map_or(false, |tam| implements_trait(cx, tam.ty, partial_eq_trait_id, &[other_ty]))
 517         // arg impls PartialEq<*other>
 518         && !other_ty
 519         .builtin_deref(true, ty::LvaluePreference::NoPreference)
 520         .map_or(false, |tam| implements_trait(cx, arg_ty, partial_eq_trait_id, &[tam.ty]))
 521         // arg impls PartialEq<other>
 522         && !implements_trait(cx, arg_ty, partial_eq_trait_id, &[other_ty])
 523     {
 524         return;
 525     }
 526
 527     span_lint_and_then(
 528         cx,
 529         CMP_OWNED,
 530         expr.span,
 531         "this creates an owned instance just for comparison",
 532         |db| {
 533             // this is as good as our recursion check can get, we can't prove that the
 534             // current function is
 535             // called by
 536             // PartialEq::eq, but we can at least ensure that this code is not part of it
 537             let parent_fn = cx.tcx.hir.get_parent(expr.id);
 538             let parent_impl = cx.tcx.hir.get_parent(parent_fn);
 539             if parent_impl != CRATE_NODE_ID {
 540                 if let map::NodeItem(item) = cx.tcx.hir.get(parent_impl) {
 541                     if let ItemImpl(.., Some(ref trait_ref), _, _) = item.node {
 542                         if trait_ref.path.def.def_id() == partial_eq_trait_id {
 543                             // we are implementing PartialEq, don't suggest not doing `to_owned`, otherwise
 544                             // we go into
 545                             // recursion
 546                             db.span_label(expr.span, "try calling implementing the comparison without allocating");
 547                             return;
 548                         }
 549                     }
 550                 }
 551             }
 552             db.span_suggestion(expr.span, "try", snip.to_string());
 553         },
 554     );
 555 }
 556
 557 /// Heuristic to see if an expression is used. Should be compatible with
 558 /// `unused_variables`'s idea
 559 /// of what it means for an expression to be "used".
 560 fn is_used(cx: &LateContext, expr: &Expr) -> bool {
 561     if let Some(parent) = get_parent_expr(cx, expr) {
 562         match parent.node {
 563             ExprAssign(_, ref rhs) | ExprAssignOp(_, _, ref rhs) => **rhs == *expr,
 564             _ => is_used(cx, parent),
 565         }
 566     } else {
 567         true
 568     }
 569 }
 570
 571 /// Test whether an expression is in a macro expansion (e.g. something
 572 /// generated by
 573 /// `#[derive(...)`] or the like).
 574 fn in_attributes_expansion(expr: &Expr) -> bool {
 575     expr.span
 576         .ctxt()
 577         .outer()
 578         .expn_info()
 579         .map_or(false, |info| matches!(info.callee.format, ExpnFormat::MacroAttribute(_)))
 580 }
 581
 582 /// Test whether `def` is a variable defined outside a macro.
 583 fn non_macro_local(cx: &LateContext, def: &def::Def) -> bool {
 584     match *def {
 585         def::Def::Local(id) | def::Def::Upvar(id, _, _) => !in_macro(cx.tcx.hir.span(id)),
 586         _ => false,
 587     }
 588 }
 589
 590 fn check_cast(cx: &LateContext, span: Span, e: &Expr, ty: &Ty) {
 591     if_chain! {
 592         if let TyPtr(MutTy { mutbl, .. }) = ty.node;
 593         if let ExprLit(ref lit) = e.node;
 594         if let LitKind::Int(value, ..) = lit.node;
 595         if value == 0;
 596         if !in_constant(cx, e.id);
 597         then {
 598             let msg = match mutbl {
 599                 Mutability::MutMutable => "`0 as *mut _` detected. Consider using `ptr::null_mut()`",
 600                 Mutability::MutImmutable => "`0 as *const _` detected. Consider using `ptr::null()`",
 601             };
 602             span_lint(cx, ZERO_PTR, span, msg);
 603         }
 604     }
 605 }