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)
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.
SlimerDudeTue 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:
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:
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.jsthat gave aconsume not definederror. 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.
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