mirror of
https://github.com/rust-lang/rust.git
synced 2026-05-22 18:15:07 +03:00
add new module style restriction lint that checks for use of inline mods
changelog: new lint: [`inline_modules`] Signed-off-by: Zihan <zihanli0822@gmail.com>
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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<SourceFile>,
|
||||
mod_ident: Ident,
|
||||
path_from_working_dir: Option<PathBuf>,
|
||||
contains_external: bool,
|
||||
has_path_attr: bool,
|
||||
mod_file: Arc<SourceFile>,
|
||||
is_cfg_test: bool,
|
||||
}
|
||||
|
||||
#[derive(Default)]
|
||||
pub struct ModStyle {
|
||||
working_dir: Option<PathBuf>,
|
||||
module_stack: Vec<ModState>,
|
||||
regular_mod_stack: Vec<ModState>,
|
||||
inline_mod_stack: Vec<ModState>,
|
||||
}
|
||||
|
||||
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<PathBuf> {
|
||||
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<Item = &'a Ident>,
|
||||
) {
|
||||
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()
|
||||
|
||||
@@ -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
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
@@ -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
|
||||
@@ -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]
|
||||
@@ -0,0 +1,2 @@
|
||||
/// Lint shouldn't fire because parent mod has a path attribute.
|
||||
mod foo {}
|
||||
@@ -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!();
|
||||
@@ -0,0 +1 @@
|
||||
mod foo {}
|
||||
@@ -0,0 +1 @@
|
||||
mod bar {}
|
||||
@@ -0,0 +1 @@
|
||||
mod foo;
|
||||
Reference in New Issue
Block a user