Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LSP code action: generate dynamic decoder #2955

Open
lpil opened this issue Apr 11, 2024 · 10 comments · May be fixed by #4106
Open

LSP code action: generate dynamic decoder #2955

lpil opened this issue Apr 11, 2024 · 10 comments · May be fixed by #4106
Labels
help wanted Contributions encouraged priority:medium

Comments

@lpil
Copy link
Member

lpil commented Apr 11, 2024

We need to decide what the recommendation is for constructors with too many fields for the decodeN functions. Perhaps we can not support them for this first version.

We also need to decide what to do for contained types we don't know the decoder for. Perhaps we can add a todo as "..." for now.

@lpil lpil added help wanted Contributions encouraged priority:medium labels Apr 11, 2024
@quentin-fox
Copy link

It could be nice to have two separate code actions for this? One to have a decodeN, which would fail if there were too many arguments for the constructor, and another to generate an inline decoder.

When writing my own inline decoders for constructors with 10+ arguments, I had to copy the all_errors implementation from the gleam decode stdlib into my own application.

fn all_errors(result: Result(a, List(DecodeError))) -> List(DecodeError) {
  case result {
    Ok(_) -> []
    Error(errors) -> errors
  }
}

When I first encountered this, I thought this would 100% be worth adding to the gleam/result package as all_errors as something like this:

pub fn all_errors(result: Result(a, List(b))) -> List(b) {
  case result {
    Ok(_) -> []
    Error(errors) -> errors
  }
}

If we did want to generate an in-line decoder for 10+ fields, then instead of having to copy this function, it could just be imported from the standard library.

@lpil
Copy link
Member Author

lpil commented Apr 22, 2024

I'd like us to have some recommendation for how to implement decoders of unbounded size prior to having a code generator for that, and I'd like to have one action rather than 2 as there should only ever be one recommended way of doing something, and the programmer may not know which option to pick.

@GearsDatapacks
Copy link
Member

Now that the new decoder API has been merged into the stdlib, I'm happy to work on this. I have a few questions regarding how this should work. Mainly for Louis to answer, but anyone is free to chime in with their opinion.

  1. Where should we offer this code action? Over the entire type definition, just the "header" (type Name(...)), or just the name?
  2. As mentioned above, we can't know what function to use to decode other custom types. We also can't know what function to use to decode type parameters. I guess we can treat those the same? We could also add parameters to the function, which are the decoders to decode each type parameter. That is probably what will need to be done by the user anyway, as you can't write a decoder to generically decode data.
  3. How do we generate decoders for types with more than one variant? I haven't done a lot of dynamic decoding so I don't know if there is some convention to denote which variant some dynamic data is, or just put some placeholder code there?
  4. What should the function it generates look like? I assume it will be an fn(dynamic.Dynamic) -> Result(TheType, List(decode.DecodeError))? (Should it be public by default or private by default?)

I've written an example type with all of the above edge-cases. If we can figure out exactly what the LSP should generate for it, then that will give me a comprehensive idea of what I have to do

/// What does the decoder for this type look like?
pub type Wibble(value, phantom) {
  Wibble(counter: Int, values: List(value), next: Wibble(value, phantom))
  Wobble(message: option.Option(String), something_custom: OtherType)
}

// This could be anything
pub type OtherType {
  OtherType(Int)
}

@GearsDatapacks
Copy link
Member

GearsDatapacks commented Dec 22, 2024

Currently, I'm thinking the above type could generate a function like this:

pub fn decode_wibble(
  dynamic: dynamic.Dynamic,
  value_decoder: decode.Decoder(value),
) -> Result(Wibble(value, phantom), List(decode.DecodeError)) {
  let wibble_decoder = {
    use counter <- decode.field("counter", decode.int)
    use values <- decode.field("values", decode.list(value_decoder))
    use next <- decode.field("next", todo as "Decoder for Wibble")
    decode.success(Wibble(counter:, values:, next:))
  }

  let wobble_decoder = {
    use message <- decode.field("message", decode.optional(decode.string))
    use something_custom <- decode.field(
      "something_custom",
      todo as "Decoder for OtherType",
    )
    decode.success(Wobble(message:, something_custom:))
  }

  decode.run(dynamic, decode.one_of(wibble_decoder, [wobble_decoder]))
}

This is currently missing:

  • A way for the type to decode itself recursively. Perhaps we want to just generate an fn() -> Decoder(TheType) instead of a function which actually runs the decoder.
  • A proper way to differentiate between the Wibble and Wobble variants

@lpil
Copy link
Member Author

lpil commented Dec 22, 2024

Where should we offer this code action? Over the entire type definition, just the "header" (type Name(...)), or just the name?

The header sounds good to me.

As mentioned above, we can't know what function to use to decode other custom types. We also can't know what function to use to decode type parameters. I guess we can treat those the same? We could also add parameters to the function, which are the decoders to decode each type parameter. That is probably what will need to be done by the user anyway, as you can't write a decoder to generically decode data.

We could insert todo for unknown field types for now?

How do we generate decoders for types with more than one variant? I haven't done a lot of dynamic decoding so I don't know if there is some convention to denote which variant some dynamic data is, or just put some placeholder code there?

Let's not support this in the first version.

What should the function it generates look like? I assume it will be an fn(dynamic.Dynamic) -> Result(TheType, List(decode.DecodeError))? (Should it be public by default or private by default?)

A private function that returns decode.Decoder(t) please

@GearsDatapacks
Copy link
Member

Ok, that sounds good to me. I'll get implementing

@lpil
Copy link
Member Author

lpil commented Dec 22, 2024

Thank you!

@GearsDatapacks
Copy link
Member

Oh and one more thing, how do we generate a decoder for a type with unnamed fields? I'm not really sure how that would be represented.

@lpil
Copy link
Member Author

lpil commented Dec 22, 2024

Let's not offer it for them. The aim isn't to generate decoders for the actual Gleam representation of these types, it's for generating the decoders folks likely want to use them for, which will probably be used with JSON.

@GearsDatapacks
Copy link
Member

Great, thanks

@GearsDatapacks GearsDatapacks linked a pull request Dec 23, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Contributions encouraged priority:medium
Projects
Status: Unfinished
Development

Successfully merging a pull request may close this issue.

3 participants