Add module_style lint to restriction

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
This commit is contained in:
Devin Ragotzy 2021-07-30 17:28:44 -04:00
parent 1fc1aee7be
commit 31738b1da3
23 changed files with 296 additions and 0 deletions

View File

@ -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
@ -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

View File

@ -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;
@ -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,
@ -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),
@ -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));

View File

@ -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) {
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();
},
);
}
}

View File

@ -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]

View File

@ -0,0 +1 @@
pub mod stuff;

View File

@ -0,0 +1,3 @@
pub mod most;
pub struct Inner;

View File

@ -0,0 +1 @@
pub struct Snarks;

View File

@ -0,0 +1,3 @@
pub mod inner;
pub struct Thing;

View File

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

View File

@ -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

View File

@ -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]

View File

@ -0,0 +1 @@
pub struct Thing;

View File

@ -0,0 +1,7 @@
#![warn(clippy::mod_module_files)]
mod bad;
fn main() {
let _ = bad::Thing;
}

View File

@ -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

View File

@ -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]

View File

@ -0,0 +1 @@
pub struct Thing;

View File

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

View File

@ -0,0 +1 @@
pub struct Foo;

View File

@ -0,0 +1 @@
pub struct Inner;

View File

@ -0,0 +1,2 @@
pub mod foo;
pub mod inner;

View File

@ -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]

View File

@ -0,0 +1 @@
pub struct Thing;

View File

@ -0,0 +1,7 @@
#![warn(clippy::mod_module_files)]
mod good;
fn main() {
let _ = good::Thing;
}