Unrolled build for rust-lang#126040

Rollup merge of rust-lang#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``
rust-lang-ci · Jun 7, 2024 · 56a59ea · 56a59ea
2 parents b74702f + 89d86ae
commit 56a59ea
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 29 deletions.
diff --git 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
@@ -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
@@ -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,24 +33,16 @@ 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 }
    |         ---^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |         |
    |         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