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`
This commit is contained in:
Parker Timmerman 2023-12-17 15:55:06 -05:00 committed by blyxyas
parent 7343db9ce3
commit fa7dd1c4e0
No known key found for this signature in database
GPG key ID: 9698805551971573
13 changed files with 257 additions and 1 deletions

View file

@ -5483,6 +5483,7 @@ Released 2018-09-13
[`ptr_eq`]: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_eq
[`ptr_offset_with_cast`]: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_offset_with_cast
[`pub_enum_variant_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#pub_enum_variant_names
[`pub_underscore_fields`]: https://rust-lang.github.io/rust-clippy/master/index.html#pub_underscore_fields
[`pub_use`]: https://rust-lang.github.io/rust-clippy/master/index.html#pub_use
[`pub_with_shorthand`]: https://rust-lang.github.io/rust-clippy/master/index.html#pub_with_shorthand
[`pub_without_shorthand`]: https://rust-lang.github.io/rust-clippy/master/index.html#pub_without_shorthand
@ -5810,4 +5811,5 @@ Released 2018-09-13
[`allowed-dotfiles`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allowed-dotfiles
[`enforce-iter-loop-reborrow`]: https://doc.rust-lang.org/clippy/lint_configuration.html#enforce-iter-loop-reborrow
[`check-private-items`]: https://doc.rust-lang.org/clippy/lint_configuration.html#check-private-items
[`pub-underscore-fields-behavior`]: https://doc.rust-lang.org/clippy/lint_configuration.html#pub-underscore-fields-behavior
<!-- end autogenerated links to configuration documentation -->

View file

@ -805,3 +805,13 @@ for _ in &mut *rmvec {}
* [`missing_errors_doc`](https://rust-lang.github.io/rust-clippy/master/index.html#missing_errors_doc)
## `pub-underscore-fields-behavior`
**Default Value:** `"PublicallyExported"`
---
**Affected lints:**
* [`pub_underscore_fields`](https://rust-lang.github.io/rust-clippy/master/index.html#pub_underscore_fields)

View file

@ -1,5 +1,5 @@
use crate::msrvs::Msrv;
use crate::types::{DisallowedPath, MacroMatcher, MatchLintBehaviour, Rename};
use crate::types::{DisallowedPath, MacroMatcher, MatchLintBehaviour, PubUnderscoreFieldsBehaviour, Rename};
use crate::ClippyConfiguration;
use rustc_data_structures::fx::FxHashSet;
use rustc_session::Session;
@ -547,6 +547,11 @@ define_Conf! {
///
/// Whether to also run the listed lints on private items.
(check_private_items: bool = false),
/// Lint: PUB_UNDERSCORE_FIELDS
///
/// Lint "public" fields in a struct that are prefixed with an underscore based on their
/// exported visibility; or whether they are marked as "pub".
(pub_underscore_fields_behavior: PubUnderscoreFieldsBehaviour = PubUnderscoreFieldsBehaviour::PublicallyExported),
}
/// Search for the configuration file.

View file

@ -126,3 +126,9 @@ unimplemented_serialize! {
Rename,
MacroMatcher,
}
#[derive(Clone, Copy, Debug, PartialEq, Eq, Deserialize, Serialize)]
pub enum PubUnderscoreFieldsBehaviour {
PublicallyExported,
AllPubFields,
}

View file

@ -576,6 +576,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::ptr::MUT_FROM_REF_INFO,
crate::ptr::PTR_ARG_INFO,
crate::ptr_offset_with_cast::PTR_OFFSET_WITH_CAST_INFO,
crate::pub_underscore_fields::PUB_UNDERSCORE_FIELDS_INFO,
crate::pub_use::PUB_USE_INFO,
crate::question_mark::QUESTION_MARK_INFO,
crate::question_mark_used::QUESTION_MARK_USED_INFO,

View file

@ -272,6 +272,7 @@ mod permissions_set_readonly_false;
mod precedence;
mod ptr;
mod ptr_offset_with_cast;
mod pub_underscore_fields;
mod pub_use;
mod question_mark;
mod question_mark_used;
@ -571,6 +572,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
verbose_bit_mask_threshold,
warn_on_all_wildcard_imports,
check_private_items,
pub_underscore_fields_behavior,
blacklisted_names: _,
cyclomatic_complexity_threshold: _,
@ -1081,6 +1083,11 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
store.register_late_pass(|_| Box::new(uninhabited_references::UninhabitedReferences));
store.register_late_pass(|_| Box::new(ineffective_open_options::IneffectiveOpenOptions));
store.register_late_pass(|_| Box::new(unconditional_recursion::UnconditionalRecursion));
store.register_late_pass(move |_| {
Box::new(pub_underscore_fields::PubUnderscoreFields {
behavior: pub_underscore_fields_behavior,
})
});
// add lints here, do not remove this comment, it's used in `new_lint`
}

View file

@ -0,0 +1,83 @@
use clippy_config::types::PubUnderscoreFieldsBehaviour;
use clippy_utils::attrs::is_doc_hidden;
use clippy_utils::diagnostics::span_lint_and_help;
use clippy_utils::is_path_lang_item;
use rustc_hir::{FieldDef, Item, ItemKind, LangItem};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::impl_lint_pass;
declare_clippy_lint! {
/// ### What it does
/// Checks whether any field of the struct is prefixed with an `_` (underscore) and also marked
/// `pub` (public)
///
/// ### Why is this bad?
/// Fields prefixed with an `_` are inferred as unused, which suggests it should not be marked
/// as `pub`, because marking it as `pub` infers it will be used.
///
/// ### Example
/// ```rust
/// struct FileHandle {
/// pub _descriptor: usize,
/// }
/// ```
/// Use instead:
/// ```rust
/// struct FileHandle {
/// _descriptor: usize,
/// }
/// ```
///
/// OR
///
/// ```rust
/// struct FileHandle {
/// pub descriptor: usize,
/// }
/// ```
#[clippy::version = "1.77.0"]
pub PUB_UNDERSCORE_FIELDS,
pedantic,
"struct field prefixed with underscore and marked public"
}
pub struct PubUnderscoreFields {
pub behavior: PubUnderscoreFieldsBehaviour,
}
impl_lint_pass!(PubUnderscoreFields => [PUB_UNDERSCORE_FIELDS]);
impl<'tcx> LateLintPass<'tcx> for PubUnderscoreFields {
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) {
// This lint only pertains to structs.
let ItemKind::Struct(variant_data, _) = &item.kind else {
return;
};
let is_visible = |field: &FieldDef<'_>| match self.behavior {
PubUnderscoreFieldsBehaviour::PublicallyExported => cx.effective_visibilities.is_reachable(field.def_id),
PubUnderscoreFieldsBehaviour::AllPubFields => {
// If there is a visibility span then the field is marked pub in some way.
!field.vis_span.is_empty()
},
};
for field in variant_data.fields() {
// Only pertains to fields that start with an underscore, and are public.
if field.ident.as_str().starts_with('_') && is_visible(field)
// We ignore fields that have `#[doc(hidden)]`.
&& !is_doc_hidden(cx.tcx.hir().attrs(field.hir_id))
// We ignore fields that are `PhantomData`.
&& !is_path_lang_item(cx, field.ty, LangItem::PhantomData)
{
span_lint_and_help(
cx,
PUB_UNDERSCORE_FIELDS,
field.vis_span.to(field.ident.span),
"field marked as public but also inferred as unused because it's prefixed with `_`",
None,
"consider removing the underscore, or making the field private",
);
}
}
}
}

View file

@ -0,0 +1 @@
pub-underscore-fields-behavior = "AllPubFields"

View file

@ -0,0 +1 @@
pub-underscore-fields-behavior = "PublicallyExported"

View file

@ -0,0 +1,60 @@
error: field marked as public but also inferred as unused because it's prefixed with `_`
--> $DIR/pub_underscore_fields.rs:15:9
|
LL | pub _b: u8,
| ^^^^^^
|
= help: consider removing the underscore, or making the field private
= note: `-D clippy::pub-underscore-fields` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::pub_underscore_fields)]`
error: field marked as public but also inferred as unused because it's prefixed with `_`
--> $DIR/pub_underscore_fields.rs:23:13
|
LL | pub(in crate::inner) _f: Option<()>,
| ^^^^^^^^^^^^^^^^^^^^^^^
|
= help: consider removing the underscore, or making the field private
error: field marked as public but also inferred as unused because it's prefixed with `_`
--> $DIR/pub_underscore_fields.rs:27:13
|
LL | pub _g: String,
| ^^^^^^
|
= help: consider removing the underscore, or making the field private
error: field marked as public but also inferred as unused because it's prefixed with `_`
--> $DIR/pub_underscore_fields.rs:34:9
|
LL | pub _a: usize,
| ^^^^^^
|
= help: consider removing the underscore, or making the field private
error: field marked as public but also inferred as unused because it's prefixed with `_`
--> $DIR/pub_underscore_fields.rs:41:9
|
LL | pub _c: i64,
| ^^^^^^
|
= help: consider removing the underscore, or making the field private
error: field marked as public but also inferred as unused because it's prefixed with `_`
--> $DIR/pub_underscore_fields.rs:44:9
|
LL | pub _e: Option<u8>,
| ^^^^^^
|
= help: consider removing the underscore, or making the field private
error: field marked as public but also inferred as unused because it's prefixed with `_`
--> $DIR/pub_underscore_fields.rs:57:9
|
LL | pub(crate) _b: Option<String>,
| ^^^^^^^^^^^^^
|
= help: consider removing the underscore, or making the field private
error: aborting due to 7 previous errors

View file

@ -0,0 +1,12 @@
error: field marked as public but also inferred as unused because it's prefixed with `_`
--> $DIR/pub_underscore_fields.rs:15:9
|
LL | pub _b: u8,
| ^^^^^^
|
= help: consider removing the underscore, or making the field private
= note: `-D clippy::pub-underscore-fields` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::pub_underscore_fields)]`
error: aborting due to 1 previous error

View file

@ -0,0 +1,66 @@
//@revisions: exported all_pub_fields
//@[all_pub_fields] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/pub_underscore_fields/all_pub_fields
//@[exported] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/pub_underscore_fields/exported
#![allow(unused)]
#![warn(clippy::pub_underscore_fields)]
use std::marker::PhantomData;
pub mod inner {
use std::marker;
pub struct PubSuper {
pub(super) a: usize,
pub _b: u8,
_c: i32,
pub _mark: marker::PhantomData<u8>,
}
mod inner2 {
pub struct PubModVisibility {
pub(in crate::inner) e: bool,
pub(in crate::inner) _f: Option<()>,
}
struct PrivateStructPubField {
pub _g: String,
}
}
}
fn main() {
pub struct StructWithOneViolation {
pub _a: usize,
}
// should handle structs with multiple violations
pub struct StructWithMultipleViolations {
a: u8,
_b: usize,
pub _c: i64,
#[doc(hidden)]
pub d: String,
pub _e: Option<u8>,
}
// shouldn't warn on anonymous fields
pub struct AnonymousFields(pub usize, i32);
// don't warn on empty structs
pub struct Empty1;
pub struct Empty2();
pub struct Empty3 {};
pub struct PubCrate {
pub(crate) a: String,
pub(crate) _b: Option<String>,
}
// shouldn't warn on fields named pub
pub struct NamedPub {
r#pub: bool,
_pub: String,
pub(crate) _mark: PhantomData<u8>,
}
}

View file

@ -49,6 +49,7 @@ error: error reading Clippy's configuration file: unknown field `foobar`, expect
missing-docs-in-crate-items
msrv
pass-by-value-size-limit
pub-underscore-fields-behavior
semicolon-inside-block-ignore-singleline
semicolon-outside-block-ignore-multiline
single-char-binding-names-threshold
@ -124,6 +125,7 @@ error: error reading Clippy's configuration file: unknown field `barfoo`, expect
missing-docs-in-crate-items
msrv
pass-by-value-size-limit
pub-underscore-fields-behavior
semicolon-inside-block-ignore-singleline
semicolon-outside-block-ignore-multiline
single-char-binding-names-threshold