Skip to content

chore: support differentating between pub and pub(mod) #33

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
scottgerring wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

chore: support differentating between pub and pub(mod) #33

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions cargo_pup_lint_config/src/struct_lint/builder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,4 +148,10 @@ impl<'a> StructConstraintBuilder<'a> {
self.add_rule_internal(StructRule::MustBePublic(self.current_severity));
self
}

/// Add a rule requiring the struct to have pub(crate) visibility
pub fn must_be_pub_crate(mut self) -> Self {
self.add_rule_internal(StructRule::MustBePubCrate(self.current_severity));
self
}
}
6 changes: 4 additions & 2 deletions cargo_pup_lint_config/src/struct_lint/types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,12 @@ pub enum StructRule {
MustBeNamed(String, Severity),
/// Enforces that the struct name does not match the specified pattern
MustNotBeNamed(String, Severity),
/// Enforces that the struct has private visibility
/// Enforces that the struct has private visibility (not pub, not pub(crate), not pub(super))
MustBePrivate(Severity),
/// Enforces that the struct has public visibility
/// Enforces that the struct has public visibility (pub)
MustBePublic(Severity),
/// Enforces that the struct has pub(crate) visibility
MustBePubCrate(Severity),
/// Enforces that the struct implements a specific trait
ImplementsTrait(String, Severity),
/// Logical AND - both rules must pass
Expand Down
74 changes: 68 additions & 6 deletions cargo_pup_lint_impl/src/lints/struct_lint/lint.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,14 @@ declare_variable_severity_lint!(
"Struct must have public visibility"
);

declare_variable_severity_lint!(
pub,
STRUCT_LINT_MUST_BE_PUB_CRATE,
STRUCT_LINT_MUST_BE_PUB_CRATE_DENY,
STRUCT_LINT_MUST_BE_PUB_CRATE_WARN,
"Struct must have pub(crate) visibility"
);

impl_lint_pass!(StructLint => [
STRUCT_LINT_MUST_BE_NAMED_DENY,
STRUCT_LINT_MUST_BE_NAMED_WARN,
Expand All @@ -194,7 +202,9 @@ impl_lint_pass!(StructLint => [
STRUCT_LINT_MUST_BE_PRIVATE_DENY,
STRUCT_LINT_MUST_BE_PRIVATE_WARN,
STRUCT_LINT_MUST_BE_PUBLIC_DENY,
STRUCT_LINT_MUST_BE_PUBLIC_WARN
STRUCT_LINT_MUST_BE_PUBLIC_WARN,
STRUCT_LINT_MUST_BE_PUB_CRATE_DENY,
STRUCT_LINT_MUST_BE_PUB_CRATE_WARN
]);

impl ArchitectureLintRule for StructLint {
Expand Down Expand Up @@ -266,6 +276,22 @@ impl<'tcx> LateLintPass<'tcx> for StructLint {
let struct_visibility = ctx.tcx.visibility(def_id);
let is_public = struct_visibility == rustc_middle::ty::Visibility::Public;

// Check if there's a visibility keyword (pub, pub(crate), pub(super), etc.)
let has_visibility_keyword = !item.vis_span.is_empty();

// Check if visibility is pub(crate):
// - Must have a visibility keyword (not inherited/private)
// - Must be restricted to the crate root
let is_pub_crate = match struct_visibility {
rustc_middle::ty::Visibility::Restricted(restricted_to) => {
has_visibility_keyword && restricted_to.is_crate_root()
}
_ => false,
};

// Truly private means no visibility keyword at all (inherited visibility)
let is_private = !has_visibility_keyword;

// Apply rules
for rule in &self.struct_rules {
match rule {
Expand Down Expand Up @@ -317,28 +343,64 @@ impl<'tcx> LateLintPass<'tcx> for StructLint {
}
}
StructRule::MustBePrivate(severity) => {
if is_public {
if !is_private {
let visibility_desc = if is_public {
"pub"
} else if is_pub_crate {
"pub(crate)"
} else {
"restricted" // pub(super) or pub(in path)
};
span_lint_and_help(
ctx,
STRUCT_LINT_MUST_BE_PRIVATE::get_by_severity(*severity),
self.name().as_str(),
definition_span,
format!("Struct '{item_name}' is public, but must be private"),
format!(
"Struct '{item_name}' has {visibility_desc} visibility, but must be private"
),
None,
"Remove the 'pub' visibility modifier",
"Remove the visibility modifier",
);
}
}
StructRule::MustBePublic(severity) => {
if !is_public {
let visibility_desc = if is_pub_crate {
"pub(crate)"
} else {
"private"
};
span_lint_and_help(
ctx,
STRUCT_LINT_MUST_BE_PUBLIC::get_by_severity(*severity),
self.name().as_str(),
definition_span,
format!("Struct '{item_name}' is private, but must be public"),
format!(
"Struct '{item_name}' has {visibility_desc} visibility, but must be pub"
),
None,
"Change the visibility to 'pub'",
);
}
}
StructRule::MustBePubCrate(severity) => {
if !is_pub_crate {
let visibility_desc = if is_public {
"pub"
} else {
"private"
};
span_lint_and_help(
ctx,
STRUCT_LINT_MUST_BE_PUB_CRATE::get_by_severity(*severity),
self.name().as_str(),
definition_span,
format!(
"Struct '{item_name}' has {visibility_desc} visibility, but must be pub(crate)"
),
None,
"Add the 'pub' visibility modifier",
"Change the visibility to 'pub(crate)'",
);
}
}
Expand Down
4 changes: 2 additions & 2 deletions test_app/Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

155 changes: 4 additions & 151 deletions test_app/expected_output
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,38 +1,3 @@
warning: proc macro 'forbidden_proc_macro' is not allowed in this module
--> proc_macro_test/src/lib.rs:6:1
|
6 | / pub fn forbidden_proc_macro(input: TokenStream) -> TokenStream {
7 | | input
8 | | }
| |_^
|
= help: Consider moving this item to a different module
= note: Applied by cargo-pup rule 'proc_macro_restriction_test'.
= note: `#[warn(module_denied_items)]` on by default

warning: proc macro attribute 'forbidden_attr_macro' is not allowed in this module
--> proc_macro_test/src/lib.rs:12:1
|
12 | / pub fn forbidden_attr_macro(_attr: TokenStream, item: TokenStream) -> TokenStream {
13 | | item
14 | | }
| |_^
|
= help: Consider moving this item to a different module
= note: Applied by cargo-pup rule 'proc_macro_restriction_test'.

warning: proc macro derive 'forbidden_derive_macro' is not allowed in this module
--> proc_macro_test/src/lib.rs:18:1
|
18 | / pub fn forbidden_derive_macro(input: TokenStream) -> TokenStream {
19 | | input
20 | | }
| |_^
|
= help: Consider moving this item to a different module
= note: Applied by cargo-pup rule 'proc_macro_restriction_test'.

warning: `proc_macro_test` (lib) generated 3 warnings
warning: unused macro definition: `forbidden_macro`
--> src/macro_restriction/mod.rs:4:14
|
Expand Down Expand Up @@ -139,19 +104,6 @@ warning: module 'nested' is not allowed in this module
= help: Consider moving this item to a different module
= note: Applied by cargo-pup rule 'item_type_restrictions'.

warning: declarative macro 'forbidden_macro' is not allowed in this module
--> src/macro_restriction/mod.rs:4:1
|
4 | / macro_rules! forbidden_macro {
5 | | () => {
6 | | println!("This macro is forbidden!");
7 | | };
8 | | }
| |_^
|
= help: Consider moving this item to a different module
= note: Applied by cargo-pup rule 'macro_restriction_test'.

warning: Wildcard imports are not allowed
--> src/module_usage/mod.rs:5:1
|
Expand Down Expand Up @@ -230,13 +182,13 @@ warning: Struct must match pattern '.*MyTraitImpl$', found 'MyBadlyNamedThing'
= note: Applied by cargo-pup rule 'trait_restrictions'.
= note: `#[warn(struct_lint_must_be_named)]` on by default

warning: Struct 'MyBadlyNamedThing' is public, but must be private
warning: Struct 'MyBadlyNamedThing' has pub visibility, but must be private
--> src/trait_impl/mod.rs:17:1
|
17 | pub struct MyBadlyNamedThing {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: Remove the 'pub' visibility modifier
= help: Remove the visibility modifier
= note: Applied by cargo-pup rule 'trait_restrictions'.
= note: `#[warn(struct_lint_must_be_private)]` on by default

Expand All @@ -259,104 +211,5 @@ error: Function 'set_height' is forbidden by lint rule
= help: Remove this function to satisfy the architectural rule
= note: Applied by cargo-pup rule 'builder_style_set_consuming_forbidden'.

error: Function 'forbidden_async_function' is forbidden by lint rule
--> src/async_functions/mod.rs:8:1
|
8 | pub async fn forbidden_async_function() -> String {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: Remove this function to satisfy the architectural rule
= note: Applied by cargo-pup rule 'async_functions_forbidden'.

error: Function 'async_result_function' is forbidden by lint rule
--> src/async_functions/mod.rs:13:1
|
13 | pub async fn async_result_function() -> Result<i32, String> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: Remove this function to satisfy the architectural rule
= note: Applied by cargo-pup rule 'async_functions_forbidden'.

error: Function 'process_async' is forbidden by lint rule
--> src/async_functions/mod.rs:33:5
|
33 | pub async fn process_async(&self) -> Result<String, String> {
| ^^^^^^^^^^^^^^^^^^
|
= help: Remove this function to satisfy the architectural rule
= note: Applied by cargo-pup rule 'async_functions_forbidden'.

error: Function 'process_item' is forbidden by lint rule
--> src/async_functions/mod.rs:55:5
|
55 | async fn process_item(&self, item: String) -> Result<String, String> {
| ^^^^^^^^^^^^^^^^^
|
= help: Remove this function to satisfy the architectural rule
= note: Applied by cargo-pup rule 'async_functions_forbidden'.

warning: Function allocates heap memory: calls allocating function: std::boxed::Box::<T>::new
--> src/no_allocation.rs:12:5
|
12 | Box::new(42)
| ^^^^^^^^^^^^
|
= help: Remove heap allocations to satisfy the NoAllocation rule
= note: Applied by cargo-pup rule 'no_allocation_check'.

warning: Function allocates heap memory: calls allocating function: std::vec::Vec::<T>::new
--> src/no_allocation.rs:17:5
|
17 | Vec::new()
| ^^^^^^^^^^
|
= help: Remove heap allocations to satisfy the NoAllocation rule
= note: Applied by cargo-pup rule 'no_allocation_check'.

warning: Function allocates heap memory: calls allocating function: std::string::ToString::to_string
--> src/no_allocation.rs:22:5
|
22 | x.to_string()
| ^^^^^^^^^^^^^
|
= help: Remove heap allocations to satisfy the NoAllocation rule
= note: Applied by cargo-pup rule 'no_allocation_check'.

warning: Function allocates heap memory: calls function that allocates: no_allocation::helper_that_allocates
--> src/no_allocation.rs:27:5
|
27 | helper_that_allocates(x)
| ^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: Remove heap allocations to satisfy the NoAllocation rule
= note: Applied by cargo-pup rule 'no_allocation_check'.

warning: Function allocates heap memory: calls allocating function: std::vec::Vec::<T>::new
--> src/no_allocation.rs:32:5
|
32 | Vec::new()
| ^^^^^^^^^^
|
= help: Remove heap allocations to satisfy the NoAllocation rule
= note: Applied by cargo-pup rule 'no_allocation_check'.

warning: Function allocates heap memory: calls function that allocates: no_allocation::deep_helper
--> src/no_allocation.rs:36:5
|
36 | deep_helper()
| ^^^^^^^^^^^^^
|
= help: Remove heap allocations to satisfy the NoAllocation rule
= note: Applied by cargo-pup rule 'no_allocation_check'.

warning: Function allocates heap memory: calls function that allocates: no_allocation::middle_helper
--> src/no_allocation.rs:41:5
|
41 | middle_helper()
| ^^^^^^^^^^^^^^^
|
= help: Remove heap allocations to satisfy the NoAllocation rule
= note: Applied by cargo-pup rule 'no_allocation_check'.

warning: `test_app` (bin "test_app") generated 27 warnings
error: could not compile `test_app` (bin "test_app") due to 6 previous errors; 27 warnings emitted
warning: `test_app` (bin "test_app") generated 19 warnings
error: could not compile `test_app` (bin "test_app") due to 2 previous errors; 19 warnings emitted
Loading
Loading