From de9092438df744895cfc2a2a35fcc577b657cd13 Mon Sep 17 00:00:00 2001 From: Tim Robinson Date: Fri, 20 Mar 2020 22:49:15 +0000 Subject: [PATCH] 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 4c638da80..b54d0b0c9 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 000000000..25f2fd061 --- /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 c747a07dd..616286b4f 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 ecc038c7f..6fccdaa4e 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 + --> $DIR/redundant_pub_crate.rs:16: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)` +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 + --> $DIR/redundant_pub_crate.rs:91: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)` +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