From 0bc622d251f8606ca86d4a5ccdbedd52afef247a Mon Sep 17 00:00:00 2001 From: Urgau Date: Thu, 31 Oct 2024 23:03:09 +0100 Subject: [PATCH] Prefer `pub(super)` in `unreachable_pub` lint suggestion --- compiler/rustc_lint/src/builtin.rs | 18 ++++ compiler/rustc_lint/src/lints.rs | 3 +- tests/ui/lint/unreachable_pub.fixed | 116 ++++++++++++++++++++++ tests/ui/lint/unreachable_pub.rs | 46 +++++++++ tests/ui/lint/unreachable_pub.stderr | 140 ++++++++++++++++++++++++--- 5 files changed, 307 insertions(+), 16 deletions(-) create mode 100644 tests/ui/lint/unreachable_pub.fixed diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs index 02d22ee49bd60..130f3cb7c2a78 100644 --- a/compiler/rustc_lint/src/builtin.rs +++ b/compiler/rustc_lint/src/builtin.rs @@ -1298,12 +1298,30 @@ impl UnreachablePub { let mut applicability = Applicability::MachineApplicable; if cx.tcx.visibility(def_id).is_public() && !cx.effective_visibilities.is_reachable(def_id) { + // prefer suggesting `pub(super)` instead of `pub(crate)` when possible, + // except when `pub(super) == pub(crate)` + let new_vis = if let Some(ty::Visibility::Restricted(restricted_did)) = + cx.effective_visibilities.effective_vis(def_id).map(|effective_vis| { + effective_vis.at_level(rustc_middle::middle::privacy::Level::Reachable) + }) + && let parent_parent = cx.tcx.parent_module_from_def_id( + cx.tcx.parent_module_from_def_id(def_id.into()).into(), + ) + && *restricted_did == parent_parent.to_local_def_id() + && !restricted_did.to_def_id().is_crate_root() + { + "pub(super)" + } else { + "pub(crate)" + }; + if vis_span.from_expansion() { applicability = Applicability::MaybeIncorrect; } let def_span = cx.tcx.def_span(def_id); cx.emit_span_lint(UNREACHABLE_PUB, def_span, BuiltinUnreachablePub { what, + new_vis, suggestion: (vis_span, applicability), help: exportable, }); diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index 38e52570e53e6..352155729e51c 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -254,7 +254,8 @@ impl<'a> LintDiagnostic<'a, ()> for BuiltinUngatedAsyncFnTrackCaller<'_> { #[diag(lint_builtin_unreachable_pub)] pub(crate) struct BuiltinUnreachablePub<'a> { pub what: &'a str, - #[suggestion(code = "pub(crate)")] + pub new_vis: &'a str, + #[suggestion(code = "{new_vis}")] pub suggestion: (Span, Applicability), #[help] pub help: bool, diff --git a/tests/ui/lint/unreachable_pub.fixed b/tests/ui/lint/unreachable_pub.fixed new file mode 100644 index 0000000000000..163e8d24b3255 --- /dev/null +++ b/tests/ui/lint/unreachable_pub.fixed @@ -0,0 +1,116 @@ +//@ check-pass +//@ edition: 2018 +//@ run-rustfix + +#![allow(unused)] +#![warn(unreachable_pub)] + +mod private_mod { + // non-leaked `pub` items in private module should be linted + pub(crate) use std::fmt; //~ WARNING unreachable_pub + pub(crate) use std::env::{Args}; // braced-use has different item spans than unbraced + //~^ WARNING unreachable_pub + + // we lint on struct definition + pub(crate) struct Hydrogen { //~ WARNING unreachable_pub + // but not on fields, even if they are `pub` as putting `pub(crate)` + // it would clutter the source code for little value + pub neutrons: usize, + pub(crate) electrons: usize + } + pub(crate) struct Calcium { + pub neutrons: usize, + } + impl Hydrogen { + // impls, too + pub(crate) fn count_neutrons(&self) -> usize { self.neutrons } //~ WARNING unreachable_pub + pub(crate) fn count_electrons(&self) -> usize { self.electrons } + } + impl Clone for Hydrogen { + fn clone(&self) -> Hydrogen { + Hydrogen { neutrons: self.neutrons, electrons: self.electrons } + } + } + + pub(crate) enum Helium {} //~ WARNING unreachable_pub + pub(crate) union Lithium { c1: usize, c2: u8 } //~ WARNING unreachable_pub + pub(crate) fn beryllium() {} //~ WARNING unreachable_pub + pub(crate) trait Boron {} //~ WARNING unreachable_pub + pub(crate) const CARBON: usize = 1; //~ WARNING unreachable_pub + pub(crate) static NITROGEN: usize = 2; //~ WARNING unreachable_pub + pub(crate) type Oxygen = bool; //~ WARNING unreachable_pub + + macro_rules! define_empty_struct_with_visibility { + ($visibility: vis, $name: ident) => { $visibility struct $name {} } + //~^ WARNING unreachable_pub + } + define_empty_struct_with_visibility!(pub(crate), Fluorine); + + extern "C" { + pub(crate) fn catalyze() -> bool; //~ WARNING unreachable_pub + } + + mod private_in_private { + pub(super) enum Helium {} //~ WARNING unreachable_pub + pub(super) fn beryllium() {} //~ WARNING unreachable_pub + } + + pub(crate) mod crate_in_private { + pub(crate) const CARBON: usize = 1; //~ WARNING unreachable_pub + } + + pub(crate) mod pub_in_private { //~ WARNING unreachable_pub + pub(crate) static NITROGEN: usize = 2; //~ WARNING unreachable_pub + } + + fn foo() { + const { + pub(crate) struct Foo; //~ WARNING unreachable_pub + }; + } + + enum Weird { + Variant = { + pub(crate) struct Foo; //~ WARNING unreachable_pub + + mod tmp { + pub(crate) struct Bar; //~ WARNING unreachable_pub + } + + let _ = tmp::Bar; + + 0 + }, + } + + pub(crate) use fpu_precision::set_precision; //~ WARNING unreachable_pub + + mod fpu_precision { + pub(crate) fn set_precision() {} //~ WARNING unreachable_pub + pub(super) fn set_micro_precision() {} //~ WARNING unreachable_pub + } + + // items leaked through signatures (see `get_neon` below) are OK + pub struct Neon {} + + // crate-visible items are OK + pub(crate) struct Sodium {} +} + +pub mod public_mod { + // module is public: these are OK, too + pub struct Magnesium {} + pub(crate) struct Aluminum {} +} + +pub fn get_neon() -> private_mod::Neon { + private_mod::Neon {} +} + +fn main() { + let _ = get_neon(); + let _ = private_mod::beryllium(); + let _ = private_mod::crate_in_private::CARBON; + let _ = private_mod::pub_in_private::NITROGEN; + let _ = unsafe { private_mod::catalyze() }; +} diff --git a/tests/ui/lint/unreachable_pub.rs b/tests/ui/lint/unreachable_pub.rs index f21f6640342c4..8524820bcfbc9 100644 --- a/tests/ui/lint/unreachable_pub.rs +++ b/tests/ui/lint/unreachable_pub.rs @@ -1,4 +1,6 @@ //@ check-pass +//@ edition: 2018 +//@ run-rustfix #![allow(unused)] #![warn(unreachable_pub)] @@ -48,6 +50,46 @@ mod private_mod { pub fn catalyze() -> bool; //~ WARNING unreachable_pub } + mod private_in_private { + pub enum Helium {} //~ WARNING unreachable_pub + pub fn beryllium() {} //~ WARNING unreachable_pub + } + + pub(crate) mod crate_in_private { + pub const CARBON: usize = 1; //~ WARNING unreachable_pub + } + + pub mod pub_in_private { //~ WARNING unreachable_pub + pub static NITROGEN: usize = 2; //~ WARNING unreachable_pub + } + + fn foo() { + const { + pub struct Foo; //~ WARNING unreachable_pub + }; + } + + enum Weird { + Variant = { + pub struct Foo; //~ WARNING unreachable_pub + + mod tmp { + pub struct Bar; //~ WARNING unreachable_pub + } + + let _ = tmp::Bar; + + 0 + }, + } + + pub use fpu_precision::set_precision; //~ WARNING unreachable_pub + + mod fpu_precision { + pub fn set_precision() {} //~ WARNING unreachable_pub + pub fn set_micro_precision() {} //~ WARNING unreachable_pub + } + // items leaked through signatures (see `get_neon` below) are OK pub struct Neon {} @@ -67,4 +109,8 @@ pub fn get_neon() -> private_mod::Neon { fn main() { let _ = get_neon(); + let _ = private_mod::beryllium(); + let _ = private_mod::crate_in_private::CARBON; + let _ = private_mod::pub_in_private::NITROGEN; + let _ = unsafe { private_mod::catalyze() }; } diff --git a/tests/ui/lint/unreachable_pub.stderr b/tests/ui/lint/unreachable_pub.stderr index 65f45fbd81689..5173ff1f0264d 100644 --- a/tests/ui/lint/unreachable_pub.stderr +++ b/tests/ui/lint/unreachable_pub.stderr @@ -1,5 +1,5 @@ warning: unreachable `pub` item - --> $DIR/unreachable_pub.rs:8:13 + --> $DIR/unreachable_pub.rs:10:13 | LL | pub use std::fmt; | --- ^^^^^^^^ @@ -8,13 +8,13 @@ LL | pub use std::fmt; | = help: or consider exporting it for use by other crates note: the lint level is defined here - --> $DIR/unreachable_pub.rs:4:9 + --> $DIR/unreachable_pub.rs:6:9 | LL | #![warn(unreachable_pub)] | ^^^^^^^^^^^^^^^ warning: unreachable `pub` item - --> $DIR/unreachable_pub.rs:9:24 + --> $DIR/unreachable_pub.rs:11:24 | LL | pub use std::env::{Args}; // braced-use has different item spans than unbraced | --- ^^^^ @@ -24,7 +24,7 @@ LL | pub use std::env::{Args}; // braced-use has different item spans than u = help: or consider exporting it for use by other crates warning: unreachable `pub` item - --> $DIR/unreachable_pub.rs:13:5 + --> $DIR/unreachable_pub.rs:15:5 | LL | pub struct Hydrogen { | ---^^^^^^^^^^^^^^^^ @@ -34,7 +34,7 @@ LL | pub struct Hydrogen { = help: or consider exporting it for use by other crates warning: unreachable `pub` item - --> $DIR/unreachable_pub.rs:24:9 + --> $DIR/unreachable_pub.rs:26:9 | LL | pub fn count_neutrons(&self) -> usize { self.neutrons } | ---^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -42,7 +42,7 @@ LL | pub fn count_neutrons(&self) -> usize { self.neutrons } | help: consider restricting its visibility: `pub(crate)` warning: unreachable `pub` item - --> $DIR/unreachable_pub.rs:33:5 + --> $DIR/unreachable_pub.rs:35:5 | LL | pub enum Helium {} | ---^^^^^^^^^^^^ @@ -52,7 +52,7 @@ LL | pub enum Helium {} = help: or consider exporting it for use by other crates warning: unreachable `pub` item - --> $DIR/unreachable_pub.rs:34:5 + --> $DIR/unreachable_pub.rs:36:5 | LL | pub union Lithium { c1: usize, c2: u8 } | ---^^^^^^^^^^^^^^ @@ -62,7 +62,7 @@ LL | pub union Lithium { c1: usize, c2: u8 } = help: or consider exporting it for use by other crates warning: unreachable `pub` item - --> $DIR/unreachable_pub.rs:35:5 + --> $DIR/unreachable_pub.rs:37:5 | LL | pub fn beryllium() {} | ---^^^^^^^^^^^^^^^ @@ -72,7 +72,7 @@ LL | pub fn beryllium() {} = help: or consider exporting it for use by other crates warning: unreachable `pub` item - --> $DIR/unreachable_pub.rs:36:5 + --> $DIR/unreachable_pub.rs:38:5 | LL | pub trait Boron {} | ---^^^^^^^^^^^^ @@ -82,7 +82,7 @@ LL | pub trait Boron {} = help: or consider exporting it for use by other crates warning: unreachable `pub` item - --> $DIR/unreachable_pub.rs:37:5 + --> $DIR/unreachable_pub.rs:39:5 | LL | pub const CARBON: usize = 1; | ---^^^^^^^^^^^^^^^^^^^^ @@ -92,7 +92,7 @@ LL | pub const CARBON: usize = 1; = help: or consider exporting it for use by other crates warning: unreachable `pub` item - --> $DIR/unreachable_pub.rs:38:5 + --> $DIR/unreachable_pub.rs:40:5 | LL | pub static NITROGEN: usize = 2; | ---^^^^^^^^^^^^^^^^^^^^^^^ @@ -102,7 +102,7 @@ LL | pub static NITROGEN: usize = 2; = help: or consider exporting it for use by other crates warning: unreachable `pub` item - --> $DIR/unreachable_pub.rs:39:5 + --> $DIR/unreachable_pub.rs:41:5 | LL | pub type Oxygen = bool; | ---^^^^^^^^^^^^ @@ -112,7 +112,7 @@ LL | pub type Oxygen = bool; = help: or consider exporting it for use by other crates warning: unreachable `pub` item - --> $DIR/unreachable_pub.rs:42:47 + --> $DIR/unreachable_pub.rs:44:47 | LL | ($visibility: vis, $name: ident) => { $visibility struct $name {} } | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -127,7 +127,7 @@ LL | define_empty_struct_with_visibility!(pub, Fluorine); = note: this warning originates in the macro `define_empty_struct_with_visibility` (in Nightly builds, run with -Z macro-backtrace for more info) warning: unreachable `pub` item - --> $DIR/unreachable_pub.rs:48:9 + --> $DIR/unreachable_pub.rs:50:9 | LL | pub fn catalyze() -> bool; | ---^^^^^^^^^^^^^^^^^^^^^^^ @@ -136,5 +136,115 @@ LL | pub fn catalyze() -> bool; | = help: or consider exporting it for use by other crates -warning: 13 warnings emitted +warning: unreachable `pub` item + --> $DIR/unreachable_pub.rs:62:5 + | +LL | pub mod pub_in_private { + | ---^^^^^^^^^^^^^^^^^^^ + | | + | help: consider restricting its visibility: `pub(crate)` + | + = help: or consider exporting it for use by other crates + +warning: unreachable `pub` item + --> $DIR/unreachable_pub.rs:68:13 + | +LL | pub struct Foo; + | ---^^^^^^^^^^^ + | | + | help: consider restricting its visibility: `pub(crate)` + | + = help: or consider exporting it for use by other crates + +warning: unreachable `pub` item + --> $DIR/unreachable_pub.rs:74:13 + | +LL | pub struct Foo; + | ---^^^^^^^^^^^ + | | + | help: consider restricting its visibility: `pub(crate)` + | + = help: or consider exporting it for use by other crates + +warning: unreachable `pub` item + --> $DIR/unreachable_pub.rs:86:13 + | +LL | pub use fpu_precision::set_precision; + | --- ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | | + | help: consider restricting its visibility: `pub(crate)` + | + = help: or consider exporting it for use by other crates + +warning: unreachable `pub` item + --> $DIR/unreachable_pub.rs:54:9 + | +LL | pub enum Helium {} + | ---^^^^^^^^^^^^ + | | + | help: consider restricting its visibility: `pub(super)` + | + = help: or consider exporting it for use by other crates + +warning: unreachable `pub` item + --> $DIR/unreachable_pub.rs:55:9 + | +LL | pub fn beryllium() {} + | ---^^^^^^^^^^^^^^^ + | | + | help: consider restricting its visibility: `pub(super)` + | + = help: or consider exporting it for use by other crates + +warning: unreachable `pub` item + --> $DIR/unreachable_pub.rs:59:9 + | +LL | pub const CARBON: usize = 1; + | ---^^^^^^^^^^^^^^^^^^^^ + | | + | help: consider restricting its visibility: `pub(crate)` + | + = help: or consider exporting it for use by other crates + +warning: unreachable `pub` item + --> $DIR/unreachable_pub.rs:63:9 + | +LL | pub static NITROGEN: usize = 2; + | ---^^^^^^^^^^^^^^^^^^^^^^^ + | | + | help: consider restricting its visibility: `pub(crate)` + | + = help: or consider exporting it for use by other crates + +warning: unreachable `pub` item + --> $DIR/unreachable_pub.rs:77:17 + | +LL | pub struct Bar; + | ---^^^^^^^^^^^ + | | + | help: consider restricting its visibility: `pub(crate)` + | + = help: or consider exporting it for use by other crates + +warning: unreachable `pub` item + --> $DIR/unreachable_pub.rs:89:9 + | +LL | pub fn set_precision() {} + | ---^^^^^^^^^^^^^^^^^^^^^^ + | | + | help: consider restricting its visibility: `pub(crate)` + | + = help: or consider exporting it for use by other crates + +warning: unreachable `pub` item + --> $DIR/unreachable_pub.rs:90:9 + | +LL | pub fn set_micro_precision() {} + | ---^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | | + | help: consider restricting its visibility: `pub(super)` + | + = help: or consider exporting it for use by other crates + +warning: 24 warnings emitted