diff --git a/CHANGELOG.md b/CHANGELOG.md index 2b645e82b8e..8b3760d1ec3 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 @@ -2906,6 +2907,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 30c684dc744..be97776187f 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -275,6 +275,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; @@ -829,6 +830,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, @@ -1038,6 +1041,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), @@ -2111,6 +2116,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; +}