Skip to content

Commit 44d8ef5

Browse files
JarchoJarcho
committed
Rework module_inception
* Don't check for repetition if the previous module crosses a body boundary. * Don't take a snippet of the entire item. * Check each item's visibility once. * Use `is_from_proc_macro` before linting
1 parent f88f9a9 commit 44d8ef5

File tree

2 files changed

+117
-65
lines changed

2 files changed

+117
-65
lines changed

‎clippy_lints/src/item_name_repetitions.rs

Lines changed: 108 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,9 @@
11
use clippy_config::Conf;
22
use clippy_utils::diagnostics::{span_lint, span_lint_and_help, span_lint_hir};
3-
use clippy_utils::is_bool;
4-
use clippy_utils::macros::span_is_local;
5-
use clippy_utils::source::is_present_in_source;
63
use clippy_utils::str_utils::{camel_case_split, count_match_end, count_match_start, to_camel_case, to_snake_case};
4+
use clippy_utils::{is_bool, is_from_proc_macro};
75
use rustc_data_structures::fx::FxHashSet;
8-
use rustc_hir::{EnumDef, FieldDef, Item, ItemKind, OwnerId, QPath, TyKind, Variant, VariantData};
6+
use rustc_hir::{Body, EnumDef, FieldDef, Item, ItemKind, QPath, TyKind, UseKind, Variant, VariantData};
97
use rustc_lint::{LateContext, LateLintPass};
108
use rustc_session::impl_lint_pass;
119
use rustc_span::symbol::Symbol;
@@ -158,14 +156,26 @@ declare_clippy_lint! {
158156
}
159157

160158
pub struct ItemNameRepetitions {
161-
modules: Vec<(Symbol, String, OwnerId)>,
159+
/// The module path the lint pass is in.
160+
modules: Vec<ModInfo>,
162161
enum_threshold: u64,
163162
struct_threshold: u64,
164163
avoid_breaking_exported_api: bool,
165164
allow_private_module_inception: bool,
166165
allowed_prefixes: FxHashSet<String>,
167166
}
168167

168+
struct ModInfo {
169+
name: Symbol,
170+
name_camel: String,
171+
/// Does this module have the `pub` visibility modifier.
172+
is_public: bool,
173+
/// How many bodies are between this module and the current lint pass position.
174+
///
175+
/// Only the most recently seen module is updated when entering/exiting a body.
176+
in_body_count: u32,
177+
}
178+
169179
impl ItemNameRepetitions {
170180
pub fn new(conf: &'static Conf) -> Self {
171181
Self {
@@ -456,82 +466,115 @@ fn check_enum_tuple_path_match(variant_name: &str, variant_data: VariantData<'_>
456466
}
457467

458468
impl LateLintPass<'_> for ItemNameRepetitions {
459-
fn check_item_post(&mut self, _cx: &LateContext<'_>, item: &Item<'_>) {
460-
let Some(_ident) = item.kind.ident() else { return };
469+
fn check_item_post(&mut self, _: &LateContext<'_>, item: &Item<'_>) {
470+
if matches!(item.kind, ItemKind::Mod(..)) {
471+
let prev = self.modules.pop();
472+
debug_assert!(prev.is_some());
473+
}
474+
}
461475

462-
let last = self.modules.pop();
463-
assert!(last.is_some());
476+
fn check_body(&mut self, _: &LateContext<'_>, _: &Body<'_>) {
477+
if let [.., last] = &mut *self.modules {
478+
last.in_body_count += 1;
479+
}
480+
}
481+
482+
fn check_body_post(&mut self, _: &LateContext<'_>, _: &Body<'_>) {
483+
if let [.., last] = &mut *self.modules {
484+
last.in_body_count -= 1;
485+
}
464486
}
465487

466488
fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) {
467-
let Some(ident) = item.kind.ident() else { return };
489+
let ident = match item.kind {
490+
ItemKind::Mod(ident, _) => {
491+
if let [.., prev] = &*self.modules
492+
&& prev.name == ident.name
493+
&& prev.in_body_count == 0
494+
&& (!self.allow_private_module_inception || prev.is_public)
495+
&& !item.span.from_expansion()
496+
&& !is_from_proc_macro(cx, item)
497+
{
498+
span_lint(
499+
cx,
500+
MODULE_INCEPTION,
501+
item.span,
502+
"module has the same name as its containing module",
503+
);
504+
}
505+
ident
506+
},
507+
508+
ItemKind::Enum(ident, def, _) => {
509+
if !ident.span.in_external_macro(cx.tcx.sess.source_map()) {
510+
self.check_variants(cx, item, &def);
511+
}
512+
ident
513+
},
514+
ItemKind::Struct(ident, data, _) => {
515+
if let VariantData::Struct { fields, .. } = data
516+
&& !ident.span.in_external_macro(cx.tcx.sess.source_map())
517+
{
518+
self.check_fields(cx, item, fields);
519+
}
520+
ident
521+
},
522+
523+
ItemKind::Const(ident, ..)
524+
| ItemKind::ExternCrate(_, ident)
525+
| ItemKind::Fn { ident, .. }
526+
| ItemKind::Macro(ident, ..)
527+
| ItemKind::Static(ident, ..)
528+
| ItemKind::Trait(_, _, ident, ..)
529+
| ItemKind::TraitAlias(ident, ..)
530+
| ItemKind::TyAlias(ident, ..)
531+
| ItemKind::Union(ident, ..)
532+
| ItemKind::Use(_, UseKind::Single(ident)) => ident,
533+
534+
ItemKind::ForeignMod { .. } | ItemKind::GlobalAsm { .. } | ItemKind::Impl(_) | ItemKind::Use(..) => return,
535+
};
468536

469537
let item_name = ident.name.as_str();
470538
let item_camel = to_camel_case(item_name);
471-
if !item.span.from_expansion() && is_present_in_source(cx, item.span)
472-
&& let [.., (mod_name, mod_camel, mod_owner_id)] = &*self.modules
473-
// constants don't have surrounding modules
474-
&& !mod_camel.is_empty()
539+
540+
if let [.., prev] = &*self.modules
541+
&& prev.is_public
542+
&& prev.in_body_count == 0
543+
&& item_camel.len() > prev.name_camel.len()
544+
&& !item.span.from_expansion()
545+
&& cx.tcx.visibility(item.owner_id).is_public()
475546
{
476-
if mod_name == &ident.name
477-
&& let ItemKind::Mod(..) = item.kind
478-
&& (!self.allow_private_module_inception || cx.tcx.visibility(mod_owner_id.def_id).is_public())
547+
if let Some(s) = item_camel.strip_prefix(&prev.name_camel)
548+
&& let Some(c) = s.chars().next()
549+
&& (c == '_' || c.is_uppercase() || c.is_numeric())
550+
&& !is_from_proc_macro(cx, item)
479551
{
480552
span_lint(
481553
cx,
482-
MODULE_INCEPTION,
483-
item.span,
484-
"module has the same name as its containing module",
554+
MODULE_NAME_REPETITIONS,
555+
ident.span,
556+
"item name starts with its containing module's name",
485557
);
486-
}
487-
488-
// The `module_name_repetitions` lint should only trigger if the item has the module in its
489-
// name. Having the same name is accepted.
490-
if cx.tcx.visibility(item.owner_id).is_public()
491-
&& cx.tcx.visibility(mod_owner_id.def_id).is_public()
492-
&& item_camel.len() > mod_camel.len()
558+
} else if let Some(s) = item_camel.strip_suffix(&prev.name_camel)
559+
&& !self.is_allowed_prefix(s)
560+
&& !is_from_proc_macro(cx, item)
493561
{
494-
let matching = count_match_start(mod_camel, &item_camel);
495-
let rmatching = count_match_end(mod_camel, &item_camel);
496-
let nchars = mod_camel.chars().count();
497-
498-
let is_word_beginning = |c: char| c == '_' || c.is_uppercase() || c.is_numeric();
499-
500-
if matching.char_count == nchars {
501-
match item_camel.chars().nth(nchars) {
502-
Some(c) if is_word_beginning(c) => span_lint(
503-
cx,
504-
MODULE_NAME_REPETITIONS,
505-
ident.span,
506-
"item name starts with its containing module's name",
507-
),
508-
_ => (),
509-
}
510-
}
511-
if rmatching.char_count == nchars
512-
&& !self.is_allowed_prefix(&item_camel[..item_camel.len() - rmatching.byte_count])
513-
{
514-
span_lint(
515-
cx,
516-
MODULE_NAME_REPETITIONS,
517-
ident.span,
518-
"item name ends with its containing module's name",
519-
);
520-
}
562+
span_lint(
563+
cx,
564+
MODULE_NAME_REPETITIONS,
565+
ident.span,
566+
"item name ends with its containing module's name",
567+
);
521568
}
522569
}
523570

524-
if span_is_local(item.span) {
525-
match item.kind {
526-
ItemKind::Enum(_, def, _) => {
527-
self.check_variants(cx, item, &def);
528-
},
529-
ItemKind::Struct(_, VariantData::Struct { fields, .. }, _) => {
530-
self.check_fields(cx, item, fields);
531-
},
532-
_ => (),
533-
}
571+
if matches!(item.kind, ItemKind::Mod(..)) {
572+
self.modules.push(ModInfo {
573+
name: ident.name,
574+
name_camel: item_camel,
575+
is_public: cx.tcx.visibility(item.owner_id).is_public(),
576+
in_body_count: 0,
577+
});
534578
}
535-
self.modules.push((ident.name, item_camel, item.owner_id));
536579
}
537580
}

‎tests/ui/module_inception.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,4 +38,13 @@ mod bar {
3838
mod bar {}
3939
}
4040

41+
mod with_inner_impl {
42+
struct S;
43+
impl S {
44+
fn f() {
45+
mod with_inner_impl {}
46+
}
47+
}
48+
}
49+
4150
fn main() {}

0 commit comments

Comments
 (0)