From 3234418806de31db283a00fbcf70cd2857c9a81e Mon Sep 17 00:00:00 2001 From: TheLazyDutchman Date: Sun, 5 Nov 2023 15:29:18 +0100 Subject: [PATCH] Point out shadowed associated types Shadowing the associated type of a supertrait is allowed. This however makes it impossible to set the associated type of the supertrait in a dyn object. This PR makes the error message for that case clearer, like adding a note that shadowing is happening, as well as suggesting renaming of one of the associated types. r=petrochenckov --- .../rustc_hir_analysis/src/astconv/errors.rs | 62 +++++++++++++++++-- ...type-shadowed-from-non-local-supertrait.rs | 15 +++++ ...-shadowed-from-non-local-supertrait.stderr | 12 ++++ ...ssociated-type-shadowed-from-supertrait.rs | 19 ++++++ ...iated-type-shadowed-from-supertrait.stderr | 15 +++++ 5 files changed, 119 insertions(+), 4 deletions(-) create mode 100644 tests/ui/associated-types/associated-type-shadowed-from-non-local-supertrait.rs create mode 100644 tests/ui/associated-types/associated-type-shadowed-from-non-local-supertrait.stderr create mode 100644 tests/ui/associated-types/associated-type-shadowed-from-supertrait.rs create mode 100644 tests/ui/associated-types/associated-type-shadowed-from-supertrait.stderr diff --git a/compiler/rustc_hir_analysis/src/astconv/errors.rs b/compiler/rustc_hir_analysis/src/astconv/errors.rs index b53ffb98bf4..eb12ec7a098 100644 --- a/compiler/rustc_hir_analysis/src/astconv/errors.rs +++ b/compiler/rustc_hir_analysis/src/astconv/errors.rs @@ -9,7 +9,7 @@ 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}; @@ -509,6 +509,7 @@ pub(crate) fn complain_about_missing_associated_types( 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. @@ -581,6 +582,32 @@ pub(crate) fn complain_about_missing_associated_types( } } + // 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::>(); + let mut names = names .into_iter() .map(|(trait_, mut assocs)| { @@ -621,16 +648,42 @@ pub(crate) fn complain_about_missing_associated_types( *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), + ); } } if potential_assoc_types.len() == assoc_items.len() { @@ -638,8 +691,8 @@ pub(crate) fn complain_about_missing_associated_types( // 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(); @@ -721,6 +774,7 @@ pub(crate) fn complain_about_missing_associated_types( err.span_help(where_constraints, where_msg); } } + err.emit(); } } diff --git a/tests/ui/associated-types/associated-type-shadowed-from-non-local-supertrait.rs b/tests/ui/associated-types/associated-type-shadowed-from-non-local-supertrait.rs new file mode 100644 index 00000000000..045a43548dc --- /dev/null +++ b/tests/ui/associated-types/associated-type-shadowed-from-non-local-supertrait.rs @@ -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 Clone for Box> { + //~^ ERROR the value of the associated type `Item` in `Iterator` must be specified + fn clone(&self) -> Self { + unimplemented!(); + } +} + +pub fn main() {} diff --git a/tests/ui/associated-types/associated-type-shadowed-from-non-local-supertrait.stderr b/tests/ui/associated-types/associated-type-shadowed-from-non-local-supertrait.stderr new file mode 100644 index 00000000000..fdb4e4fc1a8 --- /dev/null +++ b/tests/ui/associated-types/associated-type-shadowed-from-non-local-supertrait.stderr @@ -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 Clone for Box> { + | ^^^^^^^^^^^^^^^^^ associated type `Item` must be specified + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0191`. diff --git a/tests/ui/associated-types/associated-type-shadowed-from-supertrait.rs b/tests/ui/associated-types/associated-type-shadowed-from-supertrait.rs new file mode 100644 index 00000000000..95d03892838 --- /dev/null +++ b/tests/ui/associated-types/associated-type-shadowed-from-supertrait.rs @@ -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 Clone for Box> { + //~^ ERROR value of the associated type `X` in `Super` must be specified + fn clone(&self) -> Self { + unimplemented!(); + } +} + +pub fn main() {} diff --git a/tests/ui/associated-types/associated-type-shadowed-from-supertrait.stderr b/tests/ui/associated-types/associated-type-shadowed-from-supertrait.stderr new file mode 100644 index 00000000000..e6cec255eee --- /dev/null +++ b/tests/ui/associated-types/associated-type-shadowed-from-supertrait.stderr @@ -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 Clone for Box> { + | ^^^^^^^^^^ associated type `X` must be specified + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0191`.