Skip to content

Commit

Permalink
Add new lint partial_pub_fields
Browse files Browse the repository at this point in the history
Signed-off-by: TennyZhuang <[email protected]>
  • Loading branch information
TennyZhuang committed Oct 16, 2022
1 parent ff33d6e commit 40d6387
Show file tree
Hide file tree
Showing 9 changed files with 152 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_nursery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ store.register_group(true, "clippy::nursery", Some("clippy_nursery"), vec![
LintId::of(non_send_fields_in_send_ty::NON_SEND_FIELDS_IN_SEND_TY),
LintId::of(nonstandard_macro_braces::NONSTANDARD_MACRO_BRACES),
LintId::of(option_if_let_else::OPTION_IF_LET_ELSE),
LintId::of(partial_pub_fields::PARTIAL_PUB_FIELDS),
LintId::of(redundant_pub_crate::REDUNDANT_PUB_CRATE),
LintId::of(regex::TRIVIAL_REGEX),
LintId::of(strings::STRING_LIT_AS_BYTES),
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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`
}

Expand Down
81 changes: 81 additions & 0 deletions clippy_lints/src/partial_pub_fields.rs
Original file line number Diff line number Diff line change
@@ -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;
}
}
}
}
1 change: 1 addition & 0 deletions src/docs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
26 changes: 26 additions & 0 deletions src/docs/partial_pub_fields.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
### 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,
}
```
20 changes: 20 additions & 0 deletions tests/ui/partial_pub_fields.rs
Original file line number Diff line number Diff line change
@@ -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,
}
}
19 changes: 19 additions & 0 deletions tests/ui/partial_pub_fields.stderr
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 40d6387

Please sign in to comment.