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

module_name_repetitions false positive when in private module #8524

Open
ecton opened this issue Mar 11, 2022 · 5 comments
Open

module_name_repetitions false positive when in private module #8524

ecton opened this issue Mar 11, 2022 · 5 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

@ecton
Copy link

ecton commented Mar 11, 2022

Summary

When organizing large modules, I sometimes want to split a module into multiple files, but re-export all of the contents as if the module were one single module. The public interface to the module doesn't have any module name repetitions in my example below, yet the warning is still emitted because the containing module's scope isn't considered.

I believe this lint should be tightened to only emit if the module name that is being repeated is pub (and maybe pub(crate) but I trust others to make that judgement call).

This affects me often enough that I regularly disable this lint in my projects. This morning, I came up with the idea that maybe the lint could be tightened up in this particular situation.

Lint Name

module_name_repetitions

Reproducer

I tried this code:

#![warn(clippy::module_name_repetitions)]

mod error {
    pub enum Error {}
    pub enum OtherError {}
}

pub use error::{Error, OtherError};

I saw this happen:

warning: item name ends with its containing module's name
 --> src/main.rs:5:5
  |
5 |     pub enum OtherError {}
  |     ^^^^^^^^^^^^^^^^^^^^^^
  |
note: the lint level is defined here
 --> src/main.rs:1:9
  |
1 | #![warn(clippy::module_name_repetitions)]
  |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#module_name_repetitions

I expected no warnings to be emitted.

Version

rustc 1.59.0 (9d1b2106e 2022-02-23)
binary: rustc
commit-hash: 9d1b2106e23b1abd32fce1f17267604a5102f57a
commit-date: 2022-02-23
host: x86_64-unknown-linux-gnu
release: 1.59.0
LLVM version: 13.0.0

Additional Labels

No response

@ecton ecton 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 Mar 11, 2022
@wmmc88
Copy link

wmmc88 commented Sep 21, 2023

I totally agree with this! Being able to break a module into submodules spread out in several files, and re-exporting them in the original module name is something I find quite useful as projects get larger!

Does anyone think there is any merit to this lint when the module that an item is contained in is private? Is there a better way to approach the aforementioned situation which avoids the module name repetition (even if its private)?

@Centri3
Copy link
Member

Centri3 commented Sep 21, 2023

This was fixed for module_inception in #10917. The relevant change is:
https://github.com/rust-lang/rust-clippy/blob/c55669513af7e0afaf65bd9b150fd827dc4446c2/clippy_lints/src/enum_variants.rs#L262C53-L262C53

We can move this to the outer if expression and rename the configuration option and it should all work.

@TheLostLambda
Copy link

Good to see this bug report already exists! I was just about to make one after having the same issue!

As for @wmmc88 's question, the main rationale for this lint seems to be keeping public API clean, so I think it's fine to allow this by default, but perhaps we take the (inverted) approach of #10917 and add a configuration flag for disallowing it if you want to force yourself to avoid this kind of repetition within your private modules as well?

A bit like how you can opt-in to warnings about missing docs on private functions?

@TheLostLambda
Copy link

@wmmc88 A follow-up thought though: this lint doesn't (I believe) currently look at anything non-pub, so if we're adding the flag to test private modules for this repetition, that should probably also look at the private functions within?

Remembering the obvious, that this lint doesn't apply to private functions, definitely strengthens the case that it shouldn't (by default) apply to private modules either!

@vepain
Copy link

vepain commented Apr 27, 2024

Even for public modules, there are this false positives.

something
|_ mod.rs
|_ iter.rs
// mod.rs

pub mod self::iter;
// iter.rs

pub struct IterSomething {
    ...
}

If I add the following into the clippy configuration file .clippy.toml, the false positive disappears.

allowed-prefixes = ["..", "Iter"]

This is unexpected as the clippy doc declares:

  • By default, the following prefixes are allowed: to, as, into, from, try_into and try_from
  • PascalCase variant is included automatically for each snake_case variant (e.g. if try_into is included, TryInto will also be included)

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

5 participants