Rollup merge of #126040 - Urgau:unreachable_pub-fields-less, r=petrochenkov

Don't warn on fields in the `unreachable_pub` lint

This PR restrict the `unreachable_pub` lint by not linting on `pub` fields of `pub(restricted)` structs and unions. This is done because that can quickly clutter the code for an uncertain value, in particular since the "real" visibility is defined by the parent (the struct it-self).

This is meant to address one of the last concern of the `unreachable_pub` lint.

r? ``@petrochenkov``

3 files changed, 34 insertions, 29 deletions

diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs
index 87c433a5dc0..8c9abeafacf 100644
--- a/compiler/rustc_lint/src/builtin.rs
+++ b/compiler/rustc_lint/src/builtin.rs
@@ -51,7 +51,7 @@ use rustc_hir as hir;
 use rustc_hir::def::{DefKind, Res};
 use rustc_hir::def_id::{DefId, LocalDefId, CRATE_DEF_ID};
 use rustc_hir::intravisit::FnKind as HirFnKind;
-use rustc_hir::{Body, FnDecl, GenericParamKind, Node, PatKind, PredicateOrigin};
+use rustc_hir::{Body, FnDecl, GenericParamKind, PatKind, PredicateOrigin};
 use rustc_middle::bug;
 use rustc_middle::lint::in_external_macro;
 use rustc_middle::ty::layout::LayoutOf;
@@ -1423,11 +1423,20 @@ impl<'tcx> LateLintPass<'tcx> for UnreachablePub {
         self.perform_lint(cx, "item", foreign_item.owner_id.def_id, foreign_item.vis_span, true);
     }
 
-    fn check_field_def(&mut self, cx: &LateContext<'_>, field: &hir::FieldDef<'_>) {
-        if matches!(cx.tcx.parent_hir_node(field.hir_id), Node::Variant(_)) {
-            return;
-        }
-        self.perform_lint(cx, "field", field.def_id, field.vis_span, false);
+    fn check_field_def(&mut self, _cx: &LateContext<'_>, _field: &hir::FieldDef<'_>) {
+        // - If an ADT definition is reported then we don't need to check fields
+        //   (as it would add unnecessary complexity to the source code, the struct
+        //   definition is in the immediate proximity to give the "real" visibility).
+        // - If an ADT is not reported because it's not `pub` - we don't need to
+        //   check fields.
+        // - If an ADT is not reported because it's reachable - we also don't need
+        //   to check fields because then they are reachable by construction if they
+        //   are pub.
+        //
+        // Therefore in no case we check the fields.
+        //
+        // cf. https://github.com/rust-lang/rust/pull/126013#issuecomment-2152839205
+        // cf. https://github.com/rust-lang/rust/pull/126040#issuecomment-2152944506
     }
 
     fn check_impl_item(&mut self, cx: &LateContext<'_>, impl_item: &hir::ImplItem<'_>) {
diff --git a/tests/ui/lint/unreachable_pub.rs b/tests/ui/lint/unreachable_pub.rs
index 22c091e112b..f21f6640342 100644
--- a/tests/ui/lint/unreachable_pub.rs
+++ b/tests/ui/lint/unreachable_pub.rs
@@ -9,12 +9,16 @@ mod private_mod {
     pub use std::env::{Args}; // braced-use has different item spans than unbraced
     //~^ WARNING unreachable_pub
 
+    // we lint on struct definition
     pub struct Hydrogen { //~ WARNING unreachable_pub
-        // `pub` struct fields, too
-        pub neutrons: usize, //~ WARNING unreachable_pub
-        // (... but not more-restricted fields)
+        // 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 fn count_neutrons(&self) -> usize { self.neutrons } //~ WARNING unreachable_pub
diff --git a/tests/ui/lint/unreachable_pub.stderr b/tests/ui/lint/unreachable_pub.stderr
index 762834b97b9..705a537a3f1 100644
--- a/tests/ui/lint/unreachable_pub.stderr
+++ b/tests/ui/lint/unreachable_pub.stderr
@@ -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:12:5
+  --> $DIR/unreachable_pub.rs:13:5
    |
 LL |     pub struct Hydrogen {
    |     ---^^^^^^^^^^^^^^^^
@@ -33,16 +33,8 @@ LL |     pub struct Hydrogen {
    |
    = help: or consider exporting it for use by other crates
 
-warning: unreachable `pub` field
-  --> $DIR/unreachable_pub.rs:14:9
-   |
-LL |         pub neutrons: usize,
-   |         ---^^^^^^^^^^^^^^^^
-   |         |
-   |         help: consider restricting its visibility: `pub(crate)`
-
 warning: unreachable `pub` item
-  --> $DIR/unreachable_pub.rs:20:9
+  --> $DIR/unreachable_pub.rs:24:9
    |
 LL |         pub fn count_neutrons(&self) -> usize { self.neutrons }
    |         ---^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -50,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:29:5
+  --> $DIR/unreachable_pub.rs:33:5
    |
 LL |     pub enum Helium {}
    |     ---^^^^^^^^^^^^
@@ -60,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:30:5
+  --> $DIR/unreachable_pub.rs:34:5
    |
 LL |     pub union Lithium { c1: usize, c2: u8 }
    |     ---^^^^^^^^^^^^^^
@@ -70,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:31:5
+  --> $DIR/unreachable_pub.rs:35:5
    |
 LL |     pub fn beryllium() {}
    |     ---^^^^^^^^^^^^^^^
@@ -80,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:32:5
+  --> $DIR/unreachable_pub.rs:36:5
    |
 LL |     pub trait Boron {}
    |     ---^^^^^^^^^^^^
@@ -90,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:33:5
+  --> $DIR/unreachable_pub.rs:37:5
    |
 LL |     pub const CARBON: usize = 1;
    |     ---^^^^^^^^^^^^^^^^^^^^
@@ -100,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:34:5
+  --> $DIR/unreachable_pub.rs:38:5
    |
 LL |     pub static NITROGEN: usize = 2;
    |     ---^^^^^^^^^^^^^^^^^^^^^^^
@@ -110,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:35:5
+  --> $DIR/unreachable_pub.rs:39:5
    |
 LL |     pub type Oxygen = bool;
    |     ---^^^^^^^^^^^^
@@ -120,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:38:47
+  --> $DIR/unreachable_pub.rs:42:47
    |
 LL |         ($visibility: vis, $name: ident) => { $visibility struct $name {} }
    |                                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -135,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:44:9
+  --> $DIR/unreachable_pub.rs:48:9
    |
 LL |         pub fn catalyze() -> bool;
    |         ---^^^^^^^^^^^^^^^^^^^^^^
@@ -144,5 +136,5 @@ LL |         pub fn catalyze() -> bool;
    |
    = help: or consider exporting it for use by other crates
 
-warning: 14 warnings emitted
+warning: 13 warnings emitted