Add lints for discouraging eager / lazy function arguments

CHANGELOG.md

@@ -6748,6 +6748,7 @@ Released 2018-09-13
 [`disallowed_script_idents`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_script_idents
 [`disallowed_type`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_type
 [`disallowed_types`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_types
+[`discouraged_lazy_evaluation`]: https://rust-lang.github.io/rust-clippy/master/index.html#discouraged_lazy_evaluation
 [`diverging_sub_expression`]: https://rust-lang.github.io/rust-clippy/master/index.html#diverging_sub_expression
 [`doc_broken_link`]: https://rust-lang.github.io/rust-clippy/master/index.html#doc_broken_link
 [`doc_comment_double_space_linebreaks`]: https://rust-lang.github.io/rust-clippy/master/index.html#doc_comment_double_space_linebreaks
@@ -6775,6 +6776,7 @@ Released 2018-09-13
 [`duplicated_attributes`]: https://rust-lang.github.io/rust-clippy/master/index.html#duplicated_attributes
 [`duration_suboptimal_units`]: https://rust-lang.github.io/rust-clippy/master/index.html#duration_suboptimal_units
 [`duration_subsec`]: https://rust-lang.github.io/rust-clippy/master/index.html#duration_subsec
+[`eager_fun_call`]: https://rust-lang.github.io/rust-clippy/master/index.html#eager_fun_call
 [`eager_transmute`]: https://rust-lang.github.io/rust-clippy/master/index.html#eager_transmute
 [`elidable_lifetime_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#elidable_lifetime_names
 [`else_if_without_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#else_if_without_else

book/src/attribs.md

@@ -51,3 +51,62 @@ impl<'a> Drop for CounterWrapper<'a> {
     }
 }
 ```
+
+## `#[clippy::avoid_eager_arguments]`
+
+_Available since Clippy v1.XX_
+
+The `clippy::avoid_eager_arguments` attribute can be added to functions to discourage users from passing in arguments
+that are evaluated eagerly (via a function call or closure). This is useful in cases where a function may exist in
+different "variants" that accept either eager or lazy arguments, like `Option`'s `.unwrap_or` vs `.unwrap_or_else`.
+
+Library authors should provide a hint as part of the attribute, to inform users of the recommended solution.
+
+### Example
+
+```rust
+#[clippy::avoid_eager_arguments = "Prefer using `lazy_foo` instead of eagerly evaluating `argument`."]
+fn foo(argument: String) {
+    // Perform some logic that only rarely involves the use of `argument`
+}
+
+fn lazy_foo<F>(argument: F)
+where
+    F: FnOnce() -> String
+{
+    /// Perform some logic and evaluate `argument` on the fly, only if it is needed.
+}
+```
+
+## `#[clippy::optional_lazy_eval]`
+
+_Available since Clippy v1.XX_
+
+Complementary to `clippy::avoid_eager_arguments`, the `clippy::optional_lazy_eval` can be added to functions to inform
+users that are passing in a simple closure that doing so is unnecessary.
+
+Library authors should provide a hint as part of the attribute, to inform users of the recommended solution.
+
+### Example
+
+Together with `clippy::avoid_eager_arguments`:
+
+```rust
+#[clippy::avoid_eager_arguments = "Prefer using `lazy_foo` instead of eagerly evaluating `argument`."]
+fn foo(argument: String) {
+    // Perform some logic that only rarely involves the use of `argument`
+}
+
+#[clippy::optional_lazy_eval = "If `argument` does not require evaluation, prefer using `foo` instead."]
+fn lazy_foo<F>(argument: F)
+where
+    F: FnOnce() -> String
+{
+    /// Perform some logic and evaluate `argument` on the fly, only if it is needed.
+}
+
+fn main() {
+    let s = String::from("bar");
+    lazy_foo(move || s); // <- This will be linted against.
+}
+```

clippy_lints/src/declared_lints.rs

@@ -111,6 +111,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
     crate::disallowed_names::DISALLOWED_NAMES_INFO,
     crate::disallowed_script_idents::DISALLOWED_SCRIPT_IDENTS_INFO,
     crate::disallowed_types::DISALLOWED_TYPES_INFO,
+    crate::discouraged_lazy_evaluation::DISCOURAGED_LAZY_EVALUATION_INFO,
     crate::doc::DOC_BROKEN_LINK_INFO,
     crate::doc::DOC_COMMENT_DOUBLE_SPACE_LINEBREAKS_INFO,
     crate::doc::DOC_INCLUDE_WITHOUT_CFG_INFO,
@@ -137,6 +138,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
     crate::drop_forget_ref::MEM_FORGET_INFO,
     crate::duplicate_mod::DUPLICATE_MOD_INFO,
     crate::duration_suboptimal_units::DURATION_SUBOPTIMAL_UNITS_INFO,
+    crate::eager_fun_call::EAGER_FUN_CALL_INFO,
     crate::else_if_without_else::ELSE_IF_WITHOUT_ELSE_INFO,
     crate::empty_drop::EMPTY_DROP_INFO,
     crate::empty_enums::EMPTY_ENUMS_INFO,

clippy_lints/src/discouraged_lazy_evaluation.rs

@@ -0,0 +1,128 @@
+use clippy_utils::diagnostics::span_lint_and_note;
+use clippy_utils::eager_or_lazy::switch_to_eager_eval;
+use clippy_utils::{get_builtin_attr, is_from_proc_macro, sym, usage};
+use rustc_hir::def_id::LocalDefId;
+use rustc_hir::intravisit::{FnKind, Visitor, walk_body, walk_expr};
+use rustc_hir::{Body, Closure, ClosureKind, Expr, ExprKind, FnDecl};
+use rustc_lint::{LateContext, LateLintPass, LintContext};
+use rustc_session::declare_lint_pass;
+use rustc_span::Span;
+
+declare_clippy_lint! {
+    /// ### What it does
+    ///
+    /// As the counterpart to `eager_fun_call`, this lint looks for unnecessarily lazily evaluated closures used to
+    /// produce arguments for functions marked with `#[clippy::optional_lazy_eval]`.
+    ///
+    /// ### Why is this bad?
+    ///
+    /// Using eager evaluation is shorter and simpler in some cases.
+    ///
+    /// ### Known Problems
+    ///
+    /// It is possible, but not recommended for `Deref` and `Index` to have side effects. Eagerly evaluating them can change the semantics of the program.
+    ///
+    /// ### Example
+    /// ```rust,ignore
+    /// fn foo(argument: String) {
+    ///     // Perform some logic that only rarely involves the use of `argument`
+    /// }
+    ///
+    /// #[clippy::optional_lazy_eval = "If `argument` does not require evaluation, prefer using `foo` instead."]
+    /// fn lazy_foo<F>(argument: F)
+    /// where
+    ///     F: FnOnce() -> String
+    /// {
+    ///     /// Perform some logic and evaluate `argument` on the fly, only if it is needed.
+    /// }
+    ///
+    /// let s = String::new("bar");
+    /// lazy_foo(move || s);
+    /// ```
+    /// Use instead:
+    /// ```rust,ignore
+    /// let s = String::new("bar");
+    /// foo(s);
+    /// ```
+    #[clippy::version = "1.97.0"]
+    pub DISCOURAGED_LAZY_EVALUATION,
+    nursery,
+    "calling a function with an unnecessary closure used to produce an argument"
+}
+
+declare_lint_pass!(DiscouragedLazyEvaluation => [DISCOURAGED_LAZY_EVALUATION]);
+
+impl<'tcx> LateLintPass<'tcx> for DiscouragedLazyEvaluation {
+    fn check_fn(
+        &mut self,
+        cx: &LateContext<'tcx>,
+        _: FnKind<'_>,
+        _: &FnDecl<'_>,
+        body: &'tcx Body<'_>,
+        _: Span,
+        _: LocalDefId,
+    ) {
+        let mut visitor = DiscouragedLazyEvaluationVisitor { cx };
+        walk_body(&mut visitor, body);
+    }
+}
+
+struct DiscouragedLazyEvaluationVisitor<'cx, 'tcx> {
+    cx: &'cx LateContext<'tcx>,
+}
+
+impl<'tcx> Visitor<'tcx> for DiscouragedLazyEvaluationVisitor<'_, 'tcx> {
+    fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) {
+        if let ExprKind::Call(func, args) = expr.kind
+            && let ExprKind::Path(ref qpath) = func.kind
+            && let Some(def_id) = self.cx.qpath_res(qpath, func.hir_id).opt_def_id()
+        {
+            #[allow(deprecated)]
+            let attrs = self.cx.tcx.get_all_attrs(def_id);
+            let mut lazy_attrs = get_builtin_attr(self.cx.sess(), attrs, sym::optional_lazy_eval);
+            if let Some(attr) = lazy_attrs.next() {
+                let Some(message) = attr.value_str() else {
+                    walk_expr(self, expr);
+                    return;
+                };
+
+                let lazy_args: Vec<_> = args
+                    .iter()
+                    .filter(|arg| {
+                        let ExprKind::Closure(&Closure {
+                            body,
+                            kind: ClosureKind::Closure,
+                            ..
+                        }) = arg.kind
+                        else {
+                            return false;
+                        };
+
+                        let body = self.cx.tcx.hir_body(body);
+                        let body_expr = &body.value;
+
+                        if usage::BindingUsageFinder::are_params_used(self.cx, body)
+                            || is_from_proc_macro(self.cx, expr)
+                        {
+                            return false;
+                        }
+
+                        switch_to_eager_eval(self.cx, body_expr)
+                    })
+                    .collect();
+
+                if !lazy_args.is_empty() {
+                    span_lint_and_note(
+                        self.cx,
+                        DISCOURAGED_LAZY_EVALUATION,
+                        expr.span,
+                        message.as_str().to_owned(),
+                        Some(expr.span),
+                        "unnecessary closure used to produce a function argument",
+                    );
+                }
+            }
+        }
+        walk_expr(self, expr);
+    }
+}

clippy_lints/src/eager_fun_call.rs

@@ -0,0 +1,112 @@
+use clippy_utils::diagnostics::span_lint_and_note;
+use clippy_utils::eager_or_lazy::switch_to_lazy_eval;
+use clippy_utils::{get_builtin_attr, sym};
+use rustc_hir::def_id::LocalDefId;
+use rustc_hir::intravisit::{FnKind, Visitor, walk_body, walk_expr};
+use rustc_hir::{Body, Expr, ExprKind, FnDecl};
+use rustc_lint::{LateContext, LateLintPass, LintContext};
+use rustc_session::declare_lint_pass;
+use rustc_span::Span;
+
+declare_clippy_lint! {
+    /// ### What it does
+    ///
+    /// Searches for elements marked with `#[clippy::avoid_eager_arguments]` that are being called with eagerly
+    /// evaluated arguments.
+    ///
+    /// ### Why is this bad?
+    ///
+    /// If the value of an argument is expected to be rarely needed, it is usually better to evaluate that value lazily,
+    /// especially if the eager evaluation involves memory allocations or other non-trivial amounts of work.
+    ///
+    /// ### Known Problems
+    ///
+    /// If the function that is called to produce the argument has side-effects, not calling it will change the
+    /// semantics of the program, but this should not be relied on.
+    ///
+    /// The lint also cannot figure out whether the function that is being called is actually expensive to call or not.
+    ///
+    /// ### Example
+    /// ```rust,ignore
+    /// #[clippy::avoid_eager_arguments = "The value behind `argument` may not always be used, prefer using `lazy_foo` instead"]
+    /// fn foo(argument: String) {
+    ///   // ...
+    /// }
+    ///
+    /// fn lazy_foo<F>(argument: F)
+    /// where
+    ///     F: Fn() -> String
+    /// {
+    ///   // ...
+    /// }
+    ///
+    /// foo(String::new());
+    /// ```
+    /// Use instead:
+    /// ```rust,ignore
+    ///  lazy_foo(|| String::new());
+    /// ```
+    ///
+    /// ### Notes
+    ///
+    /// Library authors should provide an explanation as to why the eager evaluation is undesirable, and ideally offer
+    /// and mention an alternative function that accepts an argument that can be lazily evaluated.
+    #[clippy::version = "1.97.0"]
+    pub EAGER_FUN_CALL,
+    nursery,
+    "calling a function with eagerly evaluated arguments where it is discouraged to do so"
+}
+
+declare_lint_pass!(EagerFunCall => [EAGER_FUN_CALL]);
+
+impl<'tcx> LateLintPass<'tcx> for EagerFunCall {
+    fn check_fn(
+        &mut self,
+        cx: &LateContext<'tcx>,
+        _: FnKind<'_>,
+        _: &FnDecl<'_>,
+        body: &'tcx Body<'_>,
+        _: Span,
+        _: LocalDefId,
+    ) {
+        let mut visitor = EagerFunCallVisitor { cx };
+        walk_body(&mut visitor, body);
+    }
+}
+
+struct EagerFunCallVisitor<'cx, 'tcx> {
+    cx: &'cx LateContext<'tcx>,
+}
+
+impl<'tcx> Visitor<'tcx> for EagerFunCallVisitor<'_, 'tcx> {
+    fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) {
+        if let ExprKind::Call(func, args) = expr.kind
+            && let ExprKind::Path(ref qpath) = func.kind
+            && let Some(def_id) = self.cx.qpath_res(qpath, func.hir_id).opt_def_id()
+        {
+            #[allow(deprecated)]
+            let attrs = self.cx.tcx.get_all_attrs(def_id);
+            let mut lazy_attrs = get_builtin_attr(self.cx.sess(), attrs, sym::avoid_eager_arguments);
+            if let Some(attr) = lazy_attrs.next() {
+                let Some(message) = attr.value_str() else {
+                    walk_expr(self, expr);
+                    return;
+                };
+
+                let lazy_args: Vec<_> = args.iter().filter(|arg| switch_to_lazy_eval(self.cx, arg)).collect();
+
+                if !lazy_args.is_empty() {
+                    span_lint_and_note(
+                        self.cx,
+                        EAGER_FUN_CALL,
+                        expr.span,
+                        message.as_str().to_owned(),
+                        Some(expr.span),
+                        "function call with eagerly evaluated arguments",
+                    );
+                }
+            }
+        }
+        walk_expr(self, expr);
+    }
+}

clippy_lints/src/lib.rs

@@ -111,11 +111,13 @@ mod disallowed_methods;
 mod disallowed_names;
 mod disallowed_script_idents;
 mod disallowed_types;
+mod discouraged_lazy_evaluation;
 mod doc;
 mod double_parens;
 mod drop_forget_ref;
 mod duplicate_mod;
 mod duration_suboptimal_units;
+mod eager_fun_call;
 mod else_if_without_else;
 mod empty_drop;
 mod empty_enums;
@@ -860,6 +862,8 @@ rustc_lint::late_lint_methods!(
         ManualNoopWaker: manual_noop_waker::ManualNoopWaker = manual_noop_waker::ManualNoopWaker::new(conf),
         ByteCharSlice: byte_char_slices::ByteCharSlice = byte_char_slices::ByteCharSlice,
         ManualAssertEq: manual_assert_eq::ManualAssertEq = manual_assert_eq::ManualAssertEq,
+        EagerFunCall: eager_fun_call::EagerFunCall = eager_fun_call::EagerFunCall,
+        DiscouragedLazyEvaluation: discouraged_lazy_evaluation::DiscouragedLazyEvaluation = discouraged_lazy_evaluation::DiscouragedLazyEvaluation,
         // add late passes here, used by `cargo dev new_lint`
     ]]
 );

clippy_utils/src/attrs.rs

@@ -35,6 +35,8 @@ pub fn get_builtin_attr<'a, A: AttributeExt + 'a>(
                 // The following attributes are for the 3rd party crate authors.
                 // See book/src/attribs.md
                 | sym::has_significant_drop
+                | sym::avoid_eager_arguments
+                | sym::optional_lazy_eval
                 | sym::format_args => None,
                 _ => {
                     sess.dcx().span_err(path_span, "usage of unknown attribute");

clippy_utils/src/sym.rs

@@ -141,6 +141,7 @@ generate! {
     as_str,
     assert_failed,
     author,
+    avoid_eager_arguments,
     back,
     binary_heap_pop_if,
     binaryheap_iter,
@@ -449,6 +450,7 @@ generate! {
     open_options_new,
     option_expect,
     option_unwrap,
+    optional_lazy_eval,
     or_default,
     or_else,
     or_insert,