Rollup merge of #72923 - Patryk27:fix/52468, r=estebank

Improve E0433, so that it suggests missing imports

Closes #52468
This commit is contained in:
Dylan DPC 2020-06-04 12:12:47 +02:00 committed by GitHub
commit 085c16d552
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 198 additions and 47 deletions

View File

@ -1475,7 +1475,7 @@ crate fn show_candidates(
// This is `None` if all placement locations are inside expansions
use_placement_span: Option<Span>,
candidates: &[ImportSuggestion],
better: bool,
instead: bool,
found_use: bool,
) {
if candidates.is_empty() {
@ -1486,6 +1486,7 @@ crate fn show_candidates(
// by iterating through a hash map, so make sure they are ordered:
let mut path_strings: Vec<_> =
candidates.iter().map(|c| path_names_to_string(&c.path)).collect();
path_strings.sort();
path_strings.dedup();
@ -1494,8 +1495,9 @@ crate fn show_candidates(
} else {
("one of these", "items")
};
let instead = if better { " instead" } else { "" };
let msg = format!("consider importing {} {}{}", determiner, kind, instead);
let instead = if instead { " instead" } else { "" };
let mut msg = format!("consider importing {} {}{}", determiner, kind, instead);
if let Some(span) = use_placement_span {
for candidate in &mut path_strings {
@ -1507,12 +1509,13 @@ crate fn show_candidates(
err.span_suggestions(span, &msg, path_strings.into_iter(), Applicability::Unspecified);
} else {
let mut msg = msg;
msg.push(':');
for candidate in path_strings {
msg.push('\n');
msg.push_str(&candidate);
}
err.note(&msg);
}
}

View File

@ -29,8 +29,9 @@ use rustc_span::Span;
use smallvec::{smallvec, SmallVec};
use log::debug;
use rustc_span::source_map::{respan, Spanned};
use std::collections::BTreeSet;
use std::mem::replace;
use std::mem::{replace, take};
mod diagnostics;
crate mod lifetimes;
@ -234,6 +235,13 @@ impl<'a> PathSource<'a> {
}
}
fn is_call(self) -> bool {
match self {
PathSource::Expr(Some(&Expr { kind: ExprKind::Call(..), .. })) => true,
_ => false,
}
}
crate fn is_expected(self, res: Res) -> bool {
match self {
PathSource::Type => match res {
@ -1620,14 +1628,83 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
let report_errors = |this: &mut Self, res: Option<Res>| {
let (err, candidates) = this.smart_resolve_report_errors(path, span, source, res);
let def_id = this.parent_scope.module.normal_ancestor_id;
let better = res.is_some();
let instead = res.is_some();
let suggestion =
if res.is_none() { this.report_missing_type_error(path) } else { None };
this.r.use_injections.push(UseError { err, candidates, def_id, better, suggestion });
this.r.use_injections.push(UseError { err, candidates, def_id, instead, suggestion });
PartialRes::new(Res::Err)
};
// For paths originating from calls (like in `HashMap::new()`), tries
// to enrich the plain `failed to resolve: ...` message with hints
// about possible missing imports.
//
// Similar thing, for types, happens in `report_errors` above.
let report_errors_for_call = |this: &mut Self, parent_err: Spanned<ResolutionError<'a>>| {
if !source.is_call() {
return Some(parent_err);
}
// Before we start looking for candidates, we have to get our hands
// on the type user is trying to perform invocation on; basically:
// we're transforming `HashMap::new` into just `HashMap`
let path = if let Some((_, path)) = path.split_last() {
path
} else {
return Some(parent_err);
};
let (mut err, candidates) =
this.smart_resolve_report_errors(path, span, PathSource::Type, None);
if candidates.is_empty() {
err.cancel();
return Some(parent_err);
}
// There are two different error messages user might receive at
// this point:
// - E0412 cannot find type `{}` in this scope
// - E0433 failed to resolve: use of undeclared type or module `{}`
//
// The first one is emitted for paths in type-position, and the
// latter one - for paths in expression-position.
//
// Thus (since we're in expression-position at this point), not to
// confuse the user, we want to keep the *message* from E0432 (so
// `parent_err`), but we want *hints* from E0412 (so `err`).
//
// And that's what happens below - we're just mixing both messages
// into a single one.
let mut parent_err = this.r.into_struct_error(parent_err.span, parent_err.node);
parent_err.cancel();
err.message = take(&mut parent_err.message);
err.code = take(&mut parent_err.code);
err.children = take(&mut parent_err.children);
drop(parent_err);
let def_id = this.parent_scope.module.normal_ancestor_id;
this.r.use_injections.push(UseError {
err,
candidates,
def_id,
instead: false,
suggestion: None,
});
// We don't return `Some(parent_err)` here, because the error will
// be already printed as part of the `use` injections
None
};
let partial_res = match self.resolve_qpath_anywhere(
id,
qself,
@ -1637,14 +1714,15 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
source.defer_to_typeck(),
crate_lint,
) {
Some(partial_res) if partial_res.unresolved_segments() == 0 => {
Ok(Some(partial_res)) if partial_res.unresolved_segments() == 0 => {
if is_expected(partial_res.base_res()) || partial_res.base_res() == Res::Err {
partial_res
} else {
report_errors(self, Some(partial_res.base_res()))
}
}
Some(partial_res) if source.defer_to_typeck() => {
Ok(Some(partial_res)) if source.defer_to_typeck() => {
// Not fully resolved associated item `T::A::B` or `<T as Tr>::A::B`
// or `<T>::A::B`. If `B` should be resolved in value namespace then
// it needs to be added to the trait map.
@ -1655,25 +1733,34 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
}
let mut std_path = vec![Segment::from_ident(Ident::with_dummy_span(sym::std))];
std_path.extend(path);
if self.r.primitive_type_table.primitive_types.contains_key(&path[0].ident.name) {
let cl = CrateLint::No;
let ns = Some(ns);
if let PathResult::Module(_) | PathResult::NonModule(_) =
self.resolve_path(&std_path, ns, false, span, cl)
self.resolve_path(&std_path, Some(ns), false, span, CrateLint::No)
{
// check if we wrote `str::from_utf8` instead of `std::str::from_utf8`
// Check if we wrote `str::from_utf8` instead of `std::str::from_utf8`
let item_span =
path.iter().last().map(|segment| segment.ident.span).unwrap_or(span);
debug!("accessed item from `std` submodule as a bare type {:?}", std_path);
let mut hm = self.r.session.confused_type_with_std_module.borrow_mut();
hm.insert(item_span, span);
// In some places (E0223) we only have access to the full path
hm.insert(span, span);
}
}
partial_res
}
Err(err) => {
if let Some(err) = report_errors_for_call(self, err) {
self.r.report_error(err.span, err.node);
}
PartialRes::new(Res::Err)
}
_ => report_errors(self, None),
};
@ -1682,6 +1769,7 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
// Avoid recording definition of `A::B` in `<T as A>::B::C`.
self.r.record_partial_res(id, partial_res);
}
partial_res
}
@ -1711,17 +1799,16 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
span: Span,
defer_to_typeck: bool,
crate_lint: CrateLint,
) -> Option<PartialRes> {
) -> Result<Option<PartialRes>, Spanned<ResolutionError<'a>>> {
let mut fin_res = None;
for (i, ns) in [primary_ns, TypeNS, ValueNS].iter().cloned().enumerate() {
if i == 0 || ns != primary_ns {
match self.resolve_qpath(id, qself, path, ns, span, crate_lint) {
// If defer_to_typeck, then resolution > no resolution,
// otherwise full resolution > partial resolution > no resolution.
match self.resolve_qpath(id, qself, path, ns, span, crate_lint)? {
Some(partial_res)
if partial_res.unresolved_segments() == 0 || defer_to_typeck =>
{
return Some(partial_res);
return Ok(Some(partial_res));
}
partial_res => {
if fin_res.is_none() {
@ -1732,19 +1819,19 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
}
}
// `MacroNS`
assert!(primary_ns != MacroNS);
if qself.is_none() {
let path_seg = |seg: &Segment| PathSegment::from_ident(seg.ident);
let path = Path { segments: path.iter().map(path_seg).collect(), span };
if let Ok((_, res)) =
self.r.resolve_macro_path(&path, None, &self.parent_scope, false, false)
{
return Some(PartialRes::new(res));
return Ok(Some(PartialRes::new(res)));
}
}
fin_res
Ok(fin_res)
}
/// Handles paths that may refer to associated items.
@ -1756,7 +1843,7 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
ns: Namespace,
span: Span,
crate_lint: CrateLint,
) -> Option<PartialRes> {
) -> Result<Option<PartialRes>, Spanned<ResolutionError<'a>>> {
debug!(
"resolve_qpath(id={:?}, qself={:?}, path={:?}, ns={:?}, span={:?})",
id, qself, path, ns, span,
@ -1767,10 +1854,10 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
// This is a case like `<T>::B`, where there is no
// trait to resolve. In that case, we leave the `B`
// segment to be resolved by type-check.
return Some(PartialRes::with_unresolved_segments(
return Ok(Some(PartialRes::with_unresolved_segments(
Res::Def(DefKind::Mod, DefId::local(CRATE_DEF_INDEX)),
path.len(),
));
)));
}
// Make sure `A::B` in `<T as A::B>::C` is a trait item.
@ -1800,10 +1887,10 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
// The remaining segments (the `C` in our example) will
// have to be resolved by type-check, since that requires doing
// trait resolution.
return Some(PartialRes::with_unresolved_segments(
return Ok(Some(PartialRes::with_unresolved_segments(
partial_res.base_res(),
partial_res.unresolved_segments() + path.len() - qself.position - 1,
));
)));
}
let result = match self.resolve_path(&path, Some(ns), true, span, crate_lint) {
@ -1838,11 +1925,10 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
PartialRes::new(module.res().unwrap())
}
PathResult::Failed { is_error_from_last_segment: false, span, label, suggestion } => {
self.r.report_error(span, ResolutionError::FailedToResolve { label, suggestion });
PartialRes::new(Res::Err)
return Err(respan(span, ResolutionError::FailedToResolve { label, suggestion }));
}
PathResult::Module(..) | PathResult::Failed { .. } => return None,
PathResult::Indeterminate => bug!("indetermined path result in resolve_qpath"),
PathResult::Module(..) | PathResult::Failed { .. } => return Ok(None),
PathResult::Indeterminate => bug!("indeterminate path result in resolve_qpath"),
};
if path.len() > 1
@ -1862,7 +1948,7 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
PathResult::Module(ModuleOrUniformRoot::Module(module)) => {
module.res().unwrap()
}
_ => return Some(result),
_ => return Ok(Some(result)),
}
};
if result.base_res() == unqualified_result {
@ -1871,7 +1957,7 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
}
}
Some(result)
Ok(Some(result))
}
fn with_resolved_label(&mut self, label: Option<Label>, id: NodeId, f: impl FnOnce(&mut Self)) {

View File

@ -618,13 +618,13 @@ struct PrivacyError<'a> {
struct UseError<'a> {
err: DiagnosticBuilder<'a>,
/// Attach `use` statements for these candidates.
/// Candidates which user could `use` to access the missing type.
candidates: Vec<ImportSuggestion>,
/// The `NodeId` of the module to place the use-statements in.
/// The `DefId` of the module to place the use-statements in.
def_id: DefId,
/// Whether the diagnostic should state that it's "better".
better: bool,
/// Extra free form suggestion. Currently used to suggest new type parameter.
/// Whether the diagnostic should say "instead" (as in `consider importing ... instead`).
instead: bool,
/// Extra free-form suggestion.
suggestion: Option<(Span, &'static str, String, Applicability)>,
}
@ -2577,12 +2577,12 @@ impl<'a> Resolver<'a> {
}
fn report_with_use_injections(&mut self, krate: &Crate) {
for UseError { mut err, candidates, def_id, better, suggestion } in
for UseError { mut err, candidates, def_id, instead, suggestion } in
self.use_injections.drain(..)
{
let (span, found_use) = UsePlacementFinder::check(&self.definitions, krate, def_id);
if !candidates.is_empty() {
diagnostics::show_candidates(&mut err, span, &candidates, better, found_use);
diagnostics::show_candidates(&mut err, span, &candidates, instead, found_use);
} else if let Some((span, msg, sugg, appl)) = suggestion {
err.span_suggestion(span, msg, sugg, appl);
}

View File

@ -2,7 +2,14 @@ error[E0433]: failed to resolve: use of undeclared type or module `HashMap`
--> $DIR/issue-31997-1.rs:20:19
|
LL | let mut map = HashMap::new();
| ^^^^^^^ use of undeclared type or module `HashMap`
| ^^^^^^^ not found in this scope
|
help: consider importing one of these items
|
LL | use std::collections::HashMap;
|
LL | use std::collections::hash_map::HashMap;
|
error: aborting due to previous error

View File

@ -1,3 +1,3 @@
fn main () {
let map = HashMap::new(); //~ ERROR E0433
let map = NonExistingMap::new(); //~ ERROR E0433
}

View File

@ -1,8 +1,8 @@
error[E0433]: failed to resolve: use of undeclared type or module `HashMap`
error[E0433]: failed to resolve: use of undeclared type or module `NonExistingMap`
--> $DIR/E0433.rs:2:15
|
LL | let map = HashMap::new();
| ^^^^^^^ use of undeclared type or module `HashMap`
LL | let map = NonExistingMap::new();
| ^^^^^^^^^^^^^^ use of undeclared type or module `NonExistingMap`
error: aborting due to previous error

View File

@ -13,9 +13,15 @@ LL | fn f() { ::bar::m!(); }
| ------------ in this macro invocation
...
LL | Vec::new();
| ^^^ use of undeclared type or module `Vec`
| ^^^ not found in this scope
|
= note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
help: consider importing one of these items
|
LL | use std::prelude::v1::Vec;
|
LL | use std::vec::Vec;
|
error[E0599]: no method named `clone` found for unit type `()` in the current scope
--> $DIR/no_implicit_prelude.rs:12:12

View File

@ -0,0 +1,7 @@
fn main() {
let x1 = HashMap::new(); //~ ERROR failed to resolve
let x2 = GooMap::new(); //~ ERROR failed to resolve
let y1: HashMap; //~ ERROR cannot find type
let y2: GooMap; //~ ERROR cannot find type
}

View File

@ -0,0 +1,42 @@
error[E0433]: failed to resolve: use of undeclared type or module `GooMap`
--> $DIR/use_suggestion.rs:3:14
|
LL | let x2 = GooMap::new();
| ^^^^^^ use of undeclared type or module `GooMap`
error[E0433]: failed to resolve: use of undeclared type or module `HashMap`
--> $DIR/use_suggestion.rs:2:14
|
LL | let x1 = HashMap::new();
| ^^^^^^^ not found in this scope
|
help: consider importing one of these items
|
LL | use std::collections::HashMap;
|
LL | use std::collections::hash_map::HashMap;
|
error[E0412]: cannot find type `HashMap` in this scope
--> $DIR/use_suggestion.rs:5:13
|
LL | let y1: HashMap;
| ^^^^^^^ not found in this scope
|
help: consider importing one of these items
|
LL | use std::collections::HashMap;
|
LL | use std::collections::hash_map::HashMap;
|
error[E0412]: cannot find type `GooMap` in this scope
--> $DIR/use_suggestion.rs:6:13
|
LL | let y2: GooMap;
| ^^^^^^ not found in this scope
error: aborting due to 4 previous errors
Some errors have detailed explanations: E0412, E0433.
For more information about an error, try `rustc --explain E0412`.