Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add module_style lint to style #7543

Merged
merged 1 commit into from
Aug 25, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2796,6 +2796,7 @@ Released 2018-09-13
[`missing_safety_doc`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_safety_doc
[`mistyped_literal_suffixes`]: https://rust-lang.github.io/rust-clippy/master/index.html#mistyped_literal_suffixes
[`mixed_case_hex_literals`]: https://rust-lang.github.io/rust-clippy/master/index.html#mixed_case_hex_literals
[`mod_module_files`]: https://rust-lang.github.io/rust-clippy/master/index.html#mod_module_files
[`module_inception`]: https://rust-lang.github.io/rust-clippy/master/index.html#module_inception
[`module_name_repetitions`]: https://rust-lang.github.io/rust-clippy/master/index.html#module_name_repetitions
[`modulo_arithmetic`]: https://rust-lang.github.io/rust-clippy/master/index.html#modulo_arithmetic
Expand Down Expand Up @@ -2904,6 +2905,7 @@ Released 2018-09-13
[`search_is_some`]: https://rust-lang.github.io/rust-clippy/master/index.html#search_is_some
[`self_assignment`]: https://rust-lang.github.io/rust-clippy/master/index.html#self_assignment
[`self_named_constructors`]: https://rust-lang.github.io/rust-clippy/master/index.html#self_named_constructors
[`self_named_module_files`]: https://rust-lang.github.io/rust-clippy/master/index.html#self_named_module_files
[`semicolon_if_nothing_returned`]: https://rust-lang.github.io/rust-clippy/master/index.html#semicolon_if_nothing_returned
[`serde_api_misuse`]: https://rust-lang.github.io/rust-clippy/master/index.html#serde_api_misuse
[`shadow_reuse`]: https://rust-lang.github.io/rust-clippy/master/index.html#shadow_reuse
Expand Down
6 changes: 6 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,7 @@ mod missing_const_for_fn;
mod missing_doc;
mod missing_enforced_import_rename;
mod missing_inline;
mod module_style;
mod modulo_arithmetic;
mod multiple_crate_versions;
mod mut_key;
Expand Down Expand Up @@ -826,6 +827,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
missing_doc::MISSING_DOCS_IN_PRIVATE_ITEMS,
missing_enforced_import_rename::MISSING_ENFORCED_IMPORT_RENAMES,
missing_inline::MISSING_INLINE_IN_PUBLIC_ITEMS,
module_style::MOD_MODULE_FILES,
module_style::SELF_NAMED_MODULE_FILES,
modulo_arithmetic::MODULO_ARITHMETIC,
multiple_crate_versions::MULTIPLE_CRATE_VERSIONS,
mut_key::MUTABLE_KEY_TYPE,
Expand Down Expand Up @@ -1035,6 +1038,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(missing_doc::MISSING_DOCS_IN_PRIVATE_ITEMS),
LintId::of(missing_enforced_import_rename::MISSING_ENFORCED_IMPORT_RENAMES),
LintId::of(missing_inline::MISSING_INLINE_IN_PUBLIC_ITEMS),
LintId::of(module_style::MOD_MODULE_FILES),
LintId::of(module_style::SELF_NAMED_MODULE_FILES),
LintId::of(modulo_arithmetic::MODULO_ARITHMETIC),
LintId::of(panic_in_result_fn::PANIC_IN_RESULT_FN),
LintId::of(panic_unimplemented::PANIC),
Expand Down Expand Up @@ -2107,6 +2112,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|| box manual_map::ManualMap);
store.register_late_pass(move || box if_then_some_else_none::IfThenSomeElseNone::new(msrv));
store.register_early_pass(|| box bool_assert_comparison::BoolAssertComparison);
store.register_early_pass(move || box module_style::ModStyle);
store.register_late_pass(|| box unused_async::UnusedAsync);
let disallowed_types = conf.disallowed_types.iter().cloned().collect::<FxHashSet<_>>();
store.register_late_pass(move || box disallowed_type::DisallowedType::new(&disallowed_types));
Expand Down
178 changes: 178 additions & 0 deletions clippy_lints/src/module_style.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,178 @@
use std::{
ffi::OsString,
path::{Component, Path},
};

use rustc_ast::ast;
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_lint::{EarlyContext, EarlyLintPass, Level, LintContext};
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::{FileName, RealFileName, SourceFile, Span, SyntaxContext};

declare_clippy_lint! {
/// ### What it does
/// Checks that module layout uses only self named module files, bans mod.rs files.
///
/// ### Why is this bad?
/// Having multiple module layout styles in a project can be confusing.
///
/// ### Example
/// ```text
/// src/
/// stuff/
/// stuff_files.rs
/// mod.rs
/// lib.rs
/// ```
/// Use instead:
/// ```text
/// src/
/// stuff/
/// stuff_files.rs
/// stuff.rs
/// lib.rs
/// ```
pub MOD_MODULE_FILES,
restriction,
"checks that module layout is consistent"
}

declare_clippy_lint! {
/// ### What it does
/// Checks that module layout uses only mod.rs files.
///
/// ### Why is this bad?
/// Having multiple module layout styles in a project can be confusing.
///
/// ### Example
/// ```text
/// src/
/// stuff/
/// stuff_files.rs
/// stuff.rs
/// lib.rs
/// ```
/// Use instead:
/// ```text
/// src/
/// stuff/
/// stuff_files.rs
/// mod.rs
/// lib.rs
/// ```

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]);

impl EarlyLintPass for ModStyle {
fn check_crate(&mut self, cx: &EarlyContext<'_>, _: &ast::Crate) {
DevinR528 marked this conversation as resolved.
Show resolved Hide resolved
if cx.builder.lint_level(MOD_MODULE_FILES).0 == Level::Allow
&& cx.builder.lint_level(SELF_NAMED_MODULE_FILES).0 == Level::Allow
{
return;
}

let files = cx.sess.source_map().files();

let trim_to_src = if let RealFileName::LocalPath(p) = &cx.sess.working_dir {
p.to_string_lossy()
} else {
return;
};

// `folder_segments` is all unique folder path segments `path/to/foo.rs` gives
// `[path, to]` but not foo
let mut folder_segments = FxHashSet::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() {
match &file.name {
FileName::Real(RealFileName::LocalPath(lp))
if lp.to_string_lossy().starts_with(trim_to_src.as_ref()) =>
{
let p = lp.to_string_lossy();
let path = Path::new(p.trim_start_matches(trim_to_src.as_ref()));
if let Some(stem) = path.file_stem() {
file_map.insert(stem.to_os_string(), (file, path.to_owned()));
}
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) {
if let Some((file, path)) = file_map.get(folder) {
let mut correct = path.clone();
correct.pop();
correct.push(folder);
correct.push("mod.rs");
cx.struct_span_lint(
SELF_NAMED_MODULE_FILES,
Span::new(file.start_pos, file.start_pos, SyntaxContext::root()),
|build| {
let mut lint =
build.build(&format!("`mod.rs` files are required, found `{}`", path.display()));
lint.help(&format!("move `{}` to `{}`", path.display(), correct.display(),));
lint.emit();
},
);
}
}
}
}
}

/// 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(
path: &Path,
folder_segments: &mut FxHashSet<OsString>,
mod_folders: &mut FxHashSet<OsString>,
) {
let mut comp = path.components().rev().peekable();
let _ = comp.next();
if path.ends_with("mod.rs") {
mod_folders.insert(comp.peek().map(|c| c.as_os_str().to_owned()).unwrap_or_default());
}
let folders = comp
.filter_map(|c| {
if let Component::Normal(s) = c {
Some(s.to_os_string())
} else {
None
}
})
.collect::<Vec<_>>();
folder_segments.extend(folders);
}

/// Checks every path for the presence of `mod.rs` files and emits the lint if found.
fn check_self_named_mod_exists(cx: &EarlyContext<'_>, path: &Path, file: &SourceFile) {
if path.ends_with("mod.rs") {
let mut mod_file = path.to_path_buf();
mod_file.pop();
mod_file.set_extension("rs");

cx.struct_span_lint(
MOD_MODULE_FILES,
Span::new(file.start_pos, file.start_pos, SyntaxContext::root()),
|build| {
let mut lint = build.build(&format!("`mod.rs` files are not allowed, found `{}`", path.display()));
lint.help(&format!("move `{}` to `{}`", path.display(), mod_file.display(),));
lint.emit();
},
);
}
}
8 changes: 8 additions & 0 deletions tests/ui-cargo/module_style/fail_mod/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
[package]
name = "fail"
version = "0.1.0"
edition = "2018"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
1 change: 1 addition & 0 deletions tests/ui-cargo/module_style/fail_mod/src/bad/inner.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
pub mod stuff;
3 changes: 3 additions & 0 deletions tests/ui-cargo/module_style/fail_mod/src/bad/inner/stuff.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
pub mod most;

pub struct Inner;
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
pub struct Snarks;
3 changes: 3 additions & 0 deletions tests/ui-cargo/module_style/fail_mod/src/bad/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
pub mod inner;

pub struct Thing;
9 changes: 9 additions & 0 deletions tests/ui-cargo/module_style/fail_mod/src/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#![warn(clippy::self_named_module_files)]

mod bad;

fn main() {
let _ = bad::Thing;
let _ = bad::inner::stuff::Inner;
let _ = bad::inner::stuff::most::Snarks;
}
19 changes: 19 additions & 0 deletions tests/ui-cargo/module_style/fail_mod/src/main.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
error: `mod.rs` files are required, found `/bad/inner.rs`
--> $DIR/bad/inner.rs:1:1
|
LL | pub mod stuff;
| ^
|
= note: `-D clippy::self-named-module-files` implied by `-D warnings`
= help: move `/bad/inner.rs` to `/bad/inner/mod.rs`

error: `mod.rs` files are required, found `/bad/inner/stuff.rs`
--> $DIR/bad/inner/stuff.rs:1:1
|
LL | pub mod most;
| ^
|
= help: move `/bad/inner/stuff.rs` to `/bad/inner/stuff/mod.rs`

error: aborting due to 2 previous errors

8 changes: 8 additions & 0 deletions tests/ui-cargo/module_style/fail_no_mod/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
[package]
name = "fail"
version = "0.1.0"
edition = "2018"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
1 change: 1 addition & 0 deletions tests/ui-cargo/module_style/fail_no_mod/src/bad/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
pub struct Thing;
7 changes: 7 additions & 0 deletions tests/ui-cargo/module_style/fail_no_mod/src/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#![warn(clippy::mod_module_files)]

mod bad;

fn main() {
let _ = bad::Thing;
}
11 changes: 11 additions & 0 deletions tests/ui-cargo/module_style/fail_no_mod/src/main.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
error: `mod.rs` files are not allowed, found `/bad/mod.rs`
--> $DIR/bad/mod.rs:1:1
|
LL | pub struct Thing;
| ^
|
= note: `-D clippy::mod-module-files` implied by `-D warnings`
= help: move `/bad/mod.rs` to `/bad.rs`

error: aborting due to previous error

8 changes: 8 additions & 0 deletions tests/ui-cargo/module_style/pass_mod/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
[package]
name = "fail"
version = "0.1.0"
edition = "2018"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
1 change: 1 addition & 0 deletions tests/ui-cargo/module_style/pass_mod/src/bad/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
pub struct Thing;
10 changes: 10 additions & 0 deletions tests/ui-cargo/module_style/pass_mod/src/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
#![warn(clippy::self_named_module_files)]

mod bad;
mod more;

fn main() {
let _ = bad::Thing;
let _ = more::foo::Foo;
let _ = more::inner::Inner;
}
1 change: 1 addition & 0 deletions tests/ui-cargo/module_style/pass_mod/src/more/foo.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
pub struct Foo;
1 change: 1 addition & 0 deletions tests/ui-cargo/module_style/pass_mod/src/more/inner/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
pub struct Inner;
2 changes: 2 additions & 0 deletions tests/ui-cargo/module_style/pass_mod/src/more/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
pub mod foo;
pub mod inner;
8 changes: 8 additions & 0 deletions tests/ui-cargo/module_style/pass_no_mod/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
[package]
name = "pass"
version = "0.1.0"
edition = "2018"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
1 change: 1 addition & 0 deletions tests/ui-cargo/module_style/pass_no_mod/src/good.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
pub struct Thing;
7 changes: 7 additions & 0 deletions tests/ui-cargo/module_style/pass_no_mod/src/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#![warn(clippy::mod_module_files)]

mod good;

fn main() {
let _ = good::Thing;
}