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

Added shadowed hint for overlapping associated types #117661

Merged
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
62 changes: 58 additions & 4 deletions compiler/rustc_hir_analysis/src/astconv/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use rustc_errors::{pluralize, struct_span_err, Applicability, Diagnostic, ErrorG
use rustc_hir as hir;
use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_infer::traits::FulfillmentError;
use rustc_middle::ty::{self, suggest_constraining_type_param, Ty, TyCtxt};
use rustc_middle::ty::{self, suggest_constraining_type_param, AssocItem, AssocKind, Ty, TyCtxt};
use rustc_session::parse::feature_err;
use rustc_span::edit_distance::find_best_match_for_name;
use rustc_span::symbol::{sym, Ident};
Expand Down Expand Up @@ -509,6 +509,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
if associated_types.values().all(|v| v.is_empty()) {
return;
}

let tcx = self.tcx();
// FIXME: Marked `mut` so that we can replace the spans further below with a more
// appropriate one, but this should be handled earlier in the span assignment.
Expand Down Expand Up @@ -581,6 +582,32 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
}
}

// We get all the associated items that _are_ set,
// so that we can check if any of their names match one of the ones we are missing.
// This would mean that they are shadowing the associated type we are missing,
// and we can then use their span to indicate this to the user.
let bound_names = trait_bounds
.iter()
.filter_map(|poly_trait_ref| {
let path = poly_trait_ref.trait_ref.path.segments.last()?;
let args = path.args?;

Some(args.bindings.iter().filter_map(|binding| {
let ident = binding.ident;
let trait_def = path.res.def_id();
let assoc_item = tcx.associated_items(trait_def).find_by_name_and_kind(
tcx,
ident,
AssocKind::Type,
trait_def,
);

Some((ident.name, assoc_item?))
}))
})
.flatten()
.collect::<FxHashMap<Symbol, &AssocItem>>();

let mut names = names
.into_iter()
.map(|(trait_, mut assocs)| {
Expand Down Expand Up @@ -621,25 +648,51 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
*names.entry(item.name).or_insert(0) += 1;
}
let mut dupes = false;
let mut shadows = false;
for item in assoc_items {
let prefix = if names[&item.name] > 1 {
let trait_def_id = item.container_id(tcx);
dupes = true;
format!("{}::", tcx.def_path_str(trait_def_id))
} else if bound_names.get(&item.name).is_some_and(|x| x != &item) {
let trait_def_id = item.container_id(tcx);
shadows = true;
format!("{}::", tcx.def_path_str(trait_def_id))
} else {
String::new()
};

let mut is_shadowed = false;

if let Some(assoc_item) = bound_names.get(&item.name)
&& assoc_item != &item
{
is_shadowed = true;

let rename_message =
if assoc_item.def_id.is_local() { ", consider renaming it" } else { "" };
err.span_label(
tcx.def_span(assoc_item.def_id),
format!("`{}{}` shadowed here{}", prefix, item.name, rename_message),
);
}

let rename_message = if is_shadowed { ", consider renaming it" } else { "" };

if let Some(sp) = tcx.hir().span_if_local(item.def_id) {
err.span_label(sp, format!("`{}{}` defined here", prefix, item.name));
err.span_label(
sp,
format!("`{}{}` defined here{}", prefix, item.name, rename_message),
petrochenkov marked this conversation as resolved.
Show resolved Hide resolved
);
}
}
if potential_assoc_types.len() == assoc_items.len() {
// When the amount of missing associated types equals the number of
// extra type arguments present. A suggesting to replace the generic args with
// associated types is already emitted.
already_has_generics_args_suggestion = true;
} else if let (Ok(snippet), false) =
(tcx.sess.source_map().span_to_snippet(*span), dupes)
} else if let (Ok(snippet), false, false) =
(tcx.sess.source_map().span_to_snippet(*span), dupes, shadows)
{
let types: Vec<_> =
assoc_items.iter().map(|item| format!("{} = Type", item.name)).collect();
Expand Down Expand Up @@ -721,6 +774,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
err.span_help(where_constraints, where_msg);
}
}

err.emit();
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Test that no help message is emitted that suggests renaming the
// associated type from a non-local trait

pub trait NewIter: Iterator {
type Item;
}

impl<T> Clone for Box<dyn NewIter<Item = T>> {
//~^ ERROR the value of the associated type `Item` in `Iterator` must be specified
fn clone(&self) -> Self {
unimplemented!();
}
}

pub fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
error[E0191]: the value of the associated type `Item` in `Iterator` must be specified
--> $DIR/associated-type-shadowed-from-non-local-supertrait.rs:8:27
|
LL | type Item;
| --------- `Iterator::Item` shadowed here, consider renaming it
...
LL | impl<T> Clone for Box<dyn NewIter<Item = T>> {
| ^^^^^^^^^^^^^^^^^ associated type `Item` must be specified

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0191`.
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// Test Setting the value of an associated type
// that is shadowed from a supertrait

pub trait Super {
type X;
}

pub trait Sub: Super {
type X;
}

impl<T> Clone for Box<dyn Sub<X = T>> {
//~^ ERROR value of the associated type `X` in `Super` must be specified
fn clone(&self) -> Self {
unimplemented!();
}
}

pub fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
error[E0191]: the value of the associated type `X` in `Super` must be specified
--> $DIR/associated-type-shadowed-from-supertrait.rs:12:27
|
LL | type X;
| ------ `Super::X` defined here, consider renaming it
...
LL | type X;
| ------ `Super::X` shadowed here, consider renaming it
...
LL | impl<T> Clone for Box<dyn Sub<X = T>> {
| ^^^^^^^^^^ associated type `X` must be specified

error: aborting due to 1 previous error

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