Auto merge of #8832 - Alexendoo:duplicate-mod, r=Manishearth

Add `duplicate_mod` lint

Inspired by #8827, warns if there's a single file that is loaded by more than one `mod` item

```rust,ignore
// lib.rs
mod a;
mod b;
```
```rust,ignore
// a.rs
#[path = "./b.rs"]
mod b;
```

It adds a `canonicalize` call per `mod file;` encountered, which I don't think should be too heavy

Integration tests with common modules, e.g. [`test_utils`](2038084cf2/tests/test_utils) doesn't trigger it as each test is compiled separately, however I couldn't figure out a good way to add a test for that

changelog: Add [`duplicate_mod`] lint

CHANGELOG.md

@@ -3363,6 +3363,7 @@ Released 2018-09-13
 [`drop_copy`]: https://rust-lang.github.io/rust-clippy/master/index.html#drop_copy
 [`drop_non_drop`]: https://rust-lang.github.io/rust-clippy/master/index.html#drop_non_drop
 [`drop_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#drop_ref
+[`duplicate_mod`]: https://rust-lang.github.io/rust-clippy/master/index.html#duplicate_mod
 [`duplicate_underscore_argument`]: https://rust-lang.github.io/rust-clippy/master/index.html#duplicate_underscore_argument
 [`duration_subsec`]: https://rust-lang.github.io/rust-clippy/master/index.html#duration_subsec
 [`else_if_without_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#else_if_without_else

clippy_lints/src/duplicate_mod.rs

@@ -0,0 +1,102 @@
+use clippy_utils::diagnostics::span_lint_and_help;
+use rustc_ast::ast::{Crate, Inline, Item, ItemKind, ModKind};
+use rustc_errors::MultiSpan;
+use rustc_lint::{EarlyContext, EarlyLintPass, LintContext};
+use rustc_session::{declare_tool_lint, impl_lint_pass};
+use rustc_span::{FileName, Span};
+use std::collections::BTreeMap;
+use std::path::PathBuf;
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for files that are included as modules multiple times.
+    ///
+    /// ### Why is this bad?
+    /// Loading a file as a module more than once causes it to be compiled
+    /// multiple times, taking longer and putting duplicate content into the
+    /// module tree.
+    ///
+    /// ### Example
+    /// ```rust,ignore
+    /// // lib.rs
+    /// mod a;
+    /// mod b;
+    /// ```
+    /// ```rust,ignore
+    /// // a.rs
+    /// #[path = "./b.rs"]
+    /// mod b;
+    /// ```
+    ///
+    /// Use instead:
+    ///
+    /// ```rust,ignore
+    /// // lib.rs
+    /// mod a;
+    /// mod b;
+    /// ```
+    /// ```rust,ignore
+    /// // a.rs
+    /// use crate::b;
+    /// ```
+    #[clippy::version = "1.62.0"]
+    pub DUPLICATE_MOD,
+    suspicious,
+    "file loaded as module multiple times"
+}
+
+#[derive(PartialOrd, Ord, PartialEq, Eq)]
+struct Modules {
+    local_path: PathBuf,
+    spans: Vec<Span>,
+}
+
+#[derive(Default)]
+pub struct DuplicateMod {
+    /// map from the canonicalized path to `Modules`, `BTreeMap` to make the
+    /// order deterministic for tests
+    modules: BTreeMap<PathBuf, Modules>,
+}
+
+impl_lint_pass!(DuplicateMod => [DUPLICATE_MOD]);
+
+impl EarlyLintPass for DuplicateMod {
+    fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) {
+        if let ItemKind::Mod(_, ModKind::Loaded(_, Inline::No, mod_spans)) = &item.kind
+            && let FileName::Real(real) = cx.sess().source_map().span_to_filename(mod_spans.inner_span)
+            && let Some(local_path) = real.into_local_path()
+            && let Ok(absolute_path) = local_path.canonicalize()
+        {
+            let modules = self.modules.entry(absolute_path).or_insert(Modules {
+                local_path,
+                spans: Vec::new(),
+            });
+            modules.spans.push(item.span_with_attributes());
+        }
+    }
+
+    fn check_crate_post(&mut self, cx: &EarlyContext<'_>, _: &Crate) {
+        for Modules { local_path, spans } in self.modules.values() {
+            if spans.len() < 2 {
+                continue;
+            }
+
+            let mut multi_span = MultiSpan::from_spans(spans.clone());
+            let (&first, duplicates) = spans.split_first().unwrap();
+
+            multi_span.push_span_label(first, "first loaded here");
+            for &duplicate in duplicates {
+                multi_span.push_span_label(duplicate, "loaded again here");
+            }
+
+            span_lint_and_help(
+                cx,
+                DUPLICATE_MOD,
+                multi_span,
+                &format!("file is loaded as a module multiple times: `{}`", local_path.display()),
+                None,
+                "replace all but one `mod` item with `use` items",
+            );
+        }
+    }
+}

clippy_lints/src/lib.register_all.rs

@@ -60,6 +60,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
     LintId::of(drop_forget_ref::FORGET_NON_DROP),
     LintId::of(drop_forget_ref::FORGET_REF),
     LintId::of(drop_forget_ref::UNDROPPED_MANUALLY_DROPS),
+    LintId::of(duplicate_mod::DUPLICATE_MOD),
     LintId::of(duration_subsec::DURATION_SUBSEC),
     LintId::of(entry::MAP_ENTRY),
     LintId::of(enum_clike::ENUM_CLIKE_UNPORTABLE_VARIANT),

clippy_lints/src/lib.register_lints.rs

@@ -133,6 +133,7 @@ store.register_lints(&[
     drop_forget_ref::FORGET_NON_DROP,
     drop_forget_ref::FORGET_REF,
     drop_forget_ref::UNDROPPED_MANUALLY_DROPS,
+    duplicate_mod::DUPLICATE_MOD,
     duration_subsec::DURATION_SUBSEC,
     else_if_without_else::ELSE_IF_WITHOUT_ELSE,
     empty_drop::EMPTY_DROP,

clippy_lints/src/lib.register_suspicious.rs

@@ -14,6 +14,7 @@ store.register_group(true, "clippy::suspicious", Some("clippy_suspicious"), vec!
     LintId::of(crate_in_macro_def::CRATE_IN_MACRO_DEF),
     LintId::of(drop_forget_ref::DROP_NON_DROP),
     LintId::of(drop_forget_ref::FORGET_NON_DROP),
+    LintId::of(duplicate_mod::DUPLICATE_MOD),
     LintId::of(eval_order_dependence::EVAL_ORDER_DEPENDENCE),
     LintId::of(float_equality_without_abs::FLOAT_EQUALITY_WITHOUT_ABS),
     LintId::of(format_impl::PRINT_IN_FORMAT_IMPL),

clippy_lints/src/lib.rs

@@ -211,6 +211,7 @@ mod doc;
 mod double_comparison;
 mod double_parens;
 mod drop_forget_ref;
+mod duplicate_mod;
 mod duration_subsec;
 mod else_if_without_else;
 mod empty_drop;
@@ -902,6 +903,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(move || Box::new(large_include_file::LargeIncludeFile::new(max_include_file_size)));
     store.register_late_pass(|| Box::new(strings::TrimSplitWhitespace));
     store.register_late_pass(|| Box::new(rc_clone_in_vec_init::RcCloneInVecInit));
+    store.register_early_pass(|| Box::new(duplicate_mod::DuplicateMod::default()));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }
 

clippy_utils/src/diagnostics.rs

@@ -77,7 +77,7 @@ pub fn span_lint<T: LintContext>(cx: &T, lint: &'static Lint, sp: impl Into<Mult
 pub fn span_lint_and_help<'a, T: LintContext>(
     cx: &'a T,
     lint: &'static Lint,
-    span: Span,
+    span: impl Into<MultiSpan>,
     msg: &str,
     help_span: Option<Span>,
     help: &str,

tests/ui-cargo/duplicate_mod/fail/Cargo.toml

@@ -0,0 +1,5 @@
+[package]
+name = "duplicate_mod"
+edition = "2021"
+publish = false
+version = "0.1.0"

tests/ui-cargo/duplicate_mod/fail/src/a.rs

@@ -0,0 +1 @@
+

tests/ui-cargo/duplicate_mod/fail/src/b.rs

@@ -0,0 +1 @@
+

tests/ui-cargo/duplicate_mod/fail/src/c.rs

@@ -0,0 +1 @@
+

tests/ui-cargo/duplicate_mod/fail/src/from_other_module.rs

@@ -0,0 +1 @@
+

tests/ui-cargo/duplicate_mod/fail/src/main.rs

@@ -0,0 +1,16 @@
+mod a;
+
+mod b;
+#[path = "b.rs"]
+mod b2;
+
+mod c;
+#[path = "c.rs"]
+mod c2;
+#[path = "c.rs"]
+mod c3;
+
+mod from_other_module;
+mod other_module;
+
+fn main() {}

tests/ui-cargo/duplicate_mod/fail/src/main.stderr

@@ -0,0 +1,42 @@
+error: file is loaded as a module multiple times: `$DIR/b.rs`
+  --> $DIR/main.rs:3:1
+   |
+LL |   mod b;
+   |   ^^^^^^ first loaded here
+LL | / #[path = "b.rs"]
+LL | | mod b2;
+   | |_______^ loaded again here
+   |
+   = note: `-D clippy::duplicate-mod` implied by `-D warnings`
+   = help: replace all but one `mod` item with `use` items
+
+error: file is loaded as a module multiple times: `$DIR/c.rs`
+  --> $DIR/main.rs:7:1
+   |
+LL |   mod c;
+   |   ^^^^^^ first loaded here
+LL | / #[path = "c.rs"]
+LL | | mod c2;
+   | |_______^ loaded again here
+LL | / #[path = "c.rs"]
+LL | | mod c3;
+   | |_______^ loaded again here
+   |
+   = help: replace all but one `mod` item with `use` items
+
+error: file is loaded as a module multiple times: `$DIR/from_other_module.rs`
+  --> $DIR/main.rs:13:1
+   |
+LL |   mod from_other_module;
+   |   ^^^^^^^^^^^^^^^^^^^^^^ first loaded here
+   |
+  ::: $DIR/other_module/mod.rs:1:1
+   |
+LL | / #[path = "../from_other_module.rs"]
+LL | | mod m;
+   | |______^ loaded again here
+   |
+   = help: replace all but one `mod` item with `use` items
+
+error: aborting due to 3 previous errors
+

tests/ui-cargo/duplicate_mod/fail/src/other_module/mod.rs

@@ -0,0 +1,2 @@
+#[path = "../from_other_module.rs"]
+mod m;