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
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
@@ -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>,

@@ -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.
@@ -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 {
@@ -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,
@@ -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) {
@@ -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);
}

@@ -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(),
}
}

@@ -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
}

@@ -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
}

@@ -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);
@@ -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);
}
}

@@ -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
@@ -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");
}
}
47 changes: 38 additions & 9 deletions src/librustc_resolve/late/diagnostics.rs
Original file line number Diff line number Diff line change
@@ -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 {
@@ -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());
@@ -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());
@@ -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)
}
@@ -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() {
@@ -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);
}
@@ -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.
2 changes: 1 addition & 1 deletion src/libsyntax/parse/parser/stmt.rs
Original file line number Diff line number Diff line change
@@ -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();
1 change: 1 addition & 0 deletions src/test/ui/privacy/privacy-ns2.rs
Original file line number Diff line number Diff line change
@@ -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() {
29 changes: 24 additions & 5 deletions src/test/ui/privacy/privacy-ns2.stderr
Original file line number Diff line number Diff line change
@@ -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>;
| ^^^
@@ -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);
}
28 changes: 28 additions & 0 deletions src/test/ui/suggestions/let-binding-init-expr-as-ty.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
error[E0573]: expected type, found local variable `num`
--> $DIR/let-binding-init-expr-as-ty.rs:2:27
|
LL | let foo: i32::from_be(num);
| -- ^^^ not a type
| |
| help: use `=` if you meant to assign

error: parenthesized type parameters may only be used with a `Fn` trait
--> $DIR/let-binding-init-expr-as-ty.rs:2:19
|
LL | let foo: i32::from_be(num);
| ^^^^^^^^^^^^
|
= note: `#[deny(parenthesized_params_in_types_and_modules)]` on by default
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #42238 <https://github.com/rust-lang/rust/issues/42238>

error[E0223]: ambiguous associated type
--> $DIR/let-binding-init-expr-as-ty.rs:2:14
|
LL | let foo: i32::from_be(num);
| ^^^^^^^^^^^^^^^^^ help: use fully-qualified syntax: `<i32 as Trait>::from_be`

error: aborting due to 3 previous errors

Some errors have detailed explanations: E0223, E0573.
For more information about an error, try `rustc --explain E0223`.