Suggest method syntax x.max(y) for unresolved max(x, y) calls

[Kivooeo]

Fixes #76386

r? compiler

[Noratrieb]

personally I find the method usage of max not very nice, since max is a fully symmetric operation while a method is not. i prefer the free function, so I'm not sure whether we really should suggest its presence. i think users will be better off following the existing advice of importing the free function.

[Kivooeo]

it more looks like a typo from people who came from other languages like python and do not know about method way which is more idiomatic in rust imo, so, I guess, such suggestion woudln't harm

on top i can say it produce such suggestion between incomparable types, which may be missleading? im not sure if there an easy way to check if both types are implementing Ord to not generate this

fn main() {
    struct A;
    let a = A;
    max(a, 2);
}

error[E0425]: cannot find function `max` in this scope
 --> /home/gh-Kivooeo/test_/src/main.rs:4:5
  |
4 |     max(a, 2);
  |     ^^^ not found in this scope
  |
  = note: -Ztrack-diagnostics: created at compiler/rustc_resolve/src/late/diagnostics.rs:445:36
help: you may have meant to use the method syntax
  |
4 -     max(a, 2);
4 +     a.max(2);
  |
help: consider importing this function
  |
1 + use std::cmp::max;
  |

[fmease]

As a rule of thumb, please never unwrap span_to_snippet, it can indeed fail.

Under your PR we likely crash under a scenario like (not tested):

// file `a.rs`
#[macro_export]
macro_rules! mk { () => { max(0, 0) } }

// file `b.rs`
fn main() {
    a::mk!();
}

Then compile a.rs and delete or move a.rs and compile b.rs with --extern a.

[fmease]

Instead of using span_to_snippet, construct a multi-part suggestion that maps the span of min( or max( to empty and the span of , to .min( or .max( respectively. These subspans can be obtained via Span::{to,between,until} etc. However, you'd probably still want to guard against differing expansion levels via eq_ctxt (heavy hammer) or find_*_ancestor_* (brittle).

[fmease]

Independently, you gonna want to account for more complex expressions that need to be wrapped in parentheses. E.g: max(1 + 1, 0) → (1 + 1).max(0) not 1 + 1.max(0).

[Kivooeo]

Hm, do you have idea how can I protect myself from more complex expressions like this, always wrap left part in parentheses? or there is something better I can do about it

Also about multi-part suggestion I'm not sure if I can use it here because as far I as remember it's something with vec of strings and the final type of variable that using here is like this Option<(&Span, &str, String)>

[fmease]

Hm, do you have idea how can I protect myself from more complex expressions like this, always wrap left part in parentheses? or there is something better I can do about it

Yes, you can :) since we're trying to construct a method call, if first_arg.precedence() < ExprPrecedence::Unambiguous it needs parentheses, otherwise it doesn't. That's a good approximation that's also used by rustc_ast_pretty essentially.

[fmease]

Also about multi-part suggestion I'm not sure if I can use it here

Arf, that's annoying; in that case only suggest something if both span_to_snippets are Ok(_).

[Kivooeo]

Yep, I guess I can even add this inside let chains, right after a some zulip poll if we decide on necessity of this diagnostics

[Noratrieb]

which is more idiomatic in rust imo

I disagree, I prefer the function form over the method form (because of the mentioned asymmetry)

[Kivooeo]

Before proceeding, we should agree on whether this diagnostic adds value. Since I've only spent about 15 minutes on it so far, I'm fine with closing it if we decide it's not needed

So would be nice if we could collect some more opinions

[Noratrieb]

The change adds a suggestion for an alternative (and in the diagnostic, encouraged) way to use method syntax for min/max when someone tried to use the min/max functions.
This is a change that is good if we think the methods are preferable over the free functions. This is not a good change if we think the free functions are preferable over the methods.

So really it's a question of style. I prefer the free functions, but I'd suggest making a poll in general on Zulip to gather some other opinions if you think the method style is better or at least has some merit.