Don't lint nan_cmp and zero_ptr in constants

clippy_lints/src/lib.rs

@@ -445,14 +445,14 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
  misc::REDUNDANT_PATTERN,
  misc::SHORT_CIRCUIT_STATEMENT,
  misc::TOPLEVEL_REF_ARG,
+ misc::ZERO_PTR,
  misc_early::BUILTIN_TYPE_SHADOW,
  misc_early::DOUBLE_NEG,
  misc_early::DUPLICATE_UNDERSCORE_ARGUMENT,
  misc_early::MIXED_CASE_HEX_LITERALS,
  misc_early::REDUNDANT_CLOSURE_CALL,
  misc_early::UNNEEDED_FIELD_PATTERN,
  misc_early::ZERO_PREFIXED_LITERAL,
- misc_early::ZERO_PTR,
  mut_reference::UNNECESSARY_MUT_PASSED,
  mutex_atomic::MUTEX_ATOMIC,
  needless_bool::BOOL_COMPARISON,

clippy_lints/src/misc.rs

@@ -8,8 +8,9 @@ use rustc_const_eval::ConstContext;
 use rustc_const_math::ConstFloat;
 use syntax::codemap::{Span, Spanned, ExpnFormat};
 use utils::{get_item_name, get_parent_expr, implements_trait, in_macro, is_integer_literal, match_path, snippet,
- span_lint, span_lint_and_then, walk_ptrs_ty, last_path_segment, iter_input_pats};
+ span_lint, span_lint_and_then, walk_ptrs_ty, last_path_segment, iter_input_pats, in_constant};
 use utils::sugg::Sugg;
+use syntax::ast::LitKind;
 
 /// **What it does:** Checks for function arguments and let bindings denoted as `ref`.
 ///
@@ -172,6 +173,24 @@ declare_lint! {
  "using a short circuit boolean condition as a statement"
 }
 
+/// **What it does:** Catch casts from `0` to some pointer type
+///
+/// **Why is this bad?** This generally means `null` and is better expressed as
+/// {`std`, `core`}`::ptr::`{`null`, `null_mut`}.
+///
+/// **Known problems:** None.
+///
+/// **Example:**
+///
+/// ```rust
+/// 0 as *const u32
+/// ```
+declare_lint! {
+ pub ZERO_PTR,
+ Warn,
+ "using 0 as *{const, mut} T"
+}
+
 #[derive(Copy, Clone)]
 pub struct Pass;
 
@@ -184,7 +203,8 @@ impl LintPass for Pass {
  MODULO_ONE,
  REDUNDANT_PATTERN,
  USED_UNDERSCORE_BINDING,
- SHORT_CIRCUIT_STATEMENT)
+ SHORT_CIRCUIT_STATEMENT,
+ ZERO_PTR)
  }
 }
 
@@ -263,41 +283,48 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
  }
 
  fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
- if let ExprBinary(ref cmp, ref left, ref right) = expr.node {
- let op = cmp.node;
- if op.is_comparison() {
- if let ExprPath(QPath::Resolved(_, ref path)) = left.node {
- check_nan(cx, path, expr.span);
- }
- if let ExprPath(QPath::Resolved(_, ref path)) = right.node {
- check_nan(cx, path, expr.span);
- }
- check_to_owned(cx, left, right, true, cmp.span);
- check_to_owned(cx, right, left, false, cmp.span)
- }
- if (op == BiEq || op == BiNe) && (is_float(cx, left) || is_float(cx, right)) {
- if is_allowed(cx, left) || is_allowed(cx, right) {
- return;
+ match expr.node {
+ ExprCast(ref e, ref ty) => {
+ check_cast(cx, expr.span, e, ty);
+ return;
+ },
+ ExprBinary(ref cmp, ref left, ref right) => {
+ let op = cmp.node;
+ if op.is_comparison() {
+ if let ExprPath(QPath::Resolved(_, ref path)) = left.node {
+ check_nan(cx, path, expr);
+ }
+ if let ExprPath(QPath::Resolved(_, ref path)) = right.node {
+ check_nan(cx, path, expr);
+ }
+ check_to_owned(cx, left, right, true, cmp.span);
+ check_to_owned(cx, right, left, false, cmp.span)
  }
- 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") {
+ if (op == BiEq || op == BiNe) && (is_float(cx, left) || is_float(cx, right)) {
+ if is_allowed(cx, left) || is_allowed(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;
+ }
+ }
+ span_lint_and_then(cx, FLOAT_CMP, expr.span, "strict comparison of f32 or f64", |db| {
+ let lhs = Sugg::hir(cx, left, "..");
+ let rhs = Sugg::hir(cx, right, "..");
+
+ db.span_suggestion(expr.span,
+ "consider comparing them within some error",
+ format!("({}).abs() < error", lhs - rhs));
+ db.span_note(expr.span, "std::f32::EPSILON and std::f64::EPSILON are available.");
+ });
+ } else if op == BiRem && is_integer_literal(right, 1) {
+ span_lint(cx, MODULO_ONE, expr.span, "any number modulo 1 will be 0");
  }
- span_lint_and_then(cx, FLOAT_CMP, expr.span, "strict comparison of f32 or f64", |db| {
- let lhs = Sugg::hir(cx, left, "..");
- let rhs = Sugg::hir(cx, right, "..");
-
- db.span_suggestion(expr.span,
- "consider comparing them within some error",
- format!("({}).abs() < error", lhs - rhs));
- db.span_note(expr.span, "std::f32::EPSILON and std::f64::EPSILON are available.");
- });
- } else if op == BiRem && is_integer_literal(right, 1) {
- span_lint(cx, MODULO_ONE, expr.span, "any number modulo 1 will be 0");
- }
+ },
+ _ => {}
  }
  if in_attributes_expansion(cx, expr) {
  // Don't lint things expanded by #[derive(...)], etc
@@ -349,13 +376,15 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
  }
 }
 
-fn check_nan(cx: &LateContext, path: &Path, span: Span) {
- path.segments.last().map(|seg| if &*seg.name.as_str() == "NAN" {
- span_lint(cx,
- CMP_NAN,
- span,
- "doomed comparison with NAN, use `std::{f32,f64}::is_nan()` instead");
- });
+fn check_nan(cx: &LateContext, path: &Path, expr: &Expr) {
+ if !in_constant(cx, expr.id) {
+ path.segments.last().map(|seg| if &*seg.name.as_str() == "NAN" {
+ span_lint(cx,
+ CMP_NAN,
+ expr.span,
+ "doomed comparison with NAN, use `std::{f32,f64}::is_nan()` instead");
+ });
+ }
 }
 
 fn is_allowed(cx: &LateContext, expr: &Expr) -> bool {
@@ -489,3 +518,19 @@ fn non_macro_local(cx: &LateContext, def: &def::Def) -> bool {
  _ => false,
  }
 }
+
+fn check_cast(cx: &LateContext, span: Span, e: &Expr, ty: &Ty) {
+ if_let_chain! {[
+ let TyPtr(MutTy { mutbl, .. }) = ty.node,
+ let ExprLit(ref lit) = e.node,
+ let LitKind::Int(value, ..) = lit.node,
+ value == 0,
+ !in_constant(cx, e.id)
+ ], {
+ 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()`",
+ };
+ span_lint(cx, ZERO_PTR, span, msg);
+ }}
+}

clippy_lints/src/misc_early.rs

@@ -162,24 +162,6 @@ declare_lint! {
  "shadowing a builtin type"
 }
 
-/// **What it does:** Catch casts from `0` to some pointer type
-///
-/// **Why is this bad?** This generally means `null` and is better expressed as
-/// {`std`, `core`}`::ptr::`{`null`, `null_mut`}.
-///
-/// **Known problems:** None.
-///
-/// **Example:**
-///
-/// ```rust
-/// 0 as *const u32
-/// ```
-declare_lint! {
- pub ZERO_PTR,
- Warn,
- "using 0 as *{const, mut} T"
-}
-
 #[derive(Copy, Clone)]
 pub struct MiscEarly;
 
@@ -192,8 +174,7 @@ impl LintPass for MiscEarly {
  MIXED_CASE_HEX_LITERALS,
  UNSEPARATED_LITERAL_SUFFIX,
  ZERO_PREFIXED_LITERAL,
- BUILTIN_TYPE_SHADOW,
- ZERO_PTR)
+ BUILTIN_TYPE_SHADOW)
  }
 }
 
@@ -381,9 +362,6 @@ impl EarlyLintPass for MiscEarly {
  }
  }}
  },
- ExprKind::Cast(ref e, ref ty) => {
- check_cast(cx, expr.span, e, ty);
- },
  _ => (),
  }
  }
@@ -412,18 +390,3 @@ impl EarlyLintPass for MiscEarly {
  }
  }
 }
-
-fn check_cast(cx: &EarlyContext, span: Span, e: &Expr, ty: &Ty) {
- if_let_chain! {[
- let TyKind::Ptr(MutTy { mutbl, .. }) = ty.node,
- let ExprKind::Lit(ref lit) = e.node,
- let LitKind::Int(value, ..) = lit.node,
- value == 0
- ], {
- let msg = match mutbl {
- Mutability::Mutable => "`0 as *mut _` detected. Consider using `ptr::null_mut()`",
- Mutability::Immutable => "`0 as *const _` detected. Consider using `ptr::null()`",
- };
- span_lint(cx, ZERO_PTR, span, msg);
- }}
-}

clippy_lints/src/utils/mod.rs

@@ -10,6 +10,7 @@ use rustc::traits;
 use rustc::ty::subst::Subst;
 use rustc::ty;
 use rustc::ty::layout::TargetDataLayout;
+use rustc::mir::transform::MirSource;
 use rustc_errors;
 use std::borrow::Cow;
 use std::env;
@@ -98,6 +99,17 @@ pub mod higher;
 pub fn differing_macro_contexts(lhs: Span, rhs: Span) -> bool {
  rhs.expn_id != lhs.expn_id
 }
+
+pub fn in_constant(cx: &LateContext, id: NodeId) -> bool {
+ let parent_id = cx.tcx.hir.get_parent(id);
+ match MirSource::from_node(cx.tcx, parent_id) {
+ MirSource::Fn(_) => false,
+ MirSource::Const(_) |
+ MirSource::Static(..) |
+ MirSource::Promoted(..) => true,
+ }
+}
+
 /// Returns true if this `expn_info` was expanded by any macro.
 pub fn in_macro<'a, T: LintContext<'a>>(cx: &T, span: Span) -> bool {
  cx.sess().codemap().with_expn_info(span.expn_id, |info| {

tests/run-pass/mut_mut_macro.rs

@@ -1,12 +1,20 @@
 #![feature(plugin)]
 #![plugin(clippy)]
+#![deny(mut_mut, zero_ptr, cmp_nan)]
+#![allow(dead_code)]
 
 #[macro_use]
 extern crate lazy_static;
 
 use std::collections::HashMap;
 
-#[deny(mut_mut)]
+// ensure that we don't suggest `is_nan` and `is_null` inside constants
+// FIXME: once const fn is stable, suggest these functions again in constants
+const BAA: *const i32 = 0 as *const i32;
+static mut BAR: *const i32 = BAA;
+static mut FOO: *const i32 = 0 as *const i32;
+static mut BUH: bool = 42.0 < std::f32::NAN;
+
 #[allow(unused_variables, unused_mut)]
 fn main() {
  lazy_static! {
@@ -19,4 +27,6 @@ fn main() {
  static ref MUT_COUNT : usize = MUT_MAP.len();
  }
  assert!(*MUT_COUNT == 1);
+ // FIXME: don't lint in array length, requires `check_body`
+ //let _ = [""; (42.0 < std::f32::NAN) as usize];
 }