Skip to content

Commit 6412d74

Browse files
New lint: definition_in_module_root (#16965)
*[View all comments](https://triagebot.infra.rust-lang.org/gh-comments/rust-lang/rust-clippy/pull/16965)* Adds a restriction lint that flags item definitions (struct, enum, fn, impl, trait, etc.) in `mod.rs` files, so `mod.rs` is reserved for declarations and re-exports. - `lib.rs` and `main.rs` are not checked - `#[macro_export]` macros and macro-expanded items are skipped - `#[path]` is resolved via the source map changelog: new lint: [`definition_in_module_root`]
2 parents b2ff678 + 518f8ea commit 6412d74

25 files changed

Lines changed: 495 additions & 0 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6840,6 +6840,7 @@ Released 2018-09-13
68406840
[`default_numeric_fallback`]: https://rust-lang.github.io/rust-clippy/master/index.html#default_numeric_fallback
68416841
[`default_trait_access`]: https://rust-lang.github.io/rust-clippy/master/index.html#default_trait_access
68426842
[`default_union_representation`]: https://rust-lang.github.io/rust-clippy/master/index.html#default_union_representation
6843+
[`definition_in_module_root`]: https://rust-lang.github.io/rust-clippy/master/index.html#definition_in_module_root
68436844
[`deprecated_cfg_attr`]: https://rust-lang.github.io/rust-clippy/master/index.html#deprecated_cfg_attr
68446845
[`deprecated_clippy_cfg_attr`]: https://rust-lang.github.io/rust-clippy/master/index.html#deprecated_clippy_cfg_attr
68456846
[`deprecated_semver`]: https://rust-lang.github.io/rust-clippy/master/index.html#deprecated_semver

clippy_lints/src/declared_lints.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
9797
crate::default_instead_of_iter_empty::DEFAULT_INSTEAD_OF_ITER_EMPTY_INFO,
9898
crate::default_numeric_fallback::DEFAULT_NUMERIC_FALLBACK_INFO,
9999
crate::default_union_representation::DEFAULT_UNION_REPRESENTATION_INFO,
100+
crate::definition_in_module_root::DEFINITION_IN_MODULE_ROOT_INFO,
100101
crate::dereference::EXPLICIT_AUTO_DEREF_INFO,
101102
crate::dereference::EXPLICIT_DEREF_METHODS_INFO,
102103
crate::dereference::NEEDLESS_BORROW_INFO,
Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,161 @@
1+
use clippy_utils::diagnostics::span_lint_and_help;
2+
use rustc_ast::ast::{self, Inline, ItemKind, ModKind};
3+
use rustc_lint::{EarlyContext, EarlyLintPass, LintContext};
4+
use rustc_session::impl_lint_pass;
5+
use rustc_span::{FileName, SourceFile, sym};
6+
7+
declare_clippy_lint! {
8+
/// ### What it does
9+
/// Checks for definitions (structs, functions, traits, etc.) in `mod.rs`
10+
/// files. `lib.rs` and `main.rs` are not checked.
11+
///
12+
/// ### Why restrict this?
13+
/// `mod.rs` is well-suited to acting as a table of contents — listing
14+
/// submodules and re-exports while leaving definitions to named files.
15+
/// Naming each file after its primary definition keeps filenames
16+
/// descriptive and unique, makes editor tabs and search results easier
17+
/// to scan, and makes the filesystem tree mirror the module tree, so
18+
/// the file layout is uniquely determined by the module structure.
19+
///
20+
/// ### Example
21+
/// ```ignore
22+
/// // stuff/mod.rs
23+
/// mod bar;
24+
/// pub struct Foo { /* ... */ }
25+
/// impl Foo { /* ... */ }
26+
/// ```
27+
/// Use instead:
28+
/// ```ignore
29+
/// // stuff/mod.rs
30+
/// mod bar;
31+
/// mod foo;
32+
/// pub use foo::Foo;
33+
///
34+
/// // stuff/foo.rs
35+
/// pub struct Foo { /* ... */ }
36+
/// impl Foo { /* ... */ }
37+
/// ```
38+
///
39+
/// ### Notes
40+
/// This lint is most useful alongside `self_named_module_files`, which
41+
/// requires `mod.rs` files; together they constrain `mod.rs` to
42+
/// declarations only. Under `mod_module_files` (which forbids `mod.rs`
43+
/// entirely) this lint has nothing to fire on.
44+
///
45+
/// If a definition's name matches its parent module's name, moving it
46+
/// produces `foo/foo.rs`, which `module_inception` flags — projects
47+
/// in that situation typically also `allow(module_inception)`.
48+
#[clippy::version = "1.99.0"]
49+
pub DEFINITION_IN_MODULE_ROOT,
50+
restriction,
51+
"definitions in `mod.rs` should be in named files"
52+
}
53+
54+
impl_lint_pass!(DefinitionInModuleRoot => [DEFINITION_IN_MODULE_ROOT]);
55+
56+
#[derive(Default)]
57+
pub struct DefinitionInModuleRoot {
58+
/// Stack tracking whether items at the current nesting level are in a
59+
/// `mod.rs` file. When the stack is empty, we are at crate root depth
60+
/// (not linted).
61+
module_stack: Vec<bool>,
62+
}
63+
64+
impl EarlyLintPass for DefinitionInModuleRoot {
65+
fn check_item(&mut self, cx: &EarlyContext<'_>, item: &ast::Item) {
66+
// Handle module items: push state for their children.
67+
match &item.kind {
68+
ItemKind::Mod(.., ModKind::Loaded(_, Inline::No { .. }, mod_spans, ..)) => {
69+
let file = cx.sess().source_map().lookup_source_file(mod_spans.inner_span.lo());
70+
self.module_stack.push(is_mod_rs(&file));
71+
return;
72+
},
73+
ItemKind::Mod(..) => {
74+
// Inline module or unloaded — children are not in a root file.
75+
self.module_stack.push(false);
76+
return;
77+
},
78+
_ => {},
79+
}
80+
81+
// Skip items from macro expansion.
82+
if item.span.from_expansion() {
83+
return;
84+
}
85+
86+
// Only lint inside mod.rs files (not at crate root depth).
87+
if !self.module_stack.last().copied().unwrap_or(false) {
88+
return;
89+
}
90+
91+
let Some(kind) = definition_kind(item) else {
92+
return;
93+
};
94+
95+
let help = if let Some(ident) = item.kind.ident() {
96+
format!("move {kind} `{ident}` to a dedicated file")
97+
} else {
98+
format!("move the {kind} to a dedicated file")
99+
};
100+
101+
span_lint_and_help(
102+
cx,
103+
DEFINITION_IN_MODULE_ROOT,
104+
item.span,
105+
"definition in module root file",
106+
None,
107+
help,
108+
);
109+
}
110+
111+
fn check_item_post(&mut self, _: &EarlyContext<'_>, item: &ast::Item) {
112+
if matches!(item.kind, ItemKind::Mod(..)) {
113+
self.module_stack.pop();
114+
}
115+
}
116+
}
117+
118+
/// Returns a human-readable kind string for flagged items, or `None` for
119+
/// items that are allowed in root files (modules, imports, re-exports,
120+
/// `#[macro_export]` macros).
121+
fn definition_kind(item: &ast::Item) -> Option<&'static str> {
122+
match &item.kind {
123+
i @ (ItemKind::Struct(..)
124+
| ItemKind::Enum(..)
125+
| ItemKind::Union(..)
126+
| ItemKind::Fn(..)
127+
| ItemKind::Const(..)
128+
| ItemKind::Static(..)
129+
| ItemKind::Impl(..)
130+
| ItemKind::Trait(..)
131+
| ItemKind::TraitAlias(..)
132+
| ItemKind::TyAlias(..)
133+
| ItemKind::ForeignMod(..)) => Some(i.descr()),
134+
i @ ItemKind::MacroDef(..) if !has_macro_export(item) => Some(i.descr()),
135+
ItemKind::ExternCrate(..)
136+
| ItemKind::Use(..)
137+
| ItemKind::ConstBlock(..)
138+
| ItemKind::Mod(..)
139+
| ItemKind::GlobalAsm(..)
140+
| ItemKind::MacCall(..)
141+
| ItemKind::MacroDef(..)
142+
| ItemKind::Delegation(..)
143+
| ItemKind::DelegationMac(..) => None,
144+
}
145+
}
146+
147+
/// Check if an item has `#[macro_export]`.
148+
fn has_macro_export(item: &ast::Item) -> bool {
149+
item.attrs.iter().any(|attr| attr.has_name(sym::macro_export))
150+
}
151+
152+
/// Check if a source file is `mod.rs`.
153+
fn is_mod_rs(file: &SourceFile) -> bool {
154+
if let FileName::Real(name) = &file.name {
155+
name.local_path()
156+
.and_then(|p| p.file_name())
157+
.is_some_and(|n| n == "mod.rs")
158+
} else {
159+
false
160+
}
161+
}

clippy_lints/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ mod default_constructed_unit_structs;
103103
mod default_instead_of_iter_empty;
104104
mod default_numeric_fallback;
105105
mod default_union_representation;
106+
mod definition_in_module_root;
106107
mod dereference;
107108
mod derivable_impls;
108109
mod derive;
@@ -541,6 +542,7 @@ rustc_lint::early_lint_methods!(
541542
CfgNotTest: cfg_not_test::CfgNotTest = cfg_not_test::CfgNotTest,
542543
EmptyLineAfter: empty_line_after::EmptyLineAfter = empty_line_after::EmptyLineAfter::new(),
543544
InlineTraitBounds: inline_trait_bounds::InlineTraitBounds = inline_trait_bounds::InlineTraitBounds::default(),
545+
DefinitionInModuleRoot: definition_in_module_root::DefinitionInModuleRoot = definition_in_module_root::DefinitionInModuleRoot::default(),
544546
// add early passes here, used by `cargo dev new_lint`
545547
]]
546548
);
Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
error: definition in module root file
2+
--> src/edge_cases/mod.rs:13:1
3+
|
4+
13 | pub struct AboveMod;
5+
| ^^^^^^^^^^^^^^^^^^^^
6+
|
7+
= help: move struct `AboveMod` to a dedicated file
8+
= note: `-D clippy::definition-in-module-root` implied by `-D warnings`
9+
= help: to override `-D warnings` add `#[allow(clippy::definition_in_module_root)]`
10+
11+
error: definition in module root file
12+
--> src/edge_cases/mod.rs:20:1
13+
|
14+
20 | pub struct Foo;
15+
| ^^^^^^^^^^^^^^^
16+
|
17+
= help: move struct `Foo` to a dedicated file
18+
19+
error: definition in module root file
20+
--> src/edge_cases/mod.rs:23:1
21+
|
22+
23 | pub struct Generic<T>(T);
23+
| ^^^^^^^^^^^^^^^^^^^^^^^^^
24+
|
25+
= help: move struct `Generic` to a dedicated file
26+
27+
error: definition in module root file
28+
--> src/edge_cases/mod.rs:24:1
29+
|
30+
24 | / impl<T> Generic<T> {
31+
25 | | pub fn new(t: T) -> Self {
32+
26 | | Self(t)
33+
27 | | }
34+
28 | | }
35+
| |_^
36+
|
37+
= help: move the implementation to a dedicated file
38+
39+
error: definition in module root file
40+
--> src/edge_cases/mod.rs:31:1
41+
|
42+
31 | pub async fn frob() {}
43+
| ^^^^^^^^^^^^^^^^^^^^^^
44+
|
45+
= help: move function `frob` to a dedicated file
46+
47+
error: definition in module root file
48+
--> src/edge_cases/mod.rs:34:1
49+
|
50+
34 | / pub const fn cnst() -> u32 {
51+
35 | | 0
52+
36 | | }
53+
| |_^
54+
|
55+
= help: move function `cnst` to a dedicated file
56+
57+
error: definition in module root file
58+
--> src/edge_cases/mod.rs:39:1
59+
|
60+
39 | pub static FOO: u32 = 1;
61+
| ^^^^^^^^^^^^^^^^^^^^^^^^
62+
|
63+
= help: move static item `FOO` to a dedicated file
64+
65+
error: definition in module root file
66+
--> src/edge_cases/mod.rs:43:1
67+
|
68+
43 | / macro_rules! local_macro {
69+
44 | | () => {};
70+
45 | | }
71+
| |_^
72+
|
73+
= help: move macro definition `local_macro` to a dedicated file
74+
75+
error: definition in module root file
76+
--> src/edge_cases/mod.rs:48:1
77+
|
78+
48 | / pub trait WithDefault {
79+
49 | | fn default_method(&self) -> u32 {
80+
50 | | 42
81+
51 | | }
82+
52 | | }
83+
| |_^
84+
|
85+
= help: move trait `WithDefault` to a dedicated file
86+
87+
error: could not compile `fail-edge-cases` (lib) due to 9 previous errors
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
[package]
2+
name = "fail-edge-cases"
3+
version = "0.1.0"
4+
edition = "2021"
5+
publish = false
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
#![warn(clippy::definition_in_module_root)]
2+
3+
// Case g: extern crate is a declaration → no fire.
4+
extern crate alloc;
5+
6+
// Case b: re-export of named item from sibling → no fire.
7+
pub use super::other::Bar;
8+
9+
// Case c: glob re-export → no fire.
10+
pub use super::other::*;
11+
12+
// Case j (top half): item right above `mod foo;` → fires.
13+
pub struct AboveMod;
14+
15+
// Case j: nested file module declaration → no fire (declaration only).
16+
mod sub;
17+
18+
// Case a: derive — struct fires once, derived impls do NOT fire (from_expansion).
19+
#[derive(Clone, Debug)]
20+
pub struct Foo;
21+
22+
// Case d: generics — struct fires + impl block fires.
23+
pub struct Generic<T>(T);
24+
impl<T> Generic<T> {
25+
pub fn new(t: T) -> Self {
26+
Self(t)
27+
}
28+
}
29+
30+
// Case e: async fn fires as 'function'.
31+
pub async fn frob() {}
32+
33+
// Case f: const fn fires as 'function'.
34+
pub const fn cnst() -> u32 {
35+
0
36+
}
37+
38+
// Case h: static fires as 'static'.
39+
pub static FOO: u32 = 1;
40+
41+
// Case k: non-exported macro fires as 'macro'.
42+
#[allow(unused_macros)]
43+
macro_rules! local_macro {
44+
() => {};
45+
}
46+
47+
// Case l: trait with default method body fires once (not its inner items).
48+
pub trait WithDefault {
49+
fn default_method(&self) -> u32 {
50+
42
51+
}
52+
}
53+
54+
// Case i: inline module — children should NOT fire.
55+
#[cfg(test)]
56+
mod tests {
57+
pub struct InsideInline;
58+
pub fn t() {}
59+
}
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
// Sibling file referenced by `mod sub;` in edge_cases/mod.rs.
2+
// Items here live in a non-mod.rs file → no fire even if they are definitions.
3+
#[allow(dead_code)]
4+
pub struct InSub;
5+
#[allow(dead_code)]
6+
pub fn in_sub() {}
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
#![warn(clippy::definition_in_module_root)]
2+
3+
pub mod edge_cases;
4+
pub mod other;
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
// Sibling module with items that the edge_cases mod.rs re-exports.
2+
pub struct Bar;
3+
pub fn helper() {}

0 commit comments

Comments
 (0)