Skip to content

Commit

Permalink
Rollup merge of rust-lang#64054 - estebank:unused-import-is-to-eager,…
Browse files Browse the repository at this point in the history
… r=petrochenkov

Always emit unresolved import errors and hide unused import lint

Fix rust-lang#63724.

r? @petrochenkov
  • Loading branch information
Centril authored Sep 9, 2019
2 parents 611458b + 9a56187 commit 8926301
Show file tree
Hide file tree
Showing 7 changed files with 96 additions and 45 deletions.
89 changes: 55 additions & 34 deletions src/librustc_resolve/resolve_imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ pub enum ImportDirectiveSubclass<'a> {
}

/// One import directive.
#[derive(Debug,Clone)]
#[derive(Debug, Clone)]
crate struct ImportDirective<'a> {
/// The ID of the `extern crate`, `UseTree` etc that imported this `ImportDirective`.
///
Expand Down Expand Up @@ -447,12 +447,13 @@ impl<'a> Resolver<'a> {
}

// Define the name or return the existing binding if there is a collision.
pub fn try_define(&mut self,
module: Module<'a>,
ident: Ident,
ns: Namespace,
binding: &'a NameBinding<'a>)
-> Result<(), &'a NameBinding<'a>> {
pub fn try_define(
&mut self,
module: Module<'a>,
ident: Ident,
ns: Namespace,
binding: &'a NameBinding<'a>,
) -> Result<(), &'a NameBinding<'a>> {
let res = binding.res();
self.check_reserved_macro_name(ident, res);
self.set_binding_parent_module(binding, module);
Expand Down Expand Up @@ -480,8 +481,11 @@ impl<'a> Resolver<'a> {
};
if glob_binding.res() != nonglob_binding.res() &&
ns == MacroNS && nonglob_binding.expansion != ExpnId::root() {
resolution.binding = Some(this.ambiguity(AmbiguityKind::GlobVsExpanded,
nonglob_binding, glob_binding));
resolution.binding = Some(this.ambiguity(
AmbiguityKind::GlobVsExpanded,
nonglob_binding,
glob_binding,
));
} else {
resolution.binding = Some(nonglob_binding);
}
Expand Down Expand Up @@ -513,9 +517,11 @@ impl<'a> Resolver<'a> {
})
}

fn ambiguity(&self, kind: AmbiguityKind,
primary_binding: &'a NameBinding<'a>, secondary_binding: &'a NameBinding<'a>)
-> &'a NameBinding<'a> {
fn ambiguity(
&self, kind: AmbiguityKind,
primary_binding: &'a NameBinding<'a>,
secondary_binding: &'a NameBinding<'a>,
) -> &'a NameBinding<'a> {
self.arenas.alloc_name_binding(NameBinding {
ambiguity: Some((secondary_binding, kind)),
..primary_binding.clone()
Expand All @@ -524,8 +530,12 @@ impl<'a> Resolver<'a> {

// Use `f` to mutate the resolution of the name in the module.
// If the resolution becomes a success, define it in the module's glob importers.
fn update_resolution<T, F>(&mut self, module: Module<'a>, ident: Ident, ns: Namespace, f: F)
-> T
fn update_resolution<T, F>(
&mut self, module: Module<'a>,
ident: Ident,
ns: Namespace,
f: F,
) -> T
where F: FnOnce(&mut Resolver<'a>, &mut NameResolution<'a>) -> T
{
// Ensure that `resolution` isn't borrowed when defining in the module's glob importers,
Expand Down Expand Up @@ -627,14 +637,18 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
self.finalize_resolutions_in(module);
}

let mut has_errors = false;
let mut seen_spans = FxHashSet::default();
let mut errors = vec![];
let mut prev_root_id: NodeId = NodeId::from_u32(0);
for i in 0 .. self.r.determined_imports.len() {
let import = self.r.determined_imports[i];
let determined_imports = mem::take(&mut self.r.determined_imports);
let indeterminate_imports = mem::take(&mut self.r.indeterminate_imports);

for (is_indeterminate, import) in determined_imports
.into_iter()
.map(|i| (false, i))
.chain(indeterminate_imports.into_iter().map(|i| (true, i)))
{
if let Some(err) = self.finalize_import(import) {
has_errors = true;

if let SingleImport { source, ref source_bindings, .. } = import.subclass {
if source.name == kw::SelfLower {
Expand Down Expand Up @@ -666,25 +680,27 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
errors.push((path, err));
prev_root_id = import.root_id;
}
} else if is_indeterminate {
// Consider erroneous imports used to avoid duplicate diagnostics.
self.r.used_imports.insert((import.id, TypeNS));
let path = import_path_to_string(
&import.module_path.iter().map(|seg| seg.ident).collect::<Vec<_>>(),
&import.subclass,
import.span,
);
let err = UnresolvedImportError {
span: import.span,
label: None,
note: Vec::new(),
suggestion: None,
};
errors.push((path, err));
}
}

if !errors.is_empty() {
self.throw_unresolved_import_error(errors.clone(), None);
}

for import in &self.r.indeterminate_imports {
// Consider erroneous imports used to avoid duplicate diagnostics.
self.r.used_imports.insert((import.id, TypeNS));
}
// Report unresolved imports only if no hard error was already reported
// to avoid generating multiple errors on the same import.
if !has_errors {
for import in &self.r.indeterminate_imports {
self.throw_unresolved_import_error(errors, Some(MultiSpan::from(import.span)));
break;
}
}
}

fn throw_unresolved_import_error(
Expand Down Expand Up @@ -839,8 +855,14 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
) -> Option<UnresolvedImportError> {
let orig_vis = directive.vis.replace(ty::Visibility::Invisible);
let prev_ambiguity_errors_len = self.r.ambiguity_errors.len();
let path_res = self.r.resolve_path(&directive.module_path, None, &directive.parent_scope,
true, directive.span, directive.crate_lint());
let path_res = self.r.resolve_path(
&directive.module_path,
None,
&directive.parent_scope,
true,
directive.span,
directive.crate_lint(),
);
let no_ambiguity = self.r.ambiguity_errors.len() == prev_ambiguity_errors_len;
directive.vis.set(orig_vis);
if let PathResult::Failed { .. } | PathResult::NonModule(..) = path_res {
Expand Down Expand Up @@ -903,7 +925,6 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
}
}
};

return Some(err);
}
return None;
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/extenv/issue-55897.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use prelude::*; //~ ERROR unresolved import `prelude`

mod unresolved_env {
use env;
use env; //~ ERROR unresolved import `env`

include!(concat!(env!("NON_EXISTENT"), "/data.rs"));
//~^ ERROR cannot determine resolution for the macro `env`
Expand Down
8 changes: 7 additions & 1 deletion src/test/ui/extenv/issue-55897.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,12 @@ LL | use prelude::*;
| unresolved import
| help: a similar path exists: `std::prelude`

error[E0432]: unresolved import `env`
--> $DIR/issue-55897.rs:4:9
|
LL | use env;
| ^^^ no `env` in the root

error: cannot determine resolution for the macro `env`
--> $DIR/issue-55897.rs:6:22
|
Expand All @@ -27,6 +33,6 @@ LL | include!(concat!(env!("NON_EXISTENT"), "/data.rs"));
|
= note: import resolution is stuck, try simplifying macro imports

error: aborting due to 4 previous errors
error: aborting due to 5 previous errors

For more information about this error, try `rustc --explain E0432`.
8 changes: 4 additions & 4 deletions src/test/ui/imports/unresolved-imports-used.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@ mod qux {

use qux::quz; //~ ERROR function `quz` is private
use qux::bar; //~ ERROR unresolved import `qux::bar`
use foo::bar;
use baz::*;
use foo::bar; //~ ERROR unresolved import `foo`
use baz::*; //~ ERROR unresolved import `baz`
use qux::bar2; //~ ERROR unresolved import `qux::bar2`
use foo2::bar2;
use baz2::*;
use foo2::bar2;//~ ERROR unresolved import `foo2`
use baz2::*; //~ ERROR unresolved import `baz2`
use qux::quy; //~ ERROR unused import

fn main() {}
26 changes: 25 additions & 1 deletion src/test/ui/imports/unresolved-imports-used.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,30 @@ error[E0432]: unresolved import `qux::bar2`
LL | use qux::bar2;
| ^^^^^^^^^ no `bar2` in `qux`

error[E0432]: unresolved import `foo`
--> $DIR/unresolved-imports-used.rs:11:5
|
LL | use foo::bar;
| ^^^ maybe a missing crate `foo`?

error[E0432]: unresolved import `baz`
--> $DIR/unresolved-imports-used.rs:12:5
|
LL | use baz::*;
| ^^^ maybe a missing crate `baz`?

error[E0432]: unresolved import `foo2`
--> $DIR/unresolved-imports-used.rs:14:5
|
LL | use foo2::bar2;
| ^^^^ maybe a missing crate `foo2`?

error[E0432]: unresolved import `baz2`
--> $DIR/unresolved-imports-used.rs:15:5
|
LL | use baz2::*;
| ^^^^ maybe a missing crate `baz2`?

error[E0603]: function `quz` is private
--> $DIR/unresolved-imports-used.rs:9:10
|
Expand All @@ -28,7 +52,7 @@ note: lint level defined here
LL | #![deny(unused_imports)]
| ^^^^^^^^^^^^^^

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

Some errors have detailed explanations: E0432, E0603.
For more information about an error, try `rustc --explain E0432`.
2 changes: 1 addition & 1 deletion src/test/ui/rust-2018/uniform-paths/deadlock.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// edition:2018
// compile-flags:--extern foo --extern bar

use foo::bar; //~ ERROR unresolved import
use foo::bar; //~ ERROR can't find crate for `foo`
use bar::foo;

fn main() {}
6 changes: 3 additions & 3 deletions src/test/ui/rust-2018/uniform-paths/deadlock.stderr
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
error[E0432]: unresolved import
error[E0463]: can't find crate for `foo`
--> $DIR/deadlock.rs:4:5
|
LL | use foo::bar;
| ^^^^^^^^
| ^^^ can't find crate

error: aborting due to previous error

For more information about this error, try `rustc --explain E0432`.
For more information about this error, try `rustc --explain E0463`.

0 comments on commit 8926301

Please sign in to comment.