From 52208f3cf3949a4589cbaee69a9cb7d34ee0e9f2 Mon Sep 17 00:00:00 2001 From: Tim Robinson Date: Sun, 15 Mar 2020 17:38:20 +0000 Subject: [PATCH 1/3] Lint for `pub(crate)` items that are not crate visible due to the visibility of the module that contains them Closes #5274. --- CHANGELOG.md | 1 + README.md | 2 +- clippy_lints/src/lib.rs | 4 + clippy_lints/src/redundant_pub_crate.rs | 67 +++++++++++ src/lintlist/mod.rs | 9 +- tests/ui/redundant_pub_crate.rs | 105 +++++++++++++++++ tests/ui/redundant_pub_crate.stderr | 146 ++++++++++++++++++++++++ 7 files changed, 332 insertions(+), 2 deletions(-) create mode 100644 clippy_lints/src/redundant_pub_crate.rs create mode 100644 tests/ui/redundant_pub_crate.rs create mode 100644 tests/ui/redundant_pub_crate.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index c8d41e3b31d2..9fa2dcad4a84 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1410,6 +1410,7 @@ Released 2018-09-13 [`redundant_field_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_field_names [`redundant_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_pattern [`redundant_pattern_matching`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_pattern_matching +[`redundant_pub_crate`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_pub_crate [`redundant_static_lifetimes`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_static_lifetimes [`ref_in_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#ref_in_deref [`regex_macro`]: https://rust-lang.github.io/rust-clippy/master/index.html#regex_macro diff --git a/README.md b/README.md index 2181c296e9b2..7d6fcbc90986 100644 --- a/README.md +++ b/README.md @@ -5,7 +5,7 @@ A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code. -[There are 361 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) +[There are 362 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you: diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 791307ba43f4..601bbdce7047 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -284,6 +284,7 @@ pub mod ranges; pub mod redundant_clone; pub mod redundant_field_names; pub mod redundant_pattern_matching; +pub mod redundant_pub_crate; pub mod redundant_static_lifetimes; pub mod reference; pub mod regex; @@ -744,6 +745,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &redundant_clone::REDUNDANT_CLONE, &redundant_field_names::REDUNDANT_FIELD_NAMES, &redundant_pattern_matching::REDUNDANT_PATTERN_MATCHING, + &redundant_pub_crate::REDUNDANT_PUB_CRATE, &redundant_static_lifetimes::REDUNDANT_STATIC_LIFETIMES, &reference::DEREF_ADDROF, &reference::REF_IN_DEREF, @@ -1020,6 +1022,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| box wildcard_imports::WildcardImports); store.register_early_pass(|| box macro_use::MacroUseImports); store.register_late_pass(|| box verbose_file_reads::VerboseFileReads); + store.register_late_pass(|| box redundant_pub_crate::RedundantPubCrate::default()); store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![ LintId::of(&arithmetic::FLOAT_ARITHMETIC), @@ -1669,6 +1672,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&mutex_atomic::MUTEX_INTEGER), LintId::of(&needless_borrow::NEEDLESS_BORROW), LintId::of(&path_buf_push_overwrite::PATH_BUF_PUSH_OVERWRITE), + LintId::of(&redundant_pub_crate::REDUNDANT_PUB_CRATE), LintId::of(&use_self::USE_SELF), ]); } diff --git a/clippy_lints/src/redundant_pub_crate.rs b/clippy_lints/src/redundant_pub_crate.rs new file mode 100644 index 000000000000..4c638da80a86 --- /dev/null +++ b/clippy_lints/src/redundant_pub_crate.rs @@ -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, +} + +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, + &format!("pub(crate) {} inside private module", item.kind.descr()), + "consider using `pub` instead of `pub(crate)`", + ) + } + } + } + + 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"); + } + } +} diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index fd948953ea23..29d9c8b76d08 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -6,7 +6,7 @@ pub use lint::Lint; pub use lint::LINT_LEVELS; // begin lint list, do not remove this comment, it’s used in `update_lints` -pub const ALL_LINTS: [Lint; 361] = [ +pub const ALL_LINTS: [Lint; 362] = [ Lint { name: "absurd_extreme_comparisons", group: "correctness", @@ -1764,6 +1764,13 @@ pub const ALL_LINTS: [Lint; 361] = [ deprecation: None, module: "redundant_pattern_matching", }, + Lint { + name: "redundant_pub_crate", + group: "nursery", + desc: "Using `pub(crate)` visibility on items that are not crate visible due to the visibility of the module that contains them.", + deprecation: None, + module: "redundant_pub_crate", + }, Lint { name: "redundant_static_lifetimes", group: "style", diff --git a/tests/ui/redundant_pub_crate.rs b/tests/ui/redundant_pub_crate.rs new file mode 100644 index 000000000000..c747a07dd90f --- /dev/null +++ b/tests/ui/redundant_pub_crate.rs @@ -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() {} diff --git a/tests/ui/redundant_pub_crate.stderr b/tests/ui/redundant_pub_crate.stderr new file mode 100644 index 000000000000..ecc038c7f4ed --- /dev/null +++ b/tests/ui/redundant_pub_crate.stderr @@ -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 + From de9092438df744895cfc2a2a35fcc577b657cd13 Mon Sep 17 00:00:00 2001 From: Tim Robinson Date: Fri, 20 Mar 2020 22:49:15 +0000 Subject: [PATCH 2/3] Update for PR feedback --- clippy_lints/src/redundant_pub_crate.rs | 17 ++- tests/ui/redundant_pub_crate.fixed | 107 +++++++++++++++++ tests/ui/redundant_pub_crate.rs | 2 + tests/ui/redundant_pub_crate.stderr | 148 +++++++++++------------- 4 files changed, 189 insertions(+), 85 deletions(-) create mode 100644 tests/ui/redundant_pub_crate.fixed diff --git a/clippy_lints/src/redundant_pub_crate.rs b/clippy_lints/src/redundant_pub_crate.rs index 4c638da80a86..b54d0b0c9a13 100644 --- a/clippy_lints/src/redundant_pub_crate.rs +++ b/clippy_lints/src/redundant_pub_crate.rs @@ -1,4 +1,5 @@ -use crate::utils::span_lint_and_help; +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}; @@ -43,12 +44,20 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for RedundantPubCrate { 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( + let span = item.span.with_hi(item.ident.span.hi()); + span_lint_and_then( cx, REDUNDANT_PUB_CRATE, - item.span, + span, &format!("pub(crate) {} inside private module", item.kind.descr()), - "consider using `pub` instead of `pub(crate)`", + |db| { + db.span_suggestion( + item.vis.span, + "consider using", + "pub".to_string(), + Applicability::MachineApplicable, + ); + }, ) } } diff --git a/tests/ui/redundant_pub_crate.fixed b/tests/ui/redundant_pub_crate.fixed new file mode 100644 index 000000000000..25f2fd061b88 --- /dev/null +++ b/tests/ui/redundant_pub_crate.fixed @@ -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() {} diff --git a/tests/ui/redundant_pub_crate.rs b/tests/ui/redundant_pub_crate.rs index c747a07dd90f..616286b4f39f 100644 --- a/tests/ui/redundant_pub_crate.rs +++ b/tests/ui/redundant_pub_crate.rs @@ -1,3 +1,5 @@ +// run-rustfix +#![allow(dead_code)] #![warn(clippy::redundant_pub_crate)] mod m1 { diff --git a/tests/ui/redundant_pub_crate.stderr b/tests/ui/redundant_pub_crate.stderr index ecc038c7f4ed..6fccdaa4e203 100644 --- a/tests/ui/redundant_pub_crate.stderr +++ b/tests/ui/redundant_pub_crate.stderr @@ -1,146 +1,132 @@ error: pub(crate) function inside private module - --> $DIR/redundant_pub_crate.rs:5:5 + --> $DIR/redundant_pub_crate.rs:7:5 | LL | pub(crate) fn g() {} // private due to m1 - | ^^^^^^^^^^^^^^^^^^^^ + | ----------^^^^^ + | | + | help: consider using: `pub` | = 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 + --> $DIR/redundant_pub_crate.rs:12:9 | LL | pub(crate) fn g() {} // private due to m1_1 and m1 - | ^^^^^^^^^^^^^^^^^^^^ - | - = help: consider using `pub` instead of `pub(crate)` + | ----------^^^^^ + | | + | help: consider using: `pub` 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 | | } - | |_____^ + --> $DIR/redundant_pub_crate.rs:16:5 | - = help: consider using `pub` instead of `pub(crate)` +LL | pub(crate) mod m1_2 { + | ----------^^^^^^^^^ + | | + | help: consider using: `pub` error: pub(crate) function inside private module - --> $DIR/redundant_pub_crate.rs:17:9 + --> $DIR/redundant_pub_crate.rs:19:9 | LL | pub(crate) fn g() {} // private due to m1_2 and m1 - | ^^^^^^^^^^^^^^^^^^^^ - | - = help: consider using `pub` instead of `pub(crate)` + | ----------^^^^^ + | | + | help: consider using: `pub` error: pub(crate) function inside private module - --> $DIR/redundant_pub_crate.rs:23:9 + --> $DIR/redundant_pub_crate.rs:25:9 | LL | pub(crate) fn g() {} // private due to m1 - | ^^^^^^^^^^^^^^^^^^^^ - | - = help: consider using `pub` instead of `pub(crate)` + | ----------^^^^^ + | | + | help: consider using: `pub` error: pub(crate) function inside private module - --> $DIR/redundant_pub_crate.rs:30:5 + --> $DIR/redundant_pub_crate.rs:32:5 | LL | pub(crate) fn g() {} // already crate visible due to m2 - | ^^^^^^^^^^^^^^^^^^^^ - | - = help: consider using `pub` instead of `pub(crate)` + | ----------^^^^^ + | | + | help: consider using: `pub` error: pub(crate) function inside private module - --> $DIR/redundant_pub_crate.rs:35:9 + --> $DIR/redundant_pub_crate.rs:37:9 | LL | pub(crate) fn g() {} // private due to m2_1 - | ^^^^^^^^^^^^^^^^^^^^ - | - = help: consider using `pub` instead of `pub(crate)` + | ----------^^^^^ + | | + | help: consider using: `pub` error: pub(crate) module inside private module - --> $DIR/redundant_pub_crate.rs:39:5 + --> $DIR/redundant_pub_crate.rs:41: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)` +LL | pub(crate) mod m2_2 { + | ----------^^^^^^^^^ + | | + | help: consider using: `pub` error: pub(crate) function inside private module - --> $DIR/redundant_pub_crate.rs:42:9 + --> $DIR/redundant_pub_crate.rs:44:9 | LL | pub(crate) fn g() {} // already crate visible due to m2_2 and m2 - | ^^^^^^^^^^^^^^^^^^^^ - | - = help: consider using `pub` instead of `pub(crate)` + | ----------^^^^^ + | | + | help: consider using: `pub` error: pub(crate) function inside private module - --> $DIR/redundant_pub_crate.rs:48:9 + --> $DIR/redundant_pub_crate.rs:50:9 | LL | pub(crate) fn g() {} // already crate visible due to m2 - | ^^^^^^^^^^^^^^^^^^^^ - | - = help: consider using `pub` instead of `pub(crate)` + | ----------^^^^^ + | | + | help: consider using: `pub` error: pub(crate) function inside private module - --> $DIR/redundant_pub_crate.rs:60:9 + --> $DIR/redundant_pub_crate.rs:62:9 | LL | pub(crate) fn g() {} // private due to m3_1 - | ^^^^^^^^^^^^^^^^^^^^ - | - = help: consider using `pub` instead of `pub(crate)` + | ----------^^^^^ + | | + | help: consider using: `pub` error: pub(crate) function inside private module - --> $DIR/redundant_pub_crate.rs:67:9 + --> $DIR/redundant_pub_crate.rs:69:9 | LL | pub(crate) fn g() {} // already crate visible due to m3_2 - | ^^^^^^^^^^^^^^^^^^^^ - | - = help: consider using `pub` instead of `pub(crate)` + | ----------^^^^^ + | | + | help: consider using: `pub` error: pub(crate) function inside private module - --> $DIR/redundant_pub_crate.rs:80:5 + --> $DIR/redundant_pub_crate.rs:82:5 | LL | pub(crate) fn g() {} // private: not re-exported by `pub use m4::*` - | ^^^^^^^^^^^^^^^^^^^^ - | - = help: consider using `pub` instead of `pub(crate)` + | ----------^^^^^ + | | + | help: consider using: `pub` error: pub(crate) function inside private module - --> $DIR/redundant_pub_crate.rs:85:9 + --> $DIR/redundant_pub_crate.rs:87:9 | LL | pub(crate) fn g() {} // private due to m4_1 - | ^^^^^^^^^^^^^^^^^^^^ - | - = help: consider using `pub` instead of `pub(crate)` + | ----------^^^^^ + | | + | help: consider using: `pub` 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 | | } - | |_____^ + --> $DIR/redundant_pub_crate.rs:91:5 | - = help: consider using `pub` instead of `pub(crate)` +LL | pub(crate) mod m4_2 { + | ----------^^^^^^^^^ + | | + | help: consider using: `pub` error: pub(crate) function inside private module - --> $DIR/redundant_pub_crate.rs:92:9 + --> $DIR/redundant_pub_crate.rs:94:9 | LL | pub(crate) fn g() {} // private due to m4_2 - | ^^^^^^^^^^^^^^^^^^^^ - | - = help: consider using `pub` instead of `pub(crate)` + | ----------^^^^^ + | | + | help: consider using: `pub` error: aborting due to 16 previous errors From 870b9e8139052f7cb1ef1e5717f2f4ce523fb688 Mon Sep 17 00:00:00 2001 From: Tim Robinson Date: Mon, 23 Mar 2020 16:06:15 +0000 Subject: [PATCH 3/3] nursery group -> style --- clippy_lints/src/lib.rs | 5 ++-- clippy_lints/src/redundant_pub_crate.rs | 2 +- clippy_lints/src/utils/conf.rs | 2 +- clippy_lints/src/utils/numeric_literal.rs | 4 +-- src/lintlist/mod.rs | 2 +- tests/ui/wildcard_imports.fixed | 1 + tests/ui/wildcard_imports.rs | 1 + tests/ui/wildcard_imports.stderr | 30 +++++++++++------------ 8 files changed, 25 insertions(+), 22 deletions(-) diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 601bbdce7047..33329acb8327 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -323,7 +323,7 @@ pub mod zero_div_zero; pub use crate::utils::conf::Conf; mod reexport { - crate use rustc_ast::ast::Name; + pub use rustc_ast::ast::Name; } /// Register all pre expansion lints @@ -1324,6 +1324,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&redundant_clone::REDUNDANT_CLONE), LintId::of(&redundant_field_names::REDUNDANT_FIELD_NAMES), LintId::of(&redundant_pattern_matching::REDUNDANT_PATTERN_MATCHING), + LintId::of(&redundant_pub_crate::REDUNDANT_PUB_CRATE), LintId::of(&redundant_static_lifetimes::REDUNDANT_STATIC_LIFETIMES), LintId::of(&reference::DEREF_ADDROF), LintId::of(&reference::REF_IN_DEREF), @@ -1465,6 +1466,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&question_mark::QUESTION_MARK), LintId::of(&redundant_field_names::REDUNDANT_FIELD_NAMES), LintId::of(&redundant_pattern_matching::REDUNDANT_PATTERN_MATCHING), + LintId::of(&redundant_pub_crate::REDUNDANT_PUB_CRATE), LintId::of(&redundant_static_lifetimes::REDUNDANT_STATIC_LIFETIMES), LintId::of(®ex::REGEX_MACRO), LintId::of(®ex::TRIVIAL_REGEX), @@ -1672,7 +1674,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&mutex_atomic::MUTEX_INTEGER), LintId::of(&needless_borrow::NEEDLESS_BORROW), LintId::of(&path_buf_push_overwrite::PATH_BUF_PUSH_OVERWRITE), - LintId::of(&redundant_pub_crate::REDUNDANT_PUB_CRATE), LintId::of(&use_self::USE_SELF), ]); } diff --git a/clippy_lints/src/redundant_pub_crate.rs b/clippy_lints/src/redundant_pub_crate.rs index b54d0b0c9a13..af4ab367f5fa 100644 --- a/clippy_lints/src/redundant_pub_crate.rs +++ b/clippy_lints/src/redundant_pub_crate.rs @@ -28,7 +28,7 @@ declare_clippy_lint! { /// } /// ``` pub REDUNDANT_PUB_CRATE, - nursery, + style, "Using `pub(crate)` visibility on items that are not crate visible due to the visibility of the module that contains them." } diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs index a1d7bebc7b56..722104e5b524 100644 --- a/clippy_lints/src/utils/conf.rs +++ b/clippy_lints/src/utils/conf.rs @@ -80,7 +80,7 @@ macro_rules! define_Conf { $( mod $config { use serde::Deserialize; - crate fn deserialize<'de, D: serde::Deserializer<'de>>(deserializer: D) -> Result<$Ty, D::Error> { + pub fn deserialize<'de, D: serde::Deserializer<'de>>(deserializer: D) -> Result<$Ty, D::Error> { use super::super::{ERRORS, Error}; Ok( <$Ty>::deserialize(deserializer).unwrap_or_else(|e| { diff --git a/clippy_lints/src/utils/numeric_literal.rs b/clippy_lints/src/utils/numeric_literal.rs index 8b3492724e10..99413153d49b 100644 --- a/clippy_lints/src/utils/numeric_literal.rs +++ b/clippy_lints/src/utils/numeric_literal.rs @@ -1,7 +1,7 @@ use rustc_ast::ast::{Lit, LitFloatType, LitIntType, LitKind}; #[derive(Debug, PartialEq)] -pub(crate) enum Radix { +pub enum Radix { Binary, Octal, Decimal, @@ -26,7 +26,7 @@ pub fn format(lit: &str, type_suffix: Option<&str>, float: bool) -> String { } #[derive(Debug)] -pub(crate) struct NumericLiteral<'a> { +pub struct NumericLiteral<'a> { /// Which radix the literal was represented in. pub radix: Radix, /// The radix prefix, if present. diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index 29d9c8b76d08..50d5c8819528 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -1766,7 +1766,7 @@ pub const ALL_LINTS: [Lint; 362] = [ }, Lint { name: "redundant_pub_crate", - group: "nursery", + group: "style", desc: "Using `pub(crate)` visibility on items that are not crate visible due to the visibility of the module that contains them.", deprecation: None, module: "redundant_pub_crate", diff --git a/tests/ui/wildcard_imports.fixed b/tests/ui/wildcard_imports.fixed index f447a92715dc..69ab6e1f8c14 100644 --- a/tests/ui/wildcard_imports.fixed +++ b/tests/ui/wildcard_imports.fixed @@ -2,6 +2,7 @@ // aux-build:wildcard_imports_helper.rs #![warn(clippy::wildcard_imports)] +#![allow(clippy::redundant_pub_crate)] #![allow(unused)] #![warn(unused_imports)] diff --git a/tests/ui/wildcard_imports.rs b/tests/ui/wildcard_imports.rs index 3fd66763a9fc..e64ac4ce5190 100644 --- a/tests/ui/wildcard_imports.rs +++ b/tests/ui/wildcard_imports.rs @@ -2,6 +2,7 @@ // aux-build:wildcard_imports_helper.rs #![warn(clippy::wildcard_imports)] +#![allow(clippy::redundant_pub_crate)] #![allow(unused)] #![warn(unused_imports)] diff --git a/tests/ui/wildcard_imports.stderr b/tests/ui/wildcard_imports.stderr index bebd9c1f8520..050e4c6304f0 100644 --- a/tests/ui/wildcard_imports.stderr +++ b/tests/ui/wildcard_imports.stderr @@ -1,5 +1,5 @@ error: usage of wildcard import - --> $DIR/wildcard_imports.rs:10:5 + --> $DIR/wildcard_imports.rs:11:5 | LL | use crate::fn_mod::*; | ^^^^^^^^^^^^^^^^ help: try: `crate::fn_mod::foo` @@ -7,85 +7,85 @@ LL | use crate::fn_mod::*; = note: `-D clippy::wildcard-imports` implied by `-D warnings` error: usage of wildcard import - --> $DIR/wildcard_imports.rs:11:5 + --> $DIR/wildcard_imports.rs:12:5 | LL | use crate::mod_mod::*; | ^^^^^^^^^^^^^^^^^ help: try: `crate::mod_mod::inner_mod` error: usage of wildcard import - --> $DIR/wildcard_imports.rs:12:5 + --> $DIR/wildcard_imports.rs:13:5 | LL | use crate::multi_fn_mod::*; | ^^^^^^^^^^^^^^^^^^^^^^ help: try: `crate::multi_fn_mod::{multi_bar, multi_foo, multi_inner_mod}` error: usage of wildcard import - --> $DIR/wildcard_imports.rs:14:5 + --> $DIR/wildcard_imports.rs:15:5 | LL | use crate::struct_mod::*; | ^^^^^^^^^^^^^^^^^^^^ help: try: `crate::struct_mod::{A, inner_struct_mod}` error: usage of wildcard import - --> $DIR/wildcard_imports.rs:18:5 + --> $DIR/wildcard_imports.rs:19:5 | LL | use wildcard_imports_helper::inner::inner_for_self_import::*; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `wildcard_imports_helper::inner::inner_for_self_import::inner_extern_bar` error: usage of wildcard import - --> $DIR/wildcard_imports.rs:19:5 + --> $DIR/wildcard_imports.rs:20:5 | LL | use wildcard_imports_helper::*; | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `wildcard_imports_helper::{ExternA, extern_foo}` error: usage of wildcard import - --> $DIR/wildcard_imports.rs:88:13 + --> $DIR/wildcard_imports.rs:89:13 | LL | use crate::fn_mod::*; | ^^^^^^^^^^^^^^^^ help: try: `crate::fn_mod::foo` error: usage of wildcard import - --> $DIR/wildcard_imports.rs:94:75 + --> $DIR/wildcard_imports.rs:95:75 | LL | use wildcard_imports_helper::inner::inner_for_self_import::{self, *}; | ^ help: try: `inner_extern_foo` error: usage of wildcard import - --> $DIR/wildcard_imports.rs:95:13 + --> $DIR/wildcard_imports.rs:96:13 | LL | use wildcard_imports_helper::*; | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `wildcard_imports_helper::{ExternA, extern_foo}` error: usage of wildcard import - --> $DIR/wildcard_imports.rs:106:20 + --> $DIR/wildcard_imports.rs:107:20 | LL | use self::{inner::*, inner2::*}; | ^^^^^^^^ help: try: `inner::inner_foo` error: usage of wildcard import - --> $DIR/wildcard_imports.rs:106:30 + --> $DIR/wildcard_imports.rs:107:30 | LL | use self::{inner::*, inner2::*}; | ^^^^^^^^^ help: try: `inner2::inner_bar` error: usage of wildcard import - --> $DIR/wildcard_imports.rs:113:13 + --> $DIR/wildcard_imports.rs:114:13 | LL | use wildcard_imports_helper::*; | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `wildcard_imports_helper::{ExternExportedEnum, ExternExportedStruct, extern_exported}` error: usage of wildcard import - --> $DIR/wildcard_imports.rs:142:9 + --> $DIR/wildcard_imports.rs:143:9 | LL | use crate::in_fn_test::*; | ^^^^^^^^^^^^^^^^^^^^ help: try: `crate::in_fn_test::{ExportedEnum, ExportedStruct, exported}` error: usage of wildcard import - --> $DIR/wildcard_imports.rs:151:9 + --> $DIR/wildcard_imports.rs:152:9 | LL | use crate:: in_fn_test:: * ; | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `crate:: in_fn_test::exported` error: usage of wildcard import - --> $DIR/wildcard_imports.rs:152:9 + --> $DIR/wildcard_imports.rs:153:9 | LL | use crate:: fn_mod:: | _________^