refined
refined copied to clipboard
NullPointerException raised when attempting conversion on null string
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)
?
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?
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.
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.
After giving this some more thoughts I propose to not handle this within refined.
Reasoning as follows:
- Automatic negation of predicates in the case of
null
breaks the usage ofNot
and would need more work in several places to check for thenull
case. This complicates the internal logic and also the code base. - 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 likeOption[T]
instead ofT
for possiblenull
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?
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.
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.