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

Import trait help message is incorrect #114884

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``
Sign up for free to join this conversation on GitHub. Already have an account? Sign in 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