feat: elaborate on function lints

Author: scottgerring

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)

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

cargo_pup_lint_config/src/function_lint/types.rs

@@ -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
@@ -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

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<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,
@@ -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",
+                        );
+                    }
                 }
             }
         }

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;
+    }
+} 

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
+

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,
+                    )
+                ]
+            )
         )
     ]
 )