Skip to content

Commit fa7dd1c

Browse files
ParkMyCarblyxyas
authored andcommitted
add new lint, pub_underscore_fields
- add a new late pass lint, with config options - add ui tests for both variations of config option - update CHANGELOG.md github feedback bump version to 1.77 and run cargo collect-metadata Change `,` to `;` in `conf.rs`
1 parent 7343db9 commit fa7dd1c

13 files changed

+257
-1
lines changed

CHANGELOG.md

+2
Original file line numberDiff line numberDiff line change
@@ -5483,6 +5483,7 @@ Released 2018-09-13
54835483
[`ptr_eq`]: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_eq
54845484
[`ptr_offset_with_cast`]: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_offset_with_cast
54855485
[`pub_enum_variant_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#pub_enum_variant_names
5486+
[`pub_underscore_fields`]: https://rust-lang.github.io/rust-clippy/master/index.html#pub_underscore_fields
54865487
[`pub_use`]: https://rust-lang.github.io/rust-clippy/master/index.html#pub_use
54875488
[`pub_with_shorthand`]: https://rust-lang.github.io/rust-clippy/master/index.html#pub_with_shorthand
54885489
[`pub_without_shorthand`]: https://rust-lang.github.io/rust-clippy/master/index.html#pub_without_shorthand
@@ -5810,4 +5811,5 @@ Released 2018-09-13
58105811
[`allowed-dotfiles`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allowed-dotfiles
58115812
[`enforce-iter-loop-reborrow`]: https://doc.rust-lang.org/clippy/lint_configuration.html#enforce-iter-loop-reborrow
58125813
[`check-private-items`]: https://doc.rust-lang.org/clippy/lint_configuration.html#check-private-items
5814+
[`pub-underscore-fields-behavior`]: https://doc.rust-lang.org/clippy/lint_configuration.html#pub-underscore-fields-behavior
58135815
<!-- end autogenerated links to configuration documentation -->

book/src/lint_configuration.md

+10
Original file line numberDiff line numberDiff line change
@@ -805,3 +805,13 @@ for _ in &mut *rmvec {}
805805
* [`missing_errors_doc`](https://rust-lang.github.io/rust-clippy/master/index.html#missing_errors_doc)
806806

807807

808+
## `pub-underscore-fields-behavior`
809+
810+
811+
**Default Value:** `"PublicallyExported"`
812+
813+
---
814+
**Affected lints:**
815+
* [`pub_underscore_fields`](https://rust-lang.github.io/rust-clippy/master/index.html#pub_underscore_fields)
816+
817+

clippy_config/src/conf.rs

+6-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use crate::msrvs::Msrv;
2-
use crate::types::{DisallowedPath, MacroMatcher, MatchLintBehaviour, Rename};
2+
use crate::types::{DisallowedPath, MacroMatcher, MatchLintBehaviour, PubUnderscoreFieldsBehaviour, Rename};
33
use crate::ClippyConfiguration;
44
use rustc_data_structures::fx::FxHashSet;
55
use rustc_session::Session;
@@ -547,6 +547,11 @@ define_Conf! {
547547
///
548548
/// Whether to also run the listed lints on private items.
549549
(check_private_items: bool = false),
550+
/// Lint: PUB_UNDERSCORE_FIELDS
551+
///
552+
/// Lint "public" fields in a struct that are prefixed with an underscore based on their
553+
/// exported visibility; or whether they are marked as "pub".
554+
(pub_underscore_fields_behavior: PubUnderscoreFieldsBehaviour = PubUnderscoreFieldsBehaviour::PublicallyExported),
550555
}
551556

552557
/// Search for the configuration file.

clippy_config/src/types.rs

+6
Original file line numberDiff line numberDiff line change
@@ -126,3 +126,9 @@ unimplemented_serialize! {
126126
Rename,
127127
MacroMatcher,
128128
}
129+
130+
#[derive(Clone, Copy, Debug, PartialEq, Eq, Deserialize, Serialize)]
131+
pub enum PubUnderscoreFieldsBehaviour {
132+
PublicallyExported,
133+
AllPubFields,
134+
}

clippy_lints/src/declared_lints.rs

+1
Original file line numberDiff line numberDiff line change
@@ -576,6 +576,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
576576
crate::ptr::MUT_FROM_REF_INFO,
577577
crate::ptr::PTR_ARG_INFO,
578578
crate::ptr_offset_with_cast::PTR_OFFSET_WITH_CAST_INFO,
579+
crate::pub_underscore_fields::PUB_UNDERSCORE_FIELDS_INFO,
579580
crate::pub_use::PUB_USE_INFO,
580581
crate::question_mark::QUESTION_MARK_INFO,
581582
crate::question_mark_used::QUESTION_MARK_USED_INFO,

clippy_lints/src/lib.rs

+7
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,7 @@ mod permissions_set_readonly_false;
272272
mod precedence;
273273
mod ptr;
274274
mod ptr_offset_with_cast;
275+
mod pub_underscore_fields;
275276
mod pub_use;
276277
mod question_mark;
277278
mod question_mark_used;
@@ -571,6 +572,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
571572
verbose_bit_mask_threshold,
572573
warn_on_all_wildcard_imports,
573574
check_private_items,
575+
pub_underscore_fields_behavior,
574576

575577
blacklisted_names: _,
576578
cyclomatic_complexity_threshold: _,
@@ -1081,6 +1083,11 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
10811083
store.register_late_pass(|_| Box::new(uninhabited_references::UninhabitedReferences));
10821084
store.register_late_pass(|_| Box::new(ineffective_open_options::IneffectiveOpenOptions));
10831085
store.register_late_pass(|_| Box::new(unconditional_recursion::UnconditionalRecursion));
1086+
store.register_late_pass(move |_| {
1087+
Box::new(pub_underscore_fields::PubUnderscoreFields {
1088+
behavior: pub_underscore_fields_behavior,
1089+
})
1090+
});
10841091
// add lints here, do not remove this comment, it's used in `new_lint`
10851092
}
10861093

+83
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
use clippy_config::types::PubUnderscoreFieldsBehaviour;
2+
use clippy_utils::attrs::is_doc_hidden;
3+
use clippy_utils::diagnostics::span_lint_and_help;
4+
use clippy_utils::is_path_lang_item;
5+
use rustc_hir::{FieldDef, Item, ItemKind, LangItem};
6+
use rustc_lint::{LateContext, LateLintPass};
7+
use rustc_session::impl_lint_pass;
8+
9+
declare_clippy_lint! {
10+
/// ### What it does
11+
/// Checks whether any field of the struct is prefixed with an `_` (underscore) and also marked
12+
/// `pub` (public)
13+
///
14+
/// ### Why is this bad?
15+
/// Fields prefixed with an `_` are inferred as unused, which suggests it should not be marked
16+
/// as `pub`, because marking it as `pub` infers it will be used.
17+
///
18+
/// ### Example
19+
/// ```rust
20+
/// struct FileHandle {
21+
/// pub _descriptor: usize,
22+
/// }
23+
/// ```
24+
/// Use instead:
25+
/// ```rust
26+
/// struct FileHandle {
27+
/// _descriptor: usize,
28+
/// }
29+
/// ```
30+
///
31+
/// OR
32+
///
33+
/// ```rust
34+
/// struct FileHandle {
35+
/// pub descriptor: usize,
36+
/// }
37+
/// ```
38+
#[clippy::version = "1.77.0"]
39+
pub PUB_UNDERSCORE_FIELDS,
40+
pedantic,
41+
"struct field prefixed with underscore and marked public"
42+
}
43+
44+
pub struct PubUnderscoreFields {
45+
pub behavior: PubUnderscoreFieldsBehaviour,
46+
}
47+
impl_lint_pass!(PubUnderscoreFields => [PUB_UNDERSCORE_FIELDS]);
48+
49+
impl<'tcx> LateLintPass<'tcx> for PubUnderscoreFields {
50+
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) {
51+
// This lint only pertains to structs.
52+
let ItemKind::Struct(variant_data, _) = &item.kind else {
53+
return;
54+
};
55+
56+
let is_visible = |field: &FieldDef<'_>| match self.behavior {
57+
PubUnderscoreFieldsBehaviour::PublicallyExported => cx.effective_visibilities.is_reachable(field.def_id),
58+
PubUnderscoreFieldsBehaviour::AllPubFields => {
59+
// If there is a visibility span then the field is marked pub in some way.
60+
!field.vis_span.is_empty()
61+
},
62+
};
63+
64+
for field in variant_data.fields() {
65+
// Only pertains to fields that start with an underscore, and are public.
66+
if field.ident.as_str().starts_with('_') && is_visible(field)
67+
// We ignore fields that have `#[doc(hidden)]`.
68+
&& !is_doc_hidden(cx.tcx.hir().attrs(field.hir_id))
69+
// We ignore fields that are `PhantomData`.
70+
&& !is_path_lang_item(cx, field.ty, LangItem::PhantomData)
71+
{
72+
span_lint_and_help(
73+
cx,
74+
PUB_UNDERSCORE_FIELDS,
75+
field.vis_span.to(field.ident.span),
76+
"field marked as public but also inferred as unused because it's prefixed with `_`",
77+
None,
78+
"consider removing the underscore, or making the field private",
79+
);
80+
}
81+
}
82+
}
83+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
pub-underscore-fields-behavior = "AllPubFields"
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
pub-underscore-fields-behavior = "PublicallyExported"
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
error: field marked as public but also inferred as unused because it's prefixed with `_`
2+
--> $DIR/pub_underscore_fields.rs:15:9
3+
|
4+
LL | pub _b: u8,
5+
| ^^^^^^
6+
|
7+
= help: consider removing the underscore, or making the field private
8+
= note: `-D clippy::pub-underscore-fields` implied by `-D warnings`
9+
= help: to override `-D warnings` add `#[allow(clippy::pub_underscore_fields)]`
10+
11+
error: field marked as public but also inferred as unused because it's prefixed with `_`
12+
--> $DIR/pub_underscore_fields.rs:23:13
13+
|
14+
LL | pub(in crate::inner) _f: Option<()>,
15+
| ^^^^^^^^^^^^^^^^^^^^^^^
16+
|
17+
= help: consider removing the underscore, or making the field private
18+
19+
error: field marked as public but also inferred as unused because it's prefixed with `_`
20+
--> $DIR/pub_underscore_fields.rs:27:13
21+
|
22+
LL | pub _g: String,
23+
| ^^^^^^
24+
|
25+
= help: consider removing the underscore, or making the field private
26+
27+
error: field marked as public but also inferred as unused because it's prefixed with `_`
28+
--> $DIR/pub_underscore_fields.rs:34:9
29+
|
30+
LL | pub _a: usize,
31+
| ^^^^^^
32+
|
33+
= help: consider removing the underscore, or making the field private
34+
35+
error: field marked as public but also inferred as unused because it's prefixed with `_`
36+
--> $DIR/pub_underscore_fields.rs:41:9
37+
|
38+
LL | pub _c: i64,
39+
| ^^^^^^
40+
|
41+
= help: consider removing the underscore, or making the field private
42+
43+
error: field marked as public but also inferred as unused because it's prefixed with `_`
44+
--> $DIR/pub_underscore_fields.rs:44:9
45+
|
46+
LL | pub _e: Option<u8>,
47+
| ^^^^^^
48+
|
49+
= help: consider removing the underscore, or making the field private
50+
51+
error: field marked as public but also inferred as unused because it's prefixed with `_`
52+
--> $DIR/pub_underscore_fields.rs:57:9
53+
|
54+
LL | pub(crate) _b: Option<String>,
55+
| ^^^^^^^^^^^^^
56+
|
57+
= help: consider removing the underscore, or making the field private
58+
59+
error: aborting due to 7 previous errors
60+
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
error: field marked as public but also inferred as unused because it's prefixed with `_`
2+
--> $DIR/pub_underscore_fields.rs:15:9
3+
|
4+
LL | pub _b: u8,
5+
| ^^^^^^
6+
|
7+
= help: consider removing the underscore, or making the field private
8+
= note: `-D clippy::pub-underscore-fields` implied by `-D warnings`
9+
= help: to override `-D warnings` add `#[allow(clippy::pub_underscore_fields)]`
10+
11+
error: aborting due to 1 previous error
12+
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
//@revisions: exported all_pub_fields
2+
//@[all_pub_fields] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/pub_underscore_fields/all_pub_fields
3+
//@[exported] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/pub_underscore_fields/exported
4+
5+
#![allow(unused)]
6+
#![warn(clippy::pub_underscore_fields)]
7+
8+
use std::marker::PhantomData;
9+
10+
pub mod inner {
11+
use std::marker;
12+
13+
pub struct PubSuper {
14+
pub(super) a: usize,
15+
pub _b: u8,
16+
_c: i32,
17+
pub _mark: marker::PhantomData<u8>,
18+
}
19+
20+
mod inner2 {
21+
pub struct PubModVisibility {
22+
pub(in crate::inner) e: bool,
23+
pub(in crate::inner) _f: Option<()>,
24+
}
25+
26+
struct PrivateStructPubField {
27+
pub _g: String,
28+
}
29+
}
30+
}
31+
32+
fn main() {
33+
pub struct StructWithOneViolation {
34+
pub _a: usize,
35+
}
36+
37+
// should handle structs with multiple violations
38+
pub struct StructWithMultipleViolations {
39+
a: u8,
40+
_b: usize,
41+
pub _c: i64,
42+
#[doc(hidden)]
43+
pub d: String,
44+
pub _e: Option<u8>,
45+
}
46+
47+
// shouldn't warn on anonymous fields
48+
pub struct AnonymousFields(pub usize, i32);
49+
50+
// don't warn on empty structs
51+
pub struct Empty1;
52+
pub struct Empty2();
53+
pub struct Empty3 {};
54+
55+
pub struct PubCrate {
56+
pub(crate) a: String,
57+
pub(crate) _b: Option<String>,
58+
}
59+
60+
// shouldn't warn on fields named pub
61+
pub struct NamedPub {
62+
r#pub: bool,
63+
_pub: String,
64+
pub(crate) _mark: PhantomData<u8>,
65+
}
66+
}

tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr

+2
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ error: error reading Clippy's configuration file: unknown field `foobar`, expect
4949
missing-docs-in-crate-items
5050
msrv
5151
pass-by-value-size-limit
52+
pub-underscore-fields-behavior
5253
semicolon-inside-block-ignore-singleline
5354
semicolon-outside-block-ignore-multiline
5455
single-char-binding-names-threshold
@@ -124,6 +125,7 @@ error: error reading Clippy's configuration file: unknown field `barfoo`, expect
124125
missing-docs-in-crate-items
125126
msrv
126127
pass-by-value-size-limit
128+
pub-underscore-fields-behavior
127129
semicolon-inside-block-ignore-singleline
128130
semicolon-outside-block-ignore-multiline
129131
single-char-binding-names-threshold

0 commit comments

Comments
 (0)