user0/rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Rework module_inception

Browse source 
* 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
This commit is contained in:
Jason Newcomb 2025-05-08 02:53:00 -04:00
parent 2a6197e375
commit 26cf3eee16
2 changed files with 121 additions and 68 deletions
Download patch file Download diff file Expand all files Collapse all files

180
clippy_lints/src/item_name_repetitions.rs
View file

@ -1,11 +1,9 @@
use clippy_config::Conf;
use clippy_utils::diagnostics::{span_lint, span_lint_and_help, span_lint_hir};
use clippy_utils::is_bool;
use clippy_utils::macros::span_is_local;
use clippy_utils::source::is_present_in_source;
use clippy_utils::str_utils::{camel_case_split, count_match_end, count_match_start, to_camel_case, to_snake_case};
use clippy_utils::{is_bool, is_from_proc_macro};
use rustc_data_structures::fx::FxHashSet;
use rustc_hir::{EnumDef, FieldDef, Item, ItemKind, OwnerId, QPath, TyKind, Variant, VariantData};
use rustc_hir::{Body, EnumDef, FieldDef, Item, ItemKind, QPath, TyKind, UseKind, Variant, VariantData};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::impl_lint_pass;
use rustc_span::symbol::Symbol;
@ -158,7 +156,8 @@ declare_clippy_lint! {
}
pub struct ItemNameRepetitions {
modules: Vec<(Symbol, String, OwnerId)>,
/// The module path the lint pass is in.
modules: Vec<ModInfo>,
enum_threshold: u64,
struct_threshold: u64,
avoid_breaking_exported_api: bool,
@ -167,6 +166,17 @@ pub struct ItemNameRepetitions {
allowed_prefixes: FxHashSet<String>,
}
struct ModInfo {
name: Symbol,
name_camel: String,
/// Does this module have the `pub` visibility modifier.
is_public: bool,
/// How many bodies are between this module and the current lint pass position.
///
/// Only the most recently seen module is updated when entering/exiting a body.
in_body_count: u32,
}
impl ItemNameRepetitions {
pub fn new(conf: &'static Conf) -> Self {
Self {
@ -458,71 +468,109 @@ fn check_enum_tuple_path_match(variant_name: &str, variant_data: VariantData<'_>
}
impl LateLintPass<'_> for ItemNameRepetitions {
fn check_item_post(&mut self, _cx: &LateContext<'_>, item: &Item<'_>) {
let Some(_ident) = item.kind.ident() else { return };
fn check_item_post(&mut self, _: &LateContext<'_>, item: &Item<'_>) {
if matches!(item.kind, ItemKind::Mod(..)) {
let prev = self.modules.pop();
debug_assert!(prev.is_some());
}
}
let last = self.modules.pop();
assert!(last.is_some());
fn check_body(&mut self, _: &LateContext<'_>, _: &Body<'_>) {
if let [.., last] = &mut *self.modules {
last.in_body_count += 1;
}
}
fn check_body_post(&mut self, _: &LateContext<'_>, _: &Body<'_>) {
if let [.., last] = &mut *self.modules {
last.in_body_count -= 1;
}
}
fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) {
let Some(ident) = item.kind.ident() else { return };
let ident = match item.kind {
ItemKind::Mod(ident, _) => {
if let [.., prev] = &*self.modules
&& prev.name == ident.name
&& prev.in_body_count == 0
&& (!self.allow_private_module_inception || prev.is_public)
&& !item.span.from_expansion()
&& !is_from_proc_macro(cx, item)
{
span_lint(
cx,
MODULE_INCEPTION,
item.span,
"module has the same name as its containing module",
);
}
ident
},
ItemKind::Enum(ident, _, def) => {
if !ident.span.in_external_macro(cx.tcx.sess.source_map()) {
self.check_variants(cx, item, &def);
}
ident
},
ItemKind::Struct(ident, _, data) => {
if let VariantData::Struct { fields, .. } = data
&& !ident.span.in_external_macro(cx.tcx.sess.source_map())
{
self.check_fields(cx, item, fields);
}
ident
},
ItemKind::Const(ident, ..)
| ItemKind::ExternCrate(_, ident)
| ItemKind::Fn { ident, .. }
| ItemKind::Macro(ident, ..)
| ItemKind::Static(_, ident, ..)
| ItemKind::Trait(_, _, _, ident, ..)
| ItemKind::TraitAlias(ident, ..)
| ItemKind::TyAlias(ident, ..)
| ItemKind::Union(ident, ..)
| ItemKind::Use(_, UseKind::Single(ident)) => ident,
ItemKind::ForeignMod { .. } | ItemKind::GlobalAsm { .. } | ItemKind::Impl(_) | ItemKind::Use(..) => return,
};
let item_name = ident.name.as_str();
let item_camel = to_camel_case(item_name);
if !item.span.from_expansion() && is_present_in_source(cx, item.span)
&& let [.., (mod_name, mod_camel, mod_owner_id)] = &*self.modules
// constants don't have surrounding modules
&& !mod_camel.is_empty()
if let [.., prev] = &*self.modules
&& prev.is_public
&& prev.in_body_count == 0
&& !item.span.from_expansion()
&& !matches!(item.kind, ItemKind::Macro(..))
&& cx.tcx.visibility(item.owner_id).is_public()
{
if mod_name == &ident.name
&& let ItemKind::Mod(..) = item.kind
&& (!self.allow_private_module_inception || cx.tcx.visibility(mod_owner_id.def_id).is_public())
{
span_lint(
cx,
MODULE_INCEPTION,
item.span,
"module has the same name as its containing module",
);
}
// The `module_name_repetitions` lint should only trigger if the item has the module in its
// name. Having the same name is only accepted if `allow_exact_repetition` is set to `true`.
let both_are_public =
cx.tcx.visibility(item.owner_id).is_public() && cx.tcx.visibility(mod_owner_id.def_id).is_public();
if both_are_public && !self.allow_exact_repetitions && item_camel == *mod_camel {
span_lint(
cx,
MODULE_NAME_REPETITIONS,
ident.span,
"item name is the same as its containing module's name",
);
}
let is_macro = matches!(item.kind, ItemKind::Macro(_, _, _));
if both_are_public && item_camel.len() > mod_camel.len() && !is_macro {
let matching = count_match_start(mod_camel, &item_camel);
let rmatching = count_match_end(mod_camel, &item_camel);
let nchars = mod_camel.chars().count();
let is_word_beginning = |c: char| c == '_' || c.is_uppercase() || c.is_numeric();
if matching.char_count == nchars {
match item_camel.chars().nth(nchars) {
Some(c) if is_word_beginning(c) => span_lint(
if !self.allow_exact_repetitions && item_camel == prev.name_camel {
if !is_from_proc_macro(cx, item) {
span_lint(
cx,
MODULE_NAME_REPETITIONS,
ident.span,
"item name is the same as its containing module's name",
);
}
} else if item_camel.len() > prev.name_camel.len() {
if let Some(s) = item_camel.strip_prefix(&prev.name_camel)
&& let Some(c) = s.chars().next()
&& (c == '_' || c.is_uppercase() || c.is_numeric())
{
if !is_from_proc_macro(cx, item) {
span_lint(
cx,
MODULE_NAME_REPETITIONS,
ident.span,
"item name starts with its containing module's name",
),
_ => (),
);
}
}
if rmatching.char_count == nchars
&& !self.is_allowed_prefix(&item_camel[..item_camel.len() - rmatching.byte_count])
} else if let Some(s) = item_camel.strip_suffix(&prev.name_camel)
&& !self.is_allowed_prefix(s)
&& !is_from_proc_macro(cx, item)
{
span_lint(
cx,
@ -534,17 +582,13 @@ impl LateLintPass<'_> for ItemNameRepetitions {
}
}
if span_is_local(item.span) {
match item.kind {
ItemKind::Enum(_, _, def) => {
self.check_variants(cx, item, &def);
},
ItemKind::Struct(_, _, VariantData::Struct { fields, .. }) => {
self.check_fields(cx, item, fields);
},
_ => (),
}
if matches!(item.kind, ItemKind::Mod(..)) {
self.modules.push(ModInfo {
name: ident.name,
name_camel: item_camel,
is_public: cx.tcx.visibility(item.owner_id).is_public(),
in_body_count: 0,
});
}
self.modules.push((ident.name, item_camel, item.owner_id));
}
}

9
tests/ui/module_inception.rs
View file

@ -38,4 +38,13 @@ mod bar {
mod bar {}
}
mod with_inner_impl {
struct S;
impl S {
fn f() {
mod with_inner_impl {}
}
}
}
fn main() {}