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.

Sign up for GitHub

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

Jump to bottom

Suggest renaming a trait import if the names clash #32354

Closed
bluss opened this issue Mar 19, 2016 · 7 comments
Closed

Suggest renaming a trait import if the names clash #32354

bluss opened this issue Mar 19, 2016 · 7 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. WG-diagnostics Working group: Diagnostics

Comments

@bluss
Copy link
Member

bluss commented Mar 19, 2016

You can bring in multiple traits of the same name if you rename them.

Example use case (playground code)

use extension1::ConstructorExtension;
use extension2::ConstructorExtension;

current error:

<anon>:49:5: 49:37 error: a trait named `ConstructorExtension` has already been imported in this module [E0252]
<anon>:49 use extension2::ConstructorExtension;
              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
<anon>:49:5: 49:37 help: see the detailed explanation for E0252
<anon>:48:5: 48:37 note: previous import of `ConstructorExtension` here
<anon>:48 use extension1::ConstructorExtension;
              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Suggested note: "You can rename an imported name with as".
@GuillaumeGomez
Copy link
Member

Seems like a nice idea. I'm taking this issue.

@petrochenkov
Copy link
Contributor

Two thoughts: 1) This is not specific to traits, any import (or non-import) can be renamed in case of conflict 2) resolve already has too many overly specific errors and messages, which seemed like a nice idea and easy PR material, with no evidence of their actual helpfulness.

@Mark-Simulacrum
Copy link
Member

@GuillaumeGomez Checking in; I think no progress has been made. Do you think you'll have a chance to work on this soon?

@Mark-Simulacrum Mark-Simulacrum added the A-diagnostics Area: Messages for errors, warnings, and lints label May 2, 2017
@GuillaumeGomez
Copy link
Member

No I won't. Feel free to take it.

@Mark-Simulacrum Mark-Simulacrum added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Jul 24, 2017
@estebank
Copy link
Contributor

Mentoring instructions:

The duplicated import struct is created in src/librustc_resolve/lib.rs. A new check should be added for at least one of the bindings being an import. If only one of them is an import, do the following on the binding that is an import, if both are, choose one (I'd say new_binding):

let cm = self.session.codemap();
if let Ok(snippet) = cm.span_to_snippet(import_binding.span) {
    err.span_suggestion(import_binding.span,
                        "You can use `as` to change the binding name of the import",
                        format!("use {} as Other{}", snippet, name));
} else {
    err.span_label(import_binding.span,
                   "You can use `as` to change the binding name of the import");
}

@estebank estebank added E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. WG-diagnostics Working group: Diagnostics E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. labels Sep 29, 2017
@colinmarsh19
Copy link
Contributor

I might take a look at this issue and see what I can do. Let me know if there are any conflicts --Colin

@estebank
Copy link
Contributor

estebank commented Oct 9, 2017

@colinmarsh19 go ahead! Feel free to reach out for help here, gitter, or IRC.

@Cldfire Cldfire mentioned this issue Oct 31, 2017
Merged
kennytm added a commit to kennytm/rust that referenced this issue Nov 1, 2017
@kennytm
Rollup merge of rust-lang#45660 - Cldfire:suggest-rename-import, r=es…
0ec40c1 
…tebank

Suggest renaming import if names clash

Closes rust-lang#32354.

The output for the example in the issue looks like this:

```
~/p/local-rust-dev-testing ❯❯❯ cargo +local-s1 build
   Compiling local-rust-dev-testing v0.1.0 (file:///home/cldfire/programming_projects/local-rust-dev-testing)
error[E0252]: the name `ConstructorExtension` is defined multiple times
  --> src/main.rs:49:5
   |
48 | use extension1::ConstructorExtension;
   |     -------------------------------- previous import of the trait `ConstructorExtension` here
49 | use extension2::ConstructorExtension;
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `ConstructorExtension` reimported here
   |
   = note: `ConstructorExtension` 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
   |
49 | use extension2::ConstructorExtension as OtherConstructorExtension;
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

...
```

This is my first PR that touches the compiler in any way, so if there's something else I need to do here (e.g. add a test), please let me know :).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. WG-diagnostics Working group: Diagnostics
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@estebank @GuillaumeGomez @bluss @Mark-Simulacrum @petrochenkov @colinmarsh19