From 48fa974211990ec2dd2b8d8c6949d101fb51bb45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 8 Nov 2018 18:54:34 -0800 Subject: [PATCH] Suggest correct syntax when writing type arg instead of assoc type When confusing an associated type with a type argument, suggest the appropriate syntax. Given `Iterator`, suggest `Iterator`. --- src/librustc_typeck/astconv.rs | 114 +++++++++++------- src/librustc_typeck/collect.rs | 11 +- src/librustc_typeck/lib.rs | 2 +- ...se-type-argument-instead-of-assoc-type.rs} | 0 ...ype-argument-instead-of-assoc-type.stderr} | 8 +- 5 files changed, 88 insertions(+), 47 deletions(-) rename src/test/ui/{error-codes/E0107-b.rs => suggestions/use-type-argument-instead-of-assoc-type.rs} (100%) rename src/test/ui/{error-codes/E0107-b.stderr => suggestions/use-type-argument-instead-of-assoc-type.stderr} (75%) diff --git a/src/librustc_typeck/astconv.rs b/src/librustc_typeck/astconv.rs index 173f7ababe36..f1192cf98ae1 100644 --- a/src/librustc_typeck/astconv.rs +++ b/src/librustc_typeck/astconv.rs @@ -182,7 +182,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx>+'o { item_segment: &hir::PathSegment) -> &'tcx Substs<'tcx> { - let (substs, assoc_bindings) = item_segment.with_generic_args(|generic_args| { + let (substs, assoc_bindings, _) = item_segment.with_generic_args(|generic_args| { self.create_substs_for_ast_path( span, def_id, @@ -256,7 +256,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx>+'o { }, def.parent.is_none() && def.has_self, // `has_self` seg.infer_types || suppress_mismatch, // `infer_types` - ) + ).0 } /// Check that the correct number of generic arguments have been provided. @@ -269,7 +269,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx>+'o { position: GenericArgPosition, has_self: bool, infer_types: bool, - ) -> bool { + ) -> (bool, Option>) { // At this stage we are guaranteed that the generic arguments are in the correct order, e.g. // that lifetimes will proceed types. So it suffices to check the number of each generic // arguments in order to validate them with respect to the generic parameters. @@ -303,13 +303,13 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx>+'o { let mut err = tcx.sess.struct_span_err(span, msg); err.span_note(span_late, note); err.emit(); - return true; + return (true, None); } else { let mut multispan = MultiSpan::from_span(span); multispan.push_span_label(span_late, note.to_string()); tcx.lint_node(lint::builtin::LATE_BOUND_LIFETIME_ARGUMENTS, args.args[0].id(), multispan, msg); - return false; + return (false, None); } } } @@ -323,7 +323,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx>+'o { // For kinds without defaults (i.e. lifetimes), `required == permitted`. // For other kinds (i.e. types), `permitted` may be greater than `required`. if required <= provided && provided <= permitted { - return false; + return (false, None); } // Unfortunately lifetime and type parameter mismatches are typically styled @@ -338,19 +338,16 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx>+'o { (required, "") }; + let mut potential_assoc_types: Option> = None; let (spans, label) = if required == permitted && provided > permitted { // In the case when the user has provided too many arguments, // we want to point to the unexpected arguments. - ( - args.args[offset+permitted .. offset+provided] + let spans: Vec = args.args[offset+permitted .. offset+provided] .iter() .map(|arg| arg.span()) - .collect(), - format!( - "unexpected {} argument", - kind, - ), - ) + .collect(); + potential_assoc_types = Some(spans.clone()); + (spans, format!( "unexpected {} argument", kind)) } else { (vec![span], format!( "expected {}{} {} argument{}", @@ -377,7 +374,8 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx>+'o { } err.emit(); - provided > required // `suppress_error` + (provided > required, // `suppress_error` + potential_assoc_types) }; if !infer_lifetimes || arg_counts.lifetimes > param_counts.lifetimes { @@ -399,7 +397,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx>+'o { arg_counts.lifetimes, ) } else { - false + (false, None) } } @@ -557,7 +555,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx>+'o { generic_args: &hir::GenericArgs, infer_types: bool, self_ty: Option>) - -> (&'tcx Substs<'tcx>, Vec>) + -> (&'tcx Substs<'tcx>, Vec>, Option>) { // If the type is parameterized by this region, then replace this // region with the current anon region binding (in other words, @@ -573,7 +571,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx>+'o { assert_eq!(generic_params.has_self, self_ty.is_some()); let has_self = generic_params.has_self; - Self::check_generic_arg_count( + let (_, potential_assoc_types) = Self::check_generic_arg_count( self.tcx(), span, &generic_params, @@ -678,7 +676,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx>+'o { debug!("create_substs_for_ast_path(generic_params={:?}, self_ty={:?}) -> {:?}", generic_params, self_ty, substs); - (substs, assoc_bindings) + (substs, assoc_bindings, potential_assoc_types) } /// Instantiates the path for the given trait reference, assuming that it's @@ -720,7 +718,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx>+'o { self_ty: Ty<'tcx>, poly_projections: &mut Vec<(ty::PolyProjectionPredicate<'tcx>, Span)>, speculative: bool) - -> ty::PolyTraitRef<'tcx> + -> (ty::PolyTraitRef<'tcx>, Option>) { let trait_def_id = self.trait_def_id(trait_ref); @@ -728,11 +726,12 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx>+'o { self.prohibit_generics(trait_ref.path.segments.split_last().unwrap().1); - let (substs, assoc_bindings) = - self.create_substs_for_ast_trait_ref(trait_ref.path.span, - trait_def_id, - self_ty, - trait_ref.path.segments.last().unwrap()); + let (substs, assoc_bindings, potential_assoc_types) = self.create_substs_for_ast_trait_ref( + trait_ref.path.span, + trait_def_id, + self_ty, + trait_ref.path.segments.last().unwrap(), + ); let poly_trait_ref = ty::Binder::bind(ty::TraitRef::new(trait_def_id, substs)); let mut dup_bindings = FxHashMap::default(); @@ -747,14 +746,14 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx>+'o { debug!("instantiate_poly_trait_ref({:?}, projections={:?}) -> {:?}", trait_ref, poly_projections, poly_trait_ref); - poly_trait_ref + (poly_trait_ref, potential_assoc_types) } pub fn instantiate_poly_trait_ref(&self, poly_trait_ref: &hir::PolyTraitRef, self_ty: Ty<'tcx>, poly_projections: &mut Vec<(ty::PolyProjectionPredicate<'tcx>, Span)>) - -> ty::PolyTraitRef<'tcx> + -> (ty::PolyTraitRef<'tcx>, Option>) { self.instantiate_poly_trait_ref_inner(&poly_trait_ref.trait_ref, self_ty, poly_projections, false) @@ -767,7 +766,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx>+'o { trait_segment: &hir::PathSegment) -> ty::TraitRef<'tcx> { - let (substs, assoc_bindings) = + let (substs, assoc_bindings, _) = self.create_substs_for_ast_trait_ref(span, trait_def_id, self_ty, @@ -776,13 +775,13 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx>+'o { ty::TraitRef::new(trait_def_id, substs) } - fn create_substs_for_ast_trait_ref(&self, - span: Span, - trait_def_id: DefId, - self_ty: Ty<'tcx>, - trait_segment: &hir::PathSegment) - -> (&'tcx Substs<'tcx>, Vec>) - { + fn create_substs_for_ast_trait_ref( + &self, + span: Span, + trait_def_id: DefId, + self_ty: Ty<'tcx>, + trait_segment: &hir::PathSegment, + ) -> (&'tcx Substs<'tcx>, Vec>, Option>) { debug!("create_substs_for_ast_trait_ref(trait_segment={:?})", trait_segment); @@ -972,9 +971,11 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx>+'o { let mut projection_bounds = Vec::new(); let dummy_self = tcx.mk_ty(TRAIT_OBJECT_DUMMY_SELF); - let principal = self.instantiate_poly_trait_ref(&trait_bounds[0], - dummy_self, - &mut projection_bounds); + let (principal, potential_assoc_types) = self.instantiate_poly_trait_ref( + &trait_bounds[0], + dummy_self, + &mut projection_bounds, + ); debug!("principal: {:?}", principal); for trait_bound in trait_bounds[1..].iter() { @@ -1047,16 +1048,47 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx>+'o { if associated_types.len() == 1 { "" } else { "s" }, names, ); - for item_def_id in associated_types { - let assoc_item = tcx.associated_item(item_def_id); + let mut suggest = false; + let mut potential_assoc_types_spans = vec![]; + if let Some(potential_assoc_types) = potential_assoc_types { + if potential_assoc_types.len() == associated_types.len() { + // Only suggest when the amount of missing associated types is equals to the + // extra type arguments present, as that gives us a relatively high confidence + // that the user forgot to give the associtated type's name. The canonical + // example would be trying to use `Iterator` instead of + // `Iterator`. + suggest = true; + potential_assoc_types_spans = potential_assoc_types; + } + } + let mut suggestions = vec![]; + for (i, item_def_id) in associated_types.iter().enumerate() { + let assoc_item = tcx.associated_item(*item_def_id); err.span_label( span, format!("missing associated type `{}` value", assoc_item.ident), ); err.span_label( - tcx.def_span(item_def_id), + tcx.def_span(*item_def_id), format!("`{}` defined here", assoc_item.ident), ); + if suggest { + if let Ok(snippet) = tcx.sess.source_map().span_to_snippet( + potential_assoc_types_spans[i], + ) { + suggestions.push(( + potential_assoc_types_spans[i], + format!("{} = {}", assoc_item.ident, snippet), + )); + } + } + } + if !suggestions.is_empty() { + err.multipart_suggestion_with_applicability( + "if you meant to assign the missing associated type, use the name", + suggestions, + Applicability::MaybeIncorrect, + ); } err.emit(); } diff --git a/src/librustc_typeck/collect.rs b/src/librustc_typeck/collect.rs index f3c570e84009..a1bb0b53f1fc 100644 --- a/src/librustc_typeck/collect.rs +++ b/src/librustc_typeck/collect.rs @@ -1892,7 +1892,7 @@ fn explicit_predicates_of<'a, 'tcx>( &hir::GenericBound::Trait(ref poly_trait_ref, _) => { let mut projections = Vec::new(); - let trait_ref = AstConv::instantiate_poly_trait_ref( + let (trait_ref, _) = AstConv::instantiate_poly_trait_ref( &icx, poly_trait_ref, ty, @@ -2016,7 +2016,12 @@ pub fn compute_bounds<'gcx: 'tcx, 'tcx>( let mut projection_bounds = Vec::new(); let mut trait_bounds: Vec<_> = trait_bounds.iter().map(|&bound| { - (astconv.instantiate_poly_trait_ref(bound, param_ty, &mut projection_bounds), bound.span) + let (poly_trait_ref, _) = astconv.instantiate_poly_trait_ref( + bound, + param_ty, + &mut projection_bounds, + ); + (poly_trait_ref, bound.span) }).collect(); let region_bounds = region_bounds @@ -2057,7 +2062,7 @@ fn predicates_from_bound<'tcx>( match *bound { hir::GenericBound::Trait(ref tr, hir::TraitBoundModifier::None) => { let mut projections = Vec::new(); - let pred = astconv.instantiate_poly_trait_ref(tr, param_ty, &mut projections); + let (pred, _) = astconv.instantiate_poly_trait_ref(tr, param_ty, &mut projections); iter::once((pred.to_predicate(), tr.span)).chain( projections .into_iter() diff --git a/src/librustc_typeck/lib.rs b/src/librustc_typeck/lib.rs index 1f5998d8ca39..0fba311d7f7d 100644 --- a/src/librustc_typeck/lib.rs +++ b/src/librustc_typeck/lib.rs @@ -389,7 +389,7 @@ pub fn hir_trait_to_predicates<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, hir_trait: let env_def_id = tcx.hir.local_def_id(env_node_id); let item_cx = self::collect::ItemCtxt::new(tcx, env_def_id); let mut projections = Vec::new(); - let principal = astconv::AstConv::instantiate_poly_trait_ref_inner( + let (principal, _) = astconv::AstConv::instantiate_poly_trait_ref_inner( &item_cx, hir_trait, tcx.types.err, &mut projections, true ); diff --git a/src/test/ui/error-codes/E0107-b.rs b/src/test/ui/suggestions/use-type-argument-instead-of-assoc-type.rs similarity index 100% rename from src/test/ui/error-codes/E0107-b.rs rename to src/test/ui/suggestions/use-type-argument-instead-of-assoc-type.rs diff --git a/src/test/ui/error-codes/E0107-b.stderr b/src/test/ui/suggestions/use-type-argument-instead-of-assoc-type.stderr similarity index 75% rename from src/test/ui/error-codes/E0107-b.stderr rename to src/test/ui/suggestions/use-type-argument-instead-of-assoc-type.stderr index 1e8c683aff6a..053f70104211 100644 --- a/src/test/ui/error-codes/E0107-b.stderr +++ b/src/test/ui/suggestions/use-type-argument-instead-of-assoc-type.stderr @@ -1,5 +1,5 @@ error[E0107]: wrong number of type arguments: expected 2, found 4 - --> $DIR/E0107-b.rs:6:42 + --> $DIR/use-type-argument-instead-of-assoc-type.rs:6:42 | LL | pub struct Foo { i: Box> } | ^^^^^ ^^^^^ unexpected type argument @@ -7,7 +7,7 @@ LL | pub struct Foo { i: Box> } | unexpected type argument error[E0191]: the value of the associated types `A` (from the trait `T`), `C` (from the trait `T`) must be specified - --> $DIR/E0107-b.rs:6:26 + --> $DIR/use-type-argument-instead-of-assoc-type.rs:6:26 | LL | type A; | ------- `A` defined here @@ -20,6 +20,10 @@ LL | pub struct Foo { i: Box> } | | | missing associated type `A` value | missing associated type `C` value +help: if you meant to assign the missing associated type, use the name + | +LL | pub struct Foo { i: Box> } + | ^^^^^^^^^ ^^^^^^^^^ error: aborting due to 2 previous errors