Feature request: Add runtime exception check instead of error "compile-time refinement only works with literals"

[soronpo]

Hi, I'm posting this feature request after a small e-mail exchange with @fthomas

The code

for (i <-  -1 to 3)
  println(refineMV[Positive](i))

currently results in compile-time refinement only works with literals error.

I suggest that instead this will result in a runtime check, meaning four possible cases:

1. Compile time check pass for literal (no runtime check required)

     println(refineMV[Positive](1))
   

   Prints outs: 1

2. Compile time check fail for literal

     println(refineMV[Positive](0))
   

3. Compile time check could not run, so running runtime check instead, which passed

   for (i <-  1 to 3)
     println(refineMV[Positive](i)) 
   

   Prints outs: 1 2 3

4. Compile time check could not run, so running runtime check instead, which failed and throws exception

   for (i <-  -1 to 3)
     println(refineMV[Positive](i))  //Throws exception
   

Note: Instead of modifying refineMV you may wish to keep it as is, and add another flavor (something like refineMVRE [Macro Verify, Runtime Exception], or whatever acronym you wish). However, I don't think you should. Keep it simple with the least functions possible.

Thanks, Oron

[jvliwanag]

But the point of refineMV, and in fact, refined as a library is that it should guarantee at compile time that the values at play are guaranteed to follow the specified constraints. That is, they won't blow up possibly resulting in an exception.

For values that you get at runtime and you'd want to check if it follows the specified constraints, you'd want to use refineV instead which returns an Either. This returns either a Right if it safely passes the constraints, and Left if it fails. Again, no throwing of exceptions.

When working with a good scala library, you'd typically expect it not to throw exceptions. Rather, error cases, you'd expect it to be fully spec'd out on the types. Such as the case is in refineV returning Either.

[soronpo]

Think about a library with a function def Foo(num : Positive) : Positive (where Positiveis an alias for Int Refined Positive). As a user of such library it is very clear to me that Foo(1) will run correctly and return a value. But what if I want to run it with a loop?

for (i <- 1 to 3)
  Foo(i)   // error: compile-time refinement only works with literals error

So now I need to have separate FooNonLiteral() function, or skip the compile time check completely. To me, my suggestion makes more sense. Yes, it is not a good practice for a library to throw exception in general, but in its current condition refineMV is not usable in every case, which is a shame.

For my use case, I'm creating a DSL (domain specific language) with Scala (and hopefully refined). The earlier I catch user errors, the better. So I need refineMVfor compile time check, but if this forces me to not be able to use it for runtime non-literal variables, then refined has no use for me.

[fthomas]

my tl;dr:

• This is an interesting idea because it is a safer variant of unsafeFrom.
• We should not change the semantics of refineMV.
• By default all functions in refined should be referentially transparent and total (not throw exceptions) unless their name contains unsafe.

The library already contains the unsafeFrom function that refines values if they satisfy the constraint or throw an exception. This check always happen at runtime, even if a compile-time constant is passed to it. With @soronpo's proposal unsafeFrom could be improved to check literals at compile-time. I think this could be done without loss of its generality.

I agree with @jvliwanag that the purpose of refined is to have safer code and by default it should not use exceptions if not explicitly documented or apparent from the function names. Changing the semantics of refineMV could do more harm than good. It could mask programmer errors that only unveil at runtime.

So I think a safer unsafeFrom could be good compromise between @soronpo's idea and @jvliwanag concerns. What do you think?

Btw, the usage of unsafeFrom currently looks like this:

scala> refineV[Positive].unsafeFrom(5)
res0: Refined[Int,Positive] = 5

scala> refineV[Positive].unsafeFrom(-5)
java.lang.IllegalArgumentException: Predicate failed: (-5 > 0).

[soronpo]

Sounds reasonable. Just be sure that if passed compiler check, then runtime check would not run.

[fthomas]

I started working on this here: https://github.com/fthomas/refined/compare/topic/unsafeFrom2

[AndreasKostler]

@fthomas Any updates on this?

[fthomas]

@AndreasKostler Currently I've little time to work on this. But in principle I'm still okay with changing unsafeFrom as outlined in https://github.com/fthomas/refined/issues/188#issuecomment-232068707. The only downside of this approach I'm aware of, is that unsafeFrom can't be eta expanded (i.e. unsafeFrom _) since it would be a macro. Macros in general can't be eta expanded.

[ysusuk]

Hi guys, i know you had this discussion, but it will be really cool to have a functionality that do a compile time check and falls back to runtime if compile time failed! Here is my example:

type BucketNamePredicate = And[MinSize[W.`3`.T], MaxSize[W.`37`.T]]
type BucketName = String Refined BucketNamePredicate

def createBucketT(bucketName: BucketName) ...

sometimes i do have bucketName as literal but sometimes i want to get it from config. And i want still to use Refined type in method signature, since it clearly describes the requirements to the buckeName.

Or do you think that there is a way to model this with the solution you proposing?

[soronpo]

@ysusuk I created a solution using the singleton-ops library, which is called TwoFace and Checked values. You can read about it in the following link: https://contributors.scala-lang.org/t/twoface-values-closing-the-gap-between-run-compile-time-functionality/869

[fthomas]

@ysusuk If we change unsafeFrom as outlined in https://github.com/fthomas/refined/issues/188#issuecomment-232068707, you could invoke createBucketT with createBucketT(unsafeFrom(x)) where x is of type String and it would perform a compile-time check if x is a literal and a runtime check if it is not. Would this work for you?

The only reason this is still not implemented is my sparse free time. If someone else want to pick this up, that would be great.

[ysusuk]

yeah, that would indeed work for me. for the moment i did that workaround

type BucketNamePredicate = And[MinSize[W.`3`.T], MaxSize[W.`37`.T]]
type BucketName = String Refined BucketNamePredicate

implicit class StringOps(str: String) {
  def bucketName: BucketName =
    Refined.unsafeApply[String, BucketNamePredicate](str)

so i do have nice description of BucketName requirements, but only runtime checks then, which is a good tradeoff, if i'm reading values from configs.

So i will need just to use unsafeApply2 instead of unsafeApply

[ysusuk]

@fthomas what help do you need with https://github.com/fthomas/refined/compare/topic/unsafeFrom2?

[ysusuk]

 @fthomas i'm trying to make this one https://github.com/fthomas/refined/compare/topic/unsafeFrom2 work