Skip to content

Commit

Permalink
Auto merge of #68447 - estebank:sugg-type-param, r=petrochenkov
Browse files Browse the repository at this point in the history
Suggest defining type parameter when appropriate

```
error[E0412]: cannot find type `T` in this scope
 --> file.rs:3:12
  |
3 | impl Trait<T> for Struct {}
  |     -      ^ not found in this scope
  |     |
  |     help: you might be missing a type parameter: `<T>`
```

Fix #64298.
  • Loading branch information
bors committed Jan 27, 2020
2 parents c3681d6 + 697fdc5 commit 320ada6
Show file tree
Hide file tree
Showing 12 changed files with 178 additions and 66 deletions.
5 changes: 5 additions & 0 deletions src/librustc_resolve/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1415,11 +1415,15 @@ crate fn show_candidates(
better: bool,
found_use: bool,
) {
if candidates.is_empty() {
return;
}
// we want consistent results across executions, but candidates are produced
// by iterating through a hash map, so make sure they are ordered:
let mut path_strings: Vec<_> =
candidates.into_iter().map(|c| path_names_to_string(&c.path)).collect();
path_strings.sort();
path_strings.dedup();

let better = if better { "better " } else { "" };
let msg_diff = match path_strings.len() {
Expand All @@ -1444,6 +1448,7 @@ crate fn show_candidates(
msg.push('\n');
msg.push_str(&candidate);
}
err.note(&msg);
}
}

Expand Down
110 changes: 61 additions & 49 deletions src/librustc_resolve/late.rs
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ impl<'a> PathSource<'a> {
}

#[derive(Default)]
struct DiagnosticMetadata {
struct DiagnosticMetadata<'ast> {
/// The current trait's associated types' ident, used for diagnostic suggestions.
current_trait_assoc_types: Vec<Ident>,

Expand All @@ -333,6 +333,13 @@ struct DiagnosticMetadata {
/// The current self item if inside an ADT (used for better errors).
current_self_item: Option<NodeId>,

/// The current trait (used to suggest).
current_item: Option<&'ast Item>,

/// When processing generics and encountering a type not found, suggest introducing a type
/// param.
currently_processing_generics: bool,

/// The current enclosing function (used for better errors).
current_function: Option<Span>,

Expand All @@ -347,7 +354,7 @@ struct DiagnosticMetadata {
current_let_binding: Option<(Span, Option<Span>, Option<Span>)>,
}

struct LateResolutionVisitor<'a, 'b> {
struct LateResolutionVisitor<'a, 'b, 'ast> {
r: &'b mut Resolver<'a>,

/// The module that represents the current item scope.
Expand All @@ -364,30 +371,32 @@ struct LateResolutionVisitor<'a, 'b> {
current_trait_ref: Option<(Module<'a>, TraitRef)>,

/// Fields used to add information to diagnostic errors.
diagnostic_metadata: DiagnosticMetadata,
diagnostic_metadata: DiagnosticMetadata<'ast>,
}

/// Walks the whole crate in DFS order, visiting each item, resolving names as it goes.
impl<'a, 'tcx> Visitor<'tcx> for LateResolutionVisitor<'a, '_> {
fn visit_item(&mut self, item: &'tcx Item) {
impl<'a, 'ast> Visitor<'ast> for LateResolutionVisitor<'a, '_, 'ast> {
fn visit_item(&mut self, item: &'ast Item) {
let prev = replace(&mut self.diagnostic_metadata.current_item, Some(item));
self.resolve_item(item);
self.diagnostic_metadata.current_item = prev;
}
fn visit_arm(&mut self, arm: &'tcx Arm) {
fn visit_arm(&mut self, arm: &'ast Arm) {
self.resolve_arm(arm);
}
fn visit_block(&mut self, block: &'tcx Block) {
fn visit_block(&mut self, block: &'ast Block) {
self.resolve_block(block);
}
fn visit_anon_const(&mut self, constant: &'tcx AnonConst) {
fn visit_anon_const(&mut self, constant: &'ast AnonConst) {
debug!("visit_anon_const {:?}", constant);
self.with_constant_rib(|this| {
visit::walk_anon_const(this, constant);
});
}
fn visit_expr(&mut self, expr: &'tcx Expr) {
fn visit_expr(&mut self, expr: &'ast Expr) {
self.resolve_expr(expr, None);
}
fn visit_local(&mut self, local: &'tcx Local) {
fn visit_local(&mut self, local: &'ast Local) {
let local_spans = match local.pat.kind {
// We check for this to avoid tuple struct fields.
PatKind::Wild => None,
Expand All @@ -401,7 +410,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LateResolutionVisitor<'a, '_> {
self.resolve_local(local);
self.diagnostic_metadata.current_let_binding = original;
}
fn visit_ty(&mut self, ty: &'tcx Ty) {
fn visit_ty(&mut self, ty: &'ast Ty) {
match ty.kind {
TyKind::Path(ref qself, ref path) => {
self.smart_resolve_path(ty.id, qself.as_ref(), path, PathSource::Type);
Expand All @@ -417,7 +426,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LateResolutionVisitor<'a, '_> {
}
visit::walk_ty(self, ty);
}
fn visit_poly_trait_ref(&mut self, tref: &'tcx PolyTraitRef, m: &'tcx TraitBoundModifier) {
fn visit_poly_trait_ref(&mut self, tref: &'ast PolyTraitRef, m: &'ast TraitBoundModifier) {
self.smart_resolve_path(
tref.trait_ref.ref_id,
None,
Expand All @@ -426,7 +435,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LateResolutionVisitor<'a, '_> {
);
visit::walk_poly_trait_ref(self, tref, m);
}
fn visit_foreign_item(&mut self, foreign_item: &'tcx ForeignItem) {
fn visit_foreign_item(&mut self, foreign_item: &'ast ForeignItem) {
match foreign_item.kind {
ForeignItemKind::Fn(_, ref generics) => {
self.with_generic_param_rib(generics, ItemRibKind(HasGenericParams::Yes), |this| {
Expand All @@ -443,7 +452,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LateResolutionVisitor<'a, '_> {
}
}
}
fn visit_fn(&mut self, fn_kind: FnKind<'tcx>, declaration: &'tcx FnDecl, sp: Span, _: NodeId) {
fn visit_fn(&mut self, fn_kind: FnKind<'ast>, declaration: &'ast FnDecl, sp: Span, _: NodeId) {
let previous_value = replace(&mut self.diagnostic_metadata.current_function, Some(sp));
debug!("(resolving function) entering function");
let rib_kind = match fn_kind {
Expand Down Expand Up @@ -472,7 +481,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LateResolutionVisitor<'a, '_> {
self.diagnostic_metadata.current_function = previous_value;
}

fn visit_generics(&mut self, generics: &'tcx Generics) {
fn visit_generics(&mut self, generics: &'ast Generics) {
// For type parameter defaults, we have to ban access
// to following type parameters, as the InternalSubsts can only
// provide previous type parameters as they're built. We
Expand Down Expand Up @@ -534,11 +543,12 @@ impl<'a, 'tcx> Visitor<'tcx> for LateResolutionVisitor<'a, '_> {
}
}

fn visit_generic_arg(&mut self, arg: &'tcx GenericArg) {
fn visit_generic_arg(&mut self, arg: &'ast GenericArg) {
debug!("visit_generic_arg({:?})", arg);
let prev = replace(&mut self.diagnostic_metadata.currently_processing_generics, true);
match arg {
GenericArg::Type(ref ty) => {
// We parse const arguments as path types as we cannot distiguish them durring
// We parse const arguments as path types as we cannot distiguish them during
// parsing. We try to resolve that ambiguity by attempting resolution the type
// namespace first, and if that fails we try again in the value namespace. If
// resolution in the value namespace succeeds, we have an generic const argument on
Expand All @@ -556,7 +566,6 @@ impl<'a, 'tcx> Visitor<'tcx> for LateResolutionVisitor<'a, '_> {
)
.is_some()
};

if !check_ns(TypeNS) && check_ns(ValueNS) {
// This must be equivalent to `visit_anon_const`, but we cannot call it
// directly due to visitor lifetimes so we have to copy-paste some code.
Expand All @@ -574,6 +583,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LateResolutionVisitor<'a, '_> {
this.visit_path(path, ty.id);
});

self.diagnostic_metadata.currently_processing_generics = prev;
return;
}
}
Expand All @@ -584,11 +594,12 @@ impl<'a, 'tcx> Visitor<'tcx> for LateResolutionVisitor<'a, '_> {
GenericArg::Lifetime(lt) => self.visit_lifetime(lt),
GenericArg::Const(ct) => self.visit_anon_const(ct),
}
self.diagnostic_metadata.currently_processing_generics = prev;
}
}

impl<'a, 'b> LateResolutionVisitor<'a, '_> {
fn new(resolver: &'b mut Resolver<'a>) -> LateResolutionVisitor<'a, 'b> {
impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
fn new(resolver: &'b mut Resolver<'a>) -> LateResolutionVisitor<'a, 'b, 'ast> {
// During late resolution we only track the module component of the parent scope,
// although it may be useful to track other components as well for diagnostics.
let graph_root = resolver.graph_root;
Expand Down Expand Up @@ -724,7 +735,7 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> {
None
}

fn resolve_adt(&mut self, item: &Item, generics: &Generics) {
fn resolve_adt(&mut self, item: &'ast Item, generics: &'ast Generics) {
debug!("resolve_adt");
self.with_current_self_item(item, |this| {
this.with_generic_param_rib(generics, ItemRibKind(HasGenericParams::Yes), |this| {
Expand Down Expand Up @@ -778,7 +789,7 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> {
}
}

fn resolve_item(&mut self, item: &Item) {
fn resolve_item(&mut self, item: &'ast Item) {
let name = item.ident.name;
debug!("(resolving item) resolving {} ({:?})", name, item.kind);

Expand Down Expand Up @@ -1024,16 +1035,15 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> {
let mut new_id = None;
if let Some(trait_ref) = opt_trait_ref {
let path: Vec<_> = Segment::from_path(&trait_ref.path);
let res = self
.smart_resolve_path_fragment(
trait_ref.ref_id,
None,
&path,
trait_ref.path.span,
PathSource::Trait(AliasPossibility::No),
CrateLint::SimplePath(trait_ref.ref_id),
)
.base_res();
let res = self.smart_resolve_path_fragment(
trait_ref.ref_id,
None,
&path,
trait_ref.path.span,
PathSource::Trait(AliasPossibility::No),
CrateLint::SimplePath(trait_ref.ref_id),
);
let res = res.base_res();
if res != Res::Err {
new_id = Some(res.def_id());
let span = trait_ref.path.span;
Expand Down Expand Up @@ -1070,11 +1080,11 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> {

fn resolve_implementation(
&mut self,
generics: &Generics,
opt_trait_reference: &Option<TraitRef>,
self_type: &Ty,
generics: &'ast Generics,
opt_trait_reference: &'ast Option<TraitRef>,
self_type: &'ast Ty,
item_id: NodeId,
impl_items: &[AssocItem],
impl_items: &'ast [AssocItem],
) {
debug!("resolve_implementation");
// If applicable, create a rib for the type parameters.
Expand Down Expand Up @@ -1179,7 +1189,7 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> {
}
}

fn resolve_params(&mut self, params: &[Param]) {
fn resolve_params(&mut self, params: &'ast [Param]) {
let mut bindings = smallvec![(PatBoundCtx::Product, Default::default())];
for Param { pat, ty, .. } in params {
self.resolve_pattern(pat, PatternSource::FnParam, &mut bindings);
Expand All @@ -1188,7 +1198,7 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> {
}
}

fn resolve_local(&mut self, local: &Local) {
fn resolve_local(&mut self, local: &'ast Local) {
// Resolve the type.
walk_list!(self, visit_ty, &local.ty);

Expand Down Expand Up @@ -1307,7 +1317,7 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> {
}

/// Check the consistency of the outermost or-patterns.
fn check_consistent_bindings_top(&mut self, pat: &Pat) {
fn check_consistent_bindings_top(&mut self, pat: &'ast Pat) {
pat.walk(&mut |pat| match pat.kind {
PatKind::Or(ref ps) => {
self.check_consistent_bindings(ps);
Expand All @@ -1317,7 +1327,7 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> {
})
}

fn resolve_arm(&mut self, arm: &Arm) {
fn resolve_arm(&mut self, arm: &'ast Arm) {
self.with_rib(ValueNS, NormalRibKind, |this| {
this.resolve_pattern_top(&arm.pat, PatternSource::Match);
walk_list!(this, visit_expr, &arm.guard);
Expand All @@ -1326,14 +1336,14 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> {
}

/// Arising from `source`, resolve a top level pattern.
fn resolve_pattern_top(&mut self, pat: &Pat, pat_src: PatternSource) {
fn resolve_pattern_top(&mut self, pat: &'ast Pat, pat_src: PatternSource) {
let mut bindings = smallvec![(PatBoundCtx::Product, Default::default())];
self.resolve_pattern(pat, pat_src, &mut bindings);
}

fn resolve_pattern(
&mut self,
pat: &Pat,
pat: &'ast Pat,
pat_src: PatternSource,
bindings: &mut SmallVec<[(PatBoundCtx, FxHashSet<Ident>); 1]>,
) {
Expand Down Expand Up @@ -1544,7 +1554,7 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> {
id: NodeId,
qself: Option<&QSelf>,
path: &Path,
source: PathSource<'_>,
source: PathSource<'ast>,
) {
self.smart_resolve_path_fragment(
id,
Expand All @@ -1562,7 +1572,7 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> {
qself: Option<&QSelf>,
path: &[Segment],
span: Span,
source: PathSource<'_>,
source: PathSource<'ast>,
crate_lint: CrateLint,
) -> PartialRes {
let ns = source.namespace();
Expand All @@ -1573,7 +1583,9 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> {
let def_id = this.parent_scope.module.normal_ancestor_id;
let node_id = this.r.definitions.as_local_node_id(def_id).unwrap();
let better = res.is_some();
this.r.use_injections.push(UseError { err, candidates, node_id, better });
let suggestion =
if res.is_none() { this.report_missing_type_error(path) } else { None };
this.r.use_injections.push(UseError { err, candidates, node_id, better, suggestion });
PartialRes::new(Res::Err)
};

Expand Down Expand Up @@ -1838,11 +1850,11 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> {
}
}

fn resolve_labeled_block(&mut self, label: Option<Label>, id: NodeId, block: &Block) {
fn resolve_labeled_block(&mut self, label: Option<Label>, id: NodeId, block: &'ast Block) {
self.with_resolved_label(label, id, |this| this.visit_block(block));
}

fn resolve_block(&mut self, block: &Block) {
fn resolve_block(&mut self, block: &'ast Block) {
debug!("(resolving block) entering block");
// Move down in the graph, if there's an anonymous module rooted here.
let orig_module = self.parent_scope.module;
Expand Down Expand Up @@ -1885,7 +1897,7 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> {
debug!("(resolving block) leaving block");
}

fn resolve_expr(&mut self, expr: &Expr, parent: Option<&Expr>) {
fn resolve_expr(&mut self, expr: &'ast Expr, parent: Option<&'ast Expr>) {
// First, record candidate traits for this expression if it could
// result in the invocation of a method call.

Expand Down Expand Up @@ -2023,7 +2035,7 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> {
}
}

fn record_candidate_traits_for_expr_if_necessary(&mut self, expr: &Expr) {
fn record_candidate_traits_for_expr_if_necessary(&mut self, expr: &'ast Expr) {
match expr.kind {
ExprKind::Field(_, ident) => {
// FIXME(#6890): Even though you can't treat a method like a
Expand Down
Loading

0 comments on commit 320ada6

Please sign in to comment.