rust icon indicating copy to clipboard operation
rust copied to clipboard

Unclear doctest error when implementing a trait in a different file.

Open mark-schultz-wu opened this issue 2 years ago • 5 comments

Consider a library with the following directory structure (no playground because multiple files, but this git repository).

src
├── lib.rs
├── module_one.rs
└── module_two.rs

where module_one.rs is

#[derive(Clone, Debug)]
pub struct MyStruct {}

pub trait MyTrait {
  fn returns_true(self) -> bool;
}

and module_two.rs is

use crate::module_one::{MyStruct, MyTrait};

impl MyTrait for MyStruct {
  /// This doctest has a confusing error message
  ///
  /// ```rust
  /// use doctest::module_one::{MyStruct, MyTrait};
  /// use doctest::module_two::returns_true;
  /// let my_struct : MyStruct = MyStruct {};
  /// assert!(my_struct.returns_true());
  /// ```
  ///
  fn returns_true(self) -> bool {
    true
  }
}

The current output of cargo test --doc is

failures:

---- src/module_two.rs - module_two::MyStruct::returns_true (line 6) stdout ----
error[E0432]: unresolved import `doctest::module_two::returns_true`
 --> src/module_two.rs:8:5
  |
5 | use doctest::module_two::returns_true;
  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no `returns_true` in `module_two`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0432`.

This error message suggests that the issue is the visibility of returns_true in module_two. This is not the issue though. Deleting the entire line use doctest::module_two::returns_true; instead fixes the issue.

mark-schultz-wu avatar Aug 03 '22 18:08 mark-schultz-wu

I am not sure what the issue is. The message and the span of the diagnostic both look correct to me.

It's true that returns_true is not defined inside of module_two. The diagnostic only highlights the path and not the whole use item since that's more precise (and since it can scale to complex use trees). Do you suggest it should only highlight the last path segment returns_true? That I would understand. If you suggest it should highlight the entire line, I will say that I don't really see any gained advantage. The squiggly underline does not necessary imply that only the highlighted section needs to be removed or that anything needs to be removed, really.

This error message suggests that the issue is the visibility of returns_true in module_two.

The error message that you posted does not mention anything about visibility. Visibility has a specific meaning in Rust. It refers to the concept of private versus (restrictedly) public (no pub, pub, pub(in …), etc.).

fmease avatar Aug 04 '22 11:08 fmease

While returns_true is not defined within module_two, there is an implementation of returns_true within module_two that I want to access in this situation. It was not clear to me from this error message that the way to get access to the returns_true implementation that is given in module_two is by ignoring module_two and solely importing from module_one.

If the error message had mentioned anything regarding the way to access a method defined on some struct is by importing that struct, that would have been useful to me. Of course, I have no idea if this is feasible to do.

I mention visibility both because

  • I know that doctests can only test publicly-accessible elements of the api going into this, and
  • another error (not contained in the above MWE) that was triggered was E0425, which does mention visibility as a potential cause. If an example such as the above that triggers error E0425 would be useful let me know, so I can try to reproduce it.

mark-schultz-wu avatar Aug 04 '22 12:08 mark-schultz-wu

Ah, thanks for your reply. That cleared things up for me :)

So given the following minimized reproducer:

mod one {
    pub trait Tr {
        fn meth();
    }
}

mod two {
    use super::one::Tr;

    impl Tr for () {
        fn meth() {}
    }
}

use one::Tr;
use two::meth;

You'd like to see sth. akin to this emitted (right?):

  error[E0432]: unresolved import `two::meth`
    --> src/lib.rs:16:5
     |
  16 | use two::meth;
-    |     ^^^^^^^^^ no `meth` in `two`
+    | ----^^^^^^^^^ no `meth` in `two`
+    | |
+    | help: remove this use item
+    |
+ note: if you intended to use the method `meth` from the impl block in module `two`...
+   --> src/lib.rs:11:12
+    |
+ 11 |         fn meth() {}
+    |            ^^^^
+    |
+ note: ...nothing else needs to be imported since it is already made available through this import
+   --> src/lib.rs:15:5
+    |
+ 15 | use one::Tr;
+    |     ^^^^^^^

  warning: unused import: `one::Tr`
    --> src/lib.rs:15:5
     |
  15 | use one::Tr;
     |     ^^^^^^^
     |
     = note: `#[warn(unused_imports)]` on by default

@rustbot label C-enhancement D-terse D-newcomer-roadblock

fmease avatar Aug 04 '22 14:08 fmease

Further, we could probably also improve the diagnostic for the case when the trait isn't imported:

mod one {
    pub trait Tr {
        fn meth();
    }
}

mod two {
    use super::one::Tr;

    impl Tr for () {
        fn meth() {}
    }
}

use two::meth;
  error[E0432]: unresolved import `two::meth`
    --> src/lib.rs:15:5
     |
  15 | use two::meth;
     |     ^^^^^^^^^ no `meth` in `two`
+    |
+ note: if you intended to use the method `meth` from the impl block in module `two`...
+   --> src/lib.rs:11:12
+    |
+ 11 |         fn meth() {}
+    |            ^^^^
+    |
+ help: ...import the corresponding trait `Tr` and remove the incorrect use item
+   --> src/lib.rs:15:5
+    |
+ 15 | use two::meth;
+    | --------------
+ 15 | use one::Tr;
+    | ++++++++++++

fmease avatar Aug 04 '22 14:08 fmease

Yes, both of those look great!

mark-schultz-wu avatar Aug 04 '22 16:08 mark-schultz-wu