Prevent ParamArray and ParamDictionary parameters from binding by keyword

Src/Microsoft.Dynamic/Actions/Calls/DefaultOverloadResolver.cs

@@ -8,7 +8,7 @@
 using System.Diagnostics;
 using System.Dynamic;
 using System.Linq.Expressions;
-
+using System.Reflection;
 using Microsoft.Scripting.Actions.Calls;
 using Microsoft.Scripting.Runtime;
 using Microsoft.Scripting.Utils;
@@ -104,30 +104,31 @@ protected internal override Candidate CompareEquivalentCandidates(ApplicableCand
         #region Actual Arguments
 
         private DynamicMetaObject GetArgument(int i) {
+            Debug.Assert(i >= 0);
             return _args[(CallType == CallTypes.ImplicitInstance ? 1 : 0) + i];
         }
 
         protected override void GetNamedArguments(out IList<DynamicMetaObject> namedArgs, out IList<string> argNames) {
-            if (_signature.HasNamedArgument() || _signature.HasDictionaryArgument()) {
+            bool hasNamedArgument = _signature.HasNamedArgument();
+            bool hasDictionaryArgument = _signature.HasDictionaryArgument();
+
+            if (hasNamedArgument || hasDictionaryArgument) {
                 var objects = new List<DynamicMetaObject>();
                 var names = new List<string>();
 
-                for (int i = 0; i < _signature.ArgumentCount; i++) {
-                    if (_signature.GetArgumentKind(i) == ArgumentType.Named) {
-                        objects.Add(GetArgument(i));
-                        names.Add(_signature.GetArgumentName(i));
+                if (hasNamedArgument) {
+                    for (int i = 0; i < _signature.ArgumentCount; i++) {
+                        if (_signature.GetArgumentKind(i) == ArgumentType.Named) {
+                            objects.Add(GetArgument(i));
+                            names.Add(_signature.GetArgumentName(i));
+                        }
                     }
                 }
 
-                if (_signature.HasDictionaryArgument()) {
-                    if (objects == null) {
-                        objects = new List<DynamicMetaObject>();
-                        names = new List<string>();
-                    }
+                if (hasDictionaryArgument) {
                     SplatDictionaryArgument(names, objects);
                 }
 
-
                 names.TrimExcess();
                 objects.TrimExcess();
                 argNames = names;
@@ -138,6 +139,12 @@ protected override void GetNamedArguments(out IList<DynamicMetaObject> namedArgs
             }
         }
 
+        protected internal override bool AllowByKeywordArgument(OverloadInfo method, ParameterInfo parameter) {
+            // params arrays & dictionaries don't allow assignment by keyword
+            return base.AllowByKeywordArgument(method, parameter)
+                && !parameter.IsParamArray() && !parameter.IsParamDictionary();
+        }
+
         protected override ActualArguments CreateActualArguments(IList<DynamicMetaObject> namedArgs, IList<string> argNames, int preSplatLimit, int postSplatLimit) {
             var res = new List<DynamicMetaObject>();
 
@@ -195,8 +202,9 @@ protected override ActualArguments CreateActualArguments(IList<DynamicMetaObject
 
         private void SplatDictionaryArgument(IList<string> splattedNames, IList<DynamicMetaObject> splattedArgs) {
             Assert.NotNull(splattedNames, splattedArgs);
+            Debug.Assert(_signature.HasDictionaryArgument());
 
-            DynamicMetaObject dictMo = GetArgument(_signature.ArgumentCount - 1);
+            DynamicMetaObject dictMo = GetArgument(_signature.IndexOf(ArgumentType.Dictionary));
             IDictionary dict = (IDictionary)dictMo.Value;
             IDictionaryEnumerator dictEnum = dict.GetEnumerator();
             while (dictEnum.MoveNext()) {

Src/Microsoft.Dynamic/Actions/Calls/OverloadResolver.cs

@@ -158,21 +158,21 @@ public BindingTarget ResolveOverload(string methodName, IList<OverloadInfo> meth
         /// Checks to see if the language allows named arguments to be bound to instance fields or
         /// properties and turned into setters. By default this is only allowed on contructors.
         /// </summary>
-        internal protected virtual bool AllowMemberInitialization(OverloadInfo method) {
+        protected internal virtual bool AllowMemberInitialization(OverloadInfo method) {
 #pragma warning disable 618 // obsolete
             return AllowKeywordArgumentSetting(method.ReflectionInfo);
 #pragma warning restore 618
         }
 
         [Obsolete("Use OverloadInfo.AllowMemberInitialization instead")]
-        internal protected virtual bool AllowKeywordArgumentSetting(MethodBase method) {
+        protected internal virtual bool AllowKeywordArgumentSetting(MethodBase method) {
             return CompilerHelpers.IsConstructor(method);
         }
 
         /// <summary>
         /// Gets an expression that evaluates to the result of GetByRefArray operation.
         /// </summary>
-        internal protected virtual Expression GetByRefArrayExpression(Expression argumentArrayExpression) {
+        protected internal virtual Expression GetByRefArrayExpression(Expression argumentArrayExpression) {
             return argumentArrayExpression;
         }
 
@@ -183,6 +183,25 @@ protected virtual bool BindToUnexpandedParams(MethodCandidate candidate) {
             return true;
         }
 
+        /// <summary>
+        /// Checks whether the given parameter may be mapped to by a keyword argument.
+        /// </summary>
+        /// <remarks>
+        /// If overriden, the derived class may only add more constraints to the constraints from the base class.
+        /// </remarks>
+        /// <example>
+        /// <code><![CDATA[
+        /// protected internal override bool AllowByKeywordArgument(OverloadInfo method, ParameterInfo parameter)
+        ///   => base.AllowByKeywordArgument(method, parameter)
+        ///      && AdditionalCheckIfByKeywordAllowed(method, parameter);
+        /// ]]></code>
+        /// </example>
+        /// <seealso cref="GetNamedArguments"/>
+        /// <seealso cref="BindToUnexpandedParams"/>
+        protected internal virtual bool AllowByKeywordArgument(OverloadInfo method, ParameterInfo parameter) {
+            return  true; // no constraints, but by default OverloadResolver doesn't recognize named arguments anyway
+        }
+
         /// <summary>
         /// Called before arguments binding.
         /// </summary>
@@ -191,7 +210,7 @@ protected virtual bool BindToUnexpandedParams(MethodCandidate candidate) {
         /// A default mapping will be constructed for the remaining parameters (cleared bits).
         /// </returns>
         [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Design", "CA1045:DoNotPassTypesByReference", MessageId = "3#")]
-        internal protected virtual BitArray MapSpecialParameters(ParameterMapping mapping) {
+        protected internal virtual BitArray MapSpecialParameters(ParameterMapping mapping) {
             if (!mapping.Overload.IsStatic) {
                 var type = mapping.Overload.DeclaringType;
                 mapping.AddParameter(new ParameterWrapper(null, type, null, ParameterBindingFlags.ProhibitNull));
@@ -289,7 +308,7 @@ private void AddBasicMethodTargets(OverloadInfo method) {
 
             var mapping = new ParameterMapping(this, method, _argNames);
 
-            mapping.MapParameters(false);
+            mapping.MapParameters(reduceByRef: false);
 
             foreach (var defaultCandidate in mapping.CreateDefaultCandidates()) {
                 AddSimpleTarget(defaultCandidate);
@@ -1188,11 +1207,15 @@ public virtual Type GetGenericInferenceType(DynamicMetaObject dynamicObject) {
         }
 
         public override string ToString() {
-            string res = string.Empty;
-            foreach (CandidateSet set in _candidateSets.Values) {
-                res += set + Environment.NewLine;
+            if (_candidateSets != null) {
+                string res = string.Empty;
+                foreach (CandidateSet set in _candidateSets.Values) {
+                    res += set + Environment.NewLine;
+                }
+                return res;
+            } else {
+                return $"<unresolved {nameof(OverloadResolver)}>";
             }
-            return res;
         }
     }
 }

Src/Microsoft.Dynamic/Actions/Calls/ParameterMapping.cs

@@ -103,7 +103,7 @@ public void AddParameter(ParameterWrapper parameter) {
 
         public void MapParameter(ParameterInfo pi) {
             int indexForArgBuilder;
-            int nameIndex = _argNames.IndexOf(pi.Name);
+            int nameIndex = _resolver.AllowByKeywordArgument(Overload, pi) ? _argNames.IndexOf(pi.Name) : -1;
             if (nameIndex == -1) {
                 // positional argument, we simply consume the next argument
                 indexForArgBuilder = ArgIndex++;

Src/Microsoft.Dynamic/Actions/Calls/ParamsDictArgBuilder.cs

@@ -5,6 +5,7 @@
 using System;
 using System.Collections;
 using System.Collections.Generic;
+using System.Diagnostics;
 using System.Linq.Expressions;
 using System.Reflection;
 
@@ -38,12 +39,15 @@ public ParamsDictArgBuilder(ParameterInfo info, int argIndex, string[] names, in
 
         protected internal override Expression ToExpression(OverloadResolver resolver, RestrictedArguments args, bool[] hasBeenUsed) {
             Type dictType = ParameterInfo.ParameterType;
-            // TODO: bug: what if ConstantNames().Length > hasBeenUsed.Count(b => !b)?
+            var names = ConstantNames();
+            var expressions = GetParameters(args, hasBeenUsed);
+
+            Debug.Assert(names.Length == expressions.Count);
 
             return Expression.Call(
                 GetCreationDelegate(dictType).GetMethodInfo(),
-                Expression.NewArrayInit(typeof(string), ConstantNames()),
-                AstUtils.NewArrayHelper(typeof(object), GetParameters(args, hasBeenUsed))
+                Expression.NewArrayInit(typeof(string), names),
+                AstUtils.NewArrayHelper(typeof(object), expressions)
             );
         }
 

Src/Microsoft.Scripting/Runtime/ParamDictionaryAttribute.cs

@@ -6,8 +6,8 @@
 
 namespace Microsoft.Scripting {
     /// <summary>
-    /// This attribute is used to mark a parameter that can accept any keyword parameters that
-    /// are not bound to normal arguments.  The extra keyword parameters will be
+    /// This attribute is used to mark a parameter that can accept any keyword arguments that
+    /// are not bound to normal parameters.  The extra keyword arguments will be
     /// passed in a dictionary which is created for the call.
     /// 
     /// Most languages which support params dictionaries will support the following types:

Tests/ClrAssembly/Src/methodargs.cs

@@ -30,7 +30,9 @@ public class VariousParameters {
         public void M200(int arg) { Flag.Set(arg); }
         public void M201([DefaultParameterValue(20)] int arg) { Flag.Set(arg); }
         public void M202(params int[] arg) { Flag.Set(arg.Length); }
-        public void M203([ParamDictionaryAttribute] IDictionary<object, object> arg) { Flag.Set(arg.Count); }
+        public void M203([ParamDictionary] IDictionary<object, object> arg) { Flag.Set(arg.Count); }
+        public void M204(params object[] arg) { Flag.Set(arg.Length); }
+        public void M205([ParamDictionary] IDictionary<string, int> arg) { Flag.Set(arg.Count); }
 
         // optional (get missing value)
         // - check the value actually passed in
@@ -46,11 +48,11 @@ public class VariousParameters {
         // two parameters
         public void M300(int x, int y) { }
         public void M350(int x, params int[] y) { }
-        public void M351(int x, [ParamDictionary] IDictionary<object, object> arg) { Flag<object>.Set(arg); }
-        public void M352([ParamDictionary] IDictionary<object, object> arg, params int[] x) { Flag<object>.Set(arg); }
+        public void M351(int x, [ParamDictionary] IDictionary<object, object> y) { Flag<object>.Set(y); }
+        public void M352([ParamDictionary] IDictionary<object, object> x, params object[] y) { Flag<object>.Set(x); }
 
         public void M310(int x, [DefaultParameterValue(30)]int y) { Flag.Set(x + y); }
-        public void M320([DefaultParameterValue(40)] int y, int x) { Flag.Set(x + y); }
+        public void M320([DefaultParameterValue(40)] int x, int y) { Flag.Set(x + y); }
         public void M330([DefaultParameterValue(50)] int x, [DefaultParameterValue(60)] int y) { Flag.Set(x + y); }
 
         public void M410(int x, [Optional]int y) { }