From 09f18f61c6111674244471d49d7f82d826913022 Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Fri, 9 Feb 2024 23:19:27 -0600 Subject: [PATCH] [similar_names] don't raise if the first character is different A lot of cases of the "noise" cases of `similar_names` come from two idents with a different first letter, which is easy enough to differentiate visually but causes this lint to be raised. Do not raise the lint in these cases, as long as the first character does not have a lookalike. Link: https://github.com/rust-lang/rust-clippy/issues/10926 --- clippy_lints/src/non_expressive_names.rs | 18 +++++++++ tests/ui/similar_names.rs | 5 +-- tests/ui/similar_names.stderr | 50 ++++++------------------ 3 files changed, 32 insertions(+), 41 deletions(-) diff --git a/clippy_lints/src/non_expressive_names.rs b/clippy_lints/src/non_expressive_names.rs index 66b0a53fdd7..b8c3c7fa65a 100644 --- a/clippy_lints/src/non_expressive_names.rs +++ b/clippy_lints/src/non_expressive_names.rs @@ -131,6 +131,14 @@ fn check_single_char_names(&self) { &["iter", "item"], ]; +/// Characters that look visually similar +const SIMILAR_CHARS: &[(char, char)] = &[('l', 'i'), ('l', '1'), ('i', '1'), ('u', 'v')]; + +/// Return true if two characters are visually similar +fn chars_are_similar(a: char, b: char) -> bool { + a == b || SIMILAR_CHARS.contains(&(a, b)) || SIMILAR_CHARS.contains(&(b, a)) +} + struct SimilarNamesNameVisitor<'a, 'tcx, 'b>(&'b mut SimilarNamesLocalVisitor<'a, 'tcx>); impl<'a, 'tcx, 'b> Visitor<'tcx> for SimilarNamesNameVisitor<'a, 'tcx, 'b> { @@ -219,6 +227,16 @@ fn check_ident(&mut self, ident: Ident) { } let existing_str = existing_name.interned.as_str(); + + // The first char being different is usually enough to set identifiers apart, as long + // as the characters aren't too similar. + if !chars_are_similar( + interned_name.chars().next().expect("len >= 1"), + existing_str.chars().next().expect("len >= 1"), + ) { + continue; + } + let dissimilar = match existing_name.len.cmp(&count) { Ordering::Greater => existing_name.len - count != 1 || levenstein_not_1(interned_name, existing_str), Ordering::Less => count - existing_name.len != 1 || levenstein_not_1(existing_str, interned_name), diff --git a/tests/ui/similar_names.rs b/tests/ui/similar_names.rs index f46af56c6e2..f0969334491 100644 --- a/tests/ui/similar_names.rs +++ b/tests/ui/similar_names.rs @@ -17,13 +17,10 @@ fn main() { let specter: i32; let spectre: i32; + // ok; first letter is different enough let apple: i32; - let bpple: i32; - //~^ ERROR: binding's name is too similar to existing binding - let cpple: i32; - //~^ ERROR: binding's name is too similar to existing binding let a_bar: i32; let b_bar: i32; diff --git a/tests/ui/similar_names.stderr b/tests/ui/similar_names.stderr index 44ae3532a48..0cadb44ecf1 100644 --- a/tests/ui/similar_names.stderr +++ b/tests/ui/similar_names.stderr @@ -1,88 +1,64 @@ error: binding's name is too similar to existing binding - --> $DIR/similar_names.rs:22:9 - | -LL | let bpple: i32; - | ^^^^^ - | -note: existing binding defined here - --> $DIR/similar_names.rs:20:9 - | -LL | let apple: i32; - | ^^^^^ - = note: `-D clippy::similar-names` implied by `-D warnings` - = help: to override `-D warnings` add `#[allow(clippy::similar_names)]` - -error: binding's name is too similar to existing binding - --> $DIR/similar_names.rs:25:9 - | -LL | let cpple: i32; - | ^^^^^ - | -note: existing binding defined here - --> $DIR/similar_names.rs:20:9 - | -LL | let apple: i32; - | ^^^^^ - -error: binding's name is too similar to existing binding - --> $DIR/similar_names.rs:50:9 + --> $DIR/similar_names.rs:47:9 | LL | let bluby: i32; | ^^^^^ | note: existing binding defined here - --> $DIR/similar_names.rs:49:9 + --> $DIR/similar_names.rs:46:9 | LL | let blubx: i32; | ^^^^^ + = note: `-D clippy::similar-names` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::similar_names)]` error: binding's name is too similar to existing binding - --> $DIR/similar_names.rs:55:9 + --> $DIR/similar_names.rs:52:9 | LL | let coke: i32; | ^^^^ | note: existing binding defined here - --> $DIR/similar_names.rs:53:9 + --> $DIR/similar_names.rs:50:9 | LL | let cake: i32; | ^^^^ error: binding's name is too similar to existing binding - --> $DIR/similar_names.rs:74:9 + --> $DIR/similar_names.rs:71:9 | LL | let xyzeabc: i32; | ^^^^^^^ | note: existing binding defined here - --> $DIR/similar_names.rs:72:9 + --> $DIR/similar_names.rs:69:9 | LL | let xyz1abc: i32; | ^^^^^^^ error: binding's name is too similar to existing binding - --> $DIR/similar_names.rs:79:9 + --> $DIR/similar_names.rs:76:9 | LL | let parsee: i32; | ^^^^^^ | note: existing binding defined here - --> $DIR/similar_names.rs:77:9 + --> $DIR/similar_names.rs:74:9 | LL | let parser: i32; | ^^^^^^ error: binding's name is too similar to existing binding - --> $DIR/similar_names.rs:101:16 + --> $DIR/similar_names.rs:98:16 | LL | bpple: sprang, | ^^^^^^ | note: existing binding defined here - --> $DIR/similar_names.rs:100:16 + --> $DIR/similar_names.rs:97:16 | LL | apple: spring, | ^^^^^^ -error: aborting due to 7 previous errors +error: aborting due to 5 previous errors