add a new lint, pub_underscore_fields

- add a new late pass lint
- add ui tests
- update CHANGELOG.md

CHANGELOG.md

@@ -5357,6 +5357,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

clippy_lints/src/declared_lints.rs

@@ -561,6 +561,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,

clippy_lints/src/lib.rs

@@ -271,6 +271,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;
@@ -1123,6 +1124,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     });
     store.register_late_pass(move |_| Box::new(manual_hash_one::ManualHashOne::new(msrv())));
     store.register_late_pass(|_| Box::new(iter_without_into_iter::IterWithoutIntoIter));
+    store.register_late_pass(|_| Box::new(pub_underscore_fields::PubUnderscoreFields));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }
 

clippy_lints/src/pub_underscore_fields.rs

@@ -0,0 +1,82 @@
+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_middle::ty::Visibility;
+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,
+    /// }
+    /// ```
+    ///
+    /// // OR
+    ///
+    /// ```rust
+    /// struct FileHandle {
+    ///     pub descriptor: usize,
+    /// }
+    /// ```
+    #[clippy::version = "1.75.0"]
+    pub PUB_UNDERSCORE_FIELDS,
+    pedantic,
+    "struct field prefixed with underscore and marked public"
+}
+declare_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<'_>| {
+            let parent = cx.tcx.parent_module_from_def_id(field.def_id);
+            let grandparent = cx.tcx.parent_module_from_def_id(parent.into());
+            let visibility = cx.tcx.visibility(field.def_id);
+
+            let case_1 = parent == grandparent && !field.vis_span.is_empty();
+            let case_2 = visibility != Visibility::Restricted(parent.to_def_id());
+
+            case_1 || case_2
+        };
+
+        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",
+                );
+            }
+        }
+    }
+}

tests/ui/pub_underscore_fields.rs

@@ -0,0 +1,58 @@
+#![allow(unused)]
+#![warn(clippy::pub_underscore_fields)]
+
+use std::marker::PhantomData;
+
+mod inner {
+    use std::marker;
+
+    pub struct PubSuper {
+        pub(super) a: usize,
+        pub(super) _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<()>,
+        }
+    }
+}
+
+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>,
+    }
+}

tests/ui/pub_underscore_fields.stderr

@@ -0,0 +1,52 @@
+error: field marked as public but also inferred as unused because it's prefixed with `_`
+  --> $DIR/pub_underscore_fields.rs:11:9
+   |
+LL |         pub(super) _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:19: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:26: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:33: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:36: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:49:9
+   |
+LL |         pub(crate) _b: Option<String>,
+   |         ^^^^^^^^^^^^^
+   |
+   = help: consider removing the underscore, or making the field private
+
+error: aborting due to 6 previous errors
+