Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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/function_lint/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,12 @@ impl<'a> FunctionConstraintBuilder<'a> {
self
}

/// Require that no function matching the selector exists
pub fn must_not_exist(mut self) -> Self {
self.add_rule_internal(FunctionRule::MustNotExist(self.current_severity));
self
}

/// Create a new MaxLength rule with the current severity
pub fn create_max_length_rule(&self, length: usize) -> FunctionRule {
FunctionRule::MaxLength(length, self.current_severity)
Expand Down
15 changes: 15 additions & 0 deletions cargo_pup_lint_config/src/function_lint/matcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,21 @@ impl FunctionMatcher {
pattern.into(),
)))
}

/// Matches functions that return `Self` by value
pub fn returns_self(&self) -> FunctionMatchNode {
FunctionMatchNode::Leaf(FunctionMatch::ReturnsType(ReturnTypePattern::SelfValue))
}

/// Matches functions that return `&Self`
pub fn returns_self_ref(&self) -> FunctionMatchNode {
FunctionMatchNode::Leaf(FunctionMatch::ReturnsType(ReturnTypePattern::SelfRef))
}

/// Matches functions that return `&mut Self`
pub fn returns_self_mut_ref(&self) -> FunctionMatchNode {
FunctionMatchNode::Leaf(FunctionMatch::ReturnsType(ReturnTypePattern::SelfMutRef))
}
}

/// Node in the matcher expression tree
Expand Down
8 changes: 8 additions & 0 deletions cargo_pup_lint_config/src/function_lint/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,12 @@ pub enum ReturnTypePattern {
Regex(String),
/// Match Result<T, E> where E implements Error trait
ResultWithErrorImpl,
/// Match when the function returns `Self` by value (e.g., a consuming builder-style method)
SelfValue,
/// Match when the function returns `&Self` (immutable reference, e.g., fluent interface)
SelfRef,
/// Match when the function returns `&mut Self` (mutable reference, e.g., classic builder setter)
SelfMutRef,
}

/// Specifies how to match functions for linting
Expand Down Expand Up @@ -52,6 +58,8 @@ pub enum FunctionRule {
MaxLength(usize, Severity),
/// Enforces that Result error types must implement the Error trait
ResultErrorMustImplementError(Severity),
/// Enforces that a function matching the selector must not exist at all
MustNotExist(Severity),
}

// Helper methods for FunctionRule
Expand Down
57 changes: 57 additions & 0 deletions cargo_pup_lint_impl/src/lints/function_lint/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,16 @@ use rustc_middle::ty::TyKind;
use rustc_session::impl_lint_pass;
use rustc_span::BytePos;

// Helper: retrieve the concrete Self type of the impl the method belongs to, if any
fn get_self_type<'tcx>(
ctx: &LateContext<'tcx>,
fn_def_id: rustc_hir::def_id::DefId,
) -> Option<rustc_middle::ty::Ty<'tcx>> {
ctx.tcx
.impl_of_method(fn_def_id)
.map(|impl_def_id| ctx.tcx.type_of(impl_def_id).instantiate_identity())
}

pub struct FunctionLint {
name: String,
matches: FunctionMatch,
Expand Down Expand Up @@ -143,6 +153,23 @@ fn evaluate_function_match(
Err(_) => false,
}
}
ReturnTypePattern::SelfValue => get_self_type(ctx, fn_def_id) == Some(return_ty),
ReturnTypePattern::SelfRef => {
match (get_self_type(ctx, fn_def_id), return_ty.kind()) {
(Some(self_ty), &TyKind::Ref(_, inner, rustc_hir::Mutability::Not)) => {
inner == self_ty
}
_ => false,
}
}
ReturnTypePattern::SelfMutRef => {
match (get_self_type(ctx, fn_def_id), return_ty.kind()) {
(Some(self_ty), &TyKind::Ref(_, inner, rustc_hir::Mutability::Mut)) => {
inner == self_ty
}
_ => false,
}
}
}
}
FunctionMatch::AndMatches(left, right) => {
Expand Down Expand Up @@ -284,6 +311,21 @@ impl<'tcx> LateLintPass<'tcx> for FunctionLint {
}
}
}
FunctionRule::MustNotExist(severity) => {
let sig_span = item
.span
.with_hi(item.span.lo() + BytePos((item_name.len() + 5) as u32));

span_lint_and_help(
ctx,
FUNCTION_LINT::get_by_severity(*severity),
self.name().as_str(),
sig_span,
format!("Function '{item_name}' is forbidden by lint rule"),
None,
"Remove this function to satisfy the architectural rule",
);
}
}
}
}
Expand Down Expand Up @@ -372,6 +414,21 @@ impl<'tcx> LateLintPass<'tcx> for FunctionLint {
}
}
}
FunctionRule::MustNotExist(severity) => {
let sig_span = impl_item
.span
.with_hi(impl_item.span.lo() + BytePos((item_name.len() + 5) as u32));

span_lint_and_help(
ctx,
FUNCTION_LINT::get_by_severity(*severity),
self.name().as_str(),
sig_span,
format!("Function '{item_name}' is forbidden by lint rule"),
None,
"Remove this function to satisfy the architectural rule",
);
}
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion test_app/Cargo.lock

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

20 changes: 20 additions & 0 deletions test_app/expected_output
Original file line number Diff line number Diff line change
Expand Up @@ -240,4 +240,24 @@ warning: Struct 'MyBadlyNamedThing' is public, but must be private
= note: Applied by cargo-pup rule 'trait_restrictions'.
= note: `#[warn(struct_lint_must_be_private)]` on by default

error: Function 'with_width' is forbidden by lint rule
--> src/builder_style/mod.rs:8:5
|
8 | pub fn with_width(self, width: u32) -> Self {
| ^^^^^^^^^^^^^^^
|
= help: Remove this function to satisfy the architectural rule
= note: Applied by cargo-pup rule 'builder_style_with_consuming_forbidden'.
= note: `#[deny(function_lint)]` on by default

error: Function 'set_height' is forbidden by lint rule
--> src/builder_style/mod.rs:13:5
|
13 | pub fn set_height(self, height: u32) -> Self {
| ^^^^^^^^^^^^^^^
|
= help: Remove this function to satisfy the architectural rule
= note: Applied by cargo-pup rule 'builder_style_set_consuming_forbidden'.

warning: `test_app` (bin "test_app") generated 20 warnings
error: could not compile `test_app` (bin "test_app") due to 2 previous errors; 20 warnings emitted
20 changes: 20 additions & 0 deletions test_app/pup.ron
Original file line number Diff line number Diff line change
Expand Up @@ -99,5 +99,25 @@
),
],
)),
Function((
name: "builder_style_with_consuming_forbidden",
matches: AndMatches(
NameRegex("^with_.*"),
ReturnsType(SelfValue)
),
rules: [
MustNotExist(Error),
],
)),
Function((
name: "builder_style_set_consuming_forbidden",
matches: AndMatches(
NameRegex("^set_.*"),
ReturnsType(SelfValue)
),
rules: [
MustNotExist(Error),
],
)),
],
)
28 changes: 28 additions & 0 deletions test_app/src/builder_style/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
pub struct WidgetBuilder {
width: u32,
height: u32,
}

impl WidgetBuilder {
// BAD: Consuming builder – should be caught by the lint (returns `Self`)
pub fn with_width(self, width: u32) -> Self {
Self { width, ..self }
}

// BAD: Consumes self but uses a `set_` prefix – also forbidden
pub fn set_height(self, height: u32) -> Self {
Self { height, ..self }
}

// GOOD: Reference-based setter – allowed (`&mut self` → `&mut Self`)
pub fn set_width(&mut self, width: u32) -> &mut Self {
self.width = width;
self
}

// GOOD: Reference-based "with_" method – allowed
pub fn with_height(&mut self, height: u32) -> &mut Self {
self.height = height;
self
}
}
1 change: 1 addition & 0 deletions test_app/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ mod module_usage;
mod must_be_empty;
mod result_error;
mod trait_impl;
mod builder_style;

fn main() {
println!("Hello, world!");
Expand Down
20 changes: 20 additions & 0 deletions test_app/tests/pup_ron_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,26 @@ fn test_lint_config() {
.enforce_error_trait_implementation()
.build();

// ------------------------------------------------------------------
// Builder style lint rules (demonstrates consuming vs reference pattern)
// ------------------------------------------------------------------

builder
.function_lint()
.lint_named("builder_style_with_consuming_forbidden")
.matching(|m| m.name_regex("^with_.*").and(m.returns_self()))
.with_severity(Severity::Error)
.must_not_exist()
.build();

builder
.function_lint()
.lint_named("builder_style_set_consuming_forbidden")
.matching(|m| m.name_regex("^set_.*").and(m.returns_self()))
.with_severity(Severity::Error)
.must_not_exist()
.build();

// Write the configuration to pup.ron using the fixed write_to_file method
builder
.write_to_file("pup.ron")
Expand Down
39 changes: 39 additions & 0 deletions tests/ui/function_lint/builder_style.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
//@compile-flags: --crate-name test_builder_style
//@compile-flags: --crate-type lib

// This test verifies builder-style lints: "with_" methods returning `Self` are forbidden,
// while "set_" methods returning `&mut Self` are allowed.

pub struct WidgetBuilder {
val: i32,
}

impl WidgetBuilder {
// This should trigger the MustNotExist rule
pub fn with_val(mut self, val: i32) -> Self { //~ ERROR: Function 'with_val' is forbidden by lint rule
self.val = val;
self
}

// This is the preferred style and should compile cleanly
pub fn set_val(&mut self, val: i32) -> &mut Self {
self.val = val;
self
}

// Name matches the forbidden prefix but uses an allowed return type (&mut Self) – should be OK
pub fn with_val_ref(&mut self, val: i32) -> &mut Self {
self.val = val;
self
}

// Opposite rule: name starts with "set_" but returns Self – should trigger error
pub fn set_val_value(self, val: i32) -> Self { //~ ERROR: Function 'set_val_value' is forbidden by lint rule
Self { val }
}

// Control method that matches neither rule
pub fn touch(&self) {
let _ = self.val;
}
}
21 changes: 21 additions & 0 deletions tests/ui/function_lint/builder_style.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
error: Function 'with_val' is forbidden by lint rule
--> tests/ui/function_lint/builder_style.rs:13:5
|
LL | pub fn with_val(mut self, val: i32) -> Self {
| ^^^^^^^^^^^^^
|
= help: Remove this function to satisfy the architectural rule
= note: Applied by cargo-pup rule 'builder_style_with_methods_forbidden'.
= note: `#[deny(function_lint)]` on by default

error: Function 'set_val_value' is forbidden by lint rule
--> tests/ui/function_lint/builder_style.rs:31:5
|
LL | pub fn set_val_value(self, val: i32) -> Self {
| ^^^^^^^^^^^^^^^^^^
|
= help: Remove this function to satisfy the architectural rule
= note: Applied by cargo-pup rule 'builder_style_set_methods_forbid_self_value'.

error: aborting due to 2 previous errors

34 changes: 34 additions & 0 deletions tests/ui/function_lint/pup.ron
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,40 @@
)
]
)
),

// ======================================================================
// SECTION: Builder Style Tests (for builder_style.rs)
// ======================================================================

Function(
(
name: "builder_style_with_methods_forbidden",
matches: AndMatches(
NameRegex("^with_.*"),
ReturnsType(SelfValue)
),
rules: [
MustNotExist(
Error,
)
]
)
),

Function(
(
name: "builder_style_set_methods_forbid_self_value",
matches: AndMatches(
NameRegex("^set_.*"),
ReturnsType(SelfValue)
),
rules: [
MustNotExist(
Error,
)
]
)
)
]
)