diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForArrayDiagnosticAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForArrayDiagnosticAnalyzer.cs index 887b530668f8c..eacc1e33f9758 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForArrayDiagnosticAnalyzer.cs +++ b/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> TryGetMatches( SemanticModel semanticModel, ArrayCreationExpressionSyntax expression, + CollectionExpressionSyntax replacementExpression, INamedTypeSymbol? expressionType, bool allowSemanticsChange, CancellationToken cancellationToken, @@ -69,6 +71,7 @@ public static ImmutableArray> TryGetM var matches = UseCollectionExpressionHelpers.TryGetMatches( semanticModel, expression, + replacementExpression, expressionType, isSingletonInstance: false, allowSemanticsChange, @@ -79,8 +82,8 @@ public static ImmutableArray> 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> TryGetM if (ienumerableType is null) return default; - if (!UseCollectionExpressionHelpers.IsConstructibleCollectionType( + if (!IsConstructibleCollectionType( semanticModel.Compilation, ienumerableType.TypeArguments.Single())) { return default; @@ -114,6 +117,7 @@ public static ImmutableArray> TryGetM public static ImmutableArray> TryGetMatches( SemanticModel semanticModel, ImplicitArrayCreationExpressionSyntax expression, + CollectionExpressionSyntax replacementExpression, INamedTypeSymbol? expressionType, bool allowSemanticsChange, CancellationToken cancellationToken, @@ -121,8 +125,8 @@ public static ImmutableArray> TryGetM { // 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(), }; diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocDiagnosticAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocDiagnosticAnalyzer.cs index dc8fc2a87fc37..607899dd00c26 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocDiagnosticAnalyzer.cs +++ b/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> TryGetM return UseCollectionExpressionHelpers.TryGetMatches( semanticModel, expression, + CreateReplacementCollectionExpressionForAnalysis(expression.Initializer), expressionType, isSingletonInstance: false, allowSemanticsChange, diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/UseCollectionExpressionHelpers.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/UseCollectionExpressionHelpers.cs index 15d93e2106889..bedba7efb662a 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/UseCollectionExpressionHelpers.cs +++ b/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` should not be convertible to `ImmutableArray` 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> TryGetMatches( SemanticModel semanticModel, TArrayCreationExpressionSyntax expression, + CollectionExpressionSyntax replacementExpression, INamedTypeSymbol? expressionType, bool isSingletonInstance, bool allowSemanticsChange, @@ -914,7 +927,7 @@ public static ImmutableArray> 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 GetArguments(InvocationExpress : SeparatedList(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)]); } diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForArrayCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForArrayCodeFixProvider.cs index 6131ccff3191a..3ca81464e419a 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForArrayCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForArrayCodeFixProvider.cs @@ -103,11 +103,11 @@ ImmutableArray> 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(), diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocCodeFixProvider.cs index 7774bb8c9acd4..da987b48b87d4 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocCodeFixProvider.cs +++ b/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; diff --git a/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForArrayTests.cs b/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForArrayTests.cs index 59f759911279e..863c202e575e3 100644 --- a/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForArrayTests.cs +++ b/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 p) { } + } + """, + FixedCode = """ + using System; + + class C + { + void M(char c) + { + Split([c]); + } + + void Split(char[] p) { } + void Split(ReadOnlySpan 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 p) { } + } + """, + FixedCode = """ + using System; + + class C + { + void M(char c) + { + Split([c]); + } + + void Split(char[] p) { } + void Split(ReadOnlySpan 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[] p) { } + void Split(ReadOnlySpan p) { } + } + """, + FixedCode = """ + using System; + + class C + { + void M(char c) + { + Split([c]); + } + + void Split(T[] p) { } + void Split(ReadOnlySpan p) { } + } + """, + EditorConfig = $$""" + [*] + dotnet_style_prefer_collection_expression={{(whenTypesLooselyMatch ? "when_types_loosely_match" : "when_types_exactly_match")}} + """, + LanguageVersion = LanguageVersion.CSharp12, + ReferenceAssemblies = ReferenceAssemblies.Net.Net80, + }.RunAsync(); + } } diff --git a/src/EditorFeatures/Test/Utilities/SymbolEquivalenceComparerTests.cs b/src/EditorFeatures/Test/Utilities/SymbolEquivalenceComparerTests.cs index df74e9b865686..03699720675f1 100644 --- a/src/EditorFeatures/Test/Utilities/SymbolEquivalenceComparerTests.cs +++ b/src/EditorFeatures/Test/Utilities/SymbolEquivalenceComparerTests.cs @@ -1115,8 +1115,8 @@ void M(ref int i) { } var method_v1 = type1_v1.GetMembers("M").Single(); var method_v2 = type1_v2.GetMembers("M").Single(); - var trueComp = new SymbolEquivalenceComparer(assemblyComparer: null, distinguishRefFromOut: true, tupleNamesMustMatch: false, ignoreNullableAnnotations: false, objectAndDynamicCompareEqually: true); - var falseComp = new SymbolEquivalenceComparer(assemblyComparer: null, distinguishRefFromOut: false, tupleNamesMustMatch: false, ignoreNullableAnnotations: false, objectAndDynamicCompareEqually: true); + var trueComp = new SymbolEquivalenceComparer(assemblyComparer: null, distinguishRefFromOut: true, tupleNamesMustMatch: false, ignoreNullableAnnotations: false, objectAndDynamicCompareEqually: true, arrayAndReadOnlySpanCompareEqually: false); + var falseComp = new SymbolEquivalenceComparer(assemblyComparer: null, distinguishRefFromOut: false, tupleNamesMustMatch: false, ignoreNullableAnnotations: false, objectAndDynamicCompareEqually: true, arrayAndReadOnlySpanCompareEqually: false); Assert.False(trueComp.Equals(method_v1, method_v2)); Assert.False(trueComp.Equals(method_v2, method_v1)); @@ -1355,8 +1355,8 @@ class T Assert.Equal(NullableAnnotation.Annotated, a1.NullableAnnotation); Assert.Equal(NullableAnnotation.NotAnnotated, a2.NullableAnnotation); - var ignoreComparer = new SymbolEquivalenceComparer(assemblyComparer: null, distinguishRefFromOut: true, tupleNamesMustMatch: false, ignoreNullableAnnotations: true, objectAndDynamicCompareEqually: true); - var notIgnoreComparer = new SymbolEquivalenceComparer(assemblyComparer: null, distinguishRefFromOut: true, tupleNamesMustMatch: false, ignoreNullableAnnotations: false, objectAndDynamicCompareEqually: true); + var ignoreComparer = new SymbolEquivalenceComparer(assemblyComparer: null, distinguishRefFromOut: true, tupleNamesMustMatch: false, ignoreNullableAnnotations: true, objectAndDynamicCompareEqually: true, arrayAndReadOnlySpanCompareEqually: false); + var notIgnoreComparer = new SymbolEquivalenceComparer(assemblyComparer: null, distinguishRefFromOut: true, tupleNamesMustMatch: false, ignoreNullableAnnotations: false, objectAndDynamicCompareEqually: true, arrayAndReadOnlySpanCompareEqually: false); Assert.True(ignoreComparer.Equals(a1, a2)); Assert.True(ignoreComparer.Equals(b1, b2)); @@ -1419,8 +1419,8 @@ class T Assert.Equal(NullableAnnotation.None, a1.NullableAnnotation); Assert.Equal(NullableAnnotation.NotAnnotated, a2.NullableAnnotation); - var ignoreComparer = new SymbolEquivalenceComparer(assemblyComparer: null, distinguishRefFromOut: true, tupleNamesMustMatch: false, ignoreNullableAnnotations: true, objectAndDynamicCompareEqually: true); - var notIgnoreComparer = new SymbolEquivalenceComparer(assemblyComparer: null, distinguishRefFromOut: true, tupleNamesMustMatch: false, ignoreNullableAnnotations: false, objectAndDynamicCompareEqually: true); + var ignoreComparer = new SymbolEquivalenceComparer(assemblyComparer: null, distinguishRefFromOut: true, tupleNamesMustMatch: false, ignoreNullableAnnotations: true, objectAndDynamicCompareEqually: true, arrayAndReadOnlySpanCompareEqually: false); + var notIgnoreComparer = new SymbolEquivalenceComparer(assemblyComparer: null, distinguishRefFromOut: true, tupleNamesMustMatch: false, ignoreNullableAnnotations: false, objectAndDynamicCompareEqually: true, arrayAndReadOnlySpanCompareEqually: false); Assert.True(ignoreComparer.Equals(a1, a2)); Assert.True(ignoreComparer.Equals(b1, b2)); @@ -1482,8 +1482,8 @@ class T Assert.Equal(NullableAnnotation.None, a1.NullableAnnotation); Assert.Equal(NullableAnnotation.Annotated, a2.NullableAnnotation); - var ignoreComparer = new SymbolEquivalenceComparer(assemblyComparer: null, distinguishRefFromOut: true, tupleNamesMustMatch: false, ignoreNullableAnnotations: true, objectAndDynamicCompareEqually: true); - var notIgnoreComparer = new SymbolEquivalenceComparer(assemblyComparer: null, distinguishRefFromOut: true, tupleNamesMustMatch: false, ignoreNullableAnnotations: false, objectAndDynamicCompareEqually: true); + var ignoreComparer = new SymbolEquivalenceComparer(assemblyComparer: null, distinguishRefFromOut: true, tupleNamesMustMatch: false, ignoreNullableAnnotations: true, objectAndDynamicCompareEqually: true, arrayAndReadOnlySpanCompareEqually: false); + var notIgnoreComparer = new SymbolEquivalenceComparer(assemblyComparer: null, distinguishRefFromOut: true, tupleNamesMustMatch: false, ignoreNullableAnnotations: false, objectAndDynamicCompareEqually: true, arrayAndReadOnlySpanCompareEqually: false); Assert.True(ignoreComparer.Equals(a1, a2)); Assert.True(ignoreComparer.Equals(b1, b2)); @@ -1545,8 +1545,8 @@ class T Assert.Equal(NullableAnnotation.None, a1.NullableAnnotation); Assert.Equal(NullableAnnotation.Annotated, a2.NullableAnnotation); - var ignoreComparer = new SymbolEquivalenceComparer(assemblyComparer: null, distinguishRefFromOut: true, tupleNamesMustMatch: false, ignoreNullableAnnotations: true, objectAndDynamicCompareEqually: true); - var notIgnoreComparer = new SymbolEquivalenceComparer(assemblyComparer: null, distinguishRefFromOut: true, tupleNamesMustMatch: false, ignoreNullableAnnotations: false, objectAndDynamicCompareEqually: true); + var ignoreComparer = new SymbolEquivalenceComparer(assemblyComparer: null, distinguishRefFromOut: true, tupleNamesMustMatch: false, ignoreNullableAnnotations: true, objectAndDynamicCompareEqually: true, arrayAndReadOnlySpanCompareEqually: false); + var notIgnoreComparer = new SymbolEquivalenceComparer(assemblyComparer: null, distinguishRefFromOut: true, tupleNamesMustMatch: false, ignoreNullableAnnotations: false, objectAndDynamicCompareEqually: true, arrayAndReadOnlySpanCompareEqually: false); Assert.True(ignoreComparer.Equals(a1, a2)); Assert.True(ignoreComparer.Equals(b1, b2)); @@ -1749,7 +1749,7 @@ public void AssemblyComparer1() var tb2 = (ITypeSymbol)b2.GlobalNamespace.GetMembers("T").Single(); var tb3 = (ITypeSymbol)b3.GlobalNamespace.GetMembers("T").Single(); - var identityComparer = new SymbolEquivalenceComparer(AssemblySymbolIdentityComparer.Instance, distinguishRefFromOut: false, tupleNamesMustMatch: false, ignoreNullableAnnotations: false, objectAndDynamicCompareEqually: true); + var identityComparer = new SymbolEquivalenceComparer(AssemblySymbolIdentityComparer.Instance, distinguishRefFromOut: false, tupleNamesMustMatch: false, ignoreNullableAnnotations: false, objectAndDynamicCompareEqually: true, arrayAndReadOnlySpanCompareEqually: false); // same name: Assert.True(SymbolEquivalenceComparer.IgnoreAssembliesInstance.Equals(ta1, ta2)); @@ -1835,7 +1835,7 @@ .method public instance int32[] F( // 3 var type1 = (ITypeSymbol)c1.GlobalNamespace.GetMembers("C").Single(); var type2 = (ITypeSymbol)c2.GlobalNamespace.GetMembers("C").Single(); - var identityComparer = new SymbolEquivalenceComparer(AssemblySymbolIdentityComparer.Instance, distinguishRefFromOut: false, tupleNamesMustMatch: false, ignoreNullableAnnotations: false, objectAndDynamicCompareEqually: true); + var identityComparer = new SymbolEquivalenceComparer(AssemblySymbolIdentityComparer.Instance, distinguishRefFromOut: false, tupleNamesMustMatch: false, ignoreNullableAnnotations: false, objectAndDynamicCompareEqually: true, arrayAndReadOnlySpanCompareEqually: false); var f1 = type1.GetMembers("F"); var f2 = type2.GetMembers("F"); diff --git a/src/Features/Core/Portable/EditAndContinue/AbstractEditAndContinueAnalyzer.cs b/src/Features/Core/Portable/EditAndContinue/AbstractEditAndContinueAnalyzer.cs index 4c6ca08e79071..f3643b0c755b4 100644 --- a/src/Features/Core/Portable/EditAndContinue/AbstractEditAndContinueAnalyzer.cs +++ b/src/Features/Core/Portable/EditAndContinue/AbstractEditAndContinueAnalyzer.cs @@ -2356,10 +2356,10 @@ public int GetHashCode(IAssemblySymbol? obj) // They only affect custom attributes or metadata flags emitted on the members - all runtimes are expected to accept // these updates in metadata deltas, even if they do not have any observable effect. private static readonly SymbolEquivalenceComparer s_runtimeSymbolEqualityComparer = new( - AssemblyEqualityComparer.Instance, distinguishRefFromOut: false, tupleNamesMustMatch: false, ignoreNullableAnnotations: true, objectAndDynamicCompareEqually: true); + AssemblyEqualityComparer.Instance, distinguishRefFromOut: false, tupleNamesMustMatch: false, ignoreNullableAnnotations: true, objectAndDynamicCompareEqually: true, arrayAndReadOnlySpanCompareEqually: false); private static readonly SymbolEquivalenceComparer s_exactSymbolEqualityComparer = new( - AssemblyEqualityComparer.Instance, distinguishRefFromOut: true, tupleNamesMustMatch: true, ignoreNullableAnnotations: false, objectAndDynamicCompareEqually: false); + AssemblyEqualityComparer.Instance, distinguishRefFromOut: true, tupleNamesMustMatch: true, ignoreNullableAnnotations: false, objectAndDynamicCompareEqually: false, arrayAndReadOnlySpanCompareEqually: false); protected static bool SymbolsEquivalent(ISymbol oldSymbol, ISymbol newSymbol) => s_exactSymbolEqualityComparer.Equals(oldSymbol, newSymbol); diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Extensions/ITypeSymbolExtensions.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Extensions/ITypeSymbolExtensions.cs index 452f208687b25..c3db3f7a06779 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Extensions/ITypeSymbolExtensions.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Extensions/ITypeSymbolExtensions.cs @@ -751,8 +751,8 @@ public static bool IsSpan([NotNullWhen(true)] this ITypeSymbol? type) ContainingNamespace: { Name: nameof(System), ContainingNamespace.IsGlobalNamespace: true } }; - public static bool IsReadOnlySpan([NotNullWhen(true)] this ITypeSymbol? type) - => type is INamedTypeSymbol + public static bool IsReadOnlySpan([NotNullWhen(true)] this ISymbol? symbol) + => symbol is INamedTypeSymbol { Name: nameof(ReadOnlySpan), TypeArguments.Length: 1, diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/AbstractSpeculationAnalyzer.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/AbstractSpeculationAnalyzer.cs index b92b7bbab2887..42108cfaf4036 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/AbstractSpeculationAnalyzer.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/AbstractSpeculationAnalyzer.cs @@ -53,7 +53,9 @@ internal abstract class AbstractSpeculationAnalyzer< private SemanticModel? _lazySpeculativeSemanticModel; private static readonly SymbolEquivalenceComparer s_includeNullabilityComparer = - SymbolEquivalenceComparer.Create(distinguishRefFromOut: true, tupleNamesMustMatch: true, ignoreNullableAnnotations: false, objectAndDynamicCompareEqually: false); + SymbolEquivalenceComparer.Create(distinguishRefFromOut: true, tupleNamesMustMatch: true, ignoreNullableAnnotations: false, objectAndDynamicCompareEqually: false, arrayAndReadOnlySpanCompareEqually: false); + + private static readonly SymbolEquivalenceComparer s_arrayAndReadOnlySpanCompareEqually = s_includeNullabilityComparer.With(arrayAndReadOnlySpanCompareEqually: true); /// /// Creates a semantic analyzer for speculative syntax replacement. @@ -403,6 +405,19 @@ private static bool SymbolsAreCompatibleCore( } } } + + // We consider two method overloads compatible if one takes a T[] array for a particular parameter, and the + // other takes a ReadOnlySpan for the same parameter. This is a considered a supported and desirable API + // upgrade story for API authors. Specifically, they start with an array-based method, and then add a + // sibling ROS method. In that case, the language will prefer the latter when both are applicable. So if + // we make a code change that makes the second compatible, then we are ok with that, as the expectation is + // that the new method has the same semantics and it is desirable for code to now call that. + // + // Note: this comparer will check the method kinds, name, containing type, arity, and virtually everything + // else about the methods. The only difference it will allow between the methods is that parameter types + // can be different if they are an array vs a ReadOnlySpan. + if (s_arrayAndReadOnlySpanCompareEqually.Equals(methodSymbol, newMethodSymbol)) + return true; } return false; @@ -440,10 +455,8 @@ newSymbol is IParameterSymbol newParameterSymbol && CompareAcrossSemanticModels(parameterSymbol.Type, newParameterSymbol.Type); } - if (symbol is IMethodSymbol methodSymbol && - newSymbol is IMethodSymbol newMethodSymbol && - methodSymbol.IsLocalFunction() && - newMethodSymbol.IsLocalFunction()) + if (symbol is IMethodSymbol { MethodKind: MethodKind.LocalFunction } methodSymbol && + newSymbol is IMethodSymbol { MethodKind: MethodKind.LocalFunction } newMethodSymbol) { return symbol.Name == newSymbol.Name && methodSymbol.Parameters.Length == newMethodSymbol.Parameters.Length && diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/SymbolEquivalenceComparer.EquivalenceVisitor.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/SymbolEquivalenceComparer.EquivalenceVisitor.cs index be5ff4083ab11..7cd4731cd37ac 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/SymbolEquivalenceComparer.EquivalenceVisitor.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/SymbolEquivalenceComparer.EquivalenceVisitor.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.Collections.Immutable; using System.Diagnostics; @@ -42,12 +43,32 @@ public bool AreEquivalent(ISymbol? x, ISymbol? y, Dictionary? equivalentTypesWithDifferingAssemblies) + { + if (array.Rank != 1) + return false; + + return AreEquivalent(array.ElementType, readOnlySpanType.TypeArguments.Single(), equivalentTypesWithDifferingAssemblies); + } + internal bool AreEquivalent(CustomModifier x, CustomModifier y, Dictionary? equivalentTypesWithDifferingAssemblies) => x.IsOptional == y.IsOptional && AreEquivalent(x.Modifier, y.Modifier, equivalentTypesWithDifferingAssemblies); diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/SymbolEquivalenceComparer.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/SymbolEquivalenceComparer.cs index ba6f0919b6590..e761cf68b93ef 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/SymbolEquivalenceComparer.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/SymbolEquivalenceComparer.cs @@ -41,14 +41,17 @@ internal sealed partial class SymbolEquivalenceComparer : IEqualityComparer _equivalenceVisitors; private readonly ImmutableArray _getHashCodeVisitors; - public static readonly SymbolEquivalenceComparer Instance = Create(distinguishRefFromOut: false, tupleNamesMustMatch: false, ignoreNullableAnnotations: true, objectAndDynamicCompareEqually: true); - public static readonly SymbolEquivalenceComparer TupleNamesMustMatchInstance = Create(distinguishRefFromOut: false, tupleNamesMustMatch: true, ignoreNullableAnnotations: true, objectAndDynamicCompareEqually: true); - public static readonly SymbolEquivalenceComparer IgnoreAssembliesInstance = new(assemblyComparer: null, distinguishRefFromOut: false, tupleNamesMustMatch: false, ignoreNullableAnnotations: true, objectAndDynamicCompareEqually: true); + public static readonly SymbolEquivalenceComparer Instance = Create(distinguishRefFromOut: false, tupleNamesMustMatch: false, ignoreNullableAnnotations: true, objectAndDynamicCompareEqually: true, arrayAndReadOnlySpanCompareEqually: false); + public static readonly SymbolEquivalenceComparer TupleNamesMustMatchInstance = Create(distinguishRefFromOut: false, tupleNamesMustMatch: true, ignoreNullableAnnotations: true, objectAndDynamicCompareEqually: true, arrayAndReadOnlySpanCompareEqually: false); + public static readonly SymbolEquivalenceComparer IgnoreAssembliesInstance = new(assemblyComparer: null, distinguishRefFromOut: false, tupleNamesMustMatch: false, ignoreNullableAnnotations: true, objectAndDynamicCompareEqually: true, arrayAndReadOnlySpanCompareEqually: false); private readonly IEqualityComparer? _assemblyComparer; + + private readonly bool _distinguishRefFromOut; private readonly bool _tupleNamesMustMatch; private readonly bool _ignoreNullableAnnotations; private readonly bool _objectAndDynamicCompareEqually; + private readonly bool _arrayAndReadOnlySpanCompareEqually; public ParameterSymbolEqualityComparer ParameterEquivalenceComparer { get; } public SignatureTypeSymbolEquivalenceComparer SignatureTypeEquivalenceComparer { get; } @@ -58,12 +61,15 @@ public SymbolEquivalenceComparer( bool distinguishRefFromOut, bool tupleNamesMustMatch, bool ignoreNullableAnnotations, - bool objectAndDynamicCompareEqually) + bool objectAndDynamicCompareEqually, + bool arrayAndReadOnlySpanCompareEqually) { _assemblyComparer = assemblyComparer; + _distinguishRefFromOut = distinguishRefFromOut; _tupleNamesMustMatch = tupleNamesMustMatch; _ignoreNullableAnnotations = ignoreNullableAnnotations; _objectAndDynamicCompareEqually = objectAndDynamicCompareEqually; + _arrayAndReadOnlySpanCompareEqually = arrayAndReadOnlySpanCompareEqually; this.ParameterEquivalenceComparer = new ParameterSymbolEqualityComparer(this, distinguishRefFromOut); this.SignatureTypeEquivalenceComparer = new SignatureTypeSymbolEquivalenceComparer(this); @@ -92,9 +98,26 @@ public static SymbolEquivalenceComparer Create( bool distinguishRefFromOut, bool tupleNamesMustMatch, bool ignoreNullableAnnotations, - bool objectAndDynamicCompareEqually) + bool objectAndDynamicCompareEqually, + bool arrayAndReadOnlySpanCompareEqually) { - return new(SimpleNameAssemblyComparer.Instance, distinguishRefFromOut, tupleNamesMustMatch, ignoreNullableAnnotations, objectAndDynamicCompareEqually); + return new(SimpleNameAssemblyComparer.Instance, distinguishRefFromOut, tupleNamesMustMatch, ignoreNullableAnnotations, objectAndDynamicCompareEqually, arrayAndReadOnlySpanCompareEqually); + } + + public SymbolEquivalenceComparer With( + Optional distinguishRefFromOut = default, + Optional tupleNamesMustMatch = default, + Optional ignoreNullableAnnotations = default, + Optional objectAndDynamicCompareEqually = default, + Optional arrayAndReadOnlySpanCompareEqually = default) + { + var newDistinguishRefFromOut = distinguishRefFromOut.HasValue ? distinguishRefFromOut.Value : _distinguishRefFromOut; + var newTupleNamesMustMatch = tupleNamesMustMatch.HasValue ? tupleNamesMustMatch.Value : _tupleNamesMustMatch; + var newIgnoreNullableAnnotations = ignoreNullableAnnotations.HasValue ? ignoreNullableAnnotations.Value : _ignoreNullableAnnotations; + var newObjectAndDynamicCompareEqually = objectAndDynamicCompareEqually.HasValue ? objectAndDynamicCompareEqually.Value : _objectAndDynamicCompareEqually; + var newArrayAndReadOnlySpanCompareEqually = arrayAndReadOnlySpanCompareEqually.HasValue ? arrayAndReadOnlySpanCompareEqually.Value : _arrayAndReadOnlySpanCompareEqually; + + return new(_assemblyComparer, newDistinguishRefFromOut, newTupleNamesMustMatch, newIgnoreNullableAnnotations, newObjectAndDynamicCompareEqually, newArrayAndReadOnlySpanCompareEqually); } // Very subtle logic here. When checking if two parameters are the same, we can end up with