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

Rustc suggests both std and core imports at times #83564

Closed
leonardo-m opened this issue Mar 27, 2021 · 5 comments · Fixed by #89224
Closed

Rustc suggests both std and core imports at times #83564

leonardo-m opened this issue Mar 27, 2021 · 5 comments · Fixed by #89224
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-resolve Area: Name resolution C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@leonardo-m
Copy link

rustc often offers a help to import something from std and core:

error[E0433]: failed to resolve: use of undeclared type `NonZeroU32`
  --> ...\test.rs:20:33
   |
20 | ...                   NonZeroU32::new(5).unwrap(),
   |                       ^^^^^^^^^^ not found in this scope
   |
help: consider importing one of these items
   |
6  | use core::num::NonZeroU32;
   |
6  | use std::num::NonZeroU32;
   |

Perhaps in such situations rustc could use an heuristic to suggest only one of the two imports.
@jyn514 jyn514 added A-diagnostics Area: Messages for errors, warnings, and lints A-resolve Area: Name resolution T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-enhancement Category: An issue proposing an enhancement or a PR with one. labels Mar 27, 2021
@Manishearth
Copy link
Member

cc @estebank feels like we should at least show std first here?

@Manishearth Manishearth changed the title [ER] In some cases avoid to give both std and core import suggestions Rustc suggests both std and core imports at times Mar 31, 2021
@leonardo-m
Copy link
Author

feels like we should at least show std first here?

This is quite useful in situations like the rust playground because there sometimes there are many different sources for a function, and sometimes the std one (that's most times the one you really want) gets elided in the "others" sources.

@estebank
Copy link
Contributor

estebank commented Apr 2, 2021 •
edited
Loading

We already have a couple of hacks for these kind of suggestions, so I'd be fine with just modifying the code here to push any use suggestion that starts with core:: to the end of the list, or at the very least, after std::.

rust/compiler/rustc_resolve/src/diagnostics.rs

Line 1640 in ee0357a

path_strings.sort();

As pointed above it might make sense to bias the sort to always put std:: first. We could also consider removing all references to core::, but that makes me uneasy.

@TaKO8Ki TaKO8Ki mentioned this issue Sep 24, 2021
Merged
@bors bors closed this as completed in 04d3f93 Sep 26, 2021
@camelid camelid mentioned this issue Sep 26, 2021
Merged
camelid added a commit to camelid/rust that referenced this issue Sep 26, 2021
@camelid
Add regression test for issue rust-lang#83564
df687bd
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Sep 28, 2021
@GuillaumeGomez
Rollup merge of rust-lang#89283 - camelid:issue-83564-test, r=davidtwco
fd89811 
Add regression test for issue rust-lang#83564

cc rust-lang#83564

r? `@davidtwco`
@GuillaumeGomez GuillaumeGomez mentioned this issue Sep 28, 2021
Closed
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Sep 28, 2021
@GuillaumeGomez
Rollup merge of rust-lang#89283 - camelid:issue-83564-test, r=davidtwco
733aa50 
Add regression test for issue rust-lang#83564

cc rust-lang#83564

r? ``@davidtwco``
@GuillaumeGomez GuillaumeGomez mentioned this issue Sep 28, 2021
Merged
bors added a commit to rust-lang-ci/rust that referenced this issue Sep 29, 2021
@bors
Auto merge of rust-lang#89331 - GuillaumeGomez:rollup-b10unye, r=Guil…
50f9f78 
…laumeGomez

Rollup of 8 pull requests

Successful merges:

 - rust-lang#87260 (Libgccjit codegen)
 - rust-lang#89212 (x.py: run `rustup toolchain link` in setup)
 - rust-lang#89233 (Hide `<...> defined here` note if the source is not available)
 - rust-lang#89235 (make junit output more consistent with default format)
 - rust-lang#89255 (Fix incorrect disambiguation suggestion for associated items)
 - rust-lang#89276 (Fix the population of the `union.impls` field)
 - rust-lang#89283 (Add regression test for issue rust-lang#83564)
 - rust-lang#89318 (rustc_session: Remove lint store from `Session`)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@camelid
Copy link
Member

camelid commented Oct 26, 2021

#89224 (comment)

@camelid camelid reopened this Oct 26, 2021
@camelid
Copy link
Member

camelid commented Oct 26, 2021

The regression test (#89283), when run on the playground, does seem to have regressed between beta 1.57.0 and nightly 1.58.0, which is weird because it's a test, so it shouldn't have changed. Maybe it only regressed when there are external dependencies?

estebank added a commit to estebank/rust that referenced this issue Jun 13, 2024
@estebank
Tweak output of import suggestions
f24ad4d 
When both `std::` and `core::` items are available, only suggest the
`std::` ones. We ensure that in `no_std` crates we suggest `core::`
items.

Ensure that the list of items suggested to be imported are always in the
order of local crate items, `std`/`core` items and finally foreign crate
items.

Tweak wording of import suggestion: if there are multiple items but they
are all of the same kind, we use the kind name and not the generic "items".

Fix rust-lang#83564.
@estebank estebank mentioned this issue Jun 13, 2024
Closed
estebank added a commit to estebank/rust that referenced this issue Jun 13, 2024
@estebank
Tweak output of import suggestions
75bb5db 
When both `std::` and `core::` items are available, only suggest the
`std::` ones. We ensure that in `no_std` crates we suggest `core::`
items.

Ensure that the list of items suggested to be imported are always in the
order of local crate items, `std`/`core` items and finally foreign crate
items.

Tweak wording of import suggestion: if there are multiple items but they
are all of the same kind, we use the kind name and not the generic "items".

Fix rust-lang#83564.
estebank added a commit to estebank/rust that referenced this issue Jun 13, 2024
@estebank
Tweak output of import suggestions
c88548b 
When both `std::` and `core::` items are available, only suggest the
`std::` ones. We ensure that in `no_std` crates we suggest `core::`
items.

Ensure that the list of items suggested to be imported are always in the
order of local crate items, `std`/`core` items and finally foreign crate
items.

Tweak wording of import suggestion: if there are multiple items but they
are all of the same kind, we use the kind name and not the generic "items".

Fix rust-lang#83564.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jun 13, 2024
@matthiaskrgr
Rollup merge of rust-lang#126371 - estebank:suggest-core, r=fmease
422da40 
Tweak output of import suggestions

When both `std::` and `core::` items are available, only suggest the `std::` ones. We ensure that in `no_std` crates we suggest `core::` items.

Ensure that the list of items suggested to be imported are always in the order of local crate items, `std`/`core` items and finally foreign crate items.

Tweak wording of import suggestion: if there are multiple items but they are all of the same kind, we use the kind name and not the generic "items".

Fix rust-lang#83564.
estebank added a commit to estebank/rust that referenced this issue Jun 13, 2024
@estebank
Tweak output of import suggestions
0a65bdd 
When both `std::` and `core::` items are available, only suggest the
`std::` ones. We ensure that in `no_std` crates we suggest `core::`
items.

Ensure that the list of items suggested to be imported are always in the
order of local crate items, `std`/`core` items and finally foreign crate
items.

Tweak wording of import suggestion: if there are multiple items but they
are all of the same kind, we use the kind name and not the generic "items".

Fix rust-lang#83564.
@bors bors closed this as completed in 5de8e6e Jun 14, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jun 14, 2024
@rust-timer
Unrolled build for rust-lang#126371
a198af3 
Rollup merge of rust-lang#126371 - estebank:suggest-core, r=fmease

Tweak output of import suggestions

When both `std::` and `core::` items are available, only suggest the `std::` ones. We ensure that in `no_std` crates we suggest `core::` items.

Ensure that the list of items suggested to be imported are always in the order of local crate items, `std`/`core` items and finally foreign crate items.

Tweak wording of import suggestion: if there are multiple items but they are all of the same kind, we use the kind name and not the generic "items".

Fix rust-lang#83564.
flip1995 pushed a commit to flip1995/rust that referenced this issue Jun 27, 2024
@estebank
Tweak output of import suggestions
fa9274c 
When both `std::` and `core::` items are available, only suggest the
`std::` ones. We ensure that in `no_std` crates we suggest `core::`
items.

Ensure that the list of items suggested to be imported are always in the
order of local crate items, `std`/`core` items and finally foreign crate
items.

Tweak wording of import suggestion: if there are multiple items but they
are all of the same kind, we use the kind name and not the generic "items".

Fix rust-lang#83564.
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 A-resolve Area: Name resolution C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
5 participants
@estebank @Manishearth @leonardo-m @jyn514 @camelid