Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 31 additions & 15 deletions compiler/rustc_resolve/src/late/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -336,21 +336,37 @@ impl<'ast, 'ra, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> {
("`async` blocks are only allowed in Rust 2018 or later".to_string(), suggestion)
} else {
// check if we are in situation of typo like `True` instead of `true`.
let override_suggestion =
if ["true", "false"].contains(&item_str.to_string().to_lowercase().as_str()) {
let item_typo = item_str.to_string().to_lowercase();
Some((item_span, "you may want to use a bool value instead", item_typo))
// FIXME(vincenzopalazzo): make the check smarter,
// and maybe expand with levenshtein distance checks
} else if item_str.as_str() == "printf" {
Some((
item_span,
"you may have meant to use the `print` macro",
"print!".to_owned(),
))
} else {
suggestion
};
let override_suggestion = if ["true", "false"]
.contains(&item_str.to_string().to_lowercase().as_str())
{
let item_typo = item_str.to_string().to_lowercase();
Some((item_span, "you may want to use a bool value instead", item_typo))
// FIXME(vincenzopalazzo): make the check smarter,
// and maybe expand with levenshtein distance checks
} else if item_str.as_str() == "printf" {
Some((
item_span,
"you may have meant to use the `print` macro",
"print!".to_owned(),
))
} else if ["max", "min"].contains(&item_str.as_str())
&& let PathSource::Expr(Some(Expr {
kind: ExprKind::Call(_, args),
span: call_span,
..
})) = source
&& args.len() == 2
{
let arg0 = self.r.tcx.sess.source_map().span_to_snippet(args[0].span).unwrap();
let arg1 = self.r.tcx.sess.source_map().span_to_snippet(args[1].span).unwrap();
Comment on lines +360 to +361
Copy link
Member

@fmease fmease Sep 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

@fmease fmease Sep 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Member

@fmease fmease Sep 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Member Author

@Kivooeo Kivooeo Sep 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)>

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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(_).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Some((
*call_span,
"you may have meant to use the method syntax",
format!("{arg0}.{item_str}({arg1})"),
))
} else {
suggestion
};
(format!("not found in {mod_str}"), override_suggestion)
};

Expand Down
29 changes: 29 additions & 0 deletions tests/ui/did_you_mean/method-syntax-for-min-max.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
fn main() {
//~^ HELP consider importing this function
//~| HELP consider importing this function
//~| HELP consider importing this function
//~| HELP consider importing this function
let x = 2;
let y = 4;

max(x, y);
//~^ ERROR cannot find function `max` in this scope
//~| HELP you may have meant to use the method syntax
let _ = min(x, y);
//~^ ERROR cannot find function `min` in this scope
//~| HELP you may have meant to use the method syntax
println!("{}", min(43, 43));
//~^ ERROR cannot find function `min` in this scope
//~| HELP you may have meant to use the method syntax
let _ = vec![max(f(), g())];
//~^ ERROR cannot find function `max` in this scope
//~| HELP you may have meant to use the method syntax
}

const fn f() -> u32 {
4
}

const fn g() -> u32 {
2
}
67 changes: 67 additions & 0 deletions tests/ui/did_you_mean/method-syntax-for-min-max.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
error[E0425]: cannot find function `max` in this scope
--> $DIR/method-syntax-for-min-max.rs:9:5
|
LL | max(x, y);
| ^^^ not found in this scope
|
help: you may have meant to use the method syntax
|
LL - max(x, y);
LL + x.max(y);
|
help: consider importing this function
|
LL + use std::cmp::max;
|

error[E0425]: cannot find function `min` in this scope
--> $DIR/method-syntax-for-min-max.rs:12:13
|
LL | let _ = min(x, y);
| ^^^ not found in this scope
|
help: you may have meant to use the method syntax
|
LL - let _ = min(x, y);
LL + let _ = x.min(y);
|
help: consider importing this function
|
LL + use std::cmp::min;
|

error[E0425]: cannot find function `min` in this scope
--> $DIR/method-syntax-for-min-max.rs:15:20
|
LL | println!("{}", min(43, 43));
| ^^^ not found in this scope
|
help: you may have meant to use the method syntax
|
LL - println!("{}", min(43, 43));
LL + println!("{}", 43.min(43));
|
help: consider importing this function
|
LL + use std::cmp::min;
|

error[E0425]: cannot find function `max` in this scope
--> $DIR/method-syntax-for-min-max.rs:18:18
|
LL | let _ = vec![max(f(), g())];
| ^^^ not found in this scope
|
help: you may have meant to use the method syntax
|
LL - let _ = vec![max(f(), g())];
LL + let _ = vec![f().max(g())];
|
help: consider importing this function
|
LL + use std::cmp::max;
|

error: aborting due to 4 previous errors

For more information about this error, try `rustc --explain E0425`.
Loading