From 1f79a442e561521d0efae15da3da8acf17b580c2 Mon Sep 17 00:00:00 2001 From: Alex Macleod Date: Sat, 14 May 2022 19:35:16 +0100 Subject: [PATCH] Add `duplicate_mod` lint --- CHANGELOG.md | 1 + clippy_lints/src/duplicate_mod.rs | 102 ++++++++++++++++++ clippy_lints/src/lib.register_all.rs | 1 + clippy_lints/src/lib.register_lints.rs | 1 + clippy_lints/src/lib.register_suspicious.rs | 1 + clippy_lints/src/lib.rs | 2 + clippy_utils/src/diagnostics.rs | 2 +- tests/ui-cargo/duplicate_mod/fail/Cargo.toml | 5 + tests/ui-cargo/duplicate_mod/fail/src/a.rs | 1 + tests/ui-cargo/duplicate_mod/fail/src/b.rs | 1 + tests/ui-cargo/duplicate_mod/fail/src/c.rs | 1 + .../fail/src/from_other_module.rs | 1 + tests/ui-cargo/duplicate_mod/fail/src/main.rs | 16 +++ .../duplicate_mod/fail/src/main.stderr | 42 ++++++++ .../fail/src/other_module/mod.rs | 2 + 15 files changed, 178 insertions(+), 1 deletion(-) create mode 100644 clippy_lints/src/duplicate_mod.rs create mode 100644 tests/ui-cargo/duplicate_mod/fail/Cargo.toml create mode 100644 tests/ui-cargo/duplicate_mod/fail/src/a.rs create mode 100644 tests/ui-cargo/duplicate_mod/fail/src/b.rs create mode 100644 tests/ui-cargo/duplicate_mod/fail/src/c.rs create mode 100644 tests/ui-cargo/duplicate_mod/fail/src/from_other_module.rs create mode 100644 tests/ui-cargo/duplicate_mod/fail/src/main.rs create mode 100644 tests/ui-cargo/duplicate_mod/fail/src/main.stderr create mode 100644 tests/ui-cargo/duplicate_mod/fail/src/other_module/mod.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 47aaae9c198..e5988a19c14 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/clippy_lints/src/duplicate_mod.rs b/clippy_lints/src/duplicate_mod.rs new file mode 100644 index 00000000000..c6c7b959d4f --- /dev/null +++ b/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, +} + +#[derive(Default)] +pub struct DuplicateMod { + /// map from the canonicalized path to `Modules`, `BTreeMap` to make the + /// order deterministic for tests + modules: BTreeMap, +} + +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", + ); + } + } +} diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs index fca5e109509..78653d932c4 100644 --- a/clippy_lints/src/lib.register_all.rs +++ b/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), diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index 7489f53fd3c..155de0a0b08 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/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, diff --git a/clippy_lints/src/lib.register_suspicious.rs b/clippy_lints/src/lib.register_suspicious.rs index 940bf7ace5e..5df2dd34368 100644 --- a/clippy_lints/src/lib.register_suspicious.rs +++ b/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), diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 1c502ef1d44..27cb5bf7087 100644 --- a/clippy_lints/src/lib.rs +++ b/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` } diff --git a/clippy_utils/src/diagnostics.rs b/clippy_utils/src/diagnostics.rs index 68b5151a648..04afe5ac373 100644 --- a/clippy_utils/src/diagnostics.rs +++ b/clippy_utils/src/diagnostics.rs @@ -77,7 +77,7 @@ pub fn span_lint(cx: &T, lint: &'static Lint, sp: impl Into( cx: &'a T, lint: &'static Lint, - span: Span, + span: impl Into, msg: &str, help_span: Option, help: &str, diff --git a/tests/ui-cargo/duplicate_mod/fail/Cargo.toml b/tests/ui-cargo/duplicate_mod/fail/Cargo.toml new file mode 100644 index 00000000000..bf3c817de1c --- /dev/null +++ b/tests/ui-cargo/duplicate_mod/fail/Cargo.toml @@ -0,0 +1,5 @@ +[package] +name = "duplicate_mod" +edition = "2021" +publish = false +version = "0.1.0" diff --git a/tests/ui-cargo/duplicate_mod/fail/src/a.rs b/tests/ui-cargo/duplicate_mod/fail/src/a.rs new file mode 100644 index 00000000000..8b137891791 --- /dev/null +++ b/tests/ui-cargo/duplicate_mod/fail/src/a.rs @@ -0,0 +1 @@ + diff --git a/tests/ui-cargo/duplicate_mod/fail/src/b.rs b/tests/ui-cargo/duplicate_mod/fail/src/b.rs new file mode 100644 index 00000000000..8b137891791 --- /dev/null +++ b/tests/ui-cargo/duplicate_mod/fail/src/b.rs @@ -0,0 +1 @@ + diff --git a/tests/ui-cargo/duplicate_mod/fail/src/c.rs b/tests/ui-cargo/duplicate_mod/fail/src/c.rs new file mode 100644 index 00000000000..8b137891791 --- /dev/null +++ b/tests/ui-cargo/duplicate_mod/fail/src/c.rs @@ -0,0 +1 @@ + diff --git a/tests/ui-cargo/duplicate_mod/fail/src/from_other_module.rs b/tests/ui-cargo/duplicate_mod/fail/src/from_other_module.rs new file mode 100644 index 00000000000..8b137891791 --- /dev/null +++ b/tests/ui-cargo/duplicate_mod/fail/src/from_other_module.rs @@ -0,0 +1 @@ + diff --git a/tests/ui-cargo/duplicate_mod/fail/src/main.rs b/tests/ui-cargo/duplicate_mod/fail/src/main.rs new file mode 100644 index 00000000000..79b343da247 --- /dev/null +++ b/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() {} diff --git a/tests/ui-cargo/duplicate_mod/fail/src/main.stderr b/tests/ui-cargo/duplicate_mod/fail/src/main.stderr new file mode 100644 index 00000000000..00d7739c8a2 --- /dev/null +++ b/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 + diff --git a/tests/ui-cargo/duplicate_mod/fail/src/other_module/mod.rs b/tests/ui-cargo/duplicate_mod/fail/src/other_module/mod.rs new file mode 100644 index 00000000000..36ce7286ade --- /dev/null +++ b/tests/ui-cargo/duplicate_mod/fail/src/other_module/mod.rs @@ -0,0 +1,2 @@ +#[path = "../from_other_module.rs"] +mod m;