-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Auto merge of #5319 - 1tgr:master, r=flip1995
Lint for `pub(crate)` items that are not crate visible due to the visibility of the module that contains them changelog: Add `redundant_pub_crate` lint Closes #5274.
- Loading branch information
Showing
13 changed files
with
458 additions
and
21 deletions.
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,76 @@ | ||
| use crate::utils::span_lint_and_then; | ||
| use rustc_errors::Applicability; | ||
| 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, | ||
| style, | ||
| "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() { | ||
| let span = item.span.with_hi(item.ident.span.hi()); | ||
| span_lint_and_then( | ||
| cx, | ||
| REDUNDANT_PUB_CRATE, | ||
| span, | ||
| &format!("pub(crate) {} inside private module", item.kind.descr()), | ||
| |db| { | ||
| db.span_suggestion( | ||
| item.vis.span, | ||
| "consider using", | ||
| "pub".to_string(), | ||
| Applicability::MachineApplicable, | ||
| ); | ||
| }, | ||
| ) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| 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
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,107 @@ | ||
| // run-rustfix | ||
| #![allow(dead_code)] | ||
| #![warn(clippy::redundant_pub_crate)] | ||
|
|
||
| mod m1 { | ||
| fn f() {} | ||
| pub fn g() {} // private due to m1 | ||
| pub fn h() {} | ||
|
|
||
| mod m1_1 { | ||
| fn f() {} | ||
| pub fn g() {} // private due to m1_1 and m1 | ||
| pub fn h() {} | ||
| } | ||
|
|
||
| pub mod m1_2 { | ||
| // ^ private due to m1 | ||
| fn f() {} | ||
| pub fn g() {} // private due to m1_2 and m1 | ||
| pub fn h() {} | ||
| } | ||
|
|
||
| pub mod m1_3 { | ||
| fn f() {} | ||
| pub fn g() {} // private due to m1 | ||
| pub fn h() {} | ||
| } | ||
| } | ||
|
|
||
| pub(crate) mod m2 { | ||
| fn f() {} | ||
| pub fn g() {} // already crate visible due to m2 | ||
| pub fn h() {} | ||
|
|
||
| mod m2_1 { | ||
| fn f() {} | ||
| pub fn g() {} // private due to m2_1 | ||
| pub fn h() {} | ||
| } | ||
|
|
||
| pub mod m2_2 { | ||
| // ^ already crate visible due to m2 | ||
| fn f() {} | ||
| pub fn g() {} // already crate visible due to m2_2 and m2 | ||
| pub fn h() {} | ||
| } | ||
|
|
||
| pub mod m2_3 { | ||
| fn f() {} | ||
| pub 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 fn g() {} // private due to m3_1 | ||
| pub fn h() {} | ||
| } | ||
|
|
||
| pub(crate) mod m3_2 { | ||
| // ^ ok | ||
| fn f() {} | ||
| pub 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 fn g() {} // private: not re-exported by `pub use m4::*` | ||
| pub fn h() {} | ||
|
|
||
| mod m4_1 { | ||
| fn f() {} | ||
| pub fn g() {} // private due to m4_1 | ||
| pub fn h() {} | ||
| } | ||
|
|
||
| pub mod m4_2 { | ||
| // ^ private: not re-exported by `pub use m4::*` | ||
| fn f() {} | ||
| pub 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() {} |
Oops, something went wrong.