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

FP: used_underscore_items lints on debatably legitimate use-cases #13478

Open
mkrasnitski opened this issue Sep 30, 2024 · 0 comments
Open

FP: used_underscore_items lints on debatably legitimate use-cases #13478

mkrasnitski opened this issue Sep 30, 2024 · 0 comments
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

@mkrasnitski
Copy link
Contributor

Summary

I feel the new used_underscore_items lint is not universally applicable and would best live in restriction rather than pedantic, as there are sometimes legitimate use-cases for prefixing item names with a _.

Lint Name

used_underscore_items

Reproducer

As motivation, I'll give two examples where prefixing method names with _ makes sense:

First, as a way to reduce the impact of monomorphization on binary size for functions that have large bodies:

pub fn my_method(&self, param: impl AsRef<str>) {
    self._my_method(param.as_ref())
}

fn _my_method(&self, param: &str) {
    // large function body here
}

Second, as a way to introduce non-breaking changes when adding a new argument to a function. If we have the following initial code:

pub fn my_method(&self, param1: u32) {
    // impl
}

...and we want to add support for a second parameter without making breaking changes, we could transform it in the following way:

pub fn my_method(&self, param1: u32) {
    self._my_method(param1, None)
}

pub fn my_method_2(&self, param1: u32, param2: u64) {
    self._my_method(param1, Some(param2))
}

fn _my_method(&self, param1: u32, param2: Option<u64>) {
    // impl
}

In both cases I feel that naming the private method with a prefixed underscore is a valid choice to make, in order to preserve the name and to not conflict with the public wrapper method. Granted, maybe this still means it should be a pedantic lint, but either way I feel these two cases qualify as false positives.

Version

rustc 1.83.0-nightly (ed04567ba 2024-09-28)
binary: rustc
commit-hash: ed04567ba1d5956d1080fb8121caa005ce059e12
commit-date: 2024-09-28
host: x86_64-unknown-linux-gnu
release: 1.83.0-nightly
LLVM version: 19.1.0

Additional Labels

No response

@mkrasnitski mkrasnitski 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 Sep 30, 2024
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

No branches or pull requests

1 participant