Skip to content

Commit

Permalink
Add module_style lint to restriction
Browse files Browse the repository at this point in the history
Add tests for disallowed_mod in ui-cargo test section

Use correct algorithm to determine if mod.rs is missing

Move to two lints and remove config option

Switch lint names so they read "warn on ..."

Emit the same help info for self_named_mod_file warnings

Bail when both lints are Allow

Reword help message for both module_style lints
  • Loading branch information
DevinR528 committed Aug 22, 2021
1 parent 1fc1aee commit 5a69424
Show file tree
Hide file tree
Showing 23 changed files with 288 additions and 0 deletions.
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
170 changes: 170 additions & 0 deletions clippy_lints/src/module_style.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,170 @@
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) {
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;
};

let mut segments = FxHashSet::default();
let mut mod_folders = FxHashSet::default();
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()));
}
check_mod_file_exists(path, &mut segments, &mut mod_folders);
check_self_named_mod_exists(cx, path, file);
}
_ => {},
}
}

for folder in &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();
},
);
}
}
}
}
}

fn check_mod_file_exists(
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);
}

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;
}

0 comments on commit 5a69424

Please sign in to comment.