From 4935399ac43de9f0fc562d4123204d7418951fdd Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 12 Nov 2024 16:11:06 -0800 Subject: [PATCH 01/12] Add fluent case for 'use collection expressoin' --- ...onExpressionForFluentDiagnosticAnalyzer.cs | 11 ++- .../UseCollectionExpressionForFluentTests.cs | 89 ++++++++++++++++++- 2 files changed, 94 insertions(+), 6 deletions(-) diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForFluentDiagnosticAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForFluentDiagnosticAnalyzer.cs index 42f9408bd91ca..9202b1ad3d802 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForFluentDiagnosticAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForFluentDiagnosticAnalyzer.cs @@ -341,8 +341,8 @@ bool IsListLike(ExpressionSyntax expression) return false; return - Implements(type, compilation.IListOfTType()) || - Implements(type, compilation.IReadOnlyListOfTType()); + EqualsOrImplements(type, compilation.IListOfTType()) || + EqualsOrImplements(type, compilation.IReadOnlyListOfTType()); } bool IsIterable(ExpressionSyntax expression) @@ -354,15 +354,18 @@ bool IsIterable(ExpressionSyntax expression) if (s_bannedTypes.Contains(type.Name)) return false; - return Implements(type, compilation.IEnumerableOfTType()) || + return EqualsOrImplements(type, compilation.IEnumerableOfTType()) || type.Equals(compilation.SpanOfTType()) || type.Equals(compilation.ReadOnlySpanOfTType()); } - static bool Implements(ITypeSymbol type, INamedTypeSymbol? interfaceType) + static bool EqualsOrImplements(ITypeSymbol type, INamedTypeSymbol? interfaceType) { if (interfaceType != null) { + if (type.OriginalDefinition.Equals(interfaceType)) + return true; + foreach (var baseInterface in type.AllInterfaces) { if (interfaceType.Equals(baseInterface.OriginalDefinition)) diff --git a/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForFluentTests.cs b/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForFluentTests.cs index b3413c13b4dba..cc4097d2d84f3 100644 --- a/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForFluentTests.cs +++ b/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForFluentTests.cs @@ -2700,7 +2700,7 @@ void M() } [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/71145")] - public async Task TestNotInParallelEnumerable() + public async Task TestNotInParallelEnumerable1() { await new VerifyCS.Test { @@ -2711,6 +2711,55 @@ public async Task TestNotInParallelEnumerable() using System.Linq; using System.Linq.Expressions; + class C + { + void M() + { + const bool shouldParallelize = false; + + IEnumerable sequence = null!; + + var result = sequence.AsParallel().ToArray(); + } + } + """, + LanguageVersion = LanguageVersion.CSharp12, + }.RunAsync(); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/71145")] + public async Task TestNotInParallelEnumerable2() + { + await new VerifyCS.Test + { + TestCode = + """ + using System; + using System.Collections.Generic; + using System.Linq; + using System.Linq.Expressions; + + class C + { + void M() + { + const bool shouldParallelize = false; + + IEnumerable sequence = null!; + + var result = shouldParallelize + ? sequence.AsParallel().ToArray() + : sequence.[|ToArray|](); + } + } + """, + FixedCode = + """ + using System; + using System.Collections.Generic; + using System.Linq; + using System.Linq.Expressions; + class C { void M() @@ -2721,7 +2770,7 @@ void M() var result = shouldParallelize ? sequence.AsParallel().ToArray() - : sequence.ToArray(); + : [.. sequence]; } } """, @@ -3066,4 +3115,40 @@ void M(int[] values, int[] x) ReferenceAssemblies = ReferenceAssemblies.Net.Net80, }.RunAsync(); } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/75870")] + public async Task TestSelectToImmutableArray() + { + await new VerifyCS.Test + { + TestCode = """ + using System.Linq; + using System.Collections.Generic; + using System.Collections.Immutable; + + class C + { + ImmutableArray GetFormattedNumbers(ImmutableArray numbers) + { + return numbers.Select(n => $"Number: {n}").[|ToImmutableArray|](); + } + } + """, + FixedCode = """ + using System.Linq; + using System.Collections.Generic; + using System.Collections.Immutable; + + class C + { + ImmutableArray GetFormattedNumbers(ImmutableArray numbers) + { + return [.. numbers.Select(n => $"Number: {n}")]; + } + } + """, + LanguageVersion = LanguageVersion.CSharp12, + ReferenceAssemblies = ReferenceAssemblies.Net.Net80, + }.RunAsync(); + } } From b64c161b1b2da1ca14486c3651ecc6f8e52d827f Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 12 Nov 2024 16:45:07 -0800 Subject: [PATCH 02/12] Add create case for 'use collection expression' --- ...onExpressionForCreateDiagnosticAnalyzer.cs | 15 +++-- ...onExpressionForFluentDiagnosticAnalyzer.cs | 52 ++------------ .../UseCollectionExpressionHelpers.cs | 59 +++++++++++++++- ...ctionExpressionForCreateCodeFixProvider.cs | 3 +- .../UseCollectionExpressionForCreateTests.cs | 67 ++++++++++++++++++- .../UseCollectionExpressionForFluentTests.cs | 36 ++++++++++ 6 files changed, 174 insertions(+), 58 deletions(-) diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForCreateDiagnosticAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForCreateDiagnosticAnalyzer.cs index de116646207c7..2cfdf81b3243a 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForCreateDiagnosticAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForCreateDiagnosticAnalyzer.cs @@ -21,9 +21,7 @@ internal sealed partial class CSharpUseCollectionExpressionForCreateDiagnosticAn EnforceOnBuildValues.UseCollectionExpressionForCreate) { public const string UnwrapArgument = nameof(UnwrapArgument); - - private static readonly ImmutableDictionary s_unwrapArgumentProperties = - ImmutableDictionary.Empty.Add(UnwrapArgument, UnwrapArgument); + public const string UseSpread = nameof(UseSpread); protected override void InitializeWorker(CodeBlockStartAnalysisContext context, INamedTypeSymbol? expressionType) => context.RegisterSyntaxNodeAction(context => AnalyzeInvocationExpression(context, expressionType), SyntaxKind.InvocationExpression); @@ -40,7 +38,7 @@ private void AnalyzeInvocationExpression(SyntaxNodeAnalysisContext context, INam return; var invocationExpression = (InvocationExpressionSyntax)context.Node; - if (!IsCollectionFactoryCreate(semanticModel, invocationExpression, out var memberAccess, out var unwrapArgument, cancellationToken)) + if (!IsCollectionFactoryCreate(semanticModel, invocationExpression, out var memberAccess, out var unwrapArgument, out var useSpread, cancellationToken)) return; // Make sure we can actually use a collection expression in place of the full invocation. @@ -52,7 +50,14 @@ private void AnalyzeInvocationExpression(SyntaxNodeAnalysisContext context, INam } var locations = ImmutableArray.Create(invocationExpression.GetLocation()); - var properties = unwrapArgument ? s_unwrapArgumentProperties : ImmutableDictionary.Empty; + var properties = ImmutableDictionary.Empty; + + if (unwrapArgument) + properties = properties.Add(UnwrapArgument, ""); + + if (useSpread) + properties = properties.Add(UseSpread, ""); + if (changesSemantics) properties = properties.Add(UseCollectionInitializerHelpers.ChangesSemanticsName, ""); diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForFluentDiagnosticAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForFluentDiagnosticAnalyzer.cs index 9202b1ad3d802..50292ab3f5721 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForFluentDiagnosticAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForFluentDiagnosticAnalyzer.cs @@ -63,17 +63,6 @@ internal sealed partial class CSharpUseCollectionExpressionForFluentDiagnosticAn nameof(System.Collections.Immutable), ]; - /// - /// Set of type-names that are blocked from moving over to collection expressions because the semantics of them are - /// known to be specialized, and thus could change semantics in undesirable ways if the compiler emitted its own - /// code as an replacement. - /// - private static readonly ImmutableHashSet s_bannedTypes = [ - nameof(ParallelEnumerable), - nameof(ParallelQuery), - // Special internal runtime interface that is optimized for fast path conversions of collections. - "IIListProvider"]; - protected override void InitializeWorker(CodeBlockStartAnalysisContext context, INamedTypeSymbol? expressionType) => context.RegisterSyntaxNodeAction(context => AnalyzeMemberAccess(context, expressionType), SyntaxKind.SimpleMemberAccessExpression); @@ -272,12 +261,12 @@ private static bool AnalyzeInvocation( // Forms like `ImmutableArray.Create(...)` or `ImmutableArray.CreateRange(...)` are fine base cases. if (current is InvocationExpressionSyntax currentInvocationExpression && - IsCollectionFactoryCreate(semanticModel, currentInvocationExpression, out var factoryMemberAccess, out var unwrapArgument, cancellationToken)) + IsCollectionFactoryCreate(semanticModel, currentInvocationExpression, out var factoryMemberAccess, out var unwrapArgument, out var useSpread, cancellationToken)) { if (!IsListLike(current)) return false; - AddArgumentsInReverse(postMatchesInReverse, GetArguments(currentInvocationExpression, unwrapArgument), useSpread: false); + AddArgumentsInReverse(postMatchesInReverse, GetArguments(currentInvocationExpression, unwrapArgument), useSpread); return true; } @@ -292,7 +281,7 @@ private static bool AnalyzeInvocation( // Down to some final collection. Like `x` in `x.Concat(y).ToArray()`. If `x` is itself is something that // can be iterated, we can convert this to `[.. x, .. y]`. Note: we only want to do this if ending with one // of the ToXXX Methods. If we just have `x.AddRange(y)` it's preference to keep that, versus `[.. x, ..y]` - if (!isAdditionMatch && IsIterable(current)) + if (!isAdditionMatch && IsIterable(semanticModel, current, cancellationToken)) { AddFinalMatch(current); return true; @@ -345,37 +334,6 @@ bool IsListLike(ExpressionSyntax expression) EqualsOrImplements(type, compilation.IReadOnlyListOfTType()); } - bool IsIterable(ExpressionSyntax expression) - { - var type = semanticModel.GetTypeInfo(expression, cancellationToken).Type; - if (type is null or IErrorTypeSymbol) - return false; - - if (s_bannedTypes.Contains(type.Name)) - return false; - - return EqualsOrImplements(type, compilation.IEnumerableOfTType()) || - type.Equals(compilation.SpanOfTType()) || - type.Equals(compilation.ReadOnlySpanOfTType()); - } - - static bool EqualsOrImplements(ITypeSymbol type, INamedTypeSymbol? interfaceType) - { - if (interfaceType != null) - { - if (type.OriginalDefinition.Equals(interfaceType)) - return true; - - foreach (var baseInterface in type.AllInterfaces) - { - if (interfaceType.Equals(baseInterface.OriginalDefinition)) - return true; - } - } - - return false; - } - static bool IsLegalInitializer(InitializerExpressionSyntax? initializer) { // We can't convert any initializer that contains an initializer in it. For example `new SomeType() { { 1, @@ -429,12 +387,12 @@ private static bool IsMatch( // Check to make sure we're not calling something banned because it would change semantics. First check if the // method itself comes from a banned type (like with an extension method). var member = state.SemanticModel.GetSymbolInfo(memberAccess, cancellationToken).Symbol; - if (s_bannedTypes.Contains(member?.ContainingType.Name)) + if (BannedTypes.Contains(member?.ContainingType.Name)) return false; // Next, check if we're invoking this on a banned type. var type = state.SemanticModel.GetTypeInfo(memberAccess.Expression, cancellationToken).Type; - if (s_bannedTypes.Contains(type?.Name)) + if (BannedTypes.Contains(type?.Name)) return false; return true; diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/UseCollectionExpressionHelpers.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/UseCollectionExpressionHelpers.cs index f7fab29e21d2e..ed7a5246fcc9f 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/UseCollectionExpressionHelpers.cs +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/UseCollectionExpressionHelpers.cs @@ -29,6 +29,17 @@ internal static class UseCollectionExpressionHelpers { private static readonly CollectionExpressionSyntax s_emptyCollectionExpression = CollectionExpression(); + /// + /// Set of type-names that are blocked from moving over to collection expressions because the semantics of them are + /// known to be specialized, and thus could change semantics in undesirable ways if the compiler emitted its own + /// code as an replacement. + /// + public static readonly ImmutableHashSet BannedTypes = [ + nameof(ParallelEnumerable), + nameof(ParallelQuery), + // Special internal runtime interface that is optimized for fast path conversions of collections. + "IIListProvider"]; + private static readonly SymbolEquivalenceComparer s_tupleNamesCanDifferComparer = SymbolEquivalenceComparer.Create( // Not relevant. We are not comparing method signatures. distinguishRefFromOut: true, @@ -941,12 +952,14 @@ public static bool IsCollectionFactoryCreate( InvocationExpressionSyntax invocationExpression, [NotNullWhen(true)] out MemberAccessExpressionSyntax? memberAccess, out bool unwrapArgument, + out bool useSpread, CancellationToken cancellationToken) { const string CreateName = nameof(ImmutableArray.Create); const string CreateRangeName = nameof(ImmutableArray.CreateRange); unwrapArgument = false; + useSpread = false; memberAccess = null; // Looking for `XXX.Create(...)` @@ -988,16 +1001,18 @@ public static bool IsCollectionFactoryCreate( // `Create(params T[])` (passing as individual elements, or an array with an initializer) // `Create(ReadOnlySpan)` (passing as a stack-alloc with an initializer) // `Create(IEnumerable)` (passing as something with an initializer. - if (!IsCompatibleSignatureAndArguments(createMethod.OriginalDefinition, out unwrapArgument)) + if (!IsCompatibleSignatureAndArguments(createMethod.OriginalDefinition, out unwrapArgument, out useSpread)) return false; return true; bool IsCompatibleSignatureAndArguments( IMethodSymbol originalCreateMethod, - out bool unwrapArgument) + out bool unwrapArgument, + out bool useSpread) { unwrapArgument = false; + useSpread = false; var arguments = invocationExpression.ArgumentList.Arguments; @@ -1040,6 +1055,14 @@ bool IsCompatibleSignatureAndArguments( unwrapArgument = true; return true; } + + if (IsIterable(semanticModel, argExpression, cancellationToken)) + { + // Convert `ImmutableArray.Create(someEnumerable)` to `[.. someEnumerable]` + unwrapArgument = false; + useSpread = true; + return true; + } } } else if (originalCreateMethod.Name is CreateName) @@ -1102,6 +1125,38 @@ originalCreateMethod.Parameters is [ } } + public static bool IsIterable(SemanticModel semanticModel, ExpressionSyntax expression, CancellationToken cancellationToken) + { + var type = semanticModel.GetTypeInfo(expression, cancellationToken).Type; + if (type is null or IErrorTypeSymbol) + return false; + + if (BannedTypes.Contains(type.Name)) + return false; + + var compilation = semanticModel.Compilation; + return EqualsOrImplements(type, compilation.IEnumerableOfTType()) || + type.Equals(compilation.SpanOfTType()) || + type.Equals(compilation.ReadOnlySpanOfTType()); + } + + public static bool EqualsOrImplements(ITypeSymbol type, INamedTypeSymbol? interfaceType) + { + if (interfaceType != null) + { + if (type.OriginalDefinition.Equals(interfaceType)) + return true; + + foreach (var baseInterface in type.AllInterfaces) + { + if (interfaceType.Equals(baseInterface.OriginalDefinition)) + return true; + } + } + + return false; + } + public static bool IsCollectionEmptyAccess( SemanticModel semanticModel, ExpressionSyntax expression, diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForCreateCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForCreateCodeFixProvider.cs index ee052d56fe70e..1270da02da892 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForCreateCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForCreateCodeFixProvider.cs @@ -40,6 +40,7 @@ protected override async Task FixAsync( CancellationToken cancellationToken) { var unwrapArgument = properties.ContainsKey(CSharpUseCollectionExpressionForCreateDiagnosticAnalyzer.UnwrapArgument); + var useSpread = properties.ContainsKey(CSharpUseCollectionExpressionForCreateDiagnosticAnalyzer.UseSpread); // We want to replace `XXX.Create(...)` with the new collection expression. To do this, we go through the // following steps. First, we replace `XXX.Create(a, b, c)` with `new(a, b, c)` (a dummy object creation @@ -62,7 +63,7 @@ protected override async Task FixAsync( semanticDocument.Root.ReplaceNode(invocationExpression, dummyObjectCreation), cancellationToken).ConfigureAwait(false); dummyObjectCreation = (ImplicitObjectCreationExpressionSyntax)newSemanticDocument.Root.GetAnnotatedNodes(dummyObjectAnnotation).Single(); var expressions = dummyObjectCreation.ArgumentList.Arguments.Select(a => a.Expression); - var matches = expressions.SelectAsArray(static e => new CollectionMatch(e, UseSpread: false)); + var matches = expressions.SelectAsArray(e => new CollectionMatch(e, useSpread)); var collectionExpression = await CreateCollectionExpressionAsync( newSemanticDocument.Document, diff --git a/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForCreateTests.cs b/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForCreateTests.cs index 5c90dc6dd2fe0..14810cc0d08e3 100644 --- a/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForCreateTests.cs +++ b/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForCreateTests.cs @@ -489,7 +489,18 @@ public async Task TestCreateRange_ComputedExpression() class C { - MyCollection i = MyCollection.CreateRange(GetValues()); + MyCollection i = [|MyCollection.[|CreateRange|](|]GetValues()); + + static IEnumerable GetValues() => default; + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + FixedCode = """ + using System; + using System.Collections.Generic; + + class C + { + MyCollection i = [.. GetValues()]; static IEnumerable GetValues() => default; } @@ -507,7 +518,13 @@ public async Task TestCreateRange_ExplicitArray1() TestCode = """ class C { - MyCollection i = MyCollection.CreateRange(new int [5]); + MyCollection i = [|MyCollection.[|CreateRange|](|]new int [5]); + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + FixedCode = """ + class C + { + MyCollection i = [.. new int [5]]; } """ + s_collectionBuilderApi + s_basicCollectionApi, LanguageVersion = LanguageVersion.CSharp12, @@ -757,7 +774,15 @@ public async Task TestCreateRange_NewImplicitObject() class C { - MyCollection i = MyCollection.CreateRange({|CS0144:new() { }|]); + MyCollection i = [|MyCollection.[|CreateRange|](|]{|CS0144:new() { }|}); + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + FixedCode = """ + using System.Collections.Generic; + + class C + { + MyCollection i = [.. {|CS8754:new() { }|}]; } """ + s_collectionBuilderApi + s_basicCollectionApi, LanguageVersion = LanguageVersion.CSharp12, @@ -1264,4 +1289,40 @@ class C """ }.RunAsync(); } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/75870")] + public async Task TestIEnumerablePassedToCreateRange() + { + await new VerifyCS.Test + { + TestCode = """ + using System.Linq; + using System.Collections.Generic; + using System.Collections.Immutable; + + class C + { + ImmutableArray GetFormattedRange() + { + return [|ImmutableArray.[|CreateRange|](|]Enumerable.Range(1, 10).Select(n => $"Item {n}")); + } + } + """, + FixedCode = """ + using System.Linq; + using System.Collections.Generic; + using System.Collections.Immutable; + + class C + { + ImmutableArray GetFormattedRange() + { + return [.. Enumerable.Range(1, 10).Select(n => $"Item {n}")]; + } + } + """, + LanguageVersion = LanguageVersion.CSharp12, + ReferenceAssemblies = ReferenceAssemblies.Net.Net80, + }.RunAsync(); + } } diff --git a/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForFluentTests.cs b/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForFluentTests.cs index cc4097d2d84f3..5dc98897c820a 100644 --- a/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForFluentTests.cs +++ b/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForFluentTests.cs @@ -3151,4 +3151,40 @@ ImmutableArray GetFormattedNumbers(ImmutableArray numbers) ReferenceAssemblies = ReferenceAssemblies.Net.Net80, }.RunAsync(); } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/75870")] + public async Task TestIEnumerablePassedToListConstructor() + { + await new VerifyCS.Test + { + TestCode = """ + using System.Linq; + using System.Collections.Generic; + using System.Collections.Immutable; + + class C + { + List GetNumbers() + { + return new List(Enumerable.Range(1, 10)); + } + } + """, + FixedCode = """ + using System.Linq; + using System.Collections.Generic; + using System.Collections.Immutable; + + class C + { + List GetNumbers() + { + return [.. Enumerable.Range(1, 10)]; + } + } + """, + LanguageVersion = LanguageVersion.CSharp12, + ReferenceAssemblies = ReferenceAssemblies.Net.Net80, + }.RunAsync(); + } } From 5c9d6f6abbaf519ced535708c176836fa34600cb Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 12 Nov 2024 17:03:50 -0800 Subject: [PATCH 03/12] Add analyzers --- .../Analyzers/CSharpAnalyzers.projitems | 1 + ...onExpressionForCreateDiagnosticAnalyzer.cs | 14 +-- ...ctionExpressionForNewDiagnosticAnalyzer.cs | 86 +++++++++++++ .../UseCollectionExpressionHelpers.cs | 116 ++++++++++++------ .../CodeFixes/CSharpCodeFixes.projitems | 1 + ...llectionExpressionForNewCodeFixProvider.cs | 79 ++++++++++++ .../Core/Analyzers/EnforceOnBuildValues.cs | 1 + .../Core/Analyzers/IDEDiagnosticIds.cs | 1 + .../PredefinedCodeFixProviderNames.cs | 1 + 9 files changed, 249 insertions(+), 51 deletions(-) create mode 100644 src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForNewDiagnosticAnalyzer.cs create mode 100644 src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForNewCodeFixProvider.cs diff --git a/src/Analyzers/CSharp/Analyzers/CSharpAnalyzers.projitems b/src/Analyzers/CSharp/Analyzers/CSharpAnalyzers.projitems index 5f550c427c2b5..ba405268c06a2 100644 --- a/src/Analyzers/CSharp/Analyzers/CSharpAnalyzers.projitems +++ b/src/Analyzers/CSharp/Analyzers/CSharpAnalyzers.projitems @@ -83,6 +83,7 @@ + diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForCreateDiagnosticAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForCreateDiagnosticAnalyzer.cs index 2cfdf81b3243a..8ce1cd87c339a 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForCreateDiagnosticAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForCreateDiagnosticAnalyzer.cs @@ -20,9 +20,6 @@ internal sealed partial class CSharpUseCollectionExpressionForCreateDiagnosticAn IDEDiagnosticIds.UseCollectionExpressionForCreateDiagnosticId, EnforceOnBuildValues.UseCollectionExpressionForCreate) { - public const string UnwrapArgument = nameof(UnwrapArgument); - public const string UseSpread = nameof(UseSpread); - protected override void InitializeWorker(CodeBlockStartAnalysisContext context, INamedTypeSymbol? expressionType) => context.RegisterSyntaxNodeAction(context => AnalyzeInvocationExpression(context, expressionType), SyntaxKind.InvocationExpression); @@ -50,16 +47,7 @@ private void AnalyzeInvocationExpression(SyntaxNodeAnalysisContext context, INam } var locations = ImmutableArray.Create(invocationExpression.GetLocation()); - var properties = ImmutableDictionary.Empty; - - if (unwrapArgument) - properties = properties.Add(UnwrapArgument, ""); - - if (useSpread) - properties = properties.Add(UseSpread, ""); - - if (changesSemantics) - properties = properties.Add(UseCollectionInitializerHelpers.ChangesSemanticsName, ""); + var properties = GetDiagnosticProperties(unwrapArgument, useSpread, changesSemantics); context.ReportDiagnostic(DiagnosticHelper.Create( Descriptor, diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForNewDiagnosticAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForNewDiagnosticAnalyzer.cs new file mode 100644 index 0000000000000..4a44e3a226916 --- /dev/null +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForNewDiagnosticAnalyzer.cs @@ -0,0 +1,86 @@ +// Licensed to the .NET Foundation under one or more agreements. +// 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 Microsoft.CodeAnalysis.CodeStyle; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Shared.CodeStyle; +using Microsoft.CodeAnalysis.Text; +using Microsoft.CodeAnalysis.UseCollectionInitializer; + +namespace Microsoft.CodeAnalysis.CSharp.UseCollectionExpression; + +using static UseCollectionExpressionHelpers; + +[DiagnosticAnalyzer(LanguageNames.CSharp)] +internal sealed partial class CSharpUseCollectionExpressionForNewDiagnosticAnalyzer() + : AbstractCSharpUseCollectionExpressionDiagnosticAnalyzer( + IDEDiagnosticIds.UseCollectionExpressionForNewDiagnosticId, + EnforceOnBuildValues.UseCollectionExpressionForNew) +{ + protected override void InitializeWorker(CodeBlockStartAnalysisContext context, INamedTypeSymbol? expressionType) + => context.RegisterSyntaxNodeAction(context => AnalyzeObjectCcreationExpression(context, expressionType), SyntaxKind.ObjectCreationExpression); + + private void AnalyzeObjectCcreationExpression(SyntaxNodeAnalysisContext context, INamedTypeSymbol? expressionType) + { + var semanticModel = context.SemanticModel; + var compilation = semanticModel.Compilation; + var syntaxTree = semanticModel.SyntaxTree; + var cancellationToken = context.CancellationToken; + + // no point in analyzing if the option is off. + var option = context.GetAnalyzerOptions().PreferCollectionExpression; + if (option.Value is CollectionExpressionPreference.Never || ShouldSkipAnalysis(context, option.Notification)) + return; + + var objectCreationExpression = (ObjectCreationExpressionSyntax)context.Node; + if (objectCreationExpression is not { ArgumentList.Arguments: [var argument] }) + return; + + var symbol = semanticModel.GetSymbolInfo(objectCreationExpression, cancellationToken).Symbol; + if (symbol is not IMethodSymbol { MethodKind: MethodKind.Constructor, Parameters: [var parameter] } || + !IsIEnumerableOfTParameter(compilation, parameter)) + { + return; + } + + if (!IsArgumentCompatibleWithIEnumerableOfT(semanticModel, argument, out var unwrapArgument, out var useSpread, cancellationToken)) + return; + + // Make sure we can actually use a collection expression in place of the full invocation. + var allowSemanticsChange = option.Value is CollectionExpressionPreference.WhenTypesLooselyMatch; + if (!CanReplaceWithCollectionExpression( + semanticModel, objectCreationExpression, expressionType, isSingletonInstance: false, allowSemanticsChange, skipVerificationForReplacedNode: true, cancellationToken, out var changesSemantics)) + { + return; + } + + var locations = ImmutableArray.Create(objectCreationExpression.GetLocation()); + var properties = GetDiagnosticProperties(unwrapArgument, useSpread, changesSemantics); + + context.ReportDiagnostic(DiagnosticHelper.Create( + Descriptor, + objectCreationExpression.NewKeyword.GetLocation(), + option.Notification, + context.Options, + additionalLocations: locations, + properties)); + + var additionalUnnecessaryLocations = ImmutableArray.Create( + syntaxTree.GetLocation(TextSpan.FromBounds( + objectCreationExpression.SpanStart, + objectCreationExpression.ArgumentList.OpenParenToken.Span.End)), + objectCreationExpression.ArgumentList.CloseParenToken.GetLocation()); + + context.ReportDiagnostic(DiagnosticHelper.CreateWithLocationTags( + UnnecessaryCodeDescriptor, + additionalUnnecessaryLocations[0], + NotificationOption2.ForSeverity(UnnecessaryCodeDescriptor.DefaultSeverity), + context.Options, + additionalLocations: locations, + additionalUnnecessaryLocations: additionalUnnecessaryLocations, + properties)); + } +} diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/UseCollectionExpressionHelpers.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/UseCollectionExpressionHelpers.cs index ed7a5246fcc9f..2ddc6dc810cb3 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/UseCollectionExpressionHelpers.cs +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/UseCollectionExpressionHelpers.cs @@ -18,6 +18,7 @@ using Microsoft.CodeAnalysis.Shared.Utilities; using Microsoft.CodeAnalysis.Text; using Microsoft.CodeAnalysis.UseCollectionExpression; +using Microsoft.CodeAnalysis.UseCollectionInitializer; using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.CSharp.UseCollectionExpression; @@ -27,6 +28,9 @@ namespace Microsoft.CodeAnalysis.CSharp.UseCollectionExpression; internal static class UseCollectionExpressionHelpers { + public const string UnwrapArgument = nameof(UnwrapArgument); + public const string UseSpread = nameof(UseSpread); + private static readonly CollectionExpressionSyntax s_emptyCollectionExpression = CollectionExpression(); /// @@ -1024,45 +1028,11 @@ bool IsCompatibleSignatureAndArguments( if (originalCreateMethod.Name is CreateRangeName) { // If we have `CreateRange(IEnumerable values)` this is legal if we have an array, or no-arg object creation. - if (originalCreateMethod.Parameters is [ - { - Type: INamedTypeSymbol - { - Name: nameof(IEnumerable), - TypeArguments: [ITypeParameterSymbol { TypeParameterKind: TypeParameterKind.Method }] - } enumerableType - }] && enumerableType.OriginalDefinition.Equals(compilation.IEnumerableOfTType())) + if (originalCreateMethod.Parameters is [var parameter] && + IsIEnumerableOfTParameter(compilation, parameter) && + arguments.Count == 1) { - var argExpression = arguments[0].Expression; - if (argExpression - is ArrayCreationExpressionSyntax { Initializer: not null } - or ImplicitArrayCreationExpressionSyntax) - { - unwrapArgument = true; - return true; - } - - if (argExpression is ObjectCreationExpressionSyntax objectCreation) - { - // Can't have any arguments, as we cannot preserve them once we grab out all the elements. - if (objectCreation.ArgumentList != null && objectCreation.ArgumentList.Arguments.Count > 0) - return false; - - // If it's got an initializer, it has to be a collection initializer (or an empty object initializer); - if (objectCreation.Initializer.IsKind(SyntaxKind.ObjectCreationExpression) && objectCreation.Initializer.Expressions.Count > 0) - return false; - - unwrapArgument = true; - return true; - } - - if (IsIterable(semanticModel, argExpression, cancellationToken)) - { - // Convert `ImmutableArray.Create(someEnumerable)` to `[.. someEnumerable]` - unwrapArgument = false; - useSpread = true; - return true; - } + return IsArgumentCompatibleWithIEnumerableOfT(semanticModel, arguments[0], out unwrapArgument, out useSpread, cancellationToken); } } else if (originalCreateMethod.Name is CreateName) @@ -1125,6 +1095,60 @@ originalCreateMethod.Parameters is [ } } + public static bool IsArgumentCompatibleWithIEnumerableOfT( + SemanticModel semanticModel, ArgumentSyntax argument, out bool unwrapArgument, out bool useSpread, CancellationToken cancellationToken) + { + unwrapArgument = false; + useSpread = false; + + var argExpression = argument.Expression; + if (argExpression + is ArrayCreationExpressionSyntax { Initializer: not null } + or ImplicitArrayCreationExpressionSyntax) + { + unwrapArgument = true; + return true; + } + + if (argExpression is ObjectCreationExpressionSyntax objectCreation) + { + // Can't have any arguments, as we cannot preserve them once we grab out all the elements. + if (objectCreation.ArgumentList != null && objectCreation.ArgumentList.Arguments.Count > 0) + return false; + + // If it's got an initializer, it has to be a collection initializer (or an empty object initializer); + if (objectCreation.Initializer.IsKind(SyntaxKind.ObjectCreationExpression) && objectCreation.Initializer.Expressions.Count > 0) + return false; + + unwrapArgument = true; + return true; + } + + if (IsIterable(semanticModel, argExpression, cancellationToken)) + { + // Convert `ImmutableArray.Create(someEnumerable)` to `[.. someEnumerable]` + unwrapArgument = false; + useSpread = true; + return true; + } + + return false; + } + + public static bool IsIEnumerableOfTParameter( + Compilation compilation, IParameterSymbol parameter) + { + return parameter is + { + Type: INamedTypeSymbol + { + Name: nameof(IEnumerable), + TypeArguments: [ITypeParameterSymbol { TypeParameterKind: TypeParameterKind.Method }] + } + enumerableType + } && enumerableType.OriginalDefinition.Equals(compilation.IEnumerableOfTType()); + } + public static bool IsIterable(SemanticModel semanticModel, ExpressionSyntax expression, CancellationToken cancellationToken) { var type = semanticModel.GetTypeInfo(expression, cancellationToken).Type; @@ -1294,4 +1318,20 @@ public static SeparatedSyntaxList GetArguments(InvocationExpress public static CollectionExpressionSyntax CreateReplacementCollectionExpressionForAnalysis(InitializerExpressionSyntax? initializer) => initializer is null ? s_emptyCollectionExpression : CollectionExpression([.. initializer.Expressions.Select(ExpressionElement)]); + + public static ImmutableDictionary GetDiagnosticProperties(bool unwrapArgument, bool useSpread, bool changesSemantics) + { + var properties = ImmutableDictionary.Empty; + + if (unwrapArgument) + properties = properties.Add(UnwrapArgument, ""); + + if (useSpread) + properties = properties.Add(UseSpread, ""); + + if (changesSemantics) + properties = properties.Add(UseCollectionInitializerHelpers.ChangesSemanticsName, ""); + + return properties; + } } diff --git a/src/Analyzers/CSharp/CodeFixes/CSharpCodeFixes.projitems b/src/Analyzers/CSharp/CodeFixes/CSharpCodeFixes.projitems index 0dcf3b255450c..74ad3aea9f2a2 100644 --- a/src/Analyzers/CSharp/CodeFixes/CSharpCodeFixes.projitems +++ b/src/Analyzers/CSharp/CodeFixes/CSharpCodeFixes.projitems @@ -98,6 +98,7 @@ + diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForNewCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForNewCodeFixProvider.cs new file mode 100644 index 0000000000000..f92df9915b0fe --- /dev/null +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForNewCodeFixProvider.cs @@ -0,0 +1,79 @@ +// Licensed to the .NET Foundation under one or more agreements. +// 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.Immutable; +using System.Composition; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Editing; +using Microsoft.CodeAnalysis.Host.Mef; +using Microsoft.CodeAnalysis.UseCollectionExpression; +using Roslyn.Utilities; + +namespace Microsoft.CodeAnalysis.CSharp.UseCollectionExpression; + +using static CSharpCollectionExpressionRewriter; +using static SyntaxFactory; + +[ExportCodeFixProvider(LanguageNames.CSharp, Name = PredefinedCodeFixProviderNames.UseCollectionExpressionForNew), Shared] +[method: ImportingConstructor] +[method: Obsolete(MefConstruction.ImportingConstructorMessage, error: true)] +internal sealed partial class CSharpUseCollectionExpressionForNewCodeFixProvider() + : AbstractUseCollectionExpressionCodeFixProvider( + CSharpCodeFixesResources.Use_collection_expression, + IDEDiagnosticIds.UseCollectionExpressionForCreateDiagnosticId) +{ + public override ImmutableArray FixableDiagnosticIds { get; } = [IDEDiagnosticIds.UseCollectionExpressionForCreateDiagnosticId]; + + protected override async Task FixAsync( + Document document, + SyntaxEditor editor, + InvocationExpressionSyntax invocationExpression, + ImmutableDictionary properties, + CancellationToken cancellationToken) + { + var unwrapArgument = properties.ContainsKey(CSharpUseCollectionExpressionForCreateDiagnosticAnalyzer.UnwrapArgument); + var useSpread = properties.ContainsKey(CSharpUseCollectionExpressionForCreateDiagnosticAnalyzer.UseSpread); + + // We want to replace `XXX.Create(...)` with the new collection expression. To do this, we go through the + // following steps. First, we replace `XXX.Create(a, b, c)` with `new(a, b, c)` (a dummy object creation + // expression). We then call into our helper which replaces expressions with collection expressions. The reason + // for the dummy object creation expression is that it serves as an actual node the rewriting code can attach an + // initializer to, by which it can figure out appropriate wrapping and indentation for the collection expression + // elements. + + var semanticDocument = await SemanticDocument.CreateAsync(document, cancellationToken).ConfigureAwait(false); + + // Get the expressions that we're going to fill the new collection expression with. + var arguments = UseCollectionExpressionHelpers.GetArguments(invocationExpression, unwrapArgument); + + var dummyObjectAnnotation = new SyntaxAnnotation(); + var dummyObjectCreation = ImplicitObjectCreationExpression(ArgumentList(arguments), initializer: null) + .WithTriviaFrom(invocationExpression) + .WithAdditionalAnnotations(dummyObjectAnnotation); + + var newSemanticDocument = await semanticDocument.WithSyntaxRootAsync( + semanticDocument.Root.ReplaceNode(invocationExpression, dummyObjectCreation), cancellationToken).ConfigureAwait(false); + dummyObjectCreation = (ImplicitObjectCreationExpressionSyntax)newSemanticDocument.Root.GetAnnotatedNodes(dummyObjectAnnotation).Single(); + var expressions = dummyObjectCreation.ArgumentList.Arguments.Select(a => a.Expression); + var matches = expressions.SelectAsArray(e => new CollectionMatch(e, useSpread)); + + var collectionExpression = await CreateCollectionExpressionAsync( + newSemanticDocument.Document, + dummyObjectCreation, + preMatches: [], + matches, + static o => o.Initializer, + static (o, i) => o.WithInitializer(i), + cancellationToken).ConfigureAwait(false); + + editor.ReplaceNode(invocationExpression, collectionExpression); + } +} diff --git a/src/Analyzers/Core/Analyzers/EnforceOnBuildValues.cs b/src/Analyzers/Core/Analyzers/EnforceOnBuildValues.cs index ec58d8982660a..16bc901f9533a 100644 --- a/src/Analyzers/Core/Analyzers/EnforceOnBuildValues.cs +++ b/src/Analyzers/Core/Analyzers/EnforceOnBuildValues.cs @@ -96,6 +96,7 @@ internal static class EnforceOnBuildValues public const EnforceOnBuild UseCollectionExpressionForCreate = /*IDE0303*/ EnforceOnBuild.Recommended; public const EnforceOnBuild UseCollectionExpressionForBuilder = /*IDE0304*/ EnforceOnBuild.Recommended; public const EnforceOnBuild UseCollectionExpressionForFluent = /*IDE0305*/ EnforceOnBuild.Recommended; + public const EnforceOnBuild UseCollectionExpressionForNew = /*IDE0306*/ EnforceOnBuild.Recommended; public const EnforceOnBuild MakeAnonymousFunctionStatic = /*IDE0320*/ EnforceOnBuild.Recommended; public const EnforceOnBuild UseSystemThreadingLock = /*IDE0330*/ EnforceOnBuild.Recommended; diff --git a/src/Analyzers/Core/Analyzers/IDEDiagnosticIds.cs b/src/Analyzers/Core/Analyzers/IDEDiagnosticIds.cs index 91856fd7d7468..abb14572b56d6 100644 --- a/src/Analyzers/Core/Analyzers/IDEDiagnosticIds.cs +++ b/src/Analyzers/Core/Analyzers/IDEDiagnosticIds.cs @@ -198,6 +198,7 @@ internal static class IDEDiagnosticIds public const string UseCollectionExpressionForCreateDiagnosticId = "IDE0303"; public const string UseCollectionExpressionForBuilderDiagnosticId = "IDE0304"; public const string UseCollectionExpressionForFluentDiagnosticId = "IDE0305"; + public const string UseCollectionExpressionForNewDiagnosticId = "IDE0306"; public const string MakeAnonymousFunctionStaticDiagnosticId = "IDE0320"; diff --git a/src/Analyzers/Core/CodeFixes/PredefinedCodeFixProviderNames.cs b/src/Analyzers/Core/CodeFixes/PredefinedCodeFixProviderNames.cs index e6e5e1a492e32..9331fb48b1947 100644 --- a/src/Analyzers/Core/CodeFixes/PredefinedCodeFixProviderNames.cs +++ b/src/Analyzers/Core/CodeFixes/PredefinedCodeFixProviderNames.cs @@ -137,6 +137,7 @@ internal static class PredefinedCodeFixProviderNames public const string UseCollectionExpressionForCreate = nameof(UseCollectionExpressionForCreate); public const string UseCollectionExpressionForEmpty = nameof(UseCollectionExpressionForEmpty); public const string UseCollectionExpressionForFluent = nameof(UseCollectionExpressionForFluent); + public const string UseCollectionExpressionForNew = nameof(UseCollectionExpressionForNew); public const string UseCollectionExpressionForStackAlloc = nameof(UseCollectionExpressionForStackAlloc); public const string UseCollectionInitializer = nameof(UseCollectionInitializer); public const string UseCompoundAssignment = nameof(UseCompoundAssignment); From 4e859a3938d8338aa62d02afdc075cb7495b1a43 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 12 Nov 2024 17:19:54 -0800 Subject: [PATCH 04/12] Add fixer --- ...onExpressionForFluentDiagnosticAnalyzer.cs | 2 +- ...ctionExpressionForNewDiagnosticAnalyzer.cs | 11 ++- .../UseCollectionExpressionHelpers.cs | 43 ++++----- ...ctionExpressionForCreateCodeFixProvider.cs | 7 +- ...llectionExpressionForNewCodeFixProvider.cs | 26 +++--- .../Tests/CSharpAnalyzers.UnitTests.projitems | 1 + .../UseCollectionExpressionForArrayTests.cs | 2 +- .../UseCollectionExpressionForFluentTests.cs | 36 -------- .../UseCollectionExpressionForNewTests.cs | 92 +++++++++++++++++++ 9 files changed, 138 insertions(+), 82 deletions(-) create mode 100644 src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForNewTests.cs diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForFluentDiagnosticAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForFluentDiagnosticAnalyzer.cs index 50292ab3f5721..721d79add95a5 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForFluentDiagnosticAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForFluentDiagnosticAnalyzer.cs @@ -266,7 +266,7 @@ private static bool AnalyzeInvocation( if (!IsListLike(current)) return false; - AddArgumentsInReverse(postMatchesInReverse, GetArguments(currentInvocationExpression, unwrapArgument), useSpread); + AddArgumentsInReverse(postMatchesInReverse, GetArguments(currentInvocationExpression.ArgumentList, unwrapArgument), useSpread); return true; } diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForNewDiagnosticAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForNewDiagnosticAnalyzer.cs index 4a44e3a226916..a2dce85ef7739 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForNewDiagnosticAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForNewDiagnosticAnalyzer.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.Generic; using System.Collections.Immutable; using Microsoft.CodeAnalysis.CodeStyle; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Shared.CodeStyle; +using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.Text; using Microsoft.CodeAnalysis.UseCollectionInitializer; @@ -21,9 +23,9 @@ internal sealed partial class CSharpUseCollectionExpressionForNewDiagnosticAnaly EnforceOnBuildValues.UseCollectionExpressionForNew) { protected override void InitializeWorker(CodeBlockStartAnalysisContext context, INamedTypeSymbol? expressionType) - => context.RegisterSyntaxNodeAction(context => AnalyzeObjectCcreationExpression(context, expressionType), SyntaxKind.ObjectCreationExpression); + => context.RegisterSyntaxNodeAction(context => AnalyzeObjectCreationExpression(context, expressionType), SyntaxKind.ObjectCreationExpression); - private void AnalyzeObjectCcreationExpression(SyntaxNodeAnalysisContext context, INamedTypeSymbol? expressionType) + private void AnalyzeObjectCreationExpression(SyntaxNodeAnalysisContext context, INamedTypeSymbol? expressionType) { var semanticModel = context.SemanticModel; var compilation = semanticModel.Compilation; @@ -41,11 +43,14 @@ private void AnalyzeObjectCcreationExpression(SyntaxNodeAnalysisContext context, var symbol = semanticModel.GetSymbolInfo(objectCreationExpression, cancellationToken).Symbol; if (symbol is not IMethodSymbol { MethodKind: MethodKind.Constructor, Parameters: [var parameter] } || - !IsIEnumerableOfTParameter(compilation, parameter)) + parameter.Type.Name != nameof(IEnumerable)) { return; } + if (!Equals(compilation.IEnumerableOfTType(), parameter.Type.OriginalDefinition)) + return; + if (!IsArgumentCompatibleWithIEnumerableOfT(semanticModel, argument, out var unwrapArgument, out var useSpread, cancellationToken)) return; diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/UseCollectionExpressionHelpers.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/UseCollectionExpressionHelpers.cs index 2ddc6dc810cb3..5d3e50199e4f7 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/UseCollectionExpressionHelpers.cs +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/UseCollectionExpressionHelpers.cs @@ -1028,9 +1028,16 @@ bool IsCompatibleSignatureAndArguments( if (originalCreateMethod.Name is CreateRangeName) { // If we have `CreateRange(IEnumerable values)` this is legal if we have an array, or no-arg object creation. - if (originalCreateMethod.Parameters is [var parameter] && - IsIEnumerableOfTParameter(compilation, parameter) && - arguments.Count == 1) + if (arguments.Count == 1 && + originalCreateMethod.Parameters is [var parameter] && + parameter is + { + Type: INamedTypeSymbol + { + Name: nameof(IEnumerable), + TypeArguments: [ITypeParameterSymbol { TypeParameterKind: TypeParameterKind.Method }] + } enumerableType + } && enumerableType.OriginalDefinition.Equals(compilation.IEnumerableOfTType())) { return IsArgumentCompatibleWithIEnumerableOfT(semanticModel, arguments[0], out unwrapArgument, out useSpread, cancellationToken); } @@ -1069,13 +1076,13 @@ bool IsCompatibleSignatureAndArguments( if (arguments.Count == 1 && compilation.SupportsRuntimeCapability(RuntimeCapability.InlineArrayTypes) && originalCreateMethod.Parameters is [ + { + Type: INamedTypeSymbol { - Type: INamedTypeSymbol - { - Name: nameof(Span) or nameof(ReadOnlySpan), - TypeArguments: [ITypeParameterSymbol { TypeParameterKind: TypeParameterKind.Method }] - } spanType - }]) + Name: nameof(Span) or nameof(ReadOnlySpan), + TypeArguments: [ITypeParameterSymbol { TypeParameterKind: TypeParameterKind.Method }] + } spanType + }]) { if (spanType.OriginalDefinition.Equals(compilation.SpanOfTType()) || spanType.OriginalDefinition.Equals(compilation.ReadOnlySpanOfTType())) @@ -1135,20 +1142,6 @@ public static bool IsArgumentCompatibleWithIEnumerableOfT( return false; } - public static bool IsIEnumerableOfTParameter( - Compilation compilation, IParameterSymbol parameter) - { - return parameter is - { - Type: INamedTypeSymbol - { - Name: nameof(IEnumerable), - TypeArguments: [ITypeParameterSymbol { TypeParameterKind: TypeParameterKind.Method }] - } - enumerableType - } && enumerableType.OriginalDefinition.Equals(compilation.IEnumerableOfTType()); - } - public static bool IsIterable(SemanticModel semanticModel, ExpressionSyntax expression, CancellationToken cancellationToken) { var type = semanticModel.GetTypeInfo(expression, cancellationToken).Type; @@ -1287,9 +1280,9 @@ static bool IsPossiblyDottedName(ExpressionSyntax name) } } - public static SeparatedSyntaxList GetArguments(InvocationExpressionSyntax invocationExpression, bool unwrapArgument) + public static SeparatedSyntaxList GetArguments(ArgumentListSyntax argumentList, bool unwrapArgument) { - var arguments = invocationExpression.ArgumentList.Arguments; + var arguments = argumentList.Arguments; // If we're not unwrapping a singular argument expression, then just pass back all the explicit argument // expressions the user wrote out. diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForCreateCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForCreateCodeFixProvider.cs index 1270da02da892..d29cecd68c36e 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForCreateCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForCreateCodeFixProvider.cs @@ -21,6 +21,7 @@ namespace Microsoft.CodeAnalysis.CSharp.UseCollectionExpression; using static CSharpCollectionExpressionRewriter; using static SyntaxFactory; +using static UseCollectionExpressionHelpers; [ExportCodeFixProvider(LanguageNames.CSharp, Name = PredefinedCodeFixProviderNames.UseCollectionExpressionForCreate), Shared] [method: ImportingConstructor] @@ -39,8 +40,8 @@ protected override async Task FixAsync( ImmutableDictionary properties, CancellationToken cancellationToken) { - var unwrapArgument = properties.ContainsKey(CSharpUseCollectionExpressionForCreateDiagnosticAnalyzer.UnwrapArgument); - var useSpread = properties.ContainsKey(CSharpUseCollectionExpressionForCreateDiagnosticAnalyzer.UseSpread); + var unwrapArgument = properties.ContainsKey(UnwrapArgument); + var useSpread = properties.ContainsKey(UseSpread); // We want to replace `XXX.Create(...)` with the new collection expression. To do this, we go through the // following steps. First, we replace `XXX.Create(a, b, c)` with `new(a, b, c)` (a dummy object creation @@ -52,7 +53,7 @@ protected override async Task FixAsync( var semanticDocument = await SemanticDocument.CreateAsync(document, cancellationToken).ConfigureAwait(false); // Get the expressions that we're going to fill the new collection expression with. - var arguments = UseCollectionExpressionHelpers.GetArguments(invocationExpression, unwrapArgument); + var arguments = GetArguments(invocationExpression.ArgumentList, unwrapArgument); var dummyObjectAnnotation = new SyntaxAnnotation(); var dummyObjectCreation = ImplicitObjectCreationExpression(ArgumentList(arguments), initializer: null) diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForNewCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForNewCodeFixProvider.cs index f92df9915b0fe..464ef81fb6a90 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForNewCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForNewCodeFixProvider.cs @@ -8,7 +8,6 @@ using System.Linq; using System.Threading; using System.Threading.Tasks; -using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.CodeFixes; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Diagnostics; @@ -21,29 +20,30 @@ namespace Microsoft.CodeAnalysis.CSharp.UseCollectionExpression; using static CSharpCollectionExpressionRewriter; using static SyntaxFactory; +using static UseCollectionExpressionHelpers; [ExportCodeFixProvider(LanguageNames.CSharp, Name = PredefinedCodeFixProviderNames.UseCollectionExpressionForNew), Shared] [method: ImportingConstructor] [method: Obsolete(MefConstruction.ImportingConstructorMessage, error: true)] internal sealed partial class CSharpUseCollectionExpressionForNewCodeFixProvider() - : AbstractUseCollectionExpressionCodeFixProvider( + : AbstractUseCollectionExpressionCodeFixProvider( CSharpCodeFixesResources.Use_collection_expression, - IDEDiagnosticIds.UseCollectionExpressionForCreateDiagnosticId) + IDEDiagnosticIds.UseCollectionExpressionForNewDiagnosticId) { - public override ImmutableArray FixableDiagnosticIds { get; } = [IDEDiagnosticIds.UseCollectionExpressionForCreateDiagnosticId]; + public override ImmutableArray FixableDiagnosticIds { get; } = [IDEDiagnosticIds.UseCollectionExpressionForNewDiagnosticId]; protected override async Task FixAsync( Document document, SyntaxEditor editor, - InvocationExpressionSyntax invocationExpression, + ObjectCreationExpressionSyntax objectCreationExpression, ImmutableDictionary properties, CancellationToken cancellationToken) { - var unwrapArgument = properties.ContainsKey(CSharpUseCollectionExpressionForCreateDiagnosticAnalyzer.UnwrapArgument); - var useSpread = properties.ContainsKey(CSharpUseCollectionExpressionForCreateDiagnosticAnalyzer.UseSpread); + var unwrapArgument = properties.ContainsKey(UnwrapArgument); + var useSpread = properties.ContainsKey(UseSpread); - // We want to replace `XXX.Create(...)` with the new collection expression. To do this, we go through the - // following steps. First, we replace `XXX.Create(a, b, c)` with `new(a, b, c)` (a dummy object creation + // We want to replace `new ...(...)` with the new collection expression. To do this, we go through the + // following steps. First, we replace `new XXX(a, b, c)` with `new(a, b, c)` (a dummy object creation // expression). We then call into our helper which replaces expressions with collection expressions. The reason // for the dummy object creation expression is that it serves as an actual node the rewriting code can attach an // initializer to, by which it can figure out appropriate wrapping and indentation for the collection expression @@ -52,15 +52,15 @@ protected override async Task FixAsync( var semanticDocument = await SemanticDocument.CreateAsync(document, cancellationToken).ConfigureAwait(false); // Get the expressions that we're going to fill the new collection expression with. - var arguments = UseCollectionExpressionHelpers.GetArguments(invocationExpression, unwrapArgument); + var arguments = GetArguments(objectCreationExpression.ArgumentList!, unwrapArgument); var dummyObjectAnnotation = new SyntaxAnnotation(); var dummyObjectCreation = ImplicitObjectCreationExpression(ArgumentList(arguments), initializer: null) - .WithTriviaFrom(invocationExpression) + .WithTriviaFrom(objectCreationExpression) .WithAdditionalAnnotations(dummyObjectAnnotation); var newSemanticDocument = await semanticDocument.WithSyntaxRootAsync( - semanticDocument.Root.ReplaceNode(invocationExpression, dummyObjectCreation), cancellationToken).ConfigureAwait(false); + semanticDocument.Root.ReplaceNode(objectCreationExpression, dummyObjectCreation), cancellationToken).ConfigureAwait(false); dummyObjectCreation = (ImplicitObjectCreationExpressionSyntax)newSemanticDocument.Root.GetAnnotatedNodes(dummyObjectAnnotation).Single(); var expressions = dummyObjectCreation.ArgumentList.Arguments.Select(a => a.Expression); var matches = expressions.SelectAsArray(e => new CollectionMatch(e, useSpread)); @@ -74,6 +74,6 @@ protected override async Task FixAsync( static (o, i) => o.WithInitializer(i), cancellationToken).ConfigureAwait(false); - editor.ReplaceNode(invocationExpression, collectionExpression); + editor.ReplaceNode(objectCreationExpression, collectionExpression); } } diff --git a/src/Analyzers/CSharp/Tests/CSharpAnalyzers.UnitTests.projitems b/src/Analyzers/CSharp/Tests/CSharpAnalyzers.UnitTests.projitems index cb7713cf037fb..4151a269c71ff 100644 --- a/src/Analyzers/CSharp/Tests/CSharpAnalyzers.UnitTests.projitems +++ b/src/Analyzers/CSharp/Tests/CSharpAnalyzers.UnitTests.projitems @@ -115,6 +115,7 @@ + diff --git a/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForArrayTests.cs b/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForArrayTests.cs index 863c202e575e3..8e8bf8c71c873 100644 --- a/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForArrayTests.cs +++ b/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForArrayTests.cs @@ -19,7 +19,7 @@ namespace Microsoft.CodeAnalysis.CSharp.Analyzers.UnitTests.UseCollectionExpress CSharpUseCollectionExpressionForArrayCodeFixProvider>; [Trait(Traits.Feature, Traits.Features.CodeActionsUseCollectionExpression)] -public class UseCollectionExpressionForArrayTests +public sealed class UseCollectionExpressionForArrayTests { [Fact] public async Task TestNotInCSharp11() diff --git a/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForFluentTests.cs b/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForFluentTests.cs index 5dc98897c820a..cc4097d2d84f3 100644 --- a/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForFluentTests.cs +++ b/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForFluentTests.cs @@ -3151,40 +3151,4 @@ ImmutableArray GetFormattedNumbers(ImmutableArray numbers) ReferenceAssemblies = ReferenceAssemblies.Net.Net80, }.RunAsync(); } - - [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/75870")] - public async Task TestIEnumerablePassedToListConstructor() - { - await new VerifyCS.Test - { - TestCode = """ - using System.Linq; - using System.Collections.Generic; - using System.Collections.Immutable; - - class C - { - List GetNumbers() - { - return new List(Enumerable.Range(1, 10)); - } - } - """, - FixedCode = """ - using System.Linq; - using System.Collections.Generic; - using System.Collections.Immutable; - - class C - { - List GetNumbers() - { - return [.. Enumerable.Range(1, 10)]; - } - } - """, - LanguageVersion = LanguageVersion.CSharp12, - ReferenceAssemblies = ReferenceAssemblies.Net.Net80, - }.RunAsync(); - } } diff --git a/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForNewTests.cs b/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForNewTests.cs new file mode 100644 index 0000000000000..250bee8dfda87 --- /dev/null +++ b/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForNewTests.cs @@ -0,0 +1,92 @@ +// Licensed to the .NET Foundation under one or more agreements. +// 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.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; + +using VerifyCS = CSharpCodeFixVerifier< + CSharpUseCollectionExpressionForNewDiagnosticAnalyzer, + CSharpUseCollectionExpressionForNewCodeFixProvider>; + +[Trait(Traits.Feature, Traits.Features.CodeActionsUseCollectionExpression)] +public sealed class UseCollectionExpressionForNewTests +{ + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/75870")] + public async Task TestIEnumerablePassedToListConstructor() + { + await new VerifyCS.Test + { + TestCode = """ + using System.Linq; + using System.Collections.Generic; + using System.Collections.Immutable; + + class C + { + List GetNumbers() + { + return [|[|new|] List(|]Enumerable.Range(1, 10)); + } + } + """, + FixedCode = """ + using System.Linq; + using System.Collections.Generic; + using System.Collections.Immutable; + + class C + { + List GetNumbers() + { + return [.. Enumerable.Range(1, 10)]; + } + } + """, + LanguageVersion = LanguageVersion.CSharp12, + ReferenceAssemblies = ReferenceAssemblies.Net.Net80, + }.RunAsync(); + } + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/75870")] + public async Task TestArrayPassedToListConstructor() + { + await new VerifyCS.Test + { + TestCode = """ + using System.Linq; + using System.Collections.Generic; + using System.Collections.Immutable; + + class C + { + List GetNumbers() + { + return [|[|new|] List(|]new[] { 1, 2, 3 }); + } + } + """, + FixedCode = """ + using System.Linq; + using System.Collections.Generic; + using System.Collections.Immutable; + + class C + { + List GetNumbers() + { + return [1, 2, 3]; + } + } + """, + LanguageVersion = LanguageVersion.CSharp12, + ReferenceAssemblies = ReferenceAssemblies.Net.Net80, + }.RunAsync(); + } +} From 94879b70c78761ea6f45a92300ac341bd2fb6def Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 12 Nov 2024 17:27:24 -0800 Subject: [PATCH 05/12] Fixup --- ...ctionExpressionForNewDiagnosticAnalyzer.cs | 19 ++++++++++++----- ...llectionExpressionForNewCodeFixProvider.cs | 4 ++-- .../UseCollectionExpressionForNewTests.cs | 21 ++++++++++++------- 3 files changed, 29 insertions(+), 15 deletions(-) diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForNewDiagnosticAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForNewDiagnosticAnalyzer.cs index a2dce85ef7739..a0896323b7fcd 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForNewDiagnosticAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForNewDiagnosticAnalyzer.cs @@ -23,24 +23,33 @@ internal sealed partial class CSharpUseCollectionExpressionForNewDiagnosticAnaly EnforceOnBuildValues.UseCollectionExpressionForNew) { protected override void InitializeWorker(CodeBlockStartAnalysisContext context, INamedTypeSymbol? expressionType) - => context.RegisterSyntaxNodeAction(context => AnalyzeObjectCreationExpression(context, expressionType), SyntaxKind.ObjectCreationExpression); + { + context.RegisterSyntaxNodeAction(context => AnalyzeObjectCreationExpression(context, expressionType), SyntaxKind.ObjectCreationExpression); + context.RegisterSyntaxNodeAction(context => AnalyzeImplicitObjectCreationExpression(context, expressionType), SyntaxKind.ImplicitObjectCreationExpression); + } private void AnalyzeObjectCreationExpression(SyntaxNodeAnalysisContext context, INamedTypeSymbol? expressionType) + => AnalyzeBaseObjectCreationExpression(context, (BaseObjectCreationExpressionSyntax)context.Node, expressionType); + + private void AnalyzeImplicitObjectCreationExpression(SyntaxNodeAnalysisContext context, INamedTypeSymbol? expressionType) + => AnalyzeBaseObjectCreationExpression(context, (BaseObjectCreationExpressionSyntax)context.Node, expressionType); + + private void AnalyzeBaseObjectCreationExpression( + SyntaxNodeAnalysisContext context, BaseObjectCreationExpressionSyntax objectCreationExpression, INamedTypeSymbol? expressionType) { var semanticModel = context.SemanticModel; var compilation = semanticModel.Compilation; var syntaxTree = semanticModel.SyntaxTree; var cancellationToken = context.CancellationToken; + if (objectCreationExpression is not { ArgumentList.Arguments: [var argument], Initializer: null }) + return; + // no point in analyzing if the option is off. var option = context.GetAnalyzerOptions().PreferCollectionExpression; if (option.Value is CollectionExpressionPreference.Never || ShouldSkipAnalysis(context, option.Notification)) return; - var objectCreationExpression = (ObjectCreationExpressionSyntax)context.Node; - if (objectCreationExpression is not { ArgumentList.Arguments: [var argument] }) - return; - var symbol = semanticModel.GetSymbolInfo(objectCreationExpression, cancellationToken).Symbol; if (symbol is not IMethodSymbol { MethodKind: MethodKind.Constructor, Parameters: [var parameter] } || parameter.Type.Name != nameof(IEnumerable)) diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForNewCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForNewCodeFixProvider.cs index 464ef81fb6a90..4111e3e2db128 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForNewCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForNewCodeFixProvider.cs @@ -26,7 +26,7 @@ namespace Microsoft.CodeAnalysis.CSharp.UseCollectionExpression; [method: ImportingConstructor] [method: Obsolete(MefConstruction.ImportingConstructorMessage, error: true)] internal sealed partial class CSharpUseCollectionExpressionForNewCodeFixProvider() - : AbstractUseCollectionExpressionCodeFixProvider( + : AbstractUseCollectionExpressionCodeFixProvider( CSharpCodeFixesResources.Use_collection_expression, IDEDiagnosticIds.UseCollectionExpressionForNewDiagnosticId) { @@ -35,7 +35,7 @@ internal sealed partial class CSharpUseCollectionExpressionForNewCodeFixProvider protected override async Task FixAsync( Document document, SyntaxEditor editor, - ObjectCreationExpressionSyntax objectCreationExpression, + BaseObjectCreationExpressionSyntax objectCreationExpression, ImmutableDictionary properties, CancellationToken cancellationToken) { diff --git a/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForNewTests.cs b/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForNewTests.cs index 250bee8dfda87..a5fc8fb3683ca 100644 --- a/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForNewTests.cs +++ b/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForNewTests.cs @@ -19,12 +19,14 @@ namespace Microsoft.CodeAnalysis.CSharp.Analyzers.UnitTests.UseCollectionExpress [Trait(Traits.Feature, Traits.Features.CodeActionsUseCollectionExpression)] public sealed class UseCollectionExpressionForNewTests { - [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/75870")] - public async Task TestIEnumerablePassedToListConstructor() + [Theory, WorkItem("https://github.com/dotnet/roslyn/issues/75870")] + [InlineData("List")] + [InlineData("")] + public async Task TestIEnumerablePassedToListConstructor(string typeName) { await new VerifyCS.Test { - TestCode = """ + TestCode = $$""" using System.Linq; using System.Collections.Generic; using System.Collections.Immutable; @@ -33,7 +35,7 @@ class C { List GetNumbers() { - return [|[|new|] List(|]Enumerable.Range(1, 10)); + return [|[|new|] {{typeName}}(|]Enumerable.Range(1, 10)); } } """, @@ -54,12 +56,15 @@ List GetNumbers() ReferenceAssemblies = ReferenceAssemblies.Net.Net80, }.RunAsync(); } - [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/75870")] - public async Task TestArrayPassedToListConstructor() + + [Theory, WorkItem("https://github.com/dotnet/roslyn/issues/75870")] + [InlineData("List")] + [InlineData("")] + public async Task TestArrayPassedToListConstructor(string typeName) { await new VerifyCS.Test { - TestCode = """ + TestCode = $$""" using System.Linq; using System.Collections.Generic; using System.Collections.Immutable; @@ -68,7 +73,7 @@ class C { List GetNumbers() { - return [|[|new|] List(|]new[] { 1, 2, 3 }); + return [|[|new|] {{typeName}}(|]new[] { 1, 2, 3 }); } } """, From 428a4223c7a7c4cf320d2955557cc8febccc581f Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 12 Nov 2024 17:32:12 -0800 Subject: [PATCH 06/12] REvert --- .../UseCollectionExpressionHelpers.cs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/UseCollectionExpressionHelpers.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/UseCollectionExpressionHelpers.cs index 5d3e50199e4f7..392599f22780e 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/UseCollectionExpressionHelpers.cs +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/UseCollectionExpressionHelpers.cs @@ -1029,15 +1029,14 @@ bool IsCompatibleSignatureAndArguments( { // If we have `CreateRange(IEnumerable values)` this is legal if we have an array, or no-arg object creation. if (arguments.Count == 1 && - originalCreateMethod.Parameters is [var parameter] && - parameter is - { - Type: INamedTypeSymbol + originalCreateMethod.Parameters is [ { - Name: nameof(IEnumerable), - TypeArguments: [ITypeParameterSymbol { TypeParameterKind: TypeParameterKind.Method }] - } enumerableType - } && enumerableType.OriginalDefinition.Equals(compilation.IEnumerableOfTType())) + Type: INamedTypeSymbol + { + Name: nameof(IEnumerable), + TypeArguments: [ITypeParameterSymbol { TypeParameterKind: TypeParameterKind.Method }] + } enumerableType + }] && enumerableType.OriginalDefinition.Equals(compilation.IEnumerableOfTType())) { return IsArgumentCompatibleWithIEnumerableOfT(semanticModel, arguments[0], out unwrapArgument, out useSpread, cancellationToken); } From 05e03df04d415060e92dadef1c8c06d97dacf125 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 12 Nov 2024 17:35:55 -0800 Subject: [PATCH 07/12] check --- .../UseCollectionExpressionHelpers.cs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/UseCollectionExpressionHelpers.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/UseCollectionExpressionHelpers.cs index 392599f22780e..d2445b059bb6e 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/UseCollectionExpressionHelpers.cs +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/UseCollectionExpressionHelpers.cs @@ -1028,15 +1028,16 @@ bool IsCompatibleSignatureAndArguments( if (originalCreateMethod.Name is CreateRangeName) { // If we have `CreateRange(IEnumerable values)` this is legal if we have an array, or no-arg object creation. - if (arguments.Count == 1 && - originalCreateMethod.Parameters is [ + if (originalCreateMethod.Parameters is [ + { + Type: INamedTypeSymbol { - Type: INamedTypeSymbol - { - Name: nameof(IEnumerable), - TypeArguments: [ITypeParameterSymbol { TypeParameterKind: TypeParameterKind.Method }] - } enumerableType - }] && enumerableType.OriginalDefinition.Equals(compilation.IEnumerableOfTType())) + Name: nameof(IEnumerable), + TypeArguments: [ITypeParameterSymbol { TypeParameterKind: TypeParameterKind.Method }] + } enumerableType + }] && + enumerableType.OriginalDefinition.Equals(compilation.IEnumerableOfTType()) && + arguments.Count == 1) { return IsArgumentCompatibleWithIEnumerableOfT(semanticModel, arguments[0], out unwrapArgument, out useSpread, cancellationToken); } From 305f868d01da8b295e25395cd8ffdaca375f5b9b Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 12 Nov 2024 18:08:05 -0800 Subject: [PATCH 08/12] lint --- .../UseCollectionExpressionHelpers.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/UseCollectionExpressionHelpers.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/UseCollectionExpressionHelpers.cs index d2445b059bb6e..e3f25377f485f 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/UseCollectionExpressionHelpers.cs +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/UseCollectionExpressionHelpers.cs @@ -1076,13 +1076,13 @@ bool IsCompatibleSignatureAndArguments( if (arguments.Count == 1 && compilation.SupportsRuntimeCapability(RuntimeCapability.InlineArrayTypes) && originalCreateMethod.Parameters is [ - { - Type: INamedTypeSymbol { - Name: nameof(Span) or nameof(ReadOnlySpan), - TypeArguments: [ITypeParameterSymbol { TypeParameterKind: TypeParameterKind.Method }] - } spanType - }]) + Type: INamedTypeSymbol + { + Name: nameof(Span) or nameof(ReadOnlySpan), + TypeArguments: [ITypeParameterSymbol { TypeParameterKind: TypeParameterKind.Method }] + } spanType + }]) { if (spanType.OriginalDefinition.Equals(compilation.SpanOfTType()) || spanType.OriginalDefinition.Equals(compilation.ReadOnlySpanOfTType())) From 8a94cdbd75d4b9670cd66fffe496793c909792a5 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 12 Nov 2024 21:17:49 -0800 Subject: [PATCH 09/12] Update test --- .../Test/Diagnostics/IDEDiagnosticIDConfigurationTests.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/EditorFeatures/Test/Diagnostics/IDEDiagnosticIDConfigurationTests.cs b/src/EditorFeatures/Test/Diagnostics/IDEDiagnosticIDConfigurationTests.cs index fb37145993f67..04c391b5490ac 100644 --- a/src/EditorFeatures/Test/Diagnostics/IDEDiagnosticIDConfigurationTests.cs +++ b/src/EditorFeatures/Test/Diagnostics/IDEDiagnosticIDConfigurationTests.cs @@ -478,6 +478,9 @@ public void CSharp_VerifyIDEDiagnosticSeveritiesAreConfigurable() # IDE0305 dotnet_diagnostic.IDE0305.severity = %value% + + # IDE0306 + dotnet_diagnostic.IDE0306.severity = %value% # IDE0320 dotnet_diagnostic.IDE0320.severity = %value% @@ -898,6 +901,7 @@ public void CSharp_VerifyIDECodeStyleOptionsAreConfigurable() ("IDE0303", "dotnet_style_prefer_collection_expression", "when_types_loosely_match"), ("IDE0304", "dotnet_style_prefer_collection_expression", "when_types_loosely_match"), ("IDE0305", "dotnet_style_prefer_collection_expression", "when_types_loosely_match"), + ("IDE0306", "dotnet_style_prefer_collection_expression", "when_types_loosely_match"), ("IDE0320", "csharp_prefer_static_anonymous_function", "true"), ("IDE0330", "csharp_prefer_system_threading_lock", "true"), ("IDE1005", "csharp_style_conditional_delegate_call", "true"), From 580377b60f784a502df2505100409d29c9def538 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 12 Nov 2024 21:21:01 -0800 Subject: [PATCH 10/12] Update test --- src/EditorFeatures/CSharpTest/Formatting/CodeCleanupTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/EditorFeatures/CSharpTest/Formatting/CodeCleanupTests.cs b/src/EditorFeatures/CSharpTest/Formatting/CodeCleanupTests.cs index e17ddcc4dd94c..aebbe79ee16bc 100644 --- a/src/EditorFeatures/CSharpTest/Formatting/CodeCleanupTests.cs +++ b/src/EditorFeatures/CSharpTest/Formatting/CodeCleanupTests.cs @@ -627,8 +627,8 @@ private void Method() } [Theory] - [InlineData(LanguageNames.CSharp, 50)] - [InlineData(LanguageNames.VisualBasic, 87)] + [InlineData(LanguageNames.CSharp, 51)] + [InlineData(LanguageNames.VisualBasic, 88)] public void VerifyAllCodeStyleFixersAreSupportedByCodeCleanup(string language, int numberOfUnsupportedDiagnosticIds) { var supportedDiagnostics = GetSupportedDiagnosticIdsForCodeCleanupService(language); From 24fc1f350fa5bbd1cf33581541f34a466d33a00f Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 12 Nov 2024 21:51:11 -0800 Subject: [PATCH 11/12] Update src/Analyzers/CSharp/Tests/CSharpAnalyzers.UnitTests.projitems --- src/Analyzers/CSharp/Tests/CSharpAnalyzers.UnitTests.projitems | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Analyzers/CSharp/Tests/CSharpAnalyzers.UnitTests.projitems b/src/Analyzers/CSharp/Tests/CSharpAnalyzers.UnitTests.projitems index 4151a269c71ff..4930c8dd698b1 100644 --- a/src/Analyzers/CSharp/Tests/CSharpAnalyzers.UnitTests.projitems +++ b/src/Analyzers/CSharp/Tests/CSharpAnalyzers.UnitTests.projitems @@ -115,7 +115,7 @@ - + From 18a8664b4ad3b14eda57672219f331e0512edbc7 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 13 Nov 2024 10:29:30 -0800 Subject: [PATCH 12/12] Add missing case --- src/Features/RulesMissingDocumentation.md | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Features/RulesMissingDocumentation.md b/src/Features/RulesMissingDocumentation.md index ad42d72a548d3..68c85896685b2 100644 --- a/src/Features/RulesMissingDocumentation.md +++ b/src/Features/RulesMissingDocumentation.md @@ -11,6 +11,7 @@ IDE0302 | | Simplify collection initialization | IDE0304 | | Simplify collection initialization | IDE0305 | | Simplify collection initialization | +IDE0306 | | Simplify collection initialization | IDE0320 | | Make anonymous function static | IDE0330 | | Use 'System.Threading.Lock' | IDE1007 | | |