-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add field_scoped_visibility_modifiers lint
- Loading branch information
1 parent
6cfd4ac
commit e12bae5
Showing
6 changed files
with
120 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,67 @@ | ||
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_clippy_lint! { | ||
/// ### What it does | ||
/// Checks for usage of scoped visibility modifiers, like `pub(crate)`, on fields. These | ||
/// make a field visible within a scope between public and private. | ||
/// | ||
/// ### Why restrict this? | ||
/// Scoped visibility modifiers cause a field to be accessible within some scope between | ||
/// public and private, potentially within an entire crate. This allows for fields to be | ||
/// non-private while upholding internal invariants, but can be a code smell. Scoped visibility | ||
/// requires checking a greater area, potentially an entire crate, to verify that an invariant | ||
/// is upheld, and global analysis requires a lot of effort. | ||
/// | ||
/// ### Example | ||
/// ```no_run | ||
/// pub mod public_module { | ||
/// struct MyStruct { | ||
/// pub(crate) first_field: bool, | ||
/// pub(super) second_field: bool | ||
/// } | ||
/// } | ||
/// ``` | ||
/// Use instead: | ||
/// ```no_run | ||
/// pub mod public_module { | ||
/// struct MyStruct { | ||
/// pub first_field: bool, | ||
/// pub second_field: bool | ||
/// } | ||
/// } | ||
/// ``` | ||
#[clippy::version = "1.78.0"] | ||
pub FIELD_SCOPED_VISIBILITY_MODIFIERS, | ||
restriction, | ||
"checks for usage of a scoped visibility modifier, like `pub(crate)`, on fields" | ||
} | ||
|
||
declare_lint_pass!(FieldScopedVisibilityModifiers => [FIELD_SCOPED_VISIBILITY_MODIFIERS]); | ||
|
||
impl EarlyLintPass for FieldScopedVisibilityModifiers { | ||
fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) { | ||
let ItemKind::Struct(ref st, _) = item.kind else { | ||
return; | ||
}; | ||
for field in st.fields() { | ||
let VisibilityKind::Restricted { path, .. } = &field.vis.kind else { | ||
continue; | ||
}; | ||
if !path.segments.is_empty() && path.segments[0].ident.as_str() == "self" { | ||
// pub(self) is equivalent to not using pub at all, so we ignore it | ||
continue; | ||
} | ||
span_lint_and_help( | ||
cx, | ||
FIELD_SCOPED_VISIBILITY_MODIFIERS, | ||
field.vis.span, | ||
"scoped visibility modifier on a field", | ||
None, | ||
"consider making the field either public or private", | ||
); | ||
} | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
#![warn(clippy::field_scoped_visibility_modifiers)] | ||
|
||
pub mod pub_module { | ||
pub(in crate::pub_module) mod pub_in_path_module {} | ||
pub(super) mod pub_super_module {} | ||
struct MyStruct { | ||
private_field: bool, | ||
pub pub_field: bool, | ||
pub(crate) pub_crate_field: bool, | ||
pub(in crate::pub_module) pub_in_path_field: bool, | ||
pub(super) pub_super_field: bool, | ||
#[allow(clippy::needless_pub_self)] | ||
pub(self) pub_self_field: bool, | ||
} | ||
} | ||
pub(crate) mod pub_crate_module {} | ||
|
||
#[allow(clippy::needless_pub_self)] | ||
pub(self) mod pub_self_module {} | ||
|
||
fn main() {} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
error: scoped visibility modifier on a field | ||
--> tests/ui/field_scoped_visibility_modifiers.rs:9:9 | ||
| | ||
LL | pub(crate) pub_crate_field: bool, | ||
| ^^^^^^^^^^ | ||
| | ||
= help: consider making the field either public or private | ||
= note: `-D clippy::field-scoped-visibility-modifiers` implied by `-D warnings` | ||
= help: to override `-D warnings` add `#[allow(clippy::field_scoped_visibility_modifiers)]` | ||
|
||
error: scoped visibility modifier on a field | ||
--> tests/ui/field_scoped_visibility_modifiers.rs:10:9 | ||
| | ||
LL | pub(in crate::pub_module) pub_in_path_field: bool, | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
| | ||
= help: consider making the field either public or private | ||
|
||
error: scoped visibility modifier on a field | ||
--> tests/ui/field_scoped_visibility_modifiers.rs:11:9 | ||
| | ||
LL | pub(super) pub_super_field: bool, | ||
| ^^^^^^^^^^ | ||
| | ||
= help: consider making the field either public or private | ||
|
||
error: aborting due to 3 previous errors | ||
|