From 31738b1da3e4033ff7898ffc869e96d7faba1775 Mon Sep 17 00:00:00 2001 From: Devin Ragotzy Date: Fri, 30 Jul 2021 17:28:44 -0400 Subject: [PATCH] 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 --- CHANGELOG.md | 2 + clippy_lints/src/lib.rs | 6 + clippy_lints/src/module_style.rs | 178 ++++++++++++++++++ .../ui-cargo/module_style/fail_mod/Cargo.toml | 8 + .../module_style/fail_mod/src/bad/inner.rs | 1 + .../fail_mod/src/bad/inner/stuff.rs | 3 + .../fail_mod/src/bad/inner/stuff/most.rs | 1 + .../module_style/fail_mod/src/bad/mod.rs | 3 + .../module_style/fail_mod/src/main.rs | 9 + .../module_style/fail_mod/src/main.stderr | 19 ++ .../module_style/fail_no_mod/Cargo.toml | 8 + .../module_style/fail_no_mod/src/bad/mod.rs | 1 + .../module_style/fail_no_mod/src/main.rs | 7 + .../module_style/fail_no_mod/src/main.stderr | 11 ++ .../ui-cargo/module_style/pass_mod/Cargo.toml | 8 + .../module_style/pass_mod/src/bad/mod.rs | 1 + .../module_style/pass_mod/src/main.rs | 10 + .../module_style/pass_mod/src/more/foo.rs | 1 + .../pass_mod/src/more/inner/mod.rs | 1 + .../module_style/pass_mod/src/more/mod.rs | 2 + .../module_style/pass_no_mod/Cargo.toml | 8 + .../module_style/pass_no_mod/src/good.rs | 1 + .../module_style/pass_no_mod/src/main.rs | 7 + 23 files changed, 296 insertions(+) create mode 100644 clippy_lints/src/module_style.rs create mode 100644 tests/ui-cargo/module_style/fail_mod/Cargo.toml create mode 100644 tests/ui-cargo/module_style/fail_mod/src/bad/inner.rs create mode 100644 tests/ui-cargo/module_style/fail_mod/src/bad/inner/stuff.rs create mode 100644 tests/ui-cargo/module_style/fail_mod/src/bad/inner/stuff/most.rs create mode 100644 tests/ui-cargo/module_style/fail_mod/src/bad/mod.rs create mode 100644 tests/ui-cargo/module_style/fail_mod/src/main.rs create mode 100644 tests/ui-cargo/module_style/fail_mod/src/main.stderr create mode 100644 tests/ui-cargo/module_style/fail_no_mod/Cargo.toml create mode 100644 tests/ui-cargo/module_style/fail_no_mod/src/bad/mod.rs create mode 100644 tests/ui-cargo/module_style/fail_no_mod/src/main.rs create mode 100644 tests/ui-cargo/module_style/fail_no_mod/src/main.stderr create mode 100644 tests/ui-cargo/module_style/pass_mod/Cargo.toml create mode 100644 tests/ui-cargo/module_style/pass_mod/src/bad/mod.rs create mode 100644 tests/ui-cargo/module_style/pass_mod/src/main.rs create mode 100644 tests/ui-cargo/module_style/pass_mod/src/more/foo.rs create mode 100644 tests/ui-cargo/module_style/pass_mod/src/more/inner/mod.rs create mode 100644 tests/ui-cargo/module_style/pass_mod/src/more/mod.rs create mode 100644 tests/ui-cargo/module_style/pass_no_mod/Cargo.toml create mode 100644 tests/ui-cargo/module_style/pass_no_mod/src/good.rs create mode 100644 tests/ui-cargo/module_style/pass_no_mod/src/main.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index b9e67d361c0..4bbc1233902 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 1fadaf4770a..82178701176 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -274,6 +274,7 @@ macro_rules! declare_clippy_lint { 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::>(); store.register_late_pass(move || box disallowed_type::DisallowedType::new(&disallowed_types)); diff --git a/clippy_lints/src/module_style.rs b/clippy_lints/src/module_style.rs new file mode 100644 index 00000000000..73e93cf5a3e --- /dev/null +++ b/clippy_lints/src/module_style.rs @@ -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, + mod_folders: &mut FxHashSet, +) { + 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::>(); + 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(); + }, + ); + } +} diff --git a/tests/ui-cargo/module_style/fail_mod/Cargo.toml b/tests/ui-cargo/module_style/fail_mod/Cargo.toml new file mode 100644 index 00000000000..27b61c09fb4 --- /dev/null +++ b/tests/ui-cargo/module_style/fail_mod/Cargo.toml @@ -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] diff --git a/tests/ui-cargo/module_style/fail_mod/src/bad/inner.rs b/tests/ui-cargo/module_style/fail_mod/src/bad/inner.rs new file mode 100644 index 00000000000..91cd540a28f --- /dev/null +++ b/tests/ui-cargo/module_style/fail_mod/src/bad/inner.rs @@ -0,0 +1 @@ +pub mod stuff; diff --git a/tests/ui-cargo/module_style/fail_mod/src/bad/inner/stuff.rs b/tests/ui-cargo/module_style/fail_mod/src/bad/inner/stuff.rs new file mode 100644 index 00000000000..7713fa9d35c --- /dev/null +++ b/tests/ui-cargo/module_style/fail_mod/src/bad/inner/stuff.rs @@ -0,0 +1,3 @@ +pub mod most; + +pub struct Inner; diff --git a/tests/ui-cargo/module_style/fail_mod/src/bad/inner/stuff/most.rs b/tests/ui-cargo/module_style/fail_mod/src/bad/inner/stuff/most.rs new file mode 100644 index 00000000000..5a5eaf9670f --- /dev/null +++ b/tests/ui-cargo/module_style/fail_mod/src/bad/inner/stuff/most.rs @@ -0,0 +1 @@ +pub struct Snarks; diff --git a/tests/ui-cargo/module_style/fail_mod/src/bad/mod.rs b/tests/ui-cargo/module_style/fail_mod/src/bad/mod.rs new file mode 100644 index 00000000000..a12734db7cb --- /dev/null +++ b/tests/ui-cargo/module_style/fail_mod/src/bad/mod.rs @@ -0,0 +1,3 @@ +pub mod inner; + +pub struct Thing; diff --git a/tests/ui-cargo/module_style/fail_mod/src/main.rs b/tests/ui-cargo/module_style/fail_mod/src/main.rs new file mode 100644 index 00000000000..3e985d4e904 --- /dev/null +++ b/tests/ui-cargo/module_style/fail_mod/src/main.rs @@ -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; +} diff --git a/tests/ui-cargo/module_style/fail_mod/src/main.stderr b/tests/ui-cargo/module_style/fail_mod/src/main.stderr new file mode 100644 index 00000000000..af4c298b310 --- /dev/null +++ b/tests/ui-cargo/module_style/fail_mod/src/main.stderr @@ -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 + diff --git a/tests/ui-cargo/module_style/fail_no_mod/Cargo.toml b/tests/ui-cargo/module_style/fail_no_mod/Cargo.toml new file mode 100644 index 00000000000..27b61c09fb4 --- /dev/null +++ b/tests/ui-cargo/module_style/fail_no_mod/Cargo.toml @@ -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] diff --git a/tests/ui-cargo/module_style/fail_no_mod/src/bad/mod.rs b/tests/ui-cargo/module_style/fail_no_mod/src/bad/mod.rs new file mode 100644 index 00000000000..f19ab10d5fb --- /dev/null +++ b/tests/ui-cargo/module_style/fail_no_mod/src/bad/mod.rs @@ -0,0 +1 @@ +pub struct Thing; diff --git a/tests/ui-cargo/module_style/fail_no_mod/src/main.rs b/tests/ui-cargo/module_style/fail_no_mod/src/main.rs new file mode 100644 index 00000000000..c6e9045b8dc --- /dev/null +++ b/tests/ui-cargo/module_style/fail_no_mod/src/main.rs @@ -0,0 +1,7 @@ +#![warn(clippy::mod_module_files)] + +mod bad; + +fn main() { + let _ = bad::Thing; +} diff --git a/tests/ui-cargo/module_style/fail_no_mod/src/main.stderr b/tests/ui-cargo/module_style/fail_no_mod/src/main.stderr new file mode 100644 index 00000000000..11e15db7fb9 --- /dev/null +++ b/tests/ui-cargo/module_style/fail_no_mod/src/main.stderr @@ -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 + diff --git a/tests/ui-cargo/module_style/pass_mod/Cargo.toml b/tests/ui-cargo/module_style/pass_mod/Cargo.toml new file mode 100644 index 00000000000..27b61c09fb4 --- /dev/null +++ b/tests/ui-cargo/module_style/pass_mod/Cargo.toml @@ -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] diff --git a/tests/ui-cargo/module_style/pass_mod/src/bad/mod.rs b/tests/ui-cargo/module_style/pass_mod/src/bad/mod.rs new file mode 100644 index 00000000000..f19ab10d5fb --- /dev/null +++ b/tests/ui-cargo/module_style/pass_mod/src/bad/mod.rs @@ -0,0 +1 @@ +pub struct Thing; diff --git a/tests/ui-cargo/module_style/pass_mod/src/main.rs b/tests/ui-cargo/module_style/pass_mod/src/main.rs new file mode 100644 index 00000000000..9e08715fc05 --- /dev/null +++ b/tests/ui-cargo/module_style/pass_mod/src/main.rs @@ -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; +} diff --git a/tests/ui-cargo/module_style/pass_mod/src/more/foo.rs b/tests/ui-cargo/module_style/pass_mod/src/more/foo.rs new file mode 100644 index 00000000000..4a835673a59 --- /dev/null +++ b/tests/ui-cargo/module_style/pass_mod/src/more/foo.rs @@ -0,0 +1 @@ +pub struct Foo; diff --git a/tests/ui-cargo/module_style/pass_mod/src/more/inner/mod.rs b/tests/ui-cargo/module_style/pass_mod/src/more/inner/mod.rs new file mode 100644 index 00000000000..aa84f78cc2c --- /dev/null +++ b/tests/ui-cargo/module_style/pass_mod/src/more/inner/mod.rs @@ -0,0 +1 @@ +pub struct Inner; diff --git a/tests/ui-cargo/module_style/pass_mod/src/more/mod.rs b/tests/ui-cargo/module_style/pass_mod/src/more/mod.rs new file mode 100644 index 00000000000..d79569f78ff --- /dev/null +++ b/tests/ui-cargo/module_style/pass_mod/src/more/mod.rs @@ -0,0 +1,2 @@ +pub mod foo; +pub mod inner; diff --git a/tests/ui-cargo/module_style/pass_no_mod/Cargo.toml b/tests/ui-cargo/module_style/pass_no_mod/Cargo.toml new file mode 100644 index 00000000000..3c0896dd2cd --- /dev/null +++ b/tests/ui-cargo/module_style/pass_no_mod/Cargo.toml @@ -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] diff --git a/tests/ui-cargo/module_style/pass_no_mod/src/good.rs b/tests/ui-cargo/module_style/pass_no_mod/src/good.rs new file mode 100644 index 00000000000..f19ab10d5fb --- /dev/null +++ b/tests/ui-cargo/module_style/pass_no_mod/src/good.rs @@ -0,0 +1 @@ +pub struct Thing; diff --git a/tests/ui-cargo/module_style/pass_no_mod/src/main.rs b/tests/ui-cargo/module_style/pass_no_mod/src/main.rs new file mode 100644 index 00000000000..50211a340b9 --- /dev/null +++ b/tests/ui-cargo/module_style/pass_no_mod/src/main.rs @@ -0,0 +1,7 @@ +#![warn(clippy::mod_module_files)] + +mod good; + +fn main() { + let _ = good::Thing; +}