Collection expressions: require Add method callable with a single argument for class and struct types that do not use [CollectionBuilder]

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

@@ -35,7 +35,7 @@ public class C
 *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.
+that can be called with a single argument.
 
 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.
@@ -47,24 +47,6 @@ 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

@@ -5270,16 +5270,6 @@ public void Add(string s) { }
             {
                 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();
     }
 

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

@@ -5933,7 +5933,7 @@ boundElementInitializerExpressions[0] is not
                 var d = BindingDiagnosticBag.GetInstance();
 
                 // This assert provides some validation that, if the real invocation binding succeeds, then the HasCollectionExpressionApplicableAddMethod helper succeeds as well.
-                Debug.Assert(collectionInitializerAddMethodBinder.HasCollectionExpressionApplicableAddMethod(elementInitializer, implicitReceiver.Type, boundElementInitializerExpressions[0].Type, addMethods: out _, d));
+                Debug.Assert(collectionInitializerAddMethodBinder.HasCollectionExpressionApplicableAddMethod(elementInitializer, implicitReceiver.Type, addMethods: out _, d));
 
                 d.Free();
             }
@@ -7919,7 +7919,8 @@ protected MethodGroupResolution BindExtensionMethod(
                                       OverloadResolution.Options.IsFunctionPointerResolution |
                                       OverloadResolution.Options.InferWithDynamic |
                                       OverloadResolution.Options.IgnoreNormalFormIfHasValidParamsParameter |
-                                      OverloadResolution.Options.DynamicResolution)) == 0);
+                                      OverloadResolution.Options.DynamicResolution |
+                                      OverloadResolution.Options.DynamicConvertsToAnything)) == 0);
 
             var firstResult = new MethodGroupResolution();
             AnalyzedArguments actualArguments = null;
@@ -9980,7 +9981,8 @@ private MethodGroupResolution ResolveDefaultMethodGroup(
                                       OverloadResolution.Options.IsFunctionPointerResolution |
                                       OverloadResolution.Options.InferWithDynamic |
                                       OverloadResolution.Options.IgnoreNormalFormIfHasValidParamsParameter |
-                                      OverloadResolution.Options.DynamicResolution)) == 0);
+                                      OverloadResolution.Options.DynamicResolution |
+                                      OverloadResolution.Options.DynamicConvertsToAnything)) == 0);
 
             var methods = node.Methods;
             if (methods.Length == 0)

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

@@ -388,14 +388,6 @@ private BoundExpression BindDynamicInvocation(
             BindingDiagnosticBag diagnostics,
             CSharpSyntaxNode queryClause)
         {
-            //
-            // !!! ATTENTION !!!
-            //
-            // In terms of errors relevant for HasCollectionExpressionApplicableAddMethod check
-            // this function should be kept in sync with local function
-            // HasCollectionExpressionApplicableAddMethod.bindDynamicInvocation
-            //
-
             CheckNamedArgumentsForDynamicInvocation(arguments, diagnostics);
 
             bool hasErrors = false;
@@ -891,6 +883,14 @@ private bool CanEarlyBindSingleCandidateInvocationWithDynamicArgument(
             MemberResolutionResult<MethodSymbol> methodResolutionResult,
             MethodSymbol singleCandidate)
         {
+            //
+            // !!! ATTENTION !!!
+            //
+            // In terms of errors relevant for HasCollectionExpressionApplicableAddMethod check
+            // this function should be kept in sync with local function
+            // HasCollectionExpressionApplicableAddMethod.canEarlyBindSingleCandidateInvocationWithDynamicArgument
+            //
+
             if (boundMethodGroup.TypeArgumentsOpt.IsDefaultOrEmpty && singleCandidate.IsGenericMethod)
             {
                 // If we call an unconstructed generic function with a

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

@@ -194,10 +194,12 @@ protected override Conversion GetCollectionExpressionConversion(
                 }
 
                 if (elements.Length > 0 &&
-                    !_binder.HasCollectionExpressionApplicableAddMethod(syntax, targetType, elementType, addMethods: out _, BindingDiagnosticBag.Discarded))
+                    !_binder.HasCollectionExpressionApplicableAddMethod(syntax, targetType, addMethods: out _, BindingDiagnosticBag.Discarded))
                 {
                     return Conversion.NoConversion;
                 }
+
+                // PROTOTYPE: Probably should at least accumulate dependencies and propagate them in case of success.
             }
 
             var builder = ArrayBuilder<Conversion>.GetInstance(elements.Length);

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

@@ -126,6 +126,7 @@ public enum Options : byte
             IsFunctionPointerResolution = 0b_00010000,
             IsExtensionMethodResolution = 0b_00100000,
             DynamicResolution = 0b_01000000,
+            DynamicConvertsToAnything = 0b_10000000,
         }
 
         // Perform overload resolution on the given method group, with the given arguments and
@@ -870,6 +871,7 @@ private MemberAnalysisResult IsConstructorApplicableInNormalForm(
                 hasAnyRefOmittedArgument: false,
                 ignoreOpenTypes: false,
                 completeResults: completeResults,
+                dynamicConvertsToAnything: false,
                 useSiteInfo: ref useSiteInfo);
         }
 
@@ -911,6 +913,7 @@ private MemberAnalysisResult IsConstructorApplicableInExpandedForm(
                 hasAnyRefOmittedArgument: false,
                 ignoreOpenTypes: false,
                 completeResults: completeResults,
+                dynamicConvertsToAnything: false,
                 useSiteInfo: ref useSiteInfo);
 
             Debug.Assert(!result.IsValid || result.Kind == MemberResolutionKind.ApplicableInExpandedForm);
@@ -1063,6 +1066,7 @@ private void AddMemberToCandidateSet<TMember>(
                         arguments,
                         allowRefOmittedArguments: (options & Options.AllowRefOmittedArguments) != 0,
                         completeResults: completeResults,
+                        dynamicConvertsToAnything: (options & Options.DynamicConvertsToAnything) != 0,
                         useSiteInfo: ref useSiteInfo);
 
                     if (PreferExpandedFormOverNormalForm(normalResult, expandedResult))
@@ -1210,7 +1214,7 @@ public static bool TryInferParamsCollectionIterationType(Binder binder, TypeSymb
                                 return false;
                             }
 
-                            if (!binder.HasCollectionExpressionApplicableAddMethod(syntax, type, elementType.Type, addMethods: out _, BindingDiagnosticBag.Discarded))
+                            if (!binder.HasCollectionExpressionApplicableAddMethod(syntax, type, addMethods: out _, BindingDiagnosticBag.Discarded))
                             {
                                 return false;
                             }
@@ -3702,6 +3706,7 @@ private MemberResolutionResult<TMember> IsMemberApplicableInNormalForm<TMember>(
                 hasAnyRefOmittedArgument: hasAnyRefOmittedArgument,
                 inferWithDynamic: (options & Options.InferWithDynamic) != 0,
                 completeResults: completeResults,
+                dynamicConvertsToAnything: (options & Options.DynamicConvertsToAnything) != 0,
                 useSiteInfo: ref useSiteInfo);
 
             // If we were producing complete results and had missing arguments, we pushed on in order to call IsApplicable for
@@ -3721,6 +3726,7 @@ private MemberResolutionResult<TMember> IsMemberApplicableInExpandedForm<TMember
             AnalyzedArguments arguments,
             bool allowRefOmittedArguments,
             bool completeResults,
+            bool dynamicConvertsToAnything,
             ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
             where TMember : Symbol
         {
@@ -3763,6 +3769,7 @@ private MemberResolutionResult<TMember> IsMemberApplicableInExpandedForm<TMember
                 hasAnyRefOmittedArgument: hasAnyRefOmittedArgument,
                 inferWithDynamic: false,
                 completeResults: completeResults,
+                dynamicConvertsToAnything: dynamicConvertsToAnything,
                 useSiteInfo: ref useSiteInfo);
 
             Debug.Assert(!result.Result.IsValid || result.Result.Kind == MemberResolutionKind.ApplicableInExpandedForm);
@@ -3780,6 +3787,7 @@ private MemberResolutionResult<TMember> IsApplicable<TMember>(
             bool hasAnyRefOmittedArgument,
             bool inferWithDynamic,
             bool completeResults,
+            bool dynamicConvertsToAnything,
             ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
             where TMember : Symbol
         {
@@ -3898,6 +3906,7 @@ private MemberResolutionResult<TMember> IsApplicable<TMember>(
                 hasAnyRefOmittedArgument: hasAnyRefOmittedArgument,
                 ignoreOpenTypes: ignoreOpenTypes,
                 completeResults: completeResults,
+                dynamicConvertsToAnything: dynamicConvertsToAnything,
                 useSiteInfo: ref useSiteInfo);
             return new MemberResolutionResult<TMember>(member, leastOverriddenMember, applicableResult, hasTypeArgumentsInferredFromFunctionType);
         }
@@ -3967,6 +3976,7 @@ private MemberAnalysisResult IsApplicable(
             bool hasAnyRefOmittedArgument,
             bool ignoreOpenTypes,
             bool completeResults,
+            bool dynamicConvertsToAnything,
             ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
         {
             TypeWithAnnotations paramsElementTypeOpt;
@@ -4079,9 +4089,15 @@ private MemberAnalysisResult IsApplicable(
                         ignoreOpenTypes,
                         ref useSiteInfo,
                         forExtensionMethodThisArg,
-                        hasInterpolatedStringRefMismatch);
+                        hasInterpolatedStringRefMismatch,
+                        dynamicConvertsToAnything);
 
-                    if (forExtensionMethodThisArg && !Conversions.IsValidExtensionMethodThisArgConversion(conversion))
+                    Debug.Assert(
+                        !forExtensionMethodThisArg ||
+                        (!conversion.IsDynamic ||
+                            (ignoreOpenTypes && parameters.ParameterTypes[argumentPosition].Type.ContainsTypeParameter(parameterContainer: (MethodSymbol)candidate))));
+
+                    if (forExtensionMethodThisArg && !conversion.IsDynamic && !Conversions.IsValidExtensionMethodThisArgConversion(conversion))
                     {
                         // Return early, without checking conversions of subsequent arguments,
                         // if the instance argument is not convertible to the 'this' parameter,
@@ -4150,7 +4166,8 @@ private Conversion CheckArgumentForApplicability(
             bool ignoreOpenTypes,
             ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo,
             bool forExtensionMethodThisArg,
-            bool hasInterpolatedStringRefMismatch)
+            bool hasInterpolatedStringRefMismatch,
+            bool dynamicConvertsToAnything)
         {
             // Spec 7.5.3.1
             // For each argument in A, the parameter passing mode of the argument (i.e., value, ref, or out) is identical
@@ -4191,7 +4208,9 @@ private Conversion CheckArgumentForApplicability(
             {
                 var conversion = forExtensionMethodThisArg ?
                     Conversions.ClassifyImplicitExtensionMethodThisArgConversion(argument, argument.Type, parameterType, ref useSiteInfo) :
-                    Conversions.ClassifyImplicitConversionFromExpression(argument, parameterType, ref useSiteInfo);
+                    ((!dynamicConvertsToAnything || !argument.Type.IsDynamic()) ?
+                         Conversions.ClassifyImplicitConversionFromExpression(argument, parameterType, ref useSiteInfo) :
+                         Conversion.ImplicitDynamic);
                 Debug.Assert((!conversion.Exists) || conversion.IsImplicit, "ClassifyImplicitConversion should only return implicit conversions");
 
                 if (hasInterpolatedStringRefMismatch && !conversion.IsInterpolatedStringHandler)

src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolutionResult.cs

@@ -505,23 +505,14 @@ internal void ReportDiagnostics<T>(
                         // but no argument was supplied for it then the first such method is 
                         // the best bad method.
                         case MemberResolutionKind.RequiredParameterMissing:
-                            if ((binder.Flags & BinderFlags.CollectionExpressionConversionValidation) != 0)
+                            if ((binder.Flags & BinderFlags.CollectionExpressionConversionValidation) != 0 && receiver is null)
                             {
-                                if (receiver is null)
-                                {
-                                    Debug.Assert(firstSupported.Member is MethodSymbol { MethodKind: MethodKind.Constructor });
-                                    diagnostics.Add(
-                                        isParamsModifierValidation ?
-                                            ErrorCode.ERR_ParamsCollectionMissingConstructor :
-                                            ErrorCode.ERR_CollectionExpressionMissingConstructor,
-                                        location);
-                                }
-                                else
-                                {
-                                    Debug.Assert(firstSupported.Member is MethodSymbol { Name: "Add" });
-                                    int argumentOffset = arguments.IsExtensionMethodInvocation ? 1 : 0;
-                                    diagnostics.Add(ErrorCode.ERR_CollectionExpressionMissingAdd, location, arguments.Arguments[argumentOffset].Type, firstSupported.Member);
-                                }
+                                Debug.Assert(firstSupported.Member is MethodSymbol { MethodKind: MethodKind.Constructor });
+                                diagnostics.Add(
+                                    isParamsModifierValidation ?
+                                        ErrorCode.ERR_ParamsCollectionMissingConstructor :
+                                        ErrorCode.ERR_CollectionExpressionMissingConstructor,
+                                    location);
                             }
                             else
                             {
@@ -1127,12 +1118,7 @@ private bool HadBadArguments(
                     }
                 }
 
-                if (flags.Includes(BinderFlags.CollectionExpressionConversionValidation))
-                {
-                    Debug.Assert(arguments.Arguments.Count == argumentOffset + 1);
-                    diagnostics.Add(ErrorCode.ERR_CollectionExpressionMissingAdd, location, arguments.Arguments[argumentOffset].Type, method);
-                }
-                else
+                if (!flags.Includes(BinderFlags.CollectionExpressionConversionValidation))
                 {
                     //  The best overloaded Add method '{0}' for the collection initializer has some invalid arguments
                     diagnostics.Add(ErrorCode.ERR_BadArgTypesForCollectionAdd, location, symbols, method);

src/Compilers/CSharp/Portable/CSharpResources.resx

@@ -6863,7 +6863,7 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
     <value>Collection expression type must have an applicable constructor that can be called with no arguments.</value>
   </data>
   <data name="ERR_CollectionExpressionMissingAdd" xml:space="preserve">
-    <value>Collection expression type must have an applicable instance or extension method 'Add' that can be called with an argument of iteration type '{0}'. The best overloaded method is '{1}'.</value>
+    <value>Collection expression type '{0}' must have an instance or extension method 'Add' that can be called with a single argument.</value>
   </data>
   <data name="ERR_CollectionBuilderAttributeInvalidType" xml:space="preserve">
     <value>The CollectionBuilderAttribute builder type must be a non-generic class or struct.</value>

src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_CollectionExpression.cs

@@ -118,7 +118,7 @@ static BoundNode unwrapListElement(BoundCollectionExpression node, BoundNode ele
                 if (element is BoundCollectionExpressionSpreadElement spreadElement)
                 {
                     Debug.Assert(spreadElement.IteratorBody is { });
-                    var iteratorBody = Binder.GetUnderlyingCollectionExpressionElement(node, ((BoundExpressionStatement)spreadElement.IteratorBody).Expression);
+                    var iteratorBody = Binder.GetUnderlyingCollectionExpressionElement(node, ((BoundExpressionStatement)spreadElement.IteratorBody).Expression, throwOnErrors: true);
                     Debug.Assert(iteratorBody is { });
                     return spreadElement.Update(
                         spreadElement.Expression,
@@ -131,7 +131,7 @@ static BoundNode unwrapListElement(BoundCollectionExpression node, BoundNode ele
                 }
                 else
                 {
-                    var result = Binder.GetUnderlyingCollectionExpressionElement(node, (BoundExpression)element);
+                    var result = Binder.GetUnderlyingCollectionExpressionElement(node, (BoundExpression)element, throwOnErrors: true);
                     Debug.Assert(result is { });
                     return result;
                 }