From f9e62b61781096f11e5822d24d9ee10c74de950b Mon Sep 17 00:00:00 2001 From: Naxdy Date: Thu, 28 May 2026 12:16:19 +0200 Subject: [PATCH 1/2] Add new lint for eagerly evaluated function arguments --- CHANGELOG.md | 1 + book/src/attribs.md | 26 +++++++ clippy_lints/src/declared_lints.rs | 1 + clippy_lints/src/eager_fun_call.rs | 112 +++++++++++++++++++++++++++++ clippy_lints/src/lib.rs | 2 + clippy_utils/src/attrs.rs | 1 + clippy_utils/src/sym.rs | 1 + tests/ui/eager_fun_call.rs | 21 ++++++ tests/ui/eager_fun_call.stderr | 16 +++++ 9 files changed, 181 insertions(+) create mode 100644 clippy_lints/src/eager_fun_call.rs create mode 100644 tests/ui/eager_fun_call.rs create mode 100644 tests/ui/eager_fun_call.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 95f26ee09bbe..36469d1e99bc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6775,6 +6775,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 diff --git a/book/src/attribs.md b/book/src/attribs.md index 9b7f59705041..434b6b4fe721 100644 --- a/book/src/attribs.md +++ b/book/src/attribs.md @@ -51,3 +51,29 @@ 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(argument: F) +where + F: FnOnce() -> String +{ + /// Perform some logic and evaluate `argument` on the fly, only if it is needed. +} +``` diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 516f6e775e08..d8d21da438f8 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -137,6 +137,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, diff --git a/clippy_lints/src/eager_fun_call.rs b/clippy_lints/src/eager_fun_call.rs new file mode 100644 index 000000000000..8e8927ee7ab0 --- /dev/null +++ b/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(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); + } +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 7b190c255888..f7ea64256d9d 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -116,6 +116,7 @@ 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 +861,7 @@ 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, // add late passes here, used by `cargo dev new_lint` ]] ); diff --git a/clippy_utils/src/attrs.rs b/clippy_utils/src/attrs.rs index 6e7434771369..640ea692ac40 100644 --- a/clippy_utils/src/attrs.rs +++ b/clippy_utils/src/attrs.rs @@ -35,6 +35,7 @@ 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::format_args => None, _ => { sess.dcx().span_err(path_span, "usage of unknown attribute"); diff --git a/clippy_utils/src/sym.rs b/clippy_utils/src/sym.rs index 2661be807c75..02b30d26bee5 100644 --- a/clippy_utils/src/sym.rs +++ b/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, diff --git a/tests/ui/eager_fun_call.rs b/tests/ui/eager_fun_call.rs new file mode 100644 index 000000000000..1505a7a8e04b --- /dev/null +++ b/tests/ui/eager_fun_call.rs @@ -0,0 +1,21 @@ +#![warn(clippy::eager_fun_call)] + +#[clippy::avoid_eager_arguments = "the value of `argument` may not always be used, prefer using `lazy_foo` instead"] +fn foo(argument: String) { + let _ = argument; +} + +fn lazy_foo(argument: F) +where + F: Fn() -> String, +{ + let _ = argument(); +} + +fn main() { + let s = String::from("baz"); + foo(String::from("bar")); + //~^ eager_fun_call + foo(s); + lazy_foo(|| String::from("bar")); +} diff --git a/tests/ui/eager_fun_call.stderr b/tests/ui/eager_fun_call.stderr new file mode 100644 index 000000000000..fafb47409210 --- /dev/null +++ b/tests/ui/eager_fun_call.stderr @@ -0,0 +1,16 @@ +error: the value of `argument` may not always be used, prefer using `lazy_foo` instead + --> tests/ui/eager_fun_call.rs:17:5 + | +LL | foo(String::from("bar")); + | ^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: function call with eagerly evaluated arguments + --> tests/ui/eager_fun_call.rs:17:5 + | +LL | foo(String::from("bar")); + | ^^^^^^^^^^^^^^^^^^^^^^^^ + = note: `-D clippy::eager-fun-call` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::eager_fun_call)]` + +error: aborting due to 1 previous error + From 7f7343bba0a6d3bad220add512d378de4886dd9f Mon Sep 17 00:00:00 2001 From: Naxdy Date: Thu, 28 May 2026 15:31:11 +0200 Subject: [PATCH 2/2] Add new lint for unnecessary closure arguments --- CHANGELOG.md | 1 + book/src/attribs.md | 33 +++++ clippy_lints/src/declared_lints.rs | 1 + .../src/discouraged_lazy_evaluation.rs | 128 ++++++++++++++++++ clippy_lints/src/lib.rs | 2 + clippy_utils/src/attrs.rs | 1 + clippy_utils/src/sym.rs | 1 + tests/ui/discouraged_lazy_evaluation.rs | 21 +++ tests/ui/discouraged_lazy_evaluation.stderr | 16 +++ 9 files changed, 204 insertions(+) create mode 100644 clippy_lints/src/discouraged_lazy_evaluation.rs create mode 100644 tests/ui/discouraged_lazy_evaluation.rs create mode 100644 tests/ui/discouraged_lazy_evaluation.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 36469d1e99bc..034631bd7a5e 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/book/src/attribs.md b/book/src/attribs.md index 434b6b4fe721..70ab72109e18 100644 --- a/book/src/attribs.md +++ b/book/src/attribs.md @@ -77,3 +77,36 @@ where /// 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(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. +} +``` diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index d8d21da438f8..bfb2a8001d83 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/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, diff --git a/clippy_lints/src/discouraged_lazy_evaluation.rs b/clippy_lints/src/discouraged_lazy_evaluation.rs new file mode 100644 index 000000000000..ca71aa85229d --- /dev/null +++ b/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(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); + } +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index f7ea64256d9d..9f5adbbdae80 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -111,6 +111,7 @@ 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; @@ -862,6 +863,7 @@ rustc_lint::late_lint_methods!( 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` ]] ); diff --git a/clippy_utils/src/attrs.rs b/clippy_utils/src/attrs.rs index 640ea692ac40..8a89a2312abd 100644 --- a/clippy_utils/src/attrs.rs +++ b/clippy_utils/src/attrs.rs @@ -36,6 +36,7 @@ pub fn get_builtin_attr<'a, A: AttributeExt + 'a>( // 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"); diff --git a/clippy_utils/src/sym.rs b/clippy_utils/src/sym.rs index 02b30d26bee5..65f26e3017d1 100644 --- a/clippy_utils/src/sym.rs +++ b/clippy_utils/src/sym.rs @@ -450,6 +450,7 @@ generate! { open_options_new, option_expect, option_unwrap, + optional_lazy_eval, or_default, or_else, or_insert, diff --git a/tests/ui/discouraged_lazy_evaluation.rs b/tests/ui/discouraged_lazy_evaluation.rs new file mode 100644 index 000000000000..abfbd37990be --- /dev/null +++ b/tests/ui/discouraged_lazy_evaluation.rs @@ -0,0 +1,21 @@ +#![warn(clippy::discouraged_lazy_evaluation)] + +fn foo(argument: String) { + let _ = argument; +} + +#[clippy::optional_lazy_eval = "prefer using `foo` if `argument` does not need to be computed"] +fn lazy_foo(argument: F) +where + F: FnOnce() -> String, +{ + let _ = argument(); +} + +fn main() { + let s = String::from("baz"); + foo(s.clone()); + lazy_foo(|| format!("my custom string with {s}")); + lazy_foo(move || s); + //~^ discouraged_lazy_evaluation +} diff --git a/tests/ui/discouraged_lazy_evaluation.stderr b/tests/ui/discouraged_lazy_evaluation.stderr new file mode 100644 index 000000000000..ec9ec45a930f --- /dev/null +++ b/tests/ui/discouraged_lazy_evaluation.stderr @@ -0,0 +1,16 @@ +error: prefer using `foo` if `argument` does not need to be computed + --> tests/ui/discouraged_lazy_evaluation.rs:19:5 + | +LL | lazy_foo(move || s); + | ^^^^^^^^^^^^^^^^^^^ + | +note: unnecessary closure used to produce a function argument + --> tests/ui/discouraged_lazy_evaluation.rs:19:5 + | +LL | lazy_foo(move || s); + | ^^^^^^^^^^^^^^^^^^^ + = note: `-D clippy::discouraged-lazy-evaluation` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::discouraged_lazy_evaluation)]` + +error: aborting due to 1 previous error +