Skip to content

Commit

Permalink
[similar_names] don't raise if the first character is different
Browse files Browse the repository at this point in the history
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: rust-lang#10926
  • Loading branch information
tgross35 committed Feb 10, 2024
1 parent 4a5e30d commit 09f18f6
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 41 deletions.
18 changes: 18 additions & 0 deletions clippy_lints/src/non_expressive_names.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,14 @@ const ALLOWED_TO_BE_SIMILAR: &[&[&str]] = &[
&["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> {
Expand Down Expand Up @@ -219,6 +227,16 @@ impl<'a, 'tcx, 'b> SimilarNamesNameVisitor<'a, 'tcx, 'b> {
}

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),
Expand Down
5 changes: 1 addition & 4 deletions tests/ui/similar_names.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
50 changes: 13 additions & 37 deletions tests/ui/similar_names.stderr
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 09f18f6

Please sign in to comment.