about summary refs log tree commit diff
diff options
context:
space:
mode:
authorTennyZhuang <zty0826@gmail.com>2022-10-16 16:02:23 +0800
committerTennyZhuang <zty0826@gmail.com>2022-10-16 16:03:50 +0800
commit7ac97b69fc81fcd5cbd2a7c862187f6a6c6ea354 (patch)
treecbb596de635702a984233c4ede7e2aba31a2fc6e
parentff33d6e712ebf1068e8d6109e56f2f5d1e82054c (diff)
downloadrust-7ac97b69fc81fcd5cbd2a7c862187f6a6c6ea354.tar.gz
rust-7ac97b69fc81fcd5cbd2a7c862187f6a6c6ea354.zip
Add new lint `partial_pub_fields`
Signed-off-by: TennyZhuang <zty0826@gmail.com>
-rw-r--r--CHANGELOG.md1
-rw-r--r--clippy_lints/src/lib.register_lints.rs1
-rw-r--r--clippy_lints/src/lib.register_restriction.rs1
-rw-r--r--clippy_lints/src/lib.rs2
-rw-r--r--clippy_lints/src/partial_pub_fields.rs81
-rw-r--r--src/docs.rs1
-rw-r--r--src/docs/partial_pub_fields.txt27
-rw-r--r--tests/ui/partial_pub_fields.rs20
-rw-r--r--tests/ui/partial_pub_fields.stderr19
9 files changed, 153 insertions, 0 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index f593966c045..1e0ff5db0ee 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4134,6 +4134,7 @@ Released 2018-09-13
 [`panic_in_result_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#panic_in_result_fn
 [`panic_params`]: https://rust-lang.github.io/rust-clippy/master/index.html#panic_params
 [`panicking_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#panicking_unwrap
+[`partial_pub_fields`]: https://rust-lang.github.io/rust-clippy/master/index.html#partial_pub_fields
 [`partialeq_ne_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#partialeq_ne_impl
 [`partialeq_to_none`]: https://rust-lang.github.io/rust-clippy/master/index.html#partialeq_to_none
 [`path_buf_push_overwrite`]: https://rust-lang.github.io/rust-clippy/master/index.html#path_buf_push_overwrite
diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs
index de1253c8510..799c62743aa 100644
--- a/clippy_lints/src/lib.register_lints.rs
+++ b/clippy_lints/src/lib.register_lints.rs
@@ -479,6 +479,7 @@ store.register_lints(&[
     panic_unimplemented::TODO,
     panic_unimplemented::UNIMPLEMENTED,
     panic_unimplemented::UNREACHABLE,
+    partial_pub_fields::PARTIAL_PUB_FIELDS,
     partialeq_ne_impl::PARTIALEQ_NE_IMPL,
     partialeq_to_none::PARTIALEQ_TO_NONE,
     pass_by_ref_or_value::LARGE_TYPES_PASSED_BY_VALUE,
diff --git a/clippy_lints/src/lib.register_restriction.rs b/clippy_lints/src/lib.register_restriction.rs
index 6eb9b3d3b9b..9edced28408 100644
--- a/clippy_lints/src/lib.register_restriction.rs
+++ b/clippy_lints/src/lib.register_restriction.rs
@@ -61,6 +61,7 @@ store.register_group(true, "clippy::restriction", Some("clippy_restriction"), ve
     LintId::of(panic_unimplemented::TODO),
     LintId::of(panic_unimplemented::UNIMPLEMENTED),
     LintId::of(panic_unimplemented::UNREACHABLE),
+    LintId::of(partial_pub_fields::PARTIAL_PUB_FIELDS),
     LintId::of(pattern_type_mismatch::PATTERN_TYPE_MISMATCH),
     LintId::of(pub_use::PUB_USE),
     LintId::of(redundant_slicing::DEREF_BY_SLICING),
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index ebb0f14fef5..bf5688829e2 100644
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -324,6 +324,7 @@ mod option_if_let_else;
 mod overflow_check_conditional;
 mod panic_in_result_fn;
 mod panic_unimplemented;
+mod partial_pub_fields;
 mod partialeq_ne_impl;
 mod partialeq_to_none;
 mod pass_by_ref_or_value;
@@ -908,6 +909,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|_| Box::new(bool_to_int_with_if::BoolToIntWithIf));
     store.register_late_pass(|_| Box::new(box_default::BoxDefault));
     store.register_late_pass(|_| Box::new(implicit_saturating_add::ImplicitSaturatingAdd));
+    store.register_early_pass(|| Box::new(partial_pub_fields::PartialPubFields));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }
 
diff --git a/clippy_lints/src/partial_pub_fields.rs b/clippy_lints/src/partial_pub_fields.rs
new file mode 100644
index 00000000000..085ee08afca
--- /dev/null
+++ b/clippy_lints/src/partial_pub_fields.rs
@@ -0,0 +1,81 @@
+use clippy_utils::diagnostics::span_lint_and_help;
+use rustc_ast::ast::*;
+use rustc_lint::{EarlyContext, EarlyLintPass};
+use rustc_session::{declare_lint_pass, declare_tool_lint};
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks whether partial fields of a struct are public.
+    ///
+    /// Either make all fields of a type public, or make none of them public
+    ///
+    /// ### Why is this bad?
+    /// Most types should either be:
+    /// * Abstract data types: complex objects with opaque implementation which guard
+    /// interior invariants and expose intentionally limited API to the outside world.
+    /// * Data: relatively simple objects which group a bunch of related attributes together.
+    ///
+    /// ### Example
+    /// ```rust
+    /// pub struct Color {
+    ///     pub r,
+    ///     pub g,
+    ///     b,
+    /// }
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// pub struct Color {
+    ///     pub r,
+    ///     pub g,
+    ///     pub b,
+    /// }
+    /// ```
+    #[clippy::version = "1.66.0"]
+    pub PARTIAL_PUB_FIELDS,
+    restriction,
+    "partial fields of a struct are public"
+}
+declare_lint_pass!(PartialPubFields => [PARTIAL_PUB_FIELDS]);
+
+impl EarlyLintPass for PartialPubFields {
+    fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) {
+        let ItemKind::Struct(ref st, _) = item.kind else {
+            return;
+        };
+
+        let mut fields = st.fields().iter();
+        let Some(first_field) = fields.next() else {
+            // Empty struct.
+            return;
+        };
+        let all_pub = first_field.vis.kind.is_pub();
+        let all_priv = !all_pub;
+
+        let msg = "mixed usage of pub and non-pub fields";
+
+        for field in fields {
+            if all_priv && field.vis.kind.is_pub() {
+                span_lint_and_help(
+                    cx,
+                    &PARTIAL_PUB_FIELDS,
+                    field.vis.span,
+                    msg,
+                    None,
+                    "consider using private field here",
+                );
+                return;
+            } else if all_pub && !field.vis.kind.is_pub() {
+                span_lint_and_help(
+                    cx,
+                    &PARTIAL_PUB_FIELDS,
+                    field.vis.span,
+                    msg,
+                    None,
+                    "consider using public field here",
+                );
+                return;
+            }
+        }
+    }
+}
diff --git a/src/docs.rs b/src/docs.rs
index 41c31f91bca..b8b4286b488 100644
--- a/src/docs.rs
+++ b/src/docs.rs
@@ -395,6 +395,7 @@ docs! {
     "panic",
     "panic_in_result_fn",
     "panicking_unwrap",
+    "partial_pub_fields",
     "partialeq_ne_impl",
     "partialeq_to_none",
     "path_buf_push_overwrite",
diff --git a/src/docs/partial_pub_fields.txt b/src/docs/partial_pub_fields.txt
new file mode 100644
index 00000000000..a332ec8c28a
--- /dev/null
+++ b/src/docs/partial_pub_fields.txt
@@ -0,0 +1,27 @@
+### What it does
+Checks whether partial fields of a struct are public.
+
+Either make all fields of a type public, or make none of them public
+
+### Why is this bad?
+Most types should either be:
+* Abstract data types: complex objects with opaque implementation which guard
+interior invariants and expose intentionally limited API to the outside world.
+* Data: relatively simple objects which group a bunch of related attributes together.
+
+### Example
+```
+pub struct Color {
+    pub r,
+    pub g,
+    b,
+}
+```
+Use instead:
+```
+pub struct Color {
+    pub r,
+    pub g,
+    pub b,
+}
+```
\ No newline at end of file
diff --git a/tests/ui/partial_pub_fields.rs b/tests/ui/partial_pub_fields.rs
new file mode 100644
index 00000000000..e1a4b482799
--- /dev/null
+++ b/tests/ui/partial_pub_fields.rs
@@ -0,0 +1,20 @@
+#![allow(unused)]
+#![warn(clippy::partial_pub_fields)]
+
+fn main() {
+    // test code goes here
+
+    use std::collections::HashMap;
+
+    #[derive(Default)]
+    pub struct FileSet {
+        files: HashMap<String, u32>,
+        pub paths: HashMap<u32, String>,
+    }
+
+    pub struct Color {
+        pub r: u8,
+        pub g: u8,
+        b: u8,
+    }
+}
diff --git a/tests/ui/partial_pub_fields.stderr b/tests/ui/partial_pub_fields.stderr
new file mode 100644
index 00000000000..f7f23c85a67
--- /dev/null
+++ b/tests/ui/partial_pub_fields.stderr
@@ -0,0 +1,19 @@
+error: mixed usage of pub and non-pub fields
+  --> $DIR/partial_pub_fields.rs:12:9
+   |
+LL |         pub paths: HashMap<u32, String>,
+   |         ^^^
+   |
+   = help: consider using private field here
+   = note: `-D clippy::partial-pub-fields` implied by `-D warnings`
+
+error: mixed usage of pub and non-pub fields
+  --> $DIR/partial_pub_fields.rs:18:9
+   |
+LL |         b: u8,
+   |         ^
+   |
+   = help: consider using public field here
+
+error: aborting due to 2 previous errors
+