Skip to content

Use heuristics to suggest assignment #65566

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

Merged
merged 5 commits into from
Oct 27, 2019
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
94 changes: 58 additions & 36 deletions src/librustc_resolve/late.rs
Original file line number Diff line number Diff line change
Expand Up @@ -316,22 +316,8 @@ impl<'a> PathSource<'a> {
}
}

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

/// The module that represents the current item scope.
parent_scope: ParentScope<'a>,

/// The current set of local scopes for types and values.
/// FIXME #4948: Reuse ribs to avoid allocation.
ribs: PerNS<Vec<Rib<'a>>>,

/// The current set of local scopes, for labels.
label_ribs: Vec<Rib<'a, NodeId>>,

/// The trait that the current context can refer to.
current_trait_ref: Option<(Module<'a>, TraitRef)>,

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

Expand All @@ -350,6 +336,29 @@ struct LateResolutionVisitor<'a, 'b> {

/// Only used for better errors on `fn(): fn()`.
current_type_ascription: Vec<Span>,

/// Only used for better errors on `let <pat>: <expr, not type>;`.
current_let_binding: Option<(Span, Option<Span>, Option<Span>)>,
}

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

/// The module that represents the current item scope.
parent_scope: ParentScope<'a>,

/// The current set of local scopes for types and values.
/// FIXME #4948: Reuse ribs to avoid allocation.
ribs: PerNS<Vec<Rib<'a>>>,

/// The current set of local scopes, for labels.
label_ribs: Vec<Rib<'a, NodeId>>,

/// The trait that the current context can refer to.
current_trait_ref: Option<(Module<'a>, TraitRef)>,

/// Fields used to add information to diagnostic errors.
diagnostic_metadata: DiagnosticMetadata,
}

/// Walks the whole crate in DFS order, visiting each item, resolving names as it goes.
Expand All @@ -373,7 +382,18 @@ impl<'a, 'tcx> Visitor<'tcx> for LateResolutionVisitor<'a, '_> {
self.resolve_expr(expr, None);
}
fn visit_local(&mut self, local: &'tcx Local) {
let local_spans = match local.pat.kind {
// We check for this to avoid tuple struct fields.
PatKind::Wild => None,
_ => Some((
local.pat.span,
local.ty.as_ref().map(|ty| ty.span),
local.init.as_ref().map(|init| init.span),
)),
};
let original = replace(&mut self.diagnostic_metadata.current_let_binding, local_spans);
self.resolve_local(local);
self.diagnostic_metadata.current_let_binding = original;
}
fn visit_ty(&mut self, ty: &'tcx Ty) {
match ty.kind {
Expand Down Expand Up @@ -415,7 +435,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LateResolutionVisitor<'a, '_> {
}
}
fn visit_fn(&mut self, fn_kind: FnKind<'tcx>, declaration: &'tcx FnDecl, sp: Span, _: NodeId) {
let previous_value = replace(&mut self.current_function, Some(sp));
let previous_value = replace(&mut self.diagnostic_metadata.current_function, Some(sp));
debug!("(resolving function) entering function");
let rib_kind = match fn_kind {
FnKind::ItemFn(..) => FnItemRibKind,
Expand All @@ -441,7 +461,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LateResolutionVisitor<'a, '_> {
debug!("(resolving function) leaving function");
})
});
self.current_function = previous_value;
self.diagnostic_metadata.current_function = previous_value;
}

fn visit_generics(&mut self, generics: &'tcx Generics) {
Expand Down Expand Up @@ -475,7 +495,8 @@ impl<'a, 'tcx> Visitor<'tcx> for LateResolutionVisitor<'a, '_> {
// (We however cannot ban `Self` for defaults on *all* generic
// lists; e.g. trait generics can usefully refer to `Self`,
// such as in the case of `trait Add<Rhs = Self>`.)
if self.current_self_item.is_some() { // (`Some` if + only if we are in ADT's generics.)
if self.diagnostic_metadata.current_self_item.is_some() {
// (`Some` if + only if we are in ADT's generics.)
default_ban_rib.bindings.insert(Ident::with_dummy_span(kw::SelfUpper), Res::Err);
}

Expand Down Expand Up @@ -527,12 +548,7 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> {
},
label_ribs: Vec::new(),
current_trait_ref: None,
current_trait_assoc_types: Vec::new(),
current_self_type: None,
current_self_item: None,
current_function: None,
unused_labels: Default::default(),
current_type_ascription: Vec::new(),
diagnostic_metadata: DiagnosticMetadata::default(),
}
}

Expand Down Expand Up @@ -892,16 +908,22 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> {

fn with_current_self_type<T>(&mut self, self_type: &Ty, f: impl FnOnce(&mut Self) -> T) -> T {
// Handle nested impls (inside fn bodies)
let previous_value = replace(&mut self.current_self_type, Some(self_type.clone()));
let previous_value = replace(
&mut self.diagnostic_metadata.current_self_type,
Some(self_type.clone()),
);
let result = f(self);
self.current_self_type = previous_value;
self.diagnostic_metadata.current_self_type = previous_value;
result
}

fn with_current_self_item<T>(&mut self, self_item: &Item, f: impl FnOnce(&mut Self) -> T) -> T {
let previous_value = replace(&mut self.current_self_item, Some(self_item.id));
let previous_value = replace(
&mut self.diagnostic_metadata.current_self_item,
Some(self_item.id),
);
let result = f(self);
self.current_self_item = previous_value;
self.diagnostic_metadata.current_self_item = previous_value;
result
}

Expand All @@ -912,14 +934,14 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> {
f: impl FnOnce(&mut Self) -> T,
) -> T {
let trait_assoc_types = replace(
&mut self.current_trait_assoc_types,
&mut self.diagnostic_metadata.current_trait_assoc_types,
trait_items.iter().filter_map(|item| match &item.kind {
TraitItemKind::Type(bounds, _) if bounds.len() == 0 => Some(item.ident),
_ => None,
}).collect(),
);
let result = f(self);
self.current_trait_assoc_types = trait_assoc_types;
self.diagnostic_metadata.current_trait_assoc_types = trait_assoc_types;
result
}

Expand Down Expand Up @@ -1746,7 +1768,7 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> {

fn with_resolved_label(&mut self, label: Option<Label>, id: NodeId, f: impl FnOnce(&mut Self)) {
if let Some(label) = label {
self.unused_labels.insert(id, label.ident.span);
self.diagnostic_metadata.unused_labels.insert(id, label.ident.span);
self.with_label_rib(NormalRibKind, |this| {
let ident = label.ident.modern_and_legacy();
this.label_ribs.last_mut().unwrap().bindings.insert(ident, id);
Expand Down Expand Up @@ -1850,7 +1872,7 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> {
Some(node_id) => {
// Since this res is a label, it is never read.
self.r.label_res_map.insert(expr.id, node_id);
self.unused_labels.remove(&node_id);
self.diagnostic_metadata.unused_labels.remove(&node_id);
}
}

Expand Down Expand Up @@ -1912,9 +1934,9 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> {
}
}
ExprKind::Type(ref type_expr, _) => {
self.current_type_ascription.push(type_expr.span);
self.diagnostic_metadata.current_type_ascription.push(type_expr.span);
visit::walk_expr(self, expr);
self.current_type_ascription.pop();
self.diagnostic_metadata.current_type_ascription.pop();
}
// `async |x| ...` gets desugared to `|x| future_from_generator(|| ...)`, so we need to
// resolve the arguments within the proper scopes so that usages of them inside the
Expand Down Expand Up @@ -2073,7 +2095,7 @@ impl<'a> Resolver<'a> {
pub(crate) fn late_resolve_crate(&mut self, krate: &Crate) {
let mut late_resolution_visitor = LateResolutionVisitor::new(self);
visit::walk_crate(&mut late_resolution_visitor, krate);
for (id, span) in late_resolution_visitor.unused_labels.iter() {
for (id, span) in late_resolution_visitor.diagnostic_metadata.unused_labels.iter() {
self.session.buffer_lint(lint::builtin::UNUSED_LABELS, *id, *span, "unused label");
}
}
Expand Down
47 changes: 38 additions & 9 deletions src/librustc_resolve/late/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,26 @@ impl<'a> LateResolutionVisitor<'a, '_> {
let path_str = Segment::names_to_string(path);
let item_str = path.last().unwrap().ident;
let code = source.error_code(res.is_some());
let (base_msg, fallback_label, base_span) = if let Some(res) = res {
let (base_msg, fallback_label, base_span, could_be_expr) = if let Some(res) = res {
(format!("expected {}, found {} `{}`", expected, res.descr(), path_str),
format!("not a {}", expected),
span)
span,
match res {
Res::Def(DefKind::Fn, _) => {
// Verify whether this is a fn call or an Fn used as a type.
self.r.session.source_map().span_to_snippet(span).map(|snippet| {
snippet.ends_with(')')
}).unwrap_or(false)
}
Res::Def(DefKind::Ctor(..), _) |
Res::Def(DefKind::Method, _) |
Res::Def(DefKind::Const, _) |
Res::Def(DefKind::AssocConst, _) |
Res::SelfCtor(_) |
Res::PrimTy(_) |
Res::Local(_) => true,
_ => false,
})
} else {
let item_span = path.last().unwrap().ident.span;
let (mod_prefix, mod_str) = if path.len() == 1 {
Expand All @@ -94,7 +110,8 @@ impl<'a> LateResolutionVisitor<'a, '_> {
};
(format!("cannot find {} `{}` in {}{}", expected, item_str, mod_prefix, mod_str),
format!("not found in {}", mod_str),
item_span)
item_span,
false)
};

let code = DiagnosticId::Error(code.into());
Expand Down Expand Up @@ -134,7 +151,7 @@ impl<'a> LateResolutionVisitor<'a, '_> {
"`self` value is a keyword only available in methods with a `self` parameter",
),
});
if let Some(span) = &self.current_function {
if let Some(span) = &self.diagnostic_metadata.current_function {
err.span_label(*span, "this function doesn't have a `self` parameter");
}
return (err, Vec::new());
Expand Down Expand Up @@ -257,6 +274,18 @@ impl<'a> LateResolutionVisitor<'a, '_> {
if !levenshtein_worked {
err.span_label(base_span, fallback_label);
self.type_ascription_suggestion(&mut err, base_span);
match self.diagnostic_metadata.current_let_binding {
Some((pat_sp, Some(ty_sp), None))
if ty_sp.contains(base_span) && could_be_expr => {
err.span_suggestion_short(
pat_sp.between(ty_sp),
"use `=` if you meant to assign",
" = ".to_string(),
Applicability::MaybeIncorrect,
);
}
_ => {}
}
}
(err, candidates)
}
Expand Down Expand Up @@ -491,7 +520,9 @@ impl<'a> LateResolutionVisitor<'a, '_> {

// Fields are generally expected in the same contexts as locals.
if filter_fn(Res::Local(ast::DUMMY_NODE_ID)) {
if let Some(node_id) = self.current_self_type.as_ref().and_then(extract_node_id) {
if let Some(node_id) = self.diagnostic_metadata.current_self_type.as_ref()
.and_then(extract_node_id)
{
// Look for a field with the same name in the current self_type.
if let Some(resolution) = self.r.partial_res_map.get(&node_id) {
match resolution.base_res() {
Expand All @@ -510,7 +541,7 @@ impl<'a> LateResolutionVisitor<'a, '_> {
}
}

for assoc_type_ident in &self.current_trait_assoc_types {
for assoc_type_ident in &self.diagnostic_metadata.current_trait_assoc_types {
if *assoc_type_ident == ident {
return Some(AssocSuggestion::AssocItem);
}
Expand Down Expand Up @@ -644,11 +675,9 @@ impl<'a> LateResolutionVisitor<'a, '_> {
err: &mut DiagnosticBuilder<'_>,
base_span: Span,
) {
debug!("type_ascription_suggetion {:?}", base_span);
let cm = self.r.session.source_map();
let base_snippet = cm.span_to_snippet(base_span);
debug!("self.current_type_ascription {:?}", self.current_type_ascription);
if let Some(sp) = self.current_type_ascription.last() {
if let Some(sp) = self.diagnostic_metadata.current_type_ascription.last() {
let mut sp = *sp;
loop {
// Try to find the `:`; bail on first non-':' / non-whitespace.
Expand Down
2 changes: 1 addition & 1 deletion src/libsyntax/parse/parser/stmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ impl<'a> Parser<'a> {
err.span_suggestion_short(
colon_sp,
"use `=` if you meant to assign",
"=".to_string(),
" =".to_string(),
Applicability::MachineApplicable
);
err.emit();
Expand Down
1 change: 1 addition & 0 deletions src/test/ui/privacy/privacy-ns2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ fn test_single2() {
use foo2::Bar;

let _x : Box<Bar>; //~ ERROR expected type, found function `Bar`
let _x : Bar(); //~ ERROR expected type, found function `Bar`
}

fn test_list2() {
Expand Down
29 changes: 24 additions & 5 deletions src/test/ui/privacy/privacy-ns2.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,26 @@ LL | use foo3::Bar;
|

error[E0573]: expected type, found function `Bar`
--> $DIR/privacy-ns2.rs:47:17
--> $DIR/privacy-ns2.rs:42:14
|
LL | let _x : Bar();
| ^^^^^ not a type
|
help: use `=` if you meant to assign
|
LL | let _x = Bar();
| ^
help: possible better candidates are found in other modules, you can import them into scope
|
LL | use foo1::Bar;
|
LL | use foo2::Bar;
|
LL | use foo3::Bar;
|

error[E0573]: expected type, found function `Bar`
--> $DIR/privacy-ns2.rs:48:17
|
LL | let _x: Box<Bar>;
| ^^^
Expand All @@ -67,24 +86,24 @@ LL | use foo3::Bar;
|

error[E0603]: trait `Bar` is private
--> $DIR/privacy-ns2.rs:60:15
--> $DIR/privacy-ns2.rs:61:15
|
LL | use foo3::Bar;
| ^^^

error[E0603]: trait `Bar` is private
--> $DIR/privacy-ns2.rs:64:15
--> $DIR/privacy-ns2.rs:65:15
|
LL | use foo3::Bar;
| ^^^

error[E0603]: trait `Bar` is private
--> $DIR/privacy-ns2.rs:71:16
--> $DIR/privacy-ns2.rs:72:16
|
LL | use foo3::{Bar,Baz};
| ^^^

error: aborting due to 7 previous errors
error: aborting due to 8 previous errors

Some errors have detailed explanations: E0423, E0573, E0603.
For more information about an error, try `rustc --explain E0423`.
12 changes: 12 additions & 0 deletions src/test/ui/suggestions/let-binding-init-expr-as-ty.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
pub fn foo(num: i32) -> i32 {
let foo: i32::from_be(num);
//~^ ERROR expected type, found local variable `num`
//~| ERROR parenthesized type parameters may only be used with a `Fn` trait
//~| ERROR ambiguous associated type
//~| WARNING this was previously accepted by the compiler but is being phased out
foo
}

fn main() {
let _ = foo(42);
}
Loading