Rework module_inception

* Don't check for repetition if the previous module crosses a body boundary.
* Don't take a snippet of the entire item.
* Check each item's visibility once.
* Use `is_from_proc_macro` before linting

Author: Jarcho

clippy_lints/src/item_name_repetitions.rs

@@ -1,11 +1,9 @@
 use clippy_config::Conf;
 use clippy_utils::diagnostics::{span_lint, span_lint_and_help, span_lint_hir};
-use clippy_utils::is_bool;
-use clippy_utils::macros::span_is_local;
-use clippy_utils::source::is_present_in_source;
 use clippy_utils::str_utils::{camel_case_split, count_match_end, count_match_start, to_camel_case, to_snake_case};
+use clippy_utils::{is_bool, is_from_proc_macro};
 use rustc_data_structures::fx::FxHashSet;
-use rustc_hir::{EnumDef, FieldDef, Item, ItemKind, OwnerId, QPath, TyKind, Variant, VariantData};
+use rustc_hir::{Body, EnumDef, FieldDef, Item, ItemKind, QPath, TyKind, UseKind, Variant, VariantData};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::impl_lint_pass;
 use rustc_span::symbol::Symbol;
@@ -158,7 +156,8 @@ declare_clippy_lint! {
 }
 
 pub struct ItemNameRepetitions {
-    modules: Vec<(Symbol, String, OwnerId)>,
+    /// The module path the lint pass is in.
+    modules: Vec<ModInfo>,
     enum_threshold: u64,
     struct_threshold: u64,
     avoid_breaking_exported_api: bool,
@@ -167,6 +166,17 @@ pub struct ItemNameRepetitions {
     allowed_prefixes: FxHashSet<String>,
 }
 
+struct ModInfo {
+    name: Symbol,
+    name_camel: String,
+    /// Does this module have the `pub` visibility modifier.
+    is_public: bool,
+    /// How many bodies are between this module and the current lint pass position.
+    ///
+    /// Only the most recently seen module is updated when entering/exiting a body.
+    in_body_count: u32,
+}
+
 impl ItemNameRepetitions {
     pub fn new(conf: &'static Conf) -> Self {
         Self {
@@ -458,71 +468,109 @@ fn check_enum_tuple_path_match(variant_name: &str, variant_data: VariantData<'_>
 }
 
 impl LateLintPass<'_> for ItemNameRepetitions {
-    fn check_item_post(&mut self, _cx: &LateContext<'_>, item: &Item<'_>) {
-        let Some(_ident) = item.kind.ident() else { return };
-
-        let last = self.modules.pop();
-        assert!(last.is_some());
+    fn check_item_post(&mut self, _: &LateContext<'_>, item: &Item<'_>) {
+        if matches!(item.kind, ItemKind::Mod(..)) {
+            let prev = self.modules.pop();
+            debug_assert!(prev.is_some());
+        }
     }
 
-    fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) {
-        let Some(ident) = item.kind.ident() else { return };
-
-        let item_name = ident.name.as_str();
-        let item_camel = to_camel_case(item_name);
-        if !item.span.from_expansion() && is_present_in_source(cx, item.span)
-            && let [.., (mod_name, mod_camel, mod_owner_id)] = &*self.modules
-            // constants don't have surrounding modules
-            && !mod_camel.is_empty()
-        {
-            if mod_name == &ident.name
-                && let ItemKind::Mod(..) = item.kind
-                && (!self.allow_private_module_inception || cx.tcx.visibility(mod_owner_id.def_id).is_public())
-            {
-                span_lint(
-                    cx,
-                    MODULE_INCEPTION,
-                    item.span,
-                    "module has the same name as its containing module",
-                );
-            }
+    fn check_body(&mut self, _: &LateContext<'_>, _: &Body<'_>) {
+        if let [.., last] = &mut *self.modules {
+            last.in_body_count += 1;
+        }
+    }
 
-            // The `module_name_repetitions` lint should only trigger if the item has the module in its
-            // name. Having the same name is only accepted if `allow_exact_repetition` is set to `true`.
+    fn check_body_post(&mut self, _: &LateContext<'_>, _: &Body<'_>) {
+        if let [.., last] = &mut *self.modules {
+            last.in_body_count -= 1;
+        }
+    }
 
-            let both_are_public =
-                cx.tcx.visibility(item.owner_id).is_public() && cx.tcx.visibility(mod_owner_id.def_id).is_public();
+    fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) {
+        let ident = match item.kind {
+            ItemKind::Mod(ident, _) => {
+                if let [.., prev] = &*self.modules
+                    && prev.name == ident.name
+                    && prev.in_body_count == 0
+                    && (!self.allow_private_module_inception || prev.is_public)
+                    && !item.span.from_expansion()
+                    && !is_from_proc_macro(cx, item)
+                {
+                    span_lint(
+                        cx,
+                        MODULE_INCEPTION,
+                        item.span,
+                        "module has the same name as its containing module",
+                    );
+                }
+                ident
+            },
 
-            if both_are_public && !self.allow_exact_repetitions && item_camel == *mod_camel {
-                span_lint(
-                    cx,
-                    MODULE_NAME_REPETITIONS,
-                    ident.span,
-                    "item name is the same as its containing module's name",
-                );
-            }
+            ItemKind::Enum(ident, _, def) => {
+                if !ident.span.in_external_macro(cx.tcx.sess.source_map()) {
+                    self.check_variants(cx, item, &def);
+                }
+                ident
+            },
+            ItemKind::Struct(ident, _, data) => {
+                if let VariantData::Struct { fields, .. } = data
+                    && !ident.span.in_external_macro(cx.tcx.sess.source_map())
+                {
+                    self.check_fields(cx, item, fields);
+                }
+                ident
+            },
 
-            let is_macro = matches!(item.kind, ItemKind::Macro(_, _, _));
-            if both_are_public && item_camel.len() > mod_camel.len() && !is_macro {
-                let matching = count_match_start(mod_camel, &item_camel);
-                let rmatching = count_match_end(mod_camel, &item_camel);
-                let nchars = mod_camel.chars().count();
+            ItemKind::Const(ident, ..)
+            | ItemKind::ExternCrate(_, ident)
+            | ItemKind::Fn { ident, .. }
+            | ItemKind::Macro(ident, ..)
+            | ItemKind::Static(_, ident, ..)
+            | ItemKind::Trait(_, _, _, ident, ..)
+            | ItemKind::TraitAlias(ident, ..)
+            | ItemKind::TyAlias(ident, ..)
+            | ItemKind::Union(ident, ..)
+            | ItemKind::Use(_, UseKind::Single(ident)) => ident,
+
+            ItemKind::ForeignMod { .. } | ItemKind::GlobalAsm { .. } | ItemKind::Impl(_) | ItemKind::Use(..) => return,
+        };
 
-                let is_word_beginning = |c: char| c == '_' || c.is_uppercase() || c.is_numeric();
+        let item_name = ident.name.as_str();
+        let item_camel = to_camel_case(item_name);
 
-                if matching.char_count == nchars {
-                    match item_camel.chars().nth(nchars) {
-                        Some(c) if is_word_beginning(c) => span_lint(
+        if let [.., prev] = &*self.modules
+            && prev.is_public
+            && prev.in_body_count == 0
+            && !item.span.from_expansion()
+            && !matches!(item.kind, ItemKind::Macro(..))
+            && cx.tcx.visibility(item.owner_id).is_public()
+        {
+            if !self.allow_exact_repetitions && item_camel == prev.name_camel {
+                if !is_from_proc_macro(cx, item) {
+                    span_lint(
+                        cx,
+                        MODULE_NAME_REPETITIONS,
+                        ident.span,
+                        "item name is the same as its containing module's name",
+                    );
+                }
+            } else if item_camel.len() > prev.name_camel.len() {
+                if let Some(s) = item_camel.strip_prefix(&prev.name_camel)
+                    && let Some(c) = s.chars().next()
+                    && (c == '_' || c.is_uppercase() || c.is_numeric())
+                {
+                    if !is_from_proc_macro(cx, item) {
+                        span_lint(
                             cx,
                             MODULE_NAME_REPETITIONS,
                             ident.span,
                             "item name starts with its containing module's name",
-                        ),
-                        _ => (),
+                        );
                     }
-                }
-                if rmatching.char_count == nchars
-                    && !self.is_allowed_prefix(&item_camel[..item_camel.len() - rmatching.byte_count])
+                } else if let Some(s) = item_camel.strip_suffix(&prev.name_camel)
+                    && !self.is_allowed_prefix(s)
+                    && !is_from_proc_macro(cx, item)
                 {
                     span_lint(
                         cx,
@@ -534,17 +582,13 @@ impl LateLintPass<'_> for ItemNameRepetitions {
             }
         }
 
-        if span_is_local(item.span) {
-            match item.kind {
-                ItemKind::Enum(_, _, def) => {
-                    self.check_variants(cx, item, &def);
-                },
-                ItemKind::Struct(_, _, VariantData::Struct { fields, .. }) => {
-                    self.check_fields(cx, item, fields);
-                },
-                _ => (),
-            }
+        if matches!(item.kind, ItemKind::Mod(..)) {
+            self.modules.push(ModInfo {
+                name: ident.name,
+                name_camel: item_camel,
+                is_public: cx.tcx.visibility(item.owner_id).is_public(),
+                in_body_count: 0,
+            });
         }
-        self.modules.push((ident.name, item_camel, item.owner_id));
     }
 }

tests/ui/module_inception.rs

@@ -38,4 +38,13 @@ mod bar {
     mod bar {}
 }
 
+mod with_inner_impl {
+    struct S;
+    impl S {
+        fn f() {
+            mod with_inner_impl {}
+        }
+    }
+}
+
 fn main() {}