From 077481053cb28050d7c50e7db93d22eb332181cb Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Tue, 8 Mar 2016 14:36:21 +0100 Subject: [PATCH] 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 65fc097af713..57dedbdb39e7 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 7f2fc618ed62..f8f2c4fb9a6e 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 9c75b07356e8..2e3c60cf66a0 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 db7b2792484b..24310eb81dae 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() {