From a715ae4b32c4230d62e6404c9907e7bbe7707b57 Mon Sep 17 00:00:00 2001 From: Parker Timmerman Date: Fri, 3 Feb 2023 10:17:07 -0500 Subject: [PATCH] branch start, add new lint pub_underscore_fields, add ui tests --- CHANGELOG.md | 1 + clippy_lints/src/declared_lints.rs | 1 + clippy_lints/src/lib.rs | 2 + clippy_lints/src/pub_underscore_fields.rs | 57 ++++++++++++++++++++ tests/ui/pub_underscore_fields.rs | 51 ++++++++++++++++++ tests/ui/pub_underscore_fields.stderr | 64 +++++++++++++++++++++++ 6 files changed, 176 insertions(+) create mode 100644 clippy_lints/src/pub_underscore_fields.rs create mode 100644 tests/ui/pub_underscore_fields.rs create mode 100644 tests/ui/pub_underscore_fields.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 659e8aebcd57..432239c0929a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4673,6 +4673,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 [`question_mark`]: https://rust-lang.github.io/rust-clippy/master/index.html#question_mark [`range_minus_one`]: https://rust-lang.github.io/rust-clippy/master/index.html#range_minus_one diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 457a25826e79..a418ce126ff5 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -504,6 +504,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::ranges::MANUAL_RANGE_CONTAINS_INFO, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 2f5c4adca9f1..6dd9d65a5a8f 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -241,6 +241,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 ranges; @@ -912,6 +913,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|_| Box::new(size_of_ref::SizeOfRef)); store.register_late_pass(|_| Box::new(multiple_unsafe_ops_per_block::MultipleUnsafeOpsPerBlock)); store.register_late_pass(|_| Box::new(extra_unused_type_parameters::ExtraUnusedTypeParameters)); + store.register_early_pass(|| Box::new(pub_underscore_fields::PubUnderscoreFields)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/pub_underscore_fields.rs b/clippy_lints/src/pub_underscore_fields.rs new file mode 100644 index 000000000000..4bb5295b134a --- /dev/null +++ b/clippy_lints/src/pub_underscore_fields.rs @@ -0,0 +1,57 @@ +use clippy_utils::diagnostics::span_lint_and_help; +use rustc_ast::ast::{Item, ItemKind, VisibilityKind}; +use rustc_lint::{EarlyContext, EarlyLintPass}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; + +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, + /// } + /// ``` + #[clippy::version = "1.69.0"] + pub PUB_UNDERSCORE_FIELDS, + pedantic, + "struct field prefixed with underscore and marked public" +} +declare_lint_pass!(PubUnderscoreFields => [PUB_UNDERSCORE_FIELDS]); + +impl EarlyLintPass for PubUnderscoreFields { + fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) { + // this only pertains to structs + let ItemKind::Struct(ref st, _) = item.kind else { + return; + }; + + let msg = "field marked as public but also inferred as unused because it's prefixed with `_`"; + for field in st.fields().iter() { + if let Some(ident) = field.ident.as_ref() + && ident.as_str().starts_with('_') + && !matches!(field.vis.kind, VisibilityKind::Inherited) { + span_lint_and_help( + cx, + PUB_UNDERSCORE_FIELDS, + field.vis.span.to(ident.span), + msg, + None, + "consider removing the underscore, or making the field private", + ); + } + } + } +} diff --git a/tests/ui/pub_underscore_fields.rs b/tests/ui/pub_underscore_fields.rs new file mode 100644 index 000000000000..7d513e8cfcf8 --- /dev/null +++ b/tests/ui/pub_underscore_fields.rs @@ -0,0 +1,51 @@ +#![allow(unused)] +#![warn(clippy::pub_underscore_fields)] + +fn main() { + pub struct StructWithOneViolation { + pub _a: usize, + } + + // should handle structs with multiple violations + pub struct StructWithMultipleViolations { + a: u8, + _b: usize, + pub _c: i64, + pub d: String, + pub _e: Option, + } + + // 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, + } + + // shouldn't warn on fields named pub + pub struct NamedPub { + r#pub: bool, + _pub: String, + } + + mod inner { + pub struct PubSuper { + pub(super) a: usize, + pub(super) _b: u8, + _c: i32, + } + } + + mod inner2 { + pub struct PubModVisibility { + pub(in super::inner) e: bool, + pub(in super::inner) _f: Option<()>, + } + } +} diff --git a/tests/ui/pub_underscore_fields.stderr b/tests/ui/pub_underscore_fields.stderr new file mode 100644 index 000000000000..885c7d1357b0 --- /dev/null +++ b/tests/ui/pub_underscore_fields.stderr @@ -0,0 +1,64 @@ +error[E0433]: failed to resolve: could not find `inner` in the crate root + --> $DIR/pub_underscore_fields.rs:47:27 + | +LL | pub(in super::inner) e: bool, + | ^^^^^ could not find `inner` in the crate root + +error[E0433]: failed to resolve: could not find `inner` in the crate root + --> $DIR/pub_underscore_fields.rs:48:27 + | +LL | pub(in super::inner) _f: Option<()>, + | ^^^^^ could not find `inner` in the crate root + +error: field marked as public but also inferred as unused because it's prefixed with `_` + --> $DIR/pub_underscore_fields.rs:6:9 + | +LL | pub _a: usize, + | ^^^^^^ + | + = help: consider removing the underscore, or making the field private + = note: `-D clippy::pub-underscore-fields` implied by `-D warnings` + +error: field marked as public but also inferred as unused because it's prefixed with `_` + --> $DIR/pub_underscore_fields.rs:13: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:15:9 + | +LL | pub _e: 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:28:9 + | +LL | pub(crate) _b: 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:40:13 + | +LL | pub(super) _b: 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:48:13 + | +LL | pub(in super::inner) _f: Option<()>, + | ^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: consider removing the underscore, or making the field private + +error: aborting due to 8 previous errors + +For more information about this error, try `rustc --explain E0433`.