Skip to content

Commit

Permalink
Auto merge of #4006 - phansch:fix_module_name_repetitions_fp, r=flip1995
Browse files Browse the repository at this point in the history
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.

It was not reporting a location in #3892 because `span.lo()` and `span.hi()` both were 0.

Fixes #3892

changelog: Fix false positive in `module_name_repetitions` lint
  • Loading branch information
bors committed Apr 20, 2019
2 parents cafbe7f + 850c24e commit 7a6d5c0
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 23 deletions.
18 changes: 2 additions & 16 deletions clippy_lints/src/attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;
Expand Down Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/enum_variants.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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() {
Expand Down
14 changes: 14 additions & 0 deletions clippy_lints/src/utils/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
10 changes: 10 additions & 0 deletions tests/ui/module_name_repetitions.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// compile-flags: --test

#![warn(clippy::module_name_repetitions)]
#![allow(dead_code)]

Expand All @@ -13,4 +15,12 @@ mod foo {
pub struct Foobar;
}

#[cfg(test)]
mod test {
#[test]
fn it_works() {
assert_eq!(2 + 2, 4);
}
}

fn main() {}
10 changes: 5 additions & 5 deletions tests/ui/module_name_repetitions.stderr
Original file line number Diff line number Diff line change
@@ -1,31 +1,31 @@
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() {}
| ^^^^^^^^^^^^^^^^^^^
|
= 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;
| ^^^^^^^^^^^^^^^^^^^
Expand Down

0 comments on commit 7a6d5c0

Please sign in to comment.