Add lints for discouraging eager / lazy function arguments

[Naxdy]

Closes #13678

Basically what or_fun_call and unnecessary_lazy_eval already do, except controlled by an attribute that can be added to any function by downstream library authors. Two major use cases that immediately come to mind are anyhow's .context vs. with_context, and eyre's .wrap_err vs. .wrap_err_with.

Refer to the docs in this PR for some more detailed explanation, but the basic gist is that library authors will be able to do something like this:

#[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");

    // These two are fine
    foo(s);
    lazy_foo(|| String::from("bar"));

    let s = String::from("bar");

    // These will be linted against
    lazy_foo(move || s);
    foo(String::from("baz"));
}

Also a thought for the future: These lints could potentially replace or_fun_call and unnecessary_lazy_eval altogether if the corresponding attributes were added to the relevant methods in Option and Result. I've thought about simply amending the two, but didn't want to potentially break backwards compatibility.

Todo:

• The docs need the actual Clippy version that introduces this lint. I intentionally left it blank for now, until we know if/when this can be included.

---

changelog: Adds new lints eager_fun_call and discouraged_lazy_evaluation that check for calling functions with eagerly evaluated arguments, and unnecessary closures used to produce arguments respectively.

[rustbot]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Jarcho (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

• @rustbot author: the review is finished, PR author should check the comments and take action accordingly
• @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: 8 candidates
• 8 candidates expanded to 8 candidates
• Random selection from Jarcho, dswij, llogiq, samueltardieu

[rustbot]

☔ The latest upstream changes (possibly #17164) made this pull request unmergeable. Please resolve the merge conflicts.

[Jarcho]

This should be one lint pass. There's no reason to do almost the same initial work for every call twice.

View changes since this review

[Jarcho]

This should be check_expr.

[Jarcho]

This doesn't handle method calls. Calls to functions without arguments should also not be checked any further.

[Jarcho]

Why collect just to check if it's empty. This is what any is for.

[rustbot]

Reminder, once the PR becomes ready for a review, use @rustbot ready.

[Jarcho]

These attributes should name the alternative function in a way that allows us to find it. We can't correctly trigger the lint if we don't know which arguments to check, so at a minimum those need to be labelled as well. If we can lookup the alternative function then we can auto-detect which argument(s) is changing base on the types involved.

[Jarcho]

@rustbot label lint-nominated

[rustbot]

This lint has been nominated for inclusion.

A FCP topic has been created on Zulip.