From 6a16a8fe5bf586d617e237be1a0fa971712759fe Mon Sep 17 00:00:00 2001 From: Zihan Date: Mon, 30 Mar 2026 16:54:31 +0800 Subject: [PATCH] add new module style restriction lint that checks for use of inline mods changelog: new lint: [`inline_modules`] Signed-off-by: Zihan --- CHANGELOG.md | 1 + clippy_lints/src/declared_lints.rs | 1 + clippy_lints/src/module_style.rs | 165 +++++++++++++++--- clippy_utils/src/ast_utils/mod.rs | 17 +- .../module_style/inline_mod/Cargo.stderr | 71 ++++++++ .../module_style/inline_mod/Cargo.toml | 9 + .../module_style/inline_mod/src/foo.rs | 2 + .../module_style/inline_mod/src/lib.rs | 34 ++++ .../module_style/inline_mod/src/other.rs | 1 + .../module_style/inline_mod/src/qux/foo.rs | 1 + .../module_style/inline_mod/src/qux/mod.rs | 1 + 11 files changed, 277 insertions(+), 26 deletions(-) create mode 100644 tests/ui-cargo/module_style/inline_mod/Cargo.stderr create mode 100644 tests/ui-cargo/module_style/inline_mod/Cargo.toml create mode 100644 tests/ui-cargo/module_style/inline_mod/src/foo.rs create mode 100644 tests/ui-cargo/module_style/inline_mod/src/lib.rs create mode 100644 tests/ui-cargo/module_style/inline_mod/src/other.rs create mode 100644 tests/ui-cargo/module_style/inline_mod/src/qux/foo.rs create mode 100644 tests/ui-cargo/module_style/inline_mod/src/qux/mod.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 45bfb65b2e67..929ca2ad4095 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6692,6 +6692,7 @@ Released 2018-09-13 [`inline_asm_x86_att_syntax`]: https://rust-lang.github.io/rust-clippy/master/index.html#inline_asm_x86_att_syntax [`inline_asm_x86_intel_syntax`]: https://rust-lang.github.io/rust-clippy/master/index.html#inline_asm_x86_intel_syntax [`inline_fn_without_body`]: https://rust-lang.github.io/rust-clippy/master/index.html#inline_fn_without_body +[`inline_modules`]: https://rust-lang.github.io/rust-clippy/master/index.html#inline_modules [`inspect_for_each`]: https://rust-lang.github.io/rust-clippy/master/index.html#inspect_for_each [`int_plus_one`]: https://rust-lang.github.io/rust-clippy/master/index.html#int_plus_one [`integer_arithmetic`]: https://rust-lang.github.io/rust-clippy/master/index.html#integer_arithmetic diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 441b907eaf2f..b3e8951b76e8 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -534,6 +534,7 @@ crate::missing_trait_methods::MISSING_TRAIT_METHODS_INFO, crate::mixed_read_write_in_expression::DIVERGING_SUB_EXPRESSION_INFO, crate::mixed_read_write_in_expression::MIXED_READ_WRITE_IN_EXPRESSION_INFO, + crate::module_style::INLINE_MODULES_INFO, crate::module_style::MOD_MODULE_FILES_INFO, crate::module_style::SELF_NAMED_MODULE_FILES_INFO, crate::multi_assignments::MULTI_ASSIGNMENTS_INFO, diff --git a/clippy_lints/src/module_style.rs b/clippy_lints/src/module_style.rs index befd50a5c85f..500ee2864e46 100644 --- a/clippy_lints/src/module_style.rs +++ b/clippy_lints/src/module_style.rs @@ -1,12 +1,46 @@ +use clippy_utils::ast_utils::is_cfg_test; use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::source::snippet; 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, sym}; +use rustc_span::{FileName, Ident, SourceFile, Span, SyntaxContext, sym}; use std::path::{Component, Path, PathBuf}; use std::sync::Arc; +declare_clippy_lint! { + /// ### What it does + /// Checks that module layout does not use inline modules. + /// Inline test modules (anything annotated with `#[cfg(test)]`) are not linted. + /// + /// ### Why restrict this? + /// Having multiple module layout styles in a project can be confusing. + /// + /// ### Known problems + /// The lint currently doesn't lint inline modules whose parent module is annotated + /// with the `#[path]` attribute. + /// + /// ### Example + /// ```ignore + /// // in `src/lib.rs` + /// mod foo { + /// /* module contents */ + /// } + /// ``` + /// Use instead: + /// ```ignore + /// // in `src/lib.rs` + /// mod foo; + /// // in `src/foo.rs` (or `src/foo/mod.rs`) + /// /* module contents */ + /// ``` + #[clippy::version = "1.96.0"] + pub INLINE_MODULES, + restriction, + "checks that module layout does not use inline modules" +} + declare_clippy_lint! { /// ### What it does /// Checks that module layout uses only self named module files; bans `mod.rs` files. @@ -65,18 +99,36 @@ "checks that module layout is consistent" } -impl_lint_pass!(ModStyle => [MOD_MODULE_FILES, SELF_NAMED_MODULE_FILES]); +impl_lint_pass!(ModStyle => [ + INLINE_MODULES, + MOD_MODULE_FILES, + SELF_NAMED_MODULE_FILES, +]); pub struct ModState { + mod_file: Arc, + mod_ident: Ident, + path_from_working_dir: Option, contains_external: bool, has_path_attr: bool, - mod_file: Arc, + is_cfg_test: bool, } #[derive(Default)] pub struct ModStyle { working_dir: Option, - module_stack: Vec, + regular_mod_stack: Vec, + inline_mod_stack: Vec, +} + +impl ModStyle { + fn inside_cfg_test_inline_mod(&self) -> bool { + self.inline_mod_stack.last().is_some_and(|last| last.is_cfg_test) + } + + fn get_relative_path_from_working_dir(&self, file: &SourceFile) -> Option { + try_trim_file_path_prefix(file, self.working_dir.as_ref()?).map(Path::to_path_buf) + } } impl EarlyLintPass for ModStyle { @@ -87,45 +139,83 @@ fn check_crate(&mut self, cx: &EarlyContext<'_>, _: &ast::Crate) { 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 + && cx.builder.lint_level(INLINE_MODULES).level == Level::Allow { return; } - if let ItemKind::Mod(.., ModKind::Loaded(_, Inline::No { .. }, mod_spans, ..)) = &item.kind { + if let ItemKind::Mod(_, mod_ident, ModKind::Loaded(_, inline_or_not, 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 { + let path_from_working_dir = self.get_relative_path_from_working_dir(&mod_file); + let current = ModState { + mod_file, + mod_ident: *mod_ident, + path_from_working_dir, contains_external: false, has_path_attr, - mod_file, - }); + is_cfg_test: self.inside_cfg_test_inline_mod() || is_cfg_test(item), + }; + match inline_or_not { + Inline::Yes => { + if !current.is_cfg_test + && !item.span.from_expansion() + && self.regular_mod_stack.last().is_none_or(|last| !last.has_path_attr) + && let Some(path) = ¤t.path_from_working_dir + { + let opt_extra_mod_dir = self.regular_mod_stack.last().and_then(|last| { + if last.path_from_working_dir.as_ref()?.ends_with("mod.rs") { + None + } else { + Some(&last.mod_ident) + } + }); + check_inline_module( + cx, + path, + *mod_ident, + item.span, + opt_extra_mod_dir + .into_iter() + .chain(self.inline_mod_stack.iter().map(|state| &state.mod_ident)), + ); + } + self.inline_mod_stack.push(current); + }, + Inline::No { .. } => { + if !has_path_attr && let Some(last) = self.regular_mod_stack.last_mut() { + last.contains_external = true; + } + self.regular_mod_stack.push(current); + }, + } } } 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 + && cx.builder.lint_level(INLINE_MODULES).level == Level::Allow { return; } - 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(¤t.mod_file, src)) - else { - return; - }; - if current.contains_external { - check_self_named_module(cx, path, ¤t.mod_file); + if let ItemKind::Mod(.., ModKind::Loaded(_, inline_or_not, ..)) = &item.kind { + match inline_or_not { + Inline::Yes => { + self.inline_mod_stack.pop(); + }, + Inline::No { .. } => { + if let Some(current) = self.regular_mod_stack.pop() + && let Some(path) = ¤t.path_from_working_dir + && !current.has_path_attr + { + if current.contains_external { + check_self_named_module(cx, path, ¤t.mod_file); + } + check_mod_module(cx, path, ¤t.mod_file); + } + }, } - check_mod_module(cx, path, ¤t.mod_file); } } } @@ -173,6 +263,31 @@ fn check_mod_module(cx: &EarlyContext<'_>, path: &Path, file: &SourceFile) { } } +fn check_inline_module<'a>( + cx: &EarlyContext<'_>, + path: &Path, + mod_ident: Ident, + mod_span: Span, + ancestor_mods: impl Iterator, +) { + let Some(parent) = path.parent() else { return }; + + span_lint_and_then(cx, INLINE_MODULES, mod_span, "inline module found", |diag| { + let mut mod_folder = parent.to_path_buf(); + mod_folder.extend(ancestor_mods.map(Ident::as_str)); + let mod_name = mod_ident.as_str(); + + let mod_file = mod_folder.join(mod_name).join("mod.rs"); + let self_named_mod_file = mod_folder.join(format!("{mod_name}.rs")); + let outlined_mod = snippet(cx, mod_span.with_hi(mod_ident.span.hi()), ""); + diag.help(format!( + "move the contents of the module to `{}` or `{}`, and replace this with `{outlined_mod};`", + self_named_mod_file.display(), + mod_file.display(), + )); + }); +} + 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() diff --git a/clippy_utils/src/ast_utils/mod.rs b/clippy_utils/src/ast_utils/mod.rs index 50cfb0ed89de..af95330b0ddd 100644 --- a/clippy_utils/src/ast_utils/mod.rs +++ b/clippy_utils/src/ast_utils/mod.rs @@ -5,7 +5,8 @@ #![allow(clippy::wildcard_imports, clippy::enum_glob_use)] use crate::{both, over}; -use rustc_ast::{self as ast, *}; +use rustc_ast::{self as ast, HasAttrs, *}; +use rustc_span::sym; use rustc_span::symbol::Ident; use std::mem; @@ -1040,3 +1041,17 @@ pub fn eq_delim_args(l: &DelimArgs, r: &DelimArgs) -> bool { && l.tokens.len() == r.tokens.len() && l.tokens.iter().zip(r.tokens.iter()).all(|(a, b)| a.eq_unspanned(b)) } + +/// Checks whether `#[cfg(test)]` is directly applied to `item`. +pub fn is_cfg_test(item: &impl HasAttrs) -> bool { + item.attrs().iter().any(|attr| { + if attr.has_name(sym::cfg) + && let Some(item_list) = attr.meta_item_list() + && item_list.iter().any(|item| item.has_name(sym::test)) + { + true + } else { + false + } + }) +} diff --git a/tests/ui-cargo/module_style/inline_mod/Cargo.stderr b/tests/ui-cargo/module_style/inline_mod/Cargo.stderr new file mode 100644 index 000000000000..e3f18e37db9a --- /dev/null +++ b/tests/ui-cargo/module_style/inline_mod/Cargo.stderr @@ -0,0 +1,71 @@ +error: inline module found + --> src/other.rs:1:1 + | +1 | mod foo {} + | ^^^^^^^^^^ + | + = help: move the contents of the module to `src/other/foo.rs` or `src/other/foo/mod.rs`, and replace this with `mod foo;` + = note: `-D clippy::inline-modules` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::inline_modules)]` + +error: inline module found + --> src/qux/foo.rs:1:1 + | +1 | mod bar {} + | ^^^^^^^^^^ + | + = help: move the contents of the module to `src/qux/foo/bar.rs` or `src/qux/foo/bar/mod.rs`, and replace this with `mod bar;` + +error: inline module found + --> src/lib.rs:9:1 + | + 9 | / pub mod test_nested_inline_mods { +10 | | mod bar { +11 | | mod baz {} +12 | | } +13 | | } + | |_^ + | + = help: move the contents of the module to `src/test_nested_inline_mods.rs` or `src/test_nested_inline_mods/mod.rs`, and replace this with `pub mod test_nested_inline_mods;` + +error: inline module found + --> src/lib.rs:10:5 + | +10 | / mod bar { +11 | | mod baz {} +12 | | } + | |_____^ + | + = help: move the contents of the module to `src/test_nested_inline_mods/bar.rs` or `src/test_nested_inline_mods/bar/mod.rs`, and replace this with `mod bar;` + +error: inline module found + --> src/lib.rs:11:9 + | +11 | mod baz {} + | ^^^^^^^^^^ + | + = help: move the contents of the module to `src/test_nested_inline_mods/bar/baz.rs` or `src/test_nested_inline_mods/bar/baz/mod.rs`, and replace this with `mod baz;` + +error: inline module found + --> src/lib.rs:20:1 + | +20 | / mod partially_escaped_test_mod { +21 | | #[cfg(test)] +22 | | mod tests { +23 | | mod bar {} +24 | | } +25 | | mod baz {} +26 | | } + | |_^ + | + = help: move the contents of the module to `src/partially_escaped_test_mod.rs` or `src/partially_escaped_test_mod/mod.rs`, and replace this with `mod partially_escaped_test_mod;` + +error: inline module found + --> src/lib.rs:25:5 + | +25 | mod baz {} + | ^^^^^^^^^^ + | + = help: move the contents of the module to `src/partially_escaped_test_mod/baz.rs` or `src/partially_escaped_test_mod/baz/mod.rs`, and replace this with `mod baz;` + +error: could not compile `inline-mod` (lib) due to 7 previous errors diff --git a/tests/ui-cargo/module_style/inline_mod/Cargo.toml b/tests/ui-cargo/module_style/inline_mod/Cargo.toml new file mode 100644 index 000000000000..4108d8aaa5c0 --- /dev/null +++ b/tests/ui-cargo/module_style/inline_mod/Cargo.toml @@ -0,0 +1,9 @@ +[package] +name = "inline-mod" +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] diff --git a/tests/ui-cargo/module_style/inline_mod/src/foo.rs b/tests/ui-cargo/module_style/inline_mod/src/foo.rs new file mode 100644 index 000000000000..9e19e6e44fdd --- /dev/null +++ b/tests/ui-cargo/module_style/inline_mod/src/foo.rs @@ -0,0 +1,2 @@ +/// Lint shouldn't fire because parent mod has a path attribute. +mod foo {} diff --git a/tests/ui-cargo/module_style/inline_mod/src/lib.rs b/tests/ui-cargo/module_style/inline_mod/src/lib.rs new file mode 100644 index 000000000000..df5816e251dc --- /dev/null +++ b/tests/ui-cargo/module_style/inline_mod/src/lib.rs @@ -0,0 +1,34 @@ +#![warn(clippy::inline_modules)] + +pub mod other; +#[path = "qux/mod.rs"] +pub mod something; +#[path = "foo.rs"] +pub mod stuff; + +pub mod test_nested_inline_mods { + mod bar { + mod baz {} + } +} + +#[cfg(test)] +mod escaped_test_mod { + mod bar {} +} + +mod partially_escaped_test_mod { + #[cfg(test)] + mod tests { + mod bar {} + } + mod baz {} +} + +macro_rules! inline_mod_from_expansion { + () => { + mod _foo {} + }; +} + +inline_mod_from_expansion!(); diff --git a/tests/ui-cargo/module_style/inline_mod/src/other.rs b/tests/ui-cargo/module_style/inline_mod/src/other.rs new file mode 100644 index 000000000000..e5736aedfdc7 --- /dev/null +++ b/tests/ui-cargo/module_style/inline_mod/src/other.rs @@ -0,0 +1 @@ +mod foo {} diff --git a/tests/ui-cargo/module_style/inline_mod/src/qux/foo.rs b/tests/ui-cargo/module_style/inline_mod/src/qux/foo.rs new file mode 100644 index 000000000000..e4cdd207c663 --- /dev/null +++ b/tests/ui-cargo/module_style/inline_mod/src/qux/foo.rs @@ -0,0 +1 @@ +mod bar {} diff --git a/tests/ui-cargo/module_style/inline_mod/src/qux/mod.rs b/tests/ui-cargo/module_style/inline_mod/src/qux/mod.rs new file mode 100644 index 000000000000..f4ad3bff5c97 --- /dev/null +++ b/tests/ui-cargo/module_style/inline_mod/src/qux/mod.rs @@ -0,0 +1 @@ +mod foo;