Skip to content

Commit e43890e

Browse files
committed
Lint for pub(crate) items that are not crate visible due to the visibility of the module that contains them
Closes #5274.
1 parent fdce47b commit e43890e

File tree

7 files changed

+308
-2
lines changed

7 files changed

+308
-2
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1318,6 +1318,7 @@ Released 2018-09-13
13181318
[`redundant_field_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_field_names
13191319
[`redundant_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_pattern
13201320
[`redundant_pattern_matching`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_pattern_matching
1321+
[`redundant_pub_crate`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_pub_crate
13211322
[`redundant_static_lifetimes`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_static_lifetimes
13221323
[`ref_in_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#ref_in_deref
13231324
[`regex_macro`]: https://rust-lang.github.io/rust-clippy/master/index.html#regex_macro

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.
77

8-
[There are 361 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
8+
[There are 362 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
99

1010
We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you:
1111

clippy_lints/src/lib.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,7 @@ pub mod ranges;
282282
pub mod redundant_clone;
283283
pub mod redundant_field_names;
284284
pub mod redundant_pattern_matching;
285+
pub mod redundant_pub_crate;
285286
pub mod redundant_static_lifetimes;
286287
pub mod reference;
287288
pub mod regex;
@@ -742,6 +743,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
742743
&redundant_clone::REDUNDANT_CLONE,
743744
&redundant_field_names::REDUNDANT_FIELD_NAMES,
744745
&redundant_pattern_matching::REDUNDANT_PATTERN_MATCHING,
746+
&redundant_pub_crate::REDUNDANT_PUB_CRATE,
745747
&redundant_static_lifetimes::REDUNDANT_STATIC_LIFETIMES,
746748
&reference::DEREF_ADDROF,
747749
&reference::REF_IN_DEREF,
@@ -1018,6 +1020,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
10181020
store.register_late_pass(|| box wildcard_imports::WildcardImports);
10191021
store.register_early_pass(|| box macro_use::MacroUseImports);
10201022
store.register_late_pass(|| box verbose_file_reads::VerboseFileReads);
1023+
store.register_late_pass(|| box redundant_pub_crate::RedundantPubCrate::default());
10211024

10221025
store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![
10231026
LintId::of(&arithmetic::FLOAT_ARITHMETIC),
@@ -1667,6 +1670,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
16671670
LintId::of(&mutex_atomic::MUTEX_INTEGER),
16681671
LintId::of(&needless_borrow::NEEDLESS_BORROW),
16691672
LintId::of(&path_buf_push_overwrite::PATH_BUF_PUSH_OVERWRITE),
1673+
LintId::of(&redundant_pub_crate::REDUNDANT_PUB_CRATE),
16701674
LintId::of(&use_self::USE_SELF),
16711675
]);
16721676
}
Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
use crate::utils::span_lint;
2+
use rustc_hir::{Item, ItemKind, VisibilityKind};
3+
use rustc_lint::{LateContext, LateLintPass};
4+
use rustc_session::{declare_tool_lint, impl_lint_pass};
5+
6+
declare_clippy_lint! {
7+
/// **What it does:** Checks for items declared `pub(crate)` that are not crate visible because they
8+
/// are inside a private module.
9+
///
10+
/// **Why is this bad?** Writing `pub(crate)` is misleading when it's redundant due to the parent
11+
/// module's visibility.
12+
///
13+
/// **Known problems:** None.
14+
///
15+
/// **Example:**
16+
///
17+
/// ```rust
18+
/// mod internal {
19+
/// pub(crate) fn internal_fn() { }
20+
/// }
21+
/// ```
22+
/// This function is not visible outside the module and it can be declared with `pub` or
23+
/// private visibility
24+
/// ```rust
25+
/// mod internal {
26+
/// pub fn internal_fn() { }
27+
/// }
28+
/// ```
29+
pub REDUNDANT_PUB_CRATE,
30+
nursery,
31+
"Using `pub(crate)` visibility on items that are not crate visible due to the visibility of the module that contains them."
32+
}
33+
34+
#[derive(Default)]
35+
pub struct RedundantPubCrate {
36+
is_exported: Vec<bool>,
37+
}
38+
39+
impl_lint_pass!(RedundantPubCrate => [REDUNDANT_PUB_CRATE]);
40+
41+
impl RedundantPubCrate {
42+
pub fn new() -> Self {
43+
Self {
44+
is_exported: Vec::new(),
45+
}
46+
}
47+
}
48+
49+
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for RedundantPubCrate {
50+
fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx Item<'tcx>) {
51+
if let VisibilityKind::Crate { .. } = item.vis.node {
52+
if !cx.access_levels.is_exported(item.hir_id) {
53+
if let Some(false) = self.is_exported.last() {
54+
span_lint(
55+
cx,
56+
REDUNDANT_PUB_CRATE,
57+
item.span,
58+
&format!("pub(crate) {} inside private module", item.kind.descr()),
59+
)
60+
}
61+
}
62+
}
63+
64+
if let ItemKind::Mod { .. } = item.kind {
65+
self.is_exported.push(cx.access_levels.is_exported(item.hir_id));
66+
}
67+
}
68+
69+
fn check_item_post(&mut self, _cx: &LateContext<'a, 'tcx>, item: &'tcx Item<'tcx>) {
70+
if let ItemKind::Mod { .. } = item.kind {
71+
self.is_exported.pop().expect("unbalanced check_item/check_item_post");
72+
}
73+
}
74+
}

src/lintlist/mod.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ pub use lint::Lint;
66
pub use lint::LINT_LEVELS;
77

88
// begin lint list, do not remove this comment, it’s used in `update_lints`
9-
pub const ALL_LINTS: [Lint; 361] = [
9+
pub const ALL_LINTS: [Lint; 362] = [
1010
Lint {
1111
name: "absurd_extreme_comparisons",
1212
group: "correctness",
@@ -1764,6 +1764,13 @@ pub const ALL_LINTS: [Lint; 361] = [
17641764
deprecation: None,
17651765
module: "redundant_pattern_matching",
17661766
},
1767+
Lint {
1768+
name: "redundant_pub_crate",
1769+
group: "nursery",
1770+
desc: "Using `pub(crate)` visibility on items that are not crate visible due to the visibility of the module that contains them.",
1771+
deprecation: None,
1772+
module: "redundant_pub_crate",
1773+
},
17671774
Lint {
17681775
name: "redundant_static_lifetimes",
17691776
group: "style",

tests/ui/redundant_pub_crate.rs

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
#![warn(clippy::redundant_pub_crate)]
2+
3+
mod m1 {
4+
fn f() {}
5+
pub(crate) fn g() {} // private due to m1
6+
pub fn h() {}
7+
8+
mod m1_1 {
9+
fn f() {}
10+
pub(crate) fn g() {} // private due to m1_1 and m1
11+
pub fn h() {}
12+
}
13+
14+
pub(crate) mod m1_2 {
15+
// ^ private due to m1
16+
fn f() {}
17+
pub(crate) fn g() {} // private due to m1_2 and m1
18+
pub fn h() {}
19+
}
20+
21+
pub mod m1_3 {
22+
fn f() {}
23+
pub(crate) fn g() {} // private due to m1
24+
pub fn h() {}
25+
}
26+
}
27+
28+
pub(crate) mod m2 {
29+
fn f() {}
30+
pub(crate) fn g() {} // already crate visible due to m2
31+
pub fn h() {}
32+
33+
mod m2_1 {
34+
fn f() {}
35+
pub(crate) fn g() {} // private due to m2_1
36+
pub fn h() {}
37+
}
38+
39+
pub(crate) mod m2_2 {
40+
// ^ already crate visible due to m2
41+
fn f() {}
42+
pub(crate) fn g() {} // already crate visible due to m2_2 and m2
43+
pub fn h() {}
44+
}
45+
46+
pub mod m2_3 {
47+
fn f() {}
48+
pub(crate) fn g() {} // already crate visible due to m2
49+
pub fn h() {}
50+
}
51+
}
52+
53+
pub mod m3 {
54+
fn f() {}
55+
pub(crate) fn g() {} // ok: m3 is exported
56+
pub fn h() {}
57+
58+
mod m3_1 {
59+
fn f() {}
60+
pub(crate) fn g() {} // private due to m3_1
61+
pub fn h() {}
62+
}
63+
64+
pub(crate) mod m3_2 {
65+
// ^ ok
66+
fn f() {}
67+
pub(crate) fn g() {} // already crate visible due to m3_2
68+
pub fn h() {}
69+
}
70+
71+
pub mod m3_3 {
72+
fn f() {}
73+
pub(crate) fn g() {} // ok: m3 and m3_3 are exported
74+
pub fn h() {}
75+
}
76+
}
77+
78+
mod m4 {
79+
fn f() {}
80+
pub(crate) fn g() {} // private: not re-exported by `pub use m4::*`
81+
pub fn h() {}
82+
83+
mod m4_1 {
84+
fn f() {}
85+
pub(crate) fn g() {} // private due to m4_1
86+
pub fn h() {}
87+
}
88+
89+
pub(crate) mod m4_2 {
90+
// ^ private: not re-exported by `pub use m4::*`
91+
fn f() {}
92+
pub(crate) fn g() {} // private due to m4_2
93+
pub fn h() {}
94+
}
95+
96+
pub mod m4_3 {
97+
fn f() {}
98+
pub(crate) fn g() {} // ok: m4_3 is re-exported by `pub use m4::*`
99+
pub fn h() {}
100+
}
101+
}
102+
103+
pub use m4::*;
104+
105+
fn main() {}

tests/ui/redundant_pub_crate.stderr

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
error: pub(crate) function inside private module
2+
--> $DIR/redundant_pub_crate.rs:5:5
3+
|
4+
LL | pub(crate) fn g() {} // private due to m1
5+
| ^^^^^^^^^^^^^^^^^^^^
6+
|
7+
= note: `-D clippy::redundant-pub-crate` implied by `-D warnings`
8+
9+
error: pub(crate) function inside private module
10+
--> $DIR/redundant_pub_crate.rs:10:9
11+
|
12+
LL | pub(crate) fn g() {} // private due to m1_1 and m1
13+
| ^^^^^^^^^^^^^^^^^^^^
14+
15+
error: pub(crate) module inside private module
16+
--> $DIR/redundant_pub_crate.rs:14:5
17+
|
18+
LL | / pub(crate) mod m1_2 {
19+
LL | | // ^ private due to m1
20+
LL | | fn f() {}
21+
LL | | pub(crate) fn g() {} // private due to m1_2 and m1
22+
LL | | pub fn h() {}
23+
LL | | }
24+
| |_____^
25+
26+
error: pub(crate) function inside private module
27+
--> $DIR/redundant_pub_crate.rs:17:9
28+
|
29+
LL | pub(crate) fn g() {} // private due to m1_2 and m1
30+
| ^^^^^^^^^^^^^^^^^^^^
31+
32+
error: pub(crate) function inside private module
33+
--> $DIR/redundant_pub_crate.rs:23:9
34+
|
35+
LL | pub(crate) fn g() {} // private due to m1
36+
| ^^^^^^^^^^^^^^^^^^^^
37+
38+
error: pub(crate) function inside private module
39+
--> $DIR/redundant_pub_crate.rs:30:5
40+
|
41+
LL | pub(crate) fn g() {} // already crate visible due to m2
42+
| ^^^^^^^^^^^^^^^^^^^^
43+
44+
error: pub(crate) function inside private module
45+
--> $DIR/redundant_pub_crate.rs:35:9
46+
|
47+
LL | pub(crate) fn g() {} // private due to m2_1
48+
| ^^^^^^^^^^^^^^^^^^^^
49+
50+
error: pub(crate) module inside private module
51+
--> $DIR/redundant_pub_crate.rs:39:5
52+
|
53+
LL | / pub(crate) mod m2_2 {
54+
LL | | // ^ already crate visible due to m2
55+
LL | | fn f() {}
56+
LL | | pub(crate) fn g() {} // already crate visible due to m2_2 and m2
57+
LL | | pub fn h() {}
58+
LL | | }
59+
| |_____^
60+
61+
error: pub(crate) function inside private module
62+
--> $DIR/redundant_pub_crate.rs:42:9
63+
|
64+
LL | pub(crate) fn g() {} // already crate visible due to m2_2 and m2
65+
| ^^^^^^^^^^^^^^^^^^^^
66+
67+
error: pub(crate) function inside private module
68+
--> $DIR/redundant_pub_crate.rs:48:9
69+
|
70+
LL | pub(crate) fn g() {} // already crate visible due to m2
71+
| ^^^^^^^^^^^^^^^^^^^^
72+
73+
error: pub(crate) function inside private module
74+
--> $DIR/redundant_pub_crate.rs:60:9
75+
|
76+
LL | pub(crate) fn g() {} // private due to m3_1
77+
| ^^^^^^^^^^^^^^^^^^^^
78+
79+
error: pub(crate) function inside private module
80+
--> $DIR/redundant_pub_crate.rs:67:9
81+
|
82+
LL | pub(crate) fn g() {} // already crate visible due to m3_2
83+
| ^^^^^^^^^^^^^^^^^^^^
84+
85+
error: pub(crate) function inside private module
86+
--> $DIR/redundant_pub_crate.rs:80:5
87+
|
88+
LL | pub(crate) fn g() {} // private: not re-exported by `pub use m4::*`
89+
| ^^^^^^^^^^^^^^^^^^^^
90+
91+
error: pub(crate) function inside private module
92+
--> $DIR/redundant_pub_crate.rs:85:9
93+
|
94+
LL | pub(crate) fn g() {} // private due to m4_1
95+
| ^^^^^^^^^^^^^^^^^^^^
96+
97+
error: pub(crate) module inside private module
98+
--> $DIR/redundant_pub_crate.rs:89:5
99+
|
100+
LL | / pub(crate) mod m4_2 {
101+
LL | | // ^ private: not re-exported by `pub use m4::*`
102+
LL | | fn f() {}
103+
LL | | pub(crate) fn g() {} // private due to m4_2
104+
LL | | pub fn h() {}
105+
LL | | }
106+
| |_____^
107+
108+
error: pub(crate) function inside private module
109+
--> $DIR/redundant_pub_crate.rs:92:9
110+
|
111+
LL | pub(crate) fn g() {} // private due to m4_2
112+
| ^^^^^^^^^^^^^^^^^^^^
113+
114+
error: aborting due to 16 previous errors
115+

0 commit comments

Comments
 (0)