Fix params argument handling (pythonnet#1106)

slide · web-flow · commit 49a230b5e0a8 · 2020-05-16T22:42:16.000+02:00
Add a check to see if the last parameter is a params parameter. Set the correct flag to show that it is a paramsArray function. Fixes pythonnet#943 and pythonnet#331.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -39,8 +39,9 @@ This document follows the conventions laid out in [Keep a CHANGELOG][].
   together with Nuitka
 - Fixes bug where delegates get casts (dotnetcore)
 - Determine size of interpreter longs at runtime
-- Handling exceptions ocurred in ModuleObject's getattribute 
+- Handling exceptions ocurred in ModuleObject's getattribute
 - Fill `__classcell__` correctly for Python subclasses of .NET types
+- Fixed issue with params methods that are not passed an array.
 
 ## [2.4.0][]
 
diff --git a/src/runtime/methodbinder.cs b/src/runtime/methodbinder.cs
@@ -369,6 +369,41 @@ internal Binding Bind(IntPtr inst, IntPtr args, IntPtr kw, MethodBase info, Meth
             return null;
         }
 
+        static IntPtr HandleParamsArray(IntPtr args, int arrayStart, int pyArgCount, out bool isNewReference)
+        {
+            isNewReference = false;
+            IntPtr op;
+            // for a params method, we may have a sequence or single/multiple items
+            // here we look to see if the item at the paramIndex is there or not
+            // and then if it is a sequence itself.
+            if ((pyArgCount - arrayStart) == 1)
+            {
+                // we only have one argument left, so we need to check it
+                // to see if it is a sequence or a single item
+                IntPtr item = Runtime.PyTuple_GetItem(args, arrayStart);
+                if (!Runtime.PyString_Check(item) && Runtime.PySequence_Check(item))
+                {
+                    // it's a sequence (and not a string), so we use it as the op
+                    op = item;
+                }
+                else
+                {
+                    isNewReference = true;
+                    op = Runtime.PyTuple_GetSlice(args, arrayStart, pyArgCount);
+                    if (item != IntPtr.Zero)
+                    {
+                        Runtime.XDecref(item);
+                    }
+                }
+            }
+            else
+            {
+                isNewReference = true;
+                op = Runtime.PyTuple_GetSlice(args, arrayStart, pyArgCount);
+            }
+            return op;
+        }
+
         /// <summary>
         /// Attempts to convert Python positional argument tuple and keyword argument table
         /// into an array of managed objects, that can be passed to a method.
@@ -397,8 +432,9 @@ static object[] TryConvertArguments(ParameterInfo[] pi, bool paramsArray,
             {
                 var parameter = pi[paramIndex];
                 bool hasNamedParam = kwargDict.ContainsKey(parameter.Name);
+                bool isNewReference = false;
 
-                if (paramIndex >= pyArgCount && !hasNamedParam)
+                if (paramIndex >= pyArgCount && !(hasNamedParam || (paramsArray && paramIndex == arrayStart)))
                 {
                     if (defaultArgList != null)
                     {
@@ -415,11 +451,14 @@ static object[] TryConvertArguments(ParameterInfo[] pi, bool paramsArray,
                 }
                 else
                 {
-                    op = (arrayStart == paramIndex)
-                        // map remaining Python arguments to a tuple since
-                        // the managed function accepts it - hopefully :]
-                        ? Runtime.PyTuple_GetSlice(args, arrayStart, pyArgCount)
-                        : Runtime.PyTuple_GetItem(args, paramIndex);
+                    if(arrayStart == paramIndex)
+                    {
+                        op = HandleParamsArray(args, arrayStart, pyArgCount, out isNewReference);                                                                 
+                    }
+                    else
+                    {
+                        op = Runtime.PyTuple_GetItem(args, paramIndex);
+                    }
                 }
 
                 bool isOut;
@@ -428,7 +467,7 @@ static object[] TryConvertArguments(ParameterInfo[] pi, bool paramsArray,
                     return null;
                 }
 
-                if (arrayStart == paramIndex)
+                if (isNewReference)
                 {
                     // TODO: is this a bug? Should this happen even if the conversion fails?
                     // GetSlice() creates a new reference but GetItem()
@@ -543,7 +582,7 @@ static bool MatchesArgumentCount(int positionalArgumentCount, ParameterInfo[] pa
         {
             defaultArgList = null;
             var match = false;
-            paramsArray = false;
+            paramsArray = parameters.Length > 0 ? Attribute.IsDefined(parameters[parameters.Length - 1], typeof(ParamArrayAttribute)) : false;
 
             if (positionalArgumentCount == parameters.Length)
             {
@@ -572,7 +611,7 @@ static bool MatchesArgumentCount(int positionalArgumentCount, ParameterInfo[] pa
                         // to be passed in as the parameter value
                         defaultArgList.Add(parameters[v].GetDefaultValue());
                     }
-                    else
+                    else if(!paramsArray)
                     {
                         match = false;
                     }
diff --git a/src/tests/test_method.py b/src/tests/test_method.py
@@ -257,6 +257,37 @@ def test_non_params_array_in_last_place():
     result = MethodTest.TestNonParamsArrayInLastPlace(1, 2, 3)
     assert result
 
+def test_params_methods_with_no_params():
+    """Tests that passing no arguments to a params method
+    passes an empty array"""
+    result = MethodTest.TestValueParamsArg()
+    assert len(result) == 0
+
+    result = MethodTest.TestOneArgWithParams('Some String')
+    assert len(result) == 0
+
+    result = MethodTest.TestTwoArgWithParams('Some String', 'Some Other String')
+    assert len(result) == 0
+
+def test_params_methods_with_non_lists():
+    """Tests that passing single parameters to a params
+    method will convert into an array on the .NET side"""
+    result = MethodTest.TestOneArgWithParams('Some String', [1, 2, 3, 4])
+    assert len(result) == 4
+
+    result = MethodTest.TestOneArgWithParams('Some String', 1, 2, 3, 4)
+    assert len(result) == 4
+
+    result = MethodTest.TestOneArgWithParams('Some String', [5])
+    assert len(result) == 1
+
+    result = MethodTest.TestOneArgWithParams('Some String', 5)
+    assert len(result) == 1
+
+def test_params_method_with_lists():
+    """Tests passing multiple lists to a params object[] method"""
+    result = MethodTest.TestObjectParamsArg([],[])
+    assert len(result) == 2
 
 def test_string_out_params():
     """Test use of string out-parameters."""
