Skip to content

Import trait help message is incorrect #114884

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
disconnect3d opened this issue Aug 16, 2023 · 8 comments · Fixed by #121190
Closed

Import trait help message is incorrect #114884

disconnect3d opened this issue Aug 16, 2023 · 8 comments · Fixed by #121190
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-resolve Area: Name/path resolution done by `rustc_resolve` specifically D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@disconnect3d
Copy link

disconnect3d commented Aug 16, 2023

Code

extern crate rustc_index;

use rustc_index::{vec::Idx};

fn main() { }

Current output

# rustc --version
rustc 1.73.0-nightly (0bdb00d55 2023-08-15)

# rustc main.rs
error[E0603]: module `vec` is private
 --> main.rs:3:19
  |
3 | use rustc_index::{vec::Idx};
  |                   ^^^ private module
  |
note: the module `vec` is defined here
 --> /root/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/compiler/rustc_index/src/lib.rs:24:1
help: consider importing this trait instead
  |
3 | use rustc_index::{rustc_index::Idx};
  |                   ~~~~~~~~~~~~~~~~

error[E0658]: use of unstable library feature 'rustc_private': this crate is being loaded from the sysroot, an unstable location; did you mean to load this crate from crates.io via `Cargo.toml` instead?
 --> main.rs:1:1
  |
1 | extern crate rustc_index;
  | ^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: see issue #27812 <https://github.com/rust-lang/rust/issues/27812> for more information
  = help: add `#![feature(rustc_private)]` to the crate attributes to enable

error: aborting due to 2 previous errors

Some errors have detailed explanations: E0603, E0658.
For more information about an error, try `rustc --explain E0603`.

(The rustc_private error is irrelevant)

Desired output

The suggestion to import rustc_index::{rustc_index::Idx}; is obviously wrong. It should be rustc_index::{Idx}; (or even rustc_index::Idx).

@disconnect3d disconnect3d added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 16, 2023
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Aug 16, 2023
@compiler-errors
Copy link
Member

Span of the import suggestion needs to be adjusted.

@compiler-errors compiler-errors added A-resolve Area: Name/path resolution done by `rustc_resolve` specifically D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Aug 16, 2023
@gurry
Copy link
Contributor

gurry commented Aug 17, 2023

@rustbot claim

@gurry
Copy link
Contributor

gurry commented Aug 18, 2023

When I try to compile this snippet:

extern crate rustc_index;

use rustc_index::{vec::Idx};

fn main() { }

with my locally built rustc I get the following error:

error[E0463]: can't find crate for `rustc_index`
 --> ../issue-114884.rs:1:1
  |
1 | extern crate rustc_index;
  | ^^^^^^^^^^^^^^^^^^^^^^^^^ can't find crate
  |
  = help: maybe you need to install the missing components with: `rustup component add rust-src rustc-dev llvm-tools-preview`

error: aborting due to previous error

Running rustup component add rust-src rustc-dev llvm-tools-preview didn't help (although it does make it possible to compile with the installed nightly rustc as opposed to the locally built one).

@compiler-errors Can you please help me resolve this problem?

rustc metadata:

rustc 1.73.0-dev
binary: rustc
commit-hash: unknown
commit-date: unknown
host: x86_64-pc-windows-msvc
release: 1.73.0-dev
LLVM version: 17.0.0

@compiler-errors
Copy link
Member

compiler-errors commented Aug 18, 2023

@gurry please reproduce this issue using another way that doesn't rely on rustc-internal crates. The only part that matters is the use having braces in the path, I think.

@gurry
Copy link
Contributor

gurry commented Aug 19, 2023

Yes, I first tried with a rustc_index mod in the same file, but it didn't reproduce. So I thought maybe having a crate in the path is relevant. Anyway, I'll try more with the inline mod.

@gurry
Copy link
Contributor

gurry commented Aug 21, 2023

The issue reproduces with the following self contained snippet:

pub mod mod1 {
    pub trait TraitA {}
}

pub mod mod2 {
    mod sub_mod {
        use mod1::TraitA;
    }
}

use mod2::{sub_mod::TraitA};

fn main() { }

resulting in the following diagnostic:

error[E0603]: module `sub_mod` is private
  --> issue-114884-incorrect-import-trait-msg.rs:11:12
   |
11 | use mod2::{sub_mod::TraitA};
   |            ^^^^^^^ private module
   |
note: the module `sub_mod` is defined here
  --> issue-114884-incorrect-import-trait-msg.rs:6:5
   |
6  |     mod sub_mod {
   |     ^^^^^^^^^^^
help: consider importing this trait instead
   |
11 | use mod2::{mod1::TraitA};
   |            ~~~~~~~~~~~~

The immediate cause of the issue is the below argument to show_candidates():

Some(dedup_span.until(outer_ident.span.shrink_to_hi())),

Specifically, the problem is with dedup_span. It starts at the wrong point which is immediately after the opening brace in the path use mod2::{sub_mod::TraitA} instead of at the beginning of the path.

At first glance the solution is to simply set dedup_span to the correct value i.e. the beginning of the path which will change the suggestion from this:

11 | use mod2::{mod1::TraitA};
   |            ~~~~~~~~~~~~

to this:

11 | use mod1::TraitA;
   |      ~~~~~~~~~~~~

However, it will fail for multi-child paths such as use mod2::{mod1::TraitA, SomeOtherType}. In that case, it will produce a malformed suggestion shown below:

11 | use mod1::TraitA, SomeOtherType};
   |            ~~~~~~~~~~~~~~~~~~

Therefore what we may need is a smarter approach which can:

  • Suggest replacing the entire path in the single-child case i.e. use mod2::{mod1::TraitA} -> use mod1::TraitA and
  • Suggest the new path on a separate line in the multi-child case i.e. use mod2::{mod1::TraitA, SomeOtherType}) ->
11 | - use mod2::{TraitA, SomeOtherType};
11 | + use mod2::SomeOtherType; 
12 | + use mod1::TraitA; 

It may also be an option to just stick with the simpler approach of correcting dedup_span and not showing any suggestion whatsoever for the bothersome multi-child case.

What are you thoughts @compiler-errors ?

@gurry gurry removed their assignment Aug 22, 2023
@gurry
Copy link
Contributor

gurry commented Aug 22, 2023

@rustbot claim

@gurry gurry removed their assignment Sep 2, 2023
@estebank
Copy link
Contributor

Triage: no change

error[E0603]: module `sub_mod` is private
  --> src/main.rs:11:12
   |
11 | use mod2::{sub_mod::TraitA};
   |            ^^^^^^^ private module
   |
note: the module `sub_mod` is defined here
  --> src/main.rs:6:5
   |
6  |     mod sub_mod {
   |     ^^^^^^^^^^^
help: consider importing this trait instead
   |
11 | use mod2::{crate::mod1::TraitA};
   |            ~~~~~~~~~~~~~~~~~~~

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Mar 6, 2024
avoid overlapping privacy suggestion for single nested imports

Fixes rust-lang#114884

This PR aims to avoid confusion inside braces for import suggestions.

r? `@petrochenkov`
@bors bors closed this as completed in c7fca03 Mar 6, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Mar 6, 2024
Rollup merge of rust-lang#121190 - bvanjoi:fix-114884, r=petrochenkov

avoid overlapping privacy suggestion for single nested imports

Fixes rust-lang#114884

This PR aims to avoid confusion inside braces for import suggestions.

r? ``@petrochenkov``
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-resolve Area: Name/path resolution done by `rustc_resolve` specifically D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants