Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to allow clippy::similar_names #9514

Open
schteve opened this issue Sep 17, 2022 · 4 comments
Open

Unable to allow clippy::similar_names #9514

schteve opened this issue Sep 17, 2022 · 4 comments
Labels
C-bug Category: Clippy is not doing the correct thing

Comments

@schteve
Copy link
Contributor

schteve commented Sep 17, 2022

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 schteve added the C-bug Category: Clippy is not doing the correct thing label Sep 17, 2022
@edward-shen
Copy link

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.

@schteve
Copy link
Contributor Author

schteve commented Sep 21, 2022

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).

@Alexendoo Alexendoo transferred this issue from rust-lang/rust Sep 22, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Sep 22, 2022
Add note to clippy::non_expressive_names doc

Addresses confusion in rust-lang/rust-clippy#9514 by updating the lint docs.
@schteve
Copy link
Contributor Author

schteve commented Sep 23, 2022

Addressed the confusion with rust-lang/rust#102123

@schteve schteve closed this as completed Sep 23, 2022
@Alexendoo
Copy link
Member

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

@Alexendoo Alexendoo reopened this Sep 23, 2022
flip1995 pushed a commit to flip1995/rust-clippy that referenced this issue Sep 28, 2022
Add note to clippy::non_expressive_names doc

Addresses confusion in rust-lang#9514 by updating the lint docs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing
Projects
None yet
Development

No branches or pull requests

3 participants