From 755208d29fbf398bd78804c590d275291a7091a6 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 26 Sep 2024 13:36:25 -0700 Subject: [PATCH 01/25] Detect more fluent cases for converting collection expressions --- ...onExpressionForFluentDiagnosticAnalyzer.cs | 37 ++++- .../UseCollectionExpressionForFluentTests.cs | 136 ++++++++++++++++++ ...onInitializerTests_CollectionExpression.cs | 70 ++++++++- 3 files changed, 236 insertions(+), 7 deletions(-) diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForFluentDiagnosticAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForFluentDiagnosticAnalyzer.cs index 52084b99e4540..fb66aaf61bc5f 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForFluentDiagnosticAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForFluentDiagnosticAnalyzer.cs @@ -155,6 +155,9 @@ private static bool AnalyzeInvocation( out InitializerExpressionSyntax? existingInitializer, CancellationToken cancellationToken) { + var semanticModel = state.SemanticModel; + var compilation = semanticModel.Compilation; + existingInitializer = null; if (invocation.Expression is not MemberAccessExpressionSyntax memberAccess) return false; @@ -167,12 +170,11 @@ private static bool AnalyzeInvocation( // We don't want to offer this feature on top of some builder-type. They will commonly end with something like // `builder.ToImmutable()`. We want that case to be handled by the 'ForBuilder' analyzer instead. - var expressionType = state.SemanticModel.GetTypeInfo(memberAccess.Expression, cancellationToken).Type; + var expressionType = semanticModel.GetTypeInfo(memberAccess.Expression, cancellationToken).Type; if (expressionType is null || expressionType.Name.EndsWith("Builder", StringComparison.Ordinal)) return false; - var semanticModel = state.SemanticModel; - var compilation = semanticModel.Compilation; + var ienumerableOfTType = compilation.IEnumerableOfTType(); using var _1 = ArrayBuilder.GetInstance(out var stack); stack.Push(memberAccess.Expression); @@ -224,7 +226,6 @@ private static bool AnalyzeInvocation( { if (objectCreation is not { - ArgumentList: null or { Arguments.Count: 0 }, Initializer: null or { RawKind: (int)SyntaxKind.CollectionInitializerExpression } }) { @@ -237,7 +238,32 @@ private static bool AnalyzeInvocation( if (!IsListLike(current)) return false; - existingInitializer = objectCreation.Initializer; + if (objectCreation.ArgumentList is { Arguments.Count: 1 }) + { + // Can take a single argument if that argument is itself a collection. + var argumentType = semanticModel.GetTypeInfo(objectCreation.ArgumentList.Arguments[0].Expression, cancellationToken).Type; + if (argumentType is null) + return false; + + if (!argumentType.AllInterfaces.Any(i => Equals(i.OriginalDefinition, ienumerableOfTType))) + return false; + + // Need to push any initializer values to the matchesInReverse first, as they need to execute prior + // to the argument to the object creation itself executing. + + var synthesizedArgumentList = objectCreation.Initializer is null + ? ArgumentList() + : ArgumentList(SeparatedList(objectCreation.Initializer.Expressions.Select(Argument))); + + AddArgumentsInReverse(matchesInReverse, synthesizedArgumentList.Arguments, useSpread: false); + AddArgumentsInReverse(matchesInReverse, objectCreation.ArgumentList.Arguments, useSpread: true); + } + else if (objectCreation.ArgumentList is not null or { Arguments.Count: 0 }) + { + // Otherwise, we have to have an empty argument list. + return false; + } + return true; } @@ -359,6 +385,7 @@ static bool IsLegalInitializer(InitializerExpressionSyntax? initializer) return false; } } + return true; } } diff --git a/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForFluentTests.cs b/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForFluentTests.cs index 406b13eaa34f1..0d68e8da2d9f7 100644 --- a/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForFluentTests.cs +++ b/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForFluentTests.cs @@ -2930,4 +2930,140 @@ void M(int[] x) ReferenceAssemblies = ReferenceAssemblies.Net.Net80, }.RunAsync(); } + + [Fact] + public async Task TestObjectCreationArgument1() + { + await new VerifyCS.Test + { + TestCode = """ + using System.Linq; + using System.Collections.Generic; + + class C + { + void M(int[] values) + { + List list = new List(values).[|ToList|](); + } + } + """, + FixedCode = """ + using System.Linq; + using System.Collections.Generic; + + class C + { + void M(int[] values) + { + List list = [.. values]; + } + } + """, + LanguageVersion = LanguageVersion.CSharp12, + ReferenceAssemblies = ReferenceAssemblies.Net.Net80, + }.RunAsync(); + } + + [Fact] + public async Task TestObjectCreationArgument2() + { + await new VerifyCS.Test + { + TestCode = """ + using System.Linq; + using System.Collections.Generic; + + class C + { + void M(int[] values) + { + List list = new List(values) { 1, 2, 3 }.[|ToList|](); + } + } + """, + FixedCode = """ + using System.Linq; + using System.Collections.Generic; + + class C + { + void M(int[] values) + { + List list = [.. values, 1, 2, 3]; + } + } + """, + LanguageVersion = LanguageVersion.CSharp12, + ReferenceAssemblies = ReferenceAssemblies.Net.Net80, + }.RunAsync(); + } + + [Fact] + public async Task TestObjectCreationArgument3() + { + await new VerifyCS.Test + { + TestCode = """ + using System.Linq; + using System.Collections.Generic; + + class C + { + void M(int[] values, int[] x) + { + List list = new List(values).Concat(x).[|ToList|](); + } + } + """, + FixedCode = """ + using System.Linq; + using System.Collections.Generic; + + class C + { + void M(int[] values, int[] x) + { + List list = [.. values, .. x]; + } + } + """, + LanguageVersion = LanguageVersion.CSharp12, + ReferenceAssemblies = ReferenceAssemblies.Net.Net80, + }.RunAsync(); + } + + [Fact] + public async Task TestObjectCreationArgument4() + { + await new VerifyCS.Test + { + TestCode = """ + using System.Linq; + using System.Collections.Generic; + + class C + { + void M(int[] values, int[] x) + { + List list = new List(values) { 1, 2, 3 }.Concat(x).[|ToList|](); + } + } + """, + FixedCode = """ + using System.Linq; + using System.Collections.Generic; + + class C + { + void M(int[] values, int[] x) + { + List list = [.. values, 1, 2, 3, .. x]; + } + } + """, + LanguageVersion = LanguageVersion.CSharp12, + ReferenceAssemblies = ReferenceAssemblies.Net.Net80, + }.RunAsync(); + } } diff --git a/src/Analyzers/CSharp/Tests/UseCollectionInitializer/UseCollectionInitializerTests_CollectionExpression.cs b/src/Analyzers/CSharp/Tests/UseCollectionInitializer/UseCollectionInitializerTests_CollectionExpression.cs index 4a89e0458efa5..aeecbe9c6b37a 100644 --- a/src/Analyzers/CSharp/Tests/UseCollectionInitializer/UseCollectionInitializerTests_CollectionExpression.cs +++ b/src/Analyzers/CSharp/Tests/UseCollectionInitializer/UseCollectionInitializerTests_CollectionExpression.cs @@ -2,9 +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.Diagnostics.CodeAnalysis; -using System.Linq.Expressions; using System.Threading.Tasks; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.UseCollectionInitializer; @@ -5719,4 +5717,72 @@ void M() ReferenceAssemblies = ReferenceAssemblies.Net.Net80, }.RunAsync(); } + + [Fact] + public async Task TestObjectCreationArgument1() + { + await new VerifyCS.Test + { + TestCode = """ + using System.Linq; + using System.Collections.Generic; + + class C + { + void M(int[] values) + { + List list = [|new|] List(values); + } + } + """, + FixedCode = """ + using System.Linq; + using System.Collections.Generic; + + class C + { + void M(int[] values) + { + List list = [.. values]; + } + } + """, + LanguageVersion = LanguageVersion.CSharp12, + ReferenceAssemblies = ReferenceAssemblies.Net.Net80, + }.RunAsync(); + } + + [Fact] + public async Task TestObjectCreationArgument2() + { + await new VerifyCS.Test + { + TestCode = """ + using System.Linq; + using System.Collections.Generic; + + class C + { + void M(int[] values) + { + List list = [|new|] List(values) { 1, 2, 3 }; + } + } + """, + FixedCode = """ + using System.Linq; + using System.Collections.Generic; + + class C + { + void M(int[] values) + { + List list = [.. values, 1, 2, 3]; + } + } + """, + LanguageVersion = LanguageVersion.CSharp12, + ReferenceAssemblies = ReferenceAssemblies.Net.Net80, + }.RunAsync(); + } } From ee2f69d2287019346e4e082039a606a9dfb2a002 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 26 Sep 2024 13:36:57 -0700 Subject: [PATCH 02/25] Add workitem --- .../UseCollectionExpressionForFluentTests.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForFluentTests.cs b/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForFluentTests.cs index 0d68e8da2d9f7..64b07e979a271 100644 --- a/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForFluentTests.cs +++ b/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForFluentTests.cs @@ -2931,7 +2931,7 @@ void M(int[] x) }.RunAsync(); } - [Fact] + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/72699")] public async Task TestObjectCreationArgument1() { await new VerifyCS.Test @@ -2965,7 +2965,7 @@ void M(int[] values) }.RunAsync(); } - [Fact] + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/72699")] public async Task TestObjectCreationArgument2() { await new VerifyCS.Test @@ -2999,7 +2999,7 @@ void M(int[] values) }.RunAsync(); } - [Fact] + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/72699")] public async Task TestObjectCreationArgument3() { await new VerifyCS.Test @@ -3033,7 +3033,7 @@ void M(int[] values, int[] x) }.RunAsync(); } - [Fact] + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/72699")] public async Task TestObjectCreationArgument4() { await new VerifyCS.Test From 4e8140f9f54a8393c98b6af794ee7787b140c70f Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 26 Sep 2024 13:42:12 -0700 Subject: [PATCH 03/25] Fix --- ...onExpressionForFluentDiagnosticAnalyzer.cs | 24 +++++++++++-------- ...onInitializerTests_CollectionExpression.cs | 4 ++-- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForFluentDiagnosticAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForFluentDiagnosticAnalyzer.cs index fb66aaf61bc5f..f8c3d17b2e67a 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForFluentDiagnosticAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForFluentDiagnosticAnalyzer.cs @@ -248,23 +248,27 @@ private static bool AnalyzeInvocation( if (!argumentType.AllInterfaces.Any(i => Equals(i.OriginalDefinition, ienumerableOfTType))) return false; - // Need to push any initializer values to the matchesInReverse first, as they need to execute prior - // to the argument to the object creation itself executing. + if (matchesInReverse != null) + { + // Need to push any initializer values to the matchesInReverse first, as they need to execute prior + // to the argument to the object creation itself executing. + + if (objectCreation.Initializer != null) + AddArgumentsInReverse(matchesInReverse, ArgumentList(SeparatedList(objectCreation.Initializer.Expressions.Select(Argument))).Arguments, useSpread: false); - var synthesizedArgumentList = objectCreation.Initializer is null - ? ArgumentList() - : ArgumentList(SeparatedList(objectCreation.Initializer.Expressions.Select(Argument))); + AddArgumentsInReverse(matchesInReverse, objectCreation.ArgumentList.Arguments, useSpread: true); + } - AddArgumentsInReverse(matchesInReverse, synthesizedArgumentList.Arguments, useSpread: false); - AddArgumentsInReverse(matchesInReverse, objectCreation.ArgumentList.Arguments, useSpread: true); + return true; } - else if (objectCreation.ArgumentList is not null or { Arguments.Count: 0 }) + else if (objectCreation.ArgumentList is null or { Arguments.Count: 0 }) { // Otherwise, we have to have an empty argument list. - return false; + existingInitializer = objectCreation.Initializer; + return true; } - return true; + return false; } // Forms like `ImmutableArray.Create(...)` or `ImmutableArray.CreateRange(...)` are fine base cases. diff --git a/src/Analyzers/CSharp/Tests/UseCollectionInitializer/UseCollectionInitializerTests_CollectionExpression.cs b/src/Analyzers/CSharp/Tests/UseCollectionInitializer/UseCollectionInitializerTests_CollectionExpression.cs index aeecbe9c6b37a..fbcc4ece20f22 100644 --- a/src/Analyzers/CSharp/Tests/UseCollectionInitializer/UseCollectionInitializerTests_CollectionExpression.cs +++ b/src/Analyzers/CSharp/Tests/UseCollectionInitializer/UseCollectionInitializerTests_CollectionExpression.cs @@ -5718,7 +5718,7 @@ void M() }.RunAsync(); } - [Fact] + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/72699")] public async Task TestObjectCreationArgument1() { await new VerifyCS.Test @@ -5752,7 +5752,7 @@ void M(int[] values) }.RunAsync(); } - [Fact] + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/72699")] public async Task TestObjectCreationArgument2() { await new VerifyCS.Test From 7d557bfdac9178c0f4341b179ebbd7a1113b8d19 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 26 Sep 2024 14:24:43 -0700 Subject: [PATCH 04/25] Fixes --- ...tionExpressionForFluentDiagnosticAnalyzer.cs | 5 ++++- .../CSharpUseCollectionInitializerAnalyzer.cs | 7 ++++--- .../AbstractUseCollectionInitializerAnalyzer.cs | 16 ++++++++-------- ...seCollectionInitializerDiagnosticAnalyzer.cs | 17 +++++++---------- .../UpdateExpressionState.cs | 16 ++++++++-------- .../UseCollectionInitializerHelpers.cs | 7 +++---- .../VisualBasicCollectionInitializerAnalyzer.vb | 2 +- 7 files changed, 35 insertions(+), 35 deletions(-) diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForFluentDiagnosticAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForFluentDiagnosticAnalyzer.cs index f8c3d17b2e67a..3130d655bace5 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForFluentDiagnosticAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForFluentDiagnosticAnalyzer.cs @@ -245,8 +245,11 @@ private static bool AnalyzeInvocation( if (argumentType is null) return false; - if (!argumentType.AllInterfaces.Any(i => Equals(i.OriginalDefinition, ienumerableOfTType))) + if (!Equals(argumentType.OriginalDefinition, ienumerableOfTType) && + !argumentType.AllInterfaces.Any(i => Equals(i.OriginalDefinition, ienumerableOfTType))) + { return false; + } if (matchesInReverse != null) { diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerAnalyzer.cs index 95b976971ebfc..9eaef68450844 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerAnalyzer.cs @@ -67,14 +67,15 @@ protected override bool AnalyzeMatchesAndCollectionConstructorForCollectionExpre } var ienumerableOfTType = this.SemanticModel.Compilation.IEnumerableOfTType(); - if (constructor.Parameters[0].Type.AllInterfaces.Any(i => Equals(i.OriginalDefinition, ienumerableOfTType))) + var firstParameter = constructor.Parameters[0]; + if (Equals(firstParameter.Type.OriginalDefinition, ienumerableOfTType) || + firstParameter.Type.AllInterfaces.Any(i => Equals(i.OriginalDefinition, ienumerableOfTType))) { // Took a single argument that implements IEnumerable. We handle this by spreading that argument as the // first thing added to the collection. - matches.Insert(0, new(Statement: null, Expression: argumentList.Arguments[0].Expression, UseSpread: true)); return true; } - else if (constructor.Parameters[0] is { Type.SpecialType: SpecialType.System_Int32, Name: "capacity" }) + else if (firstParameter is { Type.SpecialType: SpecialType.System_Int32, Name: "capacity" }) { // is a single `int capacity` constructor. diff --git a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerAnalyzer.cs b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerAnalyzer.cs index 36f099096f09b..d9924999bcf82 100644 --- a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerAnalyzer.cs +++ b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerAnalyzer.cs @@ -27,7 +27,7 @@ internal abstract class AbstractUseCollectionInitializerAnalyzer< TObjectCreationExpressionSyntax, TLocalDeclarationStatementSyntax, TVariableDeclaratorSyntax, - Match, TAnalyzer> + Match, TAnalyzer> where TExpressionSyntax : SyntaxNode where TStatementSyntax : SyntaxNode where TObjectCreationExpressionSyntax : TExpressionSyntax @@ -49,11 +49,11 @@ internal abstract class AbstractUseCollectionInitializerAnalyzer< { protected abstract bool IsComplexElementInitializer(SyntaxNode expression); protected abstract bool HasExistingInvalidInitializerForCollection(); - protected abstract bool AnalyzeMatchesAndCollectionConstructorForCollectionExpression(ArrayBuilder> matches, CancellationToken cancellationToken); + protected abstract bool AnalyzeMatchesAndCollectionConstructorForCollectionExpression(ArrayBuilder> matches, CancellationToken cancellationToken); protected abstract IUpdateExpressionSyntaxHelper SyntaxHelper { get; } - public ImmutableArray> Analyze( + public ImmutableArray> Analyze( SemanticModel semanticModel, ISyntaxFacts syntaxFacts, TObjectCreationExpressionSyntax objectCreationExpression, @@ -87,7 +87,7 @@ public ImmutableArray> Analyze( } protected sealed override bool TryAddMatches( - ArrayBuilder> matches, CancellationToken cancellationToken) + ArrayBuilder> matches, CancellationToken cancellationToken) { var seenInvocation = false; var seenIndexAssignment = false; @@ -131,7 +131,7 @@ protected sealed override bool TryAddMatches( return true; } - private Match? TryAnalyzeStatement( + private Match? TryAnalyzeStatement( TStatementSyntax statement, ref bool seenInvocation, ref bool seenIndexAssignment, CancellationToken cancellationToken) { return _analyzeForCollectionExpression @@ -139,7 +139,7 @@ protected sealed override bool TryAddMatches( : TryAnalyzeStatementForCollectionInitializer(statement, ref seenInvocation, ref seenIndexAssignment, cancellationToken); } - private Match? TryAnalyzeStatementForCollectionInitializer( + private Match? TryAnalyzeStatementForCollectionInitializer( TStatementSyntax statement, ref bool seenInvocation, ref bool seenIndexAssignment, CancellationToken cancellationToken) { // At least one of these has to be false. @@ -161,7 +161,7 @@ protected sealed override bool TryAddMatches( this.State.ValuePatternMatches(instance)) { seenInvocation = true; - return new(Statement: expressionStatement, Expression: null, UseSpread: false); + return new(Statement: expressionStatement, UseSpread: false); } } @@ -171,7 +171,7 @@ protected sealed override bool TryAddMatches( this.State.ValuePatternMatches(instance)) { seenIndexAssignment = true; - return new(Statement: expressionStatement, Expression: null, UseSpread: false); + return new(Statement: expressionStatement, UseSpread: false); } } diff --git a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerDiagnosticAnalyzer.cs b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerDiagnosticAnalyzer.cs index 7d2d81a84fc9b..a663612d196d2 100644 --- a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerDiagnosticAnalyzer.cs +++ b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerDiagnosticAnalyzer.cs @@ -24,12 +24,9 @@ namespace Microsoft.CodeAnalysis.UseCollectionInitializer; /// is needed as the statement could be cases like expr.Add(x) vs. expr.AddRange(x). This property /// indicates that the latter should become a spread, without the consumer having to reexamine the statement to see /// what form it is. -internal readonly record struct Match( - TStatementSyntax? Statement, - TExpressionSyntax? Expression, - bool UseSpread) - where TStatementSyntax : SyntaxNode - where TExpressionSyntax : SyntaxNode; +internal readonly record struct Match( + TStatementSyntax Statement, + bool UseSpread) where TStatementSyntax : SyntaxNode; internal abstract partial class AbstractUseCollectionInitializerDiagnosticAnalyzer< TSyntaxKind, @@ -185,7 +182,7 @@ private void AnalyzeNode( var nodes = containingStatement is null ? ImmutableArray.Empty : [containingStatement]; - nodes = nodes.AddRange(matches.Select(static m => m.Statement ?? (SyntaxNode)m.Expression!)); + nodes = nodes.AddRange(matches.Select(static m => m.Statement)); if (syntaxFacts.ContainsInterleavedDirective(nodes, cancellationToken)) return; @@ -208,7 +205,7 @@ private void AnalyzeNode( return; - (ImmutableArray> matches, bool shouldUseCollectionExpression, bool changesSemantics)? GetCollectionInitializerMatches() + (ImmutableArray> matches, bool shouldUseCollectionExpression, bool changesSemantics)? GetCollectionInitializerMatches() { if (containingStatement is null) return null; @@ -225,7 +222,7 @@ private void AnalyzeNode( return (matches, shouldUseCollectionExpression: false, changesSemantics: false); } - (ImmutableArray> matches, bool shouldUseCollectionExpression, bool changesSemantics)? GetCollectionExpressionMatches() + (ImmutableArray> matches, bool shouldUseCollectionExpression, bool changesSemantics)? GetCollectionExpressionMatches() { if (preferExpressionOption.Value == CollectionExpressionPreference.Never) return null; @@ -251,7 +248,7 @@ private void AnalyzeNode( private void FadeOutCode( SyntaxNodeAnalysisContext context, - ImmutableArray> matches, + ImmutableArray> matches, ImmutableArray locations, ImmutableDictionary? properties) { diff --git a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/UpdateExpressionState.cs b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/UpdateExpressionState.cs index 13312bcf18e01..33d33b8e7a53b 100644 --- a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/UpdateExpressionState.cs +++ b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/UpdateExpressionState.cs @@ -337,7 +337,7 @@ private bool TryAnalyzeInvocation( /// includes calls to .Add and .AddRange, as well as foreach statements that update the /// collection, and if statements that conditionally add items to the collection-expression. /// - public Match? TryAnalyzeStatementForCollectionExpression( + public Match? TryAnalyzeStatementForCollectionExpression( IUpdateExpressionSyntaxHelper syntaxHelper, TStatementSyntax statement, CancellationToken cancellationToken) @@ -355,7 +355,7 @@ private bool TryAnalyzeInvocation( return null; - Match? TryAnalyzeExpressionStatement(TStatementSyntax expressionStatement) + Match? TryAnalyzeExpressionStatement(TStatementSyntax expressionStatement) { var expression = (TExpressionSyntax)@this.SyntaxFacts.GetExpressionOfExpressionStatement(expressionStatement); @@ -363,13 +363,13 @@ private bool TryAnalyzeInvocation( if (@this.TryAnalyzeInvocationForCollectionExpression(expression, allowLinq: false, cancellationToken, out var instance, out var useSpread) && @this.ValuePatternMatches(instance)) { - return new(Statement: expressionStatement, Sxpression: null, useSpread); + return new(Statement: expressionStatement, useSpread); } return null; } - Match? TryAnalyzeForeachStatement(TStatementSyntax foreachStatement) + Match? TryAnalyzeForeachStatement(TStatementSyntax foreachStatement) { syntaxHelper.GetPartsOfForeachStatement(foreachStatement, out var awaitKeyword, out var identifier, out _, out var foreachStatements); if (awaitKeyword != default) @@ -393,13 +393,13 @@ private bool TryAnalyzeInvocation( @this.ValuePatternMatches(instance)) { // `foreach` will become `..expr` when we make it into a collection expression. - return new(Statement: foreachStatement, Expression: null, UseSpread: true); + return new(Statement: foreachStatement, UseSpread: true); } return null; } - Match? TryAnalyzeIfStatement(TStatementSyntax ifStatement) + Match? TryAnalyzeIfStatement(TStatementSyntax ifStatement) { // look for the form: // @@ -430,7 +430,7 @@ private bool TryAnalyzeInvocation( { // add the form `.. x ? [y] : []` to the result return @this.SyntaxFacts.SupportsCollectionExpressionNaturalType(ifStatement.SyntaxTree.Options) - ? new(Statement: ifStatement, Expression: null, UseSpread: true) + ? new(Statement: ifStatement, UseSpread: true) : null; } @@ -446,7 +446,7 @@ private bool TryAnalyzeInvocation( @this.ValuePatternMatches(instance)) { // add the form `x ? y : z` to the result - return new(Statement: ifStatement, Expression, UseSpread: false); + return new(Statement: ifStatement, UseSpread: false); } } diff --git a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/UseCollectionInitializerHelpers.cs b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/UseCollectionInitializerHelpers.cs index cea848c071770..0860a10ea6eed 100644 --- a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/UseCollectionInitializerHelpers.cs +++ b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/UseCollectionInitializerHelpers.cs @@ -18,13 +18,12 @@ internal static class UseCollectionInitializerHelpers public static readonly ImmutableDictionary UseCollectionExpressionProperties = ImmutableDictionary.Empty.Add(UseCollectionExpressionName, UseCollectionExpressionName); - public static ImmutableArray GetLocationsToFade( + public static ImmutableArray GetLocationsToFade( ISyntaxFacts syntaxFacts, - Match matchInfo) + Match matchInfo) where TStatementSyntax : SyntaxNode - where TExpressionSyntax : SyntaxNode { - var match = matchInfo.Statement ?? (SyntaxNode)matchInfo.Expression!; + var match = matchInfo.Statement; var syntaxTree = match.SyntaxTree; if (syntaxFacts.IsExpressionStatement(match)) { diff --git a/src/Analyzers/VisualBasic/Analyzers/UseCollectionInitializer/VisualBasicCollectionInitializerAnalyzer.vb b/src/Analyzers/VisualBasic/Analyzers/UseCollectionInitializer/VisualBasicCollectionInitializerAnalyzer.vb index e6667bf0464bd..2a0f297dfa5c2 100644 --- a/src/Analyzers/VisualBasic/Analyzers/UseCollectionInitializer/VisualBasicCollectionInitializerAnalyzer.vb +++ b/src/Analyzers/VisualBasic/Analyzers/UseCollectionInitializer/VisualBasicCollectionInitializerAnalyzer.vb @@ -37,7 +37,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.UseCollectionInitializer Return TypeOf _objectCreationExpression.Initializer Is ObjectMemberInitializerSyntax End Function - Protected Overrides Function ValidateMatchesForCollectionExpression(matches As ArrayBuilder(Of Match(Of StatementSyntax)), cancellationToken As CancellationToken) As Boolean + Protected Overrides Function AnalyzeMatchesAndCollectionConstructorForCollectionExpression(matches As ArrayBuilder(Of Match(Of StatementSyntax)), cancellationToken As CancellationToken) As Boolean ' Only called for collection expressions, which VB does not support Throw ExceptionUtilities.Unreachable() End Function From 8d137de902994d7a76ce35825d3e387282a838e5 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 26 Sep 2024 14:42:03 -0700 Subject: [PATCH 05/25] in progress --- ...zerCodeFixProvider_CollectionExpression.cs | 26 +++++++++++++++---- ...onInitializerTests_CollectionExpression.cs | 12 ++++++--- 2 files changed, 30 insertions(+), 8 deletions(-) diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionExpression.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionExpression.cs index f84f195875f0a..7e78990c5eb35 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionExpression.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionExpression.cs @@ -5,9 +5,10 @@ using System.Collections.Immutable; using System.Threading; using System.Threading.Tasks; -using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.CSharp.UseCollectionExpression; +using Microsoft.CodeAnalysis.Shared.Collections; +using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.UseCollectionInitializer; namespace Microsoft.CodeAnalysis.CSharp.UseCollectionInitializer; @@ -18,18 +19,33 @@ internal partial class CSharpUseCollectionInitializerCodeFixProvider /// Creates the final collection-expression [...] that will replace the given expression. /// - private static Task CreateCollectionExpressionAsync( + private static async Task CreateCollectionExpressionAsync( Document document, BaseObjectCreationExpressionSyntax objectCreation, ImmutableArray> matches, CancellationToken cancellationToken) { - return CSharpCollectionExpressionRewriter.CreateCollectionExpressionAsync( + var semanticModel = await document.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false); + + using var finalMatches = TemporaryArray>.Empty; + + // If we have an argument to the constructor (that is not the 'capacity' argument), then include that as a + // spreaded value to the final collection expression before all the other elements we're adding. + if (objectCreation.ArgumentList?.Arguments is [{ Expression: var expression }]) + { + var type = semanticModel.GetTypeInfo(expression, cancellationToken).Type; + if (type?.SpecialType is not SpecialType.System_Int32) + finalMatches.Add(new(expression, UseSpread: true)); + } + + finalMatches.AddRange(matches.SelectAsArray(m => new CollectionExpressionMatch(m.Statement, m.UseSpread))); + + return await CSharpCollectionExpressionRewriter.CreateCollectionExpressionAsync( document, objectCreation, - matches.SelectAsArray(m => new CollectionExpressionMatch(m.Statement, m.UseSpread)), + finalMatches.ToImmutableAndClear(), static objectCreation => objectCreation.Initializer, static (objectCreation, initializer) => objectCreation.WithInitializer(initializer), - cancellationToken); + cancellationToken).ConfigureAwait(false); } } diff --git a/src/Analyzers/CSharp/Tests/UseCollectionInitializer/UseCollectionInitializerTests_CollectionExpression.cs b/src/Analyzers/CSharp/Tests/UseCollectionInitializer/UseCollectionInitializerTests_CollectionExpression.cs index c650d5c061155..7db9273c9efb9 100644 --- a/src/Analyzers/CSharp/Tests/UseCollectionInitializer/UseCollectionInitializerTests_CollectionExpression.cs +++ b/src/Analyzers/CSharp/Tests/UseCollectionInitializer/UseCollectionInitializerTests_CollectionExpression.cs @@ -196,14 +196,20 @@ class C [Fact] public async Task TestInField4() { - await TestMissingInRegularAndScriptAsync( - """ + await TestInRegularAndScriptAsync(""" using System.Collections.Generic; class C { List c = new List(new[] { 1, 2, 3 }); } + """, """ + using System.Collections.Generic; + + class C + { + List c = [.. new[] { 1, 2, 3 }]); + } """); } @@ -4934,7 +4940,7 @@ void M(int[] x, int[] y) { List c = [|new|] List(x); [|c.Add(|]0); - c.AddRange(y); + [|c.AddRange|](y); } } """, From 16e62efa91dafa2ef03a22e5024a7814b6ef359e Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 26 Sep 2024 14:48:41 -0700 Subject: [PATCH 06/25] in progress --- .../CSharpUseCollectionInitializerAnalyzer.cs | 5 ++-- ...zerCodeFixProvider_CollectionExpression.cs | 25 ++++--------------- ...bstractUseCollectionInitializerAnalyzer.cs | 16 ++++++------ ...CollectionInitializerDiagnosticAnalyzer.cs | 16 ++++++------ .../UpdateExpressionState.cs | 16 ++++++------ .../UseCollectionInitializerHelpers.cs | 7 +++--- ...UseCollectionInitializerCodeFixProvider.cs | 7 ++++-- 7 files changed, 39 insertions(+), 53 deletions(-) diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerAnalyzer.cs index 9eaef68450844..c61ac1628ed85 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerAnalyzer.cs @@ -45,7 +45,7 @@ protected override bool HasExistingInvalidInitializerForCollection() } protected override bool AnalyzeMatchesAndCollectionConstructorForCollectionExpression( - ArrayBuilder> matches, CancellationToken cancellationToken) + ArrayBuilder matches, CancellationToken cancellationToken) { // Constructor wasn't called with any arguments. Nothing to validate. var argumentList = _objectCreationExpression.ArgumentList; @@ -73,6 +73,7 @@ protected override bool AnalyzeMatchesAndCollectionConstructorForCollectionExpre { // Took a single argument that implements IEnumerable. We handle this by spreading that argument as the // first thing added to the collection. + matches.Insert(0, new(argumentList.Arguments[0].Expression, UseSpread: true)); return true; } else if (firstParameter is { Type.SpecialType: SpecialType.System_Int32, Name: "capacity" }) @@ -89,7 +90,7 @@ protected override bool AnalyzeMatchesAndCollectionConstructorForCollectionExpre using var _1 = ArrayBuilder.GetInstance(out var spreadElements); foreach (var match in matches) { - switch (match.Statement) + switch (match.StatementOrExpression) { case ExpressionStatementSyntax { Expression: InvocationExpressionSyntax invocation } expressionStatement: // x.AddRange(y). Have to make sure we see y.Count in the capacity list. diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionExpression.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionExpression.cs index 7e78990c5eb35..c413564b3cf7a 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionExpression.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionExpression.cs @@ -19,33 +19,18 @@ internal partial class CSharpUseCollectionInitializerCodeFixProvider /// Creates the final collection-expression [...] that will replace the given expression. /// - private static async Task CreateCollectionExpressionAsync( + private static Task CreateCollectionExpressionAsync( Document document, BaseObjectCreationExpressionSyntax objectCreation, - ImmutableArray> matches, + ImmutableArray matches, CancellationToken cancellationToken) { - var semanticModel = await document.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false); - - using var finalMatches = TemporaryArray>.Empty; - - // If we have an argument to the constructor (that is not the 'capacity' argument), then include that as a - // spreaded value to the final collection expression before all the other elements we're adding. - if (objectCreation.ArgumentList?.Arguments is [{ Expression: var expression }]) - { - var type = semanticModel.GetTypeInfo(expression, cancellationToken).Type; - if (type?.SpecialType is not SpecialType.System_Int32) - finalMatches.Add(new(expression, UseSpread: true)); - } - - finalMatches.AddRange(matches.SelectAsArray(m => new CollectionExpressionMatch(m.Statement, m.UseSpread))); - - return await CSharpCollectionExpressionRewriter.CreateCollectionExpressionAsync( + return CSharpCollectionExpressionRewriter.CreateCollectionExpressionAsync( document, objectCreation, - finalMatches.ToImmutableAndClear(), + matches.SelectAsArray(m => new CollectionExpressionMatch(m.StatementOrExpression, m.UseSpread)), static objectCreation => objectCreation.Initializer, static (objectCreation, initializer) => objectCreation.WithInitializer(initializer), - cancellationToken).ConfigureAwait(false); + cancellationToken); } } diff --git a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerAnalyzer.cs b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerAnalyzer.cs index d9924999bcf82..60552247616a9 100644 --- a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerAnalyzer.cs +++ b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerAnalyzer.cs @@ -27,7 +27,7 @@ internal abstract class AbstractUseCollectionInitializerAnalyzer< TObjectCreationExpressionSyntax, TLocalDeclarationStatementSyntax, TVariableDeclaratorSyntax, - Match, TAnalyzer> + Match, TAnalyzer> where TExpressionSyntax : SyntaxNode where TStatementSyntax : SyntaxNode where TObjectCreationExpressionSyntax : TExpressionSyntax @@ -49,11 +49,11 @@ internal abstract class AbstractUseCollectionInitializerAnalyzer< { protected abstract bool IsComplexElementInitializer(SyntaxNode expression); protected abstract bool HasExistingInvalidInitializerForCollection(); - protected abstract bool AnalyzeMatchesAndCollectionConstructorForCollectionExpression(ArrayBuilder> matches, CancellationToken cancellationToken); + protected abstract bool AnalyzeMatchesAndCollectionConstructorForCollectionExpression(ArrayBuilder matches, CancellationToken cancellationToken); protected abstract IUpdateExpressionSyntaxHelper SyntaxHelper { get; } - public ImmutableArray> Analyze( + public ImmutableArray Analyze( SemanticModel semanticModel, ISyntaxFacts syntaxFacts, TObjectCreationExpressionSyntax objectCreationExpression, @@ -87,7 +87,7 @@ public ImmutableArray> Analyze( } protected sealed override bool TryAddMatches( - ArrayBuilder> matches, CancellationToken cancellationToken) + ArrayBuilder matches, CancellationToken cancellationToken) { var seenInvocation = false; var seenIndexAssignment = false; @@ -131,7 +131,7 @@ protected sealed override bool TryAddMatches( return true; } - private Match? TryAnalyzeStatement( + private Match? TryAnalyzeStatement( TStatementSyntax statement, ref bool seenInvocation, ref bool seenIndexAssignment, CancellationToken cancellationToken) { return _analyzeForCollectionExpression @@ -139,7 +139,7 @@ protected sealed override bool TryAddMatches( : TryAnalyzeStatementForCollectionInitializer(statement, ref seenInvocation, ref seenIndexAssignment, cancellationToken); } - private Match? TryAnalyzeStatementForCollectionInitializer( + private Match? TryAnalyzeStatementForCollectionInitializer( TStatementSyntax statement, ref bool seenInvocation, ref bool seenIndexAssignment, CancellationToken cancellationToken) { // At least one of these has to be false. @@ -161,7 +161,7 @@ protected sealed override bool TryAddMatches( this.State.ValuePatternMatches(instance)) { seenInvocation = true; - return new(Statement: expressionStatement, UseSpread: false); + return new(expressionStatement, UseSpread: false); } } @@ -171,7 +171,7 @@ protected sealed override bool TryAddMatches( this.State.ValuePatternMatches(instance)) { seenIndexAssignment = true; - return new(Statement: expressionStatement, UseSpread: false); + return new(expressionStatement, UseSpread: false); } } diff --git a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerDiagnosticAnalyzer.cs b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerDiagnosticAnalyzer.cs index a663612d196d2..b2667c7ad5662 100644 --- a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerDiagnosticAnalyzer.cs +++ b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerDiagnosticAnalyzer.cs @@ -18,15 +18,13 @@ namespace Microsoft.CodeAnalysis.UseCollectionInitializer; /// Represents statements following an object initializer that should be converted into /// collection-initializer/expression elements. /// -/// The statement that follows that contains the values to add to the new -/// collection-initializer or collection-expression +/// The statement that follows that contains the values to add to the new +/// collection-initializer or collection-expression. Or the expression directly to add /// Whether or not a spread (.. x) element should be created for this statement. This /// is needed as the statement could be cases like expr.Add(x) vs. expr.AddRange(x). This property /// indicates that the latter should become a spread, without the consumer having to reexamine the statement to see /// what form it is. -internal readonly record struct Match( - TStatementSyntax Statement, - bool UseSpread) where TStatementSyntax : SyntaxNode; +internal readonly record struct Match(SyntaxNode StatementOrExpression, bool UseSpread); internal abstract partial class AbstractUseCollectionInitializerDiagnosticAnalyzer< TSyntaxKind, @@ -182,7 +180,7 @@ private void AnalyzeNode( var nodes = containingStatement is null ? ImmutableArray.Empty : [containingStatement]; - nodes = nodes.AddRange(matches.Select(static m => m.Statement)); + nodes = nodes.AddRange(matches.Select(static m => m.StatementOrExpression)); if (syntaxFacts.ContainsInterleavedDirective(nodes, cancellationToken)) return; @@ -205,7 +203,7 @@ private void AnalyzeNode( return; - (ImmutableArray> matches, bool shouldUseCollectionExpression, bool changesSemantics)? GetCollectionInitializerMatches() + (ImmutableArray matches, bool shouldUseCollectionExpression, bool changesSemantics)? GetCollectionInitializerMatches() { if (containingStatement is null) return null; @@ -222,7 +220,7 @@ private void AnalyzeNode( return (matches, shouldUseCollectionExpression: false, changesSemantics: false); } - (ImmutableArray> matches, bool shouldUseCollectionExpression, bool changesSemantics)? GetCollectionExpressionMatches() + (ImmutableArray matches, bool shouldUseCollectionExpression, bool changesSemantics)? GetCollectionExpressionMatches() { if (preferExpressionOption.Value == CollectionExpressionPreference.Never) return null; @@ -248,7 +246,7 @@ private void AnalyzeNode( private void FadeOutCode( SyntaxNodeAnalysisContext context, - ImmutableArray> matches, + ImmutableArray matches, ImmutableArray locations, ImmutableDictionary? properties) { diff --git a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/UpdateExpressionState.cs b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/UpdateExpressionState.cs index 33d33b8e7a53b..01c853c8dd27d 100644 --- a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/UpdateExpressionState.cs +++ b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/UpdateExpressionState.cs @@ -337,7 +337,7 @@ private bool TryAnalyzeInvocation( /// includes calls to .Add and .AddRange, as well as foreach statements that update the /// collection, and if statements that conditionally add items to the collection-expression. /// - public Match? TryAnalyzeStatementForCollectionExpression( + public Match? TryAnalyzeStatementForCollectionExpression( IUpdateExpressionSyntaxHelper syntaxHelper, TStatementSyntax statement, CancellationToken cancellationToken) @@ -355,7 +355,7 @@ private bool TryAnalyzeInvocation( return null; - Match? TryAnalyzeExpressionStatement(TStatementSyntax expressionStatement) + Match? TryAnalyzeExpressionStatement(TStatementSyntax expressionStatement) { var expression = (TExpressionSyntax)@this.SyntaxFacts.GetExpressionOfExpressionStatement(expressionStatement); @@ -363,13 +363,13 @@ private bool TryAnalyzeInvocation( if (@this.TryAnalyzeInvocationForCollectionExpression(expression, allowLinq: false, cancellationToken, out var instance, out var useSpread) && @this.ValuePatternMatches(instance)) { - return new(Statement: expressionStatement, useSpread); + return new(expressionStatement, useSpread); } return null; } - Match? TryAnalyzeForeachStatement(TStatementSyntax foreachStatement) + Match? TryAnalyzeForeachStatement(TStatementSyntax foreachStatement) { syntaxHelper.GetPartsOfForeachStatement(foreachStatement, out var awaitKeyword, out var identifier, out _, out var foreachStatements); if (awaitKeyword != default) @@ -393,13 +393,13 @@ private bool TryAnalyzeInvocation( @this.ValuePatternMatches(instance)) { // `foreach` will become `..expr` when we make it into a collection expression. - return new(Statement: foreachStatement, UseSpread: true); + return new(foreachStatement, UseSpread: true); } return null; } - Match? TryAnalyzeIfStatement(TStatementSyntax ifStatement) + Match? TryAnalyzeIfStatement(TStatementSyntax ifStatement) { // look for the form: // @@ -430,7 +430,7 @@ private bool TryAnalyzeInvocation( { // add the form `.. x ? [y] : []` to the result return @this.SyntaxFacts.SupportsCollectionExpressionNaturalType(ifStatement.SyntaxTree.Options) - ? new(Statement: ifStatement, UseSpread: true) + ? new(ifStatement, UseSpread: true) : null; } @@ -446,7 +446,7 @@ private bool TryAnalyzeInvocation( @this.ValuePatternMatches(instance)) { // add the form `x ? y : z` to the result - return new(Statement: ifStatement, UseSpread: false); + return new(ifStatement, UseSpread: false); } } diff --git a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/UseCollectionInitializerHelpers.cs b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/UseCollectionInitializerHelpers.cs index 0860a10ea6eed..a7a8938d83cb3 100644 --- a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/UseCollectionInitializerHelpers.cs +++ b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/UseCollectionInitializerHelpers.cs @@ -18,12 +18,11 @@ internal static class UseCollectionInitializerHelpers public static readonly ImmutableDictionary UseCollectionExpressionProperties = ImmutableDictionary.Empty.Add(UseCollectionExpressionName, UseCollectionExpressionName); - public static ImmutableArray GetLocationsToFade( + public static ImmutableArray GetLocationsToFade( ISyntaxFacts syntaxFacts, - Match matchInfo) - where TStatementSyntax : SyntaxNode + Match matchInfo) { - var match = matchInfo.Statement; + var match = matchInfo.StatementOrExpression; var syntaxTree = match.SyntaxTree; if (syntaxFacts.IsExpressionStatement(match)) { diff --git a/src/Analyzers/Core/CodeFixes/UseCollectionInitializer/AbstractUseCollectionInitializerCodeFixProvider.cs b/src/Analyzers/Core/CodeFixes/UseCollectionInitializer/AbstractUseCollectionInitializerCodeFixProvider.cs index 3bd8365d3a7b3..4ab7ee7f7f9cf 100644 --- a/src/Analyzers/Core/CodeFixes/UseCollectionInitializer/AbstractUseCollectionInitializerCodeFixProvider.cs +++ b/src/Analyzers/Core/CodeFixes/UseCollectionInitializer/AbstractUseCollectionInitializerCodeFixProvider.cs @@ -54,7 +54,7 @@ public sealed override ImmutableArray FixableDiagnosticIds protected abstract TAnalyzer GetAnalyzer(); protected abstract Task<(SyntaxNode oldNode, SyntaxNode newNode)> GetReplacementNodesAsync( - Document document, TObjectCreationExpressionSyntax objectCreation, bool useCollectionExpression, ImmutableArray> matches, CancellationToken cancellationToken); + Document document, TObjectCreationExpressionSyntax objectCreation, bool useCollectionExpression, ImmutableArray matches, CancellationToken cancellationToken); protected sealed override async Task FixAsync( Document document, @@ -86,6 +86,9 @@ protected sealed override async Task FixAsync( editor.ReplaceNode(oldNode, newNode); foreach (var match in matches) - editor.RemoveNode(match.Statement, SyntaxRemoveOptions.KeepUnbalancedDirectives); + { + if (match.StatementOrExpression is TStatementSyntax statement) + editor.RemoveNode(statement, SyntaxRemoveOptions.KeepUnbalancedDirectives); + } } } From 630649e2736022766b1cfebe3bebb948d7733dab Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 26 Sep 2024 14:53:53 -0700 Subject: [PATCH 07/25] in progress --- ...ollectionExpressionForBuilderDiagnosticAnalyzer.cs | 4 ++-- .../CollectionExpressionMatch.cs | 2 +- ...seCollectionExpressionForBuilderCodeFixProvider.cs | 5 ++--- .../CSharpUseCollectionInitializerCodeFixProvider.cs | 4 ++-- ...nitializerCodeFixProvider_CollectionInitializer.cs | 6 +++--- ...CollectionInitializerTests_CollectionExpression.cs | 4 ++-- ...tractUseCollectionInitializerDiagnosticAnalyzer.cs | 2 +- .../VisualBasicCollectionInitializerAnalyzer.vb | 2 +- ...ualBasicUseCollectionInitializerCodeFixProvider.vb | 11 +++++------ 9 files changed, 19 insertions(+), 21 deletions(-) diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForBuilderDiagnosticAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForBuilderDiagnosticAnalyzer.cs index ab5493c28229e..3a582e4a77d08 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForBuilderDiagnosticAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForBuilderDiagnosticAnalyzer.cs @@ -159,7 +159,7 @@ void FadeOutCode(SyntaxNodeAnalysisContext context, AnalysisResult analysisResul identifier, initializedSymbol: semanticModel.GetDeclaredSymbol(declarator, cancellationToken)); - using var _ = ArrayBuilder>.GetInstance(out var matches); + using var _ = ArrayBuilder.GetInstance(out var matches); // Now walk all the statement after the local declaration. using var enumerator = state.GetSubsequentStatements().GetEnumerator(); @@ -249,6 +249,6 @@ public readonly record struct AnalysisResult( Location DiagnosticLocation, LocalDeclarationStatementSyntax LocalDeclarationStatement, InvocationExpressionSyntax CreationExpression, - ImmutableArray> Matches, + ImmutableArray Matches, bool ChangesSemantics); } diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CollectionExpressionMatch.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CollectionExpressionMatch.cs index 8528791948c89..273dc0a553097 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CollectionExpressionMatch.cs +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CollectionExpressionMatch.cs @@ -6,7 +6,7 @@ namespace Microsoft.CodeAnalysis.CSharp.UseCollectionExpression; -/// +/// internal readonly record struct CollectionExpressionMatch( TMatchNode Node, bool UseSpread) where TMatchNode : SyntaxNode; diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForBuilderCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForBuilderCodeFixProvider.cs index 7079f10191638..37cfb81ca743f 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForBuilderCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForBuilderCodeFixProvider.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; @@ -66,7 +65,7 @@ protected override async Task FixAsync( var collectionExpression = await CreateCollectionExpressionAsync( newDocument, dummyObjectCreation, - analysisResult.Matches.SelectAsArray(m => new CollectionExpressionMatch(m.Statement, m.UseSpread)), + analysisResult.Matches.SelectAsArray(m => new CollectionExpressionMatch(m.StatementOrExpression, m.UseSpread)), static o => o.Initializer, static (o, i) => o.WithInitializer(i), cancellationToken).ConfigureAwait(false); @@ -92,7 +91,7 @@ static AnalysisResult TrackAnalysisResult(SyntaxNode root, AnalysisResult analys => new(analysisResult.DiagnosticLocation, root.GetCurrentNode(analysisResult.LocalDeclarationStatement)!, root.GetCurrentNode(analysisResult.CreationExpression)!, - analysisResult.Matches.SelectAsArray(m => new Match(root.GetCurrentNode(m.Statement)!, m.UseSpread)), + analysisResult.Matches.SelectAsArray(m => new Match(root.GetCurrentNode(m.StatementOrExpression)!, m.UseSpread)), analysisResult.ChangesSemantics); // Creates a new document with all of the relevant nodes in analysisResult tracked so that we can find them diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider.cs index 3203bff642c22..5482ee71ad778 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider.cs @@ -37,7 +37,7 @@ protected override CSharpUseCollectionInitializerAnalyzer GetAnalyzer() Document document, BaseObjectCreationExpressionSyntax objectCreation, bool useCollectionExpression, - ImmutableArray> matches, + ImmutableArray matches, CancellationToken cancellationToken) { var newObjectCreation = await GetNewObjectCreationAsync( @@ -49,7 +49,7 @@ private static async Task GetNewObjectCreationAsync( Document document, BaseObjectCreationExpressionSyntax objectCreation, bool useCollectionExpression, - ImmutableArray> matches, + ImmutableArray matches, CancellationToken cancellationToken) { return useCollectionExpression diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionInitializer.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionInitializer.cs index f1d0323603ec6..985875a82836f 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionInitializer.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionInitializer.cs @@ -23,7 +23,7 @@ internal partial class CSharpUseCollectionInitializerCodeFixProvider { private static BaseObjectCreationExpressionSyntax CreateObjectInitializerExpression( BaseObjectCreationExpressionSyntax objectCreation, - ImmutableArray> matches) + ImmutableArray matches) { var expressions = CreateCollectionInitializerExpressions(); var withLineBreaks = AddLineBreaks(expressions); @@ -35,13 +35,13 @@ SeparatedSyntaxList CreateCollectionInitializerExpressions() { using var _ = ArrayBuilder.GetInstance(out var nodesAndTokens); - UseInitializerHelpers.AddExistingItems, ExpressionSyntax>( + UseInitializerHelpers.AddExistingItems( objectCreation, nodesAndTokens, addTrailingComma: matches.Length > 0, static (_, expression) => expression); for (var i = 0; i < matches.Length; i++) { var match = matches[i]; - var statement = (ExpressionStatementSyntax)match.Statement; + var statement = (ExpressionStatementSyntax)match.StatementOrExpression; var trivia = statement.GetLeadingTrivia(); var leadingTrivia = i == 0 ? trivia.WithoutLeadingBlankLines() : trivia; diff --git a/src/Analyzers/CSharp/Tests/UseCollectionInitializer/UseCollectionInitializerTests_CollectionExpression.cs b/src/Analyzers/CSharp/Tests/UseCollectionInitializer/UseCollectionInitializerTests_CollectionExpression.cs index 7db9273c9efb9..6e7c54ce0de49 100644 --- a/src/Analyzers/CSharp/Tests/UseCollectionInitializer/UseCollectionInitializerTests_CollectionExpression.cs +++ b/src/Analyzers/CSharp/Tests/UseCollectionInitializer/UseCollectionInitializerTests_CollectionExpression.cs @@ -201,7 +201,7 @@ await TestInRegularAndScriptAsync(""" class C { - List c = new List(new[] { 1, 2, 3 }); + List c = [|new|] List(new[] { 1, 2, 3 }); } """, """ using System.Collections.Generic; @@ -4940,7 +4940,7 @@ void M(int[] x, int[] y) { List c = [|new|] List(x); [|c.Add(|]0); - [|c.AddRange|](y); + [|c.AddRange(|]y); } } """, diff --git a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerDiagnosticAnalyzer.cs b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerDiagnosticAnalyzer.cs index b2667c7ad5662..211273817c68e 100644 --- a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerDiagnosticAnalyzer.cs +++ b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerDiagnosticAnalyzer.cs @@ -19,7 +19,7 @@ namespace Microsoft.CodeAnalysis.UseCollectionInitializer; /// collection-initializer/expression elements. /// /// The statement that follows that contains the values to add to the new -/// collection-initializer or collection-expression. Or the expression directly to add +/// collection-initializer or collection-expression. Or the expression directly to add. /// Whether or not a spread (.. x) element should be created for this statement. This /// is needed as the statement could be cases like expr.Add(x) vs. expr.AddRange(x). This property /// indicates that the latter should become a spread, without the consumer having to reexamine the statement to see diff --git a/src/Analyzers/VisualBasic/Analyzers/UseCollectionInitializer/VisualBasicCollectionInitializerAnalyzer.vb b/src/Analyzers/VisualBasic/Analyzers/UseCollectionInitializer/VisualBasicCollectionInitializerAnalyzer.vb index 2a0f297dfa5c2..554cd29ec2269 100644 --- a/src/Analyzers/VisualBasic/Analyzers/UseCollectionInitializer/VisualBasicCollectionInitializerAnalyzer.vb +++ b/src/Analyzers/VisualBasic/Analyzers/UseCollectionInitializer/VisualBasicCollectionInitializerAnalyzer.vb @@ -37,7 +37,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.UseCollectionInitializer Return TypeOf _objectCreationExpression.Initializer Is ObjectMemberInitializerSyntax End Function - Protected Overrides Function AnalyzeMatchesAndCollectionConstructorForCollectionExpression(matches As ArrayBuilder(Of Match(Of StatementSyntax)), cancellationToken As CancellationToken) As Boolean + Protected Overrides Function AnalyzeMatchesAndCollectionConstructorForCollectionExpression(matches As ArrayBuilder(Of Match), cancellationToken As CancellationToken) As Boolean ' Only called for collection expressions, which VB does not support Throw ExceptionUtilities.Unreachable() End Function diff --git a/src/Analyzers/VisualBasic/CodeFixes/UseCollectionInitializer/VisualBasicUseCollectionInitializerCodeFixProvider.vb b/src/Analyzers/VisualBasic/CodeFixes/UseCollectionInitializer/VisualBasicUseCollectionInitializerCodeFixProvider.vb index 4d0b85942cbfe..b7e02b0fc6469 100644 --- a/src/Analyzers/VisualBasic/CodeFixes/UseCollectionInitializer/VisualBasicUseCollectionInitializerCodeFixProvider.vb +++ b/src/Analyzers/VisualBasic/CodeFixes/UseCollectionInitializer/VisualBasicUseCollectionInitializerCodeFixProvider.vb @@ -6,7 +6,6 @@ Imports System.Collections.Immutable Imports System.Composition Imports System.Diagnostics.CodeAnalysis Imports System.Threading -Imports Microsoft.CodeAnalysis.CodeActions Imports Microsoft.CodeAnalysis.CodeFixes Imports Microsoft.CodeAnalysis.Formatting Imports Microsoft.CodeAnalysis.PooledObjects @@ -42,7 +41,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.UseCollectionInitializer document As Document, objectCreation As ObjectCreationExpressionSyntax, useCollectionExpression As Boolean, - matches As ImmutableArray(Of Match(Of StatementSyntax)), + matches As ImmutableArray(Of Match), cancellationToken As CancellationToken) As Task(Of (SyntaxNode, SyntaxNode)) Contract.ThrowIfTrue(useCollectionExpression, "VB does not support collection expressions") @@ -56,7 +55,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.UseCollectionInitializer totalTrivia.Add(SyntaxFactory.ElasticMarker) For Each match In matches - For Each trivia In match.Statement.GetLeadingTrivia() + For Each trivia In match.StatementOrExpression.GetLeadingTrivia() If trivia.Kind = SyntaxKind.CommentTrivia Then totalTrivia.Add(trivia) totalTrivia.Add(SyntaxFactory.ElasticMarker) @@ -70,7 +69,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.UseCollectionInitializer Private Shared Function GetNewObjectCreation( objectCreation As ObjectCreationExpressionSyntax, - matches As ImmutableArray(Of Match(Of StatementSyntax))) As ObjectCreationExpressionSyntax + matches As ImmutableArray(Of Match)) As ObjectCreationExpressionSyntax Return UseInitializerHelpers.GetNewObjectCreation( objectCreation, @@ -80,13 +79,13 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.UseCollectionInitializer Private Shared Function CreateCollectionInitializer( objectCreation As ObjectCreationExpressionSyntax, - matches As ImmutableArray(Of Match(Of StatementSyntax))) As CollectionInitializerSyntax + matches As ImmutableArray(Of Match)) As CollectionInitializerSyntax Dim nodesAndTokens = ArrayBuilder(Of SyntaxNodeOrToken).GetInstance() AddExistingItems(objectCreation, nodesAndTokens) For i = 0 To matches.Length - 1 - Dim expressionStatement = DirectCast(matches(i).Statement, ExpressionStatementSyntax) + Dim expressionStatement = DirectCast(matches(i).StatementOrExpression, ExpressionStatementSyntax) Dim newExpression As ExpressionSyntax Dim invocationExpression = DirectCast(expressionStatement.Expression, InvocationExpressionSyntax) From 2cf30556639bac7f85d2e63aedad214f38d8022c Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 26 Sep 2024 15:00:19 -0700 Subject: [PATCH 08/25] Fix test --- ...llectionInitializerTests_CollectionExpression.cs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/Analyzers/CSharp/Tests/UseCollectionInitializer/UseCollectionInitializerTests_CollectionExpression.cs b/src/Analyzers/CSharp/Tests/UseCollectionInitializer/UseCollectionInitializerTests_CollectionExpression.cs index 6e7c54ce0de49..292869e914530 100644 --- a/src/Analyzers/CSharp/Tests/UseCollectionInitializer/UseCollectionInitializerTests_CollectionExpression.cs +++ b/src/Analyzers/CSharp/Tests/UseCollectionInitializer/UseCollectionInitializerTests_CollectionExpression.cs @@ -208,7 +208,7 @@ class C class C { - List c = [.. new[] { 1, 2, 3 }]); + List c = [.. new[] { 1, 2, 3 }]; } """); } @@ -4951,11 +4951,7 @@ class C { void M(int[] x, int[] y) { - List c = new List(x) - { - 0 - }; - c.AddRange(y); + List c = [.. x, 0, .. y]; } } """); @@ -5786,7 +5782,9 @@ void M(int[] values) List list = [.. values, 1, 2, 3]; } } - """ + """, + LanguageVersion = LanguageVersion.CSharp12, + ReferenceAssemblies = ReferenceAssemblies.Net.Net80, }.RunAsync(); } @@ -5837,6 +5835,7 @@ public void MethodTakingEnumerable(IEnumerable param) public class Class1 { } public class Class2 { } + } """, LanguageVersion = LanguageVersion.CSharp12, ReferenceAssemblies = ReferenceAssemblies.Net.Net80, From e5d4dea31f7b7a18202c523ea9289f22a2aae6bd Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 27 Sep 2024 11:34:10 -0700 Subject: [PATCH 09/25] generics --- .../CSharpUseCollectionInitializerAnalyzer.cs | 21 ++++++++++++--- ...CollectionInitializerDiagnosticAnalyzer.cs | 1 + ...CSharpUseNamedMemberInitializerAnalyzer.cs | 1 + ...pUseObjectInitializerDiagnosticAnalyzer.cs | 1 + ...UseCollectionInitializerCodeFixProvider.cs | 2 +- ...harpUseObjectInitializerCodeFixProvider.cs | 1 + ...bstractObjectCreationExpressionAnalyzer.cs | 11 +++++--- ...bstractUseCollectionInitializerAnalyzer.cs | 27 +++++++++++++------ ...CollectionInitializerDiagnosticAnalyzer.cs | 7 +++-- ...tUseObjectInitializerDiagnosticAnalyzer.cs | 3 +++ .../UseNamedMemberInitializerAnalyzer.cs | 12 ++++++++- ...UseCollectionInitializerCodeFixProvider.cs | 6 +++-- ...ractUseObjectInitializerCodeFixProvider.cs | 4 ++- ...isualBasicCollectionInitializerAnalyzer.vb | 6 ++++- ...CollectionInitializerDiagnosticAnalyzer.vb | 1 + ...lBasicUseNamedMemberInitializerAnalyzer.vb | 1 + ...cUseObjectInitializerDiagnosticAnalyzer.vb | 1 + ...UseCollectionInitializerCodeFixProvider.vb | 1 + ...asicUseObjectInitializerCodeFixProvider.vb | 1 + 19 files changed, 85 insertions(+), 23 deletions(-) diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerAnalyzer.cs index c61ac1628ed85..8e41fb32685fe 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerAnalyzer.cs @@ -22,6 +22,7 @@ internal sealed class CSharpUseCollectionInitializerAnalyzer : AbstractUseCollec ExpressionStatementSyntax, LocalDeclarationStatementSyntax, VariableDeclaratorSyntax, + InitializerExpressionSyntax, CSharpUseCollectionInitializerAnalyzer> { protected override IUpdateExpressionSyntaxHelper SyntaxHelper @@ -45,8 +46,10 @@ protected override bool HasExistingInvalidInitializerForCollection() } protected override bool AnalyzeMatchesAndCollectionConstructorForCollectionExpression( - ArrayBuilder matches, CancellationToken cancellationToken) + ArrayBuilder matches, out InitializerExpressionSyntax? existingInitializer, CancellationToken cancellationToken) { + existingInitializer = _objectCreationExpression.Initializer; + // Constructor wasn't called with any arguments. Nothing to validate. var argumentList = _objectCreationExpression.ArgumentList; if (argumentList is null || argumentList.Arguments.Count == 0) @@ -72,8 +75,20 @@ protected override bool AnalyzeMatchesAndCollectionConstructorForCollectionExpre firstParameter.Type.AllInterfaces.Any(i => Equals(i.OriginalDefinition, ienumerableOfTType))) { // Took a single argument that implements IEnumerable. We handle this by spreading that argument as the - // first thing added to the collection. - matches.Insert(0, new(argumentList.Arguments[0].Expression, UseSpread: true)); + // first thing added to the collection. Also, because we're adding the initial arguments, we want the + // initializer values to go after that. So clear out the initializer we're returning and add the values + // directly as matches. + + var index = 0; + matches.Insert(index++, new(argumentList.Arguments[0].Expression, UseSpread: true)); + existingInitializer = null; + + if (_objectCreationExpression.Initializer != null) + { + foreach (var expression in _objectCreationExpression.Initializer.Expressions) + matches.Insert(index++, new(expression, UseSpread: false)); + } + return true; } else if (firstParameter is { Type.SpecialType: SpecialType.System_Int32, Name: "capacity" }) diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerDiagnosticAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerDiagnosticAnalyzer.cs index 69b943c5b43f5..7ab93374c412b 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerDiagnosticAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerDiagnosticAnalyzer.cs @@ -26,6 +26,7 @@ internal sealed class CSharpUseCollectionInitializerDiagnosticAnalyzer : ExpressionStatementSyntax, LocalDeclarationStatementSyntax, VariableDeclaratorSyntax, + InitializerExpressionSyntax, CSharpUseCollectionInitializerAnalyzer> { protected override ISyntaxFacts SyntaxFacts diff --git a/src/Analyzers/CSharp/Analyzers/UseObjectInitializer/CSharpUseNamedMemberInitializerAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UseObjectInitializer/CSharpUseNamedMemberInitializerAnalyzer.cs index c80e3db324bbe..5cdd1910af603 100644 --- a/src/Analyzers/CSharp/Analyzers/UseObjectInitializer/CSharpUseNamedMemberInitializerAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/UseObjectInitializer/CSharpUseNamedMemberInitializerAnalyzer.cs @@ -18,6 +18,7 @@ internal sealed class CSharpUseNamedMemberInitializerAnalyzer : ExpressionStatementSyntax, LocalDeclarationStatementSyntax, VariableDeclaratorSyntax, + InitializerExpressionSyntax, CSharpUseNamedMemberInitializerAnalyzer> { protected override bool IsInitializerOfLocalDeclarationStatement(LocalDeclarationStatementSyntax localDeclarationStatement, BaseObjectCreationExpressionSyntax rootExpression, [NotNullWhen(true)] out VariableDeclaratorSyntax? variableDeclarator) diff --git a/src/Analyzers/CSharp/Analyzers/UseObjectInitializer/CSharpUseObjectInitializerDiagnosticAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UseObjectInitializer/CSharpUseObjectInitializerDiagnosticAnalyzer.cs index 3dceca5fbbd83..ce535b7b46358 100644 --- a/src/Analyzers/CSharp/Analyzers/UseObjectInitializer/CSharpUseObjectInitializerDiagnosticAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/UseObjectInitializer/CSharpUseObjectInitializerDiagnosticAnalyzer.cs @@ -22,6 +22,7 @@ internal sealed class CSharpUseObjectInitializerDiagnosticAnalyzer : ExpressionStatementSyntax, LocalDeclarationStatementSyntax, VariableDeclaratorSyntax, + InitializerExpressionSyntax, CSharpUseNamedMemberInitializerAnalyzer> { protected override bool FadeOutOperatorToken => true; diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider.cs index 5482ee71ad778..f04b8ff048a7f 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider.cs @@ -7,7 +7,6 @@ using System.Diagnostics.CodeAnalysis; using System.Threading; using System.Threading.Tasks; -using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.CodeFixes; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.UseCollectionInitializer; @@ -28,6 +27,7 @@ internal partial class CSharpUseCollectionInitializerCodeFixProvider() : ExpressionStatementSyntax, LocalDeclarationStatementSyntax, VariableDeclaratorSyntax, + InitializerExpressionSyntax, CSharpUseCollectionInitializerAnalyzer> { protected override CSharpUseCollectionInitializerAnalyzer GetAnalyzer() diff --git a/src/Analyzers/CSharp/CodeFixes/UseObjectInitializer/CSharpUseObjectInitializerCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseObjectInitializer/CSharpUseObjectInitializerCodeFixProvider.cs index 04467a74c3815..21476d00afe49 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseObjectInitializer/CSharpUseObjectInitializerCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseObjectInitializer/CSharpUseObjectInitializerCodeFixProvider.cs @@ -29,6 +29,7 @@ internal sealed class CSharpUseObjectInitializerCodeFixProvider() : ExpressionStatementSyntax, LocalDeclarationStatementSyntax, VariableDeclaratorSyntax, + InitializerExpressionSyntax, CSharpUseNamedMemberInitializerAnalyzer> { protected override CSharpUseNamedMemberInitializerAnalyzer GetAnalyzer() diff --git a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractObjectCreationExpressionAnalyzer.cs b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractObjectCreationExpressionAnalyzer.cs index e1cb4dc271488..4117043404981 100644 --- a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractObjectCreationExpressionAnalyzer.cs +++ b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractObjectCreationExpressionAnalyzer.cs @@ -18,6 +18,7 @@ internal abstract class AbstractObjectCreationExpressionAnalyzer< TObjectCreationExpressionSyntax, TLocalDeclarationStatementSyntax, TVariableDeclaratorSyntax, + TInitializerSyntax, TMatch, TAnalyzer> : IDisposable where TExpressionSyntax : SyntaxNode @@ -25,12 +26,14 @@ internal abstract class AbstractObjectCreationExpressionAnalyzer< where TObjectCreationExpressionSyntax : TExpressionSyntax where TLocalDeclarationStatementSyntax : TStatementSyntax where TVariableDeclaratorSyntax : SyntaxNode + where TInitializerSyntax : SyntaxNode where TAnalyzer : AbstractObjectCreationExpressionAnalyzer< TExpressionSyntax, TStatementSyntax, TObjectCreationExpressionSyntax, TLocalDeclarationStatementSyntax, TVariableDeclaratorSyntax, + TInitializerSyntax, TMatch, TAnalyzer>, new() { @@ -43,7 +46,7 @@ internal abstract class AbstractObjectCreationExpressionAnalyzer< protected SemanticModel SemanticModel => this.State.SemanticModel; protected abstract bool ShouldAnalyze(CancellationToken cancellationToken); - protected abstract bool TryAddMatches(ArrayBuilder matches, CancellationToken cancellationToken); + protected abstract bool TryAddMatches(ArrayBuilder matches, out TInitializerSyntax? exitingInitializer, CancellationToken cancellationToken); protected abstract bool IsInitializerOfLocalDeclarationStatement( TLocalDeclarationStatementSyntax localDeclarationStatement, TObjectCreationExpressionSyntax rootExpression, [NotNullWhen(true)] out TVariableDeclaratorSyntax? variableDeclarator); @@ -75,16 +78,16 @@ protected void Clear() _analyzeForCollectionExpression = false; } - protected ImmutableArray AnalyzeWorker(CancellationToken cancellationToken) + protected (ImmutableArray matches, TInitializerSyntax? existingInitializer) AnalyzeWorker(CancellationToken cancellationToken) { if (!ShouldAnalyze(cancellationToken)) return default; using var _ = ArrayBuilder.GetInstance(out var matches); - if (!TryAddMatches(matches, cancellationToken)) + if (!TryAddMatches(matches, out var existingInitializer, cancellationToken)) return default; - return matches.ToImmutableAndClear(); + return (matches.ToImmutableAndClear(), existingInitializer); } protected UpdateExpressionState? TryInitializeState( diff --git a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerAnalyzer.cs b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerAnalyzer.cs index 60552247616a9..9ea6743eb85a5 100644 --- a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerAnalyzer.cs +++ b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerAnalyzer.cs @@ -21,12 +21,14 @@ internal abstract class AbstractUseCollectionInitializerAnalyzer< TExpressionStatementSyntax, TLocalDeclarationStatementSyntax, TVariableDeclaratorSyntax, + TInitializerSyntax, TAnalyzer> : AbstractObjectCreationExpressionAnalyzer< TExpressionSyntax, TStatementSyntax, TObjectCreationExpressionSyntax, TLocalDeclarationStatementSyntax, TVariableDeclaratorSyntax, + TInitializerSyntax, Match, TAnalyzer> where TExpressionSyntax : SyntaxNode where TStatementSyntax : SyntaxNode @@ -36,6 +38,7 @@ internal abstract class AbstractUseCollectionInitializerAnalyzer< where TExpressionStatementSyntax : TStatementSyntax where TLocalDeclarationStatementSyntax : TStatementSyntax where TVariableDeclaratorSyntax : SyntaxNode + where TInitializerSyntax : SyntaxNode where TAnalyzer : AbstractUseCollectionInitializerAnalyzer< TExpressionSyntax, TStatementSyntax, @@ -45,15 +48,21 @@ internal abstract class AbstractUseCollectionInitializerAnalyzer< TExpressionStatementSyntax, TLocalDeclarationStatementSyntax, TVariableDeclaratorSyntax, + TInitializerSyntax, TAnalyzer>, new() { + public readonly record struct AnalysisResult( + TInitializerSyntax? ExistingInitializer, + ImmutableArray Matches); + protected abstract bool IsComplexElementInitializer(SyntaxNode expression); protected abstract bool HasExistingInvalidInitializerForCollection(); - protected abstract bool AnalyzeMatchesAndCollectionConstructorForCollectionExpression(ArrayBuilder matches, CancellationToken cancellationToken); + protected abstract bool AnalyzeMatchesAndCollectionConstructorForCollectionExpression( + ArrayBuilder matches, out TInitializerSyntax? existingInitializer, CancellationToken cancellationToken); protected abstract IUpdateExpressionSyntaxHelper SyntaxHelper { get; } - public ImmutableArray Analyze( + public AnalysisResult Analyze( SemanticModel semanticModel, ISyntaxFacts syntaxFacts, TObjectCreationExpressionSyntax objectCreationExpression, @@ -65,10 +74,11 @@ public ImmutableArray Analyze( return default; this.Initialize(state.Value, objectCreationExpression, analyzeForCollectionExpression); - var result = this.AnalyzeWorker(cancellationToken); + var analysisResult = this.AnalyzeWorker(cancellationToken); + var matches = analysisResult.matches; // If analysis failed entirely, immediately bail out. - if (result.IsDefault) + if (matches.IsDefault) return default; // Analysis succeeded, but the result may be empty or non empty. @@ -80,17 +90,18 @@ public ImmutableArray Analyze( // other words, we don't want to suggest changing `new List()` to `new List() { }` as that's just // noise. So convert empty results to an invalid result here. if (analyzeForCollectionExpression) - return result; + return new(analysisResult.existingInitializer, matches); // Downgrade an empty result to a failure for the normal collection-initializer case. - return result.IsEmpty ? default : result; + return matches.IsEmpty ? default : new(analysisResult.existingInitializer, matches); } protected sealed override bool TryAddMatches( - ArrayBuilder matches, CancellationToken cancellationToken) + ArrayBuilder matches, out TInitializerSyntax? existingInitializer, CancellationToken cancellationToken) { var seenInvocation = false; var seenIndexAssignment = false; + existingInitializer = null; var initializer = this.SyntaxFacts.GetInitializerOfBaseObjectCreationExpression(_objectCreationExpression); if (initializer != null) @@ -126,7 +137,7 @@ protected sealed override bool TryAddMatches( } if (_analyzeForCollectionExpression) - return AnalyzeMatchesAndCollectionConstructorForCollectionExpression(matches, cancellationToken); + return AnalyzeMatchesAndCollectionConstructorForCollectionExpression(matches, out existingInitializer, cancellationToken); return true; } diff --git a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerDiagnosticAnalyzer.cs b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerDiagnosticAnalyzer.cs index 211273817c68e..2329c3c91bed5 100644 --- a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerDiagnosticAnalyzer.cs +++ b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerDiagnosticAnalyzer.cs @@ -36,6 +36,7 @@ internal abstract partial class AbstractUseCollectionInitializerDiagnosticAnalyz TExpressionStatementSyntax, TLocalDeclarationStatementSyntax, TVariableDeclaratorSyntax, + TInitializerSyntax, TAnalyzer> : AbstractBuiltInCodeStyleDiagnosticAnalyzer where TSyntaxKind : struct @@ -47,6 +48,7 @@ internal abstract partial class AbstractUseCollectionInitializerDiagnosticAnalyz where TExpressionStatementSyntax : TStatementSyntax where TLocalDeclarationStatementSyntax : TStatementSyntax where TVariableDeclaratorSyntax : SyntaxNode + where TInitializerSyntax : SyntaxNode where TAnalyzer : AbstractUseCollectionInitializerAnalyzer< TExpressionSyntax, TStatementSyntax, @@ -56,6 +58,7 @@ internal abstract partial class AbstractUseCollectionInitializerDiagnosticAnalyz TExpressionStatementSyntax, TLocalDeclarationStatementSyntax, TVariableDeclaratorSyntax, + TInitializerSyntax, TAnalyzer>, new() { @@ -211,7 +214,7 @@ private void AnalyzeNode( if (!preferInitializerOption.Value) return null; - var matches = analyzer.Analyze(semanticModel, syntaxFacts, objectCreationExpression, analyzeForCollectionExpression: false, cancellationToken); + var (_, matches) = analyzer.Analyze(semanticModel, syntaxFacts, objectCreationExpression, analyzeForCollectionExpression: false, cancellationToken); // If analysis failed, we can't change this, no matter what. if (matches.IsDefault) @@ -229,7 +232,7 @@ private void AnalyzeNode( if (!this.AreCollectionExpressionsSupported(context.Compilation)) return null; - var matches = analyzer.Analyze(semanticModel, syntaxFacts, objectCreationExpression, analyzeForCollectionExpression: true, cancellationToken); + var (_, matches) = analyzer.Analyze(semanticModel, syntaxFacts, objectCreationExpression, analyzeForCollectionExpression: true, cancellationToken); // If analysis failed, we can't change this, no matter what. if (matches.IsDefault) diff --git a/src/Analyzers/Core/Analyzers/UseObjectInitializer/AbstractUseObjectInitializerDiagnosticAnalyzer.cs b/src/Analyzers/Core/Analyzers/UseObjectInitializer/AbstractUseObjectInitializerDiagnosticAnalyzer.cs index 74d6e02d130d5..20543e0302102 100644 --- a/src/Analyzers/Core/Analyzers/UseObjectInitializer/AbstractUseObjectInitializerDiagnosticAnalyzer.cs +++ b/src/Analyzers/Core/Analyzers/UseObjectInitializer/AbstractUseObjectInitializerDiagnosticAnalyzer.cs @@ -21,6 +21,7 @@ internal abstract partial class AbstractUseObjectInitializerDiagnosticAnalyzer< TAssignmentStatementSyntax, TLocalDeclarationStatementSyntax, TVariableDeclaratorSyntax, + TINitializerSyntax, TAnalyzer> : AbstractBuiltInCodeStyleDiagnosticAnalyzer where TSyntaxKind : struct @@ -31,6 +32,7 @@ internal abstract partial class AbstractUseObjectInitializerDiagnosticAnalyzer< where TAssignmentStatementSyntax : TStatementSyntax where TLocalDeclarationStatementSyntax : TStatementSyntax where TVariableDeclaratorSyntax : SyntaxNode + where TINitializerSyntax : SyntaxNode where TAnalyzer : AbstractUseNamedMemberInitializerAnalyzer< TExpressionSyntax, TStatementSyntax, @@ -39,6 +41,7 @@ internal abstract partial class AbstractUseObjectInitializerDiagnosticAnalyzer< TAssignmentStatementSyntax, TLocalDeclarationStatementSyntax, TVariableDeclaratorSyntax, + TINitializerSyntax, TAnalyzer>, new() { private static readonly DiagnosticDescriptor s_descriptor = CreateDescriptorWithId( diff --git a/src/Analyzers/Core/Analyzers/UseObjectInitializer/UseNamedMemberInitializerAnalyzer.cs b/src/Analyzers/Core/Analyzers/UseObjectInitializer/UseNamedMemberInitializerAnalyzer.cs index b9ad90458a74b..9621d89feb943 100644 --- a/src/Analyzers/Core/Analyzers/UseObjectInitializer/UseNamedMemberInitializerAnalyzer.cs +++ b/src/Analyzers/Core/Analyzers/UseObjectInitializer/UseNamedMemberInitializerAnalyzer.cs @@ -21,12 +21,14 @@ internal abstract class AbstractUseNamedMemberInitializerAnalyzer< TAssignmentStatementSyntax, TLocalDeclarationStatementSyntax, TVariableDeclaratorSyntax, + TInitializerSyntax, TAnalyzer> : AbstractObjectCreationExpressionAnalyzer< TExpressionSyntax, TStatementSyntax, TObjectCreationExpressionSyntax, TLocalDeclarationStatementSyntax, TVariableDeclaratorSyntax, + TInitializerSyntax, Match, TAnalyzer>, IDisposable where TExpressionSyntax : SyntaxNode @@ -36,6 +38,7 @@ internal abstract class AbstractUseNamedMemberInitializerAnalyzer< where TAssignmentStatementSyntax : TStatementSyntax where TLocalDeclarationStatementSyntax : TStatementSyntax where TVariableDeclaratorSyntax : SyntaxNode + where TInitializerSyntax : SyntaxNode where TAnalyzer : AbstractUseNamedMemberInitializerAnalyzer< TExpressionSyntax, TStatementSyntax, @@ -44,6 +47,7 @@ internal abstract class AbstractUseNamedMemberInitializerAnalyzer< TAssignmentStatementSyntax, TLocalDeclarationStatementSyntax, TVariableDeclaratorSyntax, + TInitializerSyntax, TAnalyzer>, new() { public ImmutableArray> Analyze( @@ -57,7 +61,7 @@ public ImmutableArray> matches, + out TInitializerSyntax? exitingInitializer, CancellationToken cancellationToken) { + // Not used in the 'use named member initializer' case. We could update this in the future so that if a user + // already has a named initializer, and adds more initialization statements afterwards, that we can merge them + // into one final initializer. + exitingInitializer = null; + using var _1 = PooledHashSet.GetInstance(out var seenNames); var initializer = this.SyntaxFacts.GetInitializerOfBaseObjectCreationExpression(_objectCreationExpression); diff --git a/src/Analyzers/Core/CodeFixes/UseCollectionInitializer/AbstractUseCollectionInitializerCodeFixProvider.cs b/src/Analyzers/Core/CodeFixes/UseCollectionInitializer/AbstractUseCollectionInitializerCodeFixProvider.cs index 4ab7ee7f7f9cf..f6f64a1a71f63 100644 --- a/src/Analyzers/Core/CodeFixes/UseCollectionInitializer/AbstractUseCollectionInitializerCodeFixProvider.cs +++ b/src/Analyzers/Core/CodeFixes/UseCollectionInitializer/AbstractUseCollectionInitializerCodeFixProvider.cs @@ -5,7 +5,6 @@ using System.Collections.Immutable; using System.Threading; using System.Threading.Tasks; -using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Editing; using Microsoft.CodeAnalysis.LanguageService; @@ -24,6 +23,7 @@ internal abstract class AbstractUseCollectionInitializerCodeFixProvider< TExpressionStatementSyntax, TLocalDeclarationStatementSyntax, TVariableDeclaratorSyntax, + TInitializerSyntax, TAnalyzer>() : AbstractUseCollectionExpressionCodeFixProvider( AnalyzersResources.Collection_initialization_can_be_simplified, @@ -37,6 +37,7 @@ internal abstract class AbstractUseCollectionInitializerCodeFixProvider< where TExpressionStatementSyntax : TStatementSyntax where TLocalDeclarationStatementSyntax : TStatementSyntax where TVariableDeclaratorSyntax : SyntaxNode + where TInitializerSyntax : SyntaxNode where TAnalyzer : AbstractUseCollectionInitializerAnalyzer< TExpressionSyntax, TStatementSyntax, @@ -46,6 +47,7 @@ internal abstract class AbstractUseCollectionInitializerCodeFixProvider< TExpressionStatementSyntax, TLocalDeclarationStatementSyntax, TVariableDeclaratorSyntax, + TInitializerSyntax, TAnalyzer>, new() { public sealed override ImmutableArray FixableDiagnosticIds @@ -75,7 +77,7 @@ protected sealed override async Task FixAsync( using var analyzer = GetAnalyzer(); var useCollectionExpression = properties.ContainsKey(UseCollectionInitializerHelpers.UseCollectionExpressionName) is true; - var matches = analyzer.Analyze( + var (initializer, matches) = analyzer.Analyze( semanticModel, syntaxFacts, objectCreation, useCollectionExpression, cancellationToken); if (matches.IsDefault) diff --git a/src/Analyzers/Core/CodeFixes/UseObjectInitializer/AbstractUseObjectInitializerCodeFixProvider.cs b/src/Analyzers/Core/CodeFixes/UseObjectInitializer/AbstractUseObjectInitializerCodeFixProvider.cs index e24f92920c2fb..5a28d88096503 100644 --- a/src/Analyzers/Core/CodeFixes/UseObjectInitializer/AbstractUseObjectInitializerCodeFixProvider.cs +++ b/src/Analyzers/Core/CodeFixes/UseObjectInitializer/AbstractUseObjectInitializerCodeFixProvider.cs @@ -5,7 +5,6 @@ using System.Collections.Immutable; using System.Threading; using System.Threading.Tasks; -using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.CodeFixes; using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Editing; @@ -25,6 +24,7 @@ internal abstract class AbstractUseObjectInitializerCodeFixProvider< TAssignmentStatementSyntax, TLocalDeclarationStatementSyntax, TVariableDeclaratorSyntax, + TInitializerSyntax, TAnalyzer> : ForkingSyntaxEditorBasedCodeFixProvider where TSyntaxKind : struct @@ -35,6 +35,7 @@ internal abstract class AbstractUseObjectInitializerCodeFixProvider< where TAssignmentStatementSyntax : TStatementSyntax where TLocalDeclarationStatementSyntax : TStatementSyntax where TVariableDeclaratorSyntax : SyntaxNode + where TInitializerSyntax : SyntaxNode where TAnalyzer : AbstractUseNamedMemberInitializerAnalyzer< TExpressionSyntax, TStatementSyntax, @@ -43,6 +44,7 @@ internal abstract class AbstractUseObjectInitializerCodeFixProvider< TAssignmentStatementSyntax, TLocalDeclarationStatementSyntax, TVariableDeclaratorSyntax, + TInitializerSyntax, TAnalyzer>, new() { protected override (string title, string equivalenceKey) GetTitleAndEquivalenceKey(CodeFixContext context) diff --git a/src/Analyzers/VisualBasic/Analyzers/UseCollectionInitializer/VisualBasicCollectionInitializerAnalyzer.vb b/src/Analyzers/VisualBasic/Analyzers/UseCollectionInitializer/VisualBasicCollectionInitializerAnalyzer.vb index 554cd29ec2269..8c67e15a6fae6 100644 --- a/src/Analyzers/VisualBasic/Analyzers/UseCollectionInitializer/VisualBasicCollectionInitializerAnalyzer.vb +++ b/src/Analyzers/VisualBasic/Analyzers/UseCollectionInitializer/VisualBasicCollectionInitializerAnalyzer.vb @@ -18,6 +18,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.UseCollectionInitializer ExpressionStatementSyntax, LocalDeclarationStatementSyntax, VariableDeclaratorSyntax, + ObjectCollectionInitializerSyntax, VisualBasicCollectionInitializerAnalyzer) Protected Overrides ReadOnly Property SyntaxHelper As IUpdateExpressionSyntaxHelper(Of ExpressionSyntax, StatementSyntax) = @@ -37,7 +38,10 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.UseCollectionInitializer Return TypeOf _objectCreationExpression.Initializer Is ObjectMemberInitializerSyntax End Function - Protected Overrides Function AnalyzeMatchesAndCollectionConstructorForCollectionExpression(matches As ArrayBuilder(Of Match), cancellationToken As CancellationToken) As Boolean + Protected Overrides Function AnalyzeMatchesAndCollectionConstructorForCollectionExpression( + matches As ArrayBuilder(Of Match), + ByRef existingInitializer As ObjectCollectionInitializerSyntax, + cancellationToken As CancellationToken) As Boolean ' Only called for collection expressions, which VB does not support Throw ExceptionUtilities.Unreachable() End Function diff --git a/src/Analyzers/VisualBasic/Analyzers/UseCollectionInitializer/VisualBasicUseCollectionInitializerDiagnosticAnalyzer.vb b/src/Analyzers/VisualBasic/Analyzers/UseCollectionInitializer/VisualBasicUseCollectionInitializerDiagnosticAnalyzer.vb index b15c4bf2bd948..20bb7e448bd97 100644 --- a/src/Analyzers/VisualBasic/Analyzers/UseCollectionInitializer/VisualBasicUseCollectionInitializerDiagnosticAnalyzer.vb +++ b/src/Analyzers/VisualBasic/Analyzers/UseCollectionInitializer/VisualBasicUseCollectionInitializerDiagnosticAnalyzer.vb @@ -22,6 +22,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.UseCollectionInitializer ExpressionStatementSyntax, LocalDeclarationStatementSyntax, VariableDeclaratorSyntax, + ObjectCollectionInitializerSyntax, VisualBasicCollectionInitializerAnalyzer) Protected Overrides ReadOnly Property SyntaxFacts As ISyntaxFacts = VisualBasicSyntaxFacts.Instance diff --git a/src/Analyzers/VisualBasic/Analyzers/UseObjectInitializer/VisualBasicUseNamedMemberInitializerAnalyzer.vb b/src/Analyzers/VisualBasic/Analyzers/UseObjectInitializer/VisualBasicUseNamedMemberInitializerAnalyzer.vb index 466210f30027e..d30a4775bf6b6 100644 --- a/src/Analyzers/VisualBasic/Analyzers/UseObjectInitializer/VisualBasicUseNamedMemberInitializerAnalyzer.vb +++ b/src/Analyzers/VisualBasic/Analyzers/UseObjectInitializer/VisualBasicUseNamedMemberInitializerAnalyzer.vb @@ -16,6 +16,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.UseObjectInitializer AssignmentStatementSyntax, LocalDeclarationStatementSyntax, VariableDeclaratorSyntax, + ObjectMemberInitializerSyntax, VisualBasicUseNamedMemberInitializerAnalyzer) Protected Overrides Function IsInitializerOfLocalDeclarationStatement(localDeclarationStatement As LocalDeclarationStatementSyntax, rootExpression As ObjectCreationExpressionSyntax, ByRef variableDeclarator As VariableDeclaratorSyntax) As Boolean diff --git a/src/Analyzers/VisualBasic/Analyzers/UseObjectInitializer/VisualBasicUseObjectInitializerDiagnosticAnalyzer.vb b/src/Analyzers/VisualBasic/Analyzers/UseObjectInitializer/VisualBasicUseObjectInitializerDiagnosticAnalyzer.vb index 2e981705283c0..8e255e92e5df2 100644 --- a/src/Analyzers/VisualBasic/Analyzers/UseObjectInitializer/VisualBasicUseObjectInitializerDiagnosticAnalyzer.vb +++ b/src/Analyzers/VisualBasic/Analyzers/UseObjectInitializer/VisualBasicUseObjectInitializerDiagnosticAnalyzer.vb @@ -20,6 +20,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.UseObjectInitializer AssignmentStatementSyntax, LocalDeclarationStatementSyntax, VariableDeclaratorSyntax, + ObjectMemberInitializerSyntax, VisualBasicUseNamedMemberInitializerAnalyzer) Protected Overrides ReadOnly Property FadeOutOperatorToken As Boolean = False diff --git a/src/Analyzers/VisualBasic/CodeFixes/UseCollectionInitializer/VisualBasicUseCollectionInitializerCodeFixProvider.vb b/src/Analyzers/VisualBasic/CodeFixes/UseCollectionInitializer/VisualBasicUseCollectionInitializerCodeFixProvider.vb index b7e02b0fc6469..3a3b3609379a4 100644 --- a/src/Analyzers/VisualBasic/CodeFixes/UseCollectionInitializer/VisualBasicUseCollectionInitializerCodeFixProvider.vb +++ b/src/Analyzers/VisualBasic/CodeFixes/UseCollectionInitializer/VisualBasicUseCollectionInitializerCodeFixProvider.vb @@ -26,6 +26,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.UseCollectionInitializer ExpressionStatementSyntax, LocalDeclarationStatementSyntax, VariableDeclaratorSyntax, + ObjectCollectionInitializerSyntax, VisualBasicCollectionInitializerAnalyzer) diff --git a/src/Analyzers/VisualBasic/CodeFixes/UseObjectInitializer/VisualBasicUseObjectInitializerCodeFixProvider.vb b/src/Analyzers/VisualBasic/CodeFixes/UseObjectInitializer/VisualBasicUseObjectInitializerCodeFixProvider.vb index bf7bfaf830d72..cc39c986077d3 100644 --- a/src/Analyzers/VisualBasic/CodeFixes/UseObjectInitializer/VisualBasicUseObjectInitializerCodeFixProvider.vb +++ b/src/Analyzers/VisualBasic/CodeFixes/UseObjectInitializer/VisualBasicUseObjectInitializerCodeFixProvider.vb @@ -22,6 +22,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.UseObjectInitializer AssignmentStatementSyntax, LocalDeclarationStatementSyntax, VariableDeclaratorSyntax, + ObjectMemberInitializerSyntax, VisualBasicUseNamedMemberInitializerAnalyzer) From 5d9233e7689f03046517cd9d33a5a4c04e505187 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 27 Sep 2024 11:44:22 -0700 Subject: [PATCH 10/25] in progress --- ...UseCollectionInitializerCodeFixProvider.cs | 19 +++++-------------- ...zerCodeFixProvider_CollectionExpression.cs | 6 +++--- ...UseCollectionInitializerCodeFixProvider.cs | 4 ++-- ...UseCollectionInitializerCodeFixProvider.vb | 3 +-- 4 files changed, 11 insertions(+), 21 deletions(-) diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider.cs index f04b8ff048a7f..7b8f3df78c151 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider.cs @@ -37,23 +37,14 @@ protected override CSharpUseCollectionInitializerAnalyzer GetAnalyzer() Document document, BaseObjectCreationExpressionSyntax objectCreation, bool useCollectionExpression, + InitializerExpressionSyntax? existingInitializer, ImmutableArray matches, CancellationToken cancellationToken) { - var newObjectCreation = await GetNewObjectCreationAsync( - document, objectCreation, useCollectionExpression, matches, cancellationToken).ConfigureAwait(false); - return (objectCreation, newObjectCreation); - } - - private static async Task GetNewObjectCreationAsync( - Document document, - BaseObjectCreationExpressionSyntax objectCreation, - bool useCollectionExpression, - ImmutableArray matches, - CancellationToken cancellationToken) - { - return useCollectionExpression - ? await CreateCollectionExpressionAsync(document, objectCreation, matches, cancellationToken).ConfigureAwait(false) + ExpressionSyntax newObjectCreation = useCollectionExpression + ? await CreateCollectionExpressionAsync(document, objectCreation, existingInitializer, matches, cancellationToken).ConfigureAwait(false) : CreateObjectInitializerExpression(objectCreation, matches); + + return (objectCreation, newObjectCreation); } } diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionExpression.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionExpression.cs index c413564b3cf7a..24fa990ca7540 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionExpression.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionExpression.cs @@ -7,8 +7,6 @@ using System.Threading.Tasks; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.CSharp.UseCollectionExpression; -using Microsoft.CodeAnalysis.Shared.Collections; -using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.UseCollectionInitializer; namespace Microsoft.CodeAnalysis.CSharp.UseCollectionInitializer; @@ -22,6 +20,7 @@ internal partial class CSharpUseCollectionInitializerCodeFixProvider private static Task CreateCollectionExpressionAsync( Document document, BaseObjectCreationExpressionSyntax objectCreation, + InitializerExpressionSyntax? existingInitializer, ImmutableArray matches, CancellationToken cancellationToken) { @@ -29,7 +28,8 @@ private static Task CreateCollectionExpressionAsync( document, objectCreation, matches.SelectAsArray(m => new CollectionExpressionMatch(m.StatementOrExpression, m.UseSpread)), - static objectCreation => objectCreation.Initializer, + // Use the initializer the analyzer recommends, regardless of what's on the object creation node. + _ => existingInitializer, static (objectCreation, initializer) => objectCreation.WithInitializer(initializer), cancellationToken); } diff --git a/src/Analyzers/Core/CodeFixes/UseCollectionInitializer/AbstractUseCollectionInitializerCodeFixProvider.cs b/src/Analyzers/Core/CodeFixes/UseCollectionInitializer/AbstractUseCollectionInitializerCodeFixProvider.cs index f6f64a1a71f63..c79b3a1482eec 100644 --- a/src/Analyzers/Core/CodeFixes/UseCollectionInitializer/AbstractUseCollectionInitializerCodeFixProvider.cs +++ b/src/Analyzers/Core/CodeFixes/UseCollectionInitializer/AbstractUseCollectionInitializerCodeFixProvider.cs @@ -56,7 +56,7 @@ public sealed override ImmutableArray FixableDiagnosticIds protected abstract TAnalyzer GetAnalyzer(); protected abstract Task<(SyntaxNode oldNode, SyntaxNode newNode)> GetReplacementNodesAsync( - Document document, TObjectCreationExpressionSyntax objectCreation, bool useCollectionExpression, ImmutableArray matches, CancellationToken cancellationToken); + Document document, TObjectCreationExpressionSyntax objectCreation, bool useCollectionExpression, TInitializerSyntax? existingInitializer, ImmutableArray matches, CancellationToken cancellationToken); protected sealed override async Task FixAsync( Document document, @@ -84,7 +84,7 @@ protected sealed override async Task FixAsync( return; var (oldNode, newNode) = await GetReplacementNodesAsync( - document, objectCreation, useCollectionExpression, matches, cancellationToken).ConfigureAwait(false); + document, objectCreation, useCollectionExpression, initializer, matches, cancellationToken).ConfigureAwait(false); editor.ReplaceNode(oldNode, newNode); foreach (var match in matches) diff --git a/src/Analyzers/VisualBasic/CodeFixes/UseCollectionInitializer/VisualBasicUseCollectionInitializerCodeFixProvider.vb b/src/Analyzers/VisualBasic/CodeFixes/UseCollectionInitializer/VisualBasicUseCollectionInitializerCodeFixProvider.vb index 3a3b3609379a4..739c905b3f5ef 100644 --- a/src/Analyzers/VisualBasic/CodeFixes/UseCollectionInitializer/VisualBasicUseCollectionInitializerCodeFixProvider.vb +++ b/src/Analyzers/VisualBasic/CodeFixes/UseCollectionInitializer/VisualBasicUseCollectionInitializerCodeFixProvider.vb @@ -8,8 +8,6 @@ Imports System.Diagnostics.CodeAnalysis Imports System.Threading Imports Microsoft.CodeAnalysis.CodeFixes Imports Microsoft.CodeAnalysis.Formatting -Imports Microsoft.CodeAnalysis.PooledObjects -Imports Microsoft.CodeAnalysis.UseCollectionInitializer Imports Microsoft.CodeAnalysis.VisualBasic.Syntax Imports Microsoft.CodeAnalysis.VisualBasic.UseObjectInitializer @@ -42,6 +40,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.UseCollectionInitializer document As Document, objectCreation As ObjectCreationExpressionSyntax, useCollectionExpression As Boolean, + existingInitializer As ObjectCollectionInitializerSyntax, matches As ImmutableArray(Of Match), cancellationToken As CancellationToken) As Task(Of (SyntaxNode, SyntaxNode)) Contract.ThrowIfTrue(useCollectionExpression, "VB does not support collection expressions") From d0b2618258fbb3e5354f972e3f23dafac188adf8 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 27 Sep 2024 11:56:21 -0700 Subject: [PATCH 11/25] in progress --- .../CSharpUseCollectionInitializerAnalyzer.cs | 25 +++++------------- ...CollectionInitializerDiagnosticAnalyzer.cs | 1 - ...CSharpUseNamedMemberInitializerAnalyzer.cs | 1 - ...pUseObjectInitializerDiagnosticAnalyzer.cs | 1 - .../CSharpCollectionExpressionRewriter.cs | 15 ++++++----- ...ectionExpressionForArrayCodeFixProvider.cs | 1 + ...tionExpressionForBuilderCodeFixProvider.cs | 1 + ...ctionExpressionForCreateCodeFixProvider.cs | 1 + ...ctionExpressionForFluentCodeFixProvider.cs | 1 + ...nExpressionForStackAllocCodeFixProvider.cs | 1 + ...UseCollectionInitializerCodeFixProvider.cs | 9 +++---- ...zerCodeFixProvider_CollectionExpression.cs | 9 ++++--- ...harpUseObjectInitializerCodeFixProvider.cs | 1 - ...bstractObjectCreationExpressionAnalyzer.cs | 14 +++++----- ...bstractUseCollectionInitializerAnalyzer.cs | 26 +++++++------------ ...CollectionInitializerDiagnosticAnalyzer.cs | 3 --- ...tUseObjectInitializerDiagnosticAnalyzer.cs | 3 --- .../UseNamedMemberInitializerAnalyzer.cs | 17 +++--------- ...UseCollectionInitializerCodeFixProvider.cs | 6 ++--- ...ractUseObjectInitializerCodeFixProvider.cs | 3 --- ...isualBasicCollectionInitializerAnalyzer.vb | 5 ++-- ...CollectionInitializerDiagnosticAnalyzer.vb | 1 - ...lBasicUseNamedMemberInitializerAnalyzer.vb | 1 - ...cUseObjectInitializerDiagnosticAnalyzer.vb | 1 - ...UseCollectionInitializerCodeFixProvider.vb | 12 +++++---- ...asicUseObjectInitializerCodeFixProvider.vb | 1 - 26 files changed, 59 insertions(+), 101 deletions(-) diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerAnalyzer.cs index 8e41fb32685fe..b3deba756f901 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerAnalyzer.cs @@ -22,7 +22,6 @@ internal sealed class CSharpUseCollectionInitializerAnalyzer : AbstractUseCollec ExpressionStatementSyntax, LocalDeclarationStatementSyntax, VariableDeclaratorSyntax, - InitializerExpressionSyntax, CSharpUseCollectionInitializerAnalyzer> { protected override IUpdateExpressionSyntaxHelper SyntaxHelper @@ -46,10 +45,10 @@ protected override bool HasExistingInvalidInitializerForCollection() } protected override bool AnalyzeMatchesAndCollectionConstructorForCollectionExpression( - ArrayBuilder matches, out InitializerExpressionSyntax? existingInitializer, CancellationToken cancellationToken) + ArrayBuilder preMatches, + ArrayBuilder postMatches, + CancellationToken cancellationToken) { - existingInitializer = _objectCreationExpression.Initializer; - // Constructor wasn't called with any arguments. Nothing to validate. var argumentList = _objectCreationExpression.ArgumentList; if (argumentList is null || argumentList.Arguments.Count == 0) @@ -75,20 +74,8 @@ protected override bool AnalyzeMatchesAndCollectionConstructorForCollectionExpre firstParameter.Type.AllInterfaces.Any(i => Equals(i.OriginalDefinition, ienumerableOfTType))) { // Took a single argument that implements IEnumerable. We handle this by spreading that argument as the - // first thing added to the collection. Also, because we're adding the initial arguments, we want the - // initializer values to go after that. So clear out the initializer we're returning and add the values - // directly as matches. - - var index = 0; - matches.Insert(index++, new(argumentList.Arguments[0].Expression, UseSpread: true)); - existingInitializer = null; - - if (_objectCreationExpression.Initializer != null) - { - foreach (var expression in _objectCreationExpression.Initializer.Expressions) - matches.Insert(index++, new(expression, UseSpread: false)); - } - + // first thing added to the collection. + preMatches.Add(new(argumentList.Arguments[0].Expression, UseSpread: true)); return true; } else if (firstParameter is { Type.SpecialType: SpecialType.System_Int32, Name: "capacity" }) @@ -103,7 +90,7 @@ protected override bool AnalyzeMatchesAndCollectionConstructorForCollectionExpre // be spread into the final collection. We'll then ensure a correspondance between both and the expression the // user is currently passing to the 'capacity' argument to make sure they're entirely congruent. using var _1 = ArrayBuilder.GetInstance(out var spreadElements); - foreach (var match in matches) + foreach (var match in postMatches) { switch (match.StatementOrExpression) { diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerDiagnosticAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerDiagnosticAnalyzer.cs index 7ab93374c412b..69b943c5b43f5 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerDiagnosticAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerDiagnosticAnalyzer.cs @@ -26,7 +26,6 @@ internal sealed class CSharpUseCollectionInitializerDiagnosticAnalyzer : ExpressionStatementSyntax, LocalDeclarationStatementSyntax, VariableDeclaratorSyntax, - InitializerExpressionSyntax, CSharpUseCollectionInitializerAnalyzer> { protected override ISyntaxFacts SyntaxFacts diff --git a/src/Analyzers/CSharp/Analyzers/UseObjectInitializer/CSharpUseNamedMemberInitializerAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UseObjectInitializer/CSharpUseNamedMemberInitializerAnalyzer.cs index 5cdd1910af603..c80e3db324bbe 100644 --- a/src/Analyzers/CSharp/Analyzers/UseObjectInitializer/CSharpUseNamedMemberInitializerAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/UseObjectInitializer/CSharpUseNamedMemberInitializerAnalyzer.cs @@ -18,7 +18,6 @@ internal sealed class CSharpUseNamedMemberInitializerAnalyzer : ExpressionStatementSyntax, LocalDeclarationStatementSyntax, VariableDeclaratorSyntax, - InitializerExpressionSyntax, CSharpUseNamedMemberInitializerAnalyzer> { protected override bool IsInitializerOfLocalDeclarationStatement(LocalDeclarationStatementSyntax localDeclarationStatement, BaseObjectCreationExpressionSyntax rootExpression, [NotNullWhen(true)] out VariableDeclaratorSyntax? variableDeclarator) diff --git a/src/Analyzers/CSharp/Analyzers/UseObjectInitializer/CSharpUseObjectInitializerDiagnosticAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UseObjectInitializer/CSharpUseObjectInitializerDiagnosticAnalyzer.cs index ce535b7b46358..3dceca5fbbd83 100644 --- a/src/Analyzers/CSharp/Analyzers/UseObjectInitializer/CSharpUseObjectInitializerDiagnosticAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/UseObjectInitializer/CSharpUseObjectInitializerDiagnosticAnalyzer.cs @@ -22,7 +22,6 @@ internal sealed class CSharpUseObjectInitializerDiagnosticAnalyzer : ExpressionStatementSyntax, LocalDeclarationStatementSyntax, VariableDeclaratorSyntax, - InitializerExpressionSyntax, CSharpUseNamedMemberInitializerAnalyzer> { protected override bool FadeOutOperatorToken => true; diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpCollectionExpressionRewriter.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpCollectionExpressionRewriter.cs index c5f2083c7c2a7..04bdcf84c6ed3 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpCollectionExpressionRewriter.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpCollectionExpressionRewriter.cs @@ -33,7 +33,8 @@ internal static class CSharpCollectionExpressionRewriter public static async Task CreateCollectionExpressionAsync( Document workspaceDocument, TParentExpression expressionToReplace, - ImmutableArray> matches, + ImmutableArray> preMatches, + ImmutableArray> postMatches, Func getInitializer, Func withInitializer, CancellationToken cancellationToken) @@ -80,7 +81,7 @@ CollectionExpressionSyntax CreateCollectionExpressionWithoutExistingElements() // Didn't have an existing initializer (or it was empty). For both cases, just create an entirely // fresh collection expression, and replace the object entirely. - if (matches is [{ Node: ExpressionSyntax expression } match]) + if (postMatches is [{ Node: ExpressionSyntax expression } match]) { // Specialize when we're taking some expression (like x.y.ToArray()) and converting to a spreaded // collection expression. We just want to trivially make that `[.. x.y]` without any specialized @@ -128,7 +129,7 @@ CollectionExpressionSyntax CreateCollectionExpressionWithoutExistingElements() // now create the elements, following that indentation preference. using var _ = ArrayBuilder.GetInstance(out var nodesAndTokens); - CreateAndAddElements(matches, nodesAndTokens, preferredIndentation: elementIndentation, forceTrailingComma: true); + CreateAndAddElements(postMatches, nodesAndTokens, preferredIndentation: elementIndentation, forceTrailingComma: true); // Add a newline between the last element and the close bracket if we don't already have one. if (nodesAndTokens.Count > 0 && nodesAndTokens.Last().GetTrailingTrivia() is [.., (kind: not SyntaxKind.EndOfLineTrivia)]) @@ -150,7 +151,7 @@ CollectionExpressionSyntax CreateCollectionExpressionWithoutExistingElements() // fresh collection expression, and do a wholesale replacement of the original object creation // expression with it. using var _ = ArrayBuilder.GetInstance(out var nodesAndTokens); - CreateAndAddElements(matches, nodesAndTokens, preferredIndentation: null, forceTrailingComma: false); + CreateAndAddElements(postMatches, nodesAndTokens, preferredIndentation: null, forceTrailingComma: false); // Remove any trailing whitespace from the last element/comma and the final close bracket. if (nodesAndTokens.Count > 0) @@ -401,7 +402,7 @@ CollectionExpressionSyntax AddMatchesToExistingNonEmptyCollectionExpression( // end. This keeps every element consistent with ending the line with a comma, which makes code easier to // maintain. CreateAndAddElements( - matches, nodesAndTokens, preferredIndentation, + postMatches, nodesAndTokens, preferredIndentation, forceTrailingComma: preferredIndentation != null && trailingComma == default); if (trailingComma != default) @@ -701,7 +702,7 @@ bool MakeMultiLineCollectionExpression() { // If there's already an initializer, and we're not adding anything to it, then just keep the initializer // as-is. No need to convert it to be multi-line if it's currently single-line. - if (initializer != null && matches.Length == 0) + if (initializer != null && postMatches.Length == 0) return false; var totalLength = 0; @@ -711,7 +712,7 @@ bool MakeMultiLineCollectionExpression() totalLength += expression.Span.Length; } - foreach (var (node, _) in matches) + foreach (var (node, _) in postMatches) { // if the statement we're replacing has any comments on it, then we need to be multiline to give them an // appropriate place to go. diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForArrayCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForArrayCodeFixProvider.cs index 3ca81464e419a..937e95b97a57b 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForArrayCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForArrayCodeFixProvider.cs @@ -72,6 +72,7 @@ and not ImplicitArrayCreationExpressionSyntax var collectionExpression = await CSharpCollectionExpressionRewriter.CreateCollectionExpressionAsync( document, arrayCreationExpression, + preMatches: [], matches, static e => e switch { diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForBuilderCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForBuilderCodeFixProvider.cs index 37cfb81ca743f..04941a6937e4c 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForBuilderCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForBuilderCodeFixProvider.cs @@ -65,6 +65,7 @@ protected override async Task FixAsync( var collectionExpression = await CreateCollectionExpressionAsync( newDocument, dummyObjectCreation, + preMatches: [], analysisResult.Matches.SelectAsArray(m => new CollectionExpressionMatch(m.StatementOrExpression, m.UseSpread)), static o => o.Initializer, static (o, i) => o.WithInitializer(i), diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForCreateCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForCreateCodeFixProvider.cs index 0466efe136cb1..d6004c154d219 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForCreateCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForCreateCodeFixProvider.cs @@ -67,6 +67,7 @@ protected override async Task FixAsync( var collectionExpression = await CreateCollectionExpressionAsync( newSemanticDocument.Document, dummyObjectCreation, + preMatches: [], matches, static o => o.Initializer, static (o, i) => o.WithInitializer(i), diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForFluentCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForFluentCodeFixProvider.cs index 09314809aa588..8d6639c8ddc19 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForFluentCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForFluentCodeFixProvider.cs @@ -91,6 +91,7 @@ protected override async Task FixAsync( var collectionExpression = await CreateCollectionExpressionAsync( newSemanticDocument.Document, dummyObjectCreation, + preMatches: [], matches, static o => o.Initializer, static (o, i) => o.WithInitializer(i), diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocCodeFixProvider.cs index da987b48b87d4..0649e538a58c8 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocCodeFixProvider.cs @@ -47,6 +47,7 @@ protected sealed override async Task FixAsync( var collectionExpression = await CSharpCollectionExpressionRewriter.CreateCollectionExpressionAsync( document, stackAllocExpression, + preMatches: [], matches, static e => e switch { diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider.cs index 7b8f3df78c151..87d8d8164358c 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider.cs @@ -27,7 +27,6 @@ internal partial class CSharpUseCollectionInitializerCodeFixProvider() : ExpressionStatementSyntax, LocalDeclarationStatementSyntax, VariableDeclaratorSyntax, - InitializerExpressionSyntax, CSharpUseCollectionInitializerAnalyzer> { protected override CSharpUseCollectionInitializerAnalyzer GetAnalyzer() @@ -37,13 +36,13 @@ protected override CSharpUseCollectionInitializerAnalyzer GetAnalyzer() Document document, BaseObjectCreationExpressionSyntax objectCreation, bool useCollectionExpression, - InitializerExpressionSyntax? existingInitializer, - ImmutableArray matches, + ImmutableArray preMatches, + ImmutableArray postMatches, CancellationToken cancellationToken) { ExpressionSyntax newObjectCreation = useCollectionExpression - ? await CreateCollectionExpressionAsync(document, objectCreation, existingInitializer, matches, cancellationToken).ConfigureAwait(false) - : CreateObjectInitializerExpression(objectCreation, matches); + ? await CreateCollectionExpressionAsync(document, objectCreation, preMatches, postMatches, cancellationToken).ConfigureAwait(false) + : CreateObjectInitializerExpression(objectCreation, postMatches); return (objectCreation, newObjectCreation); } diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionExpression.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionExpression.cs index 24fa990ca7540..47c6aa63276f6 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionExpression.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionExpression.cs @@ -20,16 +20,17 @@ internal partial class CSharpUseCollectionInitializerCodeFixProvider private static Task CreateCollectionExpressionAsync( Document document, BaseObjectCreationExpressionSyntax objectCreation, - InitializerExpressionSyntax? existingInitializer, - ImmutableArray matches, + ImmutableArray preMatches, + ImmutableArray postMatches, CancellationToken cancellationToken) { return CSharpCollectionExpressionRewriter.CreateCollectionExpressionAsync( document, objectCreation, - matches.SelectAsArray(m => new CollectionExpressionMatch(m.StatementOrExpression, m.UseSpread)), + preMatches.SelectAsArray(m => new CollectionExpressionMatch(m.StatementOrExpression, m.UseSpread)), + postMatches.SelectAsArray(m => new CollectionExpressionMatch(m.StatementOrExpression, m.UseSpread)), // Use the initializer the analyzer recommends, regardless of what's on the object creation node. - _ => existingInitializer, + static objectCreation => objectCreation.Initializer, static (objectCreation, initializer) => objectCreation.WithInitializer(initializer), cancellationToken); } diff --git a/src/Analyzers/CSharp/CodeFixes/UseObjectInitializer/CSharpUseObjectInitializerCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseObjectInitializer/CSharpUseObjectInitializerCodeFixProvider.cs index 21476d00afe49..04467a74c3815 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseObjectInitializer/CSharpUseObjectInitializerCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseObjectInitializer/CSharpUseObjectInitializerCodeFixProvider.cs @@ -29,7 +29,6 @@ internal sealed class CSharpUseObjectInitializerCodeFixProvider() : ExpressionStatementSyntax, LocalDeclarationStatementSyntax, VariableDeclaratorSyntax, - InitializerExpressionSyntax, CSharpUseNamedMemberInitializerAnalyzer> { protected override CSharpUseNamedMemberInitializerAnalyzer GetAnalyzer() diff --git a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractObjectCreationExpressionAnalyzer.cs b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractObjectCreationExpressionAnalyzer.cs index 4117043404981..33542348cc649 100644 --- a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractObjectCreationExpressionAnalyzer.cs +++ b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractObjectCreationExpressionAnalyzer.cs @@ -18,7 +18,6 @@ internal abstract class AbstractObjectCreationExpressionAnalyzer< TObjectCreationExpressionSyntax, TLocalDeclarationStatementSyntax, TVariableDeclaratorSyntax, - TInitializerSyntax, TMatch, TAnalyzer> : IDisposable where TExpressionSyntax : SyntaxNode @@ -26,14 +25,12 @@ internal abstract class AbstractObjectCreationExpressionAnalyzer< where TObjectCreationExpressionSyntax : TExpressionSyntax where TLocalDeclarationStatementSyntax : TStatementSyntax where TVariableDeclaratorSyntax : SyntaxNode - where TInitializerSyntax : SyntaxNode where TAnalyzer : AbstractObjectCreationExpressionAnalyzer< TExpressionSyntax, TStatementSyntax, TObjectCreationExpressionSyntax, TLocalDeclarationStatementSyntax, TVariableDeclaratorSyntax, - TInitializerSyntax, TMatch, TAnalyzer>, new() { @@ -46,7 +43,7 @@ internal abstract class AbstractObjectCreationExpressionAnalyzer< protected SemanticModel SemanticModel => this.State.SemanticModel; protected abstract bool ShouldAnalyze(CancellationToken cancellationToken); - protected abstract bool TryAddMatches(ArrayBuilder matches, out TInitializerSyntax? exitingInitializer, CancellationToken cancellationToken); + protected abstract bool TryAddMatches(ArrayBuilder preMatches, ArrayBuilder postMatches, CancellationToken cancellationToken); protected abstract bool IsInitializerOfLocalDeclarationStatement( TLocalDeclarationStatementSyntax localDeclarationStatement, TObjectCreationExpressionSyntax rootExpression, [NotNullWhen(true)] out TVariableDeclaratorSyntax? variableDeclarator); @@ -78,16 +75,17 @@ protected void Clear() _analyzeForCollectionExpression = false; } - protected (ImmutableArray matches, TInitializerSyntax? existingInitializer) AnalyzeWorker(CancellationToken cancellationToken) + protected (ImmutableArray preMatches, ImmutableArray postMatches) AnalyzeWorker(CancellationToken cancellationToken) { if (!ShouldAnalyze(cancellationToken)) return default; - using var _ = ArrayBuilder.GetInstance(out var matches); - if (!TryAddMatches(matches, out var existingInitializer, cancellationToken)) + using var _1 = ArrayBuilder.GetInstance(out var preMatches); + using var _2 = ArrayBuilder.GetInstance(out var postMatches); + if (!TryAddMatches(preMatches, postMatches, cancellationToken)) return default; - return (matches.ToImmutableAndClear(), existingInitializer); + return (preMatches.ToImmutableAndClear(), postMatches.ToImmutableAndClear()); } protected UpdateExpressionState? TryInitializeState( diff --git a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerAnalyzer.cs b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerAnalyzer.cs index 9ea6743eb85a5..28be1664b0303 100644 --- a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerAnalyzer.cs +++ b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerAnalyzer.cs @@ -21,14 +21,12 @@ internal abstract class AbstractUseCollectionInitializerAnalyzer< TExpressionStatementSyntax, TLocalDeclarationStatementSyntax, TVariableDeclaratorSyntax, - TInitializerSyntax, TAnalyzer> : AbstractObjectCreationExpressionAnalyzer< TExpressionSyntax, TStatementSyntax, TObjectCreationExpressionSyntax, TLocalDeclarationStatementSyntax, TVariableDeclaratorSyntax, - TInitializerSyntax, Match, TAnalyzer> where TExpressionSyntax : SyntaxNode where TStatementSyntax : SyntaxNode @@ -38,7 +36,6 @@ internal abstract class AbstractUseCollectionInitializerAnalyzer< where TExpressionStatementSyntax : TStatementSyntax where TLocalDeclarationStatementSyntax : TStatementSyntax where TVariableDeclaratorSyntax : SyntaxNode - where TInitializerSyntax : SyntaxNode where TAnalyzer : AbstractUseCollectionInitializerAnalyzer< TExpressionSyntax, TStatementSyntax, @@ -48,17 +45,16 @@ internal abstract class AbstractUseCollectionInitializerAnalyzer< TExpressionStatementSyntax, TLocalDeclarationStatementSyntax, TVariableDeclaratorSyntax, - TInitializerSyntax, TAnalyzer>, new() { public readonly record struct AnalysisResult( - TInitializerSyntax? ExistingInitializer, - ImmutableArray Matches); + ImmutableArray PreMatches, + ImmutableArray PostMatches); protected abstract bool IsComplexElementInitializer(SyntaxNode expression); protected abstract bool HasExistingInvalidInitializerForCollection(); protected abstract bool AnalyzeMatchesAndCollectionConstructorForCollectionExpression( - ArrayBuilder matches, out TInitializerSyntax? existingInitializer, CancellationToken cancellationToken); + ArrayBuilder preMatches, ArrayBuilder postMatches, CancellationToken cancellationToken); protected abstract IUpdateExpressionSyntaxHelper SyntaxHelper { get; } @@ -74,11 +70,10 @@ public AnalysisResult Analyze( return default; this.Initialize(state.Value, objectCreationExpression, analyzeForCollectionExpression); - var analysisResult = this.AnalyzeWorker(cancellationToken); - var matches = analysisResult.matches; + var (preMatches, postMatches) = this.AnalyzeWorker(cancellationToken); // If analysis failed entirely, immediately bail out. - if (matches.IsDefault) + if (preMatches.IsDefault || postMatches.IsDefault) return default; // Analysis succeeded, but the result may be empty or non empty. @@ -90,18 +85,17 @@ public AnalysisResult Analyze( // other words, we don't want to suggest changing `new List()` to `new List() { }` as that's just // noise. So convert empty results to an invalid result here. if (analyzeForCollectionExpression) - return new(analysisResult.existingInitializer, matches); + return new(preMatches, postMatches); // Downgrade an empty result to a failure for the normal collection-initializer case. - return matches.IsEmpty ? default : new(analysisResult.existingInitializer, matches); + return postMatches.IsEmpty ? default : new(preMatches, postMatches); } protected sealed override bool TryAddMatches( - ArrayBuilder matches, out TInitializerSyntax? existingInitializer, CancellationToken cancellationToken) + ArrayBuilder preMatches, ArrayBuilder postMatches, CancellationToken cancellationToken) { var seenInvocation = false; var seenIndexAssignment = false; - existingInitializer = null; var initializer = this.SyntaxFacts.GetInitializerOfBaseObjectCreationExpression(_objectCreationExpression); if (initializer != null) @@ -132,12 +126,12 @@ protected sealed override bool TryAddMatches( if (match is null) break; - matches.Add(match.Value); + postMatches.Add(match.Value); } } if (_analyzeForCollectionExpression) - return AnalyzeMatchesAndCollectionConstructorForCollectionExpression(matches, out existingInitializer, cancellationToken); + return AnalyzeMatchesAndCollectionConstructorForCollectionExpression(preMatches, postMatches, cancellationToken); return true; } diff --git a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerDiagnosticAnalyzer.cs b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerDiagnosticAnalyzer.cs index 2329c3c91bed5..fa84c88ff85a6 100644 --- a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerDiagnosticAnalyzer.cs +++ b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerDiagnosticAnalyzer.cs @@ -36,7 +36,6 @@ internal abstract partial class AbstractUseCollectionInitializerDiagnosticAnalyz TExpressionStatementSyntax, TLocalDeclarationStatementSyntax, TVariableDeclaratorSyntax, - TInitializerSyntax, TAnalyzer> : AbstractBuiltInCodeStyleDiagnosticAnalyzer where TSyntaxKind : struct @@ -48,7 +47,6 @@ internal abstract partial class AbstractUseCollectionInitializerDiagnosticAnalyz where TExpressionStatementSyntax : TStatementSyntax where TLocalDeclarationStatementSyntax : TStatementSyntax where TVariableDeclaratorSyntax : SyntaxNode - where TInitializerSyntax : SyntaxNode where TAnalyzer : AbstractUseCollectionInitializerAnalyzer< TExpressionSyntax, TStatementSyntax, @@ -58,7 +56,6 @@ internal abstract partial class AbstractUseCollectionInitializerDiagnosticAnalyz TExpressionStatementSyntax, TLocalDeclarationStatementSyntax, TVariableDeclaratorSyntax, - TInitializerSyntax, TAnalyzer>, new() { diff --git a/src/Analyzers/Core/Analyzers/UseObjectInitializer/AbstractUseObjectInitializerDiagnosticAnalyzer.cs b/src/Analyzers/Core/Analyzers/UseObjectInitializer/AbstractUseObjectInitializerDiagnosticAnalyzer.cs index 20543e0302102..74d6e02d130d5 100644 --- a/src/Analyzers/Core/Analyzers/UseObjectInitializer/AbstractUseObjectInitializerDiagnosticAnalyzer.cs +++ b/src/Analyzers/Core/Analyzers/UseObjectInitializer/AbstractUseObjectInitializerDiagnosticAnalyzer.cs @@ -21,7 +21,6 @@ internal abstract partial class AbstractUseObjectInitializerDiagnosticAnalyzer< TAssignmentStatementSyntax, TLocalDeclarationStatementSyntax, TVariableDeclaratorSyntax, - TINitializerSyntax, TAnalyzer> : AbstractBuiltInCodeStyleDiagnosticAnalyzer where TSyntaxKind : struct @@ -32,7 +31,6 @@ internal abstract partial class AbstractUseObjectInitializerDiagnosticAnalyzer< where TAssignmentStatementSyntax : TStatementSyntax where TLocalDeclarationStatementSyntax : TStatementSyntax where TVariableDeclaratorSyntax : SyntaxNode - where TINitializerSyntax : SyntaxNode where TAnalyzer : AbstractUseNamedMemberInitializerAnalyzer< TExpressionSyntax, TStatementSyntax, @@ -41,7 +39,6 @@ internal abstract partial class AbstractUseObjectInitializerDiagnosticAnalyzer< TAssignmentStatementSyntax, TLocalDeclarationStatementSyntax, TVariableDeclaratorSyntax, - TINitializerSyntax, TAnalyzer>, new() { private static readonly DiagnosticDescriptor s_descriptor = CreateDescriptorWithId( diff --git a/src/Analyzers/Core/Analyzers/UseObjectInitializer/UseNamedMemberInitializerAnalyzer.cs b/src/Analyzers/Core/Analyzers/UseObjectInitializer/UseNamedMemberInitializerAnalyzer.cs index 9621d89feb943..4550c024a4444 100644 --- a/src/Analyzers/Core/Analyzers/UseObjectInitializer/UseNamedMemberInitializerAnalyzer.cs +++ b/src/Analyzers/Core/Analyzers/UseObjectInitializer/UseNamedMemberInitializerAnalyzer.cs @@ -21,14 +21,12 @@ internal abstract class AbstractUseNamedMemberInitializerAnalyzer< TAssignmentStatementSyntax, TLocalDeclarationStatementSyntax, TVariableDeclaratorSyntax, - TInitializerSyntax, TAnalyzer> : AbstractObjectCreationExpressionAnalyzer< TExpressionSyntax, TStatementSyntax, TObjectCreationExpressionSyntax, TLocalDeclarationStatementSyntax, TVariableDeclaratorSyntax, - TInitializerSyntax, Match, TAnalyzer>, IDisposable where TExpressionSyntax : SyntaxNode @@ -38,7 +36,6 @@ internal abstract class AbstractUseNamedMemberInitializerAnalyzer< where TAssignmentStatementSyntax : TStatementSyntax where TLocalDeclarationStatementSyntax : TStatementSyntax where TVariableDeclaratorSyntax : SyntaxNode - where TInitializerSyntax : SyntaxNode where TAnalyzer : AbstractUseNamedMemberInitializerAnalyzer< TExpressionSyntax, TStatementSyntax, @@ -47,7 +44,6 @@ internal abstract class AbstractUseNamedMemberInitializerAnalyzer< TAssignmentStatementSyntax, TLocalDeclarationStatementSyntax, TVariableDeclaratorSyntax, - TInitializerSyntax, TAnalyzer>, new() { public ImmutableArray> Analyze( @@ -61,7 +57,7 @@ public ImmutableArray> matches, - out TInitializerSyntax? exitingInitializer, + ArrayBuilder> preMatches, + ArrayBuilder> postMatches, CancellationToken cancellationToken) { - // Not used in the 'use named member initializer' case. We could update this in the future so that if a user - // already has a named initializer, and adds more initialization statements afterwards, that we can merge them - // into one final initializer. - exitingInitializer = null; - using var _1 = PooledHashSet.GetInstance(out var seenNames); var initializer = this.SyntaxFacts.GetInitializerOfBaseObjectCreationExpression(_objectCreationExpression); @@ -173,7 +164,7 @@ protected sealed override bool TryAddMatches( if (!seenNames.Add(identifier.ValueText)) break; - matches.Add(new Match( + postMatches.Add(new Match( statement, leftMemberAccess, rightExpression, typeMember?.Name ?? identifier.ValueText)); } diff --git a/src/Analyzers/Core/CodeFixes/UseCollectionInitializer/AbstractUseCollectionInitializerCodeFixProvider.cs b/src/Analyzers/Core/CodeFixes/UseCollectionInitializer/AbstractUseCollectionInitializerCodeFixProvider.cs index c79b3a1482eec..47ed574bc170c 100644 --- a/src/Analyzers/Core/CodeFixes/UseCollectionInitializer/AbstractUseCollectionInitializerCodeFixProvider.cs +++ b/src/Analyzers/Core/CodeFixes/UseCollectionInitializer/AbstractUseCollectionInitializerCodeFixProvider.cs @@ -23,7 +23,6 @@ internal abstract class AbstractUseCollectionInitializerCodeFixProvider< TExpressionStatementSyntax, TLocalDeclarationStatementSyntax, TVariableDeclaratorSyntax, - TInitializerSyntax, TAnalyzer>() : AbstractUseCollectionExpressionCodeFixProvider( AnalyzersResources.Collection_initialization_can_be_simplified, @@ -37,7 +36,6 @@ internal abstract class AbstractUseCollectionInitializerCodeFixProvider< where TExpressionStatementSyntax : TStatementSyntax where TLocalDeclarationStatementSyntax : TStatementSyntax where TVariableDeclaratorSyntax : SyntaxNode - where TInitializerSyntax : SyntaxNode where TAnalyzer : AbstractUseCollectionInitializerAnalyzer< TExpressionSyntax, TStatementSyntax, @@ -47,7 +45,6 @@ internal abstract class AbstractUseCollectionInitializerCodeFixProvider< TExpressionStatementSyntax, TLocalDeclarationStatementSyntax, TVariableDeclaratorSyntax, - TInitializerSyntax, TAnalyzer>, new() { public sealed override ImmutableArray FixableDiagnosticIds @@ -56,7 +53,8 @@ public sealed override ImmutableArray FixableDiagnosticIds protected abstract TAnalyzer GetAnalyzer(); protected abstract Task<(SyntaxNode oldNode, SyntaxNode newNode)> GetReplacementNodesAsync( - Document document, TObjectCreationExpressionSyntax objectCreation, bool useCollectionExpression, TInitializerSyntax? existingInitializer, ImmutableArray matches, CancellationToken cancellationToken); + Document document, TObjectCreationExpressionSyntax objectCreation, bool useCollectionExpression, + ImmutableArray preMatches, ImmutableArray postMatches, CancellationToken cancellationToken); protected sealed override async Task FixAsync( Document document, diff --git a/src/Analyzers/Core/CodeFixes/UseObjectInitializer/AbstractUseObjectInitializerCodeFixProvider.cs b/src/Analyzers/Core/CodeFixes/UseObjectInitializer/AbstractUseObjectInitializerCodeFixProvider.cs index 5a28d88096503..2c81b61a74744 100644 --- a/src/Analyzers/Core/CodeFixes/UseObjectInitializer/AbstractUseObjectInitializerCodeFixProvider.cs +++ b/src/Analyzers/Core/CodeFixes/UseObjectInitializer/AbstractUseObjectInitializerCodeFixProvider.cs @@ -24,7 +24,6 @@ internal abstract class AbstractUseObjectInitializerCodeFixProvider< TAssignmentStatementSyntax, TLocalDeclarationStatementSyntax, TVariableDeclaratorSyntax, - TInitializerSyntax, TAnalyzer> : ForkingSyntaxEditorBasedCodeFixProvider where TSyntaxKind : struct @@ -35,7 +34,6 @@ internal abstract class AbstractUseObjectInitializerCodeFixProvider< where TAssignmentStatementSyntax : TStatementSyntax where TLocalDeclarationStatementSyntax : TStatementSyntax where TVariableDeclaratorSyntax : SyntaxNode - where TInitializerSyntax : SyntaxNode where TAnalyzer : AbstractUseNamedMemberInitializerAnalyzer< TExpressionSyntax, TStatementSyntax, @@ -44,7 +42,6 @@ internal abstract class AbstractUseObjectInitializerCodeFixProvider< TAssignmentStatementSyntax, TLocalDeclarationStatementSyntax, TVariableDeclaratorSyntax, - TInitializerSyntax, TAnalyzer>, new() { protected override (string title, string equivalenceKey) GetTitleAndEquivalenceKey(CodeFixContext context) diff --git a/src/Analyzers/VisualBasic/Analyzers/UseCollectionInitializer/VisualBasicCollectionInitializerAnalyzer.vb b/src/Analyzers/VisualBasic/Analyzers/UseCollectionInitializer/VisualBasicCollectionInitializerAnalyzer.vb index 8c67e15a6fae6..5479650b6aa05 100644 --- a/src/Analyzers/VisualBasic/Analyzers/UseCollectionInitializer/VisualBasicCollectionInitializerAnalyzer.vb +++ b/src/Analyzers/VisualBasic/Analyzers/UseCollectionInitializer/VisualBasicCollectionInitializerAnalyzer.vb @@ -18,7 +18,6 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.UseCollectionInitializer ExpressionStatementSyntax, LocalDeclarationStatementSyntax, VariableDeclaratorSyntax, - ObjectCollectionInitializerSyntax, VisualBasicCollectionInitializerAnalyzer) Protected Overrides ReadOnly Property SyntaxHelper As IUpdateExpressionSyntaxHelper(Of ExpressionSyntax, StatementSyntax) = @@ -39,8 +38,8 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.UseCollectionInitializer End Function Protected Overrides Function AnalyzeMatchesAndCollectionConstructorForCollectionExpression( - matches As ArrayBuilder(Of Match), - ByRef existingInitializer As ObjectCollectionInitializerSyntax, + preMatches As ArrayBuilder(Of Match), + postMatches As ArrayBuilder(Of Match), cancellationToken As CancellationToken) As Boolean ' Only called for collection expressions, which VB does not support Throw ExceptionUtilities.Unreachable() diff --git a/src/Analyzers/VisualBasic/Analyzers/UseCollectionInitializer/VisualBasicUseCollectionInitializerDiagnosticAnalyzer.vb b/src/Analyzers/VisualBasic/Analyzers/UseCollectionInitializer/VisualBasicUseCollectionInitializerDiagnosticAnalyzer.vb index 20bb7e448bd97..b15c4bf2bd948 100644 --- a/src/Analyzers/VisualBasic/Analyzers/UseCollectionInitializer/VisualBasicUseCollectionInitializerDiagnosticAnalyzer.vb +++ b/src/Analyzers/VisualBasic/Analyzers/UseCollectionInitializer/VisualBasicUseCollectionInitializerDiagnosticAnalyzer.vb @@ -22,7 +22,6 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.UseCollectionInitializer ExpressionStatementSyntax, LocalDeclarationStatementSyntax, VariableDeclaratorSyntax, - ObjectCollectionInitializerSyntax, VisualBasicCollectionInitializerAnalyzer) Protected Overrides ReadOnly Property SyntaxFacts As ISyntaxFacts = VisualBasicSyntaxFacts.Instance diff --git a/src/Analyzers/VisualBasic/Analyzers/UseObjectInitializer/VisualBasicUseNamedMemberInitializerAnalyzer.vb b/src/Analyzers/VisualBasic/Analyzers/UseObjectInitializer/VisualBasicUseNamedMemberInitializerAnalyzer.vb index d30a4775bf6b6..466210f30027e 100644 --- a/src/Analyzers/VisualBasic/Analyzers/UseObjectInitializer/VisualBasicUseNamedMemberInitializerAnalyzer.vb +++ b/src/Analyzers/VisualBasic/Analyzers/UseObjectInitializer/VisualBasicUseNamedMemberInitializerAnalyzer.vb @@ -16,7 +16,6 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.UseObjectInitializer AssignmentStatementSyntax, LocalDeclarationStatementSyntax, VariableDeclaratorSyntax, - ObjectMemberInitializerSyntax, VisualBasicUseNamedMemberInitializerAnalyzer) Protected Overrides Function IsInitializerOfLocalDeclarationStatement(localDeclarationStatement As LocalDeclarationStatementSyntax, rootExpression As ObjectCreationExpressionSyntax, ByRef variableDeclarator As VariableDeclaratorSyntax) As Boolean diff --git a/src/Analyzers/VisualBasic/Analyzers/UseObjectInitializer/VisualBasicUseObjectInitializerDiagnosticAnalyzer.vb b/src/Analyzers/VisualBasic/Analyzers/UseObjectInitializer/VisualBasicUseObjectInitializerDiagnosticAnalyzer.vb index 8e255e92e5df2..2e981705283c0 100644 --- a/src/Analyzers/VisualBasic/Analyzers/UseObjectInitializer/VisualBasicUseObjectInitializerDiagnosticAnalyzer.vb +++ b/src/Analyzers/VisualBasic/Analyzers/UseObjectInitializer/VisualBasicUseObjectInitializerDiagnosticAnalyzer.vb @@ -20,7 +20,6 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.UseObjectInitializer AssignmentStatementSyntax, LocalDeclarationStatementSyntax, VariableDeclaratorSyntax, - ObjectMemberInitializerSyntax, VisualBasicUseNamedMemberInitializerAnalyzer) Protected Overrides ReadOnly Property FadeOutOperatorToken As Boolean = False diff --git a/src/Analyzers/VisualBasic/CodeFixes/UseCollectionInitializer/VisualBasicUseCollectionInitializerCodeFixProvider.vb b/src/Analyzers/VisualBasic/CodeFixes/UseCollectionInitializer/VisualBasicUseCollectionInitializerCodeFixProvider.vb index 739c905b3f5ef..fe24ab3de3a44 100644 --- a/src/Analyzers/VisualBasic/CodeFixes/UseCollectionInitializer/VisualBasicUseCollectionInitializerCodeFixProvider.vb +++ b/src/Analyzers/VisualBasic/CodeFixes/UseCollectionInitializer/VisualBasicUseCollectionInitializerCodeFixProvider.vb @@ -8,6 +8,8 @@ Imports System.Diagnostics.CodeAnalysis Imports System.Threading Imports Microsoft.CodeAnalysis.CodeFixes Imports Microsoft.CodeAnalysis.Formatting +Imports Microsoft.CodeAnalysis.PooledObjects +Imports Microsoft.CodeAnalysis.UseCollectionInitializer Imports Microsoft.CodeAnalysis.VisualBasic.Syntax Imports Microsoft.CodeAnalysis.VisualBasic.UseObjectInitializer @@ -24,7 +26,6 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.UseCollectionInitializer ExpressionStatementSyntax, LocalDeclarationStatementSyntax, VariableDeclaratorSyntax, - ObjectCollectionInitializerSyntax, VisualBasicCollectionInitializerAnalyzer) @@ -40,21 +41,22 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.UseCollectionInitializer document As Document, objectCreation As ObjectCreationExpressionSyntax, useCollectionExpression As Boolean, - existingInitializer As ObjectCollectionInitializerSyntax, - matches As ImmutableArray(Of Match), + preMatches As ImmutableArray(Of Match), + postMatches As ImmutableArray(Of Match), cancellationToken As CancellationToken) As Task(Of (SyntaxNode, SyntaxNode)) + Contract.ThrowIfFalse(preMatches.IsEmpty) Contract.ThrowIfTrue(useCollectionExpression, "VB does not support collection expressions") Dim statement = objectCreation.FirstAncestorOrSelf(Of StatementSyntax) Dim newStatement = statement.ReplaceNode( objectCreation, - GetNewObjectCreation(objectCreation, matches)) + GetNewObjectCreation(objectCreation, postMatches)) Dim totalTrivia = ArrayBuilder(Of SyntaxTrivia).GetInstance() totalTrivia.AddRange(statement.GetLeadingTrivia()) totalTrivia.Add(SyntaxFactory.ElasticMarker) - For Each match In matches + For Each match In postMatches For Each trivia In match.StatementOrExpression.GetLeadingTrivia() If trivia.Kind = SyntaxKind.CommentTrivia Then totalTrivia.Add(trivia) diff --git a/src/Analyzers/VisualBasic/CodeFixes/UseObjectInitializer/VisualBasicUseObjectInitializerCodeFixProvider.vb b/src/Analyzers/VisualBasic/CodeFixes/UseObjectInitializer/VisualBasicUseObjectInitializerCodeFixProvider.vb index cc39c986077d3..bf7bfaf830d72 100644 --- a/src/Analyzers/VisualBasic/CodeFixes/UseObjectInitializer/VisualBasicUseObjectInitializerCodeFixProvider.vb +++ b/src/Analyzers/VisualBasic/CodeFixes/UseObjectInitializer/VisualBasicUseObjectInitializerCodeFixProvider.vb @@ -22,7 +22,6 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.UseObjectInitializer AssignmentStatementSyntax, LocalDeclarationStatementSyntax, VariableDeclaratorSyntax, - ObjectMemberInitializerSyntax, VisualBasicUseNamedMemberInitializerAnalyzer) From 2495ba8736a0cd4126d455dead26a0e95c0cb74d Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 27 Sep 2024 12:06:57 -0700 Subject: [PATCH 12/25] Strange --- .../CSharpCollectionExpressionRewriter.cs | 49 ++++++++++++------- ...UseCollectionInitializerCodeFixProvider.cs | 8 +-- 2 files changed, 35 insertions(+), 22 deletions(-) diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpCollectionExpressionRewriter.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpCollectionExpressionRewriter.cs index 04bdcf84c6ed3..21af2bbae0a5f 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpCollectionExpressionRewriter.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpCollectionExpressionRewriter.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using System.Collections.Immutable; +using System.Diagnostics; using System.Linq; using System.Threading; using System.Threading.Tasks; @@ -81,7 +82,7 @@ CollectionExpressionSyntax CreateCollectionExpressionWithoutExistingElements() // Didn't have an existing initializer (or it was empty). For both cases, just create an entirely // fresh collection expression, and replace the object entirely. - if (postMatches is [{ Node: ExpressionSyntax expression } match]) + if (preMatches.IsEmpty && postMatches is [{ Node: ExpressionSyntax expression } match]) { // Specialize when we're taking some expression (like x.y.ToArray()) and converting to a spreaded // collection expression. We just want to trivially make that `[.. x.y]` without any specialized @@ -129,7 +130,7 @@ CollectionExpressionSyntax CreateCollectionExpressionWithoutExistingElements() // now create the elements, following that indentation preference. using var _ = ArrayBuilder.GetInstance(out var nodesAndTokens); - CreateAndAddElements(postMatches, nodesAndTokens, preferredIndentation: elementIndentation, forceTrailingComma: true); + CreateAndAddElements(preMatches, postMatches, nodesAndTokens, preferredIndentation: elementIndentation, forceTrailingComma: true); // Add a newline between the last element and the close bracket if we don't already have one. if (nodesAndTokens.Count > 0 && nodesAndTokens.Last().GetTrailingTrivia() is [.., (kind: not SyntaxKind.EndOfLineTrivia)]) @@ -326,7 +327,8 @@ SeparatedSyntaxList FixLeadingAndTrailingWhitespace( // Used to we can uniformly add the items correctly with the requested (but optional) indentation. And so that // commas are added properly to the sequence. void CreateAndAddElements( - ImmutableArray> matches, + ImmutableArray> preMatches, + ImmutableArray> postMatches, ArrayBuilder nodesAndTokens, string? preferredIndentation, bool forceTrailingComma) @@ -702,7 +704,7 @@ bool MakeMultiLineCollectionExpression() { // If there's already an initializer, and we're not adding anything to it, then just keep the initializer // as-is. No need to convert it to be multi-line if it's currently single-line. - if (initializer != null && postMatches.Length == 0) + if (initializer != null && preMatches.Length == 0 && postMatches.Length == 0) return false; var totalLength = 0; @@ -712,27 +714,38 @@ bool MakeMultiLineCollectionExpression() totalLength += expression.Span.Length; } - foreach (var (node, _) in postMatches) + if (CheckForMultiLine(preMatches) || + CheckForMultiLine(postMatches)) { - // if the statement we're replacing has any comments on it, then we need to be multiline to give them an - // appropriate place to go. - if (node.GetLeadingTrivia().Any(static t => t.IsSingleOrMultiLineComment()) || - node.GetTrailingTrivia().Any(static t => t.IsSingleOrMultiLineComment())) - { - return true; - } + return true; + } + + return totalLength > wrappingLength; - foreach (var component in GetElementComponents(node)) + bool CheckForMultiLine(ImmutableArray> matches) + { + foreach (var (node, _) in matches) { - // if any of the expressions we're adding are multiline, then make things multiline. - if (!document.Text.AreOnSameLine(component.GetFirstToken(), component.GetLastToken())) + // if the statement we're replacing has any comments on it, then we need to be multiline to give them an + // appropriate place to go. + if (node.GetLeadingTrivia().Any(static t => t.IsSingleOrMultiLineComment()) || + node.GetTrailingTrivia().Any(static t => t.IsSingleOrMultiLineComment())) + { return true; + } - totalLength += component.Span.Length; + foreach (var component in GetElementComponents(node)) + { + // if any of the expressions we're adding are multiline, then make things multiline. + if (!document.Text.AreOnSameLine(component.GetFirstToken(), component.GetLastToken())) + return true; + + totalLength += component.Span.Length; + } } - } - return totalLength > wrappingLength; + return false; + } } static IEnumerable GetElementComponents(TMatchNode node) diff --git a/src/Analyzers/Core/CodeFixes/UseCollectionInitializer/AbstractUseCollectionInitializerCodeFixProvider.cs b/src/Analyzers/Core/CodeFixes/UseCollectionInitializer/AbstractUseCollectionInitializerCodeFixProvider.cs index 47ed574bc170c..41e31c87aa052 100644 --- a/src/Analyzers/Core/CodeFixes/UseCollectionInitializer/AbstractUseCollectionInitializerCodeFixProvider.cs +++ b/src/Analyzers/Core/CodeFixes/UseCollectionInitializer/AbstractUseCollectionInitializerCodeFixProvider.cs @@ -75,17 +75,17 @@ protected sealed override async Task FixAsync( using var analyzer = GetAnalyzer(); var useCollectionExpression = properties.ContainsKey(UseCollectionInitializerHelpers.UseCollectionExpressionName) is true; - var (initializer, matches) = analyzer.Analyze( + var (preMatches, postMatches) = analyzer.Analyze( semanticModel, syntaxFacts, objectCreation, useCollectionExpression, cancellationToken); - if (matches.IsDefault) + if (preMatches.IsDefault || postMatches.IsDefault) return; var (oldNode, newNode) = await GetReplacementNodesAsync( - document, objectCreation, useCollectionExpression, initializer, matches, cancellationToken).ConfigureAwait(false); + document, objectCreation, useCollectionExpression, preMatches, postMatches, cancellationToken).ConfigureAwait(false); editor.ReplaceNode(oldNode, newNode); - foreach (var match in matches) + foreach (var match in postMatches) { if (match.StatementOrExpression is TStatementSyntax statement) editor.RemoveNode(statement, SyntaxRemoveOptions.KeepUnbalancedDirectives); From 80ef9d09a06f690622f1173798f68a899fb54014 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 27 Sep 2024 12:09:09 -0700 Subject: [PATCH 13/25] in progress --- .../CSharpCollectionExpressionRewriter.cs | 55 +++++++------------ ...ectionExpressionForArrayCodeFixProvider.cs | 1 - ...tionExpressionForBuilderCodeFixProvider.cs | 1 - ...ctionExpressionForCreateCodeFixProvider.cs | 1 - ...ctionExpressionForFluentCodeFixProvider.cs | 1 - ...nExpressionForStackAllocCodeFixProvider.cs | 1 - ...zerCodeFixProvider_CollectionExpression.cs | 4 +- 7 files changed, 22 insertions(+), 42 deletions(-) diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpCollectionExpressionRewriter.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpCollectionExpressionRewriter.cs index 21af2bbae0a5f..393e25ad672fc 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpCollectionExpressionRewriter.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpCollectionExpressionRewriter.cs @@ -34,8 +34,7 @@ internal static class CSharpCollectionExpressionRewriter public static async Task CreateCollectionExpressionAsync( Document workspaceDocument, TParentExpression expressionToReplace, - ImmutableArray> preMatches, - ImmutableArray> postMatches, + ImmutableArray> matches, Func getInitializer, Func withInitializer, CancellationToken cancellationToken) @@ -82,7 +81,7 @@ CollectionExpressionSyntax CreateCollectionExpressionWithoutExistingElements() // Didn't have an existing initializer (or it was empty). For both cases, just create an entirely // fresh collection expression, and replace the object entirely. - if (preMatches.IsEmpty && postMatches is [{ Node: ExpressionSyntax expression } match]) + if (matches is [{ Node: ExpressionSyntax expression } match]) { // Specialize when we're taking some expression (like x.y.ToArray()) and converting to a spreaded // collection expression. We just want to trivially make that `[.. x.y]` without any specialized @@ -130,7 +129,7 @@ CollectionExpressionSyntax CreateCollectionExpressionWithoutExistingElements() // now create the elements, following that indentation preference. using var _ = ArrayBuilder.GetInstance(out var nodesAndTokens); - CreateAndAddElements(preMatches, postMatches, nodesAndTokens, preferredIndentation: elementIndentation, forceTrailingComma: true); + CreateAndAddElements(matches, nodesAndTokens, preferredIndentation: elementIndentation, forceTrailingComma: true); // Add a newline between the last element and the close bracket if we don't already have one. if (nodesAndTokens.Count > 0 && nodesAndTokens.Last().GetTrailingTrivia() is [.., (kind: not SyntaxKind.EndOfLineTrivia)]) @@ -152,7 +151,7 @@ CollectionExpressionSyntax CreateCollectionExpressionWithoutExistingElements() // fresh collection expression, and do a wholesale replacement of the original object creation // expression with it. using var _ = ArrayBuilder.GetInstance(out var nodesAndTokens); - CreateAndAddElements(postMatches, nodesAndTokens, preferredIndentation: null, forceTrailingComma: false); + CreateAndAddElements(matches, nodesAndTokens, preferredIndentation: null, forceTrailingComma: false); // Remove any trailing whitespace from the last element/comma and the final close bracket. if (nodesAndTokens.Count > 0) @@ -327,8 +326,7 @@ SeparatedSyntaxList FixLeadingAndTrailingWhitespace( // Used to we can uniformly add the items correctly with the requested (but optional) indentation. And so that // commas are added properly to the sequence. void CreateAndAddElements( - ImmutableArray> preMatches, - ImmutableArray> postMatches, + ImmutableArray> matches, ArrayBuilder nodesAndTokens, string? preferredIndentation, bool forceTrailingComma) @@ -404,7 +402,7 @@ CollectionExpressionSyntax AddMatchesToExistingNonEmptyCollectionExpression( // end. This keeps every element consistent with ending the line with a comma, which makes code easier to // maintain. CreateAndAddElements( - postMatches, nodesAndTokens, preferredIndentation, + matches, nodesAndTokens, preferredIndentation, forceTrailingComma: preferredIndentation != null && trailingComma == default); if (trailingComma != default) @@ -704,7 +702,7 @@ bool MakeMultiLineCollectionExpression() { // If there's already an initializer, and we're not adding anything to it, then just keep the initializer // as-is. No need to convert it to be multi-line if it's currently single-line. - if (initializer != null && preMatches.Length == 0 && postMatches.Length == 0) + if (initializer != null && matches.Length == 0) return false; var totalLength = 0; @@ -714,38 +712,27 @@ bool MakeMultiLineCollectionExpression() totalLength += expression.Span.Length; } - if (CheckForMultiLine(preMatches) || - CheckForMultiLine(postMatches)) + foreach (var (node, _) in matches) { - return true; - } - - return totalLength > wrappingLength; + // if the statement we're replacing has any comments on it, then we need to be multiline to give them an + // appropriate place to go. + if (node.GetLeadingTrivia().Any(static t => t.IsSingleOrMultiLineComment()) || + node.GetTrailingTrivia().Any(static t => t.IsSingleOrMultiLineComment())) + { + return true; + } - bool CheckForMultiLine(ImmutableArray> matches) - { - foreach (var (node, _) in matches) + foreach (var component in GetElementComponents(node)) { - // if the statement we're replacing has any comments on it, then we need to be multiline to give them an - // appropriate place to go. - if (node.GetLeadingTrivia().Any(static t => t.IsSingleOrMultiLineComment()) || - node.GetTrailingTrivia().Any(static t => t.IsSingleOrMultiLineComment())) - { + // if any of the expressions we're adding are multiline, then make things multiline. + if (!document.Text.AreOnSameLine(component.GetFirstToken(), component.GetLastToken())) return true; - } - foreach (var component in GetElementComponents(node)) - { - // if any of the expressions we're adding are multiline, then make things multiline. - if (!document.Text.AreOnSameLine(component.GetFirstToken(), component.GetLastToken())) - return true; - - totalLength += component.Span.Length; - } + totalLength += component.Span.Length; } - - return false; } + + return totalLength > wrappingLength; } static IEnumerable GetElementComponents(TMatchNode node) diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForArrayCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForArrayCodeFixProvider.cs index 937e95b97a57b..3ca81464e419a 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForArrayCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForArrayCodeFixProvider.cs @@ -72,7 +72,6 @@ and not ImplicitArrayCreationExpressionSyntax var collectionExpression = await CSharpCollectionExpressionRewriter.CreateCollectionExpressionAsync( document, arrayCreationExpression, - preMatches: [], matches, static e => e switch { diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForBuilderCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForBuilderCodeFixProvider.cs index 04941a6937e4c..37cfb81ca743f 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForBuilderCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForBuilderCodeFixProvider.cs @@ -65,7 +65,6 @@ protected override async Task FixAsync( var collectionExpression = await CreateCollectionExpressionAsync( newDocument, dummyObjectCreation, - preMatches: [], analysisResult.Matches.SelectAsArray(m => new CollectionExpressionMatch(m.StatementOrExpression, m.UseSpread)), static o => o.Initializer, static (o, i) => o.WithInitializer(i), diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForCreateCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForCreateCodeFixProvider.cs index d6004c154d219..0466efe136cb1 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForCreateCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForCreateCodeFixProvider.cs @@ -67,7 +67,6 @@ protected override async Task FixAsync( var collectionExpression = await CreateCollectionExpressionAsync( newSemanticDocument.Document, dummyObjectCreation, - preMatches: [], matches, static o => o.Initializer, static (o, i) => o.WithInitializer(i), diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForFluentCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForFluentCodeFixProvider.cs index 8d6639c8ddc19..09314809aa588 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForFluentCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForFluentCodeFixProvider.cs @@ -91,7 +91,6 @@ protected override async Task FixAsync( var collectionExpression = await CreateCollectionExpressionAsync( newSemanticDocument.Document, dummyObjectCreation, - preMatches: [], matches, static o => o.Initializer, static (o, i) => o.WithInitializer(i), diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocCodeFixProvider.cs index 0649e538a58c8..da987b48b87d4 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocCodeFixProvider.cs @@ -47,7 +47,6 @@ protected sealed override async Task FixAsync( var collectionExpression = await CSharpCollectionExpressionRewriter.CreateCollectionExpressionAsync( document, stackAllocExpression, - preMatches: [], matches, static e => e switch { diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionExpression.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionExpression.cs index 47c6aa63276f6..b4f8c1a775e4e 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionExpression.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionExpression.cs @@ -27,9 +27,7 @@ private static Task CreateCollectionExpressionAsync( return CSharpCollectionExpressionRewriter.CreateCollectionExpressionAsync( document, objectCreation, - preMatches.SelectAsArray(m => new CollectionExpressionMatch(m.StatementOrExpression, m.UseSpread)), - postMatches.SelectAsArray(m => new CollectionExpressionMatch(m.StatementOrExpression, m.UseSpread)), - // Use the initializer the analyzer recommends, regardless of what's on the object creation node. + preMatches.Concat(postMatches).SelectAsArray(m => new CollectionExpressionMatch(m.StatementOrExpression, m.UseSpread)), static objectCreation => objectCreation.Initializer, static (objectCreation, initializer) => objectCreation.WithInitializer(initializer), cancellationToken); From 9762535da5f6a8ef0fdab4a854630439ddd347a2 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 27 Sep 2024 12:24:35 -0700 Subject: [PATCH 14/25] in progress --- .../CSharpCollectionExpressionRewriter.cs | 119 +++++++++++------- 1 file changed, 75 insertions(+), 44 deletions(-) diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpCollectionExpressionRewriter.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpCollectionExpressionRewriter.cs index 393e25ad672fc..0c97d162902a3 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpCollectionExpressionRewriter.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpCollectionExpressionRewriter.cs @@ -7,6 +7,8 @@ using System.Collections.Immutable; using System.Diagnostics; using System.Linq; +using System.Linq.Expressions; +using System.Text.RegularExpressions; using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis.CodeActions; @@ -34,7 +36,8 @@ internal static class CSharpCollectionExpressionRewriter public static async Task CreateCollectionExpressionAsync( Document workspaceDocument, TParentExpression expressionToReplace, - ImmutableArray> matches, + ImmutableArray> preMatches, + ImmutableArray> postMatches, Func getInitializer, Func withInitializer, CancellationToken cancellationToken) @@ -81,22 +84,13 @@ CollectionExpressionSyntax CreateCollectionExpressionWithoutExistingElements() // Didn't have an existing initializer (or it was empty). For both cases, just create an entirely // fresh collection expression, and replace the object entirely. - if (matches is [{ Node: ExpressionSyntax expression } match]) + if (preMatches is [{ Node: ExpressionSyntax } preMatch] && postMatches.IsEmpty) { - // Specialize when we're taking some expression (like x.y.ToArray()) and converting to a spreaded - // collection expression. We just want to trivially make that `[.. x.y]` without any specialized - // behavior. In particular, we do not want to generate something like: - // - // [ - // .. x.y, - // ] - // - // For that sort of case. Single element collections should stay closely associated with the original - // expression. - return CollectionExpression([ - match.UseSpread - ? SpreadElement(expression.WithoutTrivia()) - : ExpressionElement(expression.WithoutTrivia())]).WithTriviaFrom(expressionToReplace); + return CreateSingleElementCollection(preMatch); + } + else if (preMatches.IsEmpty && postMatches is [{ Node: ExpressionSyntax } postMatch]) + { + return CreateSingleElementCollection(postMatch); } else if (makeMultiLineCollectionExpression) { @@ -129,7 +123,8 @@ CollectionExpressionSyntax CreateCollectionExpressionWithoutExistingElements() // now create the elements, following that indentation preference. using var _ = ArrayBuilder.GetInstance(out var nodesAndTokens); - CreateAndAddElements(matches, nodesAndTokens, preferredIndentation: elementIndentation, forceTrailingComma: true); + CreateAndAddElements(preMatches, nodesAndTokens, preferredIndentation: elementIndentation, forceTrailingComma: true); + CreateAndAddElements(postMatches, nodesAndTokens, preferredIndentation: elementIndentation, forceTrailingComma: true); // Add a newline between the last element and the close bracket if we don't already have one. if (nodesAndTokens.Count > 0 && nodesAndTokens.Last().GetTrailingTrivia() is [.., (kind: not SyntaxKind.EndOfLineTrivia)]) @@ -151,7 +146,8 @@ CollectionExpressionSyntax CreateCollectionExpressionWithoutExistingElements() // fresh collection expression, and do a wholesale replacement of the original object creation // expression with it. using var _ = ArrayBuilder.GetInstance(out var nodesAndTokens); - CreateAndAddElements(matches, nodesAndTokens, preferredIndentation: null, forceTrailingComma: false); + CreateAndAddElements(preMatches, nodesAndTokens, preferredIndentation: null, forceTrailingComma: false); + CreateAndAddElements(postMatches, nodesAndTokens, preferredIndentation: null, forceTrailingComma: false); // Remove any trailing whitespace from the last element/comma and the final close bracket. if (nodesAndTokens.Count > 0) @@ -197,6 +193,25 @@ CollectionExpressionSyntax CreateCollectionExpressionWithoutExistingElements() } } + CollectionExpressionSyntax CreateSingleElementCollection(CollectionExpressionMatch match) + { + // Specialize when we're taking some expression (like x.y.ToArray()) and converting to a spreaded + // collection expression. We just want to trivially make that `[.. x.y]` without any specialized + // behavior. In particular, we do not want to generate something like: + // + // [ + // .. x.y, + // ] + // + // For that sort of case. Single element collections should stay closely associated with the original + // expression. + var expression = (ExpressionSyntax)(object)match.Node; + return CollectionExpression([ + match.UseSpread + ? SpreadElement(expression.WithoutTrivia()) + : ExpressionElement(expression.WithoutTrivia())]).WithTriviaFrom(expressionToReplace); + } + CollectionExpressionSyntax CreateCollectionExpressionWithExistingElements() { // If the object creation expression had an initializer (with at least one element in it). Attempt to @@ -220,8 +235,8 @@ CollectionExpressionSyntax CreateCollectionExpressionWithExistingElements() // } // // Just add the new elements to this. - var finalCollection = AddMatchesToExistingNonEmptyCollectionExpression( - initialCollection, preferredIndentation: null); + var withPreMatches = AddMatchesToExistingNonEmptyCollectionExpression(preMatches, initialCollection, preferredIndentation: null, index: 0); + var finalCollection = AddMatchesToExistingNonEmptyCollectionExpression(postMatches, withPreMatches, preferredIndentation: null); return UseCollectionExpressionHelpers.ReplaceWithCollectionExpression( document.Text, initializer, finalCollection, newCollectionIsSingleLine: false); @@ -233,8 +248,8 @@ CollectionExpressionSyntax CreateCollectionExpressionWithExistingElements() var preferredIndentation = initializer.Expressions.First().GetFirstToken().GetPreferredIndentation( document, indentationOptions, cancellationToken); - var finalCollection = AddMatchesToExistingNonEmptyCollectionExpression( - initialCollection, preferredIndentation); + var withPreMatches = AddMatchesToExistingNonEmptyCollectionExpression(preMatches, initialCollection, preferredIndentation: null, index: 0); + var finalCollection = AddMatchesToExistingNonEmptyCollectionExpression(postMatches, withPreMatches, preferredIndentation); return UseCollectionExpressionHelpers.ReplaceWithCollectionExpression( document.Text, initializer, finalCollection, newCollectionIsSingleLine: false); @@ -262,7 +277,8 @@ CollectionExpressionSyntax CreateCollectionExpressionWithExistingElements() initialCollection.CloseBracketToken.WithLeadingTrivia(endOfLine, Whitespace(preferredBraceIndentation))); // Then add all new elements at the right indentation level. - var finalCollection = AddMatchesToExistingNonEmptyCollectionExpression(initialCollection, preferredItemIndentation); + var withPreMatches = AddMatchesToExistingNonEmptyCollectionExpression(preMatches, initialCollection, preferredItemIndentation, index: 0); + var finalCollection = AddMatchesToExistingNonEmptyCollectionExpression(postMatches, withPreMatches, preferredItemIndentation); return UseCollectionExpressionHelpers.ReplaceWithCollectionExpression( document.Text, initializer, finalCollection, newCollectionIsSingleLine: false); @@ -283,7 +299,8 @@ CollectionExpressionSyntax CreateCollectionExpressionWithExistingElements() FixLeadingAndTrailingWhitespace(initialCollection.Elements, preferredItemIndentation), initialCollection.CloseBracketToken.WithLeadingTrivia(endOfLine, Whitespace(braceIndentation))); - var finalCollection = AddMatchesToExistingNonEmptyCollectionExpression(initialCollection, preferredItemIndentation); + var withPreMatches = AddMatchesToExistingNonEmptyCollectionExpression(preMatches, initialCollection, preferredItemIndentation, index: 0); + var finalCollection = AddMatchesToExistingNonEmptyCollectionExpression(postMatches, withPreMatches, preferredItemIndentation); return UseCollectionExpressionHelpers.ReplaceWithCollectionExpression( document.Text, initializer, finalCollection, newCollectionIsSingleLine: false); @@ -299,6 +316,7 @@ CollectionExpressionSyntax CreateCollectionExpressionWithExistingElements() initializer, wasOnSingleLine: true); // now, add all the matches in after the existing elements. + var withPreMatches = AddMatchesToExistingNonEmptyCollectionExpression(preMatches, initialCollection, preferredIndentation: null, index: 0); var finalCollection = AddMatchesToExistingNonEmptyCollectionExpression(initialCollection, preferredIndentation: null); // Now do the actual replacement. This will ensure the location of the collection expression @@ -326,7 +344,7 @@ SeparatedSyntaxList FixLeadingAndTrailingWhitespace( // Used to we can uniformly add the items correctly with the requested (but optional) indentation. And so that // commas are added properly to the sequence. void CreateAndAddElements( - ImmutableArray> matches, + ImmutableArray> postMatches, ArrayBuilder nodesAndTokens, string? preferredIndentation, bool forceTrailingComma) @@ -338,13 +356,13 @@ void CreateAndAddElements( ? TriviaList(Space) : TriviaList(endOfLine); - foreach (var element in matches.SelectMany(m => CreateElements(m, preferredIndentation))) + foreach (var element in postMatches.SelectMany(m => CreateElements(m, preferredIndentation))) { AddCommaIfMissing(last: false); nodesAndTokens.Add(element); } - if (matches.Length > 0 && forceTrailingComma) + if (postMatches.Length > 0 && forceTrailingComma) AddCommaIfMissing(last: true); return; @@ -376,8 +394,10 @@ void AddCommaIfMissing(bool last) // Helper which takes a collection expression that already has at least one element in it and adds the new // elements to it. CollectionExpressionSyntax AddMatchesToExistingNonEmptyCollectionExpression( + ImmutableArray> matches, CollectionExpressionSyntax initialCollectionExpression, - string? preferredIndentation) + string? preferredIndentation, + int index = -1) { using var _ = ArrayBuilder.GetInstance(out var nodesAndTokens); nodesAndTokens.AddRange(initialCollectionExpression.Elements.GetWithSeparators()); @@ -402,7 +422,7 @@ CollectionExpressionSyntax AddMatchesToExistingNonEmptyCollectionExpression( // end. This keeps every element consistent with ending the line with a comma, which makes code easier to // maintain. CreateAndAddElements( - matches, nodesAndTokens, preferredIndentation, + postMatches, nodesAndTokens, preferredIndentation, forceTrailingComma: preferredIndentation != null && trailingComma == default); if (trailingComma != default) @@ -702,7 +722,7 @@ bool MakeMultiLineCollectionExpression() { // If there's already an initializer, and we're not adding anything to it, then just keep the initializer // as-is. No need to convert it to be multi-line if it's currently single-line. - if (initializer != null && matches.Length == 0) + if (initializer != null && preMatches.Length == 0 && postMatches.Length == 0) return false; var totalLength = 0; @@ -712,27 +732,38 @@ bool MakeMultiLineCollectionExpression() totalLength += expression.Span.Length; } - foreach (var (node, _) in matches) + if (CheckForMultiLine(preMatches) || + CheckForMultiLine(postMatches)) { - // if the statement we're replacing has any comments on it, then we need to be multiline to give them an - // appropriate place to go. - if (node.GetLeadingTrivia().Any(static t => t.IsSingleOrMultiLineComment()) || - node.GetTrailingTrivia().Any(static t => t.IsSingleOrMultiLineComment())) - { - return true; - } + return true; + } + + return totalLength > wrappingLength; - foreach (var component in GetElementComponents(node)) + bool CheckForMultiLine(ImmutableArray> matches) + { + foreach (var (node, _) in matches) { - // if any of the expressions we're adding are multiline, then make things multiline. - if (!document.Text.AreOnSameLine(component.GetFirstToken(), component.GetLastToken())) + // if the statement we're replacing has any comments on it, then we need to be multiline to give them an + // appropriate place to go. + if (node.GetLeadingTrivia().Any(static t => t.IsSingleOrMultiLineComment()) || + node.GetTrailingTrivia().Any(static t => t.IsSingleOrMultiLineComment())) + { return true; + } - totalLength += component.Span.Length; + foreach (var component in GetElementComponents(node)) + { + // if any of the expressions we're adding are multiline, then make things multiline. + if (!document.Text.AreOnSameLine(component.GetFirstToken(), component.GetLastToken())) + return true; + + totalLength += component.Span.Length; + } } - } - return totalLength > wrappingLength; + return false; + } } static IEnumerable GetElementComponents(TMatchNode node) From d6b32a3d79b5250ccf20782b87da999bbb72e467 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 27 Sep 2024 12:31:25 -0700 Subject: [PATCH 15/25] in progress --- .../CSharpCollectionExpressionRewriter.cs | 32 ++++++++++--------- ...ectionExpressionForArrayCodeFixProvider.cs | 1 + ...tionExpressionForBuilderCodeFixProvider.cs | 1 + ...ctionExpressionForCreateCodeFixProvider.cs | 1 + ...ctionExpressionForFluentCodeFixProvider.cs | 1 + ...nExpressionForStackAllocCodeFixProvider.cs | 1 + ...zerCodeFixProvider_CollectionExpression.cs | 1 + 7 files changed, 23 insertions(+), 15 deletions(-) diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpCollectionExpressionRewriter.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpCollectionExpressionRewriter.cs index 0c97d162902a3..93f1635be0fe6 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpCollectionExpressionRewriter.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpCollectionExpressionRewriter.cs @@ -235,8 +235,8 @@ CollectionExpressionSyntax CreateCollectionExpressionWithExistingElements() // } // // Just add the new elements to this. - var withPreMatches = AddMatchesToExistingNonEmptyCollectionExpression(preMatches, initialCollection, preferredIndentation: null, index: 0); - var finalCollection = AddMatchesToExistingNonEmptyCollectionExpression(postMatches, withPreMatches, preferredIndentation: null); + var finalCollection = AddMatchesToExistingNonEmptyCollectionExpression( + initialCollection, preferredIndentation: null); return UseCollectionExpressionHelpers.ReplaceWithCollectionExpression( document.Text, initializer, finalCollection, newCollectionIsSingleLine: false); @@ -248,8 +248,8 @@ CollectionExpressionSyntax CreateCollectionExpressionWithExistingElements() var preferredIndentation = initializer.Expressions.First().GetFirstToken().GetPreferredIndentation( document, indentationOptions, cancellationToken); - var withPreMatches = AddMatchesToExistingNonEmptyCollectionExpression(preMatches, initialCollection, preferredIndentation: null, index: 0); - var finalCollection = AddMatchesToExistingNonEmptyCollectionExpression(postMatches, withPreMatches, preferredIndentation); + var finalCollection = AddMatchesToExistingNonEmptyCollectionExpression( + initialCollection, preferredIndentation); return UseCollectionExpressionHelpers.ReplaceWithCollectionExpression( document.Text, initializer, finalCollection, newCollectionIsSingleLine: false); @@ -277,8 +277,7 @@ CollectionExpressionSyntax CreateCollectionExpressionWithExistingElements() initialCollection.CloseBracketToken.WithLeadingTrivia(endOfLine, Whitespace(preferredBraceIndentation))); // Then add all new elements at the right indentation level. - var withPreMatches = AddMatchesToExistingNonEmptyCollectionExpression(preMatches, initialCollection, preferredItemIndentation, index: 0); - var finalCollection = AddMatchesToExistingNonEmptyCollectionExpression(postMatches, withPreMatches, preferredItemIndentation); + var finalCollection = AddMatchesToExistingNonEmptyCollectionExpression(initialCollection, preferredItemIndentation); return UseCollectionExpressionHelpers.ReplaceWithCollectionExpression( document.Text, initializer, finalCollection, newCollectionIsSingleLine: false); @@ -299,8 +298,7 @@ CollectionExpressionSyntax CreateCollectionExpressionWithExistingElements() FixLeadingAndTrailingWhitespace(initialCollection.Elements, preferredItemIndentation), initialCollection.CloseBracketToken.WithLeadingTrivia(endOfLine, Whitespace(braceIndentation))); - var withPreMatches = AddMatchesToExistingNonEmptyCollectionExpression(preMatches, initialCollection, preferredItemIndentation, index: 0); - var finalCollection = AddMatchesToExistingNonEmptyCollectionExpression(postMatches, withPreMatches, preferredItemIndentation); + var finalCollection = AddMatchesToExistingNonEmptyCollectionExpression(initialCollection, preferredItemIndentation); return UseCollectionExpressionHelpers.ReplaceWithCollectionExpression( document.Text, initializer, finalCollection, newCollectionIsSingleLine: false); @@ -316,7 +314,6 @@ CollectionExpressionSyntax CreateCollectionExpressionWithExistingElements() initializer, wasOnSingleLine: true); // now, add all the matches in after the existing elements. - var withPreMatches = AddMatchesToExistingNonEmptyCollectionExpression(preMatches, initialCollection, preferredIndentation: null, index: 0); var finalCollection = AddMatchesToExistingNonEmptyCollectionExpression(initialCollection, preferredIndentation: null); // Now do the actual replacement. This will ensure the location of the collection expression @@ -344,7 +341,7 @@ SeparatedSyntaxList FixLeadingAndTrailingWhitespace( // Used to we can uniformly add the items correctly with the requested (but optional) indentation. And so that // commas are added properly to the sequence. void CreateAndAddElements( - ImmutableArray> postMatches, + ImmutableArray> matches, ArrayBuilder nodesAndTokens, string? preferredIndentation, bool forceTrailingComma) @@ -356,13 +353,13 @@ void CreateAndAddElements( ? TriviaList(Space) : TriviaList(endOfLine); - foreach (var element in postMatches.SelectMany(m => CreateElements(m, preferredIndentation))) + foreach (var element in matches.SelectMany(m => CreateElements(m, preferredIndentation))) { AddCommaIfMissing(last: false); nodesAndTokens.Add(element); } - if (postMatches.Length > 0 && forceTrailingComma) + if (matches.Length > 0 && forceTrailingComma) AddCommaIfMissing(last: true); return; @@ -394,12 +391,15 @@ void AddCommaIfMissing(bool last) // Helper which takes a collection expression that already has at least one element in it and adds the new // elements to it. CollectionExpressionSyntax AddMatchesToExistingNonEmptyCollectionExpression( - ImmutableArray> matches, CollectionExpressionSyntax initialCollectionExpression, - string? preferredIndentation, - int index = -1) + string? preferredIndentation) { using var _ = ArrayBuilder.GetInstance(out var nodesAndTokens); + + // Add any pre-items before the initializer items. + CreateAndAddElements(preMatches, nodesAndTokens, preferredIndentation, forceTrailingComma: true); + + // Now add all the initializer items. nodesAndTokens.AddRange(initialCollectionExpression.Elements.GetWithSeparators()); // If there is already a trailing comma before, remove it. We'll add it back at the end. If there is no @@ -418,6 +418,8 @@ CollectionExpressionSyntax AddMatchesToExistingNonEmptyCollectionExpression( nodesAndTokens[^1] = nodesAndTokens[^1].WithTrailingTrivia(); } + // Now add all the post matches in. + // If we're wrapping to multiple lines, and we don't already have a trailing comma, then force one at the // end. This keeps every element consistent with ending the line with a comma, which makes code easier to // maintain. diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForArrayCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForArrayCodeFixProvider.cs index 3ca81464e419a..937e95b97a57b 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForArrayCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForArrayCodeFixProvider.cs @@ -72,6 +72,7 @@ and not ImplicitArrayCreationExpressionSyntax var collectionExpression = await CSharpCollectionExpressionRewriter.CreateCollectionExpressionAsync( document, arrayCreationExpression, + preMatches: [], matches, static e => e switch { diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForBuilderCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForBuilderCodeFixProvider.cs index 37cfb81ca743f..04941a6937e4c 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForBuilderCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForBuilderCodeFixProvider.cs @@ -65,6 +65,7 @@ protected override async Task FixAsync( var collectionExpression = await CreateCollectionExpressionAsync( newDocument, dummyObjectCreation, + preMatches: [], analysisResult.Matches.SelectAsArray(m => new CollectionExpressionMatch(m.StatementOrExpression, m.UseSpread)), static o => o.Initializer, static (o, i) => o.WithInitializer(i), diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForCreateCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForCreateCodeFixProvider.cs index 0466efe136cb1..d6004c154d219 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForCreateCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForCreateCodeFixProvider.cs @@ -67,6 +67,7 @@ protected override async Task FixAsync( var collectionExpression = await CreateCollectionExpressionAsync( newSemanticDocument.Document, dummyObjectCreation, + preMatches: [], matches, static o => o.Initializer, static (o, i) => o.WithInitializer(i), diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForFluentCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForFluentCodeFixProvider.cs index 09314809aa588..8d6639c8ddc19 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForFluentCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForFluentCodeFixProvider.cs @@ -91,6 +91,7 @@ protected override async Task FixAsync( var collectionExpression = await CreateCollectionExpressionAsync( newSemanticDocument.Document, dummyObjectCreation, + preMatches: [], matches, static o => o.Initializer, static (o, i) => o.WithInitializer(i), diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocCodeFixProvider.cs index da987b48b87d4..0649e538a58c8 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocCodeFixProvider.cs @@ -47,6 +47,7 @@ protected sealed override async Task FixAsync( var collectionExpression = await CSharpCollectionExpressionRewriter.CreateCollectionExpressionAsync( document, stackAllocExpression, + preMatches: [], matches, static e => e switch { diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionExpression.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionExpression.cs index b4f8c1a775e4e..910494b58f1e7 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionExpression.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionExpression.cs @@ -28,6 +28,7 @@ private static Task CreateCollectionExpressionAsync( document, objectCreation, preMatches.Concat(postMatches).SelectAsArray(m => new CollectionExpressionMatch(m.StatementOrExpression, m.UseSpread)), + postMatches.Concat(postMatches).SelectAsArray(m => new CollectionExpressionMatch(m.StatementOrExpression, m.UseSpread)), static objectCreation => objectCreation.Initializer, static (objectCreation, initializer) => objectCreation.WithInitializer(initializer), cancellationToken); From 4f0f6ba833dd7397632900ccbf9601330045bfaa Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 27 Sep 2024 12:42:24 -0700 Subject: [PATCH 16/25] in progress --- ...onExpressionForFluentDiagnosticAnalyzer.cs | 59 ++++++++++--------- ...ctionExpressionForFluentCodeFixProvider.cs | 19 +++--- 2 files changed, 42 insertions(+), 36 deletions(-) diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForFluentDiagnosticAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForFluentDiagnosticAnalyzer.cs index 3130d655bace5..299adc0270329 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForFluentDiagnosticAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForFluentDiagnosticAnalyzer.cs @@ -133,9 +133,16 @@ private void AnalyzeMemberAccess(SyntaxNodeAnalysisContext context, INamedTypeSy { // Because we're recursing from top to bottom in the expression tree, we build up the matches in reverse. Right // before returning them, we'll reverse them again to get the proper order. - using var _ = ArrayBuilder>.GetInstance(out var matchesInReverse); - if (!AnalyzeInvocation(text, state, invocation, addMatches ? matchesInReverse : null, out var existingInitializer, cancellationToken)) + using var _1 = ArrayBuilder>.GetInstance(out var preMatchesInReverse); + using var _2 = ArrayBuilder>.GetInstance(out var postMatchesInReverse); + if (!AnalyzeInvocation( + text, state, invocation, + addMatches ? preMatchesInReverse : null, + addMatches ? postMatchesInReverse : null, + out var existingInitializer, cancellationToken)) + { return null; + } if (!CanReplaceWithCollectionExpression( state.SemanticModel, invocation, expressionType, isSingletonInstance: false, allowSemanticsChange, skipVerificationForReplacedNode: true, cancellationToken, out var changesSemantics)) @@ -143,15 +150,17 @@ private void AnalyzeMemberAccess(SyntaxNodeAnalysisContext context, INamedTypeSy return null; } - matchesInReverse.ReverseContents(); - return new AnalysisResult(existingInitializer, invocation, matchesInReverse.ToImmutable(), changesSemantics); + preMatchesInReverse.ReverseContents(); + postMatchesInReverse.ReverseContents(); + return new AnalysisResult(existingInitializer, invocation, preMatchesInReverse.ToImmutable(), postMatchesInReverse.ToImmutable(), changesSemantics); } private static bool AnalyzeInvocation( SourceText text, FluentState state, InvocationExpressionSyntax invocation, - ArrayBuilder>? matchesInReverse, + ArrayBuilder>? preMatchesInReverse, + ArrayBuilder>? postMatchesInReverse, out InitializerExpressionSyntax? existingInitializer, CancellationToken cancellationToken) { @@ -165,7 +174,7 @@ private static bool AnalyzeInvocation( // Topmost invocation must be a syntactic match for one of our collection manipulation forms. At the top level // we don't want to end with a linq method as that would be lazy, and a collection expression will eagerly // realize the collection. - if (!IsMatch(state, memberAccess, invocation, allowLinq: false, matchesInReverse, out var isAdditionMatch, cancellationToken)) + if (!IsMatch(state, memberAccess, invocation, allowLinq: false, postMatchesInReverse, out var isAdditionMatch, cancellationToken)) return false; // We don't want to offer this feature on top of some builder-type. They will commonly end with something like @@ -189,7 +198,7 @@ private static bool AnalyzeInvocation( // left hand side of the expression. In the inner expressions we can have things like `.Concat/.Append` // calls as the outer expressions will realize the collection. if (current is InvocationExpressionSyntax { Expression: MemberAccessExpressionSyntax currentMemberAccess } currentInvocation && - IsMatch(state, currentMemberAccess, currentInvocation, allowLinq: true, matchesInReverse, out _, cancellationToken)) + IsMatch(state, currentMemberAccess, currentInvocation, allowLinq: true, postMatchesInReverse, out _, cancellationToken)) { copiedData = true; stack.Push(currentMemberAccess.Expression); @@ -232,6 +241,8 @@ private static bool AnalyzeInvocation( return false; } + existingInitializer = objectCreation.Initializer; + if (!IsLegalInitializer(objectCreation.Initializer)) return false; @@ -251,23 +262,14 @@ private static bool AnalyzeInvocation( return false; } - if (matchesInReverse != null) - { - // Need to push any initializer values to the matchesInReverse first, as they need to execute prior - // to the argument to the object creation itself executing. - - if (objectCreation.Initializer != null) - AddArgumentsInReverse(matchesInReverse, ArgumentList(SeparatedList(objectCreation.Initializer.Expressions.Select(Argument))).Arguments, useSpread: false); - - AddArgumentsInReverse(matchesInReverse, objectCreation.ArgumentList.Arguments, useSpread: true); - } - + // Add the arguments to the pre-matches. They will execute before the initializer values are added. + if (objectCreation.Initializer != null) + AddArgumentsInReverse(preMatchesInReverse, ArgumentList(SeparatedList(objectCreation.Initializer.Expressions.Select(Argument))).Arguments, useSpread: false); return true; } else if (objectCreation.ArgumentList is null or { Arguments.Count: 0 }) { // Otherwise, we have to have an empty argument list. - existingInitializer = objectCreation.Initializer; return true; } @@ -281,7 +283,7 @@ private static bool AnalyzeInvocation( if (!IsListLike(current)) return false; - AddArgumentsInReverse(matchesInReverse, GetArguments(currentInvocationExpression, unwrapArgument), useSpread: false); + AddArgumentsInReverse(postMatchesInReverse, GetArguments(currentInvocationExpression, unwrapArgument), useSpread: false); return true; } @@ -310,13 +312,13 @@ private static bool AnalyzeInvocation( void AddFinalMatch(ExpressionSyntax expression) { - if (matchesInReverse is null) + if (postMatchesInReverse is null) return; // We're only adding one item to the final collection. So we're ending up with `[.. ]`. If this // originally was wrapped over multiple lines in a fluent fashion, and we're down to just a single wrapped // line, then unwrap. - if (matchesInReverse.Count == 0 && + if (postMatchesInReverse.Count == 0 && expression is InvocationExpressionSyntax { Expression: MemberAccessExpressionSyntax memberAccess } innerInvocation && text.Lines.GetLineFromPosition(expression.SpanStart).LineNumber + 1 == text.Lines.GetLineFromPosition(expression.Span.End).LineNumber && memberAccess.Expression.GetTrailingTrivia() @@ -325,7 +327,7 @@ void AddFinalMatch(ExpressionSyntax expression) .All(static t => t.IsWhitespaceOrEndOfLine())) { // Remove any whitespace around the `.`, making the singly-wrapped fluent expression into a single line. - matchesInReverse.Add(new CollectionExpressionMatch( + postMatchesInReverse.Add(new CollectionExpressionMatch( Argument(innerInvocation.WithExpression( memberAccess.Update( memberAccess.Expression.WithoutTrailingTrivia(), @@ -335,7 +337,7 @@ void AddFinalMatch(ExpressionSyntax expression) return; } - matchesInReverse.Add(new CollectionExpressionMatch(Argument(expression), UseSpread: true)); + postMatchesInReverse.Add(new CollectionExpressionMatch(Argument(expression), UseSpread: true)); } // We only want to offer this feature when the original collection was list-like (as opposed to being something @@ -501,12 +503,15 @@ static bool HasAnySuffix(string name) /// help determine the best collection expression final syntax. /// The location of the code like builder.ToImmutable() that will actually be /// replaced with the collection expression - /// The arguments being added to the collection that will be converted into elements in the - /// final collection expression. + /// The arguments being added to the collection that will be converted into elements in + /// the final collection expression *before* the existing initializer elements. + /// The arguments being added to the collection that will be converted into elements in + /// the final collection expression *after* the existing initializer elements. public readonly record struct AnalysisResult( // Location DiagnosticLocation, InitializerExpressionSyntax? ExistingInitializer, InvocationExpressionSyntax CreationExpression, - ImmutableArray> Matches, + ImmutableArray> PreMatches, + ImmutableArray> PostMatches, bool ChangesSemantics); } diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForFluentCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForFluentCodeFixProvider.cs index 8d6639c8ddc19..51b0069f0e350 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForFluentCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForFluentCodeFixProvider.cs @@ -69,7 +69,8 @@ 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 = await GetArgumentsAsync(document, analysisResult.Matches, cancellationToken).ConfigureAwait(false); + var allMatches = analysisResult.PreMatches.Concat(analysisResult.PostMatches); + var arguments = await GetArgumentsAsync(document, allMatches, cancellationToken).ConfigureAwait(false); var argumentListTrailingTrivia = analysisResult.ExistingInitializer is null ? default @@ -86,13 +87,14 @@ protected override async Task FixAsync( semanticDocument.Root.ReplaceNode(invocationExpression, dummyObjectCreation), cancellationToken).ConfigureAwait(false); dummyObjectCreation = (ImplicitObjectCreationExpressionSyntax)newSemanticDocument.Root.GetAnnotatedNodes(dummyObjectAnnotation).Single(); - var matches = CreateMatches(dummyObjectCreation.ArgumentList.Arguments, analysisResult.Matches); + var preMatches = CreateMatches(dummyObjectCreation.ArgumentList.Arguments, analysisResult.PreMatches, index: 0); + var postMatches = CreateMatches(dummyObjectCreation.ArgumentList.Arguments, analysisResult.PostMatches, index: preMatches.Length); var collectionExpression = await CreateCollectionExpressionAsync( newSemanticDocument.Document, dummyObjectCreation, - preMatches: [], - matches, + preMatches, + postMatches, static o => o.Initializer, static (o, i) => o.WithInitializer(i), cancellationToken).ConfigureAwait(false); @@ -101,16 +103,15 @@ protected override async Task FixAsync( static ImmutableArray> CreateMatches( SeparatedSyntaxList arguments, - ImmutableArray> matches) + ImmutableArray> matches, + int index) { - Contract.ThrowIfTrue(arguments.Count != matches.Length); - using var result = TemporaryArray>.Empty; - for (int i = 0, n = arguments.Count; i < n; i++) + for (int i = 0, n = matches.Length; i < n; i++) { - var argument = arguments[i]; var match = matches[i]; + var argument = arguments[i + index]; // If we're going to spread a collection expression, just take the values *within* that collection expression // and make them arguments to the collection expression we're creating. From dea5c817e24fe79283213842ac7788ade934fa03 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 27 Sep 2024 12:47:38 -0700 Subject: [PATCH 17/25] Fix --- ...llectionInitializerCodeFixProvider_CollectionExpression.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionExpression.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionExpression.cs index 910494b58f1e7..fddc4090457d2 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionExpression.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionExpression.cs @@ -27,8 +27,8 @@ private static Task CreateCollectionExpressionAsync( return CSharpCollectionExpressionRewriter.CreateCollectionExpressionAsync( document, objectCreation, - preMatches.Concat(postMatches).SelectAsArray(m => new CollectionExpressionMatch(m.StatementOrExpression, m.UseSpread)), - postMatches.Concat(postMatches).SelectAsArray(m => new CollectionExpressionMatch(m.StatementOrExpression, m.UseSpread)), + preMatches.SelectAsArray(m => new CollectionExpressionMatch(m.StatementOrExpression, m.UseSpread)), + postMatches.SelectAsArray(m => new CollectionExpressionMatch(m.StatementOrExpression, m.UseSpread)), static objectCreation => objectCreation.Initializer, static (objectCreation, initializer) => objectCreation.WithInitializer(initializer), cancellationToken); From 072425f3c53d7a07c5ee01fd277b184f1e5ec5e3 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 27 Sep 2024 12:53:17 -0700 Subject: [PATCH 18/25] in progress --- .../CSharpCollectionExpressionRewriter.cs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpCollectionExpressionRewriter.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpCollectionExpressionRewriter.cs index 93f1635be0fe6..e21a8c94a296d 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpCollectionExpressionRewriter.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpCollectionExpressionRewriter.cs @@ -123,8 +123,8 @@ CollectionExpressionSyntax CreateCollectionExpressionWithoutExistingElements() // now create the elements, following that indentation preference. using var _ = ArrayBuilder.GetInstance(out var nodesAndTokens); - CreateAndAddElements(preMatches, nodesAndTokens, preferredIndentation: elementIndentation, forceTrailingComma: true); - CreateAndAddElements(postMatches, nodesAndTokens, preferredIndentation: elementIndentation, forceTrailingComma: true); + CreateAndAddElements(preMatches, nodesAndTokens, preferredIndentation: elementIndentation, forceTrailingComma: true, moreToCome: true); + CreateAndAddElements(postMatches, nodesAndTokens, preferredIndentation: elementIndentation, forceTrailingComma: true, moreToCome: false); // Add a newline between the last element and the close bracket if we don't already have one. if (nodesAndTokens.Count > 0 && nodesAndTokens.Last().GetTrailingTrivia() is [.., (kind: not SyntaxKind.EndOfLineTrivia)]) @@ -146,8 +146,8 @@ CollectionExpressionSyntax CreateCollectionExpressionWithoutExistingElements() // fresh collection expression, and do a wholesale replacement of the original object creation // expression with it. using var _ = ArrayBuilder.GetInstance(out var nodesAndTokens); - CreateAndAddElements(preMatches, nodesAndTokens, preferredIndentation: null, forceTrailingComma: false); - CreateAndAddElements(postMatches, nodesAndTokens, preferredIndentation: null, forceTrailingComma: false); + CreateAndAddElements(preMatches, nodesAndTokens, preferredIndentation: null, forceTrailingComma: false, moreToCome: true); + CreateAndAddElements(postMatches, nodesAndTokens, preferredIndentation: null, forceTrailingComma: false, moreToCome: false); // Remove any trailing whitespace from the last element/comma and the final close bracket. if (nodesAndTokens.Count > 0) @@ -344,7 +344,8 @@ void CreateAndAddElements( ImmutableArray> matches, ArrayBuilder nodesAndTokens, string? preferredIndentation, - bool forceTrailingComma) + bool forceTrailingComma, + bool moreToCome) { // If there's no requested indentation, then we want to produce the sequence as: `a, b, c, d`. So just // a space after any comma. If there is desired indentation for an element, then we always follow a comma @@ -360,7 +361,7 @@ void CreateAndAddElements( } if (matches.Length > 0 && forceTrailingComma) - AddCommaIfMissing(last: true); + AddCommaIfMissing(last: !moreToCome); return; @@ -397,7 +398,7 @@ CollectionExpressionSyntax AddMatchesToExistingNonEmptyCollectionExpression( using var _ = ArrayBuilder.GetInstance(out var nodesAndTokens); // Add any pre-items before the initializer items. - CreateAndAddElements(preMatches, nodesAndTokens, preferredIndentation, forceTrailingComma: true); + CreateAndAddElements(preMatches, nodesAndTokens, preferredIndentation, forceTrailingComma: true, moreToCome: true); // Now add all the initializer items. nodesAndTokens.AddRange(initialCollectionExpression.Elements.GetWithSeparators()); @@ -425,7 +426,8 @@ CollectionExpressionSyntax AddMatchesToExistingNonEmptyCollectionExpression( // maintain. CreateAndAddElements( postMatches, nodesAndTokens, preferredIndentation, - forceTrailingComma: preferredIndentation != null && trailingComma == default); + forceTrailingComma: preferredIndentation != null && trailingComma == default, + moreToCome: false); if (trailingComma != default) { From 0bcb9972198d67aaf767b972c62c8023bce6ef0c Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 27 Sep 2024 12:56:42 -0700 Subject: [PATCH 19/25] Fix --- ...CSharpUseCollectionExpressionForFluentDiagnosticAnalyzer.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForFluentDiagnosticAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForFluentDiagnosticAnalyzer.cs index 299adc0270329..f2440322c30cc 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForFluentDiagnosticAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForFluentDiagnosticAnalyzer.cs @@ -263,8 +263,7 @@ private static bool AnalyzeInvocation( } // Add the arguments to the pre-matches. They will execute before the initializer values are added. - if (objectCreation.Initializer != null) - AddArgumentsInReverse(preMatchesInReverse, ArgumentList(SeparatedList(objectCreation.Initializer.Expressions.Select(Argument))).Arguments, useSpread: false); + AddArgumentsInReverse(preMatchesInReverse, objectCreation.ArgumentList.Arguments, useSpread: false); return true; } else if (objectCreation.ArgumentList is null or { Arguments.Count: 0 }) From 24611ad0b571371aa27a04dc4b68cb6207db310e Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 27 Sep 2024 12:57:46 -0700 Subject: [PATCH 20/25] Fix --- .../CSharpUseCollectionExpressionForFluentDiagnosticAnalyzer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForFluentDiagnosticAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForFluentDiagnosticAnalyzer.cs index f2440322c30cc..40f160b32d483 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForFluentDiagnosticAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForFluentDiagnosticAnalyzer.cs @@ -263,7 +263,7 @@ private static bool AnalyzeInvocation( } // Add the arguments to the pre-matches. They will execute before the initializer values are added. - AddArgumentsInReverse(preMatchesInReverse, objectCreation.ArgumentList.Arguments, useSpread: false); + AddArgumentsInReverse(preMatchesInReverse, objectCreation.ArgumentList.Arguments, useSpread: true); return true; } else if (objectCreation.ArgumentList is null or { Arguments.Count: 0 }) From 80538fa562b7979df4ebf35ad4bdc8e7fbb131a2 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 27 Sep 2024 13:00:31 -0700 Subject: [PATCH 21/25] Fix test --- .../UseCollectionExpressionForFluentTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForFluentTests.cs b/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForFluentTests.cs index 64b07e979a271..b3413c13b4dba 100644 --- a/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForFluentTests.cs +++ b/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForFluentTests.cs @@ -3058,7 +3058,7 @@ class C { void M(int[] values, int[] x) { - List list = [.. values, 1, 2, 3, .. x]; + List list = [.. values, 1, 2, 3, .. x]; } } """, From c7304ec12c5e3f32b763810405f2b89aaa9128d4 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 27 Sep 2024 13:04:11 -0700 Subject: [PATCH 22/25] Fix test --- ...onInitializerTests_CollectionExpression.cs | 35 ++++++++++++++++--- 1 file changed, 30 insertions(+), 5 deletions(-) diff --git a/src/Analyzers/CSharp/Tests/UseCollectionInitializer/UseCollectionInitializerTests_CollectionExpression.cs b/src/Analyzers/CSharp/Tests/UseCollectionInitializer/UseCollectionInitializerTests_CollectionExpression.cs index 292869e914530..7a19fa362e961 100644 --- a/src/Analyzers/CSharp/Tests/UseCollectionInitializer/UseCollectionInitializerTests_CollectionExpression.cs +++ b/src/Analyzers/CSharp/Tests/UseCollectionInitializer/UseCollectionInitializerTests_CollectionExpression.cs @@ -2365,8 +2365,7 @@ static void M() [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/17823")] public async Task TestWhenReferencedInInitializer_LocalVar2() { - await TestMissingInRegularAndScriptAsync( - """ + await TestInRegularAndScriptAsync(""" using System.Collections.Generic; using System.Linq; @@ -2374,7 +2373,19 @@ class C { void M() { - List t = new List(new int[] { 1, 2, 3 }); + List t = [|new|] List(new int[] { 1, 2, 3 }); + t.Add(t.Min() - 1); + } + } + """, """ + using System.Collections.Generic; + using System.Linq; + + class C + { + void M() + { + List t = [.. new int[] { 1, 2, 3 }]; t.Add(t.Min() - 1); } } @@ -2420,7 +2431,7 @@ static void M() [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/18260")] public async Task TestWhenReferencedInInitializer_Assignment2() { - await TestMissingInRegularAndScriptAsync( + await TestInRegularAndScriptAsync( """ using System.Collections.Generic; using System.Linq; @@ -2430,7 +2441,21 @@ class C void M() { List t = null; - t = new List(new int[] { 1, 2, 3 }); + t = [|new|] List(new int[] { 1, 2, 3 }); + t.Add(t.Min() - 1); + } + } + """, + """ + using System.Collections.Generic; + using System.Linq; + + class C + { + void M() + { + List t = null; + t = [.. new int[] { 1, 2, 3 }]; t.Add(t.Min() - 1); } } From 0ae4ccb2c4b2618a252445f2854c3817249d189f Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 27 Sep 2024 13:28:05 -0700 Subject: [PATCH 23/25] unitfy --- .../Analyzers/CSharpAnalyzers.projitems | 1 - .../CollectionExpressionMatch.cs | 12 ----------- .../Core/Analyzers/Analyzers.projitems | 1 + .../CollectionExpressionMatch.cs | 19 +++++++++++++++++ ...bstractUseCollectionInitializerAnalyzer.cs | 15 ++++++------- ...CollectionInitializerDiagnosticAnalyzer.cs | 21 +++++-------------- .../UpdateExpressionState.cs | 9 ++++---- .../UseCollectionInitializerHelpers.cs | 5 +++-- ...UseCollectionInitializerCodeFixProvider.cs | 11 +++++----- 9 files changed, 47 insertions(+), 47 deletions(-) delete mode 100644 src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CollectionExpressionMatch.cs create mode 100644 src/Analyzers/Core/Analyzers/UseCollectionExpression/CollectionExpressionMatch.cs diff --git a/src/Analyzers/CSharp/Analyzers/CSharpAnalyzers.projitems b/src/Analyzers/CSharp/Analyzers/CSharpAnalyzers.projitems index a13697111e9c7..5f550c427c2b5 100644 --- a/src/Analyzers/CSharp/Analyzers/CSharpAnalyzers.projitems +++ b/src/Analyzers/CSharp/Analyzers/CSharpAnalyzers.projitems @@ -82,7 +82,6 @@ - diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CollectionExpressionMatch.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CollectionExpressionMatch.cs deleted file mode 100644 index 273dc0a553097..0000000000000 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CollectionExpressionMatch.cs +++ /dev/null @@ -1,12 +0,0 @@ -// 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 Microsoft.CodeAnalysis.UseCollectionInitializer; - -namespace Microsoft.CodeAnalysis.CSharp.UseCollectionExpression; - -/// -internal readonly record struct CollectionExpressionMatch( - TMatchNode Node, - bool UseSpread) where TMatchNode : SyntaxNode; diff --git a/src/Analyzers/Core/Analyzers/Analyzers.projitems b/src/Analyzers/Core/Analyzers/Analyzers.projitems index cd595743cd676..ea81bd2ba5653 100644 --- a/src/Analyzers/Core/Analyzers/Analyzers.projitems +++ b/src/Analyzers/Core/Analyzers/Analyzers.projitems @@ -81,6 +81,7 @@ + diff --git a/src/Analyzers/Core/Analyzers/UseCollectionExpression/CollectionExpressionMatch.cs b/src/Analyzers/Core/Analyzers/UseCollectionExpression/CollectionExpressionMatch.cs new file mode 100644 index 0000000000000..db6671311ed8f --- /dev/null +++ b/src/Analyzers/Core/Analyzers/UseCollectionExpression/CollectionExpressionMatch.cs @@ -0,0 +1,19 @@ +// 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. + +namespace Microsoft.CodeAnalysis.UseCollectionExpression; + +/// +/// Represents statements following an initializer that should be converted into collection-initializer/expression +/// elements. +/// +/// The statement that follows that contains the values to add to the new collection-initializer or +/// collection-expression. Or the expression directly to add. +/// Whether or not a spread (.. x) element should be created for this statement. This +/// is needed as the statement could be cases like expr.Add(x) vs. expr.AddRange(x). This property +/// indicates that the latter should become a spread, without the consumer having to reexamine the statement to see +/// what form it is. +internal readonly record struct CollectionMatch( + TMatchNode Node, + bool UseSpread) where TMatchNode : SyntaxNode; diff --git a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerAnalyzer.cs b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerAnalyzer.cs index 28be1664b0303..9eafc5d7ab056 100644 --- a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerAnalyzer.cs +++ b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerAnalyzer.cs @@ -8,6 +8,7 @@ using System.Threading; using Microsoft.CodeAnalysis.LanguageService; using Microsoft.CodeAnalysis.PooledObjects; +using Microsoft.CodeAnalysis.UseCollectionExpression; using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.UseCollectionInitializer; @@ -27,7 +28,7 @@ internal abstract class AbstractUseCollectionInitializerAnalyzer< TObjectCreationExpressionSyntax, TLocalDeclarationStatementSyntax, TVariableDeclaratorSyntax, - Match, TAnalyzer> + CollectionMatch, TAnalyzer> where TExpressionSyntax : SyntaxNode where TStatementSyntax : SyntaxNode where TObjectCreationExpressionSyntax : TExpressionSyntax @@ -48,13 +49,13 @@ internal abstract class AbstractUseCollectionInitializerAnalyzer< TAnalyzer>, new() { public readonly record struct AnalysisResult( - ImmutableArray PreMatches, - ImmutableArray PostMatches); + ImmutableArray> PreMatches, + ImmutableArray> PostMatches); protected abstract bool IsComplexElementInitializer(SyntaxNode expression); protected abstract bool HasExistingInvalidInitializerForCollection(); protected abstract bool AnalyzeMatchesAndCollectionConstructorForCollectionExpression( - ArrayBuilder preMatches, ArrayBuilder postMatches, CancellationToken cancellationToken); + ArrayBuilder> preMatches, ArrayBuilder> postMatches, CancellationToken cancellationToken); protected abstract IUpdateExpressionSyntaxHelper SyntaxHelper { get; } @@ -92,7 +93,7 @@ public AnalysisResult Analyze( } protected sealed override bool TryAddMatches( - ArrayBuilder preMatches, ArrayBuilder postMatches, CancellationToken cancellationToken) + ArrayBuilder> preMatches, ArrayBuilder> postMatches, CancellationToken cancellationToken) { var seenInvocation = false; var seenIndexAssignment = false; @@ -136,7 +137,7 @@ protected sealed override bool TryAddMatches( return true; } - private Match? TryAnalyzeStatement( + private CollectionMatch? TryAnalyzeStatement( TStatementSyntax statement, ref bool seenInvocation, ref bool seenIndexAssignment, CancellationToken cancellationToken) { return _analyzeForCollectionExpression @@ -144,7 +145,7 @@ protected sealed override bool TryAddMatches( : TryAnalyzeStatementForCollectionInitializer(statement, ref seenInvocation, ref seenIndexAssignment, cancellationToken); } - private Match? TryAnalyzeStatementForCollectionInitializer( + private CollectionMatch? TryAnalyzeStatementForCollectionInitializer( TStatementSyntax statement, ref bool seenInvocation, ref bool seenIndexAssignment, CancellationToken cancellationToken) { // At least one of these has to be false. diff --git a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerDiagnosticAnalyzer.cs b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerDiagnosticAnalyzer.cs index fa84c88ff85a6..6b84494d3acfc 100644 --- a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerDiagnosticAnalyzer.cs +++ b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerDiagnosticAnalyzer.cs @@ -11,21 +11,10 @@ using Microsoft.CodeAnalysis.Shared.CodeStyle; using Microsoft.CodeAnalysis.Shared.Collections; using Microsoft.CodeAnalysis.Shared.Extensions; +using Microsoft.CodeAnalysis.UseCollectionExpression; namespace Microsoft.CodeAnalysis.UseCollectionInitializer; -/// -/// Represents statements following an object initializer that should be converted into -/// collection-initializer/expression elements. -/// -/// The statement that follows that contains the values to add to the new -/// collection-initializer or collection-expression. Or the expression directly to add. -/// Whether or not a spread (.. x) element should be created for this statement. This -/// is needed as the statement could be cases like expr.Add(x) vs. expr.AddRange(x). This property -/// indicates that the latter should become a spread, without the consumer having to reexamine the statement to see -/// what form it is. -internal readonly record struct Match(SyntaxNode StatementOrExpression, bool UseSpread); - internal abstract partial class AbstractUseCollectionInitializerDiagnosticAnalyzer< TSyntaxKind, TExpressionSyntax, @@ -180,7 +169,7 @@ private void AnalyzeNode( var nodes = containingStatement is null ? ImmutableArray.Empty : [containingStatement]; - nodes = nodes.AddRange(matches.Select(static m => m.StatementOrExpression)); + nodes = nodes.AddRange(matches.Select(static m => m.Node)); if (syntaxFacts.ContainsInterleavedDirective(nodes, cancellationToken)) return; @@ -203,7 +192,7 @@ private void AnalyzeNode( return; - (ImmutableArray matches, bool shouldUseCollectionExpression, bool changesSemantics)? GetCollectionInitializerMatches() + (ImmutableArray> matches, bool shouldUseCollectionExpression, bool changesSemantics)? GetCollectionInitializerMatches() { if (containingStatement is null) return null; @@ -220,7 +209,7 @@ private void AnalyzeNode( return (matches, shouldUseCollectionExpression: false, changesSemantics: false); } - (ImmutableArray matches, bool shouldUseCollectionExpression, bool changesSemantics)? GetCollectionExpressionMatches() + (ImmutableArray> matches, bool shouldUseCollectionExpression, bool changesSemantics)? GetCollectionExpressionMatches() { if (preferExpressionOption.Value == CollectionExpressionPreference.Never) return null; @@ -246,7 +235,7 @@ private void AnalyzeNode( private void FadeOutCode( SyntaxNodeAnalysisContext context, - ImmutableArray matches, + ImmutableArray> matches, ImmutableArray locations, ImmutableDictionary? properties) { diff --git a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/UpdateExpressionState.cs b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/UpdateExpressionState.cs index 01c853c8dd27d..48a993be1f27a 100644 --- a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/UpdateExpressionState.cs +++ b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/UpdateExpressionState.cs @@ -9,6 +9,7 @@ using System.Threading; using Microsoft.CodeAnalysis.LanguageService; using Microsoft.CodeAnalysis.Shared.Extensions; +using Microsoft.CodeAnalysis.UseCollectionExpression; using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.UseCollectionInitializer; @@ -337,7 +338,7 @@ private bool TryAnalyzeInvocation( /// includes calls to .Add and .AddRange, as well as foreach statements that update the /// collection, and if statements that conditionally add items to the collection-expression. /// - public Match? TryAnalyzeStatementForCollectionExpression( + public CollectionMatch? TryAnalyzeStatementForCollectionExpression( IUpdateExpressionSyntaxHelper syntaxHelper, TStatementSyntax statement, CancellationToken cancellationToken) @@ -355,7 +356,7 @@ private bool TryAnalyzeInvocation( return null; - Match? TryAnalyzeExpressionStatement(TStatementSyntax expressionStatement) + CollectionMatch? TryAnalyzeExpressionStatement(TStatementSyntax expressionStatement) { var expression = (TExpressionSyntax)@this.SyntaxFacts.GetExpressionOfExpressionStatement(expressionStatement); @@ -369,7 +370,7 @@ private bool TryAnalyzeInvocation( return null; } - Match? TryAnalyzeForeachStatement(TStatementSyntax foreachStatement) + CollectionMatch? TryAnalyzeForeachStatement(TStatementSyntax foreachStatement) { syntaxHelper.GetPartsOfForeachStatement(foreachStatement, out var awaitKeyword, out var identifier, out _, out var foreachStatements); if (awaitKeyword != default) @@ -399,7 +400,7 @@ private bool TryAnalyzeInvocation( return null; } - Match? TryAnalyzeIfStatement(TStatementSyntax ifStatement) + CollectionMatch? TryAnalyzeIfStatement(TStatementSyntax ifStatement) { // look for the form: // diff --git a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/UseCollectionInitializerHelpers.cs b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/UseCollectionInitializerHelpers.cs index a7a8938d83cb3..48c478b9b0ff1 100644 --- a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/UseCollectionInitializerHelpers.cs +++ b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/UseCollectionInitializerHelpers.cs @@ -7,6 +7,7 @@ using Microsoft.CodeAnalysis.LanguageService; using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.Text; +using Microsoft.CodeAnalysis.UseCollectionExpression; namespace Microsoft.CodeAnalysis.UseCollectionInitializer; @@ -20,9 +21,9 @@ internal static class UseCollectionInitializerHelpers public static ImmutableArray GetLocationsToFade( ISyntaxFacts syntaxFacts, - Match matchInfo) + CollectionMatch matchInfo) { - var match = matchInfo.StatementOrExpression; + var match = matchInfo.Node; var syntaxTree = match.SyntaxTree; if (syntaxFacts.IsExpressionStatement(match)) { diff --git a/src/Analyzers/Core/CodeFixes/UseCollectionInitializer/AbstractUseCollectionInitializerCodeFixProvider.cs b/src/Analyzers/Core/CodeFixes/UseCollectionInitializer/AbstractUseCollectionInitializerCodeFixProvider.cs index 41e31c87aa052..d1c1ed93b6077 100644 --- a/src/Analyzers/Core/CodeFixes/UseCollectionInitializer/AbstractUseCollectionInitializerCodeFixProvider.cs +++ b/src/Analyzers/Core/CodeFixes/UseCollectionInitializer/AbstractUseCollectionInitializerCodeFixProvider.cs @@ -54,7 +54,8 @@ public sealed override ImmutableArray FixableDiagnosticIds protected abstract Task<(SyntaxNode oldNode, SyntaxNode newNode)> GetReplacementNodesAsync( Document document, TObjectCreationExpressionSyntax objectCreation, bool useCollectionExpression, - ImmutableArray preMatches, ImmutableArray postMatches, CancellationToken cancellationToken); + ImmutableArray> preMatches, + ImmutableArray> postMatches, CancellationToken cancellationToken); protected sealed override async Task FixAsync( Document document, @@ -85,10 +86,10 @@ protected sealed override async Task FixAsync( document, objectCreation, useCollectionExpression, preMatches, postMatches, cancellationToken).ConfigureAwait(false); editor.ReplaceNode(oldNode, newNode); + + // We only need to remove the post-matches. The pre-matches are the arguments in teh object creation, which + // itself got replaced above. foreach (var match in postMatches) - { - if (match.StatementOrExpression is TStatementSyntax statement) - editor.RemoveNode(statement, SyntaxRemoveOptions.KeepUnbalancedDirectives); - } + editor.RemoveNode(match.Node, SyntaxRemoveOptions.KeepUnbalancedDirectives); } } From 9fbea8d2096e316f31249c6ce2b564cf4935cfa3 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 27 Sep 2024 13:31:55 -0700 Subject: [PATCH 24/25] renames --- ...ionExpressionForArrayDiagnosticAnalyzer.cs | 5 +++-- ...nExpressionForBuilderDiagnosticAnalyzer.cs | 5 +++-- ...onExpressionForFluentDiagnosticAnalyzer.cs | 21 ++++++++++--------- ...pressionForStackAllocDiagnosticAnalyzer.cs | 3 ++- .../UseCollectionExpressionHelpers.cs | 5 +++-- .../CSharpUseCollectionInitializerAnalyzer.cs | 7 ++++--- .../CSharpCollectionExpressionRewriter.cs | 13 ++++++------ ...isualBasicCollectionInitializerAnalyzer.vb | 5 +++-- ...UseCollectionInitializerCodeFixProvider.vb | 13 ++++++------ 9 files changed, 43 insertions(+), 34 deletions(-) diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForArrayDiagnosticAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForArrayDiagnosticAnalyzer.cs index eacc1e33f9758..3e1c756b5b5d3 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForArrayDiagnosticAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForArrayDiagnosticAnalyzer.cs @@ -13,6 +13,7 @@ using Microsoft.CodeAnalysis.Shared.CodeStyle; using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.Text; +using Microsoft.CodeAnalysis.UseCollectionExpression; using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.CSharp.UseCollectionExpression; @@ -57,7 +58,7 @@ private void AnalyzeArrayCreationExpression(SyntaxNodeAnalysisContext context, I ReportArrayCreationDiagnostics(context, syntaxTree, option.Notification, arrayCreationExpression, changesSemantics); } - public static ImmutableArray> TryGetMatches( + public static ImmutableArray> TryGetMatches( SemanticModel semanticModel, ArrayCreationExpressionSyntax expression, CollectionExpressionSyntax replacementExpression, @@ -114,7 +115,7 @@ public static ImmutableArray> TryGetM return matches; } - public static ImmutableArray> TryGetMatches( + public static ImmutableArray> TryGetMatches( SemanticModel semanticModel, ImplicitArrayCreationExpressionSyntax expression, CollectionExpressionSyntax replacementExpression, diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForBuilderDiagnosticAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForBuilderDiagnosticAnalyzer.cs index 3a582e4a77d08..347115c0d6cb5 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForBuilderDiagnosticAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForBuilderDiagnosticAnalyzer.cs @@ -14,6 +14,7 @@ using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.PooledObjects; using Microsoft.CodeAnalysis.Shared.CodeStyle; +using Microsoft.CodeAnalysis.UseCollectionExpression; using Microsoft.CodeAnalysis.UseCollectionInitializer; using Roslyn.Utilities; @@ -159,7 +160,7 @@ void FadeOutCode(SyntaxNodeAnalysisContext context, AnalysisResult analysisResul identifier, initializedSymbol: semanticModel.GetDeclaredSymbol(declarator, cancellationToken)); - using var _ = ArrayBuilder.GetInstance(out var matches); + using var _ = ArrayBuilder>.GetInstance(out var matches); // Now walk all the statement after the local declaration. using var enumerator = state.GetSubsequentStatements().GetEnumerator(); @@ -249,6 +250,6 @@ public readonly record struct AnalysisResult( Location DiagnosticLocation, LocalDeclarationStatementSyntax LocalDeclarationStatement, InvocationExpressionSyntax CreationExpression, - ImmutableArray Matches, + ImmutableArray> Matches, bool ChangesSemantics); } diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForFluentDiagnosticAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForFluentDiagnosticAnalyzer.cs index 40f160b32d483..6d999c673739a 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForFluentDiagnosticAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForFluentDiagnosticAnalyzer.cs @@ -15,6 +15,7 @@ using Microsoft.CodeAnalysis.Shared.CodeStyle; using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.Text; +using Microsoft.CodeAnalysis.UseCollectionExpression; using Microsoft.CodeAnalysis.UseCollectionInitializer; using Roslyn.Utilities; @@ -133,8 +134,8 @@ private void AnalyzeMemberAccess(SyntaxNodeAnalysisContext context, INamedTypeSy { // Because we're recursing from top to bottom in the expression tree, we build up the matches in reverse. Right // before returning them, we'll reverse them again to get the proper order. - using var _1 = ArrayBuilder>.GetInstance(out var preMatchesInReverse); - using var _2 = ArrayBuilder>.GetInstance(out var postMatchesInReverse); + using var _1 = ArrayBuilder>.GetInstance(out var preMatchesInReverse); + using var _2 = ArrayBuilder>.GetInstance(out var postMatchesInReverse); if (!AnalyzeInvocation( text, state, invocation, addMatches ? preMatchesInReverse : null, @@ -159,8 +160,8 @@ private static bool AnalyzeInvocation( SourceText text, FluentState state, InvocationExpressionSyntax invocation, - ArrayBuilder>? preMatchesInReverse, - ArrayBuilder>? postMatchesInReverse, + ArrayBuilder>? preMatchesInReverse, + ArrayBuilder>? postMatchesInReverse, out InitializerExpressionSyntax? existingInitializer, CancellationToken cancellationToken) { @@ -326,7 +327,7 @@ void AddFinalMatch(ExpressionSyntax expression) .All(static t => t.IsWhitespaceOrEndOfLine())) { // Remove any whitespace around the `.`, making the singly-wrapped fluent expression into a single line. - postMatchesInReverse.Add(new CollectionExpressionMatch( + postMatchesInReverse.Add(new CollectionMatch( Argument(innerInvocation.WithExpression( memberAccess.Update( memberAccess.Expression.WithoutTrailingTrivia(), @@ -336,7 +337,7 @@ void AddFinalMatch(ExpressionSyntax expression) return; } - postMatchesInReverse.Add(new CollectionExpressionMatch(Argument(expression), UseSpread: true)); + postMatchesInReverse.Add(new CollectionMatch(Argument(expression), UseSpread: true)); } // We only want to offer this feature when the original collection was list-like (as opposed to being something @@ -399,7 +400,7 @@ static bool IsLegalInitializer(InitializerExpressionSyntax? initializer) } private static void AddArgumentsInReverse( - ArrayBuilder>? matchesInReverse, + ArrayBuilder>? matchesInReverse, SeparatedSyntaxList arguments, bool useSpread) { @@ -423,7 +424,7 @@ private static bool IsMatch( MemberAccessExpressionSyntax memberAccess, InvocationExpressionSyntax invocation, bool allowLinq, - ArrayBuilder>? matchesInReverse, + ArrayBuilder>? matchesInReverse, out bool isAdditionMatch, CancellationToken cancellationToken) { @@ -510,7 +511,7 @@ public readonly record struct AnalysisResult( // Location DiagnosticLocation, InitializerExpressionSyntax? ExistingInitializer, InvocationExpressionSyntax CreationExpression, - ImmutableArray> PreMatches, - ImmutableArray> PostMatches, + ImmutableArray> PreMatches, + ImmutableArray> PostMatches, bool ChangesSemantics); } diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocDiagnosticAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocDiagnosticAnalyzer.cs index 607899dd00c26..3fea21dfca3c3 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocDiagnosticAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocDiagnosticAnalyzer.cs @@ -9,6 +9,7 @@ using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Shared.CodeStyle; using Microsoft.CodeAnalysis.Text; +using Microsoft.CodeAnalysis.UseCollectionExpression; namespace Microsoft.CodeAnalysis.CSharp.UseCollectionExpression; @@ -119,7 +120,7 @@ private void AnalyzeExplicitStackAllocExpression(SyntaxNodeAnalysisContext conte additionalUnnecessaryLocations: additionalUnnecessaryLocations)); } - public static ImmutableArray> TryGetMatches( + public static ImmutableArray> TryGetMatches( SemanticModel semanticModel, StackAllocArrayCreationExpressionSyntax expression, INamedTypeSymbol? expressionType, diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/UseCollectionExpressionHelpers.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/UseCollectionExpressionHelpers.cs index bedba7efb662a..f7fab29e21d2e 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/UseCollectionExpressionHelpers.cs +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/UseCollectionExpressionHelpers.cs @@ -17,6 +17,7 @@ using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.Shared.Utilities; using Microsoft.CodeAnalysis.Text; +using Microsoft.CodeAnalysis.UseCollectionExpression; using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.CSharp.UseCollectionExpression; @@ -814,7 +815,7 @@ private static bool ShouldReplaceExistingExpressionEntirely( return false; } - public static ImmutableArray> TryGetMatches( + public static ImmutableArray> TryGetMatches( SemanticModel semanticModel, TArrayCreationExpressionSyntax expression, CollectionExpressionSyntax replacementExpression, @@ -835,7 +836,7 @@ public static ImmutableArray> TryGetM if (getType(expression) is not ArrayTypeSyntax { RankSpecifiers: [{ Sizes: [var size] }, ..] }) return default; - using var _ = ArrayBuilder>.GetInstance(out var matches); + using var _ = ArrayBuilder>.GetInstance(out var matches); var initializer = getInitializer(expression); if (size is OmittedArraySizeExpressionSyntax) diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerAnalyzer.cs index b3deba756f901..fabc39b46613b 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerAnalyzer.cs @@ -9,6 +9,7 @@ using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.PooledObjects; using Microsoft.CodeAnalysis.Shared.Extensions; +using Microsoft.CodeAnalysis.UseCollectionExpression; using Microsoft.CodeAnalysis.UseCollectionInitializer; namespace Microsoft.CodeAnalysis.CSharp.UseCollectionInitializer; @@ -45,8 +46,8 @@ protected override bool HasExistingInvalidInitializerForCollection() } protected override bool AnalyzeMatchesAndCollectionConstructorForCollectionExpression( - ArrayBuilder preMatches, - ArrayBuilder postMatches, + ArrayBuilder> preMatches, + ArrayBuilder> postMatches, CancellationToken cancellationToken) { // Constructor wasn't called with any arguments. Nothing to validate. @@ -92,7 +93,7 @@ protected override bool AnalyzeMatchesAndCollectionConstructorForCollectionExpre using var _1 = ArrayBuilder.GetInstance(out var spreadElements); foreach (var match in postMatches) { - switch (match.StatementOrExpression) + switch (match.Node) { case ExpressionStatementSyntax { Expression: InvocationExpressionSyntax invocation } expressionStatement: // x.AddRange(y). Have to make sure we see y.Count in the capacity list. diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpCollectionExpressionRewriter.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpCollectionExpressionRewriter.cs index e21a8c94a296d..23b878b52e152 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpCollectionExpressionRewriter.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpCollectionExpressionRewriter.cs @@ -20,6 +20,7 @@ using Microsoft.CodeAnalysis.PooledObjects; using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.Text; +using Microsoft.CodeAnalysis.UseCollectionExpression; using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.CSharp.UseCollectionExpression; @@ -36,8 +37,8 @@ internal static class CSharpCollectionExpressionRewriter public static async Task CreateCollectionExpressionAsync( Document workspaceDocument, TParentExpression expressionToReplace, - ImmutableArray> preMatches, - ImmutableArray> postMatches, + ImmutableArray> preMatches, + ImmutableArray> postMatches, Func getInitializer, Func withInitializer, CancellationToken cancellationToken) @@ -193,7 +194,7 @@ CollectionExpressionSyntax CreateCollectionExpressionWithoutExistingElements() } } - CollectionExpressionSyntax CreateSingleElementCollection(CollectionExpressionMatch match) + CollectionExpressionSyntax CreateSingleElementCollection(CollectionMatch match) { // Specialize when we're taking some expression (like x.y.ToArray()) and converting to a spreaded // collection expression. We just want to trivially make that `[.. x.y]` without any specialized @@ -341,7 +342,7 @@ SeparatedSyntaxList FixLeadingAndTrailingWhitespace( // Used to we can uniformly add the items correctly with the requested (but optional) indentation. And so that // commas are added properly to the sequence. void CreateAndAddElements( - ImmutableArray> matches, + ImmutableArray> matches, ArrayBuilder nodesAndTokens, string? preferredIndentation, bool forceTrailingComma, @@ -456,7 +457,7 @@ static CollectionElementSyntax CreateCollectionElement( } IEnumerable CreateElements( - CollectionExpressionMatch match, string? preferredIndentation) + CollectionMatch match, string? preferredIndentation) { var node = match.Node; @@ -744,7 +745,7 @@ bool MakeMultiLineCollectionExpression() return totalLength > wrappingLength; - bool CheckForMultiLine(ImmutableArray> matches) + bool CheckForMultiLine(ImmutableArray> matches) { foreach (var (node, _) in matches) { diff --git a/src/Analyzers/VisualBasic/Analyzers/UseCollectionInitializer/VisualBasicCollectionInitializerAnalyzer.vb b/src/Analyzers/VisualBasic/Analyzers/UseCollectionInitializer/VisualBasicCollectionInitializerAnalyzer.vb index 5479650b6aa05..f93006673927f 100644 --- a/src/Analyzers/VisualBasic/Analyzers/UseCollectionInitializer/VisualBasicCollectionInitializerAnalyzer.vb +++ b/src/Analyzers/VisualBasic/Analyzers/UseCollectionInitializer/VisualBasicCollectionInitializerAnalyzer.vb @@ -4,6 +4,7 @@ Imports System.Threading Imports Microsoft.CodeAnalysis.PooledObjects +Imports Microsoft.CodeAnalysis.UseCollectionExpression Imports Microsoft.CodeAnalysis.UseCollectionInitializer Imports Microsoft.CodeAnalysis.VisualBasic.Syntax @@ -38,8 +39,8 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.UseCollectionInitializer End Function Protected Overrides Function AnalyzeMatchesAndCollectionConstructorForCollectionExpression( - preMatches As ArrayBuilder(Of Match), - postMatches As ArrayBuilder(Of Match), + preMatches As ArrayBuilder(Of CollectionMatch(Of SyntaxNode)), + postMatches As ArrayBuilder(Of CollectionMatch(Of SyntaxNode)), cancellationToken As CancellationToken) As Boolean ' Only called for collection expressions, which VB does not support Throw ExceptionUtilities.Unreachable() diff --git a/src/Analyzers/VisualBasic/CodeFixes/UseCollectionInitializer/VisualBasicUseCollectionInitializerCodeFixProvider.vb b/src/Analyzers/VisualBasic/CodeFixes/UseCollectionInitializer/VisualBasicUseCollectionInitializerCodeFixProvider.vb index fe24ab3de3a44..c646b08b34da3 100644 --- a/src/Analyzers/VisualBasic/CodeFixes/UseCollectionInitializer/VisualBasicUseCollectionInitializerCodeFixProvider.vb +++ b/src/Analyzers/VisualBasic/CodeFixes/UseCollectionInitializer/VisualBasicUseCollectionInitializerCodeFixProvider.vb @@ -9,6 +9,7 @@ Imports System.Threading Imports Microsoft.CodeAnalysis.CodeFixes Imports Microsoft.CodeAnalysis.Formatting Imports Microsoft.CodeAnalysis.PooledObjects +Imports Microsoft.CodeAnalysis.UseCollectionExpression Imports Microsoft.CodeAnalysis.UseCollectionInitializer Imports Microsoft.CodeAnalysis.VisualBasic.Syntax Imports Microsoft.CodeAnalysis.VisualBasic.UseObjectInitializer @@ -41,8 +42,8 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.UseCollectionInitializer document As Document, objectCreation As ObjectCreationExpressionSyntax, useCollectionExpression As Boolean, - preMatches As ImmutableArray(Of Match), - postMatches As ImmutableArray(Of Match), + preMatches As ImmutableArray(Of CollectionMatch(Of SyntaxNode)), + postMatches As ImmutableArray(Of CollectionMatch(Of SyntaxNode)), cancellationToken As CancellationToken) As Task(Of (SyntaxNode, SyntaxNode)) Contract.ThrowIfFalse(preMatches.IsEmpty) Contract.ThrowIfTrue(useCollectionExpression, "VB does not support collection expressions") @@ -57,7 +58,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.UseCollectionInitializer totalTrivia.Add(SyntaxFactory.ElasticMarker) For Each match In postMatches - For Each trivia In match.StatementOrExpression.GetLeadingTrivia() + For Each trivia In match.Node.GetLeadingTrivia() If trivia.Kind = SyntaxKind.CommentTrivia Then totalTrivia.Add(trivia) totalTrivia.Add(SyntaxFactory.ElasticMarker) @@ -71,7 +72,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.UseCollectionInitializer Private Shared Function GetNewObjectCreation( objectCreation As ObjectCreationExpressionSyntax, - matches As ImmutableArray(Of Match)) As ObjectCreationExpressionSyntax + matches As ImmutableArray(Of CollectionMatch(Of SyntaxNode))) As ObjectCreationExpressionSyntax Return UseInitializerHelpers.GetNewObjectCreation( objectCreation, @@ -81,13 +82,13 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.UseCollectionInitializer Private Shared Function CreateCollectionInitializer( objectCreation As ObjectCreationExpressionSyntax, - matches As ImmutableArray(Of Match)) As CollectionInitializerSyntax + matches As ImmutableArray(Of CollectionMatch(Of SyntaxNode))) As CollectionInitializerSyntax Dim nodesAndTokens = ArrayBuilder(Of SyntaxNodeOrToken).GetInstance() AddExistingItems(objectCreation, nodesAndTokens) For i = 0 To matches.Length - 1 - Dim expressionStatement = DirectCast(matches(i).StatementOrExpression, ExpressionStatementSyntax) + Dim expressionStatement = DirectCast(matches(i).Node, ExpressionStatementSyntax) Dim newExpression As ExpressionSyntax Dim invocationExpression = DirectCast(expressionStatement.Expression, InvocationExpressionSyntax) From e378deaf88f662ac69fe1e8f193eeafc87a81c76 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 27 Sep 2024 13:35:09 -0700 Subject: [PATCH 25/25] renames --- ...harpUseCollectionExpressionForArrayCodeFixProvider.cs | 3 +-- ...rpUseCollectionExpressionForBuilderCodeFixProvider.cs | 5 ++--- ...arpUseCollectionExpressionForCreateCodeFixProvider.cs | 2 +- ...arpUseCollectionExpressionForFluentCodeFixProvider.cs | 8 ++++---- ...seCollectionExpressionForStackAllocCodeFixProvider.cs | 2 +- .../CSharpUseCollectionInitializerCodeFixProvider.cs | 5 +++-- ...ionInitializerCodeFixProvider_CollectionExpression.cs | 9 +++++---- ...onInitializerCodeFixProvider_CollectionInitializer.cs | 7 ++++--- 8 files changed, 21 insertions(+), 20 deletions(-) diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForArrayCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForArrayCodeFixProvider.cs index 937e95b97a57b..2340ebb0b3fa2 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForArrayCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForArrayCodeFixProvider.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; @@ -98,7 +97,7 @@ and not ImplicitArrayCreationExpressionSyntax static bool IsOnSingleLine(SourceText sourceText, SyntaxNode node) => sourceText.AreOnSameLine(node.GetFirstToken(), node.GetLastToken()); - ImmutableArray> GetMatches( + ImmutableArray> GetMatches( SemanticModel semanticModel, ExpressionSyntax expression, INamedTypeSymbol? expressionType) => expression switch { diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForBuilderCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForBuilderCodeFixProvider.cs index 04941a6937e4c..d485476d70f7e 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForBuilderCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForBuilderCodeFixProvider.cs @@ -16,7 +16,6 @@ using Microsoft.CodeAnalysis.PooledObjects; using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.UseCollectionExpression; -using Microsoft.CodeAnalysis.UseCollectionInitializer; namespace Microsoft.CodeAnalysis.CSharp.UseCollectionExpression; @@ -66,7 +65,7 @@ protected override async Task FixAsync( newDocument, dummyObjectCreation, preMatches: [], - analysisResult.Matches.SelectAsArray(m => new CollectionExpressionMatch(m.StatementOrExpression, m.UseSpread)), + analysisResult.Matches, static o => o.Initializer, static (o, i) => o.WithInitializer(i), cancellationToken).ConfigureAwait(false); @@ -92,7 +91,7 @@ static AnalysisResult TrackAnalysisResult(SyntaxNode root, AnalysisResult analys => new(analysisResult.DiagnosticLocation, root.GetCurrentNode(analysisResult.LocalDeclarationStatement)!, root.GetCurrentNode(analysisResult.CreationExpression)!, - analysisResult.Matches.SelectAsArray(m => new Match(root.GetCurrentNode(m.StatementOrExpression)!, m.UseSpread)), + analysisResult.Matches.SelectAsArray(m => new CollectionMatch(root.GetCurrentNode(m.Node)!, m.UseSpread)), analysisResult.ChangesSemantics); // Creates a new document with all of the relevant nodes in analysisResult tracked so that we can find them diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForCreateCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForCreateCodeFixProvider.cs index d6004c154d219..ff99cda43402b 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForCreateCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForCreateCodeFixProvider.cs @@ -62,7 +62,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 CollectionExpressionMatch(e, UseSpread: false)); + var matches = expressions.SelectAsArray(static e => new CollectionMatch(e, UseSpread: false)); var collectionExpression = await CreateCollectionExpressionAsync( newSemanticDocument.Document, diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForFluentCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForFluentCodeFixProvider.cs index 51b0069f0e350..18e06fdfdafbb 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForFluentCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForFluentCodeFixProvider.cs @@ -101,12 +101,12 @@ protected override async Task FixAsync( editor.ReplaceNode(invocationExpression, collectionExpression); - static ImmutableArray> CreateMatches( + static ImmutableArray> CreateMatches( SeparatedSyntaxList arguments, - ImmutableArray> matches, + ImmutableArray> matches, int index) { - using var result = TemporaryArray>.Empty; + using var result = TemporaryArray>.Empty; for (int i = 0, n = matches.Length; i < n; i++) { @@ -140,7 +140,7 @@ static ImmutableArray> CreateMatches static async Task> GetArgumentsAsync( Document document, - ImmutableArray> matches, + ImmutableArray> matches, CancellationToken cancellationToken) { if (matches.IsEmpty) diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocCodeFixProvider.cs index 0649e538a58c8..38db892da4846 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocCodeFixProvider.cs @@ -70,7 +70,7 @@ protected sealed override async Task FixAsync( return; - ImmutableArray> GetMatches() + ImmutableArray> GetMatches() => stackAllocExpression switch { // if we have `stackalloc[] { ... }` we have no subsequent matches to add to the collection. All values come diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider.cs index 87d8d8164358c..e27000bf62d1d 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider.cs @@ -9,6 +9,7 @@ using System.Threading.Tasks; using Microsoft.CodeAnalysis.CodeFixes; using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.UseCollectionExpression; using Microsoft.CodeAnalysis.UseCollectionInitializer; namespace Microsoft.CodeAnalysis.CSharp.UseCollectionInitializer; @@ -36,8 +37,8 @@ protected override CSharpUseCollectionInitializerAnalyzer GetAnalyzer() Document document, BaseObjectCreationExpressionSyntax objectCreation, bool useCollectionExpression, - ImmutableArray preMatches, - ImmutableArray postMatches, + ImmutableArray> preMatches, + ImmutableArray> postMatches, CancellationToken cancellationToken) { ExpressionSyntax newObjectCreation = useCollectionExpression diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionExpression.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionExpression.cs index fddc4090457d2..765047327066e 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionExpression.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionExpression.cs @@ -7,6 +7,7 @@ using System.Threading.Tasks; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.CSharp.UseCollectionExpression; +using Microsoft.CodeAnalysis.UseCollectionExpression; using Microsoft.CodeAnalysis.UseCollectionInitializer; namespace Microsoft.CodeAnalysis.CSharp.UseCollectionInitializer; @@ -20,15 +21,15 @@ internal partial class CSharpUseCollectionInitializerCodeFixProvider private static Task CreateCollectionExpressionAsync( Document document, BaseObjectCreationExpressionSyntax objectCreation, - ImmutableArray preMatches, - ImmutableArray postMatches, + ImmutableArray> preMatches, + ImmutableArray> postMatches, CancellationToken cancellationToken) { return CSharpCollectionExpressionRewriter.CreateCollectionExpressionAsync( document, objectCreation, - preMatches.SelectAsArray(m => new CollectionExpressionMatch(m.StatementOrExpression, m.UseSpread)), - postMatches.SelectAsArray(m => new CollectionExpressionMatch(m.StatementOrExpression, m.UseSpread)), + preMatches, + postMatches, static objectCreation => objectCreation.Initializer, static (objectCreation, initializer) => objectCreation.WithInitializer(initializer), cancellationToken); diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionInitializer.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionInitializer.cs index 985875a82836f..c8c3ad9b1b932 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionInitializer.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionInitializer.cs @@ -11,6 +11,7 @@ using Microsoft.CodeAnalysis.Formatting; using Microsoft.CodeAnalysis.PooledObjects; using Microsoft.CodeAnalysis.Shared.Extensions; +using Microsoft.CodeAnalysis.UseCollectionExpression; using Microsoft.CodeAnalysis.UseCollectionInitializer; using Roslyn.Utilities; @@ -23,7 +24,7 @@ internal partial class CSharpUseCollectionInitializerCodeFixProvider { private static BaseObjectCreationExpressionSyntax CreateObjectInitializerExpression( BaseObjectCreationExpressionSyntax objectCreation, - ImmutableArray matches) + ImmutableArray> matches) { var expressions = CreateCollectionInitializerExpressions(); var withLineBreaks = AddLineBreaks(expressions); @@ -35,13 +36,13 @@ SeparatedSyntaxList CreateCollectionInitializerExpressions() { using var _ = ArrayBuilder.GetInstance(out var nodesAndTokens); - UseInitializerHelpers.AddExistingItems( + UseInitializerHelpers.AddExistingItems, ExpressionSyntax>( objectCreation, nodesAndTokens, addTrailingComma: matches.Length > 0, static (_, expression) => expression); for (var i = 0; i < matches.Length; i++) { var match = matches[i]; - var statement = (ExpressionStatementSyntax)match.StatementOrExpression; + var statement = (ExpressionStatementSyntax)match.Node; var trivia = statement.GetLeadingTrivia(); var leadingTrivia = i == 0 ? trivia.WithoutLeadingBlankLines() : trivia;