Collection expressions: require applicable constructor and Add method for target type implementing IEnumerable

docs/compilers/CSharp/Compiler Breaking Changes - DotNet 8.md

@@ -1,5 +1,42 @@
 # This document lists known breaking changes in Roslyn after .NET 7 all the way to .NET 8.
 
+## Collection expression target type must have constructor and `Add` method
+
+***Introduced in Visual Studio 2022 version 17.10***
+
+*Conversion* of a collection expression to a `struct` or `class` that implements `System.Collections.IEnumerable` and *does not* have a `CollectionBuilderAttribute`
+requires the target type to have an accessible constructor that can be called with no arguments and,
+if the collection expression is not empty, the target type must have an accessible `Add` method
+that can be called with a single argument of [*iteration type*](https://github.com/dotnet/csharpstandard/blob/standard-v7/standard/statements.md#1395-the-foreach-statement) of the target type.
+
+Previously, the constructor and `Add` methods were required for *construction* of the collection instance but not for *conversion*.
+That meant the following call was ambiguous since both `char[]` and `string` were valid target types for the collection expression.
+The call is no longer ambiguous because `string` does not have a parameterless constructor or `Add` method.
+```csharp
+Print(['a', 'b', 'c']); // calls Print(char[])
+
+static void Print(char[] arg) { }
+static void Print(string arg) { }
+```
+
+Previously, the collection expression in `y = [1, 2, 3]` was allowed since construction only requires an applicable `Add` method for each element expression.
+The collection expression is now an error because of the conversion requirement for an `Add` method than be called with an argument of the iteration type `object`.
+```csharp
+// ok: Add is not required for empty collection
+MyCollection x = [];
+
+// error CS9215: Collection expression type must have an applicable instance or extension method 'Add'
+//               that can be called with an argument of iteration type 'object'.
+//               The best overloaded method is 'MyCollection.Add(int)'.
+MyCollection y = [1, 2, 3];
+
+class MyCollection : IEnumerable
+{
+    public void Add(int i) { ... }
+    IEnumerator IEnumerable.GetEnumerator() { ... }
+}
+```
+
 ## `ref` arguments can be passed to `in` parameters
 
 ***Introduced in Visual Studio 2022 version 17.8p2***

src/Analyzers/CSharp/Tests/UseCollectionInitializer/UseCollectionInitializerTests_CollectionExpression.cs

@@ -5151,6 +5151,138 @@ void M()
             """);
     }
 
+    [Fact]
+    public async Task TestInDictionary_Empty()
+    {
+        await TestInRegularAndScriptAsync(
+            """
+            using System.Collections.Generic;
+            class Program
+            {
+                static void Main()
+                {
+                    Dictionary<string, object> d = [|new|] Dictionary<string, object>() { };
+                }
+            }
+            """,
+            """
+            using System.Collections.Generic;
+            class Program
+            {
+                static void Main()
+                {
+                    Dictionary<string, object> d = [];
+                }
+            }
+            """);
+    }
+
+    [Fact]
+    public async Task TestInDictionary_NotEmpty()
+    {
+        await TestMissingInRegularAndScriptAsync(
+            """
+            using System.Collections.Generic;
+            class Program
+            {
+                static void Main()
+                {
+                    Dictionary<string, object> d = new Dictionary<string, object>() { { string.Empty, null } };
+                }
+            }
+            """);
+    }
+
+    [Fact]
+    public async Task TestInIEnumerableAndIncompatibleAdd_Empty()
+    {
+        await TestInRegularAndScriptAsync(
+            """
+            using System.Collections;
+            class Program
+            {
+                static void Main()
+                {
+                    MyCollection c = [|new|] MyCollection() { };
+                }
+            }
+            class MyCollection : IEnumerable
+            {
+                public void Add(string s) { }
+                IEnumerator IEnumerable.GetEnumerator() => null;
+            }
+            """,
+            """
+            using System.Collections;
+            class Program
+            {
+                static void Main()
+                {
+                    MyCollection c = [];
+                }
+            }
+            class MyCollection : IEnumerable
+            {
+                public void Add(string s) { }
+                IEnumerator IEnumerable.GetEnumerator() => null;
+            }
+            """);
+    }
+
+    [Fact]
+    public async Task TestInIEnumerableAndIncompatibleAdd_NotEmpty()
+    {
+        await new VerifyCS.Test
+        {
+            ReferenceAssemblies = Testing.ReferenceAssemblies.NetCore.NetCoreApp31,
+            TestCode = """
+                using System.Collections;
+                class Program
+                {
+                    static void Main()
+                    {
+                        MyCollection c = [|new|] MyCollection() { "a", "b" };
+                    }
+                }
+                class MyCollection : IEnumerable
+                {
+                    public void Add(string s) { }
+                    IEnumerator IEnumerable.GetEnumerator() => null;
+                }
+                """,
+            FixedCode = """
+                using System.Collections;
+                class Program
+                {
+                    static void Main()
+                    {
+                        MyCollection c = ["a", "b"];
+                    }
+                }
+                class MyCollection : IEnumerable
+                {
+                    public void Add(string s) { }
+                    IEnumerator IEnumerable.GetEnumerator() => null;
+                }
+                """,
+            LanguageVersion = LanguageVersion.CSharp12,
+            TestState =
+            {
+                OutputKind = OutputKind.DynamicallyLinkedLibrary,
+            },
+            FixedState =
+            {
+                ExpectedDiagnostics =
+                {
+                    // /0/Test0.cs(6,26): error CS1503: Argument 1: cannot convert from 'object' to 'string'
+                    DiagnosticResult.CompilerError("CS1503").WithSpan(6, 26, 6, 36).WithArguments("1", "object", "string"),
+                    // /0/Test0.cs(6,26): error CS9215: Collection expression type must have an applicable instance or extension method 'Add' that can be called with an argument of iteration type 'object'. The best overloaded method is 'MyCollection.Add(string)'.
+                    DiagnosticResult.CompilerError("CS9215").WithSpan(6, 26, 6, 36).WithArguments("object", "MyCollection.Add(string)"),
+                }
+            }
+        }.RunAsync();
+    }
+
     [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/71607")]
     public async Task TestAddRangeOfCollectionExpression1()
     {

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

@@ -112,6 +112,12 @@ internal enum BinderFlags : uint
         /// </summary>
         InExpressionTree = 1 << 30,
 
+        /// <summary>
+        /// Indicates the binder is used during collection expression conversion
+        /// to verify applicable methods are available.
+        /// </summary>
+        CollectionExpressionConversionValidation = 1u << 31,
+
         // Groups
 
         AllClearedAtExecutableCodeBoundary = InLockBody | InCatchBlock | InCatchFilter | InFinallyBlock | InTryBlockOfTryCatch | InNestedFinallyBlock,

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

@@ -654,35 +654,25 @@ private BoundCollectionExpression ConvertCollectionExpression(
             if (collectionTypeKind is CollectionExpressionTypeKind.ImplementsIEnumerable)
             {
                 implicitReceiver = new BoundObjectOrCollectionValuePlaceholder(syntax, isNewInstance: true, targetType) { WasCompilerGenerated = true };
-                if (targetType is NamedTypeSymbol namedType)
-                {
-                    var analyzedArguments = AnalyzedArguments.GetInstance();
-                    // https://github.com/dotnet/roslyn/issues/68785: Use ctor with `int capacity` when the size is known.
-                    collectionCreation = BindClassCreationExpression(syntax, namedType.Name, syntax, namedType, analyzedArguments, diagnostics);
-                    collectionCreation.WasCompilerGenerated = true;
-                    analyzedArguments.Free();
-                }
-                else if (targetType is TypeParameterSymbol typeParameter)
-                {
-                    var analyzedArguments = AnalyzedArguments.GetInstance();
-                    collectionCreation = BindTypeParameterCreationExpression(syntax, typeParameter, analyzedArguments, initializerOpt: null, typeSyntax: syntax, wasTargetTyped: true, diagnostics);
-                    collectionCreation.WasCompilerGenerated = true;
-                    analyzedArguments.Free();
-                }
-                else
-                {
-                    collectionCreation = new BoundBadExpression(syntax, LookupResultKind.NotCreatable, ImmutableArray<Symbol?>.Empty, ImmutableArray<BoundExpression>.Empty, targetType);
-                }
+                collectionCreation = BindCollectionExpressionConstructor(syntax, targetType, diagnostics);
 
                 var collectionInitializerAddMethodBinder = this.WithAdditionalFlags(BinderFlags.CollectionInitializerAddMethod);
                 foreach (var element in elements)
                 {
-                    BoundNode convertedElement = BindCollectionExpressionElementAddMethod(
-                        element,
-                        collectionInitializerAddMethodBinder,
-                        implicitReceiver,
-                        diagnostics,
-                        out _);
+                    BoundNode convertedElement = element is BoundCollectionExpressionSpreadElement spreadElement ?
+                        (BoundNode)BindCollectionExpressionSpreadElementAddMethod(
+                            (SpreadElementSyntax)spreadElement.Syntax,
+                            spreadElement,
+                            collectionInitializerAddMethodBinder,
+                            implicitReceiver,
+                            diagnostics) :
+                        BindCollectionInitializerElementAddMethod(
+                            (ExpressionSyntax)element.Syntax,
+                            ImmutableArray.Create((BoundExpression)element),
+                            hasEnumerableInitializerType: true,
+                            collectionInitializerAddMethodBinder,
+                            diagnostics,
+                            implicitReceiver);
                     builder.Add(convertedElement);
                 }
             }
@@ -766,6 +756,50 @@ BoundNode bindSpreadElement(BoundCollectionExpressionSpreadElement element, Type
             }
         }
 
+        internal BoundExpression BindCollectionExpressionConstructor(SyntaxNode syntax, TypeSymbol targetType, BindingDiagnosticBag diagnostics)
+        {
+            BoundExpression collectionCreation;
+            var analyzedArguments = AnalyzedArguments.GetInstance();
+            if (targetType is NamedTypeSymbol namedType)
+            {
+                var binder = WithAdditionalFlags(BinderFlags.CollectionExpressionConversionValidation);
+                collectionCreation = binder.BindClassCreationExpression(syntax, namedType.Name, syntax, namedType, analyzedArguments, diagnostics);
+                collectionCreation.WasCompilerGenerated = true;
+            }
+            else if (targetType is TypeParameterSymbol typeParameter)
+            {
+                collectionCreation = BindTypeParameterCreationExpression(syntax, typeParameter, analyzedArguments, initializerOpt: null, typeSyntax: syntax, wasTargetTyped: true, diagnostics);
+                collectionCreation.WasCompilerGenerated = true;
+            }
+            else
+            {
+                throw ExceptionUtilities.UnexpectedValue(targetType);
+            }
+            analyzedArguments.Free();
+            return collectionCreation;
+        }
+
+        internal bool HasCollectionExpressionApplicableConstructor(SyntaxNode syntax, TypeSymbol targetType, BindingDiagnosticBag diagnostics)
+        {
+            var collectionCreation = BindCollectionExpressionConstructor(syntax, targetType, diagnostics);
+            return !collectionCreation.HasErrors;
+        }
+
+        internal bool HasCollectionExpressionApplicableAddMethod(SyntaxNode syntax, TypeSymbol targetType, TypeSymbol elementType, BindingDiagnosticBag diagnostics)
+        {
+            var implicitReceiver = new BoundObjectOrCollectionValuePlaceholder(syntax, isNewInstance: true, targetType) { WasCompilerGenerated = true };
+            var elementPlaceholder = new BoundValuePlaceholder(syntax, elementType) { WasCompilerGenerated = true };
+            var addMethodBinder = WithAdditionalFlags(BinderFlags.CollectionInitializerAddMethod | BinderFlags.CollectionExpressionConversionValidation);
+            var result = BindCollectionInitializerElementAddMethod(
+                syntax,
+                ImmutableArray.Create<BoundExpression>(elementPlaceholder),
+                hasEnumerableInitializerType: true,
+                addMethodBinder,
+                diagnostics,
+                implicitReceiver);
+            return !result.HasErrors;
+        }
+
         /// <summary>
         /// If the element is from a collection type where elements are added with collection initializers,
         /// return the argument to the collection initializer Add method or null if the element is not a
@@ -865,10 +899,24 @@ private void GenerateImplicitConversionErrorForCollectionExpression(
 
             if (collectionTypeKind != CollectionExpressionTypeKind.None)
             {
+                var elements = node.Elements;
                 var elementType = elementTypeWithAnnotations.Type;
                 Debug.Assert(elementType is { });
 
-                var elements = node.Elements;
+                if (collectionTypeKind == CollectionExpressionTypeKind.ImplementsIEnumerable)
+                {
+                    if (!HasCollectionExpressionApplicableConstructor(node.Syntax, targetType, diagnostics))
+                    {
+                        reportedErrors = true;
+                    }
+
+                    if (elements.Length > 0 &&
+                        !HasCollectionExpressionApplicableAddMethod(node.Syntax, targetType, elementType, diagnostics))
+                    {
+                        reportedErrors = true;
+                    }
+                }
+
                 var useSiteInfo = GetNewCompoundUseSiteInfo(diagnostics);
                 foreach (var element in elements)
                 {

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

@@ -5819,7 +5819,7 @@ private BoundExpression BindUnexpectedComplexElementInitializer(InitializerExpre
         }
 
         private BoundExpression BindCollectionInitializerElementAddMethod(
-            ExpressionSyntax elementInitializer,
+            SyntaxNode elementInitializer,
             ImmutableArray<BoundExpression> boundElementInitializerExpressions,
             bool hasEnumerableInitializerType,
             Binder collectionInitializerAddMethodBinder,
@@ -5937,31 +5937,6 @@ static void copyRelevantAddMethodDiagnostics(BindingDiagnosticBag source, Bindin
         }
 
 #nullable enable
-        internal BoundNode BindCollectionExpressionElementAddMethod(
-            BoundNode element,
-            Binder collectionInitializerAddMethodBinder,
-            BoundObjectOrCollectionValuePlaceholder implicitReceiver,
-            BindingDiagnosticBag diagnostics,
-            out bool hasErrors)
-        {
-            var result = element is BoundCollectionExpressionSpreadElement spreadElement ?
-                (BoundNode)BindCollectionExpressionSpreadElementAddMethod(
-                    (SpreadElementSyntax)spreadElement.Syntax,
-                    spreadElement,
-                    collectionInitializerAddMethodBinder,
-                    implicitReceiver,
-                    diagnostics) :
-                BindCollectionInitializerElementAddMethod(
-                    (ExpressionSyntax)element.Syntax,
-                    ImmutableArray.Create((BoundExpression)element),
-                    hasEnumerableInitializerType: true,
-                    collectionInitializerAddMethodBinder,
-                    diagnostics,
-                    implicitReceiver);
-            hasErrors = result.HasErrors;
-            return result;
-        }
-
         private BoundCollectionExpressionSpreadElement BindCollectionExpressionSpreadElementAddMethod(
             SpreadElementSyntax syntax,
             BoundCollectionExpressionSpreadElement element,

src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/Conversions.cs

@@ -9,7 +9,6 @@
 using System.Linq;
 using Microsoft.CodeAnalysis.CSharp.Symbols;
 using Microsoft.CodeAnalysis.PooledObjects;
-using Roslyn.Utilities;
 
 namespace Microsoft.CodeAnalysis.CSharp
 {
@@ -182,8 +181,22 @@ protected override Conversion GetCollectionExpressionConversion(
             }
 
             Debug.Assert(elementType is { });
-
             var elements = node.Elements;
+
+            if (collectionTypeKind == CollectionExpressionTypeKind.ImplementsIEnumerable)
+            {
+                if (!_binder.HasCollectionExpressionApplicableConstructor(syntax, targetType, BindingDiagnosticBag.Discarded))
+                {
+                    return Conversion.NoConversion;
+                }
+
+                if (elements.Length > 0 &&
+                    !_binder.HasCollectionExpressionApplicableAddMethod(syntax, targetType, elementType, BindingDiagnosticBag.Discarded))
+                {
+                    return Conversion.NoConversion;
+                }
+            }
+
             var builder = ArrayBuilder<Conversion>.GetInstance(elements.Length);
             foreach (var element in elements)
             {