Auto merge of #13444 - kpreid:fix-8524-private-rep, r=blyxyas

`module_name_repetitions`: don't warn if the item is in a private module.

Fixes <#8524>.

There is still a warning (as there should be) if the item is reexported by name, but not by glob; that would require further work to examine the names in the glob, and I haven't looked into that.

Credit to `@Centri3` for suggesting approximately this simple fix in <#8524 (comment)>. However, per later comment <#8524 (comment)>, I am not making it configuration-dependent, but *always* checking public items in public modules only.

changelog: [`module_name_repetitions`]: don't warn if the item is in a private module.

Author: bors

clippy_lints/src/item_name_repetitions.rs

@@ -50,11 +50,28 @@ declare_clippy_lint! {
 
 declare_clippy_lint! {
     /// ### What it does
-    /// Detects type names that are prefixed or suffixed by the
-    /// containing module's name.
+    /// Detects public item names that are prefixed or suffixed by the
+    /// containing public module's name.
     ///
     /// ### Why is this bad?
-    /// It requires the user to type the module name twice.
+    /// It requires the user to type the module name twice in each usage,
+    /// especially if they choose to import the module rather than its contents.
+    ///
+    /// Lack of such repetition is also the style used in the Rust standard library;
+    /// e.g. `io::Error` and `fmt::Error` rather than `io::IoError` and `fmt::FmtError`;
+    /// and `array::from_ref` rather than `array::array_from_ref`.
+    ///
+    /// ### Known issues
+    /// Glob re-exports are ignored; e.g. this will not warn even though it should:
+    ///
+    /// ```no_run
+    /// pub mod foo {
+    ///     mod iteration {
+    ///         pub struct FooIter {}
+    ///     }
+    ///     pub use iteration::*; // creates the path `foo::FooIter`
+    /// }
+    /// ```
     ///
     /// ### Example
     /// ```no_run
@@ -389,12 +406,12 @@ impl LateLintPass<'_> for ItemNameRepetitions {
         let item_name = item.ident.name.as_str();
         let item_camel = to_camel_case(item_name);
         if !item.span.from_expansion() && is_present_in_source(cx, item.span) {
-            if let [.., (mod_name, mod_camel, owner_id)] = &*self.modules {
+            if let [.., (mod_name, mod_camel, mod_owner_id)] = &*self.modules {
                 // constants don't have surrounding modules
                 if !mod_camel.is_empty() {
                     if mod_name == &item.ident.name
                         && let ItemKind::Mod(..) = item.kind
-                        && (!self.allow_private_module_inception || cx.tcx.visibility(owner_id.def_id).is_public())
+                        && (!self.allow_private_module_inception || cx.tcx.visibility(mod_owner_id.def_id).is_public())
                     {
                         span_lint(
                             cx,
@@ -403,9 +420,13 @@ impl LateLintPass<'_> for ItemNameRepetitions {
                             "module has the same name as its containing module",
                         );
                     }
+
                     // The `module_name_repetitions` lint should only trigger if the item has the module in its
                     // name. Having the same name is accepted.
-                    if cx.tcx.visibility(item.owner_id).is_public() && item_camel.len() > mod_camel.len() {
+                    if cx.tcx.visibility(item.owner_id).is_public()
+                        && cx.tcx.visibility(mod_owner_id.def_id).is_public()
+                        && item_camel.len() > mod_camel.len()
+                    {
                         let matching = count_match_start(mod_camel, &item_camel);
                         let rmatching = count_match_end(mod_camel, &item_camel);
                         let nchars = mod_camel.chars().count();

clippy_lints/src/literal_representation.rs

@@ -412,7 +412,6 @@ impl LiteralDigitGrouping {
     }
 }
 
-#[expect(clippy::module_name_repetitions)]
 pub struct DecimalLiteralRepresentation {
     threshold: u64,
 }

clippy_lints/src/macro_use.rs

@@ -43,7 +43,6 @@ impl MacroRefData {
 }
 
 #[derive(Default)]
-#[expect(clippy::module_name_repetitions)]
 pub struct MacroUseImports {
     /// the actual import path used and the span of the attribute above it. The value is
     /// the location, where the lint should be emitted.

clippy_lints/src/vec.rs

@@ -17,7 +17,6 @@ use rustc_middle::ty::layout::LayoutOf;
 use rustc_session::impl_lint_pass;
 use rustc_span::{DesugaringKind, Span, sym};
 
-#[expect(clippy::module_name_repetitions)]
 pub struct UselessVec {
     too_large_for_stack: u64,
     msrv: Msrv,

tests/ui-toml/item_name_repetitions/allowed_prefixes/item_name_repetitions.rs

@@ -1,7 +1,7 @@
 #![warn(clippy::module_name_repetitions)]
 #![allow(dead_code)]
 
-mod foo {
+pub mod foo {
     // #12544 - shouldn't warn if item name consists only of an allowed prefix and a module name.
     // In this test, allowed prefixes are configured to be ["bar"].
 

tests/ui-toml/item_name_repetitions/allowed_prefixes_extend/item_name_repetitions.rs

@@ -1,7 +1,7 @@
 #![warn(clippy::module_name_repetitions)]
 #![allow(dead_code)]
 
-mod foo {
+pub mod foo {
     // #12544 - shouldn't warn if item name consists only of an allowed prefix and a module name.
     // In this test, allowed prefixes are configured to be all of the default prefixes and ["bar"].
 

tests/ui/module_name_repetitions.rs

@@ -3,7 +3,7 @@
 #![warn(clippy::module_name_repetitions)]
 #![allow(dead_code)]
 
-mod foo {
+pub mod foo {
     pub fn foo() {}
     pub fn foo_bar() {}
     //~^ ERROR: item name starts with its containing module's name
@@ -20,6 +20,22 @@ mod foo {
     // Should not warn
     pub struct Foobar;
 
+    // #8524 - shouldn't warn when item is declared in a private module...
+    mod error {
+        pub struct Error;
+        pub struct FooError;
+    }
+    pub use error::Error;
+    // ... but should still warn when the item is reexported to create a *public* path with repetition.
+    pub use error::FooError;
+    //~^ ERROR: item name starts with its containing module's name
+
+    // FIXME: This should also warn because it creates the public path `foo::FooIter`.
+    mod iter {
+        pub struct FooIter;
+    }
+    pub use iter::*;
+
     // #12544 - shouldn't warn if item name consists only of an allowed prefix and a module name.
     pub fn to_foo() {}
     pub fn into_foo() {}

tests/ui/module_name_repetitions.stderr

@@ -31,5 +31,11 @@ error: item name starts with its containing module's name
 LL |     pub struct Foo7Bar;
    |                ^^^^^^^
 
-error: aborting due to 5 previous errors
+error: item name starts with its containing module's name
+  --> tests/ui/module_name_repetitions.rs:30:20
+   |
+LL |     pub use error::FooError;
+   |                    ^^^^^^^^
+
+error: aborting due to 6 previous errors