Skip to content

Commit a235cbd

Browse files
committed
module_name_repetitions: don't warn if the item is in a private module.
Fixes <#8524>. There is still a warning (as there should be) if the item is reexported by name, but not by glob; that would require further work to examine the names in the glob, and I haven't looked into that. Credit to @Centri3 for suggesting approximately this simple fix in <#8524 (comment)>. However, per later comment <#8524 (comment)>, I am not making it configuration-dependent, but *always* checking public items in public modules only.
1 parent b39323b commit a235cbd

File tree

8 files changed

+53
-13
lines changed

8 files changed

+53
-13
lines changed

clippy_lints/src/item_name_repetitions.rs

+27-6
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,28 @@ declare_clippy_lint! {
5050

5151
declare_clippy_lint! {
5252
/// ### What it does
53-
/// Detects type names that are prefixed or suffixed by the
54-
/// containing module's name.
53+
/// Detects public item names that are prefixed or suffixed by the
54+
/// containing public module's name.
5555
///
5656
/// ### Why is this bad?
57-
/// It requires the user to type the module name twice.
57+
/// It requires the user to type the module name twice in each usage,
58+
/// especially if they choose to import the module rather than its contents.
59+
///
60+
/// Lack of such repetition is also the style used in the Rust standard library;
61+
/// e.g. `io::Error` and `fmt::Error` rather than `io::IoError` and `fmt::FmtError`;
62+
/// and `array::from_ref` rather than `array::array_from_ref`.
63+
///
64+
/// ### Known issues
65+
/// Glob re-exports are ignored; e.g. this will not warn even though it should:
66+
///
67+
/// ```no_run
68+
/// pub mod foo {
69+
/// mod iteration {
70+
/// pub struct FooIter {}
71+
/// }
72+
/// pub use iteration::*; // creates the path `foo::FooIter`
73+
/// }
74+
/// ```
5875
///
5976
/// ### Example
6077
/// ```no_run
@@ -389,12 +406,12 @@ impl LateLintPass<'_> for ItemNameRepetitions {
389406
let item_name = item.ident.name.as_str();
390407
let item_camel = to_camel_case(item_name);
391408
if !item.span.from_expansion() && is_present_in_source(cx, item.span) {
392-
if let [.., (mod_name, mod_camel, owner_id)] = &*self.modules {
409+
if let [.., (mod_name, mod_camel, mod_owner_id)] = &*self.modules {
393410
// constants don't have surrounding modules
394411
if !mod_camel.is_empty() {
395412
if mod_name == &item.ident.name
396413
&& let ItemKind::Mod(..) = item.kind
397-
&& (!self.allow_private_module_inception || cx.tcx.visibility(owner_id.def_id).is_public())
414+
&& (!self.allow_private_module_inception || cx.tcx.visibility(mod_owner_id.def_id).is_public())
398415
{
399416
span_lint(
400417
cx,
@@ -403,9 +420,13 @@ impl LateLintPass<'_> for ItemNameRepetitions {
403420
"module has the same name as its containing module",
404421
);
405422
}
423+
406424
// The `module_name_repetitions` lint should only trigger if the item has the module in its
407425
// name. Having the same name is accepted.
408-
if cx.tcx.visibility(item.owner_id).is_public() && item_camel.len() > mod_camel.len() {
426+
if cx.tcx.visibility(item.owner_id).is_public()
427+
&& cx.tcx.visibility(mod_owner_id.def_id).is_public()
428+
&& item_camel.len() > mod_camel.len()
429+
{
409430
let matching = count_match_start(mod_camel, &item_camel);
410431
let rmatching = count_match_end(mod_camel, &item_camel);
411432
let nchars = mod_camel.chars().count();

clippy_lints/src/literal_representation.rs

-1
Original file line numberDiff line numberDiff line change
@@ -412,7 +412,6 @@ impl LiteralDigitGrouping {
412412
}
413413
}
414414

415-
#[expect(clippy::module_name_repetitions)]
416415
pub struct DecimalLiteralRepresentation {
417416
threshold: u64,
418417
}

clippy_lints/src/macro_use.rs

-1
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@ impl MacroRefData {
4343
}
4444

4545
#[derive(Default)]
46-
#[expect(clippy::module_name_repetitions)]
4746
pub struct MacroUseImports {
4847
/// the actual import path used and the span of the attribute above it. The value is
4948
/// the location, where the lint should be emitted.

clippy_lints/src/vec.rs

-1
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ use rustc_middle::ty::layout::LayoutOf;
1717
use rustc_session::impl_lint_pass;
1818
use rustc_span::{DesugaringKind, Span, sym};
1919

20-
#[expect(clippy::module_name_repetitions)]
2120
pub struct UselessVec {
2221
too_large_for_stack: u64,
2322
msrv: Msrv,

tests/ui-toml/item_name_repetitions/allowed_prefixes/item_name_repetitions.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
#![warn(clippy::module_name_repetitions)]
22
#![allow(dead_code)]
33

4-
mod foo {
4+
pub mod foo {
55
// #12544 - shouldn't warn if item name consists only of an allowed prefix and a module name.
66
// In this test, allowed prefixes are configured to be ["bar"].
77

tests/ui-toml/item_name_repetitions/allowed_prefixes_extend/item_name_repetitions.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
#![warn(clippy::module_name_repetitions)]
22
#![allow(dead_code)]
33

4-
mod foo {
4+
pub mod foo {
55
// #12544 - shouldn't warn if item name consists only of an allowed prefix and a module name.
66
// In this test, allowed prefixes are configured to be all of the default prefixes and ["bar"].
77

tests/ui/module_name_repetitions.rs

+17-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
#![warn(clippy::module_name_repetitions)]
44
#![allow(dead_code)]
55

6-
mod foo {
6+
pub mod foo {
77
pub fn foo() {}
88
pub fn foo_bar() {}
99
//~^ ERROR: item name starts with its containing module's name
@@ -20,6 +20,22 @@ mod foo {
2020
// Should not warn
2121
pub struct Foobar;
2222

23+
// #8524 - shouldn't warn when item is declared in a private module...
24+
mod error {
25+
pub struct Error;
26+
pub struct FooError;
27+
}
28+
pub use error::Error;
29+
// ... but should still warn when the item is reexported to create a *public* path with repetition.
30+
pub use error::FooError;
31+
//~^ ERROR: item name starts with its containing module's name
32+
33+
// FIXME: This should also warn because it creates the public path `foo::FooIter`.
34+
mod iter {
35+
pub struct FooIter;
36+
}
37+
pub use iter::*;
38+
2339
// #12544 - shouldn't warn if item name consists only of an allowed prefix and a module name.
2440
pub fn to_foo() {}
2541
pub fn into_foo() {}

tests/ui/module_name_repetitions.stderr

+7-1
Original file line numberDiff line numberDiff line change
@@ -31,5 +31,11 @@ error: item name starts with its containing module's name
3131
LL | pub struct Foo7Bar;
3232
| ^^^^^^^
3333

34-
error: aborting due to 5 previous errors
34+
error: item name starts with its containing module's name
35+
--> tests/ui/module_name_repetitions.rs:30:20
36+
|
37+
LL | pub use error::FooError;
38+
| ^^^^^^^^
39+
40+
error: aborting due to 6 previous errors
3541

0 commit comments

Comments
 (0)