Skip to content

Commit cfeac47

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 cfeac47

File tree

2 files changed

+115
-65
lines changed

2 files changed

+115
-65
lines changed

‎clippy_lints/src/item_name_repetitions.rs

Lines changed: 106 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;
@@ -157,8 +155,15 @@ declare_clippy_lint! {
157155
"structs where all fields share a prefix/postfix or contain the name of the struct"
158156
}
159157

158+
struct ModInfo {
159+
name: Symbol,
160+
name_camel: String,
161+
is_public: bool,
162+
in_body_count: u32,
163+
}
164+
160165
pub struct ItemNameRepetitions {
161-
modules: Vec<(Symbol, String, OwnerId)>,
166+
modules: Vec<ModInfo>,
162167
enum_threshold: u64,
163168
struct_threshold: u64,
164169
avoid_breaking_exported_api: bool,
@@ -456,82 +461,118 @@ fn check_enum_tuple_path_match(variant_name: &str, variant_data: VariantData<'_>
456461
}
457462

458463
impl LateLintPass<'_> for ItemNameRepetitions {
459-
fn check_item_post(&mut self, _cx: &LateContext<'_>, item: &Item<'_>) {
460-
let Some(_ident) = item.kind.ident() else { return };
464+
fn check_item_post(&mut self, _: &LateContext<'_>, item: &Item<'_>) {
465+
if matches!(item.kind, ItemKind::Mod(..)) {
466+
let prev = self.modules.pop();
467+
debug_assert!(prev.is_some());
468+
}
469+
}
470+
471+
fn check_body(&mut self, _: &LateContext<'_>, _: &Body<'_>) {
472+
if let [.., last] = &mut *self.modules {
473+
last.in_body_count += 1;
474+
}
475+
}
461476

462-
let last = self.modules.pop();
463-
assert!(last.is_some());
477+
fn check_body_post(&mut self, _: &LateContext<'_>, _: &Body<'_>) {
478+
if let [.., last] = &mut *self.modules {
479+
last.in_body_count -= 1;
480+
}
464481
}
465482

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

469535
let item_name = ident.name.as_str();
470536
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()
537+
538+
if let [.., prev] = &*self.modules
539+
&& prev.is_public
540+
&& prev.in_body_count == 0
541+
&& item_camel.len() > prev.name_camel.len()
542+
&& !item.span.from_expansion()
543+
&& cx.tcx.visibility(item.owner_id).is_public()
475544
{
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())
545+
if let Some(s) = item_camel.strip_prefix(&prev.name_camel)
546+
&& let Some(c) = s.chars().next()
547+
&& (c == '_' || c.is_uppercase() || c.is_numeric())
548+
&& !is_from_proc_macro(cx, item)
479549
{
480550
span_lint(
481551
cx,
482-
MODULE_INCEPTION,
483-
item.span,
484-
"module has the same name as its containing module",
552+
MODULE_NAME_REPETITIONS,
553+
ident.span,
554+
"item name starts with its containing module's name",
485555
);
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()
556+
} else if let Some(s) = item_camel.strip_suffix(&prev.name_camel)
557+
&& !self.is_allowed_prefix(s)
558+
&& !is_from_proc_macro(cx, item)
493559
{
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-
}
560+
span_lint(
561+
cx,
562+
MODULE_NAME_REPETITIONS,
563+
ident.span,
564+
"item name ends with its containing module's name",
565+
);
521566
}
522567
}
523568

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-
}
569+
if matches!(item.kind, ItemKind::Mod(..)) {
570+
self.modules.push(ModInfo {
571+
name: ident.name,
572+
name_camel: item_camel,
573+
is_public: cx.tcx.visibility(item.owner_id).is_public(),
574+
in_body_count: 0,
575+
});
534576
}
535-
self.modules.push((ident.name, item_camel, item.owner_id));
536577
}
537578
}

‎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)