Skip to content

#45829 when a renamed import conflict with a previous import #55113

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 6 commits into from
Oct 23, 2018
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
5 changes: 4 additions & 1 deletion src/librustc_resolve/build_reduced_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,10 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
id: item.id,
parent,
imported_module: Cell::new(Some(ModuleOrUniformRoot::Module(module))),
subclass: ImportDirectiveSubclass::ExternCrate(orig_name),
subclass: ImportDirectiveSubclass::ExternCrate {
source: orig_name,
target: ident,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

source is the optional original name if the import is renamed, target is the name it's imported at (so the original name if there was no rename)

},
root_span: item.span,
span: item.span,
module_path: Vec::new(),
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_resolve/check_unused.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ pub fn check_crate(resolver: &mut Resolver, krate: &ast::Crate) {
}
}
}
ImportDirectiveSubclass::ExternCrate(_) => {
ImportDirectiveSubclass::ExternCrate { .. } => {
resolver.maybe_unused_extern_crates.push((directive.id, directive.span));
}
ImportDirectiveSubclass::MacroUse => {
Expand Down
57 changes: 36 additions & 21 deletions src/librustc_resolve/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1234,7 +1234,7 @@ impl<'a> NameBinding<'a> {
match self.kind {
NameBindingKind::Import {
directive: &ImportDirective {
subclass: ImportDirectiveSubclass::ExternCrate(_), ..
subclass: ImportDirectiveSubclass::ExternCrate { .. }, ..
}, ..
} => true,
_ => false,
Expand All @@ -1248,15 +1248,6 @@ impl<'a> NameBinding<'a> {
}
}

fn is_renamed_extern_crate(&self) -> bool {
if let NameBindingKind::Import { directive, ..} = self.kind {
if let ImportDirectiveSubclass::ExternCrate(Some(_)) = directive.subclass {
return true;
}
}
false
}

fn is_glob_import(&self) -> bool {
match self.kind {
NameBindingKind::Import { directive, .. } => directive.is_glob(),
Expand Down Expand Up @@ -3803,7 +3794,7 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
if let NameBindingKind::Import { directive: d, .. } = binding.kind {
// Careful: we still want to rewrite paths from
// renamed extern crates.
if let ImportDirectiveSubclass::ExternCrate(None) = d.subclass {
if let ImportDirectiveSubclass::ExternCrate { source: None, .. } = d.subclass {
return
}
}
Expand Down Expand Up @@ -4783,10 +4774,17 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
};

let cm = self.session.source_map();
let rename_msg = "You can use `as` to change the binding name of the import";

if let (Ok(snippet), false) = (cm.span_to_snippet(binding.span),
binding.is_renamed_extern_crate()) {
let rename_msg = "you can use `as` to change the binding name of the import";

if let (
Ok(snippet),
NameBindingKind::Import { directive, ..},
_dummy @ false,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Side note: This could have been just false :)

) = (
cm.span_to_snippet(binding.span),
binding.kind.clone(),
binding.span.is_dummy(),
) {
let suggested_name = if name.as_str().chars().next().unwrap().is_uppercase() {
format!("Other{}", name)
} else {
Expand All @@ -4795,13 +4793,30 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {

err.span_suggestion_with_applicability(
binding.span,
rename_msg,
if snippet.ends_with(';') {
format!("{} as {};", &snippet[..snippet.len() - 1], suggested_name)
} else {
format!("{} as {}", snippet, suggested_name)
&rename_msg,
match (&directive.subclass, snippet.as_ref()) {
(ImportDirectiveSubclass::SingleImport { .. }, "self") =>
format!("self as {}", suggested_name),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imports with self are a simple case, as it's only possible in the form import foo::{ self }

(ImportDirectiveSubclass::SingleImport { source, .. }, _) =>
format!(
"{} as {}{}",
&snippet[..((source.span.hi().0 - binding.span.lo().0) as usize)],
suggested_name,
if snippet.ends_with(";") {
";"
} else {
""
}
),
(ImportDirectiveSubclass::ExternCrate { source, target, .. }, _) =>
format!(
"extern crate {} as {};",
source.unwrap_or(target.name),
suggested_name,
),
(_, _) => unreachable!(),
},
Applicability::MachineApplicable,
Applicability::MaybeIncorrect,
);
} else {
err.span_label(binding.span, rename_msg);
Expand Down
7 changes: 5 additions & 2 deletions src/librustc_resolve/resolve_imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,10 @@ pub enum ImportDirectiveSubclass<'a> {
max_vis: Cell<ty::Visibility>, // The visibility of the greatest re-export.
// n.b. `max_vis` is only used in `finalize_import` to check for re-export errors.
},
ExternCrate(Option<Name>),
ExternCrate {
source: Option<Name>,
target: Ident,
},
MacroUse,
}

Expand Down Expand Up @@ -1342,7 +1345,7 @@ fn import_directive_subclass_to_string(subclass: &ImportDirectiveSubclass) -> St
match *subclass {
SingleImport { source, .. } => source.to_string(),
GlobImport { .. } => "*".to_string(),
ExternCrate(_) => "<extern crate>".to_string(),
ExternCrate { .. } => "<extern crate>".to_string(),
MacroUse => "#[macro_use]".to_string(),
}
}
2 changes: 1 addition & 1 deletion src/test/ui/blind/blind-item-block-item-shadow.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ LL | use foo::Bar;
| ^^^^^^^^ `Bar` reimported here
|
= note: `Bar` must be defined only once in the type namespace of this block
help: You can use `as` to change the binding name of the import
help: you can use `as` to change the binding name of the import
|
LL | use foo::Bar as OtherBar;
| ^^^^^^^^^^^^^^^^^^^^
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/blind/blind-item-item-shadow.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ LL | use foo::foo;
| ^^^^^^^^ `foo` reimported here
|
= note: `foo` must be defined only once in the type namespace of this module
help: You can use `as` to change the binding name of the import
help: you can use `as` to change the binding name of the import
|
LL | use foo::foo as other_foo;
| ^^^^^^^^^^^^^^^^^^^^^
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/double-import.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ LL | use sub2::foo; //~ ERROR the name `foo` is defined multiple times
| ^^^^^^^^^ `foo` reimported here
|
= note: `foo` must be defined only once in the value namespace of this module
help: You can use `as` to change the binding name of the import
help: you can use `as` to change the binding name of the import
|
LL | use sub2::foo as other_foo; //~ ERROR the name `foo` is defined multiple times
| ^^^^^^^^^^^^^^^^^^^^^^
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/double-type-import.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ LL | use self::bar::X;
| ^^^^^^^^^^^^ `X` reimported here
|
= note: `X` must be defined only once in the type namespace of this module
help: You can use `as` to change the binding name of the import
help: you can use `as` to change the binding name of the import
|
LL | use self::bar::X as OtherX;
| ^^^^^^^^^^^^^^^^^^^^^^
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/duplicate/duplicate-check-macro-exports.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ LL | macro_rules! panic { () => {} } //~ ERROR the name `panic` is defined multi
| ^^^^^^^^^^^^^^^^^^ `panic` redefined here
|
= note: `panic` must be defined only once in the macro namespace of this module
help: You can use `as` to change the binding name of the import
help: you can use `as` to change the binding name of the import
|
LL | pub use std::panic as other_panic;
| ^^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/error-codes/E0252.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ LL | use bar::baz; //~ ERROR E0252
| ^^^^^^^^ `baz` reimported here
|
= note: `baz` must be defined only once in the type namespace of this module
help: You can use `as` to change the binding name of the import
help: you can use `as` to change the binding name of the import
|
LL | use bar::baz as other_baz; //~ ERROR E0252
| ^^^^^^^^^^^^^^^^^^^^^
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/error-codes/E0254.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ LL | use foo::alloc;
| ^^^^^^^^^^ `alloc` reimported here
|
= note: `alloc` must be defined only once in the type namespace of this module
help: You can use `as` to change the binding name of the import
help: you can use `as` to change the binding name of the import
|
LL | use foo::alloc as other_alloc;
| ^^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/error-codes/E0255.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ LL | fn foo() {} //~ ERROR E0255
| ^^^^^^^^ `foo` redefined here
|
= note: `foo` must be defined only once in the value namespace of this module
help: You can use `as` to change the binding name of the import
help: you can use `as` to change the binding name of the import
|
LL | use bar::foo as other_foo;
| ^^^^^^^^^^^^^^^^^^^^^
Expand Down
9 changes: 5 additions & 4 deletions src/test/ui/error-codes/E0259.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,13 @@ LL | extern crate alloc;
| ------------------- previous import of the extern crate `alloc` here
LL |
LL | extern crate libc as alloc;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
| `alloc` reimported here
| You can use `as` to change the binding name of the import
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ `alloc` reimported here
|
= note: `alloc` must be defined only once in the type namespace of this module
help: you can use `as` to change the binding name of the import
|
LL | extern crate libc as other_alloc;
|

error: aborting due to previous error

Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/error-codes/E0260.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ LL | mod alloc {
| ^^^^^^^^^ `alloc` redefined here
|
= note: `alloc` must be defined only once in the type namespace of this module
help: You can use `as` to change the binding name of the import
help: you can use `as` to change the binding name of the import
|
LL | extern crate alloc as other_alloc;
|
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/error-codes/E0430.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ LL | use std::fmt::{self, self}; //~ ERROR E0430
| previous import of the module `fmt` here
|
= note: `fmt` must be defined only once in the type namespace of this module
help: You can use `as` to change the binding name of the import
help: you can use `as` to change the binding name of the import
|
LL | use std::fmt::{self, self as other_fmt}; //~ ERROR E0430
| ^^^^^^^^^^^^^^^^^
Expand Down
9 changes: 5 additions & 4 deletions src/test/ui/extern/extern-crate-rename.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,13 @@ error[E0259]: the name `m1` is defined multiple times
LL | extern crate m1;
| ---------------- previous import of the extern crate `m1` here
LL | extern crate m2 as m1; //~ ERROR is defined multiple times
| ^^^^^^^^^^^^^^^^^^^^^^
| |
| `m1` reimported here
| You can use `as` to change the binding name of the import
| ^^^^^^^^^^^^^^^^^^^^^^ `m1` reimported here
|
= note: `m1` must be defined only once in the type namespace of this module
help: you can use `as` to change the binding name of the import
|
LL | extern crate m2 as other_m1; //~ ERROR is defined multiple times
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to previous error

Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/imports/duplicate.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ LL | use a::foo; //~ ERROR the name `foo` is defined multiple times
| ^^^^^^ `foo` reimported here
|
= note: `foo` must be defined only once in the value namespace of this module
help: You can use `as` to change the binding name of the import
help: you can use `as` to change the binding name of the import
|
LL | use a::foo as other_foo; //~ ERROR the name `foo` is defined multiple times
| ^^^^^^^^^^^^^^^^^^^
Expand Down
6 changes: 3 additions & 3 deletions src/test/ui/issues/issue-19498.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ LL | mod A {} //~ ERROR the name `A` is defined multiple times
| ^^^^^ `A` redefined here
|
= note: `A` must be defined only once in the type namespace of this module
help: You can use `as` to change the binding name of the import
help: you can use `as` to change the binding name of the import
|
LL | use self::A as OtherA;
| ^^^^^^^^^^^^^^^^^
Expand All @@ -23,7 +23,7 @@ LL | pub mod B {} //~ ERROR the name `B` is defined multiple times
| ^^^^^^^^^ `B` redefined here
|
= note: `B` must be defined only once in the type namespace of this module
help: You can use `as` to change the binding name of the import
help: you can use `as` to change the binding name of the import
|
LL | use self::B as OtherB;
| ^^^^^^^^^^^^^^^^^
Expand All @@ -37,7 +37,7 @@ LL | mod D {} //~ ERROR the name `D` is defined multiple times
| ^^^^^ `D` redefined here
|
= note: `D` must be defined only once in the type namespace of this module
help: You can use `as` to change the binding name of the import
help: you can use `as` to change the binding name of the import
|
LL | use C::D as OtherD;
| ^^^^^^^^^^^^^^
Expand Down
10 changes: 5 additions & 5 deletions src/test/ui/issues/issue-24081.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ LL | type Add = bool; //~ ERROR the name `Add` is defined multiple times
| ^^^^^^^^^^^^^^^^ `Add` redefined here
|
= note: `Add` must be defined only once in the type namespace of this module
help: You can use `as` to change the binding name of the import
help: you can use `as` to change the binding name of the import
|
LL | use std::ops::Add as OtherAdd;
| ^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -23,7 +23,7 @@ LL | struct Sub { x: f32 } //~ ERROR the name `Sub` is defined multiple times
| ^^^^^^^^^^ `Sub` redefined here
|
= note: `Sub` must be defined only once in the type namespace of this module
help: You can use `as` to change the binding name of the import
help: you can use `as` to change the binding name of the import
|
LL | use std::ops::Sub as OtherSub;
| ^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -38,7 +38,7 @@ LL | enum Mul { A, B } //~ ERROR the name `Mul` is defined multiple times
| ^^^^^^^^ `Mul` redefined here
|
= note: `Mul` must be defined only once in the type namespace of this module
help: You can use `as` to change the binding name of the import
help: you can use `as` to change the binding name of the import
|
LL | use std::ops::Mul as OtherMul;
| ^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -53,7 +53,7 @@ LL | mod Div { } //~ ERROR the name `Div` is defined multiple times
| ^^^^^^^ `Div` redefined here
|
= note: `Div` must be defined only once in the type namespace of this module
help: You can use `as` to change the binding name of the import
help: you can use `as` to change the binding name of the import
|
LL | use std::ops::Div as OtherDiv;
| ^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -68,7 +68,7 @@ LL | trait Rem { } //~ ERROR the name `Rem` is defined multiple times
| ^^^^^^^^^ `Rem` redefined here
|
= note: `Rem` must be defined only once in the type namespace of this module
help: You can use `as` to change the binding name of the import
help: you can use `as` to change the binding name of the import
|
LL | use std::ops::Rem as OtherRem;
| ^^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down
8 changes: 4 additions & 4 deletions src/test/ui/issues/issue-25396.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ LL | use bar::baz; //~ ERROR the name `baz` is defined multiple times
| ^^^^^^^^ `baz` reimported here
|
= note: `baz` must be defined only once in the type namespace of this module
help: You can use `as` to change the binding name of the import
help: you can use `as` to change the binding name of the import
|
LL | use bar::baz as other_baz; //~ ERROR the name `baz` is defined multiple times
| ^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -21,7 +21,7 @@ LL | use bar::Quux; //~ ERROR the name `Quux` is defined multiple times
| ^^^^^^^^^ `Quux` reimported here
|
= note: `Quux` must be defined only once in the type namespace of this module
help: You can use `as` to change the binding name of the import
help: you can use `as` to change the binding name of the import
|
LL | use bar::Quux as OtherQuux; //~ ERROR the name `Quux` is defined multiple times
| ^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -35,7 +35,7 @@ LL | use bar::blah; //~ ERROR the name `blah` is defined multiple times
| ^^^^^^^^^ `blah` reimported here
|
= note: `blah` must be defined only once in the type namespace of this module
help: You can use `as` to change the binding name of the import
help: you can use `as` to change the binding name of the import
|
LL | use bar::blah as other_blah; //~ ERROR the name `blah` is defined multiple times
| ^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -49,7 +49,7 @@ LL | use bar::WOMP; //~ ERROR the name `WOMP` is defined multiple times
| ^^^^^^^^^ `WOMP` reimported here
|
= note: `WOMP` must be defined only once in the value namespace of this module
help: You can use `as` to change the binding name of the import
help: you can use `as` to change the binding name of the import
|
LL | use bar::WOMP as OtherWOMP; //~ ERROR the name `WOMP` is defined multiple times
| ^^^^^^^^^^^^^^^^^^^^^^
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/issues/issue-26886.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ LL | use std::sync::Arc; //~ ERROR the name `Arc` is defined multiple times
| ^^^^^^^^^^^^^^ `Arc` reimported here
|
= note: `Arc` must be defined only once in the type namespace of this module
help: You can use `as` to change the binding name of the import
help: you can use `as` to change the binding name of the import
|
LL | use std::sync::Arc as OtherArc; //~ ERROR the name `Arc` is defined multiple times
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -22,7 +22,7 @@ LL | use std::sync; //~ ERROR the name `sync` is defined multiple times
| ^^^^^^^^^ `sync` reimported here
|
= note: `sync` must be defined only once in the type namespace of this module
help: You can use `as` to change the binding name of the import
help: you can use `as` to change the binding name of the import
|
LL | use std::sync as other_sync; //~ ERROR the name `sync` is defined multiple times
| ^^^^^^^^^^^^^^^^^^^^^^^
Expand Down
Loading