Detect more fluent cases for converting collection expressions

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

src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForFluentDiagnosticAnalyzer.cs

@@ -133,46 +133,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<CollectionExpressionMatch<ArgumentSyntax>>.GetInstance(out var preMatchesInReverse);
+        using var _2 = ArrayBuilder<CollectionExpressionMatch<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<CollectionExpressionMatch<ArgumentSyntax>>? preMatchesInReverse,
+        ArrayBuilder<CollectionExpressionMatch<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 +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);
@@ -224,21 +235,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 +282,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 +311,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 +326,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 CollectionExpressionMatch<ArgumentSyntax>(
                     Argument(innerInvocation.WithExpression(
                         memberAccess.Update(
                             memberAccess.Expression.WithoutTrailingTrivia(),
@@ -302,7 +336,7 @@ void AddFinalMatch(ExpressionSyntax expression)
                 return;
             }
 
-            matchesInReverse.Add(new CollectionExpressionMatch<ArgumentSyntax>(Argument(expression), UseSpread: true));
+            postMatchesInReverse.Add(new CollectionExpressionMatch<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,6 +393,7 @@ static bool IsLegalInitializer(InitializerExpressionSyntax? initializer)
                         return false;
                 }
             }
+
             return true;
         }
     }
@@ -467,12 +502,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<CollectionExpressionMatch<ArgumentSyntax>> PreMatches,
+        ImmutableArray<CollectionExpressionMatch<ArgumentSyntax>> PostMatches,
         bool ChangesSemantics);
 }

src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CollectionExpressionMatch.cs

@@ -6,7 +6,7 @@
 
 namespace Microsoft.CodeAnalysis.CSharp.UseCollectionExpression;
 
-/// <inheritdoc cref="Match{TStatementSyntax}"/>
+/// <inheritdoc cref="Match"/>
 internal readonly record struct CollectionExpressionMatch<TMatchNode>(
     TMatchNode Node,
     bool UseSpread) where TMatchNode : SyntaxNode;