From 850c24edd3a97d0b2e905fdc4aab079b17c9bb71 Mon Sep 17 00:00:00 2001 From: Philipp Hansch Date: Fri, 19 Apr 2019 12:53:03 +0200 Subject: [PATCH] Fix false positive in module_name_repetitions lint This lint was triggering on modules inside expanded attrs, like for example `#[cfg(test)]` and possibly more. --- clippy_lints/src/attrs.rs | 18 ++---------------- clippy_lints/src/enum_variants.rs | 4 ++-- clippy_lints/src/utils/mod.rs | 14 ++++++++++++++ tests/ui/module_name_repetitions.rs | 10 ++++++++++ tests/ui/module_name_repetitions.stderr | 10 +++++----- 5 files changed, 33 insertions(+), 23 deletions(-) diff --git a/clippy_lints/src/attrs.rs b/clippy_lints/src/attrs.rs index 45250e1eca62..3f62a02b4573 100644 --- a/clippy_lints/src/attrs.rs +++ b/clippy_lints/src/attrs.rs @@ -2,8 +2,8 @@ use crate::reexport::*; use crate::utils::{ - in_macro, last_line_of_span, paths, snippet_opt, span_lint, span_lint_and_sugg, span_lint_and_then, - without_block_comments, + in_macro, is_present_in_source, last_line_of_span, paths, snippet_opt, span_lint, span_lint_and_sugg, + span_lint_and_then, without_block_comments, }; use if_chain::if_chain; use rustc::hir::*; @@ -481,20 +481,6 @@ fn is_word(nmi: &NestedMetaItem, expected: &str) -> bool { } } -// If the snippet is empty, it's an attribute that was inserted during macro -// expansion and we want to ignore those, because they could come from external -// sources that the user has no control over. -// For some reason these attributes don't have any expansion info on them, so -// we have to check it this way until there is a better way. -fn is_present_in_source(cx: &LateContext<'_, '_>, span: Span) -> bool { - if let Some(snippet) = snippet_opt(cx, span) { - if snippet.is_empty() { - return false; - } - } - true -} - declare_lint_pass!(DeprecatedCfgAttribute => [DEPRECATED_CFG_ATTR]); impl EarlyLintPass for DeprecatedCfgAttribute { diff --git a/clippy_lints/src/enum_variants.rs b/clippy_lints/src/enum_variants.rs index 20cd747dfc4e..27229710641d 100644 --- a/clippy_lints/src/enum_variants.rs +++ b/clippy_lints/src/enum_variants.rs @@ -1,6 +1,6 @@ //! lint on enum variants that are prefixed or suffixed by the same characters -use crate::utils::{camel_case, in_macro}; +use crate::utils::{camel_case, in_macro, is_present_in_source}; use crate::utils::{span_help_and_lint, span_lint}; use rustc::lint::{EarlyContext, EarlyLintPass, Lint, LintArray, LintPass}; use rustc::{declare_tool_lint, impl_lint_pass}; @@ -244,7 +244,7 @@ impl EarlyLintPass for EnumVariantNames { let item_name = item.ident.as_str(); let item_name_chars = item_name.chars().count(); let item_camel = to_camel_case(&item_name); - if !in_macro(item.span) { + if !in_macro(item.span) && is_present_in_source(cx, item.span) { if let Some(&(ref mod_name, ref mod_camel)) = self.modules.last() { // constants don't have surrounding modules if !mod_camel.is_empty() { diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs index e9c078544db5..31f6658b4e8b 100644 --- a/clippy_lints/src/utils/mod.rs +++ b/clippy_lints/src/utils/mod.rs @@ -93,6 +93,20 @@ pub fn in_macro(span: Span) -> bool { span.ctxt().outer().expn_info().is_some() } +// If the snippet is empty, it's an attribute that was inserted during macro +// expansion and we want to ignore those, because they could come from external +// sources that the user has no control over. +// For some reason these attributes don't have any expansion info on them, so +// we have to check it this way until there is a better way. +pub fn is_present_in_source<'a, T: LintContext<'a>>(cx: &T, span: Span) -> bool { + if let Some(snippet) = snippet_opt(cx, span) { + if snippet.is_empty() { + return false; + } + } + true +} + /// Checks if type is struct, enum or union type with the given def path. pub fn match_type(cx: &LateContext<'_, '_>, ty: Ty<'_>, path: &[&str]) -> bool { match ty.sty { diff --git a/tests/ui/module_name_repetitions.rs b/tests/ui/module_name_repetitions.rs index 1719845cb21b..669bf01a84c1 100644 --- a/tests/ui/module_name_repetitions.rs +++ b/tests/ui/module_name_repetitions.rs @@ -1,3 +1,5 @@ +// compile-flags: --test + #![warn(clippy::module_name_repetitions)] #![allow(dead_code)] @@ -13,4 +15,12 @@ mod foo { pub struct Foobar; } +#[cfg(test)] +mod test { + #[test] + fn it_works() { + assert_eq!(2 + 2, 4); + } +} + fn main() {} diff --git a/tests/ui/module_name_repetitions.stderr b/tests/ui/module_name_repetitions.stderr index 5bce2c9ba602..bdd217a969c0 100644 --- a/tests/ui/module_name_repetitions.stderr +++ b/tests/ui/module_name_repetitions.stderr @@ -1,5 +1,5 @@ error: item name starts with its containing module's name - --> $DIR/module_name_repetitions.rs:6:5 + --> $DIR/module_name_repetitions.rs:8:5 | LL | pub fn foo_bar() {} | ^^^^^^^^^^^^^^^^^^^ @@ -7,25 +7,25 @@ LL | pub fn foo_bar() {} = note: `-D clippy::module-name-repetitions` implied by `-D warnings` error: item name ends with its containing module's name - --> $DIR/module_name_repetitions.rs:7:5 + --> $DIR/module_name_repetitions.rs:9:5 | LL | pub fn bar_foo() {} | ^^^^^^^^^^^^^^^^^^^ error: item name starts with its containing module's name - --> $DIR/module_name_repetitions.rs:8:5 + --> $DIR/module_name_repetitions.rs:10:5 | LL | pub struct FooCake {} | ^^^^^^^^^^^^^^^^^^^^^ error: item name ends with its containing module's name - --> $DIR/module_name_repetitions.rs:9:5 + --> $DIR/module_name_repetitions.rs:11:5 | LL | pub enum CakeFoo {} | ^^^^^^^^^^^^^^^^^^^ error: item name starts with its containing module's name - --> $DIR/module_name_repetitions.rs:10:5 + --> $DIR/module_name_repetitions.rs:12:5 | LL | pub struct Foo7Bar; | ^^^^^^^^^^^^^^^^^^^