fix self_named_module_files: track module with relative path

tracks each sub folder by its relative path from crate's working
directory instead of individual path segments

changelog: none

Signed-off-by: Zihan <[email protected]>

Author: zihan0822

clippy_lints/src/lib.rs

@@ -670,7 +670,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
     store.register_late_pass(|_| Box::new(from_str_radix_10::FromStrRadix10));
     store.register_late_pass(move |_| Box::new(if_then_some_else_none::IfThenSomeElseNone::new(conf)));
     store.register_late_pass(|_| Box::new(bool_assert_comparison::BoolAssertComparison));
-    store.register_early_pass(move || Box::new(module_style::ModStyle));
+    store.register_early_pass(move || Box::new(module_style::ModStyle::default()));
     store.register_late_pass(|_| Box::<unused_async::UnusedAsync>::default());
     store.register_late_pass(move |tcx| Box::new(disallowed_types::DisallowedTypes::new(tcx, conf)));
     store.register_late_pass(move |tcx| Box::new(missing_enforced_import_rename::ImportRename::new(tcx, conf)));

clippy_lints/src/module_style.rs

@@ -1,12 +1,11 @@
 use clippy_utils::diagnostics::span_lint_and_then;
-use rustc_ast::ast;
-use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexSet};
+use rustc_ast::ast::{self, Inline, ItemKind, ModKind};
+use rustc_data_structures::fx::FxHashSet;
 use rustc_lint::{EarlyContext, EarlyLintPass, Level, LintContext};
 use rustc_session::impl_lint_pass;
 use rustc_span::def_id::LOCAL_CRATE;
 use rustc_span::{FileName, SourceFile, Span, SyntaxContext};
-use std::ffi::OsStr;
-use std::path::{Component, Path};
+use std::path::{Path, PathBuf};
 
 declare_clippy_lint! {
     /// ### What it does
@@ -67,100 +66,95 @@ declare_clippy_lint! {
     "checks that module layout is consistent"
 }
 
-pub struct ModStyle;
-
 impl_lint_pass!(ModStyle => [MOD_MODULE_FILES, SELF_NAMED_MODULE_FILES]);
 
+#[derive(Default)]
+pub struct ModStyle {
+    mod_folders: FxHashSet<PathBuf>,
+    working_dir: Option<PathBuf>,
+}
+
 impl EarlyLintPass for ModStyle {
     fn check_crate(&mut self, cx: &EarlyContext<'_>, _: &ast::Crate) {
         if cx.builder.lint_level(MOD_MODULE_FILES).level == Level::Allow
             && cx.builder.lint_level(SELF_NAMED_MODULE_FILES).level == Level::Allow
         {
             return;
         }
-
-        let files = cx.sess().source_map().files();
-
-        let Some(trim_to_src) = cx.sess().opts.working_dir.local_path() else {
+        let Some(working_dir) = cx.sess().opts.working_dir.local_path() else {
             return;
         };
-
-        // `folder_segments` is all unique folder path segments `path/to/foo.rs` gives
-        // `[path, to]` but not foo
-        let mut folder_segments = FxIndexSet::default();
-        // `mod_folders` is all the unique folder names that contain a mod.rs file
-        let mut mod_folders = FxHashSet::default();
-        // `file_map` maps file names to the full path including the file name
-        // `{ foo => path/to/foo.rs, .. }
-        let mut file_map = FxHashMap::default();
-        for file in files.iter() {
-            if let FileName::Real(name) = &file.name
-                && let Some(lp) = name.local_path()
-                && file.cnum == LOCAL_CRATE
-            {
-                // [#8887](https://github.com/rust-lang/rust-clippy/issues/8887)
-                // Only check files in the current crate.
-                // Fix false positive that crate dependency in workspace sub directory
-                // is checked unintentionally.
-                let path = if lp.is_relative() {
-                    lp
-                } else if let Ok(relative) = lp.strip_prefix(trim_to_src) {
-                    relative
+        self.mod_folders = cx
+            .sess()
+            .source_map()
+            .files()
+            .iter()
+            .filter_map(|file| {
+                let path = trim_file_path_to(file, working_dir)?;
+                if path.extension()?.eq("rs") {
+                    check_mod_module(cx, path, file);
+                    let mut mod_folder = path.to_path_buf();
+                    mod_folder.pop();
+                    Some(mod_folder)
                 } else {
-                    continue;
-                };
-
-                if let Some(stem) = path.file_stem() {
-                    file_map.insert(stem, (file, path));
+                    None
                 }
-                process_paths_for_mod_files(path, &mut folder_segments, &mut mod_folders);
-                check_self_named_mod_exists(cx, path, file);
-            }
+            })
+            .collect();
+        self.working_dir = Some(working_dir.to_path_buf());
+    }
+
+    fn check_item(&mut self, cx: &EarlyContext<'_>, item: &ast::Item) {
+        if let ItemKind::Mod(.., mod_kind) = &item.kind
+            && let ModKind::Loaded(_, inline, mod_spans, _) = mod_kind
+            && matches!(inline, Inline::No)
+            && let mod_file = cx.sess().source_map().lookup_source_file(mod_spans.inner_span.lo())
+            && let Some(working_dir) = self.working_dir.as_ref()
+            && let Some(mod_path) = trim_file_path_to(&mod_file, working_dir.as_path())
+        {
+            check_self_named_module(cx, mod_path, &mod_file, &self.mod_folders);
         }
+    }
+}
 
-        for folder in &folder_segments {
-            if !mod_folders.contains(folder)
-                && let Some((file, path)) = file_map.get(folder)
-            {
-                span_lint_and_then(
-                    cx,
-                    SELF_NAMED_MODULE_FILES,
-                    Span::new(file.start_pos, file.start_pos, SyntaxContext::root(), None),
-                    format!("`mod.rs` files are required, found `{}`", path.display()),
-                    |diag| {
-                        let mut correct = path.to_path_buf();
-                        correct.pop();
-                        correct.push(folder);
-                        correct.push("mod.rs");
-                        diag.help(format!("move `{}` to `{}`", path.display(), correct.display(),));
-                    },
-                );
-            }
+fn trim_file_path_to<'a>(file: &'a SourceFile, prefix: &'a Path) -> Option<&'a Path> {
+    if let FileName::Real(name) = &file.name
+        && let Some(mut path) = name.local_path()
+        && file.cnum == LOCAL_CRATE
+    {
+        if !path.is_relative() {
+            path = path.strip_prefix(prefix).ok()?;
         }
+        Some(path)
+    } else {
+        None
     }
 }
 
-/// For each `path` we add each folder component to `folder_segments` and if the file name
-/// is `mod.rs` we add it's parent folder to `mod_folders`.
-fn process_paths_for_mod_files<'a>(
-    path: &'a Path,
-    folder_segments: &mut FxIndexSet<&'a OsStr>,
-    mod_folders: &mut FxHashSet<&'a OsStr>,
-) {
-    let mut comp = path.components().rev().peekable();
-    let _: Option<_> = comp.next();
-    if path.ends_with("mod.rs") {
-        mod_folders.insert(comp.peek().map(|c| c.as_os_str()).unwrap_or_default());
+fn check_self_named_module(cx: &EarlyContext<'_>, path: &Path, file: &SourceFile, mod_folders: &FxHashSet<PathBuf>) {
+    if !path.ends_with("mod.rs")
+        && let mod_folder = path.with_extension("")
+        && mod_folders.contains(&mod_folder)
+    {
+        span_lint_and_then(
+            cx,
+            SELF_NAMED_MODULE_FILES,
+            Span::new(file.start_pos, file.start_pos, SyntaxContext::root(), None),
+            format!("`mod.rs` files are required, found `{}`", path.display()),
+            |diag| {
+                let mut mod_file = mod_folder.to_path_buf();
+                mod_file.push("mod.rs");
+
+                diag.help(format!("move `{}` to `{}`", path.display(), mod_file.display()));
+            },
+        );
     }
-    let folders = comp.filter_map(|c| if let Component::Normal(s) = c { Some(s) } else { None });
-    folder_segments.extend(folders);
 }
 
-/// Checks every path for the presence of `mod.rs` files and emits the lint if found.
 /// We should not emit a lint for test modules in the presence of `mod.rs`.
 /// Using `mod.rs` in integration tests is a [common pattern](https://doc.rust-lang.org/book/ch11-03-test-organization.html#submodules-in-integration-test)
 /// for code-sharing between tests.
-fn check_self_named_mod_exists(cx: &EarlyContext<'_>, path: &Path, file: &SourceFile) {
+fn check_mod_module(cx: &EarlyContext<'_>, path: &Path, file: &SourceFile) {
     if path.ends_with("mod.rs") && !path.starts_with("tests") {
         span_lint_and_then(
             cx,

tests/ui-cargo/module_style/duplicated_mod_names_14697/Cargo.stderr

@@ -0,0 +1,11 @@
+error: `mod.rs` files are required, found `src/foo.rs`
+ --> src/foo.rs:1:1
+  |
+1 | pub mod bar;
+  | ^
+  |
+  = help: move `src/foo.rs` to `src/foo/mod.rs`
+  = note: `-D clippy::self-named-module-files` implied by `-D warnings`
+  = help: to override `-D warnings` add `#[allow(clippy::self_named_module_files)]`
+
+error: could not compile `duplicated-mod-names-14697` (lib) due to 1 previous error

tests/ui-cargo/module_style/duplicated_mod_names_14697/Cargo.toml

@@ -0,0 +1,11 @@
+# Should trigger when multiple mods with the same name exist and not all of them follow self-named convention.
+# See issue #14697.
+[package]
+name = "duplicated-mod-names-14697"
+version = "0.1.0"
+edition = "2024"
+publish = false
+
+# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
+
+[dependencies]

tests/ui-cargo/module_style/duplicated_mod_names_14697/src/foo.rs

@@ -0,0 +1 @@
+pub mod bar;

tests/ui-cargo/module_style/duplicated_mod_names_14697/src/foo/bar.rs

@@ -0,0 +1 @@
+

tests/ui-cargo/module_style/duplicated_mod_names_14697/src/lib.rs

@@ -0,0 +1,4 @@
+#![warn(clippy::self_named_module_files)]
+
+pub mod foo;
+pub mod other;

tests/ui-cargo/module_style/duplicated_mod_names_14697/src/other/foo/mod.rs

@@ -0,0 +1 @@
+

tests/ui-cargo/module_style/duplicated_mod_names_14697/src/other/mod.rs

@@ -0,0 +1 @@
+pub mod foo;

tests/ui-cargo/module_style/segment_with_mod_name_10271_11916/Cargo.toml

@@ -0,0 +1,10 @@
+# Should not produce FP when irrelavant path segment shares the same name with module.
+# See issue #10271 and #11916.
+[package]
+name = "segment-with-mod-name-10271-11916"
+version = "0.1.0"
+edition = "2024"
+publish = false
+
+[workspace]
+members = ["foo/bar"]