clippy_lints/src/non_expressive_names.rs

   1 // Copyright 2014-2018 The Rust Project Developers. See the COPYRIGHT
   2 // file at the top-level directory of this distribution.
   3 //
   4 // Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
   5 // http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
   6 // <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
   7 // option. This file may not be copied, modified, or distributed
   8 // except according to those terms.
   9
  10 use crate::utils::{span_lint, span_lint_and_then};
  11 use rustc::lint::{EarlyContext, EarlyLintPass, LintArray, LintPass};
  12 use rustc::{declare_tool_lint, lint_array};
  13 use syntax::ast::*;
  14 use syntax::attr;
  15 use syntax::source_map::Span;
  16 use syntax::symbol::LocalInternedString;
  17 use syntax::visit::{walk_block, walk_expr, walk_pat, Visitor};
  18
  19 /// **What it does:** Checks for names that are very similar and thus confusing.
  20 ///
  21 /// **Why is this bad?** It's hard to distinguish between names that differ only
  22 /// by a single character.
  23 ///
  24 /// **Known problems:** None?
  25 ///
  26 /// **Example:**
  27 /// ```rust
  28 /// let checked_exp = something;
  29 /// let checked_expr = something_else;
  30 /// ```
  31 declare_clippy_lint! {
  32     pub SIMILAR_NAMES,
  33     pedantic,
  34     "similarly named items and bindings"
  35 }
  36
  37 /// **What it does:** Checks for too many variables whose name consists of a
  38 /// single character.
  39 ///
  40 /// **Why is this bad?** It's hard to memorize what a variable means without a
  41 /// descriptive name.
  42 ///
  43 /// **Known problems:** None?
  44 ///
  45 /// **Example:**
  46 /// ```rust
  47 /// let (a, b, c, d, e, f, g) = (...);
  48 /// ```
  49 declare_clippy_lint! {
  50     pub MANY_SINGLE_CHAR_NAMES,
  51     style,
  52     "too many single character bindings"
  53 }
  54
  55 /// **What it does:** Checks if you have variables whose name consists of just
  56 /// underscores and digits.
  57 ///
  58 /// **Why is this bad?** It's hard to memorize what a variable means without a
  59 /// descriptive name.
  60 ///
  61 /// **Known problems:** None?
  62 ///
  63 /// **Example:**
  64 /// ```rust
  65 /// let _1 = 1;
  66 /// let ___1 = 1;
  67 /// let __1___2 = 11;
  68 /// ```
  69 declare_clippy_lint! {
  70     pub JUST_UNDERSCORES_AND_DIGITS,
  71     style,
  72     "unclear name"
  73 }
  74
  75 pub struct NonExpressiveNames {
  76     pub single_char_binding_names_threshold: u64,
  77 }
  78
  79 impl LintPass for NonExpressiveNames {
  80     fn get_lints(&self) -> LintArray {
  81         lint_array!(SIMILAR_NAMES, MANY_SINGLE_CHAR_NAMES, JUST_UNDERSCORES_AND_DIGITS)
  82     }
  83 }
  84
  85 struct ExistingName {
  86     interned: LocalInternedString,
  87     span: Span,
  88     len: usize,
  89     whitelist: &'static [&'static str],
  90 }
  91
  92 struct SimilarNamesLocalVisitor<'a, 'tcx: 'a> {
  93     names: Vec<ExistingName>,
  94     cx: &'a EarlyContext<'tcx>,
  95     lint: &'a NonExpressiveNames,
  96     single_char_names: Vec<char>,
  97 }
  98
  99 // this list contains lists of names that are allowed to be similar
 100 // the assumption is that no name is ever contained in multiple lists.
 101 #[rustfmt::skip]
 102 const WHITELIST: &[&[&str]] = &[
 103     &["parsed", "parser"],
 104     &["lhs", "rhs"],
 105     &["tx", "rx"],
 106     &["set", "get"],
 107     &["args", "arms"],
 108     &["qpath", "path"],
 109     &["lit", "lint"],
 110 ];
 111
 112 struct SimilarNamesNameVisitor<'a: 'b, 'tcx: 'a, 'b>(&'b mut SimilarNamesLocalVisitor<'a, 'tcx>);
 113
 114 impl<'a, 'tcx: 'a, 'b> Visitor<'tcx> for SimilarNamesNameVisitor<'a, 'tcx, 'b> {
 115     fn visit_pat(&mut self, pat: &'tcx Pat) {
 116         match pat.node {
 117             PatKind::Ident(_, ident, _) => self.check_name(ident.span, ident.name),
 118             PatKind::Struct(_, ref fields, _) => {
 119                 for field in fields {
 120                     if !field.node.is_shorthand {
 121                         self.visit_pat(&field.node.pat);
 122                     }
 123                 }
 124             },
 125             _ => walk_pat(self, pat),
 126         }
 127     }
 128     fn visit_mac(&mut self, _mac: &Mac) {
 129         // do not check macs
 130     }
 131 }
 132
 133 fn get_whitelist(interned_name: &str) -> Option<&'static [&'static str]> {
 134     for &allow in WHITELIST {
 135         if whitelisted(interned_name, allow) {
 136             return Some(allow);
 137         }
 138     }
 139     None
 140 }
 141
 142 fn whitelisted(interned_name: &str, list: &[&str]) -> bool {
 143     list.iter()
 144         .any(|&name| interned_name.starts_with(name) || interned_name.ends_with(name))
 145 }
 146
 147 impl<'a, 'tcx, 'b> SimilarNamesNameVisitor<'a, 'tcx, 'b> {
 148     fn check_short_name(&mut self, c: char, span: Span) {
 149         // make sure we ignore shadowing
 150         if self.0.single_char_names.contains(&c) {
 151             return;
 152         }
 153         self.0.single_char_names.push(c);
 154         if self.0.single_char_names.len() as u64 >= self.0.lint.single_char_binding_names_threshold {
 155             span_lint(
 156                 self.0.cx,
 157                 MANY_SINGLE_CHAR_NAMES,
 158                 span,
 159                 &format!(
 160                     "{}th binding whose name is just one char",
 161                     self.0.single_char_names.len()
 162                 ),
 163             );
 164         }
 165     }
 166     fn check_name(&mut self, span: Span, name: Name) {
 167         let interned_name = name.as_str();
 168         if interned_name.chars().any(char::is_uppercase) {
 169             return;
 170         }
 171         if interned_name.chars().all(|c| c.is_digit(10) || c == '_') {
 172             span_lint(
 173                 self.0.cx,
 174                 JUST_UNDERSCORES_AND_DIGITS,
 175                 span,
 176                 "consider choosing a more descriptive name",
 177             );
 178             return;
 179         }
 180         let count = interned_name.chars().count();
 181         if count < 3 {
 182             if count == 1 {
 183                 let c = interned_name.chars().next().expect("already checked");
 184                 self.check_short_name(c, span);
 185             }
 186             return;
 187         }
 188         for existing_name in &self.0.names {
 189             if whitelisted(&interned_name, existing_name.whitelist) {
 190                 continue;
 191             }
 192             let mut split_at = None;
 193             if existing_name.len > count {
 194                 if existing_name.len - count != 1 || levenstein_not_1(&interned_name, &existing_name.interned) {
 195                     continue;
 196                 }
 197             } else if existing_name.len < count {
 198                 if count - existing_name.len != 1 || levenstein_not_1(&existing_name.interned, &interned_name) {
 199                     continue;
 200                 }
 201             } else {
 202                 let mut interned_chars = interned_name.chars();
 203                 let mut existing_chars = existing_name.interned.chars();
 204                 let first_i = interned_chars.next().expect("we know we have at least one char");
 205                 let first_e = existing_chars.next().expect("we know we have at least one char");
 206                 let eq_or_numeric = |(a, b): (char, char)| a == b || a.is_numeric() && b.is_numeric();
 207
 208                 if eq_or_numeric((first_i, first_e)) {
 209                     let last_i = interned_chars.next_back().expect("we know we have at least two chars");
 210                     let last_e = existing_chars.next_back().expect("we know we have at least two chars");
 211                     if eq_or_numeric((last_i, last_e)) {
 212                         if interned_chars
 213                             .zip(existing_chars)
 214                             .filter(|&ie| !eq_or_numeric(ie))
 215                             .count()
 216                             != 1
 217                         {
 218                             continue;
 219                         }
 220                     } else {
 221                         let second_last_i = interned_chars
 222                             .next_back()
 223                             .expect("we know we have at least three chars");
 224                         let second_last_e = existing_chars
 225                             .next_back()
 226                             .expect("we know we have at least three chars");
 227                         if !eq_or_numeric((second_last_i, second_last_e))
 228                             || second_last_i == '_'
 229                             || !interned_chars.zip(existing_chars).all(eq_or_numeric)
 230                         {
 231                             // allowed similarity foo_x, foo_y
 232                             // or too many chars differ (foo_x, boo_y) or (foox, booy)
 233                             continue;
 234                         }
 235                         split_at = interned_name.char_indices().rev().next().map(|(i, _)| i);
 236                     }
 237                 } else {
 238                     let second_i = interned_chars.next().expect("we know we have at least two chars");
 239                     let second_e = existing_chars.next().expect("we know we have at least two chars");
 240                     if !eq_or_numeric((second_i, second_e))
 241                         || second_i == '_'
 242                         || !interned_chars.zip(existing_chars).all(eq_or_numeric)
 243                     {
 244                         // allowed similarity x_foo, y_foo
 245                         // or too many chars differ (x_foo, y_boo) or (xfoo, yboo)
 246                         continue;
 247                     }
 248                     split_at = interned_name.chars().next().map(|c| c.len_utf8());
 249                 }
 250             }
 251             span_lint_and_then(
 252                 self.0.cx,
 253                 SIMILAR_NAMES,
 254                 span,
 255                 "binding's name is too similar to existing binding",
 256                 |diag| {
 257                     diag.span_note(existing_name.span, "existing binding defined here");
 258                     if let Some(split) = split_at {
 259                         diag.span_help(
 260                             span,
 261                             &format!(
 262                                 "separate the discriminating character by an \
 263                                  underscore like: `{}_{}`",
 264                                 &interned_name[..split],
 265                                 &interned_name[split..]
 266                             ),
 267                         );
 268                     }
 269                 },
 270             );
 271             return;
 272         }
 273         self.0.names.push(ExistingName {
 274             whitelist: get_whitelist(&interned_name).unwrap_or(&[]),
 275             interned: interned_name,
 276             span,
 277             len: count,
 278         });
 279     }
 280 }
 281
 282 impl<'a, 'b> SimilarNamesLocalVisitor<'a, 'b> {
 283     /// ensure scoping rules work
 284     fn apply<F: for<'c> Fn(&'c mut Self)>(&mut self, f: F) {
 285         let n = self.names.len();
 286         let single_char_count = self.single_char_names.len();
 287         f(self);
 288         self.names.truncate(n);
 289         self.single_char_names.truncate(single_char_count);
 290     }
 291 }
 292
 293 impl<'a, 'tcx> Visitor<'tcx> for SimilarNamesLocalVisitor<'a, 'tcx> {
 294     fn visit_local(&mut self, local: &'tcx Local) {
 295         if let Some(ref init) = local.init {
 296             self.apply(|this| walk_expr(this, &**init));
 297         }
 298         // add the pattern after the expression because the bindings aren't available
 299         // yet in the init
 300         // expression
 301         SimilarNamesNameVisitor(self).visit_pat(&*local.pat);
 302     }
 303     fn visit_block(&mut self, blk: &'tcx Block) {
 304         self.apply(|this| walk_block(this, blk));
 305     }
 306     fn visit_arm(&mut self, arm: &'tcx Arm) {
 307         self.apply(|this| {
 308             // just go through the first pattern, as either all patterns
 309             // bind the same bindings or rustc would have errored much earlier
 310             SimilarNamesNameVisitor(this).visit_pat(&arm.pats[0]);
 311             this.apply(|this| walk_expr(this, &arm.body));
 312         });
 313     }
 314     fn visit_item(&mut self, _: &Item) {
 315         // do not recurse into inner items
 316     }
 317     fn visit_mac(&mut self, _mac: &Mac) {
 318         // do not check macs
 319     }
 320 }
 321
 322 impl EarlyLintPass for NonExpressiveNames {
 323     fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) {
 324         if let ItemKind::Fn(ref decl, _, _, ref blk) = item.node {
 325             do_check(self, cx, &item.attrs, decl, blk);
 326         }
 327     }
 328
 329     fn check_impl_item(&mut self, cx: &EarlyContext<'_>, item: &ImplItem) {
 330         if let ImplItemKind::Method(ref sig, ref blk) = item.node {
 331             do_check(self, cx, &item.attrs, &sig.decl, blk);
 332         }
 333     }
 334 }
 335
 336 fn do_check(lint: &mut NonExpressiveNames, cx: &EarlyContext<'_>, attrs: &[Attribute], decl: &FnDecl, blk: &Block) {
 337     if !attr::contains_name(attrs, "test") {
 338         let mut visitor = SimilarNamesLocalVisitor {
 339             names: Vec::new(),
 340             cx,
 341             lint,
 342             single_char_names: Vec::new(),
 343         };
 344         // initialize with function arguments
 345         for arg in &decl.inputs {
 346             SimilarNamesNameVisitor(&mut visitor).visit_pat(&arg.pat);
 347         }
 348         // walk all other bindings
 349         walk_block(&mut visitor, blk);
 350     }
 351 }
 352
 353 /// Precondition: `a_name.chars().count() < b_name.chars().count()`.
 354 fn levenstein_not_1(a_name: &str, b_name: &str) -> bool {
 355     debug_assert!(a_name.chars().count() < b_name.chars().count());
 356     let mut a_chars = a_name.chars();
 357     let mut b_chars = b_name.chars();
 358     while let (Some(a), Some(b)) = (a_chars.next(), b_chars.next()) {
 359         if a == b {
 360             continue;
 361         }
 362         if let Some(b2) = b_chars.next() {
 363             // check if there's just one character inserted
 364             return a != b2 || a_chars.ne(b_chars);
 365         } else {
 366             // tuple
 367             // ntuple
 368             return true;
 369         }
 370     }
 371     // for item in items
 372     true
 373 }