Fix serialization of attribute data to account for named params argument (#22231)

src/Compilers/CSharp/Portable/Binder/Binder_Attributes.cs

@@ -578,8 +578,12 @@ private TypeSymbol BindNamedAttributeArgumentType(AttributeArgumentSyntax namedA
 
                 if (parameter.IsParams && parameter.Type.IsSZArray() && i + 1 == parameterCount)
                 {
-                    reorderedArgument = GetParamArrayArgument(parameter, constructorArgsArray, argumentsCount, argsConsumedCount, this.Conversions);
-                    sourceIndices = sourceIndices ?? CreateSourceIndicesArray(i, parameterCount);
+                    reorderedArgument = GetParamArrayArgument(parameter, constructorArgsArray, constructorArgumentNamesOpt, argumentsCount,
+                        argsConsumedCount, this.Conversions, out bool foundNamed);
+                    if (!foundNamed)
+                    {
+                        sourceIndices = sourceIndices ?? CreateSourceIndicesArray(i, parameterCount);
+                    }
                 }
                 else if (argsConsumedCount < argumentsCount)
                 {
@@ -820,41 +824,48 @@ private TypedConstant GetDefaultValueArgument(ParameterSymbol parameter, Attribu
             }
         }
 
-        private static TypedConstant GetParamArrayArgument(ParameterSymbol parameter, ImmutableArray<TypedConstant> constructorArgsArray, int argumentsCount, int argsConsumedCount, Conversions conversions)
+        private static TypedConstant GetParamArrayArgument(ParameterSymbol parameter, ImmutableArray<TypedConstant> constructorArgsArray,
+            ImmutableArray<string> constructorArgumentNamesOpt, int argumentsCount, int argsConsumedCount, Conversions conversions, out bool foundNamed)
         {
             Debug.Assert(argsConsumedCount <= argumentsCount);
 
+            // If there's a named argument, we'll use that
+            if (!constructorArgumentNamesOpt.IsDefault)
+            {
+                int argIndex = constructorArgumentNamesOpt.IndexOf(parameter.Name);
+                if (argIndex >= 0)
+                {
+                    foundNamed = true;
+                    if (TryGetNormalParamValue(parameter, constructorArgsArray, argIndex, conversions, out var namedValue))
+                    {
+                        return namedValue;
+                    }
+
+                    // A named argument for a params parameter is necessarily the only one for that parameter
+                    return new TypedConstant(parameter.Type, ImmutableArray.Create(constructorArgsArray[argIndex]));
+                }
+            }
+
             int paramArrayArgCount = argumentsCount - argsConsumedCount;
+            foundNamed = false;
+
+            // If there are zero arguments left
             if (paramArrayArgCount == 0)
             {
                 return new TypedConstant(parameter.Type, ImmutableArray<TypedConstant>.Empty);
             }
 
-            // If there's exactly one argument and it's an array of an appropriate type, then just return it.
-            if (paramArrayArgCount == 1 && constructorArgsArray[argsConsumedCount].Kind == TypedConstantKind.Array)
+            // If there's exactly one argument left, we'll try to use it in normal form
+            if (paramArrayArgCount == 1 &&
+                TryGetNormalParamValue(parameter, constructorArgsArray, argsConsumedCount, conversions, out var lastValue))
             {
-                TypeSymbol argumentType = (TypeSymbol)constructorArgsArray[argsConsumedCount].Type;
-
-                // Easy out (i.e. don't both classifying conversion).
-                if (argumentType == parameter.Type)
-                {
-                    return constructorArgsArray[argsConsumedCount];
-                }
-
-                HashSet<DiagnosticInfo> useSiteDiagnostics = null; // ignoring, since already bound argument and parameter
-                Conversion conversion = conversions.ClassifyBuiltInConversion(argumentType, parameter.Type, ref useSiteDiagnostics);
-
-                // NOTE: Won't always succeed, even though we've performed overload resolution.
-                // For example, passing int[] to params object[] actually treats the int[] as an element of the object[].
-                if (conversion.IsValid && conversion.Kind == ConversionKind.ImplicitReference)
-                {
-                    return constructorArgsArray[argsConsumedCount];
-                }
+                return lastValue;
             }
 
             Debug.Assert(!constructorArgsArray.IsDefault);
             Debug.Assert(argsConsumedCount <= constructorArgsArray.Length);
 
+            // Take the trailing arguments as an array for expanded form
             var values = new TypedConstant[paramArrayArgCount];
 
             for (int i = 0; i < paramArrayArgCount; i++)
@@ -865,6 +876,39 @@ private static TypedConstant GetParamArrayArgument(ParameterSymbol parameter, Im
             return new TypedConstant(parameter.Type, values.AsImmutableOrNull());
         }
 
+        private static bool TryGetNormalParamValue(ParameterSymbol parameter, ImmutableArray<TypedConstant> constructorArgsArray,
+            int argIndex, Conversions conversions, out TypedConstant result)
+        {
+            TypedConstant argument = constructorArgsArray[argIndex];
+            if (argument.Kind != TypedConstantKind.Array)
+            {
+                result = default;
+                return false;
+            }
+
+            TypeSymbol argumentType = (TypeSymbol)argument.Type;
+            // Easy out (i.e. don't bother classifying conversion).
+            if (argumentType == parameter.Type)
+            {
+                result = argument;
+                return true;
+            }
+
+            HashSet<DiagnosticInfo> useSiteDiagnostics = null; // ignoring, since already bound argument and parameter
+            Conversion conversion = conversions.ClassifyBuiltInConversion(argumentType, parameter.Type, ref useSiteDiagnostics);
+
+            // NOTE: Won't always succeed, even though we've performed overload resolution.
+            // For example, passing int[] to params object[] actually treats the int[] as an element of the object[].
+            if (conversion.IsValid && conversion.Kind == ConversionKind.ImplicitReference)
+            {
+                result = argument;
+                return true;
+            }
+
+            result = default;
+            return false;
+        }
+
         #endregion
 
         #region AttributeExpressionVisitor

src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests.cs

@@ -6,7 +6,6 @@
 using System.IO;
 using System.Linq;
 using System.Reflection;
-using System.Threading;
 using Microsoft.CodeAnalysis.CSharp.Symbols;
 using Microsoft.CodeAnalysis.CSharp.Symbols.Metadata.PE;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
@@ -62,6 +61,286 @@ class C
             CompileAndVerify(source, sourceSymbolValidator: attributeValidator, symbolValidator: null);
         }
 
+        [Fact]
+        [WorkItem(20741, "https://github.com/dotnet/roslyn/issues/20741")]
+        public void TestNamedArgumentOnStringParamsArgument()
+        {
+            var comp = CreateCompilationWithMscorlib46(@"
+using System;
+
+class MarkAttribute : Attribute
+{
+    public MarkAttribute(bool a, params object[] b)
+    {
+    }
+}
+
+[Mark(b: new string[] { ""Hello"", ""World"" }, a: true)]
+[Mark(b: ""Hello"", true)]
+static class Program
+{
+}", parseOptions: TestOptions.Regular7_2);
+            comp.VerifyDiagnostics(
+                // (11,2): error CS0182: An attribute argument must be a constant expression, typeof expression or array creation expression of an attribute parameter type
+                // [Mark(b: new string[] { "Hello", "World" }, a: true)]
+                Diagnostic(ErrorCode.ERR_BadAttributeArgument, @"Mark(b: new string[] { ""Hello"", ""World"" }, a: true)").WithLocation(11, 2),
+                // (12,7): error CS8323: Named argument 'b' is used out-of-position but is followed by an unnamed argument
+                // [Mark(b: "Hello", true)]
+                Diagnostic(ErrorCode.ERR_BadNonTrailingNamedArgument, "b").WithArguments("b").WithLocation(12, 7)
+                );
+        }
+
+        [Fact]
+        [WorkItem(20741, "https://github.com/dotnet/roslyn/issues/20741")]
+        public void TestNamedArgumentOnOrderedObjectParamsArgument()
+        {
+            var comp = CreateCompilationWithMscorlib46(@"
+using System;
+using System.Reflection;
+
+sealed class MarkAttribute : Attribute
+{
+    public MarkAttribute(bool a, params object[] b)
+    {
+        B = b;
+    }
+    public object[] B { get; }
+}
+
+[Mark(a: true, b: new object[] { ""Hello"", ""World"" })]
+static class Program
+{
+    public static void Main()
+    {
+        var attr = typeof(Program).GetCustomAttribute<MarkAttribute>();
+        Console.Write($""B.Length={attr.B.Length}, B[0]={attr.B[0]}, B[1]={attr.B[1]}"");
+    }
+}", options: TestOptions.DebugExe);
+            comp.VerifyDiagnostics();
+
+            CompileAndVerify(comp, expectedOutput: @"B.Length=2, B[0]=Hello, B[1]=World");
+
+            var program = (NamedTypeSymbol)comp.GetMember("Program");
+            var attributeData = (SourceAttributeData)program.GetAttributes()[0];
+            Assert.Equal(new[] { 0, -1 }, attributeData.ConstructorArgumentsSourceIndices);
+        }
+
+        [Fact]
+        [WorkItem(20741, "https://github.com/dotnet/roslyn/issues/20741")]
+        public void TestNamedArgumentOnObjectParamsArgument()
+        {
+            var comp = CreateCompilationWithMscorlib46(@"
+using System;
+using System.Reflection;
+
+sealed class MarkAttribute : Attribute
+{
+    public MarkAttribute(bool a, params object[] b)
+    {
+        B = b;
+    }
+    public object[] B { get; }
+}
+
+[Mark(b: new object[] { ""Hello"", ""World"" }, a: true)]
+static class Program
+{
+    public static void Main()
+    {
+        var attr = typeof(Program).GetCustomAttribute<MarkAttribute>();
+        Console.Write($""B.Length={attr.B.Length}, B[0]={attr.B[0]}, B[1]={attr.B[1]}"");
+    }
+}", options: TestOptions.DebugExe);
+            comp.VerifyDiagnostics();
+
+            CompileAndVerify(comp, expectedOutput: @"B.Length=2, B[0]=Hello, B[1]=World");
+
+            var program = (NamedTypeSymbol)comp.GetMember("Program");
+            var attributeData = (SourceAttributeData)program.GetAttributes()[0];
+            Assert.Equal(new[] { 1, -1 }, attributeData.ConstructorArgumentsSourceIndices);
+        }
+
+        [Fact]
+        [WorkItem(20741, "https://github.com/dotnet/roslyn/issues/20741")]
+        public void TestNamedArgumentOnObjectParamsArgument2()
+        {
+            var comp = CreateCompilationWithMscorlib46(@"
+using System;
+using System.Reflection;
+
+sealed class MarkAttribute : Attribute
+{
+    public MarkAttribute(bool a, params object[] b)
+    {
+        A = a;
+        B = b;
+    }
+    public bool A { get; }
+    public object[] B { get; }
+}
+
+[Mark(b: ""Hello"", a: true)]
+static class Program
+{
+    public static void Main()
+    {
+        var attr = typeof(Program).GetCustomAttribute<MarkAttribute>();
+        Console.Write($""A={attr.A}, B.Length={attr.B.Length}, B[0]={attr.B[0]}"");
+    }
+}", options: TestOptions.DebugExe, parseOptions: TestOptions.Regular7_2);
+            comp.VerifyDiagnostics();
+
+            CompileAndVerify(comp, expectedOutput: @"A=True, B.Length=1, B[0]=Hello");
+
+            var program = (NamedTypeSymbol)comp.GetMember("Program");
+            var attributeData = (SourceAttributeData)program.GetAttributes()[0];
+            Assert.Equal(new[] { 1, -1 }, attributeData.ConstructorArgumentsSourceIndices);
+        }
+
+        [Fact]
+        [WorkItem(20741, "https://github.com/dotnet/roslyn/issues/20741")]
+        public void TestNamedArgumentOnObjectParamsArgument3()
+        {
+            var comp = CreateCompilationWithMscorlib46(@"
+using System;
+using System.Reflection;
+
+sealed class MarkAttribute : Attribute
+{
+    public MarkAttribute(bool a, params object[] b)
+    {
+        B = b;
+    }
+    public object[] B { get; }
+}
+
+[Mark(true, new object[] { ""Hello"" }, new object[] { ""World"" })]
+static class Program
+{
+    public static void Main()
+    {
+        var attr = typeof(Program).GetCustomAttribute<MarkAttribute>();
+        var worldArray = (object[])attr.B[1];
+        Console.Write(worldArray[0]);
+    }
+}", options: TestOptions.DebugExe);
+            comp.VerifyDiagnostics();
+
+            CompileAndVerify(comp, expectedOutput: @"World");
+
+            var program = (NamedTypeSymbol)comp.GetMember("Program");
+            var attributeData = (SourceAttributeData)program.GetAttributes()[0];
+            Assert.Equal(new[] { 0, -1 }, attributeData.ConstructorArgumentsSourceIndices);
+        }
+
+        [Fact]
+        [WorkItem(20741, "https://github.com/dotnet/roslyn/issues/20741")]
+        public void TestNamedArgumentOnObjectParamsArgument4()
+        {
+            var comp = CreateCompilationWithMscorlib46(@"
+using System;
+using System.Reflection;
+
+sealed class MarkAttribute : Attribute
+{
+    public MarkAttribute(bool a, params object[] b)
+    {
+        B = b;
+    }
+    public object[] B { get; }
+}
+
+[Mark(a: true, new object[] { ""Hello"" }, new object[] { ""World"" })]
+static class Program
+{
+    public static void Main()
+    {
+        var attr = typeof(Program).GetCustomAttribute<MarkAttribute>();
+        var worldArray = (object[])attr.B[1];
+        Console.Write(worldArray[0]);
+    }
+}", options: TestOptions.DebugExe, parseOptions: TestOptions.Regular7_2);
+            comp.VerifyDiagnostics();
+
+            CompileAndVerify(comp, expectedOutput: @"World");
+
+            var program = (NamedTypeSymbol)comp.GetMember("Program");
+            var attributeData = (SourceAttributeData)program.GetAttributes()[0];
+            Assert.Equal(new[] { 0, -1 }, attributeData.ConstructorArgumentsSourceIndices);
+        }
+
+        [Fact]
+        [WorkItem(20741, "https://github.com/dotnet/roslyn/issues/20741")]
+        public void TestNamedArgumentOnObjectParamsArgument5()
+        {
+            var comp = CreateCompilationWithMscorlib46(@"
+using System;
+using System.Reflection;
+
+sealed class MarkAttribute : Attribute
+{
+    public MarkAttribute(bool a, params object[] b)
+    {
+        B = b;
+    }
+    public object[] B { get; }
+}
+
+[Mark(b: null, a: true)]
+static class Program
+{
+    public static void Main()
+    {
+        var attr = typeof(Program).GetCustomAttribute<MarkAttribute>();
+        Console.Write(attr.B == null);
+    }
+}", options: TestOptions.DebugExe);
+            comp.VerifyDiagnostics();
+
+            CompileAndVerify(comp, expectedOutput: @"True");
+
+            var program = (NamedTypeSymbol)comp.GetMember("Program");
+            var attributeData = (SourceAttributeData)program.GetAttributes()[0];
+            Assert.Equal(new[] { 1, -1 }, attributeData.ConstructorArgumentsSourceIndices);
+        }
+
+        [Fact]
+        [WorkItem(20741, "https://github.com/dotnet/roslyn/issues/20741")]
+        public void TestNamedArgumentOnNonParamsArgument()
+        {
+            var comp = CreateCompilationWithMscorlib46(@"
+using System;
+using System.Reflection;
+
+sealed class MarkAttribute : Attribute
+{
+    public MarkAttribute(int a, int b)
+    {
+        A = a;
+        B = b;
+    }
+    public int A { get; }
+    public int B { get;  }
+}
+
+[Mark(b: 42, a: 1)]
+static class Program
+{
+    public static void Main()
+    {
+        var attr = typeof(Program).GetCustomAttribute<MarkAttribute>();
+        Console.Write($""A={attr.A}, B={attr.B}"");
+    }
+}", options: TestOptions.DebugExe);
+            comp.VerifyDiagnostics();
+
+            CompileAndVerify(comp, expectedOutput: "A=1, B=42");
+
+            var program = (NamedTypeSymbol)comp.GetMember("Program");
+            var attributeData = (SourceAttributeData)program.GetAttributes()[0];
+            Assert.Equal(new[] { 1, 0 }, attributeData.ConstructorArgumentsSourceIndices);
+        }
+
         [WorkItem(984896, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/984896")]
         [Fact]
         public void TestAssemblyAttributesErr()

src/Compilers/VisualBasic/Test/Emit/Attributes/AttributeTests.vb

@@ -63,6 +63,54 @@ End Class
             CompilationUtils.AssertNoDiagnostics(comp)
         End Sub
 
+        <Fact>
+        <WorkItem(20741, "https://github.com/dotnet/roslyn/issues/20741")>
+        Public Sub TestNamedArgumentOnStringParamsArgument()
+            Dim source =
+                <compilation>
+                    <file name="a.vb">
+                        <![CDATA[
+Imports System
+
+Class MarkAttribute
+    Inherits Attribute
+
+    Public Sub New(ByVal otherArg As Boolean, ParamArray args As Object())
+    End Sub
+End Class
+
+<Mark(args:=New String() {"Hello", "World"}, otherArg:=True)>
+Module Program
+
+    Private Sub Test(ByVal otherArg As Boolean, ParamArray args As Object())
+    End Sub
+
+    Sub Main()
+        Console.WriteLine("Method call")
+        Test(args:=New String() {"Hello", "World"}, otherArg:=True)
+    End Sub
+End Module
+]]>
+                    </file>
+                </compilation>
+
+            Dim comp = CreateCompilationWithMscorlibAndVBRuntime(source)
+            comp.AssertTheseDiagnostics(<errors><![CDATA[
+BC30455: Argument not specified for parameter 'otherArg' of 'Public Sub New(otherArg As Boolean, ParamArray args As Object())'.
+<Mark(args:=New String() {"Hello", "World"}, otherArg:=True)>
+ ~~~~
+BC30661: Field or property 'args' is not found.
+<Mark(args:=New String() {"Hello", "World"}, otherArg:=True)>
+      ~~~~
+BC30661: Field or property 'otherArg' is not found.
+<Mark(args:=New String() {"Hello", "World"}, otherArg:=True)>
+                                             ~~~~~~~~
+BC30587: Named argument cannot match a ParamArray parameter.
+        Test(args:=New String() {"Hello", "World"}, otherArg:=True)
+             ~~~~
+                                        ]]></errors>)
+        End Sub
+
         ''' <summary>
         ''' This function is the same as PEParameterSymbolParamArray
         ''' except that we check attributes first (to check for race