Skip to content

Commit

Permalink
Impl Display rather than ToString (#663)
Browse files Browse the repository at this point in the history
Updated the 3 places where ToString impls were generated (in
`type_entry.rs`) and replaced them with an equivalent Display impl.
ToString is implemented for all types that implement Display and for
this reason the docs recommend not to implement ToString directly [0]
and is actually linted against by clippy in the default groups [1].

The snapshots were updated by running `EXPECTORATE=overwrite cargo test
--workspace` and then afterwards I did a brief manual verification that
the generated results seemed sensible.

Updated the README for `cargo-typify` to switch out the ToString impl
for the Display version. Since the output in the README is a simplified
version of the real output (the real output has a lot more fields and
noise), I manually added the Display impl in the style of the rest of
the example (nb. the Display impl does correctly appear in the real
output).

0: https://doc.rust-lang.org/std/string/trait.ToString.html
1: https://rust-lang.github.io/rust-clippy/master/index.html#/to_string_trait_impl
  • Loading branch information
louisdewar authored Sep 25, 2024
1 parent 261e1e0 commit 0ef2496
Show file tree
Hide file tree
Showing 15 changed files with 3,872 additions and 3,866 deletions.
8 changes: 4 additions & 4 deletions cargo-typify/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -104,11 +104,11 @@ impl std::convert::TryFrom<String> for IdOrName {
value.parse()
}
}
impl ToString for IdOrName {
fn to_string(&self) -> String {
impl ::std::fmt::Display for IdOrName {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
Self::Id(x) => x.to_string(),
Self::Name(x) => x.to_string(),
Self::Id(x) => x.fmt(f),
Self::Name(x) => x.fmt(f),
}
}
}
Expand Down
20 changes: 10 additions & 10 deletions typify-impl/src/type_entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -733,7 +733,7 @@ impl TypeEntry {
)
}

// ToString and FromStr impls for enums that are made exclusively of
// Display and FromStr impls for enums that are made exclusively of
// simple variants (and are not untagged).
let simple_enum_impl = bespoke_impls
.contains(&TypeEntryEnumImpl::AllSimpleVariants)
Expand All @@ -751,10 +751,10 @@ impl TypeEntry {
.unzip();

quote! {
impl ToString for #type_name {
fn to_string(&self) -> String {
impl ::std::fmt::Display for #type_name {
fn fmt(&self, f: &mut ::std::fmt::Formatter<'_>) -> ::std::fmt::Result {
match *self {
#(Self::#match_variants => #match_strs.to_string(),)*
#(Self::#match_variants => write!(f, #match_strs),)*
}
}
}
Expand Down Expand Up @@ -866,10 +866,10 @@ impl TypeEntry {
.map(|variant| format_ident!("{}", variant.name));

quote! {
impl ToString for #type_name {
fn to_string(&self) -> String {
impl ::std::fmt::Display for #type_name {
fn fmt(&self, f: &mut ::std::fmt::Formatter<'_>) -> ::std::fmt::Result {
match self {
#(Self::#variant_name(x) => x.to_string(),)*
#(Self::#variant_name(x) => x.fmt(f),)*
}
}
}
Expand Down Expand Up @@ -1314,9 +1314,9 @@ impl TypeEntry {
.has_impl(type_space, TypeSpaceImpl::Display)
.then(|| {
quote! {
impl ToString for #type_name {
fn to_string(&self) -> String {
self.0.to_string()
impl ::std::fmt::Display for #type_name {
fn fmt(&self, f: &mut ::std::fmt::Formatter<'_>) -> ::std::fmt::Result {
self.0.fmt(f)
}
}
}
Expand Down
10 changes: 5 additions & 5 deletions typify-impl/tests/generator.out
Original file line number Diff line number Diff line change
Expand Up @@ -185,12 +185,12 @@ mod types {
value.clone()
}
}
impl ToString for StringEnum {
fn to_string(&self) -> String {
impl ::std::fmt::Display for StringEnum {
fn fmt(&self, f: &mut ::std::fmt::Formatter<'_>) -> ::std::fmt::Result {
match *self {
Self::One => "One".to_string(),
Self::Two => "Two".to_string(),
Self::BuckleMyShoe => "BuckleMyShoe".to_string(),
Self::One => write!(f, "One"),
Self::Two => write!(f, "Two"),
Self::BuckleMyShoe => write!(f, "BuckleMyShoe"),
}
}
}
Expand Down
Loading

0 comments on commit 0ef2496

Please sign in to comment.