trying to import a use statement should not talk of privacy

[Manishearth]

fn main() {
    use foo::mem;
}

pub mod foo {
    use std::mem;
}

gives the error:

error: module `mem` is private
 --> <anon>:2:9
  |
2 |     use foo::mem;
  |         ^^^^^^^^

error: aborting due to previous error

However, really, we don't consider use std::mem to be introducing a module, so it's confusing when we say that it's private. This gives off the impression that we typed the correct path, but the library author neglected to make it public.

We should instead give an error that the module/type/etc does not exist, but perhaps note that there is a use statement that could be made pub use if the error is from the same crate.

[Mark-Simulacrum]

I somewhat disagree. I think we shouldn't say that module "mem" is private, instead something more along the lines of imported module... would be better, I think. I don't think the "is private" itself is wrong -- it seems fairly clear that mem is inside foo, it's just not public.

[Manishearth]

From the point of view of another crate what imports you have should not really be a visible detail.

The problem is that the way imports are generally taught; use is thought of as bringing in a name locally, not bringing in a name that can be imported from other modules (even though you can do it from e.g. a submodule)

[tbu-]

However, if you're accidently importing a private name (say, a struct found in the source code of the crate), then the current error message is helpful. I encountered this a couple of times.

Changing this to not mentioning the privacy at all would make me wonder whether I mistyped some part of the path instead of wondering about the privacy.

[Manishearth]

Yes, I am talking about importing a use statement only.

The point is to distinguish between the case where you typed the correct path but it is private, and you typed the wrong path but does exist as a private use statement. Currently the two give the same error, even though they are generally fixed in very different ways.

[nikomatsakis]

I find this confusing as well. (JFYI I am planning on suggesting this issue for the upcoming "Boston rust hackathon" meetup.)

[estebank]

Proposed output:

error[E0603]: module `mem` is private
 --> src/main.rs:2:9
  |
2 |     use foo::mem;
  |         ^^^^^^^^
...
6 |     use std::mem;
  |     ^^^^^^^^^^^^^ private module
  |     |
  |     help: consider making it public: `pub use std::mem;`

This would be shown only when a span is available, which is the only time when you can actually do something about it. When there's no span available, produce the output for nonexistent modules:

error[E0432]: unresolved import `foo::mem`
 --> src/main.rs:2:9
  |
2 |     use foo::mem;
  |         ^^^^^^^^ no `mem` in the root

[oli-obk]

When there's no span available, produce the output for nonexistent modules:

or we could emit a suggestion to use std::mem directly here, which often is what we wanted in the first place.

[estebank]

@oli-obk that would only apply to use statements, but I agree.

[nikomatsakis]

I think making a pub use is almost never what the person actually wants. I would prefer that we start with the assumption that they just forgot to import something and it happens to match an import used in another module -- at least, this is always what happens to me.

[kornelski]

I've ran into a variant of this error by getting module name slightly wrong (playground):

pub mod definer {
    pub struct PubStruct;
}

mod user {
    use crate::definer::PubStruct;
}

mod fail {
    use crate::user::PubStruct;
    // error[E0603]: struct `PubStruct` is private
}

The problem was I've used crate::user::PubStruct instead of crate::definer::PubStruct. It's the same struct, just a layer of indirection too much.

I find this message misleading, because:

• The message says struct PubStruct is private, but that's not really true: the struct PubStruct is public. It's probably referring to the use PubStruct statement being private, but that detail isn't clear from the message, and it's not a useful information, because I wanted the struct, not the use.

• Existence of a private use "leaks" outside of the module. If mod user didn't have the use statement, the message would be correct "error[E0432]: unresolved import crate::user::PubStruct"

[Manishearth]

Closed #57619 as a dupe of this

[kornelski]

The error message is a bit better now. It still talks about privacy.

   |
10 |     use crate::user::PubStruct;
   |                      ^^^^^^^^^ private struct import
   |
note: the struct import `PubStruct` is defined here...
  --> src/lib.rs:6:9
   |
6  |     use crate::definer::PubStruct;
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...and refers to the struct `PubStruct` which is defined here
  --> src/lib.rs:2:5
   |
2  |     pub struct PubStruct;
   |     ^^^^^^^^^^^^^^^^^^^^^ consider importing it directly

[petrochenkov]

The current output follows #42909 (comment) and talks about the privacy intentionally.
The output was implemented by me, with this issue in mind, I forgot to close this issue back then, but I do consider it addressed.

[jplatte]

For some context, I've found this issue after looking for one when seeing the current error mentioned in https://internals.rust-lang.org/t/cautiously-consolidating-the-standard-library/14169.

IMHO the current error message is still confusing in many cases. I guess it makes sense to talk about privacy if you try to import sth. that is not accessible within the same crate, and maybe also another crate in the same workspace. But for entirely out-of-tree stuff, I would never expect private uses to be shown in error messages.

[estebank]

I think the two improvements left to do are providing a structured suggestion for the new import, or at least changing the last span label to mention the ADT's full path, and maybe eliding the chain for external crates and just provide a "not found" and suggestion. That might be confusing to people that expect the reexport to be available, but when that happens in a separate crate the likelihood of the user dealing with both is low. Maybe we can get around that with slight rewording, maybe just a note saying "the path is available as a reexport but it is private" or something.

[kornelski]

I think swapping order of the error message and the note would improve it — instead of an error about privacy with a note about wrong path, make it an error about wrong path, with a note about privacy.

1. The main part of the error should be the same as for paths that don't exist ("error[E0432]: unresolved import")

2. If possible to customize the error for same crate/same workspace, then it'd be nice to add a note that explains there's a re-export, but it's not for public use. I wouldn't even mention re-exports for std and 3rd party crates, because that just leaks an implementation detail. This privacy leak becomes a problem for library authors, because they can't freely import things in the implementation without risking user confusion.

3. Provide auto-fix suggestion with the right path to import. Just like there's auto-fix for "help: consider importing this struct" when you use a name without importing it first. If there's "did you mean" with a correct path, then explanations aren't even that important. At worst the user will scratch their head and move on.

[estebank]

On #118066:

error[E0603]: module import `mem` is private
  --> $DIR/private-std-reexport-suggest-public.rs:4:14
   |
LL |     use foo::mem;
   |              ^^^ private module import
   |
note: the module import `mem` is defined here...
  --> $DIR/private-std-reexport-suggest-public.rs:8:9
   |
LL |     use std::mem;
   |         ^^^^^^^^
note: ...and refers to the module `mem` which is defined here
  --> $SRC_DIR/std/src/lib.rs:LL:COL
   |
   = note: you could import this
help: import `mem` through the re-export
   |
LL |     use std::mem;
   |         ~~~~~~~~