NullPointerException raised when attempting conversion on null string

[umbreak]

I have a type defined as: type Some = String Refined MatchesRegex[W.`"[a-zA-Z_][a-zA-Z0-9-_.]*"`.T]

and then I try to get this type back from a string:

//string is a val coming from a call to a java method, which can return null
RefType.applyRef[Some](string)

The result is the following exception on runtime:

java.lang.NullPointerException:
at eu.timepit.refined.StringValidate.$anonfun$matchesRegexValidate$1(string.scala:129)
at eu.timepit.refined.StringValidate.$anonfun$matchesRegexValidate$1$adapted(string.scala:129)
at eu.timepit.refined.api.Validate$$anon$3.validate(Validate.scala:79)
at eu.timepit.refined.internal.RefinePartiallyApplied.apply(RefinePartiallyApplied.scala:15)
at eu.timepit.refined.internal.ApplyRefPartiallyApplied.apply(ApplyRefPartiallyApplied.scala:19)
...

Since applyRef is returning a Either[String, Some], shouldn't that type of error (the string being null) return a Left(Not allowed empty string)?

[jan0sch]

Hmm... The same also happens when using NonEmpty.

import eu.timepit.refined._
import eu.timepit.refined.api._
import eu.timepit.refined.boolean._
import eu.timepit.refined.collection._
import eu.timepit.refined.numeric._
import eu.timepit.refined.string._

@ type SomeString = String Refined MatchesRegex[W.`"[a-zA-Z_][a-zA-Z0-9-_.]*"`.T] 
defined type SomeString

@ type SomeNonEmptyString = String Refined And[NonEmpty, MatchesRegex[W.`"[a-zA-Z_][a-zA-Z0-9-_.]*"`.T]] 
defined type SomeNonEmptyString

@ val n: String = null 
n: String = null

@ RefType.applyRef[SomeString](n) 
java.lang.NullPointerException
  eu.timepit.refined.StringValidate.$anonfun$matchesRegexValidate$1(string.scala:129)
  eu.timepit.refined.StringValidate.$anonfun$matchesRegexValidate$1$adapted(string.scala:129)
  eu.timepit.refined.api.Validate$$anon$3.validate(Validate.scala:79)
  eu.timepit.refined.internal.RefinePartiallyApplied.apply(RefinePartiallyApplied.scala:15)
  eu.timepit.refined.internal.ApplyRefPartiallyApplied.apply(ApplyRefPartiallyApplied.scala:19)
  ammonite.$sess.cmd9$.<init>(cmd9.sc:1)
  ammonite.$sess.cmd9$.<clinit>(cmd9.sc)

@ type SomeNonEmptyString = String Refined And[NonEmpty, MatchesRegex[W.`"[a-zA-Z_][a-zA-Z0-9-_.]*"`.T]] 
defined type SomeNonEmptyString

@ RefType.applyRef[SomeNonEmptyString](n) 
java.lang.NullPointerException
  eu.timepit.refined.CollectionValidate.$anonfun$emptyValidate$1(collection.scala:125)
  eu.timepit.refined.CollectionValidate.$anonfun$emptyValidate$1$adapted(collection.scala:125)
  eu.timepit.refined.api.Validate$$anon$3.validate(Validate.scala:79)
  eu.timepit.refined.BooleanValidate$$anon$1.validate(boolean.scala:61)
  eu.timepit.refined.BooleanValidate$$anon$2.validate(boolean.scala:86)
  eu.timepit.refined.internal.RefinePartiallyApplied.apply(RefinePartiallyApplied.scala:15)
  eu.timepit.refined.internal.ApplyRefPartiallyApplied.apply(ApplyRefPartiallyApplied.scala:19)
  ammonite.$sess.cmd11$.<init>(cmd11.sc:1)
  ammonite.$sess.cmd11$.<clinit>(cmd11.sc)

@fthomas I would like to take a look at this. Any recommendations for me?

[umbreak]

The matches method can throw:

• NullPointerException when the passed string is null
• java.util.regex.PatternSyntaxException when the regular expression's syntax is invalid

Those two cases should be handled on the code and return a Left.

[jan0sch]

My approach is to generally fail a predicate if the given value "is null". This works for the matches stuff:

scala> type MString = String Refined MatchesRegex[W.`"[a-zA-Z_][a-zA-Z0-9-_.]*"`.T]
defined type alias MString

scala> val s: String = null
s: String = null

scala> RefType.applyRef[MString](s)
res0: Either[String,MString] = Left(Predicate failed: "null".matches("[a-zA-Z_][a-zA-Z0-9-_.]*").)

However this also has consequences for other predicates like NonEmpty:

scala> type NString = String Refined NonEmpty
defined type alias NString

scala> val s: String = null
s: String = null

scala> RefType.applyRef[NString](s)
java.lang.NullPointerException
  at eu.timepit.refined.api.Refined$.toString$extension(Refined.scala:11)
  at eu.timepit.refined.api.Refined.toString(Refined.scala:11)
  at java.lang.String.valueOf(String.java:2994)

The exception can be avoided by fixing the value.toString line in Refined.scala: value.toString -> if (value == null) "NULL" else value.toString

But then this happens for NonEmpty:

scala> RefType.applyRef[NString](s)
res0: Either[String,NString] = Right(NULL)

@fthomas Any suggestions how we should proceed from here? It would be cool if we could avoid handling the null case in each and every checker on a lower level.

[jan0sch]

After giving this some more thoughts I propose to not handle this within refined.

Reasoning as follows:

1. Automatic negation of predicates in the case of null breaks the usage of Not and would need more work in several places to check for the null case. This complicates the internal logic and also the code base.
2. Checking for null should be the job of the developer. These errors usually appear when data from the "outside" is not properly checked at runtime. When using things like refined-circe this can be circumvented by using proper fields like Option[T] instead of T for possible null values.

It would be convenient if refined handled these cases but as mentioned it would complicate things.

@fthomas Any more thoughts on this or can the issue be closed?

[hseeberger]

I totally agree with @jan0sch. Dealing with potential null values is not a concern of this library. I would consider secretly dealing with null very counter intuitive.

[fthomas]

I agree with @jan0sch and @hseeberger. Null checking is not the concern of refined and as @jan0sch demonstrated it would add complexity which I would rather avoid.

That being said, in some cases refined accidentally handles null values:

scala> val s: String = null
s: String = null

scala> refineV[Regex](s)
res4: Either[String, Refined[String, Regex]] = Left(Regex predicate failed: null)

scala> refineV[Not[Regex]](s)
java.lang.NullPointerException
  at eu.timepit.refined.api.Refined$.toString$extension(Refined.scala:11)

This is because the Validate[String, Regex] instance is implemented via Validate.fromPartial. I'd say this is unfortunate but it is not a reason to add explicit null checks.