mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Auto merge of #12258 - tgross35:similiar-names-update, r=Alexendoo

Browse Source 
[`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.

Helps with https://github.com/rust-lang/rust-clippy/issues/10926 (does not fix)

This is per-commit reviewable, the first commit is just refactoring.

changelog: [`similar_names`]: don't raise if the first character is different
This commit is contained in:
bors 2024-02-10 17:17:41 +00:00
commit e67b7e02da
11 changed files with 120 additions and 140 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
clippy_lints/src/item_name_repetitions.rs
View File

@ -385,7 +385,6 @@ impl LateLintPass<'_> for ItemNameRepetitions {
assert!(last.is_some());
}
#[expect(clippy::similar_names)]
fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) {
let item_name = item.ident.name.as_str();
let item_camel = to_camel_case(item_name);

144
clippy_lints/src/non_expressive_names.rs
View File

@ -119,7 +119,6 @@ impl<'a, 'tcx> SimilarNamesLocalVisitor<'a, 'tcx> {
// this list contains lists of names that are allowed to be similar
// the assumption is that no name is ever contained in multiple lists.
#[rustfmt::skip]
const ALLOWED_TO_BE_SIMILAR: &[&[&str]] = &[
&["parsed", "parser"],
&["lhs", "rhs"],
@ -132,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> {
@ -189,7 +196,6 @@ impl<'a, 'tcx, 'b> SimilarNamesNameVisitor<'a, 'tcx, 'b> {
}
}
#[expect(clippy::too_many_lines)]
fn check_ident(&mut self, ident: Ident) {
let interned_name = ident.name.as_str();
if interned_name.chars().any(char::is_uppercase) {
@ -219,71 +225,28 @@ impl<'a, 'tcx, 'b> SimilarNamesNameVisitor<'a, 'tcx, 'b> {
if allowed_to_be_similar(interned_name, existing_name.exemptions) {
continue;
}
match existing_name.len.cmp(&count) {
Ordering::Greater => {
if existing_name.len - count != 1
|| levenstein_not_1(interned_name, existing_name.interned.as_str())
{
continue;
}
},
Ordering::Less => {
if count - existing_name.len != 1
|| levenstein_not_1(existing_name.interned.as_str(), interned_name)
{
continue;
}
},
Ordering::Equal => {
let mut interned_chars = interned_name.chars();
let interned_str = existing_name.interned.as_str();
let mut existing_chars = interned_str.chars();
let first_i = interned_chars.next().expect("we know we have at least one char");
let first_e = existing_chars.next().expect("we know we have at least one char");
let eq_or_numeric = |(a, b): (char, char)| a == b || a.is_numeric() && b.is_numeric();
if eq_or_numeric((first_i, first_e)) {
let last_i = interned_chars.next_back().expect("we know we have at least two chars");
let last_e = existing_chars.next_back().expect("we know we have at least two chars");
if eq_or_numeric((last_i, last_e)) {
if interned_chars
.zip(existing_chars)
.filter(|&ie| !eq_or_numeric(ie))
.count()
!= 1
{
continue;
}
} else {
let second_last_i = interned_chars
.next_back()
.expect("we know we have at least three chars");
let second_last_e = existing_chars
.next_back()
.expect("we know we have at least three chars");
if !eq_or_numeric((second_last_i, second_last_e))
|| second_last_i == '_'
|| !interned_chars.zip(existing_chars).all(eq_or_numeric)
{
// allowed similarity foo_x, foo_y
// or too many chars differ (foo_x, boo_y) or (foox, booy)
continue;
}
}
} else {
let second_i = interned_chars.next().expect("we know we have at least two chars");
let second_e = existing_chars.next().expect("we know we have at least two chars");
if !eq_or_numeric((second_i, second_e))
|| second_i == '_'
|| !interned_chars.zip(existing_chars).all(eq_or_numeric)
{
// allowed similarity x_foo, y_foo
// or too many chars differ (x_foo, y_boo) or (xfoo, yboo)
continue;
}
}
},
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),
Ordering::Equal => Self::equal_length_strs_not_similar(interned_name, existing_str),
};
if dissimilar {
continue;
}
span_lint_and_then(
self.0.cx,
SIMILAR_NAMES,
@ -302,6 +265,57 @@ impl<'a, 'tcx, 'b> SimilarNamesNameVisitor<'a, 'tcx, 'b> {
len: count,
});
}
fn equal_length_strs_not_similar(interned_name: &str, existing_name: &str) -> bool {
let mut interned_chars = interned_name.chars();
let mut existing_chars = existing_name.chars();
let first_i = interned_chars.next().expect("we know we have at least one char");
let first_e = existing_chars.next().expect("we know we have at least one char");
let eq_or_numeric = |(a, b): (char, char)| a == b || a.is_numeric() && b.is_numeric();
if eq_or_numeric((first_i, first_e)) {
let last_i = interned_chars.next_back().expect("we know we have at least two chars");
let last_e = existing_chars.next_back().expect("we know we have at least two chars");
if eq_or_numeric((last_i, last_e)) {
if interned_chars
.zip(existing_chars)
.filter(|&ie| !eq_or_numeric(ie))
.count()
!= 1
{
return true;
}
} else {
let second_last_i = interned_chars
.next_back()
.expect("we know we have at least three chars");
let second_last_e = existing_chars
.next_back()
.expect("we know we have at least three chars");
if !eq_or_numeric((second_last_i, second_last_e))
|| second_last_i == '_'
|| !interned_chars.zip(existing_chars).all(eq_or_numeric)
{
// allowed similarity foo_x, foo_y
// or too many chars differ (foo_x, boo_y) or (foox, booy)
return true;
}
}
} else {
let second_i = interned_chars.next().expect("we know we have at least two chars");
let second_e = existing_chars.next().expect("we know we have at least two chars");
if !eq_or_numeric((second_i, second_e))
|| second_i == '_'
|| !interned_chars.zip(existing_chars).all(eq_or_numeric)
{
// allowed similarity x_foo, y_foo
// or too many chars differ (x_foo, y_boo) or (xfoo, yboo)
return true;
}
}
false
}
}
impl<'a, 'b> SimilarNamesLocalVisitor<'a, 'b> {

1
clippy_lints/src/operators/double_comparison.rs
View File

@ -8,7 +8,6 @@ use rustc_span::Span;
use super::DOUBLE_COMPARISONS;
#[expect(clippy::similar_names)]
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, op: BinOpKind, lhs: &'tcx Expr<'_>, rhs: &'tcx Expr<'_>, span: Span) {
let (lkind, llhs, lrhs, rkind, rlhs, rrhs) = match (&lhs.kind, &rhs.kind) {
(ExprKind::Binary(lb, llhs, lrhs), ExprKind::Binary(rb, rlhs, rrhs)) => {

2
clippy_lints/src/operators/op_ref.rs
View File

@ -11,7 +11,7 @@ use rustc_middle::ty::{self, Ty};
use super::OP_REF;
#[expect(clippy::similar_names, clippy::too_many_lines)]
#[expect(clippy::too_many_lines)]
pub(crate) fn check<'tcx>(
cx: &LateContext<'tcx>,
e: &'tcx Expr<'_>,

4
clippy_utils/src/ast_utils.rs
View File

@ -2,7 +2,7 @@
//!
//! - The `eq_foobar` functions test for semantic equality but ignores `NodeId`s and `Span`s.
#![allow(clippy::similar_names, clippy::wildcard_imports, clippy::enum_glob_use)]
#![allow(clippy::wildcard_imports, clippy::enum_glob_use)]
use crate::{both, over};
use rustc_ast::ptr::P;
@ -296,7 +296,7 @@ pub fn eq_item<K>(l: &Item<K>, r: &Item<K>, mut eq_kind: impl FnMut(&K, &K) -> b
eq_id(l.ident, r.ident) && over(&l.attrs, &r.attrs, eq_attr) && eq_vis(&l.vis, &r.vis) && eq_kind(&l.kind, &r.kind)
}
#[expect(clippy::too_many_lines)] // Just a big match statement
#[expect(clippy::similar_names, clippy::too_many_lines)] // Just a big match statement
pub fn eq_item_kind(l: &ItemKind, r: &ItemKind) -> bool {
use ItemKind::*;
match (l, r) {

5
clippy_utils/src/hir_utils.rs
View File

@ -132,7 +132,6 @@ impl HirEqInterExpr<'_, '_, '_> {
}
/// Checks whether two blocks are the same.
#[expect(clippy::similar_names)]
fn eq_block(&mut self, left: &Block<'_>, right: &Block<'_>) -> bool {
use TokenKind::{Semi, Whitespace};
if left.stmts.len() != right.stmts.len() {
@ -247,7 +246,7 @@ impl HirEqInterExpr<'_, '_, '_> {
res
}
#[expect(clippy::similar_names, clippy::too_many_lines)]
#[expect(clippy::too_many_lines)]
pub fn eq_expr(&mut self, left: &Expr<'_>, right: &Expr<'_>) -> bool {
if !self.check_ctxt(left.span.ctxt(), right.span.ctxt()) {
return false;
@ -463,7 +462,6 @@ impl HirEqInterExpr<'_, '_, '_> {
}
}
#[expect(clippy::similar_names)]
fn eq_qpath(&mut self, left: &QPath<'_>, right: &QPath<'_>) -> bool {
match (left, right) {
(&QPath::Resolved(ref lty, lpath), &QPath::Resolved(ref rty, rpath)) => {
@ -1158,7 +1156,6 @@ pub fn hash_expr(cx: &LateContext<'_>, e: &Expr<'_>) -> u64 {
h.finish()
}
#[expect(clippy::similar_names)]
fn eq_span_tokens(
cx: &LateContext<'_>,
left: impl SpanRange,

1
clippy_utils/src/qualify_min_const_fn.rs
View File

@ -389,7 +389,6 @@ fn is_const_fn(tcx: TyCtxt<'_>, def_id: DefId, msrv: &Msrv) -> bool {
})
}
#[expect(clippy::similar_names)] // bit too pedantic
fn is_ty_const_destruct<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, body: &Body<'tcx>) -> bool {
// FIXME(effects, fee1-dead) revert to const destruct once it works again
#[expect(unused)]

1
tests/ui/approx_const.rs
View File

@ -1,5 +1,4 @@
#[warn(clippy::approx_constant)]
#[allow(clippy::similar_names)]
fn main() {
let my_e = 2.7182;
//~^ ERROR: approximate value of `f{32, 64}::consts::E` found

46
tests/ui/approx_const.stderr
View File

@ -1,5 +1,5 @@
error: approximate value of `f{32, 64}::consts::E` found
--> $DIR/approx_const.rs:4:16
--> $DIR/approx_const.rs:3:16
|
LL | let my_e = 2.7182;
| ^^^^^^
@ -9,7 +9,7 @@ LL | let my_e = 2.7182;
= help: to override `-D warnings` add `#[allow(clippy::approx_constant)]`
error: approximate value of `f{32, 64}::consts::E` found
--> $DIR/approx_const.rs:6:20
--> $DIR/approx_const.rs:5:20
|
LL | let almost_e = 2.718;
| ^^^^^
@ -17,7 +17,7 @@ LL | let almost_e = 2.718;
= help: consider using the constant directly
error: approximate value of `f{32, 64}::consts::FRAC_1_PI` found
--> $DIR/approx_const.rs:10:24
--> $DIR/approx_const.rs:9:24
|
LL | let my_1_frac_pi = 0.3183;
| ^^^^^^
@ -25,7 +25,7 @@ LL | let my_1_frac_pi = 0.3183;
= help: consider using the constant directly
error: approximate value of `f{32, 64}::consts::FRAC_1_SQRT_2` found
--> $DIR/approx_const.rs:14:28
--> $DIR/approx_const.rs:13:28
|
LL | let my_frac_1_sqrt_2 = 0.70710678;
| ^^^^^^^^^^
@ -33,7 +33,7 @@ LL | let my_frac_1_sqrt_2 = 0.70710678;
= help: consider using the constant directly
error: approximate value of `f{32, 64}::consts::FRAC_1_SQRT_2` found
--> $DIR/approx_const.rs:16:32
--> $DIR/approx_const.rs:15:32
|
LL | let almost_frac_1_sqrt_2 = 0.70711;
| ^^^^^^^
@ -41,7 +41,7 @@ LL | let almost_frac_1_sqrt_2 = 0.70711;
= help: consider using the constant directly
error: approximate value of `f{32, 64}::consts::FRAC_2_PI` found
--> $DIR/approx_const.rs:20:24
--> $DIR/approx_const.rs:19:24
|
LL | let my_frac_2_pi = 0.63661977;
| ^^^^^^^^^^
@ -49,7 +49,7 @@ LL | let my_frac_2_pi = 0.63661977;
= help: consider using the constant directly
error: approximate value of `f{32, 64}::consts::FRAC_2_SQRT_PI` found
--> $DIR/approx_const.rs:24:27
--> $DIR/approx_const.rs:23:27
|
LL | let my_frac_2_sq_pi = 1.128379;
| ^^^^^^^^
@ -57,7 +57,7 @@ LL | let my_frac_2_sq_pi = 1.128379;
= help: consider using the constant directly
error: approximate value of `f{32, 64}::consts::FRAC_PI_2` found
--> $DIR/approx_const.rs:28:24
--> $DIR/approx_const.rs:27:24
|
LL | let my_frac_pi_2 = 1.57079632679;
| ^^^^^^^^^^^^^
@ -65,7 +65,7 @@ LL | let my_frac_pi_2 = 1.57079632679;
= help: consider using the constant directly
error: approximate value of `f{32, 64}::consts::FRAC_PI_3` found
--> $DIR/approx_const.rs:32:24
--> $DIR/approx_const.rs:31:24
|
LL | let my_frac_pi_3 = 1.04719755119;
| ^^^^^^^^^^^^^
@ -73,7 +73,7 @@ LL | let my_frac_pi_3 = 1.04719755119;
= help: consider using the constant directly
error: approximate value of `f{32, 64}::consts::FRAC_PI_4` found
--> $DIR/approx_const.rs:36:24
--> $DIR/approx_const.rs:35:24
|
LL | let my_frac_pi_4 = 0.785398163397;
| ^^^^^^^^^^^^^^
@ -81,7 +81,7 @@ LL | let my_frac_pi_4 = 0.785398163397;
= help: consider using the constant directly
error: approximate value of `f{32, 64}::consts::FRAC_PI_6` found
--> $DIR/approx_const.rs:40:24
--> $DIR/approx_const.rs:39:24
|
LL | let my_frac_pi_6 = 0.523598775598;
| ^^^^^^^^^^^^^^
@ -89,7 +89,7 @@ LL | let my_frac_pi_6 = 0.523598775598;
= help: consider using the constant directly
error: approximate value of `f{32, 64}::consts::FRAC_PI_8` found
--> $DIR/approx_const.rs:44:24
--> $DIR/approx_const.rs:43:24
|
LL | let my_frac_pi_8 = 0.3926990816987;
| ^^^^^^^^^^^^^^^
@ -97,7 +97,7 @@ LL | let my_frac_pi_8 = 0.3926990816987;
= help: consider using the constant directly
error: approximate value of `f{32, 64}::consts::LN_10` found
--> $DIR/approx_const.rs:48:20
--> $DIR/approx_const.rs:47:20
|
LL | let my_ln_10 = 2.302585092994046;
| ^^^^^^^^^^^^^^^^^
@ -105,7 +105,7 @@ LL | let my_ln_10 = 2.302585092994046;
= help: consider using the constant directly
error: approximate value of `f{32, 64}::consts::LN_2` found
--> $DIR/approx_const.rs:52:19
--> $DIR/approx_const.rs:51:19
|
LL | let my_ln_2 = 0.6931471805599453;
| ^^^^^^^^^^^^^^^^^^
@ -113,7 +113,7 @@ LL | let my_ln_2 = 0.6931471805599453;
= help: consider using the constant directly
error: approximate value of `f{32, 64}::consts::LOG10_E` found
--> $DIR/approx_const.rs:56:22
--> $DIR/approx_const.rs:55:22
|
LL | let my_log10_e = 0.4342944819032518;
| ^^^^^^^^^^^^^^^^^^
@ -121,7 +121,7 @@ LL | let my_log10_e = 0.4342944819032518;
= help: consider using the constant directly
error: approximate value of `f{32, 64}::consts::LOG2_E` found
--> $DIR/approx_const.rs:60:21
--> $DIR/approx_const.rs:59:21
|
LL | let my_log2_e = 1.4426950408889634;
| ^^^^^^^^^^^^^^^^^^
@ -129,7 +129,7 @@ LL | let my_log2_e = 1.4426950408889634;
= help: consider using the constant directly
error: approximate value of `f{32, 64}::consts::LOG2_10` found
--> $DIR/approx_const.rs:64:19
--> $DIR/approx_const.rs:63:19
|
LL | let log2_10 = 3.321928094887362;
| ^^^^^^^^^^^^^^^^^
@ -137,7 +137,7 @@ LL | let log2_10 = 3.321928094887362;
= help: consider using the constant directly
error: approximate value of `f{32, 64}::consts::LOG10_2` found
--> $DIR/approx_const.rs:68:19
--> $DIR/approx_const.rs:67:19
|
LL | let log10_2 = 0.301029995663981;
| ^^^^^^^^^^^^^^^^^
@ -145,7 +145,7 @@ LL | let log10_2 = 0.301029995663981;
= help: consider using the constant directly
error: approximate value of `f{32, 64}::consts::PI` found
--> $DIR/approx_const.rs:72:17
--> $DIR/approx_const.rs:71:17
|
LL | let my_pi = 3.1415;
| ^^^^^^
@ -153,7 +153,7 @@ LL | let my_pi = 3.1415;
= help: consider using the constant directly
error: approximate value of `f{32, 64}::consts::PI` found
--> $DIR/approx_const.rs:74:21
--> $DIR/approx_const.rs:73:21
|
LL | let almost_pi = 3.14;
| ^^^^
@ -161,7 +161,7 @@ LL | let almost_pi = 3.14;
= help: consider using the constant directly
error: approximate value of `f{32, 64}::consts::SQRT_2` found
--> $DIR/approx_const.rs:78:18
--> $DIR/approx_const.rs:77:18
|
LL | let my_sq2 = 1.4142;
| ^^^^^^
@ -169,7 +169,7 @@ LL | let my_sq2 = 1.4142;
= help: consider using the constant directly
error: approximate value of `f{32, 64}::consts::TAU` found
--> $DIR/approx_const.rs:82:18
--> $DIR/approx_const.rs:81:18
|
LL | let my_tau = 6.2832;
| ^^^^^^
@ -177,7 +177,7 @@ LL | let my_tau = 6.2832;
= help: consider using the constant directly
error: approximate value of `f{32, 64}::consts::TAU` found
--> $DIR/approx_const.rs:84:22
--> $DIR/approx_const.rs:83:22
|
LL | let almost_tau = 6.28;
| ^^^^

5
tests/ui/similar_names.rs
View File

@ -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;

50
tests/ui/similar_names.stderr
View File

@ -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