-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Lint for pub(crate)
items that are not crate visible due to the visibility of the module that contains them
#5319
Merged
Merged
Changes from 1 commit
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,67 @@ | ||
use crate::utils::span_lint_and_help; | ||
use rustc_hir::{Item, ItemKind, VisibilityKind}; | ||
use rustc_lint::{LateContext, LateLintPass}; | ||
use rustc_session::{declare_tool_lint, impl_lint_pass}; | ||
|
||
declare_clippy_lint! { | ||
/// **What it does:** Checks for items declared `pub(crate)` that are not crate visible because they | ||
/// are inside a private module. | ||
/// | ||
/// **Why is this bad?** Writing `pub(crate)` is misleading when it's redundant due to the parent | ||
/// module's visibility. | ||
/// | ||
/// **Known problems:** None. | ||
/// | ||
/// **Example:** | ||
/// | ||
/// ```rust | ||
/// mod internal { | ||
/// pub(crate) fn internal_fn() { } | ||
/// } | ||
/// ``` | ||
/// This function is not visible outside the module and it can be declared with `pub` or | ||
/// private visibility | ||
/// ```rust | ||
/// mod internal { | ||
/// pub fn internal_fn() { } | ||
/// } | ||
/// ``` | ||
pub REDUNDANT_PUB_CRATE, | ||
nursery, | ||
"Using `pub(crate)` visibility on items that are not crate visible due to the visibility of the module that contains them." | ||
} | ||
|
||
#[derive(Default)] | ||
pub struct RedundantPubCrate { | ||
is_exported: Vec<bool>, | ||
} | ||
|
||
impl_lint_pass!(RedundantPubCrate => [REDUNDANT_PUB_CRATE]); | ||
|
||
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for RedundantPubCrate { | ||
fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx Item<'tcx>) { | ||
if let VisibilityKind::Crate { .. } = item.vis.node { | ||
if !cx.access_levels.is_exported(item.hir_id) { | ||
if let Some(false) = self.is_exported.last() { | ||
span_lint_and_help( | ||
cx, | ||
REDUNDANT_PUB_CRATE, | ||
item.span, | ||
1tgr marked this conversation as resolved.
Show resolved
Hide resolved
|
||
&format!("pub(crate) {} inside private module", item.kind.descr()), | ||
"consider using `pub` instead of `pub(crate)`", | ||
1tgr marked this conversation as resolved.
Show resolved
Hide resolved
|
||
) | ||
} | ||
} | ||
} | ||
|
||
if let ItemKind::Mod { .. } = item.kind { | ||
self.is_exported.push(cx.access_levels.is_exported(item.hir_id)); | ||
} | ||
} | ||
|
||
fn check_item_post(&mut self, _cx: &LateContext<'a, 'tcx>, item: &'tcx Item<'tcx>) { | ||
if let ItemKind::Mod { .. } = item.kind { | ||
self.is_exported.pop().expect("unbalanced check_item/check_item_post"); | ||
} | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,105 @@ | ||
#![warn(clippy::redundant_pub_crate)] | ||
|
||
mod m1 { | ||
fn f() {} | ||
pub(crate) fn g() {} // private due to m1 | ||
pub fn h() {} | ||
|
||
mod m1_1 { | ||
fn f() {} | ||
pub(crate) fn g() {} // private due to m1_1 and m1 | ||
pub fn h() {} | ||
} | ||
|
||
pub(crate) mod m1_2 { | ||
// ^ private due to m1 | ||
fn f() {} | ||
pub(crate) fn g() {} // private due to m1_2 and m1 | ||
pub fn h() {} | ||
} | ||
|
||
pub mod m1_3 { | ||
fn f() {} | ||
pub(crate) fn g() {} // private due to m1 | ||
pub fn h() {} | ||
} | ||
} | ||
|
||
pub(crate) mod m2 { | ||
fn f() {} | ||
pub(crate) fn g() {} // already crate visible due to m2 | ||
pub fn h() {} | ||
|
||
mod m2_1 { | ||
fn f() {} | ||
pub(crate) fn g() {} // private due to m2_1 | ||
pub fn h() {} | ||
} | ||
|
||
pub(crate) mod m2_2 { | ||
// ^ already crate visible due to m2 | ||
fn f() {} | ||
pub(crate) fn g() {} // already crate visible due to m2_2 and m2 | ||
pub fn h() {} | ||
} | ||
|
||
pub mod m2_3 { | ||
fn f() {} | ||
pub(crate) fn g() {} // already crate visible due to m2 | ||
pub fn h() {} | ||
} | ||
} | ||
|
||
pub mod m3 { | ||
fn f() {} | ||
pub(crate) fn g() {} // ok: m3 is exported | ||
pub fn h() {} | ||
|
||
mod m3_1 { | ||
fn f() {} | ||
pub(crate) fn g() {} // private due to m3_1 | ||
pub fn h() {} | ||
} | ||
|
||
pub(crate) mod m3_2 { | ||
// ^ ok | ||
fn f() {} | ||
pub(crate) fn g() {} // already crate visible due to m3_2 | ||
pub fn h() {} | ||
} | ||
|
||
pub mod m3_3 { | ||
fn f() {} | ||
pub(crate) fn g() {} // ok: m3 and m3_3 are exported | ||
pub fn h() {} | ||
} | ||
} | ||
|
||
mod m4 { | ||
fn f() {} | ||
pub(crate) fn g() {} // private: not re-exported by `pub use m4::*` | ||
pub fn h() {} | ||
|
||
mod m4_1 { | ||
fn f() {} | ||
pub(crate) fn g() {} // private due to m4_1 | ||
pub fn h() {} | ||
} | ||
|
||
pub(crate) mod m4_2 { | ||
// ^ private: not re-exported by `pub use m4::*` | ||
fn f() {} | ||
pub(crate) fn g() {} // private due to m4_2 | ||
pub fn h() {} | ||
} | ||
|
||
pub mod m4_3 { | ||
fn f() {} | ||
pub(crate) fn g() {} // ok: m4_3 is re-exported by `pub use m4::*` | ||
pub fn h() {} | ||
} | ||
} | ||
|
||
pub use m4::*; | ||
|
||
fn main() {} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,146 @@ | ||
error: pub(crate) function inside private module | ||
--> $DIR/redundant_pub_crate.rs:5:5 | ||
| | ||
LL | pub(crate) fn g() {} // private due to m1 | ||
| ^^^^^^^^^^^^^^^^^^^^ | ||
| | ||
= note: `-D clippy::redundant-pub-crate` implied by `-D warnings` | ||
= help: consider using `pub` instead of `pub(crate)` | ||
|
||
error: pub(crate) function inside private module | ||
--> $DIR/redundant_pub_crate.rs:10:9 | ||
| | ||
LL | pub(crate) fn g() {} // private due to m1_1 and m1 | ||
| ^^^^^^^^^^^^^^^^^^^^ | ||
| | ||
= help: consider using `pub` instead of `pub(crate)` | ||
|
||
error: pub(crate) module inside private module | ||
--> $DIR/redundant_pub_crate.rs:14:5 | ||
| | ||
LL | / pub(crate) mod m1_2 { | ||
LL | | // ^ private due to m1 | ||
LL | | fn f() {} | ||
LL | | pub(crate) fn g() {} // private due to m1_2 and m1 | ||
LL | | pub fn h() {} | ||
LL | | } | ||
| |_____^ | ||
| | ||
= help: consider using `pub` instead of `pub(crate)` | ||
|
||
error: pub(crate) function inside private module | ||
--> $DIR/redundant_pub_crate.rs:17:9 | ||
| | ||
LL | pub(crate) fn g() {} // private due to m1_2 and m1 | ||
| ^^^^^^^^^^^^^^^^^^^^ | ||
| | ||
= help: consider using `pub` instead of `pub(crate)` | ||
|
||
error: pub(crate) function inside private module | ||
--> $DIR/redundant_pub_crate.rs:23:9 | ||
| | ||
LL | pub(crate) fn g() {} // private due to m1 | ||
| ^^^^^^^^^^^^^^^^^^^^ | ||
| | ||
= help: consider using `pub` instead of `pub(crate)` | ||
|
||
error: pub(crate) function inside private module | ||
--> $DIR/redundant_pub_crate.rs:30:5 | ||
| | ||
LL | pub(crate) fn g() {} // already crate visible due to m2 | ||
| ^^^^^^^^^^^^^^^^^^^^ | ||
| | ||
= help: consider using `pub` instead of `pub(crate)` | ||
|
||
error: pub(crate) function inside private module | ||
--> $DIR/redundant_pub_crate.rs:35:9 | ||
| | ||
LL | pub(crate) fn g() {} // private due to m2_1 | ||
| ^^^^^^^^^^^^^^^^^^^^ | ||
| | ||
= help: consider using `pub` instead of `pub(crate)` | ||
|
||
error: pub(crate) module inside private module | ||
--> $DIR/redundant_pub_crate.rs:39:5 | ||
| | ||
LL | / pub(crate) mod m2_2 { | ||
LL | | // ^ already crate visible due to m2 | ||
LL | | fn f() {} | ||
LL | | pub(crate) fn g() {} // already crate visible due to m2_2 and m2 | ||
LL | | pub fn h() {} | ||
LL | | } | ||
| |_____^ | ||
| | ||
= help: consider using `pub` instead of `pub(crate)` | ||
|
||
error: pub(crate) function inside private module | ||
--> $DIR/redundant_pub_crate.rs:42:9 | ||
| | ||
LL | pub(crate) fn g() {} // already crate visible due to m2_2 and m2 | ||
| ^^^^^^^^^^^^^^^^^^^^ | ||
| | ||
= help: consider using `pub` instead of `pub(crate)` | ||
|
||
error: pub(crate) function inside private module | ||
--> $DIR/redundant_pub_crate.rs:48:9 | ||
| | ||
LL | pub(crate) fn g() {} // already crate visible due to m2 | ||
| ^^^^^^^^^^^^^^^^^^^^ | ||
| | ||
= help: consider using `pub` instead of `pub(crate)` | ||
|
||
error: pub(crate) function inside private module | ||
--> $DIR/redundant_pub_crate.rs:60:9 | ||
| | ||
LL | pub(crate) fn g() {} // private due to m3_1 | ||
| ^^^^^^^^^^^^^^^^^^^^ | ||
| | ||
= help: consider using `pub` instead of `pub(crate)` | ||
|
||
error: pub(crate) function inside private module | ||
--> $DIR/redundant_pub_crate.rs:67:9 | ||
| | ||
LL | pub(crate) fn g() {} // already crate visible due to m3_2 | ||
| ^^^^^^^^^^^^^^^^^^^^ | ||
| | ||
= help: consider using `pub` instead of `pub(crate)` | ||
|
||
error: pub(crate) function inside private module | ||
--> $DIR/redundant_pub_crate.rs:80:5 | ||
| | ||
LL | pub(crate) fn g() {} // private: not re-exported by `pub use m4::*` | ||
| ^^^^^^^^^^^^^^^^^^^^ | ||
| | ||
= help: consider using `pub` instead of `pub(crate)` | ||
|
||
error: pub(crate) function inside private module | ||
--> $DIR/redundant_pub_crate.rs:85:9 | ||
| | ||
LL | pub(crate) fn g() {} // private due to m4_1 | ||
| ^^^^^^^^^^^^^^^^^^^^ | ||
| | ||
= help: consider using `pub` instead of `pub(crate)` | ||
|
||
error: pub(crate) module inside private module | ||
--> $DIR/redundant_pub_crate.rs:89:5 | ||
| | ||
LL | / pub(crate) mod m4_2 { | ||
LL | | // ^ private: not re-exported by `pub use m4::*` | ||
LL | | fn f() {} | ||
LL | | pub(crate) fn g() {} // private due to m4_2 | ||
LL | | pub fn h() {} | ||
LL | | } | ||
| |_____^ | ||
| | ||
= help: consider using `pub` instead of `pub(crate)` | ||
|
||
error: pub(crate) function inside private module | ||
--> $DIR/redundant_pub_crate.rs:92:9 | ||
| | ||
LL | pub(crate) fn g() {} // private due to m4_2 | ||
| ^^^^^^^^^^^^^^^^^^^^ | ||
| | ||
= help: consider using `pub` instead of `pub(crate)` | ||
|
||
error: aborting due to 16 previous errors | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can put it in the
style
group.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You have to run
cargo dev update_lints
after changing the group.