lint on binding-names that are too similar

README.md

@@ -14,7 +14,7 @@ Table of contents:
 * [License](#license)
 
 ##Lints
-There are 136 lints included in this crate:
+There are 138 lints included in this crate:
 
 name                                                                                                                 | default | meaning
 ---------------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
@@ -74,6 +74,7 @@ name
 [let_unit_value](https://github.com/Manishearth/rust-clippy/wiki#let_unit_value)                                     | warn    | creating a let binding to a value of unit type, which usually can't be used afterwards
 [linkedlist](https://github.com/Manishearth/rust-clippy/wiki#linkedlist)                                             | warn    | usage of LinkedList, usually a vector is faster, or a more specialized data structure like a VecDeque
 [manual_swap](https://github.com/Manishearth/rust-clippy/wiki#manual_swap)                                           | warn    | manual swap
+[many_single_char_names](https://github.com/Manishearth/rust-clippy/wiki#many_single_char_names)                     | warn    | too many single character bindings
 [map_clone](https://github.com/Manishearth/rust-clippy/wiki#map_clone)                                               | warn    | using `.map(|x| x.clone())` to clone an iterator or option's contents (recommends `.cloned()` instead)
 [map_entry](https://github.com/Manishearth/rust-clippy/wiki#map_entry)                                               | warn    | use of `contains_key` followed by `insert` on a `HashMap` or `BTreeMap`
 [match_bool](https://github.com/Manishearth/rust-clippy/wiki#match_bool)                                             | warn    | a match on boolean expression; recommends `if..else` block instead
@@ -119,6 +120,7 @@ name
 [shadow_same](https://github.com/Manishearth/rust-clippy/wiki#shadow_same)                                           | allow   | rebinding a name to itself, e.g. `let mut x = &mut x`
 [shadow_unrelated](https://github.com/Manishearth/rust-clippy/wiki#shadow_unrelated)                                 | allow   | The name is re-bound without even using the original value
 [should_implement_trait](https://github.com/Manishearth/rust-clippy/wiki#should_implement_trait)                     | warn    | defining a method that should be implementing a std trait
+[similar_names](https://github.com/Manishearth/rust-clippy/wiki#similar_names)                                       | warn    | similarly named items and bindings
 [single_char_pattern](https://github.com/Manishearth/rust-clippy/wiki#single_char_pattern)                           | warn    | using a single-character str where a char could be used, e.g. `_.split("x")`
 [single_match](https://github.com/Manishearth/rust-clippy/wiki#single_match)                                         | warn    | a match statement with a single nontrivial arm (i.e, where the other arm is `_ => {}`) is used; recommends `if let` instead
 [single_match_else](https://github.com/Manishearth/rust-clippy/wiki#single_match_else)                               | allow   | a match statement with a two arms where the second arm's pattern is a wildcard; recommends `if let` instead

src/consts.rs

@@ -82,7 +82,7 @@ impl Constant {
 impl PartialEq for Constant {
     fn eq(&self, other: &Constant) -> bool {
         match (self, other) {
-            (&Constant::Str(ref ls, ref lsty), &Constant::Str(ref rs, ref rsty)) => ls == rs && lsty == rsty,
+            (&Constant::Str(ref ls, ref l_sty), &Constant::Str(ref rs, ref r_sty)) => ls == rs && l_sty == r_sty,
             (&Constant::Binary(ref l), &Constant::Binary(ref r)) => l == r,
             (&Constant::Char(l), &Constant::Char(r)) => l == r,
             (&Constant::Int(l), &Constant::Int(r)) => l.is_negative() == r.is_negative() && l.to_u64_unchecked() == r.to_u64_unchecked(),
@@ -145,8 +145,8 @@ impl Hash for Constant {
 impl PartialOrd for Constant {
     fn partial_cmp(&self, other: &Constant) -> Option<Ordering> {
         match (self, other) {
-            (&Constant::Str(ref ls, ref lsty), &Constant::Str(ref rs, ref rsty)) => {
-                if lsty == rsty {
+            (&Constant::Str(ref ls, ref l_sty), &Constant::Str(ref rs, ref r_sty)) => {
+                if l_sty == r_sty {
                     Some(ls.cmp(rs))
                 } else {
                     None
@@ -354,6 +354,7 @@ impl<'c, 'cc> ConstEvalLateContext<'c, 'cc> {
         }
     }
 
+
     fn binop_apply<F>(&mut self, left: &Expr, right: &Expr, op: F) -> Option<Constant>
         where F: Fn(Constant, Constant) -> Option<Constant>
     {

src/lib.rs

@@ -3,6 +3,7 @@
 #![feature(rustc_private, collections)]
 #![feature(iter_arith)]
 #![feature(custom_attribute)]
+#![feature(slice_patterns)]
 #![allow(indexing_slicing, shadow_reuse, unknown_lints)]
 
 // this only exists to allow the "dogfood" integration test to work
@@ -84,6 +85,7 @@ pub mod needless_features;
 pub mod needless_update;
 pub mod new_without_default;
 pub mod no_effect;
+pub mod non_expressive_names;
 pub mod open_options;
 pub mod overflow_check_conditional;
 pub mod panic;
@@ -200,6 +202,9 @@ pub fn plugin_registrar(reg: &mut Registry) {
     reg.register_late_lint_pass(box types::CharLitAsU8);
     reg.register_late_lint_pass(box print::PrintLint);
     reg.register_late_lint_pass(box vec::UselessVec);
+    reg.register_early_lint_pass(box non_expressive_names::NonExpressiveNames {
+        max_single_char_names: conf.max_single_char_names,
+    });
     reg.register_late_lint_pass(box drop_ref::DropRefPass);
     reg.register_late_lint_pass(box types::AbsurdExtremeComparisons);
     reg.register_late_lint_pass(box regex::RegexPass::default());
@@ -326,6 +331,8 @@ pub fn plugin_registrar(reg: &mut Registry) {
         needless_update::NEEDLESS_UPDATE,
         new_without_default::NEW_WITHOUT_DEFAULT,
         no_effect::NO_EFFECT,
+        non_expressive_names::MANY_SINGLE_CHAR_NAMES,
+        non_expressive_names::SIMILAR_NAMES,
         open_options::NONSENSICAL_OPEN_OPTIONS,
         overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL,
         panic::PANIC_PARAMS,

src/needless_bool.rs

@@ -6,7 +6,7 @@ use rustc::lint::*;
 use rustc_front::hir::*;
 use syntax::ast::LitKind;
 use syntax::codemap::Spanned;
-use utils::{span_lint, span_lint_and_then, snippet};
+use utils::{span_lint, span_lint_and_then, snippet, snippet_opt};
 
 /// **What it does:** This lint checks for expressions of the form `if c { true } else { false }` (or vice versa) and suggest using the condition directly.
 ///
@@ -47,44 +47,39 @@ impl LintPass for NeedlessBool {
 
 impl LateLintPass for NeedlessBool {
     fn check_expr(&mut self, cx: &LateContext, e: &Expr) {
+        use self::Expression::*;
         if let ExprIf(ref pred, ref then_block, Some(ref else_expr)) = e.node {
+            let reduce = |hint: &str, not| {
+                let hint = match snippet_opt(cx, pred.span) {
+                    Some(pred_snip) => format!("`{}{}`", not, pred_snip),
+                    None => hint.into(),
+                };
+                span_lint_and_then(cx,
+                                   NEEDLESS_BOOL,
+                                   e.span,
+                                   "this if-then-else expression returns a bool literal", |db| {
+                    db.span_suggestion(e.span, "you can reduce it to", hint);
+                });
+            };
             match (fetch_bool_block(then_block), fetch_bool_expr(else_expr)) {
-                (Some(true), Some(true)) => {
+                (RetBool(true), RetBool(true)) |
+                (Bool(true), Bool(true)) => {
                     span_lint(cx,
                               NEEDLESS_BOOL,
                               e.span,
                               "this if-then-else expression will always return true");
                 }
-                (Some(false), Some(false)) => {
+                (RetBool(false), RetBool(false)) |
+                (Bool(false), Bool(false)) => {
                     span_lint(cx,
                               NEEDLESS_BOOL,
                               e.span,
                               "this if-then-else expression will always return false");
                 }
-                (Some(true), Some(false)) => {
-                    let pred_snip = snippet(cx, pred.span, "..");
-                    let hint = if pred_snip == ".." {
-                        "its predicate".into()
-                    } else {
-                        format!("`{}`", pred_snip)
-                    };
-                    span_lint(cx,
-                              NEEDLESS_BOOL,
-                              e.span,
-                              &format!("you can reduce this if-then-else expression to just {}", hint));
-                }
-                (Some(false), Some(true)) => {
-                    let pred_snip = snippet(cx, pred.span, "..");
-                    let hint = if pred_snip == ".." {
-                        "`!` and its predicate".into()
-                    } else {
-                        format!("`!{}`", pred_snip)
-                    };
-                    span_lint(cx,
-                              NEEDLESS_BOOL,
-                              e.span,
-                              &format!("you can reduce this if-then-else expression to just {}", hint));
-                }
+                (RetBool(true), RetBool(false)) => reduce("its predicate", "return "),
+                (Bool(true), Bool(false)) => reduce("its predicate", ""),
+                (RetBool(false), RetBool(true)) => reduce("`!` and its predicate", "return !"),
+                (Bool(false), Bool(true)) => reduce("`!` and its predicate", "!"),
                 _ => (),
             }
         }
@@ -102,9 +97,10 @@ impl LintPass for BoolComparison {
 
 impl LateLintPass for BoolComparison {
     fn check_expr(&mut self, cx: &LateContext, e: &Expr) {
+        use self::Expression::*;
         if let ExprBinary(Spanned{ node: BiEq, .. }, ref left_side, ref right_side) = e.node {
             match (fetch_bool_expr(left_side), fetch_bool_expr(right_side)) {
-                (Some(true), None) => {
+                (Bool(true), Other) => {
                     let hint = snippet(cx, right_side.span, "..").into_owned();
                     span_lint_and_then(cx,
                                        BOOL_COMPARISON,
@@ -114,7 +110,7 @@ impl LateLintPass for BoolComparison {
                                            db.span_suggestion(e.span, "try simplifying it as shown:", hint);
                                        });
                 }
-                (None, Some(true)) => {
+                (Other, Bool(true)) => {
                     let hint = snippet(cx, left_side.span, "..").into_owned();
                     span_lint_and_then(cx,
                                        BOOL_COMPARISON,
@@ -124,7 +120,7 @@ impl LateLintPass for BoolComparison {
                                            db.span_suggestion(e.span, "try simplifying it as shown:", hint);
                                        });
                 }
-                (Some(false), None) => {
+                (Bool(false), Other) => {
                     let hint = format!("!{}", snippet(cx, right_side.span, ".."));
                     span_lint_and_then(cx,
                                        BOOL_COMPARISON,
@@ -134,7 +130,7 @@ impl LateLintPass for BoolComparison {
                                            db.span_suggestion(e.span, "try simplifying it as shown:", hint);
                                        });
                 }
-                (None, Some(false)) => {
+                (Other, Bool(false)) => {
                     let hint = format!("!{}", snippet(cx, left_side.span, ".."));
                     span_lint_and_then(cx,
                                        BOOL_COMPARISON,
@@ -150,24 +146,42 @@ impl LateLintPass for BoolComparison {
     }
 }
 
-fn fetch_bool_block(block: &Block) -> Option<bool> {
-    if block.stmts.is_empty() {
-        block.expr.as_ref().and_then(|e| fetch_bool_expr(e))
-    } else {
-        None
+enum Expression {
+    Bool(bool),
+    RetBool(bool),
+    Other,
+}
+
+fn fetch_bool_block(block: &Block) -> Expression {
+    match (&*block.stmts, block.expr.as_ref()) {
+        ([], Some(e)) => fetch_bool_expr(&**e),
+        ([ref e], None) => if let StmtSemi(ref e, _) = e.node {
+            if let ExprRet(_) = e.node {
+                fetch_bool_expr(&**e)
+            } else {
+                Expression::Other
+            }
+        } else {
+            Expression::Other
+        },
+        _ => Expression::Other,
     }
 }
 
-fn fetch_bool_expr(expr: &Expr) -> Option<bool> {
+fn fetch_bool_expr(expr: &Expr) -> Expression {
     match expr.node {
         ExprBlock(ref block) => fetch_bool_block(block),
         ExprLit(ref lit_ptr) => {
             if let LitKind::Bool(value) = lit_ptr.node {
-                Some(value)
+                Expression::Bool(value)
             } else {
-                None
+                Expression::Other
             }
-        }
-        _ => None,
+        },
+        ExprRet(Some(ref expr)) => match fetch_bool_expr(expr) {
+            Expression::Bool(value) => Expression::RetBool(value),
+            _ => Expression::Other,
+        },
+        _ => Expression::Other,
     }
 }