Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Properly suggest E::assoc when we encounter E::Variant::assoc #132567

Merged
merged 2 commits into from
Nov 6, 2024
Merged
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: 36 additions & 10 deletions compiler/rustc_resolve/src/late/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -459,7 +459,7 @@ impl<'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> {
return (err, Vec::new());
}

let (found, mut candidates) = self.try_lookup_name_relaxed(
let (found, suggested_candidates, mut candidates) = self.try_lookup_name_relaxed(
&mut err,
source,
path,
Expand All @@ -478,7 +478,15 @@ impl<'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> {
}

let mut fallback = self.suggest_trait_and_bounds(&mut err, source, res, span, &base_error);
fallback |= self.suggest_typo(&mut err, source, path, following_seg, span, &base_error);
fallback |= self.suggest_typo(
&mut err,
source,
path,
following_seg,
span,
&base_error,
suggested_candidates,
);

if fallback {
// Fallback label.
Expand Down Expand Up @@ -589,7 +597,16 @@ impl<'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> {
span: Span,
res: Option<Res>,
base_error: &BaseError,
) -> (bool, Vec<ImportSuggestion>) {
) -> (bool, FxHashSet<String>, Vec<ImportSuggestion>) {
let span = match following_seg {
Some(_) if path[0].ident.span.eq_ctxt(path[path.len() - 1].ident.span) => {
Copy link
Member

@Nadrieril Nadrieril Nov 3, 2024

Choose a reason for hiding this comment

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

Is the branch on following_seq necessary? Presumably the path-derived span would be the same as the input span but are there cases where that's not true?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

It's always macros

// The path `span` that comes in includes any following segments, which we don't
// want to replace in the suggestions.
path[0].ident.span.to(path[path.len() - 1].ident.span)
}
_ => span,
};
let mut suggested_candidates = FxHashSet::default();
// Try to lookup name in more relaxed fashion for better error reporting.
let ident = path.last().unwrap().ident;
let is_expected = &|res| source.is_expected(res);
Expand Down Expand Up @@ -646,6 +663,11 @@ impl<'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> {
};
let msg = format!("{preamble}try using the variant's enum");

suggested_candidates.extend(
enum_candidates
.iter()
.map(|(_variant_path, enum_ty_path)| enum_ty_path.clone()),
);
err.span_suggestions(
span,
msg,
Expand All @@ -658,7 +680,8 @@ impl<'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> {
// Try finding a suitable replacement.
let typo_sugg = self
.lookup_typo_candidate(path, following_seg, source.namespace(), is_expected)
.to_opt_suggestion();
.to_opt_suggestion()
.filter(|sugg| !suggested_candidates.contains(sugg.candidate.as_str()));
if let [segment] = path
&& !matches!(source, PathSource::Delegation)
&& self.self_type_is_available()
Expand Down Expand Up @@ -719,7 +742,7 @@ impl<'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> {
}
}
self.r.add_typo_suggestion(err, typo_sugg, ident_span);
return (true, candidates);
return (true, suggested_candidates, candidates);
}

// If the first argument in call is `self` suggest calling a method.
Expand All @@ -737,7 +760,7 @@ impl<'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> {
format!("self.{path_str}({args_snippet})"),
Applicability::MachineApplicable,
);
return (true, candidates);
return (true, suggested_candidates, candidates);
}
}

Expand All @@ -754,7 +777,7 @@ impl<'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> {
) {
// We do this to avoid losing a secondary span when we override the main error span.
self.r.add_typo_suggestion(err, typo_sugg, ident_span);
return (true, candidates);
return (true, suggested_candidates, candidates);
}
}

Expand All @@ -772,7 +795,7 @@ impl<'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> {
ident.span,
format!("the binding `{path_str}` is available in a different scope in the same function"),
);
return (true, candidates);
return (true, suggested_candidates, candidates);
}
}
}
Expand All @@ -781,7 +804,7 @@ impl<'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> {
candidates = self.smart_resolve_partial_mod_path_errors(path, following_seg);
}

(false, candidates)
(false, suggested_candidates, candidates)
}

fn suggest_trait_and_bounds(
Expand Down Expand Up @@ -869,13 +892,16 @@ impl<'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> {
following_seg: Option<&Segment>,
span: Span,
base_error: &BaseError,
suggested_candidates: FxHashSet<String>,
) -> bool {
let is_expected = &|res| source.is_expected(res);
let ident_span = path.last().map_or(span, |ident| ident.ident.span);
let typo_sugg =
self.lookup_typo_candidate(path, following_seg, source.namespace(), is_expected);
let mut fallback = false;
let typo_sugg = typo_sugg.to_opt_suggestion();
let typo_sugg = typo_sugg
.to_opt_suggestion()
.filter(|sugg| !suggested_candidates.contains(sugg.candidate.as_str()));
if !self.r.add_typo_suggestion(err, typo_sugg, ident_span) {
fallback = true;
match self.diag_metadata.current_let_binding {
Expand Down
15 changes: 15 additions & 0 deletions tests/ui/enum/assoc-fn-call-on-variant.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#[derive(Default)]
enum E {
A {},
B {},
#[default]
C,
}

impl E {
fn f() {}
}

fn main() {
E::A::f(); //~ ERROR failed to resolve: `A` is a variant, not a module
}
14 changes: 14 additions & 0 deletions tests/ui/enum/assoc-fn-call-on-variant.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
error[E0433]: failed to resolve: `A` is a variant, not a module
--> $DIR/assoc-fn-call-on-variant.rs:14:8
|
LL | E::A::f();
| ^ `A` is a variant, not a module
|
help: there is an enum variant `E::A`; try using the variant's enum
|
LL | E::f();
| ~

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0433`.
6 changes: 1 addition & 5 deletions tests/ui/resolve/resolve-variant-assoc-item.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ LL | E::V::associated_item;
|
help: there is an enum variant `E::V`; try using the variant's enum
|
LL | E;
LL | E::associated_item;
| ~

error[E0433]: failed to resolve: `V` is a variant, not a module
Expand All @@ -17,10 +17,6 @@ LL | V::associated_item;
|
help: there is an enum variant `E::V`; try using the variant's enum
|
LL | E;
| ~
help: an enum with a similar name exists
|
LL | E::associated_item;
| ~

Expand Down
Loading