Skip to content

Commit 6a42df7

Browse files
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``
2 parents 75aa9d1 + 89d86ae commit 6a42df7

File tree

3 files changed

+34
-29
lines changed

3 files changed

+34
-29
lines changed

compiler/rustc_lint/src/builtin.rs

+15-6
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ use rustc_hir as hir;
5151
use rustc_hir::def::{DefKind, Res};
5252
use rustc_hir::def_id::{DefId, LocalDefId, CRATE_DEF_ID};
5353
use rustc_hir::intravisit::FnKind as HirFnKind;
54-
use rustc_hir::{Body, FnDecl, GenericParamKind, Node, PatKind, PredicateOrigin};
54+
use rustc_hir::{Body, FnDecl, GenericParamKind, PatKind, PredicateOrigin};
5555
use rustc_middle::bug;
5656
use rustc_middle::lint::in_external_macro;
5757
use rustc_middle::ty::layout::LayoutOf;
@@ -1423,11 +1423,20 @@ impl<'tcx> LateLintPass<'tcx> for UnreachablePub {
14231423
self.perform_lint(cx, "item", foreign_item.owner_id.def_id, foreign_item.vis_span, true);
14241424
}
14251425

1426-
fn check_field_def(&mut self, cx: &LateContext<'_>, field: &hir::FieldDef<'_>) {
1427-
if matches!(cx.tcx.parent_hir_node(field.hir_id), Node::Variant(_)) {
1428-
return;
1429-
}
1430-
self.perform_lint(cx, "field", field.def_id, field.vis_span, false);
1426+
fn check_field_def(&mut self, _cx: &LateContext<'_>, _field: &hir::FieldDef<'_>) {
1427+
// - If an ADT definition is reported then we don't need to check fields
1428+
// (as it would add unnecessary complexity to the source code, the struct
1429+
// definition is in the immediate proximity to give the "real" visibility).
1430+
// - If an ADT is not reported because it's not `pub` - we don't need to
1431+
// check fields.
1432+
// - If an ADT is not reported because it's reachable - we also don't need
1433+
// to check fields because then they are reachable by construction if they
1434+
// are pub.
1435+
//
1436+
// Therefore in no case we check the fields.
1437+
//
1438+
// cf. https://github.com/rust-lang/rust/pull/126013#issuecomment-2152839205
1439+
// cf. https://github.com/rust-lang/rust/pull/126040#issuecomment-2152944506
14311440
}
14321441

14331442
fn check_impl_item(&mut self, cx: &LateContext<'_>, impl_item: &hir::ImplItem<'_>) {

tests/ui/lint/unreachable_pub.rs

+7-3
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,16 @@ mod private_mod {
99
pub use std::env::{Args}; // braced-use has different item spans than unbraced
1010
//~^ WARNING unreachable_pub
1111

12+
// we lint on struct definition
1213
pub struct Hydrogen { //~ WARNING unreachable_pub
13-
// `pub` struct fields, too
14-
pub neutrons: usize, //~ WARNING unreachable_pub
15-
// (... but not more-restricted fields)
14+
// but not on fields, even if they are `pub` as putting `pub(crate)`
15+
// it would clutter the source code for little value
16+
pub neutrons: usize,
1617
pub(crate) electrons: usize
1718
}
19+
pub(crate) struct Calcium {
20+
pub neutrons: usize,
21+
}
1822
impl Hydrogen {
1923
// impls, too
2024
pub fn count_neutrons(&self) -> usize { self.neutrons } //~ WARNING unreachable_pub

tests/ui/lint/unreachable_pub.stderr

+12-20
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ LL | pub use std::env::{Args}; // braced-use has different item spans than u
2424
= help: or consider exporting it for use by other crates
2525

2626
warning: unreachable `pub` item
27-
--> $DIR/unreachable_pub.rs:12:5
27+
--> $DIR/unreachable_pub.rs:13:5
2828
|
2929
LL | pub struct Hydrogen {
3030
| ---^^^^^^^^^^^^^^^^
@@ -33,24 +33,16 @@ LL | pub struct Hydrogen {
3333
|
3434
= help: or consider exporting it for use by other crates
3535

36-
warning: unreachable `pub` field
37-
--> $DIR/unreachable_pub.rs:14:9
38-
|
39-
LL | pub neutrons: usize,
40-
| ---^^^^^^^^^^^^^^^^
41-
| |
42-
| help: consider restricting its visibility: `pub(crate)`
43-
4436
warning: unreachable `pub` item
45-
--> $DIR/unreachable_pub.rs:20:9
37+
--> $DIR/unreachable_pub.rs:24:9
4638
|
4739
LL | pub fn count_neutrons(&self) -> usize { self.neutrons }
4840
| ---^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
4941
| |
5042
| help: consider restricting its visibility: `pub(crate)`
5143

5244
warning: unreachable `pub` item
53-
--> $DIR/unreachable_pub.rs:29:5
45+
--> $DIR/unreachable_pub.rs:33:5
5446
|
5547
LL | pub enum Helium {}
5648
| ---^^^^^^^^^^^^
@@ -60,7 +52,7 @@ LL | pub enum Helium {}
6052
= help: or consider exporting it for use by other crates
6153

6254
warning: unreachable `pub` item
63-
--> $DIR/unreachable_pub.rs:30:5
55+
--> $DIR/unreachable_pub.rs:34:5
6456
|
6557
LL | pub union Lithium { c1: usize, c2: u8 }
6658
| ---^^^^^^^^^^^^^^
@@ -70,7 +62,7 @@ LL | pub union Lithium { c1: usize, c2: u8 }
7062
= help: or consider exporting it for use by other crates
7163

7264
warning: unreachable `pub` item
73-
--> $DIR/unreachable_pub.rs:31:5
65+
--> $DIR/unreachable_pub.rs:35:5
7466
|
7567
LL | pub fn beryllium() {}
7668
| ---^^^^^^^^^^^^^^^
@@ -80,7 +72,7 @@ LL | pub fn beryllium() {}
8072
= help: or consider exporting it for use by other crates
8173

8274
warning: unreachable `pub` item
83-
--> $DIR/unreachable_pub.rs:32:5
75+
--> $DIR/unreachable_pub.rs:36:5
8476
|
8577
LL | pub trait Boron {}
8678
| ---^^^^^^^^^^^^
@@ -90,7 +82,7 @@ LL | pub trait Boron {}
9082
= help: or consider exporting it for use by other crates
9183

9284
warning: unreachable `pub` item
93-
--> $DIR/unreachable_pub.rs:33:5
85+
--> $DIR/unreachable_pub.rs:37:5
9486
|
9587
LL | pub const CARBON: usize = 1;
9688
| ---^^^^^^^^^^^^^^^^^^^^
@@ -100,7 +92,7 @@ LL | pub const CARBON: usize = 1;
10092
= help: or consider exporting it for use by other crates
10193

10294
warning: unreachable `pub` item
103-
--> $DIR/unreachable_pub.rs:34:5
95+
--> $DIR/unreachable_pub.rs:38:5
10496
|
10597
LL | pub static NITROGEN: usize = 2;
10698
| ---^^^^^^^^^^^^^^^^^^^^^^^
@@ -110,7 +102,7 @@ LL | pub static NITROGEN: usize = 2;
110102
= help: or consider exporting it for use by other crates
111103

112104
warning: unreachable `pub` item
113-
--> $DIR/unreachable_pub.rs:35:5
105+
--> $DIR/unreachable_pub.rs:39:5
114106
|
115107
LL | pub type Oxygen = bool;
116108
| ---^^^^^^^^^^^^
@@ -120,7 +112,7 @@ LL | pub type Oxygen = bool;
120112
= help: or consider exporting it for use by other crates
121113

122114
warning: unreachable `pub` item
123-
--> $DIR/unreachable_pub.rs:38:47
115+
--> $DIR/unreachable_pub.rs:42:47
124116
|
125117
LL | ($visibility: vis, $name: ident) => { $visibility struct $name {} }
126118
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -135,7 +127,7 @@ LL | define_empty_struct_with_visibility!(pub, Fluorine);
135127
= note: this warning originates in the macro `define_empty_struct_with_visibility` (in Nightly builds, run with -Z macro-backtrace for more info)
136128

137129
warning: unreachable `pub` item
138-
--> $DIR/unreachable_pub.rs:44:9
130+
--> $DIR/unreachable_pub.rs:48:9
139131
|
140132
LL | pub fn catalyze() -> bool;
141133
| ---^^^^^^^^^^^^^^^^^^^^^^
@@ -144,5 +136,5 @@ LL | pub fn catalyze() -> bool;
144136
|
145137
= help: or consider exporting it for use by other crates
146138

147-
warning: 14 warnings emitted
139+
warning: 13 warnings emitted
148140

0 commit comments

Comments
 (0)