Prefer pub(super) in unreachable_pub lint suggestion

Urgau · Urgau · commit fca6a563051b · 2024-11-02T13:59:36.000+01:00
diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs
@@ -1295,15 +1295,57 @@ impl UnreachablePub {
         vis_span: Span,
         exportable: bool,
     ) {
+        fn opt_parent_module(tcx: TyCtxt<'_>, mut did: LocalDefId) -> Option<LocalDefId> {
+            while let Some(parent) = tcx.opt_local_parent(did) {
+                did = parent;
+                if tcx.def_kind(did) == DefKind::Mod {
+                    return Some(did);
+                }
+            }
+            None
+        }
+
         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
+            let new_vis = if let Some(reachable_vis) =
+                cx.effective_visibilities.effective_vis(def_id).map(|effective_vis| {
+                    effective_vis.at_level(rustc_middle::middle::privacy::Level::Reachable)
+                }) {
+                let rustc_middle::ty::Visibility::Restricted(restricted_id) = reachable_vis else {
+                    unreachable!()
+                };
+
+                if let Some(parent) = opt_parent_module(cx.tcx, def_id.into()) {
+                    if let Some(parent_parent) = opt_parent_module(cx.tcx, parent) {
+                        if &parent_parent == restricted_id {
+                            // current item is restricted to the parent of it's parent (aka super)
+                            "pub(super)"
+                        } else {
+                            // current item is not restricted to the parent of it's parent
+                            "pub(crate)"
+                        }
+                    } else {
+                        // unreachable pub at crate root, is that even possible?
+                        "pub(crate)"
+                    }
+                } else {
+                    // current item has no parent, is that even possible?
+                    "pub(crate)"
+                }
+            } else {
+                // current item can't be reach from crate, probably in an anon-const
+                "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
@@ -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/issue-103317.fixed b/tests/ui/lint/issue-103317.fixed
@@ -4,7 +4,7 @@
 #[warn(unreachable_pub)]
 mod inner {
     #[allow(unused)]
-    pub(crate) enum T {
+    pub(super) enum T {
         //~^ WARN unreachable `pub` item
         A(u8),
         X { a: f32, b: () },
diff --git a/tests/ui/lint/issue-103317.stderr b/tests/ui/lint/issue-103317.stderr
@@ -4,7 +4,7 @@ warning: unreachable `pub` item
 LL |     pub enum T {
    |     ---^^^^^^^
    |     |
-   |     help: consider restricting its visibility: `pub(crate)`
+   |     help: consider restricting its visibility: `pub(super)`
    |
    = help: or consider exporting it for use by other crates
 note: the lint level is defined here
diff --git a/tests/ui/lint/unreachable_pub.fixed 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(super) use std::fmt; //~ WARNING unreachable_pub
+    pub(super) use std::env::{Args}; // braced-use has different item spans than unbraced
+    //~^ WARNING unreachable_pub
+
+    // we lint on struct definition
+    pub(super) 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(super) 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(super) enum Helium {} //~ WARNING unreachable_pub
+    pub(super) union Lithium { c1: usize, c2: u8 } //~ WARNING unreachable_pub
+    pub(super) fn beryllium() {} //~ WARNING unreachable_pub
+    pub(super) trait Boron {} //~ WARNING unreachable_pub
+    pub(super) const CARBON: usize = 1; //~ WARNING unreachable_pub
+    pub(super) static NITROGEN: usize = 2; //~ WARNING unreachable_pub
+    pub(super) 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(super), Fluorine);
+
+    extern "C" {
+        pub(super) 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(super) 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(super) use fpu_precision::set_precision; //~ WARNING unreachable_pub
+
+    mod fpu_precision {
+        pub(crate) fn set_precision<T>() {} //~ WARNING unreachable_pub
+        pub(super) fn set_micro_precision<T>() {} //~ 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
@@ -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<T>() {} //~ WARNING unreachable_pub
+        pub fn set_micro_precision<T>() {} //~ 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

Original file line number	Diff line number	Diff line change
@@ -4,7 +4,7 @@ warning: unreachable `pub` item
`4`	`4`	`LL \| pub enum T {`
`5`	`5`	`\| ---^^^^^^^`
`6`	`6`	`\| \|`
`7`		- \| help: consider restricting its visibility: `pub(crate)`
	`7`	+ \| help: consider restricting its visibility: `pub(super)`
`8`	`8`	`\|`
`9`	`9`	`= help: or consider exporting it for use by other crates`
`10`	`10`	`note: the lint level is defined here`