Skip to content

DefaultBinder named parameter support skipping default parameters #71013

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 6, 2022
Merged

DefaultBinder named parameter support skipping default parameters #71013

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9885ad4
DefaultBinder named parameter support skipping default parameters
buyaa-n Jun 20, 2022
5b9e273
Apply feedback, add more tests
buyaa-n Jul 6, 2022
c855d69
Revert unwanted change
buyaa-n Jul 6, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
110 changes: 69 additions & 41 deletions src/libraries/System.Private.CoreLib/src/System/DefaultBinder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public sealed override MethodBase BindToMethod(
if (names == null)
{
// Default mapping
for (j = 0; j < args.Length; j++)
for (j = 0; j < par.Length; j++)
paramOrder[i][j] = j;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

paramOrder[i][j] array keeps track of real(defined) position of parameters, in case args count is less than defined parameters its not tracked, in order to support any number of named parameters defined in any order we need to keep track of all parameters

}
else
Expand Down Expand Up @@ -196,43 +196,54 @@ public sealed override MethodBase BindToMethod(
if (pCls.IsByRef)
pCls = pCls.GetElementType()!;

// the type is the same
if (pCls == argTypes[paramOrder[i][j]])
continue;
int index = paramOrder[i][j];
if (index < args.Length)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In case only 1st and 3rd named arguments provided from the 4 parameters the order in paramOrder[i] will be {0, 2, 1, 3} and argTypes[paramOrder[i][1]] will cause IndexOutOfBoundsException, therefore first need to check if current parameter is within provided arguments range

{
// the type is the same
if (pCls == argTypes[index])
continue;

// a default value is available
if (defaultValueBinding && args[paramOrder[i][j]] == Type.Missing)
continue;
// a default value is available
if (defaultValueBinding && args[index] == Type.Missing)
continue;

// the argument was null, so it matches with everything
if (args[paramOrder[i][j]] == null)
continue;
// the argument was null, so it matches with everything
if (args[index] == null)
continue;

// the type is Object, so it will match everything
if (pCls == typeof(object))
continue;
// the type is Object, so it will match everything
if (pCls == typeof(object))
continue;

// now do a "classic" type check
if (pCls.IsPrimitive)
{
if (argTypes[paramOrder[i][j]] == null || !CanChangePrimitive(args[paramOrder[i][j]]?.GetType(), pCls))
// now do a "classic" type check
if (pCls.IsPrimitive)
{
break;
if (argTypes[index] == null || !CanChangePrimitive(args[index]!.GetType(), pCls))
{
break;
}
}
else
{
if (argTypes[index] == null)
continue;

if (!pCls.IsAssignableFrom(argTypes[index]))
{
if (Marshal.IsBuiltInComSupported && argTypes[index].IsCOMObject)
{
if (pCls.IsInstanceOfType(args[index]))
continue;
}
break;
}
}
}
else
{
if (argTypes[paramOrder[i][j]] == null)
continue;

if (!pCls.IsAssignableFrom(argTypes[paramOrder[i][j]]))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Until this row nothing is changed except using index I defined above: int index = paramOrder[i][j]; instead of each paramOrder[i][j] access

if (defaultValueBinding && par[j].HasDefaultValue)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in case index not within the provided arguments range (index >= args.Length) allow it to continue only if the parameter has default value and BindingFlags.OptionalParamBinding is set

{
if (Marshal.IsBuiltInComSupported && argTypes[paramOrder[i][j]].IsCOMObject)
{
if (pCls.IsInstanceOfType(args[paramOrder[i][j]]))
continue;
}
break;
continue;
}
}
#endregion
Expand Down Expand Up @@ -313,17 +324,25 @@ public sealed override MethodBase BindToMethod(
{
object?[] objs = new object[parms.Length];

for (i = 0; i < args.Length; i++)
objs[i] = args[i];

for (; i < parms.Length - 1; i++)
objs[i] = parms[i].DefaultValue;

if (paramArrayTypes[0] != null)
objs[i] = Array.CreateInstance(paramArrayTypes[0], 0); // create an empty array for the

else
objs[i] = parms[i].DefaultValue;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here the provided args count is less than defined parameters the missing arguments filled with defaults, but it was not supporting skipped arguments, the update fixes that

for (i = 0; i < parms.Length; i++)
{
int k = paramOrder[0][i];
if (k < args.Length)
{
objs[i] = args[k];
}
else
{
if (i == parms.Length - 1 && paramArrayTypes[0] != null)
{
objs[i] = Array.CreateInstance(paramArrayTypes[0], 0); // create an empty array for the
}
else
{
objs[i] = parms[i].DefaultValue;
}
}
}

args = objs;
}
Expand Down Expand Up @@ -1138,10 +1157,19 @@ private static void ReorderParams(int[] paramOrder, object?[] vars)
{
object?[] varsCopy = new object[vars.Length];
for (int i = 0; i < vars.Length; i++)
{
varsCopy[i] = vars[i];
}

for (int i = 0; i < vars.Length; i++)
vars[i] = varsCopy[paramOrder[i]];
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method reorders named arguments in parameter order, again it was not supporting if any of the arguments skipped

for (int i = 0, j = 0; i < vars.Length; j++)
{
if (paramOrder[j] < vars.Length)
{
vars[i] = varsCopy[paramOrder[j]];
paramOrder[j] = i;
i++;
}
}
}

// This method will create the mapping between the Parameters and the underlying
Expand Down
213 changes: 213 additions & 0 deletions src/libraries/System.Reflection/tests/DefaultBinderTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,213 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using Xunit;

namespace System.Reflection.Tests
{
public class DefaultBinderTests
{
private const BindingFlags flags = BindingFlags.Public | BindingFlags.Instance | BindingFlags.Static;
private const BindingFlags invokeFlags = flags | BindingFlags.InvokeMethod | BindingFlags.OptionalParamBinding;
private static Binder binder = Type.DefaultBinder;

[Fact]
public static void DefaultBinderAllUnspecifiedParametersHasDefaultsTest()
{
MethodInfo[] methods = typeof(Sample).GetMethods();
object[] methodArgs = new object[] { "value" };
MethodBase method = binder.BindToMethod(flags, methods, ref methodArgs, null, null, new[] { "par1" }, out var _);
Assert.NotNull(method);
Assert.Equal("MethodMoreParameters", method.Name);
}

[Fact]
public static void DefaultBinderNamedParametersInOrderTest()
{
MethodInfo[] methods = typeof(Sample).GetMethods();
object[] methodArgs = new object[] { "value", 1 };
MethodBase method = binder.BindToMethod(flags, methods, ref methodArgs, null, null, new[] { "param1", "param2" }, out var _);
Assert.NotNull(method);
Assert.Equal("SampleMethod", method.Name);
}

[Fact]
public static void DefaultBinderNamedParametersOutOrderTest()
{
MethodInfo[] methods = typeof(Sample).GetMethods();
object[] methodArgs = new object[] { 1, "value" };
MethodBase method = binder.BindToMethod(flags, methods, ref methodArgs, null, null, new[] { "param2", "param1" }, out var _);
Assert.NotNull(method);
Assert.Equal("SampleMethod", method.Name);
}

[Fact]
public static void DefaultBinderNamedParametersSkippedOrderTest()
{
MethodInfo[] methods = typeof(Sample).GetMethods();
object[] methodArgs = new object[] { "value", true };
MethodBase method = binder.BindToMethod(flags, methods, ref methodArgs, null, null, new[] { "param1", "param3" }, out var _);
Assert.NotNull(method);
Assert.Equal("SampleMethod", method.Name);
}

[Fact]
public static void DefaultBinderNamedParametersMissingRequiredParameterThrowsMissingMethodException()
{
MethodInfo[] methods = new MethodInfo[] { typeof(Sample).GetMethod("NoDefaultParameterMethod") };
object[] methodArgs = new object[] { "value", 1 };
Assert.Throws<MissingMethodException>(() => binder.BindToMethod(flags, methods, ref methodArgs, null, null, new[] { "param1", "param2" }, out var _));
}

[Fact]
public static void DefaultBinderNamedParametersMixedOrderNoDefaults ()
{
MethodInfo[] methods = typeof(Sample).GetMethods();
object[] methodArgs = new object[] { true, "value", 3.14, 1 };
MethodBase method = binder.BindToMethod(flags, methods, ref methodArgs, null, null, new[] { "param3", "param1", "param4", "param2" }, out var _);
Assert.NotNull(method);
Assert.Equal("NoDefaultParameterMethod", method.Name);
}

[Fact]
public static void DefaultBinderNoNamedParametersInOrderNoDefaults()
{
MethodInfo[] methods = typeof(Sample).GetMethods();
object[] methodArgs = new object[] { "value", 1, true, 3.14 };
MethodBase method = binder.BindToMethod(flags, methods, ref methodArgs, null, null, null, out var _);
Assert.NotNull(method);
Assert.Equal("NoDefaultParameterMethod", method.Name);
}

[Fact]
public static void DefaultBinderNoNamedParametersNoArgumentsAllDefaults()
{
MethodInfo[] methods = typeof(Test).GetMethods();
object[] methodArgs = new object[] { null, null };
MethodBase method = binder.BindToMethod(flags, methods, ref methodArgs, null, null, null, out var _);
Assert.NotNull(method);
Assert.Equal("SampleMethod", method.Name);
}

[Fact]
public static void DefaultBinderNoNamedParametersOutOrderThrows()
{
MethodInfo[] methods = typeof(Sample).GetMethods();
object[] methodArgs = new object[] { true, "value", 3.14, 1 };
Assert.Throws<MissingMethodException>(() => binder.BindToMethod(flags, methods, ref methodArgs, null, null, null, out var _));
}

[Fact]
public static void DefaultBinderNamedParametersSkippedAndOutOfOrderTest()
{
MethodInfo[] methods = typeof(Sample).GetMethods();
object[] methodArgs = new object[] { 8, "value" };
MethodBase method = binder.BindToMethod(flags, methods, ref methodArgs, null, null, new[] { "par5", "par1" }, out var _);
Assert.NotNull(method);
Assert.Equal("MethodMoreParameters", method.Name);
}

[Fact]
public static void InvokeWithNamedParameters1st2ndTest()
{
Type t = typeof(Sample);
object instance = Activator.CreateInstance(t);
object[] methodArgs = new object[] { "value", 3 };
string[] paramNames = new string[] { "param1", "param2" };

int result = (int)t.InvokeMember("SampleMethod", invokeFlags, null, instance, methodArgs, null, null, paramNames);
Assert.Equal(3, result);
}

[Fact]
public static void InvokeWithNamedParameters1st3rd()
{
Type t = typeof(Sample);
object instance = Activator.CreateInstance(t);
object[] methodArgs = new object[] { "value", true };
string[] paramNames = new string[] { "param1", "param3" };

int result = (int)t.InvokeMember("SampleMethod", invokeFlags, null, instance, methodArgs, null, null, paramNames);
Assert.Equal(7, result);
}

[Fact]
public static void InvokeWithNamedParameters1st4th()
{
Type t = typeof(Sample);
object instance = Activator.CreateInstance(t);
object[] methodArgs = new object[] { "value", true };
string[] paramNames = new string[] { "param1", "param4" };

int result = (int)t.InvokeMember("AnotherMethod", invokeFlags, null, instance, methodArgs, null, null, paramNames);
Assert.Equal(7, result);
}

[Fact]
public static void InvokeWithNamedParameters4th1st()
{
Type t = typeof(Sample);
object instance = Activator.CreateInstance(t);
object[] methodArgs = new object[] { true, "value" };
string[] paramNames = new string[] { "param4", "param1" };

int result = (int)t.InvokeMember("AnotherMethod", invokeFlags, null, instance, methodArgs, null, null, paramNames);
Assert.Equal(7, result);
}

[Fact]
public static void InvokeWithNamedParametersOutOfOrder()
{
Type t = typeof(Sample);
object instance = Activator.CreateInstance(t);
object[] methodArgs = new object[] { 3, "value2", true, "value" };
string[] paramNames = new string[] { "param3", "param2", "param4", "param1" };

int result = (int)t.InvokeMember("AnotherMethod", invokeFlags, null, instance, methodArgs, null, null, paramNames);
Assert.Equal(8, result);
}

public class Test
{
public void TestMethod(int param1) { }
public void SampleMethod(int param2 = 2, bool param3 = false) { }
}

public class Sample
{
public static int SampleMethod(int param1 = 2)
{
return 1;
}

public static int SampleMethod(int param2 = 2, bool param3 = false)
{
return 1;
}

public void NoDefaultParameterMethod(string param1, int param2, bool param3, double param4) { }

public static int SampleMethod(string param1, int param2 = 2, bool param3 = false)
{
if (param3)
{
return param2 + param1.Length;
}

return param2;
}

public int AnotherMethod(string param1, string param2 = "", int param3 = 2, bool param4 = false)
{
if (param4)
{
return param3 + param1.Length;
}

return param3;
}

public void MethodMoreParameters(string par1, string par2 = "", int par3 = 2, bool par4 = false, short par5 = 1) { }
}
}
}
1 change: 1 addition & 0 deletions src/libraries/System.Reflection/tests/System.Reflection.Tests.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
<Compile Include="AssemblyTests.cs" />
<Compile Include="ConstructorInfoTests.cs" />
<Compile Include="CustomAttributeTests.cs" />
<Compile Include="DefaultBinderTests.cs" />
<Compile Include="EventInfoTests.cs" />
<Compile Include="FieldInfoTests.cs" />
<Compile Include="GetTypeTests.cs" />
Expand Down