missing_doc_code_examples lint complains about foreign trait implementations

[skreborn]

The missing_doc_code_examples lint seems to be erroneously complaining about missing documentation examples where local types implement foreign traits (both from 3rd party crates and the standard library).

The example below should pass all checks as far as I can tell:

/// Foo.
pub struct Foo {
  /// Bar.
  pub bar: i32,
  /// Baz.
  pub baz: i32,
}

impl Neg for Foo {
  type Output = Self;

  #[inline]
  fn neg(self) -> Self::Output {
    Self::Output {
      bar: -self.bar,
      baz: -self.baz,
    }
  }
}

However, running cargo doc results in the following warnings:

warning: missing code example in this documentation
   --> foo.rs:9:1
    |
    | / impl Neg for Foo {
    | |   type Output = Self;
    | |
    | |   #[inline]
...   |
    | |   }
    | | }
    | |_^

warning: missing code example in this documentation
   --> foo.rs:13:3
    |
    | /   fn neg(self) -> Self::Output {
    | |     Self::Output {
    | |       bar: -self.bar,
    | |       baz: -self.baz,
    | |     }
    | |   }
    | |___^

Meta

rustc --version --verbose:

rustc 1.48.0-nightly (73dc675b9 2020-09-06)
binary: rustc
commit-hash: 73dc675b9437c2a51a975a9f58cc66f05463c351
commit-date: 2020-09-06
host: x86_64-pc-windows-msvc
release: 1.48.0-nightly
LLVM version: 11.0

[jyn514]

cc @GuillaumeGomez

[GuillaumeGomez]

I think the current lint behavior is correct. The issue here is on the core lib side. I opened #76568 to fix it.

[jyn514]

Maybe the lint shouldn't warn when implementing a foreign trait? By the same rationale as #56922.

[GuillaumeGomez]

Hmmm... I have shared feelings here: if a trait doesn't provided documentation "by default", doesn't it make sense to complain about it on the implementors?

[jyn514]

I guess if you can add docs on the implementation it makes sense - you have a way to fix it.

[GuillaumeGomez]

That's mainly my position yes. :)

[skreborn]

I guess if you can add docs on the implementation it makes sense - you have a way to fix it.

Unfortunately, that drags with itself more issues. First of all, I have to implement said examples on all of the implementations I write. Second, I have to copy (and correctly maintain said copy) the original example-less documentation onto every instance so that it's not lost by the language server and/or rustdoc.

While this isn't entirely impossible, it's a huge chore, especially when you implement multiple traits on tens of types at a time. While std could get its issues fixed, that hardly helps us with the rest of the crate ecosystem. As of now, I have about 300 warnings, and actually useful information just drowns in that flood.

[GuillaumeGomez]

You can disable the lint on a specific implementation in the worst case. However, the goal is to enforce more documentation through the ecosystem. If users start to complain about it to the crates' maintainers, I'm pretty sure the situation will improve pretty quickly.

[jyn514]

However, the goal is to enforce more documentation through the ecosystem. If users start to complain about it to the crates' maintainers, I'm pretty sure the situation will improve pretty quickly

I don't really like this approach - it should be fixable by you, not by bugging upstream maintainers. @Manishearth was saying something like that for #74563 I think.

[GuillaumeGomez]

You can fix it on your side by adding the examples yourself in your implementation. So on that on point, even if not the best, it's still possible.

[skreborn]

I suppose in the worst case, I could fork the upstream project, add examples, and hope that it either gets merged or the developer doesn't cause too many conflicts so I can keep it up-to-date.

But even then, it takes a lot of back-and-forth, a lot of time and effort on both ends to get from my linter complaining about missing examples to a new crate release that fixes it - even if everything goes quick and smooth.

There should at least be a way for me to temporarily ignore warnings coming from (or rather because of) 3rd party and standard library crates, so that - at the minimum - I can fix my own linting issues without having to comb through thousands of lines of all too similar lines of warnings every time I generate documentation for testing purposes.

[jyn514]

There should at least be a way for me to temporarily ignore warnings

#[allow(missing_doc_code_examples)] on the implementation?

[GuillaumeGomez]

Like I said, you can ignore the lint on the implementations, or directly on a module if it only contains such cases. Adding a #[allow(...)] doesn't seem to be such a burden...

[skreborn]

Let it be known that I was today days old when I learned that you can apply #[allow(missing_doc_code_examples)] to a single impl block. I suppose that will have to suffice for now.

[Manishearth]

I strongly feel that we should not warn on foreign trait implementations here. Firstly, as @jyn514 said, a lint should be fixable by you. Yes, you can add examples yourself, but that isn't actually the real fix here in most cases. In most cases trait implementations are lightweight and you rarely document them, so you're not actually giving the user a "fix", you're asking them to do something they don't actually want to shut the lint up. That's not good, that is not the actual fix, the actual fix is asking people to fix it upstream, and we should not lint that downstream.

"The lint can be ignored" is not good enough for normal situations -- if it was some weird confluence of situations, then, sure, but a foreign trait impl is an extremely normal thing to do.

[GuillaumeGomez]

Coming back on this after a while and I now agree with this issue: we shouldn't lint on foreign trait implementations. I'll send a fix in the following days.

[GuillaumeGomez]

I just tried locally and it seems like this issue has already been fixed. Closing it then.

[jyn514]

@GuillaumeGomez we should still test that it behaves properly

[GuillaumeGomez]

I'll send a test if there isn't already one then.