refactor module_style (#15469)

Previous approach roughly works as follows:
For each file, it marks every directory segment of its relative path to
crate's working directory as a potential module. If segment `foo`
exists, `foo.rs` exists and `foo/mod.rs` does not exist, the lint will
trigger. The main issue with that is it ignores all the prefixes.
#14697 also explained a bit how it works.

This PR refactored previous approach with `check_item` and
`check_item_post`. We also skip mod tagged with `#[path = ...]`.

Fixes #14697
Fixes #10271
Fixes #11916

changelog: [`self_named_module_files`]: don't lint mod tagged with path
attribute
changelog: [`mod_module_files`]: don't lint mod tagged with path
attribute

Author: Jarcho

clippy_lints/src/lib.rs

@@ -671,7 +671,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_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 rustc_span::{FileName, SourceFile, Span, SyntaxContext, sym};
+use std::path::{Path, PathBuf};
+use std::sync::Arc;
 
 declare_clippy_lint! {
     /// ### What it does
@@ -60,107 +59,97 @@ declare_clippy_lint! {
     ///     mod.rs
     ///   lib.rs
     /// ```
-
     #[clippy::version = "1.57.0"]
     pub SELF_NAMED_MODULE_FILES,
     restriction,
     "checks that module layout is consistent"
 }
 
-pub struct ModStyle;
-
 impl_lint_pass!(ModStyle => [MOD_MODULE_FILES, SELF_NAMED_MODULE_FILES]);
 
+pub struct ModState {
+    contains_external: bool,
+    has_path_attr: bool,
+    mod_file: Arc<SourceFile>,
+}
+
+#[derive(Default)]
+pub struct ModStyle {
+    working_dir: Option<PathBuf>,
+    module_stack: Vec<ModState>,
+}
+
 impl EarlyLintPass for ModStyle {
     fn check_crate(&mut self, cx: &EarlyContext<'_>, _: &ast::Crate) {
+        self.working_dir = cx.sess().opts.working_dir.local_path().map(Path::to_path_buf);
+    }
+
+    fn check_item(&mut self, cx: &EarlyContext<'_>, item: &ast::Item) {
         if cx.builder.lint_level(MOD_MODULE_FILES).level == Level::Allow
             && cx.builder.lint_level(SELF_NAMED_MODULE_FILES).level == Level::Allow
         {
             return;
         }
+        if let ItemKind::Mod(.., ModKind::Loaded(_, Inline::No { .. }, mod_spans, ..)) = &item.kind {
+            let has_path_attr = item.attrs.iter().any(|attr| attr.has_name(sym::path));
+            if !has_path_attr && let Some(current) = self.module_stack.last_mut() {
+                current.contains_external = true;
+            }
+            let mod_file = cx.sess().source_map().lookup_source_file(mod_spans.inner_span.lo());
+            self.module_stack.push(ModState {
+                contains_external: false,
+                has_path_attr,
+                mod_file,
+            });
+        }
+    }
 
-        let files = cx.sess().source_map().files();
-
-        let Some(trim_to_src) = cx.sess().opts.working_dir.local_path() else {
+    fn check_item_post(&mut self, cx: &EarlyContext<'_>, item: &ast::Item) {
+        if cx.builder.lint_level(MOD_MODULE_FILES).level == Level::Allow
+            && cx.builder.lint_level(SELF_NAMED_MODULE_FILES).level == Level::Allow
+        {
             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
-                } else {
-                    continue;
-                };
-
-                if let Some(stem) = path.file_stem() {
-                    file_map.insert(stem, (file, path));
-                }
-                process_paths_for_mod_files(path, &mut folder_segments, &mut mod_folders);
-                check_self_named_mod_exists(cx, path, file);
-            }
         }
 
-        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(),));
-                    },
-                );
+        if let ItemKind::Mod(.., ModKind::Loaded(_, Inline::No { .. }, ..)) = &item.kind
+            && let Some(current) = self.module_stack.pop()
+            && !current.has_path_attr
+        {
+            let Some(path) = self
+                .working_dir
+                .as_ref()
+                .and_then(|src| try_trim_file_path_prefix(&current.mod_file, src))
+            else {
+                return;
+            };
+            if current.contains_external {
+                check_self_named_module(cx, path, &current.mod_file);
             }
+            check_mod_module(cx, path, &current.mod_file);
         }
     }
 }
 
-/// 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) {
+    if !path.ends_with("mod.rs") {
+        let mut mod_folder = path.with_extension("");
+        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| {
+                mod_folder.push("mod.rs");
+                diag.help(format!("move `{}` to `{}`", path.display(), mod_folder.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,
@@ -177,3 +166,17 @@ fn check_self_named_mod_exists(cx: &EarlyContext<'_>, path: &Path, file: &Source
         );
     }
 }
+
+fn try_trim_file_path_prefix<'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
+    }
+}

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/fail_mod/Cargo.stderr

@@ -1,19 +1,19 @@
-error: `mod.rs` files are required, found `src/bad/inner.rs`
- --> src/bad/inner.rs:1:1
+error: `mod.rs` files are required, found `src/bad/inner/stuff.rs`
+ --> src/bad/inner/stuff.rs:1:1
   |
-1 | pub mod stuff;
+1 | pub mod most;
   | ^
   |
-  = help: move `src/bad/inner.rs` to `src/bad/inner/mod.rs`
+  = help: move `src/bad/inner/stuff.rs` to `src/bad/inner/stuff/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: `mod.rs` files are required, found `src/bad/inner/stuff.rs`
- --> src/bad/inner/stuff.rs:1:1
+error: `mod.rs` files are required, found `src/bad/inner.rs`
+ --> src/bad/inner.rs:1:1
   |
-1 | pub mod most;
+1 | pub mod stuff;
   | ^
   |
-  = help: move `src/bad/inner/stuff.rs` to `src/bad/inner/stuff/mod.rs`
+  = help: move `src/bad/inner.rs` to `src/bad/inner/mod.rs`
 
 error: could not compile `fail-mod` (bin "fail-mod") due to 2 previous errors