Skip to content
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

Fix help for duplicated names: extern crate (...) as (...) #45856

Merged
merged 1 commit into from
Nov 10, 2017
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
4 changes: 2 additions & 2 deletions src/librustc_resolve/build_reduced_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ impl<'a> Resolver<'a> {
}
}

ItemKind::ExternCrate(_) => {
ItemKind::ExternCrate(as_name) => {
self.crate_loader.process_item(item, &self.definitions);

// n.b. we don't need to look at the path option here, because cstore already did
Expand All @@ -265,7 +265,7 @@ impl<'a> Resolver<'a> {
id: item.id,
parent,
imported_module: Cell::new(Some(module)),
subclass: ImportDirectiveSubclass::ExternCrate,
subclass: ImportDirectiveSubclass::ExternCrate(as_name),
span: item.span,
module_path: Vec::new(),
vis: Cell::new(vis),
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 @@ -120,7 +120,7 @@ pub fn check_crate(resolver: &mut Resolver, krate: &ast::Crate) {
_ if directive.used.get() ||
directive.vis.get() == ty::Visibility::Public ||
directive.span.source_equal(&DUMMY_SP) => {}
ImportDirectiveSubclass::ExternCrate => {
ImportDirectiveSubclass::ExternCrate(_) => {
resolver.maybe_unused_extern_crates.push((directive.id, directive.span));
}
ImportDirectiveSubclass::MacroUse => {
Expand Down
14 changes: 12 additions & 2 deletions src/librustc_resolve/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1118,7 +1118,7 @@ impl<'a> NameBinding<'a> {
match self.kind {
NameBindingKind::Import {
directive: &ImportDirective {
subclass: ImportDirectiveSubclass::ExternCrate, ..
subclass: ImportDirectiveSubclass::ExternCrate(_), ..
}, ..
} => true,
_ => false,
Expand All @@ -1132,6 +1132,15 @@ 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 @@ -3700,7 +3709,8 @@ impl<'a> Resolver<'a> {
let cm = self.session.codemap();
let rename_msg = "You can use `as` to change the binding name of the import";

if let Ok(snippet) = cm.span_to_snippet(binding.span) {
if let (Ok(snippet), false) = (cm.span_to_snippet(binding.span),
binding.is_renamed_extern_crate()) {
err.span_suggestion(binding.span,
rename_msg,
format!("{} as Other{}", snippet, name));
Expand Down
6 changes: 3 additions & 3 deletions src/librustc_resolve/resolve_imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use rustc::hir::def_id::DefId;
use rustc::hir::def::*;
use rustc::util::nodemap::{FxHashMap, FxHashSet};

use syntax::ast::{Ident, SpannedIdent, NodeId};
use syntax::ast::{Ident, Name, SpannedIdent, NodeId};
use syntax::ext::base::Determinacy::{self, Determined, Undetermined};
use syntax::ext::hygiene::Mark;
use syntax::parse::token;
Expand All @@ -48,7 +48,7 @@ pub enum ImportDirectiveSubclass<'a> {
max_vis: Cell<ty::Visibility>, // The visibility of the greatest reexport.
// n.b. `max_vis` is only used in `finalize_import` to check for reexport errors.
},
ExternCrate,
ExternCrate(Option<Name>),
MacroUse,
}

Expand Down Expand Up @@ -923,7 +923,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(),
}
}
1 change: 1 addition & 0 deletions src/test/compile-fail/E0259.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,6 @@ extern crate libc as alloc;
//~^ ERROR E0259
//~| NOTE `alloc` reimported here
//~| NOTE `alloc` must be defined only once in the type namespace of this module
//~| NOTE You can use `as` to change the binding name of the import

fn main() {}
11 changes: 11 additions & 0 deletions src/test/ui/suggestions/auxiliary/m1.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

pub fn foo() {}
11 changes: 11 additions & 0 deletions src/test/ui/suggestions/auxiliary/m2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

pub fn bar() {}
18 changes: 18 additions & 0 deletions src/test/ui/suggestions/extern-crate-rename.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// aux-build:m1.rs
// aux-build:m2.rs


extern crate m1;
extern crate m2 as m1;

fn main() {}
15 changes: 15 additions & 0 deletions src/test/ui/suggestions/extern-crate-rename.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
error[E0259]: the name `m1` is defined multiple times
--> $DIR/extern-crate-rename.rs:16:1
|
15 | extern crate m1;
| ---------------- previous import of the extern crate `m1` here
16 | extern crate m2 as m1;
| ^^^^^^^^^^^^^^^^^^^^^^
| |
| `m1` reimported here
Copy link
Contributor

Choose a reason for hiding this comment

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

FWIW, I find this message confusing. I think it's the phrasing "import of the extern crate m1", whereas I see this statement as importing the crate m2 -- but aliasing it to m1. I wonder if some small tweaks to the wording might help. Or maybe it's just fine as is, it's kind of a corner case, and if all goes to plan extern crate is going away anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The same wording issue happens with use.

| You can use `as` to change the binding name of the import
|
= note: `m1` must be defined only once in the type namespace of this module

error: aborting due to previous error