Merge: Warn call on nullable receiver

Followup of #1375 and #394

This introduce an advice on calls on a nullable receiver.
Since the conversion could be complex, I propose a light approach with advices (quiet warnings) instead of hard errors, so that:

1. jenkins can track them
2. nitpick (thus vim) can show them
3. mirgration can be done in an incremental and iterative way
4. issues with a strict call-on-nullable policy can be identified without breaking things
5. @Morriar can bank nitcoins in future PR

If (when?) all call on nullable can be removed, then the advice will become an error.

Pull-Request: #1414
Reviewed-by: Alexis Laferrière <alexis.laf@xymus.net>
Reviewed-by: Romain Chanoir <chanoir.romain@courrier.uqam.ca>

diff --git a/src/metrics/nullables_metrics.nit b/src/metrics/nullables_metrics.nit
index 2c96462..aa145c6 100644 (file)
--- a/src/metrics/nullables_metrics.nit
+++ b/src/metrics/nullables_metrics.nit
@@ -131,8 +131,8 @@ private class NullableSends
                        end
                        t = t.anchor_to(self.nclassdef.mclassdef.mmodule, self.nclassdef.mclassdef.bound_mtype)
                        if t isa MNullableType then
-                               var name = n.callsite.mproperty.name
-                               if name == "==" or name == "!=" or name == "is_same_instance" then
+                               var p = n.callsite.mproperty
+                               if p.is_null_safe then
                                        self.nullable_eq_sends += 1
                                else
                                        self.nullable_sends += 1

diff --git a/src/model/model.nit b/src/model/model.nit
index 908cb60..4e9be82 100644 (file)
--- a/src/model/model.nit
+++ b/src/model/model.nit
@@ -2156,6 +2156,10 @@ class MMethod
        do
                return self.is_init
        end
+
+       # A specific method that is safe to call on null.
+       # Currently, only `==`, `!=` and `is_same_instance` are safe
+       fun is_null_safe: Bool do return name == "==" or name == "!=" or name == "is_same_instance"
 end
 
 # A global attribute

diff --git a/src/semantize/typing.nit b/src/semantize/typing.nit
index 3c7c9b1..44ed320 100644 (file)
--- a/src/semantize/typing.nit
+++ b/src/semantize/typing.nit
@@ -301,15 +301,9 @@ private class TypeVisitor
 
                #debug("recv: {recvtype} (aka {unsafe_type})")
                if recvtype isa MNullType then
-                       # `null` only accepts some methods of object.
-                       if name == "==" or name == "!=" or name == "is_same_instance" then
-                               var objclass = get_mclass(node, "Object")
-                               if objclass == null then return null # Forward error
-                               unsafe_type = objclass.mclass_type
-                       else
-                               self.error(node, "Error: method `{name}` called on `null`.")
-                               return null
-                       end
+                       var objclass = get_mclass(node, "Object")
+                       if objclass == null then return null # Forward error
+                       unsafe_type = objclass.mclass_type
                end
 
                var mproperty = self.try_get_mproperty_by_name2(node, unsafe_type, name)
@@ -331,6 +325,14 @@ private class TypeVisitor
 
                assert mproperty isa MMethod
 
+               # `null` only accepts some methods of object.
+               if recvtype isa MNullType and not mproperty.is_null_safe then
+                       self.error(node, "Error: method `{name}` called on `null`.")
+                       return null
+               else if unsafe_type isa MNullableType and not mproperty.is_null_safe then
+                       modelbuilder.advice(node, "call-on-nullable", "Warning: method call on a nullable receiver `{recvtype}`.")
+               end
+
                if is_toplevel_context and recv_is_self and not mproperty.is_toplevel then
                        error(node, "Error: `{name}` is not a top-level method, thus need a receiver.")
                end

diff --git a/tests/sav/base_adaptive_loop3_alt3.res b/tests/sav/base_adaptive_loop3_alt3.res
index 23b0f98..9dd9003 100644 (file)
--- a/tests/sav/base_adaptive_loop3_alt3.res
+++ b/tests/sav/base_adaptive_loop3_alt3.res
@@ -1 +1 @@
-alt/base_adaptive_loop3_alt3.nit:29,10--13: Error: method `next` called on `null`.
+alt/base_adaptive_loop3_alt3.nit:29,10--13: Error: method `next` does not exists in `null`.

diff --git a/tests/sav/base_var_null_alt12.res b/tests/sav/base_var_null_alt12.res
index ba1fd0e..88f7f46 100644 (file)
--- a/tests/sav/base_var_null_alt12.res
+++ b/tests/sav/base_var_null_alt12.res
@@ -1 +1 @@
-alt/base_var_null_alt12.nit:45,3--4: Error: method `+` called on `null`.
+alt/base_var_null_alt12.nit:45,3--4: Error: method `+` does not exists in `null`.

diff --git a/tests/sav/base_var_null_alt4.res b/tests/sav/base_var_null_alt4.res
index 551e7aa..423e515 100644 (file)
--- a/tests/sav/base_var_null_alt4.res
+++ b/tests/sav/base_var_null_alt4.res
@@ -1 +1 @@
-alt/base_var_null_alt4.nit:31,3--4: Error: method `+` called on `null`.
+alt/base_var_null_alt4.nit:31,3--4: Error: method `+` does not exists in `null`.

diff --git a/tests/sav/base_var_null_alt8.res b/tests/sav/base_var_null_alt8.res
index ad947f4..f168d60 100644 (file)
--- a/tests/sav/base_var_null_alt8.res
+++ b/tests/sav/base_var_null_alt8.res
@@ -1 +1 @@
-alt/base_var_null_alt8.nit:38,3--4: Error: method `+` called on `null`.
+alt/base_var_null_alt8.nit:38,3--4: Error: method `+` does not exists in `null`.

diff --git a/tests/sav/error_expr_not_ok_alt6.res b/tests/sav/error_expr_not_ok_alt6.res
index 0e499b0..bd0a8fc 100644 (file)
--- a/tests/sav/error_expr_not_ok_alt6.res
+++ b/tests/sav/error_expr_not_ok_alt6.res
@@ -8,9 +8,9 @@ alt/error_expr_not_ok_alt6.nit:41,16--19: Type Error: expected `Int`, got `null`
 alt/error_expr_not_ok_alt6.nit:42,7--9: Type Error: expected `A`, got `Int`.
 alt/error_expr_not_ok_alt6.nit:43,5--8: Error: method `fail` does not exists in `Int`.
 alt/error_expr_not_ok_alt6.nit:45,7--10: Type Error: expected `A`, got `null`.
-alt/error_expr_not_ok_alt6.nit:46,6--9: Error: method `fail` called on `null`.
+alt/error_expr_not_ok_alt6.nit:46,6--9: Error: method `fail` does not exists in `null`.
 alt/error_expr_not_ok_alt6.nit:49,7--10: Type Error: expected `A`, got `null`.
-alt/error_expr_not_ok_alt6.nit:50,6--10: Error: method `trash` called on `null`.
+alt/error_expr_not_ok_alt6.nit:50,6--10: Error: method `trash` does not exists in `null`.
 alt/error_expr_not_ok_alt6.nit:60,4--7: Type Error: expected `Bool`, got `Int`.
 alt/error_expr_not_ok_alt6.nit:60,20: Type Error: expected `A`, got `Int`.
 alt/error_expr_not_ok_alt6.nit:62,10--13: Type Error: expected `Bool`, got `Int`.