Merge: Check operator

Previously, no checking was done on the signature of operators.
While this is not an issue for the model nor the tools this could yield to not POLA error messages when the operator is used. eg.

~~~nit
class A
   fun +(other: A) do print "hello"
end
var a = new A
var b = a + a # Error expected expression
a + a # Error unexpected operator +
# no way do invoke `+` in fact.
~~~

With the PR, we have

~~~
Error: mandatory return type for `+`.
fun +(other: A) do print "hello"
    ^
~~~

The following errors are added by the PR:

* mandatory return
* not enough parameters
* too much parameters
* illegal variadic parameter

Pull-Request: #1269
Reviewed-by: Alexandre Terrasa <alexandre@moz-code.org>
Reviewed-by: Alexis Laferrière <alexis.laf@xymus.net>
Reviewed-by: Romain Chanoir <chanoir.romain@courrier.uqam.ca>

diff --git a/src/modelize/modelize_property.nit b/src/modelize/modelize_property.nit
index 9cc9580..c0d3890 100644 (file)
--- a/src/modelize/modelize_property.nit
+++ b/src/modelize/modelize_property.nit
@@ -790,11 +790,17 @@ redef class AMethPropdef
                        name = amethodid.collect_text
                        name_node = amethodid
 
-                       if name == "+" and self.n_signature.n_params.length == 0 then
+                       var arity = self.n_signature.n_params.length
+                       if name == "+" and arity == 0 then
                                name = "unary +"
-                       end
-                       if name == "-" and self.n_signature.n_params.length == 0 then
+                       else if name == "-" and arity == 0 then
                                name = "unary -"
+                       else
+                               if amethodid.is_binary and arity != 1 then
+                                       modelbuilder.error(self.n_signature, "Syntax Error: binary operator `{name}` requires exactly one parameter; got {arity}.")
+                               else if amethodid.min_arity > arity then
+                                       modelbuilder.error(self.n_signature, "Syntax Error: `{name}` requires at least {amethodid.min_arity} parameter(s); got {arity}.")
+                               end
                        end
                end
 
@@ -868,6 +874,9 @@ redef class AMethPropdef
                        end
                end
 
+               var accept_special_last_parameter = self.n_methid == null or self.n_methid.accept_special_last_parameter
+               var return_is_mandatory = self.n_methid != null and self.n_methid.return_is_mandatory
+
                # Retrieve info from the signature AST
                var param_names = new Array[String] # Names of parameters from the AST
                var param_types = new Array[MType] # Types of parameters from the AST
@@ -941,6 +950,14 @@ redef class AMethPropdef
                # In `new`-factories, the return type is by default the classtype.
                if ret_type == null and mpropdef.mproperty.is_new then ret_type = mclassdef.mclass.mclass_type
 
+               # Special checks for operator methods
+               if not accept_special_last_parameter and mparameters.not_empty and mparameters.last.is_vararg then
+                       modelbuilder.error(self.n_signature.n_params.last, "Error: illegal variadic parameter `{mparameters.last}` for `{mpropdef.mproperty.name}`.")
+               end
+               if ret_type == null and return_is_mandatory then
+                       modelbuilder.error(self.n_methid, "Error: mandatory return type for `{mpropdef.mproperty.name}`.")
+               end
+
                msignature = new MSignature(mparameters, ret_type)
                mpropdef.msignature = msignature
                mpropdef.is_abstract = self.get_single_annotation("abstract", modelbuilder) != null
@@ -1022,6 +1039,56 @@ redef class AMethPropdef
        end
 end
 
+redef class AMethid
+       # Is a return required?
+       #
+       # * True for operators and brackets.
+       # * False for id and assignment.
+       fun return_is_mandatory: Bool do return true
+
+       # Can the last parameter be special like a vararg?
+       #
+       # * False for operators: the last one is in fact the only one.
+       # * False for assignments: it is the right part of the assignment.
+       # * True for ids and brackets.
+       fun accept_special_last_parameter: Bool do return false
+
+       # The minimum required number of parameters.
+       #
+       # * 1 for binary operators
+       # * 1 for brackets
+       # * 1 for assignments
+       # * 2 for bracket assignments
+       # * 0 for ids
+       fun min_arity: Int do return 1
+
+       # Is the `self` a binary operator?
+       fun is_binary: Bool do return true
+end
+
+redef class AIdMethid
+       redef fun return_is_mandatory do return false
+       redef fun accept_special_last_parameter do return true
+       redef fun min_arity do return 0
+       redef fun is_binary do return false
+end
+
+redef class ABraMethid
+       redef fun accept_special_last_parameter do return true
+       redef fun is_binary do return false
+end
+
+redef class ABraassignMethid
+       redef fun return_is_mandatory do return false
+       redef fun min_arity do return 2
+       redef fun is_binary do return false
+end
+
+redef class AAssignMethid
+       redef fun return_is_mandatory do return false
+       redef fun is_binary do return false
+end
+
 redef class AAttrPropdef
        redef type MPROPDEF: MAttributeDef
 

diff --git a/tests/error_operators.nit b/tests/error_operators.nit
new file mode 100644 (file)
index 0000000..db94029
--- /dev/null
+++ b/tests/error_operators.nit
@@ -0,0 +1,91 @@
+# This file is part of NIT ( http://www.nitlanguage.org ).
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# no return
+class A
+       fun + do abort
+       fun +(a: A) do abort
+       fun - do abort
+       fun -(a: A) do abort
+       fun *(a: A) do abort
+       fun /(a: A) do abort
+       fun %(a: A) do abort
+       fun <(a: A) do abort
+       fun >(a: A) do abort
+       fun <=(a: A) do abort
+       fun >=(a: A) do abort
+       fun <=>(a: A) do abort
+       fun <<(a: A) do abort
+       fun >>(a: A) do abort
+       fun foo=(a: A) do abort # should be fine
+       fun [](a: A) do abort
+       fun []=(a, b: A) do abort # should be fine
+end
+
+# not enough parameters
+class B
+       fun +: A do abort # should be fine
+       fun -: A do abort # should be fine
+       fun *: A do abort
+       fun /: A do abort
+       fun %: A do abort
+       fun <: A do abort
+       fun >: A do abort
+       fun <=: A do abort
+       fun >=: A do abort
+       fun <=>: A do abort
+       fun <<: A do abort
+       fun >>: A do abort
+       fun foo= do abort
+       fun []: A do abort
+       fun []=(a: A) do abort
+end
+
+# too much parameters
+class C
+       fun +(a,b,c:A): A do abort
+       fun -(a,b,c:A): A do abort
+       fun *(a,b,c:A): A do abort
+       fun /(a,b,c:A): A do abort
+       fun %(a,b,c:A): A do abort
+       fun <(a,b,c:A): A do abort
+       fun >(a,b,c:A): A do abort
+       fun <=(a,b,c:A): A do abort
+       fun >=(a,b,c:A): A do abort
+       fun <=>(a,b,c:A): A do abort
+       fun <<(a,b,c:A): A do abort
+       fun >>(a,b,c:A): A do abort
+       fun foo=(a,b,c:A) do abort # should be fine
+       fun [](a,b,c:A): A do abort # should be fine
+       fun []=(a,b,c:A) do abort # should be fine
+end
+
+# bad vararg
+class D
+       fun +(a:A...): A do abort
+       fun -(a:A...): A do abort
+       fun *(a:A...): A do abort
+       fun /(a:A...): A do abort
+       fun %(a:A...): A do abort
+       fun <(a:A...): A do abort
+       fun >(a:A...): A do abort
+       fun <=(a:A...): A do abort
+       fun >=(a:A...): A do abort
+       fun <=>(a:A...): A do abort
+       fun <<(a:A...): A do abort
+       fun >>(a:A...): A do abort
+       fun foo=(a,b,c:A, d:A...) do abort
+       fun [](a,b,c:A, d:A...): A do abort # should be fine
+       fun []=(a,b,c:A, d:A...) do abort
+end

diff --git a/tests/sav/error_operators.res b/tests/sav/error_operators.res
new file mode 100644 (file)
index 0000000..1949e18
--- /dev/null
+++ b/tests/sav/error_operators.res
@@ -0,0 +1,54 @@
+error_operators.nit:17,6: Error: mandatory return type for `unary +`.
+error_operators.nit:18,6: Error: mandatory return type for `+`.
+error_operators.nit:19,6: Error: mandatory return type for `unary -`.
+error_operators.nit:20,6: Error: mandatory return type for `-`.
+error_operators.nit:21,6: Error: mandatory return type for `*`.
+error_operators.nit:22,6: Error: mandatory return type for `/`.
+error_operators.nit:23,6: Error: mandatory return type for `%`.
+error_operators.nit:24,6: Error: mandatory return type for `<`.
+error_operators.nit:25,6: Error: mandatory return type for `>`.
+error_operators.nit:26,6--7: Error: mandatory return type for `<=`.
+error_operators.nit:27,6--7: Error: mandatory return type for `>=`.
+error_operators.nit:28,6--8: Error: mandatory return type for `<=>`.
+error_operators.nit:29,6--7: Error: mandatory return type for `<<`.
+error_operators.nit:30,6--7: Error: mandatory return type for `>>`.
+error_operators.nit:32,6--7: Error: mandatory return type for `[]`.
+error_operators.nit:40,9: Syntax Error: binary operator `*` requires exactly one parameter; got 0.
+error_operators.nit:41,9: Syntax Error: binary operator `/` requires exactly one parameter; got 0.
+error_operators.nit:42,9: Syntax Error: binary operator `%` requires exactly one parameter; got 0.
+error_operators.nit:43,9: Syntax Error: binary operator `<` requires exactly one parameter; got 0.
+error_operators.nit:44,9: Syntax Error: binary operator `>` requires exactly one parameter; got 0.
+error_operators.nit:45,10: Syntax Error: binary operator `<=` requires exactly one parameter; got 0.
+error_operators.nit:46,10: Syntax Error: binary operator `>=` requires exactly one parameter; got 0.
+error_operators.nit:47,11: Syntax Error: binary operator `<=>` requires exactly one parameter; got 0.
+error_operators.nit:48,10: Syntax Error: binary operator `<<` requires exactly one parameter; got 0.
+error_operators.nit:49,10: Syntax Error: binary operator `>>` requires exactly one parameter; got 0.
+error_operators.nit:50,14: Syntax Error: `foo=` requires at least 1 parameter(s); got 0.
+error_operators.nit:51,10: Syntax Error: `[]` requires at least 1 parameter(s); got 0.
+error_operators.nit:52,9--14: Syntax Error: `[]=` requires at least 2 parameter(s); got 1.
+error_operators.nit:57,7--18: Syntax Error: binary operator `+` requires exactly one parameter; got 3.
+error_operators.nit:58,7--18: Syntax Error: binary operator `-` requires exactly one parameter; got 3.
+error_operators.nit:59,7--18: Syntax Error: binary operator `*` requires exactly one parameter; got 3.
+error_operators.nit:60,7--18: Syntax Error: binary operator `/` requires exactly one parameter; got 3.
+error_operators.nit:61,7--18: Syntax Error: binary operator `%` requires exactly one parameter; got 3.
+error_operators.nit:62,7--18: Syntax Error: binary operator `<` requires exactly one parameter; got 3.
+error_operators.nit:63,7--18: Syntax Error: binary operator `>` requires exactly one parameter; got 3.
+error_operators.nit:64,8--19: Syntax Error: binary operator `<=` requires exactly one parameter; got 3.
+error_operators.nit:65,8--19: Syntax Error: binary operator `>=` requires exactly one parameter; got 3.
+error_operators.nit:66,9--20: Syntax Error: binary operator `<=>` requires exactly one parameter; got 3.
+error_operators.nit:67,8--19: Syntax Error: binary operator `<<` requires exactly one parameter; got 3.
+error_operators.nit:68,8--19: Syntax Error: binary operator `>>` requires exactly one parameter; got 3.
+error_operators.nit:76,8--13: Error: illegal variadic parameter `a: A...` for `+`.
+error_operators.nit:77,8--13: Error: illegal variadic parameter `a: A...` for `-`.
+error_operators.nit:78,8--13: Error: illegal variadic parameter `a: A...` for `*`.
+error_operators.nit:79,8--13: Error: illegal variadic parameter `a: A...` for `/`.
+error_operators.nit:80,8--13: Error: illegal variadic parameter `a: A...` for `%`.
+error_operators.nit:81,8--13: Error: illegal variadic parameter `a: A...` for `<`.
+error_operators.nit:82,8--13: Error: illegal variadic parameter `a: A...` for `>`.
+error_operators.nit:83,9--14: Error: illegal variadic parameter `a: A...` for `<=`.
+error_operators.nit:84,9--14: Error: illegal variadic parameter `a: A...` for `>=`.
+error_operators.nit:85,10--15: Error: illegal variadic parameter `a: A...` for `<=>`.
+error_operators.nit:86,9--14: Error: illegal variadic parameter `a: A...` for `<<`.
+error_operators.nit:87,9--14: Error: illegal variadic parameter `a: A...` for `>>`.
+error_operators.nit:88,20--25: Error: illegal variadic parameter `d: A...` for `foo=`.
+error_operators.nit:90,19--24: Error: illegal variadic parameter `d: A...` for `[]=`.

diff --git a/tests/sav/test_ffi_c_operators.res b/tests/sav/test_ffi_c_operators.res
index e80f5de..893e1ea 100644 (file)
--- a/tests/sav/test_ffi_c_operators.res
+++ b/tests/sav/test_ffi_c_operators.res
@@ -20,6 +20,8 @@ false
 false
 true
 true
+32
+8
 52
 456
 123

diff --git a/tests/test_ffi_c_operators.nit b/tests/test_ffi_c_operators.nit
index 271589e..5261b5e 100644 (file)
--- a/tests/test_ffi_c_operators.nit
+++ b/tests/test_ffi_c_operators.nit
@@ -102,14 +102,16 @@ class A
                return A_value( recv ) <= A_value( other );
        `}
 
-       fun >>( other : A ) import value, value=, A `{
+       fun >>( other : A ): A import value, value=, A `{
                int new_val = A_value( recv ) >> A_value( other );
                A_value__assign( recv, new_val );
+               return recv;
        `}
 
-       fun <<( other : A ) import value, A `{
+       fun <<( other : A ): A import value, A `{
                int new_val = A_value( recv ) << A_value( other );
                A_value__assign( recv, new_val );
+               return recv;
        `}
 
        fun []( index : Int ) : A import A `{
@@ -154,13 +156,13 @@ print new A( 1 ) >= new A( 100 ) # false
 print new A( 100 ) >= new A( 100 ) # true
 print new A( 100 ) >= new A( 1 ) # true
 
-#var x = new A( 1 )
-#x << new A( 5 )
-#print x # 16
+var x = new A( 1 )
+x = x << new A( 5 )
+print x # 32
 
-#var y = new A( 32 )
-#y >> new A( 2 )
-#print y # 8
+var y = new A( 32 )
+y = y >> new A( 2 )
+print y # 8
 
 var a = new A( 456 )
 print a[ 52 ] # 52