Merge pull request #75267 from CyrusNajmabadi/moreFluentCases

src/Analyzers/CSharp/Analyzers/CSharpAnalyzers.projitems

@@ -82,7 +82,6 @@
     <Compile Include="$(MSBuildThisFileDirectory)UseCoalesceExpression\CSharpUseCoalesceExpressionForNullableTernaryConditionalCheckDiagnosticAnalyzer.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)UseCoalesceExpression\UseCoalesceExpressionHelpers.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)UseCollectionExpression\AbstractCSharpUseCollectionExpressionDiagnosticAnalyzer.cs" />
-    <Compile Include="$(MSBuildThisFileDirectory)UseCollectionExpression\CollectionExpressionMatch.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)UseCollectionExpression\CSharpUseCollectionExpressionForBuilderDiagnosticAnalyzer.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)UseCollectionExpression\CSharpUseCollectionExpressionForFluentDiagnosticAnalyzer.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)UseCollectionExpression\CSharpUseCollectionExpressionForEmptyDiagnosticAnalyzer.cs" />

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<CollectionExpressionMatch<StatementSyntax>> TryGetMatches(
+    public static ImmutableArray<CollectionMatch<StatementSyntax>> TryGetMatches(
         SemanticModel semanticModel,
         ArrayCreationExpressionSyntax expression,
         CollectionExpressionSyntax replacementExpression,
@@ -114,7 +115,7 @@ public static ImmutableArray<CollectionExpressionMatch<StatementSyntax>> TryGetM
         return matches;
     }
 
-    public static ImmutableArray<CollectionExpressionMatch<StatementSyntax>> TryGetMatches(
+    public static ImmutableArray<CollectionMatch<StatementSyntax>> TryGetMatches(
         SemanticModel semanticModel,
         ImplicitArrayCreationExpressionSyntax expression,
         CollectionExpressionSyntax replacementExpression,

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<Match<StatementSyntax>>.GetInstance(out var matches);
+        using var _ = ArrayBuilder<CollectionMatch<SyntaxNode>>.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<Match<StatementSyntax>> Matches,
+        ImmutableArray<CollectionMatch<SyntaxNode>> Matches,
         bool ChangesSemantics);
 }

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,46 +134,57 @@ 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<CollectionExpressionMatch<ArgumentSyntax>>.GetInstance(out var matchesInReverse);
-        if (!AnalyzeInvocation(text, state, invocation, addMatches ? matchesInReverse : null, out var existingInitializer, cancellationToken))
+        using var _1 = ArrayBuilder<CollectionMatch<ArgumentSyntax>>.GetInstance(out var preMatchesInReverse);
+        using var _2 = ArrayBuilder<CollectionMatch<ArgumentSyntax>>.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))
         {
             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<CollectionExpressionMatch<ArgumentSyntax>>? matchesInReverse,
+        ArrayBuilder<CollectionMatch<ArgumentSyntax>>? preMatchesInReverse,
+        ArrayBuilder<CollectionMatch<ArgumentSyntax>>? postMatchesInReverse,
         out InitializerExpressionSyntax? existingInitializer,
         CancellationToken cancellationToken)
     {
+        var semanticModel = state.SemanticModel;
+        var compilation = semanticModel.Compilation;
+
         existingInitializer = null;
         if (invocation.Expression is not MemberAccessExpressionSyntax memberAccess)
             return false;
 
         // 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
         // `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<ExpressionSyntax>.GetInstance(out var stack);
         stack.Push(memberAccess.Expression);
@@ -187,7 +199,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);
@@ -224,21 +236,44 @@ private static bool AnalyzeInvocation(
             {
                 if (objectCreation is not
                     {
-                        ArgumentList: null or { Arguments.Count: 0 },
                         Initializer: null or { RawKind: (int)SyntaxKind.CollectionInitializerExpression }
                     })
                 {
                     return false;
                 }
 
+                existingInitializer = objectCreation.Initializer;
+
                 if (!IsLegalInitializer(objectCreation.Initializer))
                     return false;
 
                 if (!IsListLike(current))
                     return false;
 
-                existingInitializer = objectCreation.Initializer;
-                return true;
+                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 (!Equals(argumentType.OriginalDefinition, ienumerableOfTType) &&
+                        !argumentType.AllInterfaces.Any(i => Equals(i.OriginalDefinition, ienumerableOfTType)))
+                    {
+                        return false;
+                    }
+
+                    // Add the arguments to the pre-matches.  They will execute before the initializer values are added.
+                    AddArgumentsInReverse(preMatchesInReverse, objectCreation.ArgumentList.Arguments, useSpread: true);
+                    return true;
+                }
+                else if (objectCreation.ArgumentList is null or { Arguments.Count: 0 })
+                {
+                    // Otherwise, we have to have an empty argument list.
+                    return true;
+                }
+
+                return false;
             }
 
             // Forms like `ImmutableArray.Create(...)` or `ImmutableArray.CreateRange(...)` are fine base cases.
@@ -248,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;
             }
 
@@ -277,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 `[.. <expr>]`.  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()
@@ -292,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<ArgumentSyntax>(
+                postMatchesInReverse.Add(new CollectionMatch<ArgumentSyntax>(
                     Argument(innerInvocation.WithExpression(
                         memberAccess.Update(
                             memberAccess.Expression.WithoutTrailingTrivia(),
@@ -302,7 +337,7 @@ void AddFinalMatch(ExpressionSyntax expression)
                 return;
             }
 
-            matchesInReverse.Add(new CollectionExpressionMatch<ArgumentSyntax>(Argument(expression), UseSpread: true));
+            postMatchesInReverse.Add(new CollectionMatch<ArgumentSyntax>(Argument(expression), UseSpread: true));
         }
 
         // We only want to offer this feature when the original collection was list-like (as opposed to being something
@@ -359,12 +394,13 @@ static bool IsLegalInitializer(InitializerExpressionSyntax? initializer)
                         return false;
                 }
             }
+
             return true;
         }
     }
 
     private static void AddArgumentsInReverse(
-        ArrayBuilder<CollectionExpressionMatch<ArgumentSyntax>>? matchesInReverse,
+        ArrayBuilder<CollectionMatch<ArgumentSyntax>>? matchesInReverse,
         SeparatedSyntaxList<ArgumentSyntax> arguments,
         bool useSpread)
     {
@@ -388,7 +424,7 @@ private static bool IsMatch(
         MemberAccessExpressionSyntax memberAccess,
         InvocationExpressionSyntax invocation,
         bool allowLinq,
-        ArrayBuilder<CollectionExpressionMatch<ArgumentSyntax>>? matchesInReverse,
+        ArrayBuilder<CollectionMatch<ArgumentSyntax>>? matchesInReverse,
         out bool isAdditionMatch,
         CancellationToken cancellationToken)
     {
@@ -467,12 +503,15 @@ static bool HasAnySuffix(string name)
     /// help determine the best collection expression final syntax.</param>
     /// <param name="CreationExpression">The location of the code like <c>builder.ToImmutable()</c> that will actually be
     /// replaced with the collection expression</param>
-    /// <param name="Matches">The arguments being added to the collection that will be converted into elements in the
-    /// final collection expression.</param>
+    /// <param name="PreMatches">The arguments being added to the collection that will be converted into elements in
+    /// the final collection expression *before* the existing initializer elements.</param>
+    /// <param name="PostMatches">The arguments being added to the collection that will be converted into elements in
+    /// the final collection expression *after* the existing initializer elements.</param>
     public readonly record struct AnalysisResult(
         // Location DiagnosticLocation,
         InitializerExpressionSyntax? ExistingInitializer,
         InvocationExpressionSyntax CreationExpression,
-        ImmutableArray<CollectionExpressionMatch<ArgumentSyntax>> Matches,
+        ImmutableArray<CollectionMatch<ArgumentSyntax>> PreMatches,
+        ImmutableArray<CollectionMatch<ArgumentSyntax>> PostMatches,
         bool ChangesSemantics);
 }

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<CollectionExpressionMatch<StatementSyntax>> TryGetMatches(
+    public static ImmutableArray<CollectionMatch<StatementSyntax>> TryGetMatches(
         SemanticModel semanticModel,
         StackAllocArrayCreationExpressionSyntax expression,
         INamedTypeSymbol? expressionType,

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<CollectionExpressionMatch<StatementSyntax>> TryGetMatches<TArrayCreationExpressionSyntax>(
+    public static ImmutableArray<CollectionMatch<StatementSyntax>> TryGetMatches<TArrayCreationExpressionSyntax>(
         SemanticModel semanticModel,
         TArrayCreationExpressionSyntax expression,
         CollectionExpressionSyntax replacementExpression,
@@ -835,7 +836,7 @@ public static ImmutableArray<CollectionExpressionMatch<StatementSyntax>> TryGetM
         if (getType(expression) is not ArrayTypeSyntax { RankSpecifiers: [{ Sizes: [var size] }, ..] })
             return default;
 
-        using var _ = ArrayBuilder<CollectionExpressionMatch<StatementSyntax>>.GetInstance(out var matches);
+        using var _ = ArrayBuilder<CollectionMatch<StatementSyntax>>.GetInstance(out var matches);
 
         var initializer = getInitializer(expression);
         if (size is OmittedArraySizeExpressionSyntax)