Skip to content

Commit 52208f3

Browse files
1tgr1tgr
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 d556bb7 commit 52208f3

File tree

7 files changed

+332
-2
lines changed

7 files changed

+332
-2
lines changed

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -1410,6 +1410,7 @@ Released 2018-09-13
14101410
[`redundant_field_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_field_names
14111411
[`redundant_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_pattern
14121412
[`redundant_pattern_matching`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_pattern_matching
1413+
[`redundant_pub_crate`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_pub_crate
14131414
[`redundant_static_lifetimes`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_static_lifetimes
14141415
[`ref_in_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#ref_in_deref
14151416
[`regex_macro`]: https://rust-lang.github.io/rust-clippy/master/index.html#regex_macro

‎README.md

+1-1
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

+4
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,7 @@ pub mod ranges;
284284
pub mod redundant_clone;
285285
pub mod redundant_field_names;
286286
pub mod redundant_pattern_matching;
287+
pub mod redundant_pub_crate;
287288
pub mod redundant_static_lifetimes;
288289
pub mod reference;
289290
pub mod regex;
@@ -744,6 +745,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
744745
&redundant_clone::REDUNDANT_CLONE,
745746
&redundant_field_names::REDUNDANT_FIELD_NAMES,
746747
&redundant_pattern_matching::REDUNDANT_PATTERN_MATCHING,
748+
&redundant_pub_crate::REDUNDANT_PUB_CRATE,
747749
&redundant_static_lifetimes::REDUNDANT_STATIC_LIFETIMES,
748750
&reference::DEREF_ADDROF,
749751
&reference::REF_IN_DEREF,
@@ -1020,6 +1022,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
10201022
store.register_late_pass(|| box wildcard_imports::WildcardImports);
10211023
store.register_early_pass(|| box macro_use::MacroUseImports);
10221024
store.register_late_pass(|| box verbose_file_reads::VerboseFileReads);
1025+
store.register_late_pass(|| box redundant_pub_crate::RedundantPubCrate::default());
10231026

10241027
store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![
10251028
LintId::of(&arithmetic::FLOAT_ARITHMETIC),
@@ -1669,6 +1672,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
16691672
LintId::of(&mutex_atomic::MUTEX_INTEGER),
16701673
LintId::of(&needless_borrow::NEEDLESS_BORROW),
16711674
LintId::of(&path_buf_push_overwrite::PATH_BUF_PUSH_OVERWRITE),
1675+
LintId::of(&redundant_pub_crate::REDUNDANT_PUB_CRATE),
16721676
LintId::of(&use_self::USE_SELF),
16731677
]);
16741678
}

‎clippy_lints/src/redundant_pub_crate.rs

+67
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
use crate::utils::span_lint_and_help;
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<'a, 'tcx> LateLintPass<'a, 'tcx> for RedundantPubCrate {
42+
fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx Item<'tcx>) {
43+
if let VisibilityKind::Crate { .. } = item.vis.node {
44+
if !cx.access_levels.is_exported(item.hir_id) {
45+
if let Some(false) = self.is_exported.last() {
46+
span_lint_and_help(
47+
cx,
48+
REDUNDANT_PUB_CRATE,
49+
item.span,
50+
&format!("pub(crate) {} inside private module", item.kind.descr()),
51+
"consider using `pub` instead of `pub(crate)`",
52+
)
53+
}
54+
}
55+
}
56+
57+
if let ItemKind::Mod { .. } = item.kind {
58+
self.is_exported.push(cx.access_levels.is_exported(item.hir_id));
59+
}
60+
}
61+
62+
fn check_item_post(&mut self, _cx: &LateContext<'a, 'tcx>, item: &'tcx Item<'tcx>) {
63+
if let ItemKind::Mod { .. } = item.kind {
64+
self.is_exported.pop().expect("unbalanced check_item/check_item_post");
65+
}
66+
}
67+
}

‎src/lintlist/mod.rs

+8-1
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

+105
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

+146
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
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+
= help: consider using `pub` instead of `pub(crate)`
9+
10+
error: pub(crate) function inside private module
11+
--> $DIR/redundant_pub_crate.rs:10:9
12+
|
13+
LL | pub(crate) fn g() {} // private due to m1_1 and m1
14+
| ^^^^^^^^^^^^^^^^^^^^
15+
|
16+
= help: consider using `pub` instead of `pub(crate)`
17+
18+
error: pub(crate) module inside private module
19+
--> $DIR/redundant_pub_crate.rs:14:5
20+
|
21+
LL | / pub(crate) mod m1_2 {
22+
LL | | // ^ private due to m1
23+
LL | | fn f() {}
24+
LL | | pub(crate) fn g() {} // private due to m1_2 and m1
25+
LL | | pub fn h() {}
26+
LL | | }
27+
| |_____^
28+
|
29+
= help: consider using `pub` instead of `pub(crate)`
30+
31+
error: pub(crate) function inside private module
32+
--> $DIR/redundant_pub_crate.rs:17:9
33+
|
34+
LL | pub(crate) fn g() {} // private due to m1_2 and m1
35+
| ^^^^^^^^^^^^^^^^^^^^
36+
|
37+
= help: consider using `pub` instead of `pub(crate)`
38+
39+
error: pub(crate) function inside private module
40+
--> $DIR/redundant_pub_crate.rs:23:9
41+
|
42+
LL | pub(crate) fn g() {} // private due to m1
43+
| ^^^^^^^^^^^^^^^^^^^^
44+
|
45+
= help: consider using `pub` instead of `pub(crate)`
46+
47+
error: pub(crate) function inside private module
48+
--> $DIR/redundant_pub_crate.rs:30:5
49+
|
50+
LL | pub(crate) fn g() {} // already crate visible due to m2
51+
| ^^^^^^^^^^^^^^^^^^^^
52+
|
53+
= help: consider using `pub` instead of `pub(crate)`
54+
55+
error: pub(crate) function inside private module
56+
--> $DIR/redundant_pub_crate.rs:35:9
57+
|
58+
LL | pub(crate) fn g() {} // private due to m2_1
59+
| ^^^^^^^^^^^^^^^^^^^^
60+
|
61+
= help: consider using `pub` instead of `pub(crate)`
62+
63+
error: pub(crate) module inside private module
64+
--> $DIR/redundant_pub_crate.rs:39:5
65+
|
66+
LL | / pub(crate) mod m2_2 {
67+
LL | | // ^ already crate visible due to m2
68+
LL | | fn f() {}
69+
LL | | pub(crate) fn g() {} // already crate visible due to m2_2 and m2
70+
LL | | pub fn h() {}
71+
LL | | }
72+
| |_____^
73+
|
74+
= help: consider using `pub` instead of `pub(crate)`
75+
76+
error: pub(crate) function inside private module
77+
--> $DIR/redundant_pub_crate.rs:42:9
78+
|
79+
LL | pub(crate) fn g() {} // already crate visible due to m2_2 and m2
80+
| ^^^^^^^^^^^^^^^^^^^^
81+
|
82+
= help: consider using `pub` instead of `pub(crate)`
83+
84+
error: pub(crate) function inside private module
85+
--> $DIR/redundant_pub_crate.rs:48:9
86+
|
87+
LL | pub(crate) fn g() {} // already crate visible due to m2
88+
| ^^^^^^^^^^^^^^^^^^^^
89+
|
90+
= help: consider using `pub` instead of `pub(crate)`
91+
92+
error: pub(crate) function inside private module
93+
--> $DIR/redundant_pub_crate.rs:60:9
94+
|
95+
LL | pub(crate) fn g() {} // private due to m3_1
96+
| ^^^^^^^^^^^^^^^^^^^^
97+
|
98+
= help: consider using `pub` instead of `pub(crate)`
99+
100+
error: pub(crate) function inside private module
101+
--> $DIR/redundant_pub_crate.rs:67:9
102+
|
103+
LL | pub(crate) fn g() {} // already crate visible due to m3_2
104+
| ^^^^^^^^^^^^^^^^^^^^
105+
|
106+
= help: consider using `pub` instead of `pub(crate)`
107+
108+
error: pub(crate) function inside private module
109+
--> $DIR/redundant_pub_crate.rs:80:5
110+
|
111+
LL | pub(crate) fn g() {} // private: not re-exported by `pub use m4::*`
112+
| ^^^^^^^^^^^^^^^^^^^^
113+
|
114+
= help: consider using `pub` instead of `pub(crate)`
115+
116+
error: pub(crate) function inside private module
117+
--> $DIR/redundant_pub_crate.rs:85:9
118+
|
119+
LL | pub(crate) fn g() {} // private due to m4_1
120+
| ^^^^^^^^^^^^^^^^^^^^
121+
|
122+
= help: consider using `pub` instead of `pub(crate)`
123+
124+
error: pub(crate) module inside private module
125+
--> $DIR/redundant_pub_crate.rs:89:5
126+
|
127+
LL | / pub(crate) mod m4_2 {
128+
LL | | // ^ private: not re-exported by `pub use m4::*`
129+
LL | | fn f() {}
130+
LL | | pub(crate) fn g() {} // private due to m4_2
131+
LL | | pub fn h() {}
132+
LL | | }
133+
| |_____^
134+
|
135+
= help: consider using `pub` instead of `pub(crate)`
136+
137+
error: pub(crate) function inside private module
138+
--> $DIR/redundant_pub_crate.rs:92:9
139+
|
140+
LL | pub(crate) fn g() {} // private due to m4_2
141+
| ^^^^^^^^^^^^^^^^^^^^
142+
|
143+
= help: consider using `pub` instead of `pub(crate)`
144+
145+
error: aborting due to 16 previous errors
146+

0 commit comments

Comments
 (0)