rust-clippy icon indicating copy to clipboard operation
rust-clippy copied to clipboard

Unable to allow clippy::similar_names

Open schteve opened this issue 1 year ago • 4 comments

I am unable to disable (allow) the clippy::similar_names lint the way I expect. Usually I would place #[allow(clippy::x)] before the offending line, and the lint error would go away. In this case the allow seems to have no effect.

For example, this code shows the lint error being given despite being allowed on the testb line (playground):

#![deny(clippy::pedantic)]

fn main() {
    let testa = 0u32;
    #[allow(clippy::similar_names)]
    let testb = 0u32;
    println!("{testa}, {testb}"); // Use variables
}

error: binding's name is too similar to existing binding
 --> src/main.rs:6:9
  |
6 |     let testb = 0u32;
  |         ^^^^^
  |
note: the lint level is defined here
 --> src/main.rs:1:9
  |
1 | #![deny(clippy::pedantic)]
  |         ^^^^^^^^^^^^^^^^
  = note: `#[deny(clippy::similar_names)]` implied by `#[deny(clippy::pedantic)]`
note: existing binding defined here
 --> src/main.rs:4:9
  |
4 |     let testa = 0u32;
  |         ^^^^^
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#similar_names

I expected that allowing on the testb line would hide the error since that's the line referenced by the error message. To cover my bases, I can also put the allow on testa with the same result (playground):

#![deny(clippy::pedantic)]

fn main() {
    #[allow(clippy::similar_names)]
    let testa = 0u32;
    #[allow(clippy::similar_names)]
    let testb = 0u32;
    println!("{testa}, {testb}"); // Use variables
}

What does work is allowing on the containing function (playground). But of course this prevents other instances from being caught so is not an ideal workaround.

#![deny(clippy::pedantic)]

#[allow(clippy::similar_names)]
fn main() {
    let testa = 0u32;
    let testb = 0u32;
    println!("{testa}, {testb}"); // Use variables
}

I found this on stable 1.63 but also seems to occur on beta and nightly.

schteve avatar Sep 17 '22 23:09 schteve

I think there's just confusion in what the semantics of this lint does. I think you're expecting to be "Exclude this name from the list of names that are checked for similar variable names", but the lint itself is closer to the semantics of "Make sure nothing is similar in the applied scope", which does nothing in your examples since testa or testb would be the only names in those contexts.

edward-shen avatar Sep 21 '22 21:09 edward-shen

That's certainly possible! I don't know how to tell what the intent is. In showing this issue to several other people, no one thought what I was trying to do was unreasonable.

If it's the case that we don't want the lint to work the way I was expecting, it would be helpful to make this more obvious. Maybe the solution is just an update to the lint docs to make this clear.

I suppose the current behavior is reasonable, although it does allow some cases to slip through. For example if there's a single block with one set of similar names you want to ignore, but you want to be warned about any other similar names. Seems pretty unlikely though which is why I think it's reasonable (as long as the expectation is clear).

schteve avatar Sep 21 '22 22:09 schteve

Addressed the confusion with https://github.com/rust-lang/rust/pull/102123

schteve avatar Sep 23 '22 11:09 schteve

I'd say we can still fix this to not fire a lint if there's an allow on the relevant binding

Alexendoo avatar Sep 23 '22 12:09 Alexendoo