Merge pull request #69520 from CyrusNajmabadi/specialCaseImmutableArray

Do not convert immutable arrays on downlevel runtimes

eng/Versions.props

@@ -21,7 +21,7 @@
     <!-- Versions used by several individual references below -->
     <RoslynDiagnosticsNugetPackageVersion>3.11.0-beta1.23364.2</RoslynDiagnosticsNugetPackageVersion>
     <MicrosoftCodeAnalysisNetAnalyzersVersion>8.0.0-preview.23364.2</MicrosoftCodeAnalysisNetAnalyzersVersion>
-    <MicrosoftCodeAnalysisTestingVersion>1.1.2-beta1.23407.1</MicrosoftCodeAnalysisTestingVersion>
+    <MicrosoftCodeAnalysisTestingVersion>1.1.2-beta1.23411.1</MicrosoftCodeAnalysisTestingVersion>
     <MicrosoftVisualStudioExtensibilityTestingVersion>0.1.149-beta</MicrosoftVisualStudioExtensibilityTestingVersion>
     <!-- CodeStyleAnalyzerVersion should we updated together with version of dotnet-format in dotnet-tools.json -->
     <CodeStyleAnalyzerVersion>4.6.0</CodeStyleAnalyzerVersion>

src/Analyzers/CSharp/Analyzers/UseCollectionExpression/UseCollectionExpressionHelpers.cs

@@ -51,6 +51,12 @@ public static bool CanReplaceWithCollectionExpression(
         if (originalTypeInfo.ConvertedType is null or IErrorTypeSymbol)
             return false;
 
+        if (originalTypeInfo.ConvertedType.OriginalDefinition is INamedTypeSymbol { IsValueType: true } structType &&
+            !IsValidStructType(compilation, structType))
+        {
+            return false;
+        }
+
         // Conservatively, avoid making this change if the original expression was itself converted. Consider, for
         // example, `IEnumerable<string> x = new List<string>()`.  If we change that to `[]` we will still compile,
         // but it's possible we'll end up with different types at runtime that may cause problems.
@@ -97,6 +103,32 @@ public static bool CanReplaceWithCollectionExpression(
             return false;
 
         return true;
+
+        // Special case.  Block value types without an explicit no-arg constructor, which also do not have the
+        // collection-builder-attribute on them.  This prevents us from offering to convert value-type-collections who
+        // are invalid in their `default` state (like ImmutableArray<T>).  The presumption is that if the
+        // collection-initializer pattern is valid for these value types that they will supply an explicit constructor
+        // to initialize themselves.
+        static bool IsValidStructType(Compilation compilation, INamedTypeSymbol structType)
+        {
+            // Span<> and ReadOnlySpan<> are always valid targets.
+            if (structType.Equals(compilation.SpanOfTType()) || structType.Equals(compilation.ReadOnlySpanOfTType()))
+                return true;
+
+            // If we have a real no-arg constructor, then presume the type is intended to be new-ed up and used as a
+            // collection initializer.
+            var noArgConstructor = structType.Constructors.FirstOrDefault(c => !c.IsStatic && c.Parameters.Length == 0);
+            if (noArgConstructor is { IsImplicitlyDeclared: false })
+                return true;
+
+            // If it has a [CollectionBuilder] attribute on it, it can definitely be used for a collection expression.
+            var collectionBuilderType = compilation.CollectionBuilderAttribute();
+            if (structType.GetAttributes().Any(a => a.AttributeClass?.Equals(collectionBuilderType) is true))
+                return true;
+
+            // Otherwise, presume this is unsafe to use with collection expressions.
+            return false;
+        }
     }
 
     private static bool IsInTargetTypedLocation(SemanticModel semanticModel, ExpressionSyntax expression, CancellationToken cancellationToken)

src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForCreateTests.cs

@@ -2,11 +2,13 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
+using System.Collections.Immutable;
 using System.Threading.Tasks;
 using Microsoft.CodeAnalysis.CSharp.UseCollectionExpression;
 using Microsoft.CodeAnalysis.Editor.UnitTests.CodeActions;
 using Microsoft.CodeAnalysis.Test.Utilities;
 using Microsoft.CodeAnalysis.Testing;
+using Roslyn.Test.Utilities;
 using Xunit;
 
 namespace Microsoft.CodeAnalysis.CSharp.Analyzers.UnitTests.UseCollectionExpression;
@@ -813,4 +815,116 @@ class C
             ReferenceAssemblies = ReferenceAssemblies.Net.Net70,
         }.RunAsync();
     }
+
+    [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/69507")]
+    public async Task NotForImmutableArrayNet70()
+    {
+        await new VerifyCS.Test
+        {
+            TestCode = """
+                using System;
+                using System.Collections.Immutable;
+
+                class C
+                {
+                    void M()
+                    {
+                        ImmutableArray<int> v = ImmutableArray.Create(1, 2, 3);
+                    }
+                }
+                """,
+            LanguageVersion = LanguageVersion.CSharp12,
+            ReferenceAssemblies = ReferenceAssemblies.Net.Net70,
+        }.RunAsync();
+    }
+
+    [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/69507")]
+    public async Task ForImmutableArrayNet80()
+    {
+        await new VerifyCS.Test
+        {
+            TestCode = """
+                using System;
+                using System.Collections.Immutable;
+
+                class C
+                {
+                    void M()
+                    {
+                        ImmutableArray<int> v = [|ImmutableArray.[|Create|](|]1, 2, 3);
+                    }
+                }
+                """,
+            FixedCode = """
+                using System;
+                using System.Collections.Immutable;
+
+                class C
+                {
+                    void M()
+                    {
+                        ImmutableArray<int> v = [1, 2, 3];
+                    }
+                }
+                """,
+            LanguageVersion = LanguageVersion.CSharp12,
+            ReferenceAssemblies = ReferenceAssemblies.Net.Net80,
+        }.RunAsync();
+    }
+
+    [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/69507"), WorkItem("https://github.com/dotnet/roslyn/issues/69521")]
+    public async Task NotForImmutableListNet70()
+    {
+        await new VerifyCS.Test
+        {
+            TestCode = """
+                using System;
+                using System.Collections.Immutable;
+
+                class C
+                {
+                    void M()
+                    {
+                        ImmutableList<int> v = ImmutableList.Create(1, 2, 3);
+                    }
+                }
+                """,
+            LanguageVersion = LanguageVersion.CSharp12,
+            ReferenceAssemblies = ReferenceAssemblies.Net.Net70,
+        }.RunAsync();
+    }
+
+    [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/69507")]
+    public async Task ForImmutableListNet80()
+    {
+        await new VerifyCS.Test
+        {
+            TestCode = """
+                using System;
+                using System.Collections.Immutable;
+
+                class C
+                {
+                    void M()
+                    {
+                        ImmutableList<int> v = [|ImmutableList.[|Create|](|]1, 2, 3);
+                    }
+                }
+                """,
+            FixedCode = """
+                using System;
+                using System.Collections.Immutable;
+
+                class C
+                {
+                    void M()
+                    {
+                        ImmutableList<int> v = [1, 2, 3];
+                    }
+                }
+                """,
+            LanguageVersion = LanguageVersion.CSharp12,
+            ReferenceAssemblies = ReferenceAssemblies.Net.Net80,
+        }.RunAsync();
+    }
 }