implicitly require fmt::Display on all type parameters unless overridden

[cosmicexplorer]

Problem

I love this crate!! I wanted to use it on an enum struct with a type parameter but I ran into the Problem in #19!

Solution

Adding the Display impl

I didn't take a look at derive_more as mentioned in #19; instead, I noticed that we are not modifying struct or enum definitions to implicitly require fmt::Display, but that our own fmt::Display impl was unable to infer it for arguments which depended on a type parameter. For example, the following fails to compile right now:

use displaydoc::Display;

/// oh no, an error: {0}
#[derive(Display)]
pub struct Error<E>(pub E);

let error: Error<&str> = Error("muahaha i am an error");
assert!("oh no, an error: muahaha i am an error" == &format!("{}", error));

Producing:

error[E0599]: the method `__displaydoc_display` exists for reference `&E`, but its trait bounds were not satisfied
 --> src/lib.rs:41:1
  |
6 | /// oh no, an error: {0}
  | ^^^^^^^^^^^^^^^^^^^^^^^^ method cannot be called on `&E` due to unsatisfied trait bounds
  |
  = note: the following trait bounds were not satisfied:
          `E: std::fmt::Display`
          which is required by `&E: DisplayToDisplayDoc`

So I basically just tried to hack it so that where E: core::fmt::Display would be written where we generate our Display impl.

Result

The benefit of this change is that it lets an enum deriving displaydoc::Display become usable as a generic parameterized result type for a generic method without making that enum have to carry the where _: Display, as demonstrated below. This behavior is very similar to the built-in #[derive(...)] macros for Debug, PartialEq, Hash, etc:

  /// Each individual element that can be matched against some input in a case.
  #[derive(Debug, Display, Copy, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
  pub enum CaseElement<Lit, PR> {
    /// literal value {0}
    Lit(Lit),
    /// production reference {0}
    Prod(PR),
  }

[cosmicexplorer]

Realized the "shoddy workaround" would break existing code, so was thinking about changing this to not adding the where _: Display clause if there are any predicate clauses for that type parameter already, and not try to scan for "Display" specifically. Will try to implement that now.

[cosmicexplorer]

Done!

[yaahc]

This is amazing Danny, thank you for this PR, particularly the comments which made reviewing this a breeze.

I love this crate!!

This brings me joy 😄

[yaahc]

Hey @cosmicexplorer, are you still interested in landing this change?

[cosmicexplorer]

Yes! I ended up moving across the country very soon after providing this first draft and there's absolutely no reason I should have put this off as it remains useful to me! Upon finally mustering the courage to check over your comments just now I'm seeing that I need to think a little harder about where constraints e.g. where crate::Type: Display as you mentioned! I seem to recall that part was very much a first draft and I wish I'd written down my thinking in comments at the time!

Sorry for making this drag out so long, I will respond to the rest of your comments by tomorrow. Thanks for your patience!

[yaahc]

Sorry for making this drag out so long, I will respond to the rest of your comments by tomorrow. Thanks for your patience!

No problem at all, take your time Danny.

[yaahc]

Gonna bump the MSRV to 1.45 to make mixed_site compile then I'll restart the CI.

[yaahc]

Thank you again!