derive_more icon indicating copy to clipboard operation
derive_more copied to clipboard

more `FromStr` attributes

Open dotcarmen opened this issue 6 months ago • 5 comments

#102 is similar, but I'm using this issue to expand and propose the design for a PR I'm writing:

the FromStr derive as-is still doesn't capture all use cases, so I'd like to propose some more attributes, based on serde's available attributes:

  • #[from_str(rename = "...")]
    • available for unit struct container or enum variants
    • incompatible with rename_all on the same container/variant
    • changes the string value from the one derived from the container/variant ident to the string literal value
  • #[from_str(default)]
    • only valid for exactly 1 enum variant
    • incompatible with rename_all and rename on the same variant
    • this variant is returned only when no other variant is successfully parsed
    • changes FromStr::Error to be Infallible
  • #[from_str(forward)] (aka #[serde(transparent)])
    • only valid for enum variants
      • this could also be used for allowing newtype structs
    • forwards the FromStr implementation to the field type
    • only allows for a single inner tuple type (eg Foo(Inner))
  • #[from_str(alias = "...")]
    • available for unit struct container or enum variants
    • allows for matches of the given string literal to result in construction of the container/variant
    • incompatible with forward and default
    • rename_all case conversions don't affect this value
    • rename is not required to use this attribute
  • #[from_str(skip)]
    • only available for enum variants
    • skips any string from matching this variant
    • incompatible with all other attributes

dotcarmen avatar Jun 03 '25 14:06 dotcarmen

I wanted to implement from_str(forward) i.e. split it from #482 I was wondering if it made sense to make this the default behavior instead?

I don't think we will ever support parsing both the variant's name and the inner field so it would be reasonable to expect

#[test]
fn test() {
  use derive_more::FromStr;

  #[derive(FromStr)]
  enum Enum {
    A,
    B,
    Number(u8)
  }

  assert_eq!(Enum::from_str("A").unwrap(), Enum::A);
  assert_eq!(Enum::from_str("2").unwrap(), Enum::Number(2));
  assert(Enum::from_str("Number").is_err());

A more general question with this kind of "forward" or "transparent" behavior is, what should happen if multiple new-type enum variants exists? I think the following would be mostly the expected behavior, but it would require us to try every new-type variant until one matches (Also an issue for error messages if none match) and I am not sure what the best bahavior in case of Char vs B is, both match and Char is first so one could expect it to be parsed, but B is a literal enum variant and we might want to give those higher precedence.

#[test]
fn test() {
  use derive_more::FromStr;

  #[derive(FromStr)]
  enum Enum {
    A,
    Number(u8),
    Char(char),
    B
  }

  assert_eq!(Enum::from_str("A").unwrap(), Enum::A);
  assert_eq!(Enum::from_str("2").unwrap(), Enum::Number(2));
  assert_eq!(Enum::from_str("c").unwrap(), Enum::Char('c'));
  assert_eq!(Enum::from_str("B").unwrap(), Enum::Char('B')); // or should this be `Enum::B`

What are your thoughts @JelteF?

We could also as an initial version only support a single non literal variant and define it as a singular other and first try to match all regular enum variants before parsing it.

This would, without any breaking changes, be able to be expanded upon in the future.

ModProg avatar Sep 05 '25 15:09 ModProg

@ModProg @JelteF I think that the reasonable solution is to:

  1. First consider unit variants like they're considered now.
  2. Consider any variants with fields in their declaration order.
#[derive(FromStr)]
enum Enum {
    A,
    Number(u8),
    Char(char),
    Other(String),
    B,
}

assert_eq!(Enum::from_str("A").unwrap(), Enum::A);
assert_eq!(Enum::from_str("2").unwrap(), Enum::Number(2));
assert_eq!(Enum::from_str("c").unwrap(), Enum::Char('c'));
assert_eq!(Enum::from_str("whatever").unwrap(), Enum::Other("whatever".into()));
assert_eq!(Enum::from_str("B").unwrap(), Enum::B);

However:

#[derive(FromStr)]
enum Enum {
    A,
    Other(String),
    Number(u8),
    Char(char),
    B,
}

assert_eq!(Enum::from_str("A").unwrap(), Enum::A);
assert_eq!(Enum::from_str("2").unwrap(), Enum::Other("2".into()));
assert_eq!(Enum::from_str("c").unwrap(), Enum::Other("c".into()));
assert_eq!(Enum::from_str("whatever").unwrap(), Enum::Other("whatever".into()));
assert_eq!(Enum::from_str("B").unwrap(), Enum::B);

I don't think that we need an explicit #[from_str(forward)] attribute too.

tyranron avatar Sep 10 '25 13:09 tyranron

A useful point would be to (have a way to) unify the annotations with derive_more::Display - so it roundtrips (ie. something.to_string().parse::<Something>() is same as the original).

It's not necessarily a must, but quoting the docs:

However, if a type has a lossless Display implementation whose output is meant to be conveniently machine-parseable and not just meant for human consumption, then the type may wish to accept the same format in FromStr, and document that usage. Having both Display and FromStr implementations where the result of Display cannot be parsed with FromStr may surprise users.

utdemir avatar Sep 24 '25 21:09 utdemir

One issue we'd have there is the ability to map multiple values to the same "to string" representation. Though we could probably figure this out and throw a compilation error, we will probably need to completely skip "fmt" as that'd be quite difficult to implement for parsing.

It'll still make sense to have separate attributes and have a shared one additionally that still needs a name, that only supports rename, and rename all.

ModProg avatar Oct 01 '25 11:10 ModProg

#[from_str(rename = "...")]

I'm noticing a lack of this feature block my use case where I'd Enum::from("foo_bar") == Enum::FooBar.

tisonkun avatar Oct 29 '25 04:10 tisonkun