diff --git a/clippy_lints/src/enum_variants.rs b/clippy_lints/src/enum_variants.rs index 17c3de480480..8211621fa0e0 100644 --- a/clippy_lints/src/enum_variants.rs +++ b/clippy_lints/src/enum_variants.rs @@ -47,8 +47,32 @@ declare_lint! { "type names prefixed/postfixed with their containing module's name" } +/// **What it does:** Checks for modules that have the same name as their parent module +/// +/// **Why is this bad?** A typical beginner mistake is to have `mod foo;` and again `mod foo { .. }` in `foo.rs`. +/// The expectation is that items inside the inner `mod foo { .. }` are then available +/// through `foo::x`, but they are only available through `foo::foo::x`. +/// If this is done on purpose, it would be better to choose a more representative module name. +/// +/// **Known problems:** None. +/// +/// **Example:** +/// ```rust +/// // lib.rs +/// mod foo; +/// // foo.rs +/// mod foo { +/// ... +/// } +/// ``` +declare_lint! { + pub MODULE_INCEPTION, + Warn, + "modules that have the same name as their parent module" +} + pub struct EnumVariantNames { - modules: Vec, + modules: Vec<(InternedString, String)>, threshold: u64, } @@ -60,7 +84,7 @@ impl EnumVariantNames { impl LintPass for EnumVariantNames { fn get_lints(&self) -> LintArray { - lint_array!(ENUM_VARIANT_NAMES, STUTTER) + lint_array!(ENUM_VARIANT_NAMES, STUTTER, MODULE_INCEPTION) } } @@ -171,9 +195,12 @@ impl EarlyLintPass for EnumVariantNames { let item_name_chars = item_name.chars().count(); let item_camel = to_camel_case(&item_name); if item.vis == Visibility::Public && !in_macro(cx, item.span) { - if let Some(mod_camel) = self.modules.last() { + if let Some(&(ref mod_name, ref mod_camel)) = self.modules.last() { // constants don't have surrounding modules if !mod_camel.is_empty() { + if mod_name == &item_name { + span_lint(cx, MODULE_INCEPTION, item.span, "item has the same name as its containing module"); + } let matching = partial_match(mod_camel, &item_camel); let rmatching = partial_rmatch(mod_camel, &item_camel); let nchars = mod_camel.chars().count(); @@ -189,6 +216,6 @@ impl EarlyLintPass for EnumVariantNames { if let ItemKind::Enum(ref def, _) = item.node { check_variant(cx, self.threshold, def, &item_name, item_name_chars, item.span); } - self.modules.push(item_camel); + self.modules.push((item_name, item_camel)); } } diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 36d466ed9c95..46a2a737d7c1 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -96,7 +96,6 @@ pub mod minmax; pub mod misc; pub mod misc_early; pub mod missing_doc; -pub mod module_inception; pub mod mut_mut; pub mod mut_reference; pub mod mutex_atomic; @@ -175,7 +174,6 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { reg.register_late_lint_pass(box utils::internal_lints::LintWithoutLintPass::default()); reg.register_late_lint_pass(box types::TypePass); reg.register_late_lint_pass(box booleans::NonminimalBool); - reg.register_early_lint_pass(box module_inception::Pass); reg.register_late_lint_pass(box eq_op::EqOp); reg.register_early_lint_pass(box enum_variants::EnumVariantNames::new(conf.enum_variant_name_threshold)); reg.register_late_lint_pass(box enum_glob_use::EnumGlobUse); @@ -329,6 +327,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { entry::MAP_ENTRY, enum_clike::ENUM_CLIKE_UNPORTABLE_VARIANT, enum_variants::ENUM_VARIANT_NAMES, + enum_variants::MODULE_INCEPTION, eq_op::EQ_OP, escape::BOXED_LOCAL, eta_reduction::REDUNDANT_CLOSURE, @@ -391,7 +390,6 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { misc_early::REDUNDANT_CLOSURE_CALL, misc_early::UNNEEDED_FIELD_PATTERN, misc_early::ZERO_PREFIXED_LITERAL, - module_inception::MODULE_INCEPTION, mut_reference::UNNECESSARY_MUT_PASSED, mutex_atomic::MUTEX_ATOMIC, needless_bool::BOOL_COMPARISON, diff --git a/clippy_lints/src/module_inception.rs b/clippy_lints/src/module_inception.rs deleted file mode 100644 index 10c8154d100e..000000000000 --- a/clippy_lints/src/module_inception.rs +++ /dev/null @@ -1,50 +0,0 @@ -use rustc::lint::*; -use syntax::ast::*; -use utils::span_lint; - -/// **What it does:** Checks for modules that have the same name as their parent module -/// -/// **Why is this bad?** A typical beginner mistake is to have `mod foo;` and again `mod foo { .. }` in `foo.rs`. -/// The expectation is that items inside the inner `mod foo { .. }` are then available -/// through `foo::x`, but they are only available through `foo::foo::x`. -/// If this is done on purpose, it would be better to choose a more representative module name. -/// -/// **Known problems:** None. -/// -/// **Example:** -/// ```rust -/// // lib.rs -/// mod foo; -/// // foo.rs -/// mod foo { -/// ... -/// } -/// ``` -declare_lint! { - pub MODULE_INCEPTION, - Warn, - "modules that have the same name as their parent module" -} - -pub struct Pass; - -impl LintPass for Pass { - fn get_lints(&self) -> LintArray { - lint_array![MODULE_INCEPTION] - } -} - -impl EarlyLintPass for Pass { - fn check_item(&mut self, cx: &EarlyContext, item: &Item) { - if let ItemKind::Mod(ref module) = item.node { - for sub_item in &module.items { - if let ItemKind::Mod(_) = sub_item.node { - if item.ident == sub_item.ident { - span_lint(cx, MODULE_INCEPTION, sub_item.span, - "module has the same name as its containing module"); - } - } - } - } - } -} diff --git a/tests/compile-fail/module_inception.rs b/tests/compile-fail/module_inception.rs index 861ed504c86e..f6228cbb7950 100644 --- a/tests/compile-fail/module_inception.rs +++ b/tests/compile-fail/module_inception.rs @@ -4,20 +4,26 @@ mod foo { mod bar { - mod bar { //~ ERROR module has the same name as its containing module + pub mod bar { //~ ERROR item has the same name as its containing module mod foo {} } mod foo {} } - mod foo { //~ ERROR module has the same name as its containing module + pub mod foo { //~ ERROR item has the same name as its containing module mod bar {} } } +mod cake { + mod cake { + // no error, since module is not public + } +} + // No warning. See . mod bar { #[allow(module_inception)] - mod bar { + pub mod bar { } }