Merge pull request #74934 from CyrusNajmabadi/arrayROS

src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForArrayDiagnosticAnalyzer.cs

@@ -17,7 +17,7 @@
 
 namespace Microsoft.CodeAnalysis.CSharp.UseCollectionExpression;
 
-using static SyntaxFactory;
+using static UseCollectionExpressionHelpers;
 
 [DiagnosticAnalyzer(LanguageNames.CSharp)]
 internal sealed partial class CSharpUseCollectionExpressionForArrayDiagnosticAnalyzer()
@@ -49,7 +49,8 @@ private void AnalyzeArrayCreationExpression(SyntaxNodeAnalysisContext context, I
 
         // Analyze the statements that follow to see if they can initialize this array.
         var allowSemanticsChange = option.Value is CollectionExpressionPreference.WhenTypesLooselyMatch;
-        var matches = TryGetMatches(semanticModel, arrayCreationExpression, expressionType, allowSemanticsChange, cancellationToken, out var changesSemantics);
+        var replacementExpression = CreateReplacementCollectionExpressionForAnalysis(arrayCreationExpression.Initializer);
+        var matches = TryGetMatches(semanticModel, arrayCreationExpression, replacementExpression, expressionType, allowSemanticsChange, cancellationToken, out var changesSemantics);
         if (matches.IsDefault)
             return;
 
@@ -59,6 +60,7 @@ private void AnalyzeArrayCreationExpression(SyntaxNodeAnalysisContext context, I
     public static ImmutableArray<CollectionExpressionMatch<StatementSyntax>> TryGetMatches(
         SemanticModel semanticModel,
         ArrayCreationExpressionSyntax expression,
+        CollectionExpressionSyntax replacementExpression,
         INamedTypeSymbol? expressionType,
         bool allowSemanticsChange,
         CancellationToken cancellationToken,
@@ -69,6 +71,7 @@ public static ImmutableArray<CollectionExpressionMatch<StatementSyntax>> TryGetM
         var matches = UseCollectionExpressionHelpers.TryGetMatches(
             semanticModel,
             expression,
+            replacementExpression,
             expressionType,
             isSingletonInstance: false,
             allowSemanticsChange,
@@ -79,8 +82,8 @@ public static ImmutableArray<CollectionExpressionMatch<StatementSyntax>> TryGetM
         if (matches.IsDefault)
             return default;
 
-        if (!UseCollectionExpressionHelpers.CanReplaceWithCollectionExpression(
-                semanticModel, expression, expressionType, isSingletonInstance: false, allowSemanticsChange, skipVerificationForReplacedNode: true, cancellationToken, out changesSemantics))
+        if (!CanReplaceWithCollectionExpression(
+                semanticModel, expression, replacementExpression, expressionType, isSingletonInstance: false, allowSemanticsChange, skipVerificationForReplacedNode: true, cancellationToken, out changesSemantics))
         {
             return default;
         }
@@ -101,7 +104,7 @@ public static ImmutableArray<CollectionExpressionMatch<StatementSyntax>> TryGetM
             if (ienumerableType is null)
                 return default;
 
-            if (!UseCollectionExpressionHelpers.IsConstructibleCollectionType(
+            if (!IsConstructibleCollectionType(
                     semanticModel.Compilation, ienumerableType.TypeArguments.Single()))
             {
                 return default;
@@ -114,15 +117,16 @@ public static ImmutableArray<CollectionExpressionMatch<StatementSyntax>> TryGetM
     public static ImmutableArray<CollectionExpressionMatch<StatementSyntax>> TryGetMatches(
         SemanticModel semanticModel,
         ImplicitArrayCreationExpressionSyntax expression,
+        CollectionExpressionSyntax replacementExpression,
         INamedTypeSymbol? expressionType,
         bool allowSemanticsChange,
         CancellationToken cancellationToken,
         out bool changesSemantics)
     {
         // if we have `new[] { ... }` we have no subsequent matches to add to the collection. All values come
         // from within the initializer.
-        if (!UseCollectionExpressionHelpers.CanReplaceWithCollectionExpression(
-                semanticModel, expression, expressionType, isSingletonInstance: false, allowSemanticsChange, skipVerificationForReplacedNode: true, cancellationToken, out changesSemantics))
+        if (!CanReplaceWithCollectionExpression(
+                semanticModel, expression, replacementExpression, expressionType, isSingletonInstance: false, allowSemanticsChange, skipVerificationForReplacedNode: true, cancellationToken, out changesSemantics))
         {
             return default;
         }
@@ -154,11 +158,10 @@ private void AnalyzeArrayInitializerExpression(SyntaxNodeAnalysisContext context
 
         // Have to actually examine what would happen when we do the replacement, as the replaced value may interact
         // with inference based on the values within.
-        var replacementCollectionExpression = CollectionExpression(
-            [.. initializer.Expressions.Select(ExpressionElement)]);
+        var replacementCollectionExpression = CreateReplacementCollectionExpressionForAnalysis(initializer);
 
         var allowSemanticsChange = option.Value is CollectionExpressionPreference.WhenTypesLooselyMatch;
-        if (!UseCollectionExpressionHelpers.CanReplaceWithCollectionExpression(
+        if (!CanReplaceWithCollectionExpression(
                 semanticModel, arrayCreationExpression, replacementCollectionExpression,
                 expressionType, isSingletonInstance: false, allowSemanticsChange, skipVerificationForReplacedNode: true, cancellationToken,
                 out var changesSemantics))
@@ -170,8 +173,8 @@ private void AnalyzeArrayInitializerExpression(SyntaxNodeAnalysisContext context
         {
             var matches = initializer.Parent switch
             {
-                ArrayCreationExpressionSyntax arrayCreation => TryGetMatches(semanticModel, arrayCreation, expressionType, allowSemanticsChange, cancellationToken, out _),
-                ImplicitArrayCreationExpressionSyntax arrayCreation => TryGetMatches(semanticModel, arrayCreation, expressionType, allowSemanticsChange, cancellationToken, out _),
+                ArrayCreationExpressionSyntax arrayCreation => TryGetMatches(semanticModel, arrayCreation, replacementCollectionExpression, expressionType, allowSemanticsChange, cancellationToken, out _),
+                ImplicitArrayCreationExpressionSyntax arrayCreation => TryGetMatches(semanticModel, arrayCreation, replacementCollectionExpression, expressionType, allowSemanticsChange, cancellationToken, out _),
                 _ => throw ExceptionUtilities.Unreachable(),
             };
 

src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocDiagnosticAnalyzer.cs

@@ -12,6 +12,8 @@
 
 namespace Microsoft.CodeAnalysis.CSharp.UseCollectionExpression;
 
+using static UseCollectionExpressionHelpers;
+
 [DiagnosticAnalyzer(LanguageNames.CSharp)]
 internal sealed partial class CSharpUseCollectionExpressionForStackAllocDiagnosticAnalyzer
     : AbstractCSharpUseCollectionExpressionDiagnosticAnalyzer
@@ -127,6 +129,7 @@ public static ImmutableArray<CollectionExpressionMatch<StatementSyntax>> TryGetM
         return UseCollectionExpressionHelpers.TryGetMatches(
             semanticModel,
             expression,
+            CreateReplacementCollectionExpressionForAnalysis(expression.Initializer),
             expressionType,
             isSingletonInstance: false,
             allowSemanticsChange,

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

@@ -33,11 +33,15 @@ internal static class UseCollectionExpressionHelpers
         distinguishRefFromOut: true,
         // Not relevant.  We are not comparing method signatures.
         objectAndDynamicCompareEqually: false,
+        // Not relevant.  We are not comparing method signatures.
+        arrayAndReadOnlySpanCompareEqually: false,
         // The value we're tweaking.
         tupleNamesMustMatch: false,
         // We do not want to ignore this.  `ImmutableArray<string?>` should not be convertible to `ImmutableArray<string>`
         ignoreNullableAnnotations: false);
 
+    private static readonly SymbolEquivalenceComparer s_arrayAndReadOnlySpanCompareEquallyComparer = s_tupleNamesCanDifferComparer.With(arrayAndReadOnlySpanCompareEqually: true);
+
     public static bool CanReplaceWithCollectionExpression(
         SemanticModel semanticModel,
         ExpressionSyntax expression,
@@ -147,9 +151,17 @@ public static bool CanReplaceWithCollectionExpression(
 
         // The new expression's converted type has to equal the old expressions as well.  Otherwise, we're now
         // converting this to some different collection type unintentionally.
+        //
+        // Note: it's acceptable to be originally converting to an array, and now converting to a ROS.  This occurs with
+        // APIs that started out just taking an array, but which now have an overload that takes a span.  APIs should
+        // only do this when the new api has the same semantics (outside of perf), and the language and runtime strongly
+        // want code to call the new api.  So it's desirable to change here.
         var replacedTypeInfo = speculationAnalyzer.SpeculativeSemanticModel.GetTypeInfo(speculationAnalyzer.ReplacedExpression, cancellationToken);
-        if (!originalTypeInfo.ConvertedType.Equals(replacedTypeInfo.ConvertedType))
+        if (!originalTypeInfo.ConvertedType.Equals(replacedTypeInfo.ConvertedType) &&
+            !s_arrayAndReadOnlySpanCompareEquallyComparer.Equals(originalTypeInfo.ConvertedType, replacedTypeInfo.ConvertedType))
+        {
             return false;
+        }
 
         return true;
 
@@ -805,6 +817,7 @@ private static bool ShouldReplaceExistingExpressionEntirely(
     public static ImmutableArray<CollectionExpressionMatch<StatementSyntax>> TryGetMatches<TArrayCreationExpressionSyntax>(
         SemanticModel semanticModel,
         TArrayCreationExpressionSyntax expression,
+        CollectionExpressionSyntax replacementExpression,
         INamedTypeSymbol? expressionType,
         bool isSingletonInstance,
         bool allowSemanticsChange,
@@ -914,7 +927,7 @@ public static ImmutableArray<CollectionExpressionMatch<StatementSyntax>> TryGetM
         }
 
         if (!CanReplaceWithCollectionExpression(
-                semanticModel, expression, expressionType, isSingletonInstance, allowSemanticsChange, skipVerificationForReplacedNode: true, cancellationToken, out changesSemantics))
+                semanticModel, expression, replacementExpression, expressionType, isSingletonInstance, allowSemanticsChange, skipVerificationForReplacedNode: true, cancellationToken, out changesSemantics))
         {
             return default;
         }
@@ -1222,4 +1235,7 @@ public static SeparatedSyntaxList<ArgumentSyntax> GetArguments(InvocationExpress
             : SeparatedList<ArgumentSyntax>(initializer.Expressions.GetWithSeparators().Select(
                 nodeOrToken => nodeOrToken.IsToken ? nodeOrToken : Argument((ExpressionSyntax)nodeOrToken.AsNode()!)));
     }
+
+    public static CollectionExpressionSyntax CreateReplacementCollectionExpressionForAnalysis(InitializerExpressionSyntax? initializer)
+        => initializer is null ? s_emptyCollectionExpression : CollectionExpression([.. initializer.Expressions.Select(ExpressionElement)]);
 }

src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForArrayCodeFixProvider.cs

@@ -103,11 +103,11 @@ ImmutableArray<CollectionExpressionMatch<StatementSyntax>> GetMatches(
             {
                 ImplicitArrayCreationExpressionSyntax arrayCreation
                     => CSharpUseCollectionExpressionForArrayDiagnosticAnalyzer.TryGetMatches(
-                        semanticModel, arrayCreation, expressionType, allowSemanticsChange: true, cancellationToken, out _),
+                        semanticModel, arrayCreation, CreateReplacementCollectionExpressionForAnalysis(arrayCreation.Initializer), expressionType, allowSemanticsChange: true, cancellationToken, out _),
 
                 ArrayCreationExpressionSyntax arrayCreation
                     => CSharpUseCollectionExpressionForArrayDiagnosticAnalyzer.TryGetMatches(
-                        semanticModel, arrayCreation, expressionType, allowSemanticsChange: true, cancellationToken, out _),
+                        semanticModel, arrayCreation, CreateReplacementCollectionExpressionForAnalysis(arrayCreation.Initializer), expressionType, allowSemanticsChange: true, cancellationToken, out _),
 
                 // We validated this is unreachable in the caller.
                 _ => throw ExceptionUtilities.Unreachable(),

src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocCodeFixProvider.cs

@@ -7,7 +7,6 @@
 using System.Composition;
 using System.Threading;
 using System.Threading.Tasks;
-using Microsoft.CodeAnalysis.CodeActions;
 using Microsoft.CodeAnalysis.CodeFixes;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
 using Microsoft.CodeAnalysis.Diagnostics;

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

@@ -2,6 +2,7 @@
 // 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;
 using System.Collections.Generic;
 using System.Threading.Tasks;
 using Microsoft.CodeAnalysis.CSharp.UseCollectionExpression;
@@ -5758,4 +5759,130 @@ class C
             LanguageVersion = LanguageVersion.CSharp12,
         }.RunAsync();
     }
+
+    [Theory, CombinatorialData, WorkItem("https://github.com/dotnet/roslyn/issues/74931")]
+    public async Task AllowSwitchToReadOnlySpanCSharp12(bool implicitType, bool whenTypesLooselyMatch)
+    {
+        await new VerifyCS.Test
+        {
+            TestCode = $$"""
+                using System;
+
+                class C
+                {
+                    void M(char c)
+                    {
+                        Split([|[|new|]{{(implicitType ? "" : " char")}}[]|] { c });
+                    }
+
+                    void Split(char[] p) { }
+                    void Split(ReadOnlySpan<char> p) { }
+                }
+                """,
+            FixedCode = """
+                using System;
+                
+                class C
+                {
+                    void M(char c)
+                    {
+                        Split([c]);
+                    }
+                
+                    void Split(char[] p) { }
+                    void Split(ReadOnlySpan<char> p) { }
+                }
+                """,
+            EditorConfig = $$"""
+                [*]
+                dotnet_style_prefer_collection_expression={{(whenTypesLooselyMatch ? "when_types_loosely_match" : "when_types_exactly_match")}}
+                """,
+            LanguageVersion = LanguageVersion.CSharp12,
+            ReferenceAssemblies = ReferenceAssemblies.Net.Net80,
+        }.RunAsync();
+    }
+
+    [Theory, CombinatorialData, WorkItem("https://github.com/dotnet/roslyn/issues/74931")]
+    public async Task AllowSwitchToReadOnlySpanCSharp13(bool implicitType, bool whenTypesLooselyMatch)
+    {
+        await new VerifyCS.Test
+        {
+            TestCode = $$"""
+                using System;
+
+                class C
+                {
+                    void M(char c)
+                    {
+                        Split([|[|new|]{{(implicitType ? "" : " char")}}[]|] { c });
+                    }
+
+                    void Split(char[] p) { }
+                    void Split(ReadOnlySpan<char> p) { }
+                }
+                """,
+            FixedCode = """
+                using System;
+                
+                class C
+                {
+                    void M(char c)
+                    {
+                        Split([c]);
+                    }
+                
+                    void Split(char[] p) { }
+                    void Split(ReadOnlySpan<char> p) { }
+                }
+                """,
+            EditorConfig = $$"""
+                [*]
+                dotnet_style_prefer_collection_expression={{(whenTypesLooselyMatch ? "when_types_loosely_match" : "when_types_exactly_match")}}
+                """,
+            LanguageVersion = LanguageVersion.CSharp13,
+            ReferenceAssemblies = ReferenceAssemblies.Net.Net80,
+        }.RunAsync();
+    }
+
+    [Theory, CombinatorialData, WorkItem("https://github.com/dotnet/roslyn/issues/74931")]
+    public async Task AllowSwitchToReadOnlySpanGeneric1(bool implicitType, bool whenTypesLooselyMatch)
+    {
+        await new VerifyCS.Test
+        {
+            TestCode = $$"""
+                using System;
+
+                class C
+                {
+                    void M(char c)
+                    {
+                        Split([|[|new|]{{(implicitType ? "" : " char")}}[]|] { c });
+                    }
+
+                    void Split<T>(T[] p) { }
+                    void Split<T>(ReadOnlySpan<T> p) { }
+                }
+                """,
+            FixedCode = """
+                using System;
+                
+                class C
+                {
+                    void M(char c)
+                    {
+                        Split([c]);
+                    }
+                
+                    void Split<T>(T[] p) { }
+                    void Split<T>(ReadOnlySpan<T> p) { }
+                }
+                """,
+            EditorConfig = $$"""
+                [*]
+                dotnet_style_prefer_collection_expression={{(whenTypesLooselyMatch ? "when_types_loosely_match" : "when_types_exactly_match")}}
+                """,
+            LanguageVersion = LanguageVersion.CSharp12,
+            ReferenceAssemblies = ReferenceAssemblies.Net.Net80,
+        }.RunAsync();
+    }
 }