kitlangton / neotype

A friendly newtype library for Scala 3
Apache License 2.0
158 stars 16 forks source link

Should make be non-final? #61

Open amumurst opened 3 months ago

amumurst commented 3 months ago

Hey, thanks for the awesome lib!

I am in the process of migrating to it and have ran into a couple validations we have that can't be checked at compiletime (pretty nasty ones as in 100+ lines of code).

Rather than trying to chase macro issues I figured that I would just like to have an escape hatch to say "ok this thing wont work with just providing compiletime strings, but you still get all the nice decoder/encoders automatically having made sure they are validated at runtime"

For a simple example of something that does not work:

type PastYear = PastYear.Type
object PastYear extends neotype.Newtype[Int]:
  override inline def validate(input: Int) =
    val thisYear = java.time.LocalDate.now().getYear
    input < thisYear

val x = PastYear(2023) //crashes compiler

What I did locally for now was to fork and publish locally a version where the make function is not final. That allowed me to produce this instead

type PastYear = PastYear.Type
object PastYear extends neotype.Newtype[Int]:
  override inline def validate(input: String): String | Boolean =
      s"PastYear can not be compile time checked due to macro limits, use make(input) instead of apply and handle error"
  override def make(input: Int): Either[String, PastYear] =
    val thisYear = java.time.LocalDate.now().getYear
    if(input < thisYear) Right(unsafeMake(input)) else Left(s"$input is in the past")

val x = PastYear(2023) //still crashes compiler but with a nicer error message

but now I can still use it by PastYear.make(2023) and handle the error

This would also allow make to change the input, for instance cleaning up a string:

type Name = Name.Type
object Name extends neotype.Newtype[String]:
   override def make(input: String): Either[String, Name] = 
      Right(unsafeMake(input.trim))

Open for other suggestions on how this could be achieved :-)

kitlangton commented 3 months ago

Firstly, thanks for the issue. I appreciate it! 🥰

I do aim to support evaluating as much as possible at compile-time. So if you're able to share any of your mega validation function, I could get to work patching up the holes—of course, I completely understand if you're unable to share that! I probably don't support the Java time constructs yet, so I'll add that ASAP. I'm actually considering extracting the compile-time evaluation code out into a separate library, so it could be used for other shenanigans.

s"PastYear can not be compile time checked due to macro limits, use make(input) instead of apply and handle error"

Perhaps another approach would be adding an annotation (e.g., @runtimeOnly) indicating that the validate method isn't expected to be executed at compile-time. This could simply output the message you've written there, and look less like a bug than the current behavior.

This would also allow make to change the input, for instance cleaning up a string:

Ha! I didn't even consider that use case, but it's a great one. Ideally, this transformation should be possible at both compile time and run time. I could change the validate method to also allow for an Either[String, A] as the return value. This way, it could either fail with an error or successfully map the input to a new value.

It's already a bit unusual that the API involves a union type, and adding more options could make it even more confusing. Perhaps it could just be Boolean | Either[String, A] versus Boolean | String, so Right(value) would allow you to transform the value?

Ideally, we could keep the API minimal while supporting as many use cases as possible. Opting out of compile-time validation should be a relatively rare requirement, so perhaps adding an annotation for that might be best, as users would need to read the documentation to know it exists. If we allow the make method to be overridable, it could be confusing, as it introduces more ways to implement a valid new type.

Anyhow! I hope that all makes sense 😄 Let me know what you think!

kitlangton commented 3 months ago

I think I'm overcomplicating this w/ the whole Union type thing. It was probably too clever to begin with.

What if validate (perhaps a new method name) returns an Either[String, A]. That'll support every use case: custom error messages + fallible transformations.

The only downside is that it makes it slightly more awkward for the simple cases. Instead of input.nonEmpty, you'd have to write if input.nonEmpty then Right(input) else Left("Input must be non-empty"). But it's probably worth it.