#2475 (Bug Fixes) Nullable Types

SlimerDude Thu 24 Sep 2015

Here's a patch that fixes the quirks around Type.fits() when nullable generic types are involved. (I do a lot of reflection and this keeps biting me in the proverbial.)

This fixes the bugs in Java and JS as mentioned in:

  • `http://fantom.org/forum/topic/2256`
  • `http://fantom.org/forum/topic/2453`

and brings the two runtimes into alignment.

diff -r fd89993adbdd src/sys/java/fan/sys/FuncType.java
--- a/src/sys/java/fan/sys/FuncType.java	Wed Sep 23 17:17:28 2015 -0400
+++ b/src/sys/java/fan/sys/FuncType.java	Fri Sep 25 01:03:00 2015 +0100
@@ -83,6 +83,10 @@
   public boolean is(Type type)
   {
     if (this == type) return true;
+    if (type instanceof NullableType)
+    {
+      type = ((NullableType) type).root;
+    }
     if (type instanceof FuncType)
     {
       FuncType t = (FuncType)type;
diff -r fd89993adbdd src/sys/java/fan/sys/ListType.java
--- a/src/sys/java/fan/sys/ListType.java	Wed Sep 23 17:17:28 2015 -0400
+++ b/src/sys/java/fan/sys/ListType.java	Fri Sep 25 01:03:00 2015 +0100
@@ -57,6 +57,10 @@
 
   public boolean is(Type type)
   {
+    if (type instanceof NullableType)
+    {
+      type = ((NullableType) type).root;
+    }
     if (type instanceof ListType)
     {
       ListType t = (ListType)type;
diff -r fd89993adbdd src/sys/java/fan/sys/MapType.java
--- a/src/sys/java/fan/sys/MapType.java	Wed Sep 23 17:17:28 2015 -0400
+++ b/src/sys/java/fan/sys/MapType.java	Fri Sep 25 01:03:00 2015 +0100
@@ -62,6 +62,10 @@
 
   public boolean is(Type type)
   {
+    if (type instanceof NullableType)
+    {
+      type = ((NullableType) type).root;
+    }
     if (type instanceof MapType)
     {
       MapType t = (MapType)type;
diff -r fd89993adbdd src/sys/js/fan/Type.js
--- a/src/sys/js/fan/Type.js	Wed Sep 23 17:17:28 2015 -0400
+++ b/src/sys/js/fan/Type.js	Fri Sep 25 01:03:00 2015 +0100
@@ -759,9 +759,12 @@
 
 fan.sys.ListType.prototype.is = function(that)
 {
+  if (that instanceof fan.sys.NullableType)
+    that = that.m_root;
+
   if (that instanceof fan.sys.ListType)
   {
-    if (that.v.qname() == "sys::Obj") return true;
+    if (this.v.qname() == "sys::Obj") return true;
     return this.v.is(that.v);
   }
   if (that instanceof fan.sys.Type)
@@ -848,11 +851,13 @@
 
 fan.sys.MapType.prototype.is = function(that)
 {
-  if (that.isNullable()) that = that.m_root;
+  if (that instanceof fan.sys.NullableType)
+    that = that.m_root;
 
   if (that instanceof fan.sys.MapType)
   {
-    return this.k.is(that.k) && this.v.is(that.v);
+    return (this.k.qname() == "sys::Obj" || this.k.is(that.k)) &&
+           (this.v.qname() == "sys::Obj" || this.v.is(that.v));
   }
   if (that instanceof fan.sys.Type)
   {
@@ -960,6 +965,10 @@
 fan.sys.FuncType.prototype.is = function(that)
 {
   if (this == that) return true;
+
+  if (that instanceof fan.sys.NullableType)
+    that = that.m_root;
+
   if (that instanceof fan.sys.FuncType)
   {
     // match return type (if void is needed, anything matches)

Some unit tests that exercise all bases:

diff -r fd89993adbdd src/testSys/fan/TypeTest.fan
--- a/src/testSys/fan/TypeTest.fan	Wed Sep 23 17:17:28 2015 -0400
+++ b/src/testSys/fan/TypeTest.fan	Fri Sep 25 00:58:54 2015 +0100
@@ -174,6 +174,49 @@
     // void doesn't fit anything
     verifyFalse(Void#.fits(Obj#))
     verifyFalse(Obj#.fits(Void#))
+
+    // Nullability checks and bug fixes - see http://fantom.org/forum/topic/2256
+    verify(Int#.fits(Int?#))
+    verify(Int?#.fits(Int#))
+
+    verify(Int[]#.fits(Int[]?#))  // bug fix
+    verify(Int[]?#.fits(Int[]#))
+
+    verify(Int[]#.fits(Int?[]#))
+    verify(Int?[]#.fits(Int[]#))
+
+    verify([Int:Int]#.fits([Int:Int]?#))  // bug fix
+    verify([Int:Int]?#.fits([Int:Int]#))
+
+    verify([Int:Int]#.fits([Int:Int?]#))
+    verify([Int:Int?]#.fits([Int:Int]#))
+
+    verify(|Int|#.fits(|Int|?#))  // bug fix
+    verify(|Int|?#.fits(|Int|#))
+
+    verify(|Int|#.fits(|Int?|#))
+    verify(|Int?|#.fits(|Int|#))
+
+    // Bug fixes - see http://fantom.org/forum/topic/2453
+    list1 := (Int[]) Obj[,]
+    list2 := (Int[]) Obj?[,]
+    list3 := (Int?[]) Obj[,]
+    list4 := (Int?[]) Obj?[,]
+
+    list5 := (Int[]?) Obj[,]
+    list6 := (Int[]?) Obj?[,]
+    list7 := (Int?[]?) Obj[,]
+    list8 := (Int?[]?) Obj?[,]
+
+    map1 := (Int:Int[]) Obj:Obj[:]
+    map2 := (Int:Int[]) Obj:Obj?[:]
+    map3 := (Int:Int?[]) Obj:Obj[:]
+    map4 := (Int:Int?[]) Obj:Obj?[:]
+
+    map5 := (Int:Int[]?) Obj:Obj[:]
+    map6 := (Int:Int[]?) Obj:Obj?[:]
+    map7 := (Int:Int?[]?) Obj:Obj[:]
+    map8 := (Int:Int?[]?) Obj:Obj?[:]
   }
 
 //////////////////////////////////////////////////////////////////////////

Note the unit tests unearthed an error in TypeParser.js that gave a consume not defined error. Here's a fix for that also:

diff -r fd89993adbdd src/sys/js/fanx/TypeParser.js
--- a/src/sys/js/fanx/TypeParser.js	Wed Sep 23 17:17:28 2015 -0400
+++ b/src/sys/js/fanx/TypeParser.js	Fri Sep 25 00:59:03 2015 +0100
@@ -83,7 +83,7 @@
     type = type.toListOf();
     if (this.cur == '?')
     {
-      consume('?');
+      this.consume('?');
       type = type.toNullable();
     }
   }

brian Mon 28 Sep 2015

I agree that Type.fits should probably just ignore nullability. I pushed a fix - its actually much simpler to just handle that in Type.fits before delegating to the internal Type.is implementations.

SlimerDude Tue 29 Sep 2015

Cool, nice fix.

I agree that Type.fits should probably just ignore nullability.

Thanks! But alas, I can't take credit for the idea, see:

Andy, the following code still fails in the Javascript runtime:

diff -r fe538d44fa4d src/testSys/fan/TypeTest.fan
--- a/src/testSys/fan/TypeTest.fan	Mon Sep 28 13:18:07 2015 -0400
+++ b/src/testSys/fan/TypeTest.fan	Tue Sep 29 09:36:50 2015 +0100
@@ -197,6 +197,20 @@
     // void doesn't fit anything
     verifyFits(Void#, Obj#,  false)
     verifyFits(Obj#,  Void#, false)
+    
+    // JS Bug Fixes - see http://fantom.org/forum/topic/2453
+    // sys::CastErr: sys::Obj[] cannot be cast to sys::Int[]?
+    list1 := (Int[]?) Obj[,]
+    list2 := (Int[]?) Obj?[,]
+    list3 := (Int?[]?) Obj[,]
+    list4 := (Int?[]?) Obj?[,]
+
+    // JS Type Parsing Bug - see http://fantom.org/forum/topic/2475
+    // sys::Err: "consume" is not defined.
+    map1 := (Int:Int[]?) Obj:Obj[:]
+    map2 := (Int:Int[]?) Obj:Obj?[:]
+    map3 := (Int:Int?[]?) Obj:Obj[:]
+    map4 := (Int:Int?[]?) Obj:Obj?[:]
   }
 
   Void verifyFits(Type a, Type b, Bool expected)

Which can be fixed with the following updated patch:

diff -r fe538d44fa4d src/sys/js/fan/Type.js
--- a/src/sys/js/fan/Type.js	Mon Sep 28 13:18:07 2015 -0400
+++ b/src/sys/js/fan/Type.js	Tue Sep 29 09:27:25 2015 +0100
@@ -761,7 +761,7 @@
 {
   if (that instanceof fan.sys.ListType)
   {
-    if (that.v.qname() == "sys::Obj") return true;
+    if (this.v.qname() == "sys::Obj") return true;
     return this.v.is(that.v);
   }
   if (that instanceof fan.sys.Type)
@@ -848,8 +848,6 @@
 
 fan.sys.MapType.prototype.is = function(that)
 {
-  if (that.isNullable()) that = that.m_root;
-
   if (that instanceof fan.sys.MapType)
   {
     return this.k.is(that.k) && this.v.is(that.v);
diff -r fe538d44fa4d src/sys/js/fanx/TypeParser.js
--- a/src/sys/js/fanx/TypeParser.js	Mon Sep 28 13:18:07 2015 -0400
+++ b/src/sys/js/fanx/TypeParser.js	Tue Sep 29 09:27:25 2015 +0100
@@ -83,7 +83,7 @@
     type = type.toListOf();
     if (this.cur == '?')
     {
-      consume('?');
+      this.consume('?');
       type = type.toNullable();
     }
   }

brian Tue 29 Sep 2015

Thanks, I pushed a fix those JS problems

Login or Signup to reply.