Auto merge of #7543 - DevinR528:disallowed-mod, r=Manishearth

Add module_style lint to style

changelog: Add new [`module_style`] style lint

This is a configurable (no mod file/mod file) lint that determines if `mod.rs` is used consistently or if `mod.rs` is never used (using the new mod layout).

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

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::<FxHashSet<_>>();
     store.register_late_pass(move || box disallowed_type::DisallowedType::new(&disallowed_types));

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

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]

tests/ui-cargo/module_style/fail_mod/src/bad/inner.rs

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

tests/ui-cargo/module_style/fail_mod/src/bad/inner/stuff.rs

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

tests/ui-cargo/module_style/fail_mod/src/bad/inner/stuff/most.rs

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

tests/ui-cargo/module_style/fail_mod/src/bad/mod.rs

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

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

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
+

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]

tests/ui-cargo/module_style/fail_no_mod/src/bad/mod.rs

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

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

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
+

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]

tests/ui-cargo/module_style/pass_mod/src/bad/mod.rs

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

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

tests/ui-cargo/module_style/pass_mod/src/more/foo.rs

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

tests/ui-cargo/module_style/pass_mod/src/more/inner/mod.rs

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

tests/ui-cargo/module_style/pass_mod/src/more/mod.rs

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

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]

tests/ui-cargo/module_style/pass_no_mod/src/good.rs

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

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