Lint against patterns like <Result<_,_> as anyhow::Context>::context(r, &format!("...", ...)) in favor of .with_context(|| ...)

[anp]

What it does

Warn users that they will be unconditionally calling a function (or causing a heap allocation) even when the result which is the receiver is Ok(...).

This would be analogous to expect_fun_call or or_fun_call but for anyhow::Context.

Advantage

• Discourages unnecessary heap allocations and other expensive function calls

Drawbacks

• Encodes a clippy lint for an error-handling crate which is not used universally
• Might have false positives -- not all functions are too expensive to call unconditionally

Example

use anyhow::Context;

let foo = 42;
fallible_fn().context(&format!("error: {foo}"))?;

Could be written as:

use anyhow::Context;

let foo = 42;
fallible_fn().with_context(|| format!("error: {foo}"))?;

[haxorcize]

Big +1

Just ran into this precise issue a few hours ago in production code. A heavily used infra function was doing ok_or(anyhow!("....{}...")) instead of with_context(|| format!(...)).

That code was slowing down the system significantly, and removing it improved performance 6-7x. Found it via profiling. I was telling of my team mates about it, and said "I wish there was a clippy lint about it", and he sent me this issue. Amazing timing 😄

[anp]

Cool! Although FWIW I think that particular pattern should already be covered by clippy::or_fun_call. I think it's not yet in the perf category because it's still in the nursery, but I recently added it to a pretty large codebase and found a whole lot of .ok_or(anyhow!(...)) that I cleaned up.

[y21]

We usually try to avoid adding new crate-specific lints and instead try to generalize them. In this case, maybe a way forward would be to add a new clippy attribute for or_fun_call that anyhow can then add to its context method, like #[clippy::lazy = "with_context"] fn context(..) {} to have clippy pick up uses of it. All methods defined in the standard library that or_fun_call currently lints could then also be annotated as such.

[anp]

We usually try to avoid adding new crate-specific lints and instead try to generalize them.

Very fair! I’ve seen some crate specific lints in the past but I agree it would be useful to generalize. The first case I ran into on my work codebase was actually one that wouldn’t be covered by either or_fun_call or my suggestion above.

In this case, maybe a way forward would be to add a new clippy attribute for or_fun_call that anyhow can then add to its context method, like #[clippy::lazy = "with_context"] fn context(..) {} to have clippy pick up uses of it. All methods defined in the standard library that or_fun_call currently lints could then also be annotated as such.

I like that idea! How hard would that be to implement? Is there a different process for defining lint annotations like that vs lints themselves?