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