improve associated-type suggestions from bounds#152471
improve associated-type suggestions from bounds#152471JohnTitor wants to merge 2 commits intorust-lang:mainfrom
Conversation
rustbot
added
S-waiting-on-review
| match suggestions.as_slice() { | ||
| [one] => { | ||
| err.span_suggestion_verbose( | ||
| ident_span, | ||
| "use the associated type", | ||
| one, | ||
| Applicability::MaybeIncorrect, | ||
| ); | ||
| } | ||
| _ => { | ||
| err.span_suggestions( | ||
| ident_span, | ||
| "use an associated type", | ||
| suggestions, | ||
| Applicability::MaybeIncorrect, | ||
| ); | ||
| } | ||
| }; |
There was a problem hiding this comment.
I think we have span_suggestions_verbose already? That would make the match unnecessary. If not, then we should just publish a PR making span_suggestions be always verbose and be done with it (inline suggestions are rarely the best option and we should inverse the default to verbose and opt-into inline suggestions on a per case basis).
There was a problem hiding this comment.
AFAIK no, but I've found span_suggestions_with_style and it could be used here, I guess: 391a395 (this PR)
How about this?
| [one] => { | ||
| err.span_suggestion_verbose( | ||
| ident_span, | ||
| "use the associated type", |
There was a problem hiding this comment.
What about "you might have meant to use an associated type of the same name"?
There was a problem hiding this comment.
Yeah, it should be! Since we emit the suggestion as MaybeIncorrect, your wording should be better. Addressed in 391a395 (this PR)
| fn path_to_string_without_assoc_item_bindings(path: &Path) -> String { | ||
| let mut path = path.clone(); | ||
| for segment in &mut path.segments { | ||
| let mut remove_args = false; | ||
| if let Some(args) = segment.args.as_deref_mut() | ||
| && let ast::GenericArgs::AngleBracketed(angle_bracketed) = args | ||
| { | ||
| angle_bracketed.args.retain(|arg| matches!(arg, ast::AngleBracketedArg::Arg(_))); | ||
| remove_args = angle_bracketed.args.is_empty(); | ||
| } | ||
| if remove_args { | ||
| segment.args = None; | ||
| } | ||
| } | ||
| path_to_string(&path) | ||
| } |
There was a problem hiding this comment.
I wonder if we don't already have something like this in pprint? I know there are a bunch of modifier macros that you can surround a format!("{ty}") with that will produce different output depending on the modifier.
There was a problem hiding this comment.
I've checked existing support a bit but couldn't find anything for what we do (in particular, ast::Path) here.
| fn assoc_type_required_generic_args_suggestion(&self, assoc_type_def_id: DefId) -> String { | ||
| self.r.item_required_generic_args_suggestion(assoc_type_def_id) | ||
| } |
There was a problem hiding this comment.
Do we need this? It's only used twice :)
| if let Some(item) = self.diag_metadata.current_item | ||
| && let Some(generics) = item.kind.generics() | ||
| { | ||
| record_from_generics(self, generics); | ||
| } | ||
|
|
||
| if let Some(assoc) = self.diag_metadata.current_impl_item { |
There was a problem hiding this comment.
Could we add a test case for an impl declared within an fn, and vice-versa? I'm intrigued if this would cause record_from_generics to then provide suggestions that would incorrectly mention outer type params. This would only happen if diag_metadata isn't being properly updated elsewhere, so this logic might still be right.
There was a problem hiding this comment.
Ah, good catch!
I've tweaked if branch and added tests you asked: 391a395 (this PR)
JohnTitor
force-pushed
the
sugg-assoc-items-from-bounds
branch
from
9645914 to
485f95f
Compare
JohnTitor
force-pushed
the
sugg-assoc-items-from-bounds
branch
from
6f5575f to
391a395
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
@bors r+ |
rust-bors
bot
added
S-waiting-on-bors
…unds, r=estebank improve associated-type suggestions from bounds Should address invalid suggestions I could come up with, but the suggestion is too hand-crafted and invalid patterns may exist. r? @estebank Fix rust-lang#73321
Rollup of 11 pull requests Successful merges: - #151998 (Set hidden visibility on naked functions in compiler-builtins) - #149460 (rustdoc: sort stable items first) - #152076 (Feed `ErrorGuaranteed` from late lifetime resolution errors through to bound variable resolution) - #152471 (improve associated-type suggestions from bounds) - #152573 (move `escape_symbol_name` to `cg_ssa`) - #152594 (c-variadic: implement `va_arg` for `wasm64`) - #151386 (rustdoc: more js cleanup) - #152567 (nix-dev-shell: fix a typo) - #152568 (Port `#[lang]` and `#[panic_handler]` to the new attribute parsers) - #152575 (layout_of unexpected rigid alias delayed bug) - #152587 (A couple of tiny polonius things)
3 participants
Should address invalid suggestions I could come up with, but the suggestion is too hand-crafted and invalid patterns may exist.
r? @estebank
Fix #73321