Merge pull request #69280 from CyrusNajmabadi/useCollectionExpression4

Add support for generating the `.. x ? [y] : []` pattern in a collection-expression
dotnet · Aug 2, 2023 · 21c31f9 · 21c31f9
2 parents 41044c4 + 027d00c
commit 21c31f9
Show file tree

Hide file tree

Showing 22 changed files with 1,103 additions and 184 deletions.
diff --git a/src/Analyzers/CSharp/Analyzers/CSharpAnalyzers.projitems b/src/Analyzers/CSharp/Analyzers/CSharpAnalyzers.projitems
@@ -83,6 +83,7 @@
     <Compile Include="$(MSBuildThisFileDirectory)UseCollectionExpression\CSharpUseCollectionExpressionForEmptyDiagnosticAnalyzer.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)UseCollectionExpression\CSharpUseCollectionExpressionForArrayDiagnosticAnalyzer.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)UseCollectionExpression\UseCollectionExpressionHelpers.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)UseCollectionInitializer\CSharpUseCollectionInitializerAnalyzer.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)UseCompoundAssignment\CSharpUseCompoundAssignmentDiagnosticAnalyzer.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)UseCompoundAssignment\CSharpUseCompoundCoalesceAssignmentDiagnosticAnalyzer.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)UseCompoundAssignment\Utilities.cs" />

diff --git a/...yzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerAnalyzer.cs b/...yzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerAnalyzer.cs
@@ -0,0 +1,51 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System.Collections.Generic;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.UseCollectionInitializer;
+using Roslyn.Utilities;
+
+namespace Microsoft.CodeAnalysis.CSharp.UseCollectionInitializer;
+
+internal sealed class CSharpUseCollectionInitializerAnalyzer : AbstractUseCollectionInitializerAnalyzer<
+    ExpressionSyntax,
+    StatementSyntax,
+    BaseObjectCreationExpressionSyntax,
+    MemberAccessExpressionSyntax,
+    InvocationExpressionSyntax,
+    ExpressionStatementSyntax,
+    ForEachStatementSyntax,
+    IfStatementSyntax,
+    VariableDeclaratorSyntax,
+    CSharpUseCollectionInitializerAnalyzer>
+{
+    protected override bool IsComplexElementInitializer(SyntaxNode expression)
+        => expression.IsKind(SyntaxKind.ComplexElementInitializerExpression);
+
+    protected override void GetPartsOfForeachStatement(
+        ForEachStatementSyntax statement,
+        out SyntaxToken identifier,
+        out ExpressionSyntax expression,
+        out IEnumerable<StatementSyntax> statements)
+    {
+        identifier = statement.Identifier;
+        expression = statement.Expression;
+        statements = ExtractEmbeddedStatements(statement.Statement);
+    }
+
+    protected override void GetPartsOfIfStatement(
+        IfStatementSyntax statement,
+        out ExpressionSyntax condition,
+        out IEnumerable<StatementSyntax> whenTrueStatements,
+        out IEnumerable<StatementSyntax>? whenFalseStatements)
+    {
+        condition = statement.Condition;
+        whenTrueStatements = ExtractEmbeddedStatements(statement.Statement);
+        whenFalseStatements = statement.Else != null ? ExtractEmbeddedStatements(statement.Else.Statement) : null;
+    }
+
+    private static IEnumerable<StatementSyntax> ExtractEmbeddedStatements(StatementSyntax embeddedStatement)
+        => embeddedStatement is BlockSyntax block ? block.Statements : SpecializedCollections.SingletonEnumerable(embeddedStatement);
+}
diff --git a/...rp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerDiagnosticAnalyzer.cs b/...rp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerDiagnosticAnalyzer.cs
@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
+using System.Collections.Generic;
 using System.Threading;
 using Microsoft.CodeAnalysis.CSharp.Analyzers.UseCollectionExpression;
 using Microsoft.CodeAnalysis.CSharp.Extensions;
@@ -11,11 +12,12 @@
 using Microsoft.CodeAnalysis.Diagnostics;
 using Microsoft.CodeAnalysis.LanguageService;
 using Microsoft.CodeAnalysis.UseCollectionInitializer;
+using Roslyn.Utilities;
 
 namespace Microsoft.CodeAnalysis.CSharp.UseCollectionInitializer;
 
 [DiagnosticAnalyzer(LanguageNames.CSharp)]
-internal class CSharpUseCollectionInitializerDiagnosticAnalyzer :
+internal sealed class CSharpUseCollectionInitializerDiagnosticAnalyzer :
     AbstractUseCollectionInitializerDiagnosticAnalyzer<
         SyntaxKind,
         ExpressionSyntax,
@@ -25,8 +27,13 @@ internal class CSharpUseCollectionInitializerDiagnosticAnalyzer :
         InvocationExpressionSyntax,
         ExpressionStatementSyntax,
         ForEachStatementSyntax,
-        VariableDeclaratorSyntax>
+        IfStatementSyntax,
+        VariableDeclaratorSyntax,
+        CSharpUseCollectionInitializerAnalyzer>
 {
+    protected override CSharpUseCollectionInitializerAnalyzer GetAnalyzer()
+        => CSharpUseCollectionInitializerAnalyzer.Allocate();
+
     protected override bool AreCollectionInitializersSupported(Compilation compilation)
         => compilation.LanguageVersion() >= LanguageVersion.CSharp3;
 

diff --git a/...Sharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider.cs b/...Sharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider.cs
@@ -3,6 +3,8 @@
 // See the LICENSE file in the project root for more information.
 
 using System;
+using System.Collections;
+using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.Composition;
 using System.Diagnostics.CodeAnalysis;
@@ -33,14 +35,19 @@ internal class CSharpUseCollectionInitializerCodeFixProvider :
             InvocationExpressionSyntax,
             ExpressionStatementSyntax,
             ForEachStatementSyntax,
-            VariableDeclaratorSyntax>
+            IfStatementSyntax,
+            VariableDeclaratorSyntax,
+            CSharpUseCollectionInitializerAnalyzer>
     {
         [ImportingConstructor]
         [SuppressMessage("RoslynDiagnosticsReliability", "RS0033:Importing constructor should be [Obsolete]", Justification = "Used in test code: https://github.com/dotnet/roslyn/issues/42814")]
         public CSharpUseCollectionInitializerCodeFixProvider()
         {
         }
 
+        protected override CSharpUseCollectionInitializerAnalyzer GetAnalyzer()
+            => CSharpUseCollectionInitializerAnalyzer.Allocate();
+
         protected override StatementSyntax GetNewStatement(
             SourceText sourceText,
             StatementSyntax statement,
@@ -62,7 +69,7 @@ private static ExpressionSyntax GetNewObjectCreation(
             ImmutableArray<Match<StatementSyntax>> matches)
         {
             return useCollectionExpression
-                ? CreateCollectionExpression(objectCreation, matches, MakeMultiLine(sourceText, objectCreation, matches, wrappingLength))
+                ? CreateCollectionExpression(sourceText, objectCreation, wrappingLength, matches)
                 : CreateObjectInitializerExpression(objectCreation, matches);
         }
 
@@ -76,15 +83,16 @@ private static BaseObjectCreationExpressionSyntax CreateObjectInitializerExpress
         }
 
         private static CollectionExpressionSyntax CreateCollectionExpression(
+            SourceText sourceText,
             BaseObjectCreationExpressionSyntax objectCreation,
-            ImmutableArray<Match<StatementSyntax>> matches,
-            bool makeMultiLine)
+            int wrappingLength,
+            ImmutableArray<Match<StatementSyntax>> matches)
         {
             var elements = CreateElements<CollectionElementSyntax>(
                 objectCreation, matches,
                 static (match, expression) => match?.UseSpread is true ? SpreadElement(expression) : ExpressionElement(expression));
 
-            if (makeMultiLine)
+            if (MakeMultiLine(sourceText, objectCreation, matches, wrappingLength))
                 elements = AddLineBreaks(elements, includeFinalLineBreak: false);
 
             return CollectionExpression(elements).WithTriviaFrom(objectCreation);
@@ -140,42 +148,48 @@ private static bool MakeMultiLine(
             if (!sourceText.AreOnSameLine(objectCreation.GetFirstToken(), objectCreation.GetLastToken()))
                 return true;
 
-            foreach (var match in matches)
-            {
-                var expression = GetExpression(match);
-
-                // If we have anything like: `new Dictionary<X,Y> { { A, B }, { C, D } }` then always make multiline.
-                // Similarly, if we have `new Dictionary<X,Y> { [A] = B }`.
-                if (expression is InitializerExpressionSyntax or AssignmentExpressionSyntax)
-                    return true;
-
-                // if any of the expressions we're adding are multiline, then make things multiline.
-                if (!sourceText.AreOnSameLine(expression.GetFirstToken(), expression.GetLastToken()))
-                    return true;
-            }
-
             var totalLength = "{}".Length;
             foreach (var match in matches)
             {
-                var expression = GetExpression(match);
-                totalLength += expression.Span.Length;
-                totalLength += ", ".Length;
+                foreach (var component in GetElementComponents(match.Statement))
+                {
+                    // if any of the expressions we're adding are multiline, then make things multiline.
+                    if (!sourceText.AreOnSameLine(component.GetFirstToken(), component.GetLastToken()))
+                        return true;
 
-                if (totalLength > wrappingLength)
-                    return true;
+                    totalLength += component.Span.Length;
+                    totalLength += ", ".Length;
+
+                    if (totalLength > wrappingLength)
+                        return true;
+                }
             }
 
             return false;
 
-            static ExpressionSyntax GetExpression(Match<StatementSyntax> match)
-                => match.Statement switch
+            static IEnumerable<SyntaxNode> GetElementComponents(StatementSyntax statement)
+            {
+                if (statement is ExpressionStatementSyntax expressionStatement)
                 {
-                    ExpressionStatementSyntax expressionStatement => expressionStatement.Expression,
-                    ForEachStatementSyntax foreachStatement => foreachStatement.Expression,
-                    _ => throw ExceptionUtilities.Unreachable(),
-                };
+                    yield return expressionStatement.Expression;
+                }
+                else if (statement is ForEachStatementSyntax foreachStatement)
+                {
+                    yield return foreachStatement.Expression;
+                }
+                else if (statement is IfStatementSyntax ifStatement)
+                {
+                    yield return ifStatement.Condition;
+                    yield return UnwrapEmbeddedStatement(ifStatement.Statement);
+                    if (ifStatement.Else != null)
+                        yield return UnwrapEmbeddedStatement(ifStatement.Else.Statement);
+                }
+            }
         }
 
+        private static StatementSyntax UnwrapEmbeddedStatement(StatementSyntax statement)
+            => statement is BlockSyntax { Statements: [var innerStatement] } ? innerStatement : statement;
+
         public static SeparatedSyntaxList<TNode> AddLineBreaks<TNode>(
             SeparatedSyntaxList<TNode> nodes, bool includeFinalLineBreak)
             where TNode : SyntaxNode
@@ -207,7 +221,8 @@ private static SeparatedSyntaxList<TElement> CreateElements<TElement>(
         {
             using var _ = ArrayBuilder<SyntaxNodeOrToken>.GetInstance(out var nodesAndTokens);
 
-            UseInitializerHelpers.AddExistingItems(objectCreation, nodesAndTokens, createElement);
+            UseInitializerHelpers.AddExistingItems(
+                objectCreation, nodesAndTokens, addTrailingComma: matches.Length > 0, createElement);
 
             for (var i = 0; i < matches.Length; i++)
             {
@@ -241,6 +256,30 @@ private static SeparatedSyntaxList<TElement> CreateElements<TElement>(
                     if (i < matches.Length - 1)
                         nodesAndTokens.Add(Token(SyntaxKind.CommaToken));
                 }
+                else if (statement is IfStatementSyntax ifStatement)
+                {
+                    var trueStatement = (ExpressionStatementSyntax)UnwrapEmbeddedStatement(ifStatement.Statement);
+
+                    if (ifStatement.Else is null)
+                    {
+                        // Create: x ? [y] : []
+                        var expression = ConditionalExpression(
+                            ifStatement.Condition.Parenthesize(),
+                            CollectionExpression(SingletonSeparatedList<CollectionElementSyntax>(ExpressionElement(ConvertExpression(trueStatement.Expression)))),
+                            CollectionExpression());
+                        nodesAndTokens.Add(createElement(match, expression));
+                    }
+                    else
+                    {
+                        // Create: x ? y : z
+                        var falseStatement = (ExpressionStatementSyntax)UnwrapEmbeddedStatement(ifStatement.Else.Statement);
+                        var expression = ConditionalExpression(ifStatement.Condition.Parenthesize(), ConvertExpression(trueStatement.Expression), ConvertExpression(falseStatement.Expression));
+                        nodesAndTokens.Add(createElement(match, expression));
+                    }
+
+                    if (i < matches.Length - 1)
+                        nodesAndTokens.Add(Token(SyntaxKind.CommaToken));
+                }
                 else
                 {
                     throw ExceptionUtilities.Unreachable();

diff --git a/...lyzers/CSharp/CodeFixes/UseObjectInitializer/CSharpUseObjectInitializerCodeFixProvider.cs b/...lyzers/CSharp/CodeFixes/UseObjectInitializer/CSharpUseObjectInitializerCodeFixProvider.cs
@@ -56,7 +56,7 @@ private static SeparatedSyntaxList<ExpressionSyntax> CreateExpressions(
             using var _ = ArrayBuilder<SyntaxNodeOrToken>.GetInstance(out var nodesAndTokens);
 
             UseInitializerHelpers.AddExistingItems<ObjectInitializerMatch, ExpressionSyntax>(
-                objectCreation, nodesAndTokens, static (_, e) => e);
+                objectCreation, nodesAndTokens, addTrailingComma: true, static (_, e) => e);
 
             for (var i = 0; i < matches.Length; i++)
             {

diff --git a/src/Analyzers/CSharp/CodeFixes/UseObjectInitializer/UseInitializerHelpers.cs b/src/Analyzers/CSharp/CodeFixes/UseObjectInitializer/UseInitializerHelpers.cs
@@ -34,6 +34,7 @@ public static BaseObjectCreationExpressionSyntax GetNewObjectCreation(
         public static void AddExistingItems<TMatch, TElementSyntax>(
             BaseObjectCreationExpressionSyntax objectCreation,
             ArrayBuilder<SyntaxNodeOrToken> nodesAndTokens,
+            bool addTrailingComma,
             Func<TMatch?, ExpressionSyntax, TElementSyntax> createElement)
             where TMatch : struct
             where TElementSyntax : SyntaxNode
@@ -51,7 +52,7 @@ public static void AddExistingItems<TMatch, TElementSyntax>(
 
             // If we have an odd number of elements already, add a comma at the end so that we can add the rest of the
             // items afterwards without a syntax issue.
-            if (nodesAndTokens.Count % 2 == 1)
+            if (addTrailingComma && nodesAndTokens.Count % 2 == 1)
             {
                 var last = nodesAndTokens.Last();
                 nodesAndTokens.RemoveLast();