From 87e6099ad7eb145c8fb2704d267eeaa3fdba2a2f Mon Sep 17 00:00:00 2001 From: Andre Bogus Date: Sun, 6 Sep 2015 20:57:06 +0200 Subject: [PATCH] fix false positive len_zero in is_empty() --- src/len_zero.rs | 22 +++++++++++++--------- src/misc.rs | 21 ++++++--------------- src/utils.rs | 15 ++++++++++++++- 3 files changed, 33 insertions(+), 25 deletions(-) diff --git a/src/len_zero.rs b/src/len_zero.rs index c30a98d8537..206f18217d1 100644 --- a/src/len_zero.rs +++ b/src/len_zero.rs @@ -5,7 +5,7 @@ use syntax::codemap::{Span, Spanned}; use rustc::middle::def_id::DefId; use rustc::middle::ty::{self, MethodTraitItemId, ImplOrTraitItemId}; -use utils::{span_lint, walk_ptrs_ty, snippet}; +use utils::{snippet, span_lint, walk_ptrs_ty, with_item_name}; declare_lint!(pub LEN_ZERO, Warn, "checking `.len() == 0` or `.len() > 0` (or similar) when `.is_empty()` \ @@ -33,14 +33,14 @@ impl LintPass for LenZero { } fn check_expr(&mut self, cx: &Context, expr: &Expr) { - if let &ExprBinary(Spanned{node: cmp, ..}, ref left, ref right) = - &expr.node { - match cmp { - BiEq => check_cmp(cx, expr.span, left, right, ""), - BiGt | BiNe => check_cmp(cx, expr.span, left, right, "!"), - _ => () - } + if let ExprBinary(Spanned{node: cmp, ..}, ref left, ref right) = + expr.node { + match cmp { + BiEq => check_cmp(cx, expr.span, left, right, ""), + BiGt | BiNe => check_cmp(cx, expr.span, left, right, "!"), + _ => () } + } } } @@ -89,7 +89,11 @@ fn is_self_sig(sig: &MethodSig) -> bool { false } else { sig.decl.inputs.len() == 1 } } -fn check_cmp(cx: &Context, span: Span, left: &Expr, right: &Expr, op: &str) { +fn check_cmp(cx: &Context, span: Span, left: &Expr, right: &Expr, op: &str) { + // check if we are in an is_empty() method + if let Some(true) = with_item_name(cx, left, |n| n == "is_empty") { + return; + } match (&left.node, &right.node) { (&ExprLit(ref lit), &ExprMethodCall(ref method, _, ref args)) => check_len_zero(cx, span, method, args, lit, op), diff --git a/src/misc.rs b/src/misc.rs index eb3a93941be..87cc512f819 100644 --- a/src/misc.rs +++ b/src/misc.rs @@ -5,10 +5,9 @@ use reexport::*; use rustc_front::util::{is_comparison_binop, binop_to_string}; use syntax::codemap::{Span, Spanned}; use rustc_front::visit::FnKind; -use rustc::front::map::Node::*; use rustc::middle::ty; -use utils::{match_path, snippet, span_lint, walk_ptrs_ty}; +use utils::{match_path, snippet, span_lint, walk_ptrs_ty, with_item_name}; use consts::constant; declare_lint!(pub TOPLEVEL_REF_ARG, Warn, @@ -93,19 +92,11 @@ impl LintPass for FloatCmp { false, |c| c.0.as_float().map_or(false, |f| f == 0.0)) { return; } - let parent_id = cx.tcx.map.get_parent(expr.id); - match cx.tcx.map.find(parent_id) { - Some(NodeItem(&Item{ ref ident, .. })) | - Some(NodeTraitItem(&TraitItem{ id: _, ref ident, .. })) | - Some(NodeImplItem(&ImplItem{ id: _, ref ident, .. })) => { - let name = ident.name.as_str(); - if &*name == "eq" || &*name == "ne" || - name.starts_with("eq_") || - name.ends_with("_eq") { - return; - } - }, - _ => (), + if let Some(true) = with_item_name(cx, expr, |name| + name == "eq" || name == "ne" || + name.as_str().starts_with("eq_") || + name.as_str().ends_with("_eq")) { + return; } span_lint(cx, FLOAT_CMP, expr.span, &format!( "{}-comparison of f32 or f64 detected. Consider changing this to \ diff --git a/src/utils.rs b/src/utils.rs index 33e53710df3..c781b74b359 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -2,7 +2,7 @@ use rustc::lint::*; use rustc_front::hir::*; use reexport::*; use syntax::codemap::{ExpnInfo, Span, ExpnFormat}; -use rustc::front::map::Node::NodeExpr; +use rustc::front::map::Node::*; use rustc::middle::def_id::DefId; use rustc::middle::ty; use std::borrow::Cow; @@ -100,6 +100,19 @@ pub fn match_path(path: &Path, segments: &[&str]) -> bool { |(a, b)| &a.identifier.name == b) } +pub fn with_item_name(cx: &Context, expr: &Expr, f: F) -> Option +where F: FnOnce(Name) -> T { + let parent_id = cx.tcx.map.get_parent(expr.id); + match cx.tcx.map.find(parent_id) { + Some(NodeItem(&Item{ ref ident, .. })) | + Some(NodeTraitItem(&TraitItem{ id: _, ref ident, .. })) | + Some(NodeImplItem(&ImplItem{ id: _, ref ident, .. })) => { + Some(f(ident.name)) + }, + _ => None, + } +} + /// convert a span to a code snippet if available, otherwise use default, e.g. /// `snippet(cx, expr.span, "..")` pub fn snippet<'a>(cx: &Context, span: Span, default: &'a str) -> Cow<'a, str> {