From 06ca1fc0a6d88ff5fbe0dabcc595687861c8c9b1 Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Tue, 1 Mar 2016 10:13:54 +0100 Subject: [PATCH 01/10] lint on binding-names that are too similar --- Cargo.toml | 1 + README.md | 1 + src/consts.rs | 7 +- src/lib.rs | 6 + src/non_expressive_names.rs | 150 +++++++++++++++++++++ src/types.rs | 6 +- src/utils/hir.rs | 46 +++---- src/utils/mod.rs | 4 +- tests/compile-fail/approx_const.rs | 2 +- tests/compile-fail/drop_ref.rs | 2 +- tests/compile-fail/for_loop.rs | 2 +- tests/compile-fail/len_zero.rs | 28 ++-- tests/compile-fail/methods.rs | 1 + tests/compile-fail/non_expressive_names.rs | 37 +++++ 14 files changed, 245 insertions(+), 48 deletions(-) create mode 100644 src/non_expressive_names.rs create mode 100644 tests/compile-fail/non_expressive_names.rs diff --git a/Cargo.toml b/Cargo.toml index dd675981f..844408df2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,6 +23,7 @@ regex_macros = { version = "0.1.33", optional = true } semver = "0.2.1" toml = "0.1" unicode-normalization = "0.1" +strsim = "0.4.0" [dev-dependencies] compiletest_rs = "0.1.0" diff --git a/README.md b/README.md index 52223ef09..2ee4dfc5e 100644 --- a/README.md +++ b/README.md @@ -119,6 +119,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 diff --git a/src/consts.rs b/src/consts.rs index 3e08f1b74..f04ad1d21 100644 --- a/src/consts.rs +++ b/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 { 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(&mut self, left: &Expr, right: &Expr, op: F) -> Option where F: Fn(Constant, Constant) -> Option { diff --git a/src/lib.rs b/src/lib.rs index 17f35e075..ef6b1f3a2 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -31,6 +31,9 @@ extern crate unicode_normalization; // for semver check in attrs.rs extern crate semver; +// for levensthein distance +extern crate strsim; + // for regex checking extern crate regex_syntax; @@ -84,6 +87,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 +204,7 @@ 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::SimilarNames(1)); 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,7 @@ pub fn plugin_registrar(reg: &mut Registry) { needless_update::NEEDLESS_UPDATE, new_without_default::NEW_WITHOUT_DEFAULT, no_effect::NO_EFFECT, + non_expressive_names::SIMILAR_NAMES, open_options::NONSENSICAL_OPEN_OPTIONS, overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL, panic::PANIC_PARAMS, diff --git a/src/non_expressive_names.rs b/src/non_expressive_names.rs new file mode 100644 index 000000000..944dcaeb0 --- /dev/null +++ b/src/non_expressive_names.rs @@ -0,0 +1,150 @@ +use rustc::lint::*; +use syntax::codemap::Span; +use syntax::parse::token::InternedString; +use syntax::ast::*; +use syntax::visit::{self, FnKind}; +use utils::{span_note_and_lint, in_macro}; +use strsim::levenshtein; + +/// **What it does:** This lint warns about names that are very similar and thus confusing +/// +/// **Why is this bad?** It's hard to distinguish between names that differ only by a single character +/// +/// **Known problems:** None? +/// +/// **Example:** `checked_exp` and `checked_expr` +declare_lint! { + pub SIMILAR_NAMES, + Warn, + "similarly named items and bindings" +} + +pub struct SimilarNames(pub usize); + +impl LintPass for SimilarNames { + fn get_lints(&self) -> LintArray { + lint_array!(SIMILAR_NAMES) + } +} + +struct SimilarNamesLocalVisitor<'a, 'b: 'a> { + names: Vec<(InternedString, Span)>, + cx: &'a EarlyContext<'b>, + limit: usize, +} + +const WHITELIST: &'static [&'static str] = &[ + "lhs", "rhs", +]; + +struct SimilarNamesNameVisitor<'a, 'b: 'a, 'c: 'b>(&'a mut SimilarNamesLocalVisitor<'b, 'c>); + +impl<'v, 'a, 'b, 'c> visit::Visitor<'v> for SimilarNamesNameVisitor<'a, 'b, 'c> { + fn visit_pat(&mut self, pat: &'v Pat) { + if let PatKind::Ident(_, id, _) = pat.node { + self.check_name(id.span, id.node.name); + } + visit::walk_pat(self, pat); + } +} + +impl<'a, 'b, 'c> SimilarNamesNameVisitor<'a, 'b, 'c> { + fn check_name(&mut self, span: Span, name: Name) { + if in_macro(self.0.cx, span) { + return; + } + let interned_name = name.as_str(); + if interned_name.chars().any(char::is_uppercase) { + return; + } + if interned_name.chars().count() < 3 { + return; + } + for &allow in WHITELIST { + if interned_name == allow { + return; + } + if interned_name.len() <= allow.len() { + continue; + } + // allow_* + let allow_start = allow.chars().chain(Some('_')); + if interned_name.chars().zip(allow_start).all(|(l, r)| l == r) { + return; + } + // *_allow + let allow_end = Some('_').into_iter().chain(allow.chars()); + if interned_name.chars().rev().zip(allow_end.rev()).all(|(l, r)| l == r) { + return; + } + } + for &(ref existing_name, sp) in &self.0.names { + let dist = levenshtein(&interned_name, &existing_name); + // equality is caught by shadow lints + if dist == 0 { + continue; + } + // if they differ enough it's all good + if dist > self.0.limit { + continue; + } + // are we doing stuff like `for item in items`? + if interned_name.starts_with(&**existing_name) || + existing_name.starts_with(&*interned_name) || + interned_name.ends_with(&**existing_name) || + existing_name.ends_with(&*interned_name) { + continue; + } + if dist == 1 { + // are we doing stuff like a_bar, b_bar, c_bar? + if interned_name.chars().next() != existing_name.chars().next() && interned_name.chars().nth(1) == Some('_') { + continue; + } + // are we doing stuff like foo_x, foo_y, foo_z? + if interned_name.chars().rev().next() != existing_name.chars().rev().next() && interned_name.chars().rev().nth(1) == Some('_') { + continue; + } + } + span_note_and_lint(self.0.cx, SIMILAR_NAMES, span, "binding's name is too similar to existing binding", sp, "existing binding defined here"); + return; + } + self.0.names.push((interned_name, span)); + } +} + +impl<'v, 'a, 'b> visit::Visitor<'v> for SimilarNamesLocalVisitor<'a, 'b> { + fn visit_local(&mut self, local: &'v Local) { + SimilarNamesNameVisitor(self).visit_local(local) + } + fn visit_block(&mut self, blk: &'v Block) { + // ensure scoping rules work + let n = self.names.len(); + visit::walk_block(self, blk); + self.names.truncate(n); + } + fn visit_arm(&mut self, arm: &'v Arm) { + let n = self.names.len(); + // just go through the first pattern, as either all patterns bind the same bindings or rustc would have errored much earlier + SimilarNamesNameVisitor(self).visit_pat(&arm.pats[0]); + self.names.truncate(n); + } + fn visit_item(&mut self, _: &'v Item) { + // do nothing + } +} + +impl EarlyLintPass for SimilarNames { + fn check_fn(&mut self, cx: &EarlyContext, _: FnKind, decl: &FnDecl, blk: &Block, _: Span, _: NodeId) { + let mut visitor = SimilarNamesLocalVisitor { + names: Vec::new(), + cx: cx, + limit: self.0, + }; + // initialize with function arguments + for arg in &decl.inputs { + visit::walk_pat(&mut SimilarNamesNameVisitor(&mut visitor), &arg.pat); + } + // walk all other bindings + visit::walk_block(&mut visitor, blk); + } +} diff --git a/src/types.rs b/src/types.rs index c5acbb41e..1dc1f55b7 100644 --- a/src/types.rs +++ b/src/types.rs @@ -638,7 +638,7 @@ fn detect_absurd_comparison<'a>(cx: &LateContext, op: BinOp_, lhs: &'a Expr, rhs Lt, Le, }; - let (rel, lhs2, rhs2) = match op { + let (rel, normalized_lhs, normalized_rhs) = match op { BiLt => (Rel::Lt, lhs, rhs), BiLe => (Rel::Le, lhs, rhs), BiGt => (Rel::Lt, rhs, lhs), @@ -646,8 +646,8 @@ fn detect_absurd_comparison<'a>(cx: &LateContext, op: BinOp_, lhs: &'a Expr, rhs _ => return None, }; - let lx = detect_extreme_expr(cx, lhs2); - let rx = detect_extreme_expr(cx, rhs2); + let lx = detect_extreme_expr(cx, normalized_lhs); + let rx = detect_extreme_expr(cx, normalized_rhs); Some(match rel { Rel::Lt => { diff --git a/src/utils/hir.rs b/src/utils/hir.rs index b4b786f97..bc7bc358a 100644 --- a/src/utils/hir.rs +++ b/src/utils/hir.rs @@ -67,24 +67,24 @@ impl<'a, 'tcx: 'a> SpanlessEq<'a, 'tcx> { } match (&left.node, &right.node) { - (&ExprAddrOf(lmut, ref le), &ExprAddrOf(rmut, ref re)) => lmut == rmut && self.eq_expr(le, re), + (&ExprAddrOf(l_mut, ref le), &ExprAddrOf(r_mut, ref re)) => l_mut == r_mut && self.eq_expr(le, re), (&ExprAgain(li), &ExprAgain(ri)) => both(&li, &ri, |l, r| l.node.name.as_str() == r.node.name.as_str()), (&ExprAssign(ref ll, ref lr), &ExprAssign(ref rl, ref rr)) => self.eq_expr(ll, rl) && self.eq_expr(lr, rr), (&ExprAssignOp(ref lo, ref ll, ref lr), &ExprAssignOp(ref ro, ref rl, ref rr)) => { lo.node == ro.node && self.eq_expr(ll, rl) && self.eq_expr(lr, rr) } (&ExprBlock(ref l), &ExprBlock(ref r)) => self.eq_block(l, r), - (&ExprBinary(lop, ref ll, ref lr), &ExprBinary(rop, ref rl, ref rr)) => { - lop.node == rop.node && self.eq_expr(ll, rl) && self.eq_expr(lr, rr) + (&ExprBinary(l_op, ref ll, ref lr), &ExprBinary(r_op, ref rl, ref rr)) => { + l_op.node == r_op.node && self.eq_expr(ll, rl) && self.eq_expr(lr, rr) } (&ExprBreak(li), &ExprBreak(ri)) => both(&li, &ri, |l, r| l.node.name.as_str() == r.node.name.as_str()), (&ExprBox(ref l), &ExprBox(ref r)) => self.eq_expr(l, r), - (&ExprCall(ref lfun, ref largs), &ExprCall(ref rfun, ref rargs)) => { - !self.ignore_fn && self.eq_expr(lfun, rfun) && self.eq_exprs(largs, rargs) + (&ExprCall(ref l_fun, ref l_args), &ExprCall(ref r_fun, ref r_args)) => { + !self.ignore_fn && self.eq_expr(l_fun, r_fun) && self.eq_exprs(l_args, r_args) } (&ExprCast(ref lx, ref lt), &ExprCast(ref rx, ref rt)) => self.eq_expr(lx, rx) && self.eq_ty(lt, rt), - (&ExprField(ref lfexp, ref lfident), &ExprField(ref rfexp, ref rfident)) => { - lfident.node == rfident.node && self.eq_expr(lfexp, rfexp) + (&ExprField(ref l_f_exp, ref l_f_ident), &ExprField(ref r_f_exp, ref r_f_ident)) => { + l_f_ident.node == r_f_ident.node && self.eq_expr(l_f_exp, r_f_exp) } (&ExprIndex(ref la, ref li), &ExprIndex(ref ra, ref ri)) => self.eq_expr(la, ra) && self.eq_expr(li, ri), (&ExprIf(ref lc, ref lt, ref le), &ExprIf(ref rc, ref rt, ref re)) => { @@ -101,25 +101,25 @@ impl<'a, 'tcx: 'a> SpanlessEq<'a, 'tcx> { over(&l.pats, &r.pats, |l, r| self.eq_pat(l, r)) }) } - (&ExprMethodCall(ref lname, ref ltys, ref largs), - &ExprMethodCall(ref rname, ref rtys, ref rargs)) => { + (&ExprMethodCall(ref l_name, ref l_tys, ref l_args), + &ExprMethodCall(ref r_name, ref r_tys, ref r_args)) => { // TODO: tys - !self.ignore_fn && lname.node == rname.node && ltys.is_empty() && rtys.is_empty() && - self.eq_exprs(largs, rargs) + !self.ignore_fn && l_name.node == r_name.node && l_tys.is_empty() && r_tys.is_empty() && + self.eq_exprs(l_args, r_args) } (&ExprRepeat(ref le, ref ll), &ExprRepeat(ref re, ref rl)) => self.eq_expr(le, re) && self.eq_expr(ll, rl), (&ExprRet(ref l), &ExprRet(ref r)) => both(l, r, |l, r| self.eq_expr(l, r)), - (&ExprPath(ref lqself, ref lsubpath), &ExprPath(ref rqself, ref rsubpath)) => { - both(lqself, rqself, |l, r| self.eq_qself(l, r)) && self.eq_path(lsubpath, rsubpath) + (&ExprPath(ref l_qself, ref l_subpath), &ExprPath(ref r_qself, ref r_subpath)) => { + both(l_qself, r_qself, |l, r| self.eq_qself(l, r)) && self.eq_path(l_subpath, r_subpath) } - (&ExprStruct(ref lpath, ref lf, ref lo), &ExprStruct(ref rpath, ref rf, ref ro)) => { - self.eq_path(lpath, rpath) && + (&ExprStruct(ref l_path, ref lf, ref lo), &ExprStruct(ref r_path, ref rf, ref ro)) => { + self.eq_path(l_path, r_path) && both(lo, ro, |l, r| self.eq_expr(l, r)) && over(lf, rf, |l, r| self.eq_field(l, r)) } - (&ExprTup(ref ltup), &ExprTup(ref rtup)) => self.eq_exprs(ltup, rtup), + (&ExprTup(ref l_tup), &ExprTup(ref r_tup)) => self.eq_exprs(l_tup, r_tup), (&ExprTupField(ref le, li), &ExprTupField(ref re, ri)) => li.node == ri.node && self.eq_expr(le, re), - (&ExprUnary(lop, ref le), &ExprUnary(rop, ref re)) => lop == rop && self.eq_expr(le, re), + (&ExprUnary(l_op, ref le), &ExprUnary(r_op, ref re)) => l_op == r_op && self.eq_expr(le, re), (&ExprVec(ref l), &ExprVec(ref r)) => self.eq_exprs(l, r), (&ExprWhile(ref lc, ref lb, ref ll), &ExprWhile(ref rc, ref rb, ref rl)) => { self.eq_expr(lc, rc) && self.eq_block(lb, rb) && both(ll, rl, |l, r| l.name.as_str() == r.name.as_str()) @@ -179,16 +179,16 @@ impl<'a, 'tcx: 'a> SpanlessEq<'a, 'tcx> { fn eq_ty(&self, left: &Ty, right: &Ty) -> bool { match (&left.node, &right.node) { - (&TyVec(ref lvec), &TyVec(ref rvec)) => self.eq_ty(lvec, rvec), + (&TyVec(ref l_vec), &TyVec(ref r_vec)) => self.eq_ty(l_vec, r_vec), (&TyFixedLengthVec(ref lt, ref ll), &TyFixedLengthVec(ref rt, ref rl)) => { self.eq_ty(lt, rt) && self.eq_expr(ll, rl) } - (&TyPtr(ref lmut), &TyPtr(ref rmut)) => lmut.mutbl == rmut.mutbl && self.eq_ty(&*lmut.ty, &*rmut.ty), - (&TyRptr(_, ref lrmut), &TyRptr(_, ref rrmut)) => { - lrmut.mutbl == rrmut.mutbl && self.eq_ty(&*lrmut.ty, &*rrmut.ty) + (&TyPtr(ref l_mut), &TyPtr(ref r_mut)) => l_mut.mutbl == r_mut.mutbl && self.eq_ty(&*l_mut.ty, &*r_mut.ty), + (&TyRptr(_, ref l_rmut), &TyRptr(_, ref r_rmut)) => { + l_rmut.mutbl == r_rmut.mutbl && self.eq_ty(&*l_rmut.ty, &*r_rmut.ty) } - (&TyPath(ref lq, ref lpath), &TyPath(ref rq, ref rpath)) => { - both(lq, rq, |l, r| self.eq_qself(l, r)) && self.eq_path(lpath, rpath) + (&TyPath(ref lq, ref l_path), &TyPath(ref rq, ref r_path)) => { + both(lq, rq, |l, r| self.eq_qself(l, r)) && self.eq_path(l_path, r_path) } (&TyTup(ref l), &TyTup(ref r)) => over(l, r, |l, r| self.eq_ty(l, r)), (&TyInfer, &TyInfer) => true, diff --git a/src/utils/mod.rs b/src/utils/mod.rs index 2934f1c4f..de303a35f 100644 --- a/src/utils/mod.rs +++ b/src/utils/mod.rs @@ -102,8 +102,8 @@ macro_rules! if_let_chain { /// Returns true if the two spans come from differing expansions (i.e. one is from a macro and one /// isn't). -pub fn differing_macro_contexts(sp1: Span, sp2: Span) -> bool { - sp1.expn_id != sp2.expn_id +pub fn differing_macro_contexts(lhs: Span, rhs: Span) -> bool { + rhs.expn_id != lhs.expn_id } /// Returns true if this `expn_info` was expanded by any macro. pub fn in_macro(cx: &T, span: Span) -> bool { diff --git a/tests/compile-fail/approx_const.rs b/tests/compile-fail/approx_const.rs index 148746bfa..3660fb419 100644 --- a/tests/compile-fail/approx_const.rs +++ b/tests/compile-fail/approx_const.rs @@ -2,7 +2,7 @@ #![plugin(clippy)] #[deny(approx_constant)] -#[allow(unused, shadow_unrelated)] +#[allow(unused, shadow_unrelated, similar_names)] fn main() { let my_e = 2.7182; //~ERROR approximate value of `f{32, 64}::E` found let almost_e = 2.718; //~ERROR approximate value of `f{32, 64}::E` found diff --git a/tests/compile-fail/drop_ref.rs b/tests/compile-fail/drop_ref.rs index 3e4c0a9d8..8454a4715 100644 --- a/tests/compile-fail/drop_ref.rs +++ b/tests/compile-fail/drop_ref.rs @@ -2,7 +2,7 @@ #![plugin(clippy)] #![deny(drop_ref)] -#![allow(toplevel_ref_arg)] +#![allow(toplevel_ref_arg, similar_names)] use std::mem::drop; diff --git a/tests/compile-fail/for_loop.rs b/tests/compile-fail/for_loop.rs index bbdf9d8f1..b111439ba 100644 --- a/tests/compile-fail/for_loop.rs +++ b/tests/compile-fail/for_loop.rs @@ -88,7 +88,7 @@ impl Unrelated { #[deny(needless_range_loop, explicit_iter_loop, iter_next_loop, reverse_range_loop, explicit_counter_loop)] #[deny(unused_collect)] -#[allow(linkedlist, shadow_unrelated, unnecessary_mut_passed, cyclomatic_complexity)] +#[allow(linkedlist, shadow_unrelated, unnecessary_mut_passed, cyclomatic_complexity, similar_names)] fn main() { const MAX_LEN: usize = 42; diff --git a/tests/compile-fail/len_zero.rs b/tests/compile-fail/len_zero.rs index 5168f80b8..4fd0203b9 100644 --- a/tests/compile-fail/len_zero.rs +++ b/tests/compile-fail/len_zero.rs @@ -92,38 +92,38 @@ fn main() { println!("Nor should this!"); } - let hie = HasIsEmpty; - if hie.len() == 0 { + let has_is_empty = HasIsEmpty; + if has_is_empty.len() == 0 { //~^ERROR length comparison to zero //~|HELP consider using `is_empty` - //~|SUGGESTION hie.is_empty() + //~|SUGGESTION has_is_empty.is_empty() println!("Or this!"); } - if hie.len() != 0 { + if has_is_empty.len() != 0 { //~^ERROR length comparison to zero //~|HELP consider using `is_empty` - //~|SUGGESTION !hie.is_empty() + //~|SUGGESTION !has_is_empty.is_empty() println!("Or this!"); } - if hie.len() > 0 { + if has_is_empty.len() > 0 { //~^ERROR length comparison to zero //~|HELP consider using `is_empty` - //~|SUGGESTION !hie.is_empty() + //~|SUGGESTION !has_is_empty.is_empty() println!("Or this!"); } - assert!(!hie.is_empty()); + assert!(!has_is_empty.is_empty()); - let wie : &WithIsEmpty = &Wither; - if wie.len() == 0 { + let with_is_empty: &WithIsEmpty = &Wither; + if with_is_empty.len() == 0 { //~^ERROR length comparison to zero //~|HELP consider using `is_empty` - //~|SUGGESTION wie.is_empty() + //~|SUGGESTION with_is_empty.is_empty() println!("Or this!"); } - assert!(!wie.is_empty()); + assert!(!with_is_empty.is_empty()); - let hwie = HasWrongIsEmpty; - if hwie.len() == 0 { //no error as HasWrongIsEmpty does not have .is_empty() + let has_wrong_is_empty = HasWrongIsEmpty; + if has_wrong_is_empty.len() == 0 { //no error as HasWrongIsEmpty does not have .is_empty() println!("Or this!"); } } diff --git a/tests/compile-fail/methods.rs b/tests/compile-fail/methods.rs index 344016a3b..b1a8f6cf7 100644 --- a/tests/compile-fail/methods.rs +++ b/tests/compile-fail/methods.rs @@ -286,6 +286,7 @@ fn or_fun_call() { //~|SUGGESTION btree.entry(42).or_insert_with(String::new); } +#[allow(similar_names)] fn main() { use std::io; diff --git a/tests/compile-fail/non_expressive_names.rs b/tests/compile-fail/non_expressive_names.rs new file mode 100644 index 000000000..a9a06f423 --- /dev/null +++ b/tests/compile-fail/non_expressive_names.rs @@ -0,0 +1,37 @@ +#![feature(plugin)] +#![plugin(clippy)] +#![deny(clippy)] +#![allow(unused)] + +fn main() { + let specter: i32; + let spectre: i32; + + let apple: i32; //~ NOTE: existing binding defined here + let bpple: i32; //~ ERROR: name is too similar + let cpple: i32; //~ ERROR: name is too similar + + let a_bar: i32; + let b_bar: i32; + let c_bar: i32; + + let foo_x: i32; + let foo_y: i32; + + let rhs: i32; + let lhs: i32; + + let bla_rhs: i32; + let bla_lhs: i32; + + let blubrhs: i32; //~ NOTE: existing binding defined here + let blublhs: i32; //~ ERROR: name is too similar + + let blubx: i32; //~ NOTE: existing binding defined here + let bluby: i32; //~ ERROR: name is too similar + + let cake: i32; //~ NOTE: existing binding defined here + let caked: i32; //~ NOTE: existing binding defined here + let cakes: i32; //~ ERROR: name is too similar + let coke: i32; //~ ERROR: name is too similar +} From 5373ffdeb85d55b39671b31fc15a8183d86ff662 Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Tue, 1 Mar 2016 10:34:45 +0100 Subject: [PATCH 02/10] suggest inserting underscores for simple cases --- src/non_expressive_names.rs | 30 +++++++++++++++++----- tests/compile-fail/non_expressive_names.rs | 4 +-- 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/src/non_expressive_names.rs b/src/non_expressive_names.rs index 944dcaeb0..fc8bb69c5 100644 --- a/src/non_expressive_names.rs +++ b/src/non_expressive_names.rs @@ -3,7 +3,7 @@ use syntax::codemap::Span; use syntax::parse::token::InternedString; use syntax::ast::*; use syntax::visit::{self, FnKind}; -use utils::{span_note_and_lint, in_macro}; +use utils::{span_lint_and_then, in_macro}; use strsim::levenshtein; /// **What it does:** This lint warns about names that are very similar and thus confusing @@ -95,17 +95,35 @@ impl<'a, 'b, 'c> SimilarNamesNameVisitor<'a, 'b, 'c> { existing_name.ends_with(&*interned_name) { continue; } + let mut split_at = None; if dist == 1 { // are we doing stuff like a_bar, b_bar, c_bar? - if interned_name.chars().next() != existing_name.chars().next() && interned_name.chars().nth(1) == Some('_') { - continue; + if interned_name.chars().next() != existing_name.chars().next() { + if interned_name.chars().nth(1) == Some('_') { + continue; + } + split_at = interned_name.chars().next().map(|c| c.len_utf8()); } // are we doing stuff like foo_x, foo_y, foo_z? - if interned_name.chars().rev().next() != existing_name.chars().rev().next() && interned_name.chars().rev().nth(1) == Some('_') { - continue; + if interned_name.chars().rev().next() != existing_name.chars().rev().next() { + if interned_name.chars().rev().nth(1) == Some('_') { + continue; + } + split_at = interned_name.char_indices().rev().next().map(|(i, _)| i); } } - span_note_and_lint(self.0.cx, SIMILAR_NAMES, span, "binding's name is too similar to existing binding", sp, "existing binding defined here"); + span_lint_and_then(self.0.cx, + SIMILAR_NAMES, + span, + "binding's name is too similar to existing binding", + |diag| { + diag.span_note(sp, "existing binding defined here"); + if let Some(split) = split_at { + diag.span_help(span, &format!("separate the discriminating character by an underscore like: `{}_{}`", + &interned_name[..split], + &interned_name[split..])); + } + }); return; } self.0.names.push((interned_name, span)); diff --git a/tests/compile-fail/non_expressive_names.rs b/tests/compile-fail/non_expressive_names.rs index a9a06f423..35dba1a82 100644 --- a/tests/compile-fail/non_expressive_names.rs +++ b/tests/compile-fail/non_expressive_names.rs @@ -29,9 +29,9 @@ fn main() { let blubx: i32; //~ NOTE: existing binding defined here let bluby: i32; //~ ERROR: name is too similar + //~| HELP: separate the discriminating character by an underscore like: `blub_y` let cake: i32; //~ NOTE: existing binding defined here - let caked: i32; //~ NOTE: existing binding defined here - let cakes: i32; //~ ERROR: name is too similar + let cakes: i32; let coke: i32; //~ ERROR: name is too similar } From 463897fd399482bd99fa80871269139f13d740c8 Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Tue, 1 Mar 2016 13:05:39 +0100 Subject: [PATCH 03/10] lint on too many single character bindings --- README.md | 1 + src/lib.rs | 6 +- src/non_expressive_names.rs | 66 ++++++++++++++++++---- tests/compile-fail/eta.rs | 2 +- tests/compile-fail/for_loop.rs | 1 + tests/compile-fail/non_expressive_names.rs | 27 +++++++++ 6 files changed, 91 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 2ee4dfc5e..65fc097af 100644 --- a/README.md +++ b/README.md @@ -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 diff --git a/src/lib.rs b/src/lib.rs index ef6b1f3a2..82383e2b7 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -204,7 +204,10 @@ 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::SimilarNames(1)); + reg.register_early_lint_pass(box non_expressive_names::NonExpressiveNames { + similarity_threshold: 1, + max_single_char_names: 5, + }); 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()); @@ -331,6 +334,7 @@ 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, diff --git a/src/non_expressive_names.rs b/src/non_expressive_names.rs index fc8bb69c5..7f2fc618e 100644 --- a/src/non_expressive_names.rs +++ b/src/non_expressive_names.rs @@ -3,7 +3,7 @@ use syntax::codemap::Span; use syntax::parse::token::InternedString; use syntax::ast::*; use syntax::visit::{self, FnKind}; -use utils::{span_lint_and_then, in_macro}; +use utils::{span_lint_and_then, in_macro, span_lint}; use strsim::levenshtein; /// **What it does:** This lint warns about names that are very similar and thus confusing @@ -19,18 +19,35 @@ declare_lint! { "similarly named items and bindings" } -pub struct SimilarNames(pub usize); +/// **What it does:** This lint warns about having too many variables whose name consists of a single character +/// +/// **Why is this bad?** It's hard to memorize what a variable means without a descriptive name. +/// +/// **Known problems:** None? +/// +/// **Example:** let (a, b, c, d, e, f, g) = (...); +declare_lint! { + pub MANY_SINGLE_CHAR_NAMES, + Warn, + "too many single character bindings" +} -impl LintPass for SimilarNames { +pub struct NonExpressiveNames { + pub similarity_threshold: usize, + pub max_single_char_names: usize, +} + +impl LintPass for NonExpressiveNames { fn get_lints(&self) -> LintArray { - lint_array!(SIMILAR_NAMES) + lint_array!(SIMILAR_NAMES, MANY_SINGLE_CHAR_NAMES) } } struct SimilarNamesLocalVisitor<'a, 'b: 'a> { names: Vec<(InternedString, Span)>, cx: &'a EarlyContext<'b>, - limit: usize, + lint: &'a NonExpressiveNames, + single_char_names: Vec, } const WHITELIST: &'static [&'static str] = &[ @@ -57,7 +74,15 @@ impl<'a, 'b, 'c> SimilarNamesNameVisitor<'a, 'b, 'c> { if interned_name.chars().any(char::is_uppercase) { return; } - if interned_name.chars().count() < 3 { + let count = interned_name.chars().count(); + if count < 3 { + if count == 1 { + let c = interned_name.chars().next().expect("already checked"); + // make sure we ignore shadowing + if !self.0.single_char_names.contains(&c) { + self.0.single_char_names.push(c); + } + } return; } for &allow in WHITELIST { @@ -85,7 +110,7 @@ impl<'a, 'b, 'c> SimilarNamesNameVisitor<'a, 'b, 'c> { continue; } // if they differ enough it's all good - if dist > self.0.limit { + if dist > self.0.lint.similarity_threshold { continue; } // are we doing stuff like `for item in items`? @@ -119,7 +144,8 @@ impl<'a, 'b, 'c> SimilarNamesNameVisitor<'a, 'b, 'c> { |diag| { diag.span_note(sp, "existing binding defined here"); if let Some(split) = split_at { - diag.span_help(span, &format!("separate the discriminating character by an underscore like: `{}_{}`", + diag.span_help(span, &format!("separate the discriminating character \ + by an underscore like: `{}_{}`", &interned_name[..split], &interned_name[split..])); } @@ -130,6 +156,19 @@ impl<'a, 'b, 'c> SimilarNamesNameVisitor<'a, 'b, 'c> { } } +impl<'a, 'b> SimilarNamesLocalVisitor<'a, 'b> { + fn check_single_char_count(&self, span: Span) { + if self.single_char_names.len() < self.lint.max_single_char_names { + return; + } + span_lint(self.cx, + MANY_SINGLE_CHAR_NAMES, + span, + &format!("scope contains {} bindings whose name are just one char", + self.single_char_names.len())); + } +} + impl<'v, 'a, 'b> visit::Visitor<'v> for SimilarNamesLocalVisitor<'a, 'b> { fn visit_local(&mut self, local: &'v Local) { SimilarNamesNameVisitor(self).visit_local(local) @@ -137,26 +176,33 @@ impl<'v, 'a, 'b> visit::Visitor<'v> for SimilarNamesLocalVisitor<'a, 'b> { fn visit_block(&mut self, blk: &'v Block) { // ensure scoping rules work let n = self.names.len(); + let single_char_count = self.single_char_names.len(); visit::walk_block(self, blk); self.names.truncate(n); + self.check_single_char_count(blk.span); + self.single_char_names.truncate(single_char_count); } fn visit_arm(&mut self, arm: &'v Arm) { let n = self.names.len(); + let single_char_count = self.single_char_names.len(); // just go through the first pattern, as either all patterns bind the same bindings or rustc would have errored much earlier SimilarNamesNameVisitor(self).visit_pat(&arm.pats[0]); self.names.truncate(n); + self.check_single_char_count(arm.body.span); + self.single_char_names.truncate(single_char_count); } fn visit_item(&mut self, _: &'v Item) { // do nothing } } -impl EarlyLintPass for SimilarNames { +impl EarlyLintPass for NonExpressiveNames { fn check_fn(&mut self, cx: &EarlyContext, _: FnKind, decl: &FnDecl, blk: &Block, _: Span, _: NodeId) { let mut visitor = SimilarNamesLocalVisitor { names: Vec::new(), cx: cx, - limit: self.0, + lint: &self, + single_char_names: Vec::new(), }; // initialize with function arguments for arg in &decl.inputs { diff --git a/tests/compile-fail/eta.rs b/tests/compile-fail/eta.rs index 0e72efe65..3fd089bf5 100644 --- a/tests/compile-fail/eta.rs +++ b/tests/compile-fail/eta.rs @@ -1,6 +1,6 @@ #![feature(plugin)] #![plugin(clippy)] -#![allow(unknown_lints, unused, no_effect, redundant_closure_call)] +#![allow(unknown_lints, unused, no_effect, redundant_closure_call, many_single_char_names)] #![deny(redundant_closure)] fn main() { diff --git a/tests/compile-fail/for_loop.rs b/tests/compile-fail/for_loop.rs index b111439ba..064f66537 100644 --- a/tests/compile-fail/for_loop.rs +++ b/tests/compile-fail/for_loop.rs @@ -89,6 +89,7 @@ impl Unrelated { #[deny(needless_range_loop, explicit_iter_loop, iter_next_loop, reverse_range_loop, explicit_counter_loop)] #[deny(unused_collect)] #[allow(linkedlist, shadow_unrelated, unnecessary_mut_passed, cyclomatic_complexity, similar_names)] +#[allow(many_single_char_names)] fn main() { const MAX_LEN: usize = 42; diff --git a/tests/compile-fail/non_expressive_names.rs b/tests/compile-fail/non_expressive_names.rs index 35dba1a82..9c75b0735 100644 --- a/tests/compile-fail/non_expressive_names.rs +++ b/tests/compile-fail/non_expressive_names.rs @@ -35,3 +35,30 @@ fn main() { let cakes: i32; let coke: i32; //~ ERROR: name is too similar } + + +fn bla() { + let a: i32; + let (b, c, d): (i32, i64, i16); + { + { + let cdefg: i32; + let blar: i32; + } + { //~ ERROR: scope contains 5 bindings whose name are just one char + let e: i32; + } + { //~ ERROR: scope contains 6 bindings whose name are just one char + let e: i32; + let f: i32; + } + match 5 { + 1 => println!(""), + e => panic!(), //~ ERROR: scope contains 5 bindings whose name are just one char + } + match 5 { + 1 => println!(""), + _ => panic!(), + } + } +} From 077481053cb28050d7c50e7db93d22eb332181cb Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Tue, 8 Mar 2016 14:36:21 +0100 Subject: [PATCH 04/10] refactoring and bugfix --- README.md | 2 +- src/non_expressive_names.rs | 64 ++++++++++--------- tests/compile-fail/non_expressive_names.rs | 44 +++++++++++-- .../overflow_check_conditional.rs | 1 + 4 files changed, 74 insertions(+), 37 deletions(-) diff --git a/README.md b/README.md index 65fc097af..57dedbdb3 100644 --- a/README.md +++ b/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 ---------------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ diff --git a/src/non_expressive_names.rs b/src/non_expressive_names.rs index 7f2fc618e..f8f2c4fb9 100644 --- a/src/non_expressive_names.rs +++ b/src/non_expressive_names.rs @@ -76,13 +76,23 @@ impl<'a, 'b, 'c> SimilarNamesNameVisitor<'a, 'b, 'c> { } let count = interned_name.chars().count(); if count < 3 { - if count == 1 { - let c = interned_name.chars().next().expect("already checked"); - // make sure we ignore shadowing - if !self.0.single_char_names.contains(&c) { - self.0.single_char_names.push(c); - } + if count != 1 { + return; } + let c = interned_name.chars().next().expect("already checked"); + // make sure we ignore shadowing + if self.0.single_char_names.contains(&c) { + return; + } + self.0.single_char_names.push(c); + if self.0.single_char_names.len() < self.0.lint.max_single_char_names { + return; + } + span_lint(self.0.cx, + MANY_SINGLE_CHAR_NAMES, + span, + &format!("{}th binding whose name is just one char", + self.0.single_char_names.len())); return; } for &allow in WHITELIST { @@ -157,39 +167,33 @@ impl<'a, 'b, 'c> SimilarNamesNameVisitor<'a, 'b, 'c> { } impl<'a, 'b> SimilarNamesLocalVisitor<'a, 'b> { - fn check_single_char_count(&self, span: Span) { - if self.single_char_names.len() < self.lint.max_single_char_names { - return; - } - span_lint(self.cx, - MANY_SINGLE_CHAR_NAMES, - span, - &format!("scope contains {} bindings whose name are just one char", - self.single_char_names.len())); + /// ensure scoping rules work + fn apply Fn(&'c mut Self)>(&mut self, f: F) { + let n = self.names.len(); + let single_char_count = self.single_char_names.len(); + f(self); + self.names.truncate(n); + self.single_char_names.truncate(single_char_count); } } impl<'v, 'a, 'b> visit::Visitor<'v> for SimilarNamesLocalVisitor<'a, 'b> { fn visit_local(&mut self, local: &'v Local) { - SimilarNamesNameVisitor(self).visit_local(local) + if let Some(ref init) = local.init { + self.apply(|this| visit::walk_expr(this, &**init)); + } + // add the pattern after the expression because the bindings aren't available yet in the init expression + SimilarNamesNameVisitor(self).visit_pat(&*local.pat); } fn visit_block(&mut self, blk: &'v Block) { - // ensure scoping rules work - let n = self.names.len(); - let single_char_count = self.single_char_names.len(); - visit::walk_block(self, blk); - self.names.truncate(n); - self.check_single_char_count(blk.span); - self.single_char_names.truncate(single_char_count); + self.apply(|this| visit::walk_block(this, blk)); } fn visit_arm(&mut self, arm: &'v Arm) { - let n = self.names.len(); - let single_char_count = self.single_char_names.len(); - // just go through the first pattern, as either all patterns bind the same bindings or rustc would have errored much earlier - SimilarNamesNameVisitor(self).visit_pat(&arm.pats[0]); - self.names.truncate(n); - self.check_single_char_count(arm.body.span); - self.single_char_names.truncate(single_char_count); + self.apply(|this| { + // just go through the first pattern, as either all patterns bind the same bindings or rustc would have errored much earlier + SimilarNamesNameVisitor(this).visit_pat(&arm.pats[0]); + this.apply(|this| visit::walk_expr(this, &arm.body)); + }); } fn visit_item(&mut self, _: &'v Item) { // do nothing diff --git a/tests/compile-fail/non_expressive_names.rs b/tests/compile-fail/non_expressive_names.rs index 9c75b0735..2e3c60cf6 100644 --- a/tests/compile-fail/non_expressive_names.rs +++ b/tests/compile-fail/non_expressive_names.rs @@ -34,8 +34,40 @@ fn main() { let cake: i32; //~ NOTE: existing binding defined here let cakes: i32; let coke: i32; //~ ERROR: name is too similar + + match 5 { + cheese @ 1 => {}, + rabbit => panic!(), + } + let cheese: i32; + match (42, 43) { + (cheese1, 1) => {}, + (cheese2, 2) => panic!(), + _ => println!(""), + } } +#[derive(Clone, Debug)] +enum MaybeInst { + Split, + Split1(usize), + Split2(usize), +} + +struct InstSplit { + uiae: usize, +} + +impl MaybeInst { + fn fill(&mut self) { + let filled = match *self { + MaybeInst::Split1(goto1) => panic!(1), + MaybeInst::Split2(goto2) => panic!(2), + _ => unimplemented!(), + }; + unimplemented!() + } +} fn bla() { let a: i32; @@ -45,16 +77,16 @@ fn bla() { let cdefg: i32; let blar: i32; } - { //~ ERROR: scope contains 5 bindings whose name are just one char - let e: i32; + { + let e: i32; //~ ERROR: 5th binding whose name is just one char } - { //~ ERROR: scope contains 6 bindings whose name are just one char - let e: i32; - let f: i32; + { + let e: i32; //~ ERROR: 5th binding whose name is just one char + let f: i32; //~ ERROR: 6th binding whose name is just one char } match 5 { 1 => println!(""), - e => panic!(), //~ ERROR: scope contains 5 bindings whose name are just one char + e => panic!(), //~ ERROR: 5th binding whose name is just one char } match 5 { 1 => println!(""), diff --git a/tests/compile-fail/overflow_check_conditional.rs b/tests/compile-fail/overflow_check_conditional.rs index db7b27924..24310eb81 100644 --- a/tests/compile-fail/overflow_check_conditional.rs +++ b/tests/compile-fail/overflow_check_conditional.rs @@ -1,6 +1,7 @@ #![feature(plugin)] #![plugin(clippy)] +#![allow(many_single_char_names)] #![deny(overflow_check_conditional)] fn main() { From aa1ecb6fce5b6e44f5a7e8c8ca2e15391dd64489 Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Mon, 14 Mar 2016 10:18:09 +0100 Subject: [PATCH 05/10] fix and rebase --- src/non_expressive_names.rs | 13 ++++++------- tests/compile-fail/blacklisted_name.rs | 2 +- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/non_expressive_names.rs b/src/non_expressive_names.rs index f8f2c4fb9..294af274b 100644 --- a/src/non_expressive_names.rs +++ b/src/non_expressive_names.rs @@ -85,14 +85,13 @@ impl<'a, 'b, 'c> SimilarNamesNameVisitor<'a, 'b, 'c> { return; } self.0.single_char_names.push(c); - if self.0.single_char_names.len() < self.0.lint.max_single_char_names { - return; + if self.0.single_char_names.len() >= self.0.lint.max_single_char_names { + span_lint(self.0.cx, + MANY_SINGLE_CHAR_NAMES, + span, + &format!("{}th binding whose name is just one char", + self.0.single_char_names.len())); } - span_lint(self.0.cx, - MANY_SINGLE_CHAR_NAMES, - span, - &format!("{}th binding whose name is just one char", - self.0.single_char_names.len())); return; } for &allow in WHITELIST { diff --git a/tests/compile-fail/blacklisted_name.rs b/tests/compile-fail/blacklisted_name.rs index efcb810a3..1afcd94a0 100755 --- a/tests/compile-fail/blacklisted_name.rs +++ b/tests/compile-fail/blacklisted_name.rs @@ -3,7 +3,7 @@ #![allow(dead_code)] #![allow(single_match)] -#![allow(unused_variables)] +#![allow(unused_variables, similar_names)] #![deny(blacklisted_name)] fn test(foo: ()) {} //~ERROR use of a blacklisted/placeholder name `foo` From 24cdb14d5a99e3a12250f449b16d05cdd4a7d53c Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Mon, 14 Mar 2016 14:34:47 +0100 Subject: [PATCH 06/10] refactor for speed --- Cargo.toml | 1 - src/lib.rs | 4 - src/non_expressive_names.rs | 178 ++++++++++++++------- tests/compile-fail/non_expressive_names.rs | 5 + 4 files changed, 125 insertions(+), 63 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 844408df2..dd675981f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,7 +23,6 @@ regex_macros = { version = "0.1.33", optional = true } semver = "0.2.1" toml = "0.1" unicode-normalization = "0.1" -strsim = "0.4.0" [dev-dependencies] compiletest_rs = "0.1.0" diff --git a/src/lib.rs b/src/lib.rs index 82383e2b7..a42db8d1f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -31,9 +31,6 @@ extern crate unicode_normalization; // for semver check in attrs.rs extern crate semver; -// for levensthein distance -extern crate strsim; - // for regex checking extern crate regex_syntax; @@ -205,7 +202,6 @@ pub fn plugin_registrar(reg: &mut Registry) { 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 { - similarity_threshold: 1, max_single_char_names: 5, }); reg.register_late_lint_pass(box drop_ref::DropRefPass); diff --git a/src/non_expressive_names.rs b/src/non_expressive_names.rs index 294af274b..0c7c97ce4 100644 --- a/src/non_expressive_names.rs +++ b/src/non_expressive_names.rs @@ -4,7 +4,6 @@ use syntax::parse::token::InternedString; use syntax::ast::*; use syntax::visit::{self, FnKind}; use utils::{span_lint_and_then, in_macro, span_lint}; -use strsim::levenshtein; /// **What it does:** This lint warns about names that are very similar and thus confusing /// @@ -33,7 +32,6 @@ declare_lint! { } pub struct NonExpressiveNames { - pub similarity_threshold: usize, pub max_single_char_names: usize, } @@ -44,7 +42,7 @@ impl LintPass for NonExpressiveNames { } struct SimilarNamesLocalVisitor<'a, 'b: 'a> { - names: Vec<(InternedString, Span)>, + names: Vec<(InternedString, Span, usize)>, cx: &'a EarlyContext<'b>, lint: &'a NonExpressiveNames, single_char_names: Vec, @@ -65,7 +63,43 @@ impl<'v, 'a, 'b, 'c> visit::Visitor<'v> for SimilarNamesNameVisitor<'a, 'b, 'c> } } +fn whitelisted(interned_name: &str) -> bool { + for &allow in WHITELIST { + if interned_name == allow { + return true; + } + if interned_name.len() <= allow.len() { + continue; + } + // allow_* + let allow_start = allow.chars().chain(Some('_')); + if interned_name.chars().zip(allow_start).all(|(l, r)| l == r) { + return true; + } + // *_allow + let allow_end = Some('_').into_iter().chain(allow.chars()); + if interned_name.chars().rev().zip(allow_end.rev()).all(|(l, r)| l == r) { + return true; + } + } + false +} + impl<'a, 'b, 'c> SimilarNamesNameVisitor<'a, 'b, 'c> { + fn check_short_name(&mut self, c: char, span: Span) { + // make sure we ignore shadowing + if self.0.single_char_names.contains(&c) { + return; + } + self.0.single_char_names.push(c); + if self.0.single_char_names.len() >= self.0.lint.max_single_char_names { + span_lint(self.0.cx, + MANY_SINGLE_CHAR_NAMES, + span, + &format!("{}th binding whose name is just one char", + self.0.single_char_names.len())); + } + } fn check_name(&mut self, span: Span, name: Name) { if in_macro(self.0.cx, span) { return; @@ -80,67 +114,68 @@ impl<'a, 'b, 'c> SimilarNamesNameVisitor<'a, 'b, 'c> { return; } let c = interned_name.chars().next().expect("already checked"); - // make sure we ignore shadowing - if self.0.single_char_names.contains(&c) { - return; - } - self.0.single_char_names.push(c); - if self.0.single_char_names.len() >= self.0.lint.max_single_char_names { - span_lint(self.0.cx, - MANY_SINGLE_CHAR_NAMES, - span, - &format!("{}th binding whose name is just one char", - self.0.single_char_names.len())); - } + self.check_short_name(c, span); return; } - for &allow in WHITELIST { - if interned_name == allow { - return; - } - if interned_name.len() <= allow.len() { - continue; - } - // allow_* - let allow_start = allow.chars().chain(Some('_')); - if interned_name.chars().zip(allow_start).all(|(l, r)| l == r) { - return; - } - // *_allow - let allow_end = Some('_').into_iter().chain(allow.chars()); - if interned_name.chars().rev().zip(allow_end.rev()).all(|(l, r)| l == r) { - return; - } + if whitelisted(&interned_name) { + return; } - for &(ref existing_name, sp) in &self.0.names { - let dist = levenshtein(&interned_name, &existing_name); - // equality is caught by shadow lints - if dist == 0 { - continue; - } - // if they differ enough it's all good - if dist > self.0.lint.similarity_threshold { - continue; - } - // are we doing stuff like `for item in items`? - if interned_name.starts_with(&**existing_name) || - existing_name.starts_with(&*interned_name) || - interned_name.ends_with(&**existing_name) || - existing_name.ends_with(&*interned_name) { - continue; - } + for &(ref existing_name, sp, existing_len) in &self.0.names { let mut split_at = None; - if dist == 1 { - // are we doing stuff like a_bar, b_bar, c_bar? - if interned_name.chars().next() != existing_name.chars().next() { - if interned_name.chars().nth(1) == Some('_') { + if existing_len > count { + if existing_len - count != 1 { + continue; + } + if levenstein_not_1(&interned_name, &existing_name) { + continue; + } + } else if existing_len < count { + if count - existing_len != 1 { + continue; + } + if levenstein_not_1(&existing_name, &interned_name) { + continue; + } + } else { + let mut interned_chars = interned_name.chars(); + let mut existing_chars = existing_name.chars(); + + if interned_chars.next() != existing_chars.next() { + let i = interned_chars.next().expect("we know we have more than 1 char"); + let e = existing_chars.next().expect("we know we have more than 1 char"); + if i == e { + if i == '_' { + // allowed similarity x_foo, y_foo + // or too many chars differ (x_foo, y_boo) + continue; + } else if interned_chars.ne(existing_chars) { + // too many chars differ + continue + } + } else { + // too many chars differ continue; } split_at = interned_name.chars().next().map(|c| c.len_utf8()); - } - // are we doing stuff like foo_x, foo_y, foo_z? - if interned_name.chars().rev().next() != existing_name.chars().rev().next() { - if interned_name.chars().rev().nth(1) == Some('_') { + } else if interned_chars.next_back() == existing_chars.next_back() { + if interned_chars.zip(existing_chars).filter(|&(i, e)| i != e).count() != 1 { + // too many chars differ, or none differ (aka shadowing) + continue; + } + } else { + let i = interned_chars.next_back().expect("we know we have more than 2 chars"); + let e = existing_chars.next_back().expect("we know we have more than 2 chars"); + if i == e { + if i == '_' { + // allowed similarity foo_x, foo_x + // or too many chars differ (foo_x, boo_x) + continue; + } else if interned_chars.ne(existing_chars) { + // too many chars differ + continue + } + } else { + // too many chars differ continue; } split_at = interned_name.char_indices().rev().next().map(|(i, _)| i); @@ -161,7 +196,7 @@ impl<'a, 'b, 'c> SimilarNamesNameVisitor<'a, 'b, 'c> { }); return; } - self.0.names.push((interned_name, span)); + self.0.names.push((interned_name, span, count)); } } @@ -215,3 +250,30 @@ impl EarlyLintPass for NonExpressiveNames { visit::walk_block(&mut visitor, blk); } } + +/// precondition: a_name.chars().count() < b_name.chars().count() +fn levenstein_not_1(a_name: &str, b_name: &str) -> bool { + debug_assert!(a_name.chars().count() < b_name.chars().count()); + let mut a_chars = a_name.chars(); + let mut b_chars = b_name.chars(); + while let (Some(a), Some(b)) = (a_chars.next(), b_chars.next()) { + if a == b { + continue; + } + if let Some(b2) = b_chars.next() { + // check if there's just one character inserted + if a == b2 && a_chars.eq(b_chars) { + return false; + } else { + // two charaters don't match + return true; + } + } else { + // tuple + // ntuple + return true; + } + } + // for item in items + true +} diff --git a/tests/compile-fail/non_expressive_names.rs b/tests/compile-fail/non_expressive_names.rs index 2e3c60cf6..c374c3c43 100644 --- a/tests/compile-fail/non_expressive_names.rs +++ b/tests/compile-fail/non_expressive_names.rs @@ -15,6 +15,11 @@ fn main() { let b_bar: i32; let c_bar: i32; + let items = [5]; + for item in &items { + loop {} + } + let foo_x: i32; let foo_y: i32; From ea1c2406cc2711ac19bc651b665ab081cfac987f Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Mon, 14 Mar 2016 14:56:44 +0100 Subject: [PATCH 07/10] make single char names threshold configurable --- src/lib.rs | 2 +- src/non_expressive_names.rs | 4 ++-- src/utils/conf.rs | 2 ++ 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index a42db8d1f..8bbaff653 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -202,7 +202,7 @@ pub fn plugin_registrar(reg: &mut Registry) { 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: 5, + 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); diff --git a/src/non_expressive_names.rs b/src/non_expressive_names.rs index 0c7c97ce4..9ad91b421 100644 --- a/src/non_expressive_names.rs +++ b/src/non_expressive_names.rs @@ -32,7 +32,7 @@ declare_lint! { } pub struct NonExpressiveNames { - pub max_single_char_names: usize, + pub max_single_char_names: u64, } impl LintPass for NonExpressiveNames { @@ -92,7 +92,7 @@ impl<'a, 'b, 'c> SimilarNamesNameVisitor<'a, 'b, 'c> { return; } self.0.single_char_names.push(c); - if self.0.single_char_names.len() >= self.0.lint.max_single_char_names { + if self.0.single_char_names.len() as u64 >= self.0.lint.max_single_char_names { span_lint(self.0.cx, MANY_SINGLE_CHAR_NAMES, span, diff --git a/src/utils/conf.rs b/src/utils/conf.rs index 6636e30ab..2411e4899 100644 --- a/src/utils/conf.rs +++ b/src/utils/conf.rs @@ -153,6 +153,8 @@ define_Conf! { ("too-many-arguments-threshold", too_many_arguments_threshold, 7 => u64), /// Lint: TYPE_COMPLEXITY. The maximum complexity a type can have ("type-complexity-threshold", type_complexity_threshold, 250 => u64), + /// Lint: MANY_SINGLE_CHAR_NAMES. The maximum number of single char bindings a scope may have + ("single-char-binding-names-threshold", max_single_char_names, 5 => u64), } /// Read the `toml` configuration file. The function will ignore “File not found” errors iif From 9dc282e31db5c70ca9583152557ebbf87df8ee95 Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Mon, 14 Mar 2016 16:41:41 +0100 Subject: [PATCH 08/10] improve needless_bool to catch odd construct in non_expressive_names --- src/lib.rs | 1 + src/needless_bool.rs | 94 +++++++++++++++++------------ src/non_expressive_names.rs | 7 +-- tests/compile-fail/needless_bool.rs | 26 ++++++++ 4 files changed, 82 insertions(+), 46 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 8bbaff653..f6179f4f9 100644 --- a/src/lib.rs +++ b/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 diff --git a/src/needless_bool.rs b/src/needless_bool.rs index 625f8b0ca..58afc86b8 100644 --- a/src/needless_bool.rs +++ b/src/needless_bool.rs @@ -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 pred_snip = snippet(cx, pred.span, ".."); + let hint = if pred_snip == ".." { + hint.into() + } else { + format!("`{}{}`", not, pred_snip) + }; + span_lint(cx, + NEEDLESS_BOOL, + e.span, + &format!("you can reduce this if-then-else expression to just {}", 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 { - 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 { +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, } } diff --git a/src/non_expressive_names.rs b/src/non_expressive_names.rs index 9ad91b421..b7d2ac80a 100644 --- a/src/non_expressive_names.rs +++ b/src/non_expressive_names.rs @@ -262,12 +262,7 @@ fn levenstein_not_1(a_name: &str, b_name: &str) -> bool { } if let Some(b2) = b_chars.next() { // check if there's just one character inserted - if a == b2 && a_chars.eq(b_chars) { - return false; - } else { - // two charaters don't match - return true; - } + return !(a == b2 && a_chars.eq(b_chars)); } else { // tuple // ntuple diff --git a/tests/compile-fail/needless_bool.rs b/tests/compile-fail/needless_bool.rs index c2ad24bc4..eff2bdc9b 100644 --- a/tests/compile-fail/needless_bool.rs +++ b/tests/compile-fail/needless_bool.rs @@ -10,4 +10,30 @@ fn main() { if x { true } else { false }; //~ERROR you can reduce this if-then-else expression to just `x` if x { false } else { true }; //~ERROR you can reduce this if-then-else expression to just `!x` if x { x } else { false }; // would also be questionable, but we don't catch this yet + bool_ret(x); + bool_ret2(x); + bool_ret3(x); + bool_ret4(x); +} + +#[deny(needless_bool)] +#[allow(if_same_then_else)] +fn bool_ret(x: bool) -> bool { + if x { return true } else { return true }; //~ERROR this if-then-else expression will always return true +} + +#[deny(needless_bool)] +#[allow(if_same_then_else)] +fn bool_ret2(x: bool) -> bool { + if x { return false } else { return false }; //~ERROR this if-then-else expression will always return false +} + +#[deny(needless_bool)] +fn bool_ret3(x: bool) -> bool { + if x { return true } else { return false }; //~ERROR you can reduce this if-then-else expression to just `return x` +} + +#[deny(needless_bool)] +fn bool_ret4(x: bool) -> bool { + if x { return false } else { return true }; //~ERROR you can reduce this if-then-else expression to just `return !x` } From 6a566a1009fefdbfe30e8475836f6b06fed81b3c Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Mon, 14 Mar 2016 17:13:10 +0100 Subject: [PATCH 09/10] use snippet_opt and span_suggestion --- src/needless_bool.rs | 20 ++++++++++---------- tests/compile-fail/needless_bool.rs | 20 ++++++++++++++++---- 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/src/needless_bool.rs b/src/needless_bool.rs index 58afc86b8..43e7cfdda 100644 --- a/src/needless_bool.rs +++ b/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. /// @@ -50,16 +50,16 @@ impl LateLintPass for NeedlessBool { use self::Expression::*; if let ExprIf(ref pred, ref then_block, Some(ref else_expr)) = e.node { let reduce = |hint: &str, not| { - let pred_snip = snippet(cx, pred.span, ".."); - let hint = if pred_snip == ".." { - hint.into() - } else { - format!("`{}{}`", not, pred_snip) + let hint = match snippet_opt(cx, pred.span) { + Some(pred_snip) => format!("`{}{}`", not, pred_snip), + None => hint.into(), }; - span_lint(cx, - NEEDLESS_BOOL, - e.span, - &format!("you can reduce this if-then-else expression to just {}", hint)); + 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)) { (RetBool(true), RetBool(true)) | diff --git a/tests/compile-fail/needless_bool.rs b/tests/compile-fail/needless_bool.rs index eff2bdc9b..7f2d7754b 100644 --- a/tests/compile-fail/needless_bool.rs +++ b/tests/compile-fail/needless_bool.rs @@ -7,8 +7,14 @@ fn main() { let x = true; if x { true } else { true }; //~ERROR this if-then-else expression will always return true if x { false } else { false }; //~ERROR this if-then-else expression will always return false - if x { true } else { false }; //~ERROR you can reduce this if-then-else expression to just `x` - if x { false } else { true }; //~ERROR you can reduce this if-then-else expression to just `!x` + if x { true } else { false }; + //~^ ERROR this if-then-else expression returns a bool literal + //~| HELP you can reduce it to + //~| SUGGESTION `x` + if x { false } else { true }; + //~^ ERROR this if-then-else expression returns a bool literal + //~| HELP you can reduce it to + //~| SUGGESTION `!x` if x { x } else { false }; // would also be questionable, but we don't catch this yet bool_ret(x); bool_ret2(x); @@ -30,10 +36,16 @@ fn bool_ret2(x: bool) -> bool { #[deny(needless_bool)] fn bool_ret3(x: bool) -> bool { - if x { return true } else { return false }; //~ERROR you can reduce this if-then-else expression to just `return x` + if x { return true } else { return false }; + //~^ ERROR this if-then-else expression returns a bool literal + //~| HELP you can reduce it to + //~| SUGGESTION `return x` } #[deny(needless_bool)] fn bool_ret4(x: bool) -> bool { - if x { return false } else { return true }; //~ERROR you can reduce this if-then-else expression to just `return !x` + if x { return false } else { return true }; + //~^ ERROR this if-then-else expression returns a bool literal + //~| HELP you can reduce it to + //~| SUGGESTION `return !x` } From fa739e4a0b17f43f955416788120705001a82666 Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Thu, 17 Mar 2016 13:04:33 +0100 Subject: [PATCH 10/10] update for compiletest update --- tests/compile-fail/non_expressive_names.rs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/compile-fail/non_expressive_names.rs b/tests/compile-fail/non_expressive_names.rs index c374c3c43..ac412fb44 100644 --- a/tests/compile-fail/non_expressive_names.rs +++ b/tests/compile-fail/non_expressive_names.rs @@ -1,6 +1,15 @@ #![feature(plugin)] #![plugin(clippy)] #![deny(clippy)] +//~^ NOTE: lint level defined here +//~| NOTE: lint level defined here +//~| NOTE: lint level defined here +//~| NOTE: lint level defined here +//~| NOTE: lint level defined here +//~| NOTE: lint level defined here +//~| NOTE: lint level defined here +//~| NOTE: lint level defined here +//~| NOTE: lint level defined here #![allow(unused)] fn main() { @@ -8,8 +17,13 @@ fn main() { let spectre: i32; let apple: i32; //~ NOTE: existing binding defined here + //~^ NOTE: existing binding defined here let bpple: i32; //~ ERROR: name is too similar + //~| HELP: separate the discriminating character by an underscore like: `b_pple` + //~| HELP: for further information visit let cpple: i32; //~ ERROR: name is too similar + //~| HELP: separate the discriminating character by an underscore like: `c_pple` + //~| HELP: for further information visit let a_bar: i32; let b_bar: i32; @@ -31,14 +45,17 @@ fn main() { let blubrhs: i32; //~ NOTE: existing binding defined here let blublhs: i32; //~ ERROR: name is too similar + //~| HELP: for further information visit let blubx: i32; //~ NOTE: existing binding defined here let bluby: i32; //~ ERROR: name is too similar + //~| HELP: for further information visit //~| HELP: separate the discriminating character by an underscore like: `blub_y` let cake: i32; //~ NOTE: existing binding defined here let cakes: i32; let coke: i32; //~ ERROR: name is too similar + //~| HELP: for further information visit match 5 { cheese @ 1 => {}, @@ -84,14 +101,18 @@ fn bla() { } { let e: i32; //~ ERROR: 5th binding whose name is just one char + //~| HELP: for further information visit } { let e: i32; //~ ERROR: 5th binding whose name is just one char + //~| HELP: for further information visit let f: i32; //~ ERROR: 6th binding whose name is just one char + //~| HELP: for further information visit } match 5 { 1 => println!(""), e => panic!(), //~ ERROR: 5th binding whose name is just one char + //~| HELP: for further information visit } match 5 { 1 => println!(""),