diff --git a/cargo_pup_lint_config/src/function_lint/builder.rs b/cargo_pup_lint_config/src/function_lint/builder.rs index 66dbec5..c8c10ee 100644 --- a/cargo_pup_lint_config/src/function_lint/builder.rs +++ b/cargo_pup_lint_config/src/function_lint/builder.rs @@ -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) diff --git a/cargo_pup_lint_config/src/function_lint/matcher.rs b/cargo_pup_lint_config/src/function_lint/matcher.rs index ea6fefe..d07bd66 100644 --- a/cargo_pup_lint_config/src/function_lint/matcher.rs +++ b/cargo_pup_lint_config/src/function_lint/matcher.rs @@ -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 diff --git a/cargo_pup_lint_config/src/function_lint/types.rs b/cargo_pup_lint_config/src/function_lint/types.rs index 3d57f4a..c546c4d 100644 --- a/cargo_pup_lint_config/src/function_lint/types.rs +++ b/cargo_pup_lint_config/src/function_lint/types.rs @@ -16,6 +16,12 @@ pub enum ReturnTypePattern { Regex(String), /// Match Result 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 @@ -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 diff --git a/cargo_pup_lint_impl/src/lints/function_lint/lint.rs b/cargo_pup_lint_impl/src/lints/function_lint/lint.rs index 11410e0..6c46e85 100644 --- a/cargo_pup_lint_impl/src/lints/function_lint/lint.rs +++ b/cargo_pup_lint_impl/src/lints/function_lint/lint.rs @@ -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> { + 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, @@ -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) => { @@ -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", + ); + } } } } @@ -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", + ); + } } } } diff --git a/test_app/Cargo.lock b/test_app/Cargo.lock index 42d4688..3f396fa 100644 --- a/test_app/Cargo.lock +++ b/test_app/Cargo.lock @@ -322,7 +322,7 @@ dependencies = [ [[package]] name = "test_app" -version = "0.1.0" +version = "0.1.1" dependencies = [ "anyhow", "cargo_pup_lint_config", diff --git a/test_app/expected_output b/test_app/expected_output index 3a29d3f..ca01113 100644 --- a/test_app/expected_output +++ b/test_app/expected_output @@ -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 diff --git a/test_app/pup.ron b/test_app/pup.ron index 6230754..bf5036b 100644 --- a/test_app/pup.ron +++ b/test_app/pup.ron @@ -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), + ], + )), ], ) \ No newline at end of file diff --git a/test_app/src/builder_style/mod.rs b/test_app/src/builder_style/mod.rs new file mode 100644 index 0000000..10f45a3 --- /dev/null +++ b/test_app/src/builder_style/mod.rs @@ -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 + } +} \ No newline at end of file diff --git a/test_app/src/main.rs b/test_app/src/main.rs index 5d7bf74..9e2fa8f 100644 --- a/test_app/src/main.rs +++ b/test_app/src/main.rs @@ -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!"); diff --git a/test_app/tests/pup_ron_test.rs b/test_app/tests/pup_ron_test.rs index afbf67f..a70451e 100644 --- a/test_app/tests/pup_ron_test.rs +++ b/test_app/tests/pup_ron_test.rs @@ -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") diff --git a/tests/ui/function_lint/builder_style.rs b/tests/ui/function_lint/builder_style.rs new file mode 100644 index 0000000..4612925 --- /dev/null +++ b/tests/ui/function_lint/builder_style.rs @@ -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; + } +} \ No newline at end of file diff --git a/tests/ui/function_lint/builder_style.stderr b/tests/ui/function_lint/builder_style.stderr new file mode 100644 index 0000000..a768583 --- /dev/null +++ b/tests/ui/function_lint/builder_style.stderr @@ -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 + diff --git a/tests/ui/function_lint/pup.ron b/tests/ui/function_lint/pup.ron index e748ebe..8548763 100644 --- a/tests/ui/function_lint/pup.ron +++ b/tests/ui/function_lint/pup.ron @@ -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, + ) + ] + ) ) ] ) \ No newline at end of file