Skip to content

extra_unused_type_parameters probably should not warn empty functions #10319

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

Closed
taiki-e opened this issue Feb 11, 2023 · 1 comment · Fixed by #10321
Closed

extra_unused_type_parameters probably should not warn empty functions #10319

taiki-e opened this issue Feb 11, 2023 · 1 comment · Fixed by #10321
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have

Comments

@taiki-e
Copy link
Member

taiki-e commented Feb 11, 2023

Summary

It is a popular pattern to define an empty function with an unused type parameter with trait bounds to ensure that the type implements the trait.
extra_unused_type_parameters probably should not warn this pattern (at least by default).

Mentioning @mkrasnitski who implemented this lint in #10028.

Lint Name

extra_unused_type_parameters

Reproducer

I tried this code:

#![warn(clippy::extra_unused_type_parameters)]

#[allow(dead_code)]
fn assert_send<T: Send>() {}

#[allow(dead_code)]
struct S(());

const _: fn() = || {
    assert_send::<S>();
};

I saw this happen:

warning: type parameter goes unused in function definition
 --> src/lib.rs:4:15
  |
4 | fn assert_send<T: Send>() {}
  |               ^^^^^^^^^
  |
  = help: consider removing the parameter
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#extra_unused_type_parameters
note: the lint level is defined here
 --> src/lib.rs:1:9
  |
1 | #![warn(clippy::extra_unused_type_parameters)]
  |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

I expected to see this happen: no warning

Version

rustc 1.69.0-nightly (2773383a3 2023-02-10)
binary: rustc
commit-hash: 2773383a314a4b8f481ce2bed12c32de794ffbe9
commit-date: 2023-02-10
host: aarch64-apple-darwin
release: 1.69.0-nightly
LLVM version: 15.0.7

Additional Labels

No response

@taiki-e taiki-e added C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have labels Feb 11, 2023
@mkrasnitski
Copy link
Contributor

mkrasnitski commented Feb 11, 2023

Hmm, this makes me think about the best way to address this. Obviously, the original intent of the lint here is more-or-less correct, in that the only valid way to use the function is by adding a turbofish. But in this case that concern is irrelevant.

It's also a bit difficult to pick exactly which features of the function to check for when turning off the lint. I think maybe a sane choice would be: if a parameter has a bound on it and the function is empty, then don't consider that parameter unused. Of course, the question is how to define if a function is empty - should only the body be empty, or should the function take no arguments as well? I think just checking the body might be the better option here.

Open to opinions here - I'm not sure if avoiding false negatives or false positives is more important.

zecakeh added a commit to zecakeh/matrix-rust-sdk that referenced this issue Feb 13, 2023
Triggers false positives.
See discussion in rust-lang/rust-clippy#10319.
Possibly fixed in rust-lang/rust-clippy#10321.

Signed-off-by: Kévin Commaille <[email protected]>
jplatte pushed a commit to matrix-org/matrix-rust-sdk that referenced this issue Feb 13, 2023
Triggers false positives.
See discussion in rust-lang/rust-clippy#10319.
Possibly fixed in rust-lang/rust-clippy#10321.

Signed-off-by: Kévin Commaille <[email protected]>
@bors bors closed this as completed in 595f783 Feb 15, 2023
jplatte pushed a commit to matrix-org/matrix-rust-sdk-crypto-wasm that referenced this issue Jul 13, 2023
Triggers false positives.
See discussion in rust-lang/rust-clippy#10319.
Possibly fixed in rust-lang/rust-clippy#10321.

Signed-off-by: Kévin Commaille <[email protected]>
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 I-false-positive Issue: The lint was triggered on code it shouldn't have
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants