From 00eadd688f39ed22ceeec47f3bc8e22166a4c732 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 28 Jul 2023 09:17:17 -0700 Subject: [PATCH 1/7] Simplify --- ...CollectionInitializerDiagnosticAnalyzer.cs | 61 +++++++++++++------ 1 file changed, 42 insertions(+), 19 deletions(-) diff --git a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerDiagnosticAnalyzer.cs b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerDiagnosticAnalyzer.cs index 8391c8dddefbf..a8503e6cfe0c0 100644 --- a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerDiagnosticAnalyzer.cs +++ b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerDiagnosticAnalyzer.cs @@ -30,6 +30,7 @@ internal abstract partial class AbstractUseCollectionInitializerDiagnosticAnalyz TInvocationExpressionSyntax, TExpressionStatementSyntax, TForeachStatementSyntax, + TIfStatementSyntax, TVariableDeclaratorSyntax> : AbstractBuiltInCodeStyleDiagnosticAnalyzer where TSyntaxKind : struct @@ -40,6 +41,7 @@ internal abstract partial class AbstractUseCollectionInitializerDiagnosticAnalyz where TInvocationExpressionSyntax : TExpressionSyntax where TExpressionStatementSyntax : TStatementSyntax where TForeachStatementSyntax : TStatementSyntax + where TIfStatementSyntax : TStatementSyntax where TVariableDeclaratorSyntax : SyntaxNode { @@ -127,11 +129,14 @@ private void AnalyzeNode(SyntaxNodeAnalysisContext context, INamedTypeSymbol ien if (containingStatement == null) return; - var (matches, shouldUseCollectionExpression) = GetMatches(); - // If we got no matches, then we def can't convert this. - if (matches.IsDefaultOrEmpty) + // Analyze the surrounding statements. First, try a broader set of statements if the language supports + // collection expressions. + var result = GetCollectionExpressionMatches() ?? GetCollectionInitializerMatches(); + if (result is null) return; + var (matches, shouldUseCollectionExpression) = result.Value; + var nodes = ImmutableArray.Create(containingStatement).AddRange(matches.Select(static m => m.Statement)); var syntaxFacts = GetSyntaxFacts(); if (syntaxFacts.ContainsInterleavedDirective(nodes, cancellationToken)) @@ -150,29 +155,47 @@ private void AnalyzeNode(SyntaxNodeAnalysisContext context, INamedTypeSymbol ien return; - (ImmutableArray> matches, bool shouldUseCollectionExpression) GetMatches() + (ImmutableArray> matches, bool shouldUseCollectionExpression)? GetCollectionInitializerMatches() { - // Analyze the surrounding statements. First, try a broader set of statements if the language supports - // collection expressions. - var analyzeForCollectionExpression = AreCollectionExpressionsSupported(); var matches = UseCollectionInitializerAnalyzer< TExpressionSyntax, TStatementSyntax, TObjectCreationExpressionSyntax, TMemberAccessExpressionSyntax, TInvocationExpressionSyntax, TExpressionStatementSyntax, TForeachStatementSyntax, TVariableDeclaratorSyntax>.Analyze( - semanticModel, GetSyntaxFacts(), objectCreationExpression, analyzeForCollectionExpression, cancellationToken); + semanticModel, GetSyntaxFacts(), objectCreationExpression, areCollectionExpressionsSupported: false, cancellationToken); + + // If analysis failed, we can't change this, no matter what. + if (matches.IsDefault) + return null; - // if this was a normal (non-collection-expr) analysis, then just return what we got. - if (!analyzeForCollectionExpression) - return (matches, shouldUseCollectionExpression: false); + // for collection initializers we always want at least one element to add to the initializer. In other + // words, we don't want to suggest changing `new List()` to `new List() { }` as that's just + // noise. + if (matches.IsEmpty) + return null; - // If we succeeded in finding matches, and this is a location a collection expression is legal in, then convert to that. - if (!matches.IsDefaultOrEmpty && CanUseCollectionExpression(semanticModel, objectCreationExpression, cancellationToken)) - return (matches, analyzeForCollectionExpression); + return (matches, shouldUseCollectionExpression: false); + } + + (ImmutableArray> matches, bool shouldUseCollectionExpression)? GetCollectionExpressionMatches() + { + // Don't bother analyzing for the collection expression case if the lang/version doesn't even support it. + if (!AreCollectionExpressionsSupported()) + return null; - // we tried collection expression, and were not successful. try again, this time without collection exprs. - analyzeForCollectionExpression = false; - matches = UseCollectionInitializerAnalyzer< + var matches = UseCollectionInitializerAnalyzer< TExpressionSyntax, TStatementSyntax, TObjectCreationExpressionSyntax, TMemberAccessExpressionSyntax, TInvocationExpressionSyntax, TExpressionStatementSyntax, TForeachStatementSyntax, TVariableDeclaratorSyntax>.Analyze( - semanticModel, GetSyntaxFacts(), objectCreationExpression, analyzeForCollectionExpression, cancellationToken); - return (matches, analyzeForCollectionExpression); + semanticModel, GetSyntaxFacts(), objectCreationExpression, areCollectionExpressionsSupported: true, cancellationToken); + + // If analysis failed, we can't change this, no matter what. + if (matches.IsDefault) + return null; + + // Analysis succeeded. Note: for collection expressions, it's fine for this result to be empty. In + // other words, it's ok to offer changing `new List() { 1 }` (on its own) to `[1]`. + + // Check if it would actually be legal to use a collection expression here though. + if (!CanUseCollectionExpression(semanticModel, objectCreationExpression, cancellationToken)) + return null; + + return (matches, shouldUseCollectionExpression: true); } bool AreCollectionExpressionsSupported() From 51e266a5a4b56a0b4ee49c8c96f818547fb73aae Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 28 Jul 2023 09:42:35 -0700 Subject: [PATCH 2/7] In progress --- ...CollectionInitializerDiagnosticAnalyzer.cs | 14 +- .../UseCollectionInitializerAnalyzer.cs | 206 ++++++++++++------ ...UseCollectionInitializerCodeFixProvider.cs | 4 +- .../Core/Services/SyntaxFacts/ISyntaxFacts.cs | 2 + 4 files changed, 157 insertions(+), 69 deletions(-) diff --git a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerDiagnosticAnalyzer.cs b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerDiagnosticAnalyzer.cs index a8503e6cfe0c0..84d1f47236ea2 100644 --- a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerDiagnosticAnalyzer.cs +++ b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerDiagnosticAnalyzer.cs @@ -17,6 +17,16 @@ 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 + /// 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; @@ -158,7 +168,7 @@ private void AnalyzeNode(SyntaxNodeAnalysisContext context, INamedTypeSymbol ien (ImmutableArray> matches, bool shouldUseCollectionExpression)? GetCollectionInitializerMatches() { var matches = UseCollectionInitializerAnalyzer< - TExpressionSyntax, TStatementSyntax, TObjectCreationExpressionSyntax, TMemberAccessExpressionSyntax, TInvocationExpressionSyntax, TExpressionStatementSyntax, TForeachStatementSyntax, TVariableDeclaratorSyntax>.Analyze( + TExpressionSyntax, TStatementSyntax, TObjectCreationExpressionSyntax, TMemberAccessExpressionSyntax, TInvocationExpressionSyntax, TExpressionStatementSyntax, TForeachStatementSyntax, TIfStatementSyntax, TVariableDeclaratorSyntax>.Analyze( semanticModel, GetSyntaxFacts(), objectCreationExpression, areCollectionExpressionsSupported: false, cancellationToken); // If analysis failed, we can't change this, no matter what. @@ -181,7 +191,7 @@ private void AnalyzeNode(SyntaxNodeAnalysisContext context, INamedTypeSymbol ien return null; var matches = UseCollectionInitializerAnalyzer< - TExpressionSyntax, TStatementSyntax, TObjectCreationExpressionSyntax, TMemberAccessExpressionSyntax, TInvocationExpressionSyntax, TExpressionStatementSyntax, TForeachStatementSyntax, TVariableDeclaratorSyntax>.Analyze( + TExpressionSyntax, TStatementSyntax, TObjectCreationExpressionSyntax, TMemberAccessExpressionSyntax, TInvocationExpressionSyntax, TExpressionStatementSyntax, TForeachStatementSyntax, TIfStatementSyntax, TVariableDeclaratorSyntax>.Analyze( semanticModel, GetSyntaxFacts(), objectCreationExpression, areCollectionExpressionsSupported: true, cancellationToken); // If analysis failed, we can't change this, no matter what. diff --git a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/UseCollectionInitializerAnalyzer.cs b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/UseCollectionInitializerAnalyzer.cs index 9a285a1824c25..d77c113992463 100644 --- a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/UseCollectionInitializerAnalyzer.cs +++ b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/UseCollectionInitializerAnalyzer.cs @@ -21,6 +21,7 @@ internal sealed class UseCollectionInitializerAnalyzer< TInvocationExpressionSyntax, TExpressionStatementSyntax, TForeachStatementSyntax, + TIfStatementSyntax, TVariableDeclaratorSyntax> : AbstractObjectCreationExpressionAnalyzer< TExpressionSyntax, TStatementSyntax, @@ -34,10 +35,11 @@ internal sealed class UseCollectionInitializerAnalyzer< where TInvocationExpressionSyntax : TExpressionSyntax where TExpressionStatementSyntax : TStatementSyntax where TForeachStatementSyntax : TStatementSyntax + where TIfStatementSyntax : TStatementSyntax where TVariableDeclaratorSyntax : SyntaxNode { - private static readonly ObjectPool> s_pool - = SharedPools.Default>(); + private static readonly ObjectPool> s_pool + = SharedPools.Default>(); public static ImmutableArray> Analyze( SemanticModel semanticModel, @@ -107,86 +109,158 @@ protected override void AddMatches(ArrayBuilder> matches continue; } - if (extractedChild is TExpressionStatementSyntax expressionStatement) + if (extractedChild is TExpressionStatementSyntax expressionStatement && + TryProcessExpressionStatement(expressionStatement)) { - if (!seenIndexAssignment) + continue; + } + else if (extractedChild is TForeachStatementSyntax foreachStatement && + TryProcessForeachStatement(foreachStatement)) + { + continue; + } + else if (extractedChild is TIfStatementSyntax ifStatement && + TryProcessIfStatement(ifStatement)) + { + continue; + } + + // Something we didn't understand. Stop here. + return; + } + + bool TryProcessExpressionStatement(TExpressionStatementSyntax expressionStatement) + { + // Can't mix Adds and indexing. + if (!seenIndexAssignment) + { + // Look for a call to Add or AddRange + if (TryAnalyzeInvocation( + expressionStatement, + addName: WellKnownMemberNames.CollectionInitializerAddMethodName, + requiredArgumentName: null, + out var instance) && + ValuePatternMatches(instance)) { - // Look for a call to Add or AddRange - if (TryAnalyzeInvocation( - expressionStatement, - addName: WellKnownMemberNames.CollectionInitializerAddMethodName, - requiredArgumentName: null, - out var instance) && - ValuePatternMatches(instance)) - { - seenInvocation = true; - matches.Add(new Match(expressionStatement, UseSpread: false)); - continue; - } - else if ( - _analyzeForCollectionExpression && - TryAnalyzeInvocation( - expressionStatement, - addName: nameof(List.AddRange), - requiredArgumentName: null, - out instance)) - { - seenInvocation = true; + seenInvocation = true; + matches.Add(new Match(expressionStatement, UseSpread: false)); + return true; + } + else if ( + _analyzeForCollectionExpression && + TryAnalyzeInvocation( + expressionStatement, + addName: nameof(List.AddRange), + requiredArgumentName: null, + out instance)) + { + seenInvocation = true; - // AddRange(x) will become `..x` when we make it into a collection expression. - matches.Add(new Match(expressionStatement, UseSpread: true)); - continue; - } + // AddRange(x) will become `..x` when we make it into a collection expression. + matches.Add(new Match(expressionStatement, UseSpread: true)); + return true; + } + } + + // Can't mix Adds and indexing. Can't use indexing with a collection expression. + if (!seenInvocation && !_analyzeForCollectionExpression) + { + if (TryAnalyzeIndexAssignment(expressionStatement, out var instance)) + { + seenIndexAssignment = true; + matches.Add(new Match(expressionStatement, UseSpread: false)); + return true; } + } + + return false; + } - if (!seenInvocation && !_analyzeForCollectionExpression) + bool TryProcessForeachStatement(TForeachStatementSyntax foreachStatement) + { + // if we're not producing a collection expression, then we cannot convert any foreach'es into + // `[..expr]` elements. + if (_analyzeForCollectionExpression) + { + _syntaxFacts.GetPartsOfForeachStatement(foreachStatement, out var identifier, out _, out var foreachStatements); + if (identifier != default) { - if (TryAnalyzeIndexAssignment(expressionStatement, out var instance)) + // must be of the form: + // + // foreach (var x in expr) + // dest.Add(x) + // + // By passing 'x' into TryAnalyzeInvocation below, we ensure that it is an enumerated value from `expr` + // being added to `dest`. + if (foreachStatements.ToImmutableArray() is [TExpressionStatementSyntax childExpressionStatement] && + TryAnalyzeInvocation( + childExpressionStatement, + addName: WellKnownMemberNames.CollectionInitializerAddMethodName, + requiredArgumentName: identifier.Text, + out var instance) && + ValuePatternMatches(instance)) { - seenIndexAssignment = true; - matches.Add(new Match(expressionStatement, UseSpread: false)); - continue; + // `foreach` will become `..expr` when we make it into a collection expression. + matches.Add(new Match(foreachStatement, UseSpread: true)); + return true; } } - - return; } - else if (extractedChild is TForeachStatementSyntax foreachStatement) + + return false; + } + + bool TryProcessIfStatement(TIfStatementSyntax ifStatement) + { + if (!_analyzeForCollectionExpression) + return false; + + // look for the form: + // + // if (x) + // expr.Add(y) + // + // or + // + // if (x) + // expr.Add(y) + // else + // expr.Add(z) + + _syntaxFacts.GetPartsOfIfStatement(ifStatement, out _, out var whenTrue, out var whenFalse); + var whenTrueStatements = whenTrue.ToImmutableArray(); + var whenFalseStatements = whenFalse.ToImmutableArray(); + + if (whenTrueStatements is [TExpressionStatementSyntax trueChildStatement] && + TryAnalyzeInvocation( + trueChildStatement, + addName: WellKnownMemberNames.CollectionInitializerAddMethodName, + requiredArgumentName: null, + out var instance) && + ValuePatternMatches(instance)) { - // if we're not producing a collection expression, then we cannot convert any foreach'es into - // `[..expr]` elements. - if (!_analyzeForCollectionExpression) - return; + if (whenTrueStatements.Length == 0) + { + // add the form `.. x ? [y] : []` to the result + matches.Add(new Match(ifStatement, UseSpread: true)); + return true; + } - _syntaxFacts.GetPartsOfForeachStatement(foreachStatement, out var identifier, out _, out var foreachStatements); - if (identifier == default) - return; - - // must be of the form: - // - // foreach (var x in expr) - // dest.Add(x) - // - // By passing 'x' into TryAnalyzeInvocation below, we ensure that it is an enumerated value from `expr` - // being added to `dest`. - if (foreachStatements.ToImmutableArray() is not [TExpressionStatementSyntax childExpressionStatement] || - !TryAnalyzeInvocation( - childExpressionStatement, + if (whenFalseStatements is [TExpressionStatementSyntax falseChildStatement] && + TryAnalyzeInvocation( + falseChildStatement, addName: WellKnownMemberNames.CollectionInitializerAddMethodName, - requiredArgumentName: identifier.Text, - out var instance) || - !ValuePatternMatches(instance)) + requiredArgumentName: null, + out instance) && + ValuePatternMatches(instance)) { - return; + // add the form `x ? y : z` to the result + matches.Add(new Match(ifStatement, UseSpread: false)); + return true; } - - // `foreach` will become `..expr` when we make it into a collection expression. - matches.Add(new Match(foreachStatement, UseSpread: true)); - } - else - { - return; } + + return false; } } diff --git a/src/Analyzers/Core/CodeFixes/UseCollectionInitializer/AbstractUseCollectionInitializerCodeFixProvider.cs b/src/Analyzers/Core/CodeFixes/UseCollectionInitializer/AbstractUseCollectionInitializerCodeFixProvider.cs index 5b9fbadb6e04a..276a6756e3b12 100644 --- a/src/Analyzers/Core/CodeFixes/UseCollectionInitializer/AbstractUseCollectionInitializerCodeFixProvider.cs +++ b/src/Analyzers/Core/CodeFixes/UseCollectionInitializer/AbstractUseCollectionInitializerCodeFixProvider.cs @@ -30,6 +30,7 @@ internal abstract class AbstractUseCollectionInitializerCodeFixProvider< TInvocationExpressionSyntax, TExpressionStatementSyntax, TForeachStatementSyntax, + TIfStatementSyntax, TVariableDeclaratorSyntax> : SyntaxEditorBasedCodeFixProvider where TSyntaxKind : struct @@ -40,6 +41,7 @@ internal abstract class AbstractUseCollectionInitializerCodeFixProvider< where TInvocationExpressionSyntax : TExpressionSyntax where TExpressionStatementSyntax : TStatementSyntax where TForeachStatementSyntax : TStatementSyntax + where TIfStatementSyntax : TStatementSyntax where TVariableDeclaratorSyntax : SyntaxNode { public sealed override ImmutableArray FixableDiagnosticIds @@ -102,7 +104,7 @@ protected sealed override async Task FixAllAsync( var (originalObjectCreation, useCollectionExpression) = originalObjectCreationNodes.Pop(); var objectCreation = currentRoot.GetCurrentNodes(originalObjectCreation).Single(); - var matches = UseCollectionInitializerAnalyzer.Analyze( + var matches = UseCollectionInitializerAnalyzer.Analyze( semanticModel, syntaxFacts, objectCreation, useCollectionExpression, cancellationToken); if (matches.IsDefaultOrEmpty) diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Services/SyntaxFacts/ISyntaxFacts.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Services/SyntaxFacts/ISyntaxFacts.cs index 1dc1f8a966a50..6f4325c4a36e3 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Services/SyntaxFacts/ISyntaxFacts.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Services/SyntaxFacts/ISyntaxFacts.cs @@ -200,6 +200,8 @@ internal interface ISyntaxFacts bool IsExpressionOfForeach([NotNullWhen(true)] SyntaxNode? node); void GetPartsOfForeachStatement(SyntaxNode statement, out SyntaxToken identifier, out SyntaxNode expression, out IEnumerable statements); + void GetPartsOfIfStatement(SyntaxNode statement, out SyntaxNode condition, out IEnumerable whenTrueStatements, out IEnumerable whenFalseStatements); + void GetPartsOfTupleExpression(SyntaxNode node, out SyntaxToken openParen, out SeparatedSyntaxList arguments, out SyntaxToken closeParen) where TArgumentSyntax : SyntaxNode; From 7fe6746747797dc1b2aad6ecf9aed47c6658284d Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 28 Jul 2023 10:05:45 -0700 Subject: [PATCH 3/7] in progress --- .../Core/Analyzers/Analyzers.projitems | 2 +- ...bstractObjectCreationExpressionAnalyzer.cs | 6 ++--- ...stractUseCollectionInitializerAnalyzer.cs} | 24 +++++++++++++++---- ...CollectionInitializerDiagnosticAnalyzer.cs | 23 +++++++++++++----- .../SyntaxFacts/VisualBasicSyntaxFacts.vb | 5 ++++ 5 files changed, 45 insertions(+), 15 deletions(-) rename src/Analyzers/Core/Analyzers/UseCollectionInitializer/{UseCollectionInitializerAnalyzer.cs => AbstractUseCollectionInitializerAnalyzer.cs} (95%) diff --git a/src/Analyzers/Core/Analyzers/Analyzers.projitems b/src/Analyzers/Core/Analyzers/Analyzers.projitems index 1c12ab9611ae8..73a50892f9e5f 100644 --- a/src/Analyzers/Core/Analyzers/Analyzers.projitems +++ b/src/Analyzers/Core/Analyzers/Analyzers.projitems @@ -82,7 +82,7 @@ - + diff --git a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractObjectCreationExpressionAnalyzer.cs b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractObjectCreationExpressionAnalyzer.cs index 6317b8d68204b..f476ebe740045 100644 --- a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractObjectCreationExpressionAnalyzer.cs +++ b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractObjectCreationExpressionAnalyzer.cs @@ -38,6 +38,9 @@ protected AbstractObjectCreationExpressionAnalyzer() { } + protected abstract bool ShouldAnalyze(); + protected abstract void AddMatches(ArrayBuilder matches); + public void Initialize( SemanticModel semanticModel, ISyntaxFacts syntaxFacts, @@ -64,9 +67,6 @@ protected void Clear() _initializedSymbol = null; } - protected abstract bool ShouldAnalyze(); - protected abstract void AddMatches(ArrayBuilder matches); - protected ImmutableArray AnalyzeWorker() { if (!ShouldAnalyze()) diff --git a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/UseCollectionInitializerAnalyzer.cs b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerAnalyzer.cs similarity index 95% rename from src/Analyzers/Core/Analyzers/UseCollectionInitializer/UseCollectionInitializerAnalyzer.cs rename to src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerAnalyzer.cs index d77c113992463..ab2accd3a2eac 100644 --- a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/UseCollectionInitializerAnalyzer.cs +++ b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerAnalyzer.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; using System.Collections.Generic; using System.Collections.Immutable; using System.Diagnostics.CodeAnalysis; @@ -13,7 +14,7 @@ namespace Microsoft.CodeAnalysis.UseCollectionInitializer { - internal sealed class UseCollectionInitializerAnalyzer< + internal abstract class AbstractUseCollectionInitializerAnalyzer< TExpressionSyntax, TStatementSyntax, TObjectCreationExpressionSyntax, @@ -22,12 +23,13 @@ internal sealed class UseCollectionInitializerAnalyzer< TExpressionStatementSyntax, TForeachStatementSyntax, TIfStatementSyntax, - TVariableDeclaratorSyntax> : AbstractObjectCreationExpressionAnalyzer< + TVariableDeclaratorSyntax, + TAnalyzer> : AbstractObjectCreationExpressionAnalyzer< TExpressionSyntax, TStatementSyntax, TObjectCreationExpressionSyntax, TVariableDeclaratorSyntax, - Match> + Match>, IDisposable where TExpressionSyntax : SyntaxNode where TStatementSyntax : SyntaxNode where TObjectCreationExpressionSyntax : TExpressionSyntax @@ -37,9 +39,21 @@ internal sealed class UseCollectionInitializerAnalyzer< where TForeachStatementSyntax : TStatementSyntax where TIfStatementSyntax : TStatementSyntax where TVariableDeclaratorSyntax : SyntaxNode + where TAnalyzer : AbstractUseCollectionInitializerAnalyzer< + TExpressionSyntax, + TStatementSyntax, + TObjectCreationExpressionSyntax, + TMemberAccessExpressionSyntax, + TInvocationExpressionSyntax, + TExpressionStatementSyntax, + TForeachStatementSyntax, + TIfStatementSyntax, + TVariableDeclaratorSyntax, + TAnalyzer>, new() { - private static readonly ObjectPool> s_pool - = SharedPools.Default>(); + private static readonly ObjectPool s_pool = SharedPools.Default(); + + public abstract void Dispose(); public static ImmutableArray> Analyze( SemanticModel semanticModel, diff --git a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerDiagnosticAnalyzer.cs b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerDiagnosticAnalyzer.cs index 84d1f47236ea2..5bfbed606b893 100644 --- a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerDiagnosticAnalyzer.cs +++ b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerDiagnosticAnalyzer.cs @@ -2,7 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -using System; using System.Collections; using System.Collections.Immutable; using System.Linq; @@ -41,7 +40,8 @@ internal abstract partial class AbstractUseCollectionInitializerDiagnosticAnalyz TExpressionStatementSyntax, TForeachStatementSyntax, TIfStatementSyntax, - TVariableDeclaratorSyntax> + TVariableDeclaratorSyntax, + TAnalyzer> : AbstractBuiltInCodeStyleDiagnosticAnalyzer where TSyntaxKind : struct where TExpressionSyntax : SyntaxNode @@ -53,6 +53,16 @@ internal abstract partial class AbstractUseCollectionInitializerDiagnosticAnalyz where TForeachStatementSyntax : TStatementSyntax where TIfStatementSyntax : TStatementSyntax where TVariableDeclaratorSyntax : SyntaxNode + where TAnalyzer : AbstractUseCollectionInitializerAnalyzer< + TExpressionSyntax, + TStatementSyntax, + TObjectCreationExpressionSyntax, + TMemberAccessExpressionSyntax, + TInvocationExpressionSyntax, + TExpressionStatementSyntax, + TForeachStatementSyntax, + TIfStatementSyntax, + TVariableDeclaratorSyntax> { public override DiagnosticAnalyzerCategory GetAnalyzerCategory() @@ -85,6 +95,8 @@ protected AbstractUseCollectionInitializerDiagnosticAnalyzer() protected abstract bool AreCollectionExpressionsSupported(Compilation compilation); protected abstract bool CanUseCollectionExpression(SemanticModel semanticModel, TObjectCreationExpressionSyntax objectCreationExpression, CancellationToken cancellationToken); + protected abstract TAnalyzer GetAnalyzer(); + protected sealed override void InitializeWorker(AnalysisContext context) => context.RegisterCompilationStartAction(OnCompilationStart); @@ -141,6 +153,7 @@ private void AnalyzeNode(SyntaxNodeAnalysisContext context, INamedTypeSymbol ien // Analyze the surrounding statements. First, try a broader set of statements if the language supports // collection expressions. + using var analyzer = GetAnalyzer(); var result = GetCollectionExpressionMatches() ?? GetCollectionInitializerMatches(); if (result is null) return; @@ -167,8 +180,7 @@ private void AnalyzeNode(SyntaxNodeAnalysisContext context, INamedTypeSymbol ien (ImmutableArray> matches, bool shouldUseCollectionExpression)? GetCollectionInitializerMatches() { - var matches = UseCollectionInitializerAnalyzer< - TExpressionSyntax, TStatementSyntax, TObjectCreationExpressionSyntax, TMemberAccessExpressionSyntax, TInvocationExpressionSyntax, TExpressionStatementSyntax, TForeachStatementSyntax, TIfStatementSyntax, TVariableDeclaratorSyntax>.Analyze( + var matches = analyzer.Analyze( semanticModel, GetSyntaxFacts(), objectCreationExpression, areCollectionExpressionsSupported: false, cancellationToken); // If analysis failed, we can't change this, no matter what. @@ -190,8 +202,7 @@ private void AnalyzeNode(SyntaxNodeAnalysisContext context, INamedTypeSymbol ien if (!AreCollectionExpressionsSupported()) return null; - var matches = UseCollectionInitializerAnalyzer< - TExpressionSyntax, TStatementSyntax, TObjectCreationExpressionSyntax, TMemberAccessExpressionSyntax, TInvocationExpressionSyntax, TExpressionStatementSyntax, TForeachStatementSyntax, TIfStatementSyntax, TVariableDeclaratorSyntax>.Analyze( + var matches = analyzer.Analyze( semanticModel, GetSyntaxFacts(), objectCreationExpression, areCollectionExpressionsSupported: true, cancellationToken); // If analysis failed, we can't change this, no matter what. diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/VisualBasic/Services/SyntaxFacts/VisualBasicSyntaxFacts.vb b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/VisualBasic/Services/SyntaxFacts/VisualBasicSyntaxFacts.vb index 8b5ab188f7d24..5be089c172fbb 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/VisualBasic/Services/SyntaxFacts/VisualBasicSyntaxFacts.vb +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/VisualBasic/Services/SyntaxFacts/VisualBasicSyntaxFacts.vb @@ -1823,6 +1823,11 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.LanguageService statements = If(foreachBlock Is Nothing, SpecializedCollections.EmptyEnumerable(Of SyntaxNode), foreachBlock.Statements) End Sub + Public Sub GetPartsOfIfStatement(statement As IfStatementSyntax, ByRef condition As SyntaxNode, ByRef whenTrueStatements As IEnumerable(Of SyntaxNode), ByRef whenFalseStatements As IEnumerable(Of SyntaxNode)) Implements ISyntaxFacts.GetPartsOfIfStatement + Dim ifStatement = DirectCast(statement, IfStatementSyntax) + + End Sub + Public Sub GetPartsOfInterpolationExpression(node As SyntaxNode, ByRef stringStartToken As SyntaxToken, ByRef contents As SyntaxList(Of SyntaxNode), ByRef stringEndToken As SyntaxToken) Implements ISyntaxFacts.GetPartsOfInterpolationExpression Dim interpolatedStringExpression = DirectCast(node, InterpolatedStringExpressionSyntax) stringStartToken = interpolatedStringExpression.DollarSignDoubleQuoteToken From e6481fbd8f1c2639b5a58a903a2c6e525503b06e Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 28 Jul 2023 10:27:27 -0700 Subject: [PATCH 4/7] Typesafe --- .../Analyzers/CSharpAnalyzers.projitems | 1 + .../CSharpUseCollectionInitializerAnalyzer.cs | 48 +++++++++++++ ...CollectionInitializerDiagnosticAnalyzer.cs | 11 ++- ...UseCollectionInitializerCodeFixProvider.cs | 7 +- ...bstractUseCollectionInitializerAnalyzer.cs | 68 +++++++++---------- ...CollectionInitializerDiagnosticAnalyzer.cs | 12 ++-- ...UseCollectionInitializerCodeFixProvider.cs | 21 +++++- ...isualBasicCollectionInitializerAnalyzer.vb | 32 +++++++++ ...CollectionInitializerDiagnosticAnalyzer.vb | 10 ++- .../Analyzers/VisualBasicAnalyzers.projitems | 1 + ...UseCollectionInitializerCodeFixProvider.vb | 8 ++- .../Services/SyntaxFacts/CSharpSyntaxFacts.cs | 10 +-- .../Core/Services/SyntaxFacts/ISyntaxFacts.cs | 4 +- .../SyntaxFacts/ISyntaxFactsExtensions.cs | 6 -- .../SyntaxFacts/VisualBasicSyntaxFacts.vb | 17 +---- 15 files changed, 174 insertions(+), 82 deletions(-) create mode 100644 src/Analyzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerAnalyzer.cs create mode 100644 src/Analyzers/VisualBasic/Analyzers/UseCollectionInitializer/VisualBasicCollectionInitializerAnalyzer.vb diff --git a/src/Analyzers/CSharp/Analyzers/CSharpAnalyzers.projitems b/src/Analyzers/CSharp/Analyzers/CSharpAnalyzers.projitems index a0072d0f28454..b669aaeffb169 100644 --- a/src/Analyzers/CSharp/Analyzers/CSharpAnalyzers.projitems +++ b/src/Analyzers/CSharp/Analyzers/CSharpAnalyzers.projitems @@ -83,6 +83,7 @@ + diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerAnalyzer.cs new file mode 100644 index 0000000000000..9b765929afe4c --- /dev/null +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerAnalyzer.cs @@ -0,0 +1,48 @@ +// 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 void GetPartsOfForeachStatement( + ForEachStatementSyntax statement, + out SyntaxToken identifier, + out ExpressionSyntax expression, + out IEnumerable statements) + { + identifier = statement.Identifier; + expression = statement.Expression; + statements = ExtractEmbeddedStatements(statement.Statement); + } + + protected override void GetPartsOfIfStatement( + IfStatementSyntax statement, + out ExpressionSyntax condition, + out IEnumerable whenTrueStatements, + out IEnumerable whenFalseStatements) + { + condition = statement.Condition; + whenTrueStatements = ExtractEmbeddedStatements(statement.Statement); + whenFalseStatements = statement.Else != null ? ExtractEmbeddedStatements(statement.Else.Statement) : SpecializedCollections.EmptyEnumerable(); + } + + private static IEnumerable ExtractEmbeddedStatements(StatementSyntax embeddedStatement) + => embeddedStatement is BlockSyntax block ? block.Statements : SpecializedCollections.SingletonEnumerable(embeddedStatement); +} diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerDiagnosticAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerDiagnosticAnalyzer.cs index f301f56711619..448727c596144 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerDiagnosticAnalyzer.cs +++ b/src/Analyzers/CSharp/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/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider.cs index 469fb3530944a..113eb8952fbc0 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider.cs @@ -33,7 +33,9 @@ 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")] @@ -41,6 +43,9 @@ public CSharpUseCollectionInitializerCodeFixProvider() { } + protected override CSharpUseCollectionInitializerAnalyzer GetAnalyzer() + => CSharpUseCollectionInitializerAnalyzer.Allocate(); + protected override StatementSyntax GetNewStatement( SourceText sourceText, StatementSyntax statement, diff --git a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerAnalyzer.cs b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerAnalyzer.cs index ab2accd3a2eac..c70590c5491d7 100644 --- a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerAnalyzer.cs +++ b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerAnalyzer.cs @@ -53,26 +53,27 @@ internal abstract class AbstractUseCollectionInitializerAnalyzer< { private static readonly ObjectPool s_pool = SharedPools.Default(); - public abstract void Dispose(); + protected abstract void GetPartsOfForeachStatement(TForeachStatementSyntax statement, out SyntaxToken identifier, out TExpressionSyntax expression, out IEnumerable statements); + protected abstract void GetPartsOfIfStatement(TIfStatementSyntax statement, out TExpressionSyntax condition, out IEnumerable whenTrueStatements, out IEnumerable whenFalseStatements); - public static ImmutableArray> Analyze( + public static TAnalyzer Allocate() + => s_pool.Allocate(); + + public void Dispose() + { + this.Clear(); + s_pool.Free((TAnalyzer)this); + } + + public ImmutableArray> Analyze( SemanticModel semanticModel, ISyntaxFacts syntaxFacts, TObjectCreationExpressionSyntax objectCreationExpression, bool areCollectionExpressionsSupported, CancellationToken cancellationToken) { - var analyzer = s_pool.Allocate(); - analyzer.Initialize(semanticModel, syntaxFacts, objectCreationExpression, areCollectionExpressionsSupported, cancellationToken); - try - { - return analyzer.AnalyzeWorker(); - } - finally - { - analyzer.Clear(); - s_pool.Free(analyzer); - } + this.Initialize(semanticModel, syntaxFacts, objectCreationExpression, areCollectionExpressionsSupported, cancellationToken); + return this.AnalyzeWorker(); } protected override void AddMatches(ArrayBuilder> matches) @@ -196,28 +197,25 @@ bool TryProcessForeachStatement(TForeachStatementSyntax foreachStatement) // `[..expr]` elements. if (_analyzeForCollectionExpression) { - _syntaxFacts.GetPartsOfForeachStatement(foreachStatement, out var identifier, out _, out var foreachStatements); - if (identifier != default) + this.GetPartsOfForeachStatement(foreachStatement, out var identifier, out _, out var foreachStatements); + // must be of the form: + // + // foreach (var x in expr) + // dest.Add(x) + // + // By passing 'x' into TryAnalyzeInvocation below, we ensure that it is an enumerated value from `expr` + // being added to `dest`. + if (foreachStatements.ToImmutableArray() is [TExpressionStatementSyntax childExpressionStatement] && + TryAnalyzeInvocation( + childExpressionStatement, + addName: WellKnownMemberNames.CollectionInitializerAddMethodName, + requiredArgumentName: identifier.Text, + out var instance) && + ValuePatternMatches(instance)) { - // must be of the form: - // - // foreach (var x in expr) - // dest.Add(x) - // - // By passing 'x' into TryAnalyzeInvocation below, we ensure that it is an enumerated value from `expr` - // being added to `dest`. - if (foreachStatements.ToImmutableArray() is [TExpressionStatementSyntax childExpressionStatement] && - TryAnalyzeInvocation( - childExpressionStatement, - addName: WellKnownMemberNames.CollectionInitializerAddMethodName, - requiredArgumentName: identifier.Text, - out var instance) && - ValuePatternMatches(instance)) - { - // `foreach` will become `..expr` when we make it into a collection expression. - matches.Add(new Match(foreachStatement, UseSpread: true)); - return true; - } + // `foreach` will become `..expr` when we make it into a collection expression. + matches.Add(new Match(foreachStatement, UseSpread: true)); + return true; } } @@ -241,7 +239,7 @@ bool TryProcessIfStatement(TIfStatementSyntax ifStatement) // else // expr.Add(z) - _syntaxFacts.GetPartsOfIfStatement(ifStatement, out _, out var whenTrue, out var whenFalse); + this.GetPartsOfIfStatement(ifStatement, out _, out var whenTrue, out var whenFalse); var whenTrueStatements = whenTrue.ToImmutableArray(); var whenFalseStatements = whenFalse.ToImmutableArray(); diff --git a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerDiagnosticAnalyzer.cs b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerDiagnosticAnalyzer.cs index 5bfbed606b893..5e574ee96b2a7 100644 --- a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerDiagnosticAnalyzer.cs +++ b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerDiagnosticAnalyzer.cs @@ -62,7 +62,8 @@ internal abstract partial class AbstractUseCollectionInitializerDiagnosticAnalyz TExpressionStatementSyntax, TForeachStatementSyntax, TIfStatementSyntax, - TVariableDeclaratorSyntax> + TVariableDeclaratorSyntax, + TAnalyzer>, new() { public override DiagnosticAnalyzerCategory GetAnalyzerCategory() @@ -153,6 +154,7 @@ private void AnalyzeNode(SyntaxNodeAnalysisContext context, INamedTypeSymbol ien // Analyze the surrounding statements. First, try a broader set of statements if the language supports // collection expressions. + var syntaxFacts = GetSyntaxFacts(); using var analyzer = GetAnalyzer(); var result = GetCollectionExpressionMatches() ?? GetCollectionInitializerMatches(); if (result is null) @@ -161,7 +163,6 @@ private void AnalyzeNode(SyntaxNodeAnalysisContext context, INamedTypeSymbol ien var (matches, shouldUseCollectionExpression) = result.Value; var nodes = ImmutableArray.Create(containingStatement).AddRange(matches.Select(static m => m.Statement)); - var syntaxFacts = GetSyntaxFacts(); if (syntaxFacts.ContainsInterleavedDirective(nodes, cancellationToken)) return; @@ -180,8 +181,7 @@ private void AnalyzeNode(SyntaxNodeAnalysisContext context, INamedTypeSymbol ien (ImmutableArray> matches, bool shouldUseCollectionExpression)? GetCollectionInitializerMatches() { - var matches = analyzer.Analyze( - semanticModel, GetSyntaxFacts(), objectCreationExpression, areCollectionExpressionsSupported: false, cancellationToken); + var matches = analyzer.Analyze(semanticModel, syntaxFacts, objectCreationExpression, areCollectionExpressionsSupported: false, cancellationToken); // If analysis failed, we can't change this, no matter what. if (matches.IsDefault) @@ -202,8 +202,7 @@ private void AnalyzeNode(SyntaxNodeAnalysisContext context, INamedTypeSymbol ien if (!AreCollectionExpressionsSupported()) return null; - var matches = analyzer.Analyze( - semanticModel, GetSyntaxFacts(), objectCreationExpression, areCollectionExpressionsSupported: true, cancellationToken); + var matches = analyzer.Analyze(semanticModel, syntaxFacts, objectCreationExpression, areCollectionExpressionsSupported: true, cancellationToken); // If analysis failed, we can't change this, no matter what. if (matches.IsDefault) @@ -228,7 +227,6 @@ bool AreCollectionExpressionsSupported() if (!option.Value) return false; - var syntaxFacts = GetSyntaxFacts(); var arguments = syntaxFacts.GetArgumentsOfObjectCreationExpression(objectCreationExpression); if (arguments.Count != 0) return false; diff --git a/src/Analyzers/Core/CodeFixes/UseCollectionInitializer/AbstractUseCollectionInitializerCodeFixProvider.cs b/src/Analyzers/Core/CodeFixes/UseCollectionInitializer/AbstractUseCollectionInitializerCodeFixProvider.cs index 276a6756e3b12..92687c2e3bb15 100644 --- a/src/Analyzers/Core/CodeFixes/UseCollectionInitializer/AbstractUseCollectionInitializerCodeFixProvider.cs +++ b/src/Analyzers/Core/CodeFixes/UseCollectionInitializer/AbstractUseCollectionInitializerCodeFixProvider.cs @@ -31,7 +31,8 @@ internal abstract class AbstractUseCollectionInitializerCodeFixProvider< TExpressionStatementSyntax, TForeachStatementSyntax, TIfStatementSyntax, - TVariableDeclaratorSyntax> + TVariableDeclaratorSyntax, + TAnalyzer> : SyntaxEditorBasedCodeFixProvider where TSyntaxKind : struct where TExpressionSyntax : SyntaxNode @@ -43,10 +44,23 @@ internal abstract class AbstractUseCollectionInitializerCodeFixProvider< where TForeachStatementSyntax : TStatementSyntax where TIfStatementSyntax : TStatementSyntax where TVariableDeclaratorSyntax : SyntaxNode + where TAnalyzer : AbstractUseCollectionInitializerAnalyzer< + TExpressionSyntax, + TStatementSyntax, + TObjectCreationExpressionSyntax, + TMemberAccessExpressionSyntax, + TInvocationExpressionSyntax, + TExpressionStatementSyntax, + TForeachStatementSyntax, + TIfStatementSyntax, + TVariableDeclaratorSyntax, + TAnalyzer>, new() { public sealed override ImmutableArray FixableDiagnosticIds => ImmutableArray.Create(IDEDiagnosticIds.UseCollectionInitializerDiagnosticId); + protected abstract TAnalyzer GetAnalyzer(); + protected abstract TStatementSyntax GetNewStatement( SourceText sourceText, TStatementSyntax statement, TObjectCreationExpressionSyntax objectCreation, int wrappingLength, bool useCollectionExpression, ImmutableArray> matches); @@ -99,13 +113,14 @@ protected sealed override async Task FixAllAsync( #endif CodeActionOptions.DefaultCollectionExpressionWrappingLength; + using var analyzer = GetAnalyzer(); + while (originalObjectCreationNodes.Count > 0) { var (originalObjectCreation, useCollectionExpression) = originalObjectCreationNodes.Pop(); var objectCreation = currentRoot.GetCurrentNodes(originalObjectCreation).Single(); - var matches = UseCollectionInitializerAnalyzer.Analyze( - semanticModel, syntaxFacts, objectCreation, useCollectionExpression, cancellationToken); + var matches = analyzer.Analyze(semanticModel, syntaxFacts, objectCreation, useCollectionExpression, cancellationToken); if (matches.IsDefaultOrEmpty) continue; diff --git a/src/Analyzers/VisualBasic/Analyzers/UseCollectionInitializer/VisualBasicCollectionInitializerAnalyzer.vb b/src/Analyzers/VisualBasic/Analyzers/UseCollectionInitializer/VisualBasicCollectionInitializerAnalyzer.vb new file mode 100644 index 0000000000000..6196f0e80b9f3 --- /dev/null +++ b/src/Analyzers/VisualBasic/Analyzers/UseCollectionInitializer/VisualBasicCollectionInitializerAnalyzer.vb @@ -0,0 +1,32 @@ +' 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. + +Imports Microsoft.CodeAnalysis.UseCollectionInitializer +Imports Microsoft.CodeAnalysis.VisualBasic.Syntax + +Namespace Microsoft.CodeAnalysis.VisualBasic.UseCollectionInitializer + Friend NotInheritable Class VisualBasicCollectionInitializerAnalyzer + Inherits AbstractUseCollectionInitializerAnalyzer(Of + ExpressionSyntax, + StatementSyntax, + ObjectCreationExpressionSyntax, + MemberAccessExpressionSyntax, + InvocationExpressionSyntax, + ExpressionStatementSyntax, + ForEachStatementSyntax, + IfStatementSyntax, + VariableDeclaratorSyntax, + VisualBasicCollectionInitializerAnalyzer) + + Protected Overrides Sub GetPartsOfForeachStatement(statement As ForEachStatementSyntax, ByRef identifier As SyntaxToken, ByRef expression As ExpressionSyntax, ByRef statements As IEnumerable(Of StatementSyntax)) + ' Only called for collection expressions, which VB does not support + Throw ExceptionUtilities.Unreachable() + End Sub + + Protected Overrides Sub GetPartsOfIfStatement(statement As IfStatementSyntax, ByRef condition As ExpressionSyntax, ByRef whenTrueStatements As IEnumerable(Of StatementSyntax), ByRef whenFalseStatements As IEnumerable(Of StatementSyntax)) + ' Only called for collection expressions, which VB does not support + Throw ExceptionUtilities.Unreachable() + End Sub + End Class +End Namespace diff --git a/src/Analyzers/VisualBasic/Analyzers/UseCollectionInitializer/VisualBasicUseCollectionInitializerDiagnosticAnalyzer.vb b/src/Analyzers/VisualBasic/Analyzers/UseCollectionInitializer/VisualBasicUseCollectionInitializerDiagnosticAnalyzer.vb index c5c1f66a95662..4089e0c54adfc 100644 --- a/src/Analyzers/VisualBasic/Analyzers/UseCollectionInitializer/VisualBasicUseCollectionInitializerDiagnosticAnalyzer.vb +++ b/src/Analyzers/VisualBasic/Analyzers/UseCollectionInitializer/VisualBasicUseCollectionInitializerDiagnosticAnalyzer.vb @@ -11,7 +11,7 @@ Imports Microsoft.CodeAnalysis.VisualBasic.Syntax Namespace Microsoft.CodeAnalysis.VisualBasic.UseCollectionInitializer - Friend Class VisualBasicUseCollectionInitializerDiagnosticAnalyzer + Friend NotInheritable Class VisualBasicUseCollectionInitializerDiagnosticAnalyzer Inherits AbstractUseCollectionInitializerDiagnosticAnalyzer(Of SyntaxKind, ExpressionSyntax, @@ -21,7 +21,13 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.UseCollectionInitializer InvocationExpressionSyntax, ExpressionStatementSyntax, ForEachStatementSyntax, - VariableDeclaratorSyntax) + IfStatementSyntax, + VariableDeclaratorSyntax, + VisualBasicCollectionInitializerAnalyzer) + + Protected Overrides Function GetAnalyzer() As VisualBasicCollectionInitializerAnalyzer + Return VisualBasicCollectionInitializerAnalyzer.Allocate() + End Function Protected Overrides Function AreCollectionInitializersSupported(compilation As Compilation) As Boolean Return True diff --git a/src/Analyzers/VisualBasic/Analyzers/VisualBasicAnalyzers.projitems b/src/Analyzers/VisualBasic/Analyzers/VisualBasicAnalyzers.projitems index bd6a33fecf476..e2341a657e1aa 100644 --- a/src/Analyzers/VisualBasic/Analyzers/VisualBasicAnalyzers.projitems +++ b/src/Analyzers/VisualBasic/Analyzers/VisualBasicAnalyzers.projitems @@ -48,6 +48,7 @@ + diff --git a/src/Analyzers/VisualBasic/CodeFixes/UseCollectionInitializer/VisualBasicUseCollectionInitializerCodeFixProvider.vb b/src/Analyzers/VisualBasic/CodeFixes/UseCollectionInitializer/VisualBasicUseCollectionInitializerCodeFixProvider.vb index 5f2529f168a1c..07776cb3075f6 100644 --- a/src/Analyzers/VisualBasic/CodeFixes/UseCollectionInitializer/VisualBasicUseCollectionInitializerCodeFixProvider.vb +++ b/src/Analyzers/VisualBasic/CodeFixes/UseCollectionInitializer/VisualBasicUseCollectionInitializerCodeFixProvider.vb @@ -24,13 +24,19 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.UseCollectionInitializer InvocationExpressionSyntax, ExpressionStatementSyntax, ForEachStatementSyntax, - VariableDeclaratorSyntax) + IfStatementSyntax, + VariableDeclaratorSyntax, + VisualBasicCollectionInitializerAnalyzer) Public Sub New() End Sub + Protected Overrides Function GetAnalyzer() As VisualBasicCollectionInitializerAnalyzer + Return VisualBasicCollectionInitializerAnalyzer.Allocate() + End Function + Protected Overrides Function GetNewStatement( sourceText As SourceText, statement As StatementSyntax, diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/Services/SyntaxFacts/CSharpSyntaxFacts.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/Services/SyntaxFacts/CSharpSyntaxFacts.cs index 690b5736dcb20..9f3a1552c0aa5 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/Services/SyntaxFacts/CSharpSyntaxFacts.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/Services/SyntaxFacts/CSharpSyntaxFacts.cs @@ -1614,16 +1614,10 @@ public void GetPartsOfConditionalExpression(SyntaxNode node, out SyntaxNode cond whenFalse = conditionalExpression.WhenFalse; } - public void GetPartsOfForeachStatement(SyntaxNode statement, out SyntaxToken identifier, out SyntaxNode expression, out IEnumerable statements) + public SyntaxNode GetExpressionOfForeachStatement(SyntaxNode statement) { var commonForeach = (CommonForEachStatementSyntax)statement; - identifier = commonForeach is ForEachStatementSyntax { Identifier: var foreachIdentifier } - ? foreachIdentifier - : default; - expression = commonForeach.Expression; - statements = commonForeach.Statement is BlockSyntax block - ? block.Statements - : SpecializedCollections.SingletonEnumerable(commonForeach.Statement); + return commonForeach.Expression; } public void GetPartsOfGenericName(SyntaxNode node, out SyntaxToken identifier, out SeparatedSyntaxList typeArguments) diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Services/SyntaxFacts/ISyntaxFacts.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Services/SyntaxFacts/ISyntaxFacts.cs index 6f4325c4a36e3..c71f1225a6df9 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Services/SyntaxFacts/ISyntaxFacts.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Services/SyntaxFacts/ISyntaxFacts.cs @@ -198,9 +198,7 @@ internal interface ISyntaxFacts bool IsCastExpression([NotNullWhen(true)] SyntaxNode? node); bool IsExpressionOfForeach([NotNullWhen(true)] SyntaxNode? node); - void GetPartsOfForeachStatement(SyntaxNode statement, out SyntaxToken identifier, out SyntaxNode expression, out IEnumerable statements); - - void GetPartsOfIfStatement(SyntaxNode statement, out SyntaxNode condition, out IEnumerable whenTrueStatements, out IEnumerable whenFalseStatements); + SyntaxNode GetExpressionOfForeachStatement(SyntaxNode statement); void GetPartsOfTupleExpression(SyntaxNode node, out SyntaxToken openParen, out SeparatedSyntaxList arguments, out SyntaxToken closeParen) where TArgumentSyntax : SyntaxNode; diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Services/SyntaxFacts/ISyntaxFactsExtensions.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Services/SyntaxFacts/ISyntaxFactsExtensions.cs index 275f41c8ed63f..322e028bc75c9 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Services/SyntaxFacts/ISyntaxFactsExtensions.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Services/SyntaxFacts/ISyntaxFactsExtensions.cs @@ -518,12 +518,6 @@ public static SyntaxNode GetExpressionOfParenthesizedExpression(this ISyntaxFact return expression; } - public static SyntaxNode GetExpressionOfForeachStatement(this ISyntaxFacts syntaxFacts, SyntaxNode node) - { - syntaxFacts.GetPartsOfForeachStatement(node, out _, out var expression, out _); - return expression; - } - public static SyntaxToken GetIdentifierOfGenericName(this ISyntaxFacts syntaxFacts, SyntaxNode node) { syntaxFacts.GetPartsOfGenericName(node, out var identifier, out _); diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/VisualBasic/Services/SyntaxFacts/VisualBasicSyntaxFacts.vb b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/VisualBasic/Services/SyntaxFacts/VisualBasicSyntaxFacts.vb index 5be089c172fbb..e39797f051f14 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/VisualBasic/Services/SyntaxFacts/VisualBasicSyntaxFacts.vb +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/VisualBasic/Services/SyntaxFacts/VisualBasicSyntaxFacts.vb @@ -1813,20 +1813,9 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.LanguageService whenFalse = conditionalExpression.WhenFalse End Sub - Public Sub GetPartsOfForeachStatement(statement As SyntaxNode, ByRef identifier As SyntaxToken, ByRef expression As SyntaxNode, ByRef statements As IEnumerable(Of SyntaxNode)) Implements ISyntaxFacts.GetPartsOfForeachStatement - Dim foreachStatement = DirectCast(statement, ForEachStatementSyntax) - Dim declarator = TryCast(foreachStatement.ControlVariable, VariableDeclaratorSyntax) - identifier = If(declarator Is Nothing, Nothing, declarator.Names(0).Identifier) - expression = foreachStatement.Expression - - Dim foreachBlock = TryCast(foreachStatement.Parent, ForEachBlockSyntax) - statements = If(foreachBlock Is Nothing, SpecializedCollections.EmptyEnumerable(Of SyntaxNode), foreachBlock.Statements) - End Sub - - Public Sub GetPartsOfIfStatement(statement As IfStatementSyntax, ByRef condition As SyntaxNode, ByRef whenTrueStatements As IEnumerable(Of SyntaxNode), ByRef whenFalseStatements As IEnumerable(Of SyntaxNode)) Implements ISyntaxFacts.GetPartsOfIfStatement - Dim ifStatement = DirectCast(statement, IfStatementSyntax) - - End Sub + Public Function GetExpressionOfForeachStatement(statement As SyntaxNode) As SyntaxNode Implements ISyntaxFacts.GetExpressionOfForeachStatement + Return DirectCast(statement, ForEachStatementSyntax).Expression + End Function Public Sub GetPartsOfInterpolationExpression(node As SyntaxNode, ByRef stringStartToken As SyntaxToken, ByRef contents As SyntaxList(Of SyntaxNode), ByRef stringEndToken As SyntaxToken) Implements ISyntaxFacts.GetPartsOfInterpolationExpression Dim interpolatedStringExpression = DirectCast(node, InterpolatedStringExpressionSyntax) From a302e5dc9e688df8b93fec32cb13c2c2db86a1ee Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 28 Jul 2023 10:37:27 -0700 Subject: [PATCH 5/7] testS --- .../UseCollectionInitializerTests.cs | 116 +++++++++++++ ...onInitializerTests_CollectionExpression.cs | 164 ++++++++++++++++++ 2 files changed, 280 insertions(+) diff --git a/src/Analyzers/CSharp/Tests/UseCollectionInitializer/UseCollectionInitializerTests.cs b/src/Analyzers/CSharp/Tests/UseCollectionInitializer/UseCollectionInitializerTests.cs index a96d2931126fa..524a97c6c40ae 100644 --- a/src/Analyzers/CSharp/Tests/UseCollectionInitializer/UseCollectionInitializerTests.cs +++ b/src/Analyzers/CSharp/Tests/UseCollectionInitializer/UseCollectionInitializerTests.cs @@ -154,6 +154,122 @@ void M(int[] x) """); } + [Fact] + public async Task TestOnVariableDeclarator_If1() + { + await TestInRegularAndScriptAsync( + """ + using System.Collections.Generic; + + class C + { + void M(bool b) + { + var c = [|new|] List(); + [|c.Add(|]1); + if (b) + c.Add(2); + } + } + """, + """ + using System.Collections.Generic; + + class C + { + void M(bool b) + { + var c = new List + { + 1 + }; + if (b) + c.Add(2); + } + } + """); + } + + [Fact] + public async Task TestOnVariableDeclarator_If2() + { + await TestInRegularAndScriptAsync( + """ + using System.Collections.Generic; + + class C + { + void M(bool b) + { + var c = [|new|] List(); + [|c.Add(|]1); + if (b) + c.Add(2); + else + c.Add(3); + } + } + """, + """ + using System.Collections.Generic; + + class C + { + void M(bool b) + { + var c = new List + { + 1 + }; + if (b) + c.Add(2); + else + c.Add(3); + } + } + """); + } + + [Fact] + public async Task TestOnVariableDeclarator_If3() + { + await TestInRegularAndScriptAsync( + """ + using System.Collections.Generic; + + class C + { + void M(bool b) + { + var c = [|new|] List(); + [|c.Add(|]1); + if (b) + { + c.Add(2); + } + } + } + """, + """ + using System.Collections.Generic; + + class C + { + void M(bool b) + { + var c = new List + { + 1 + }; + if (b) + { + c.Add(2); + } + } + } + """); + } + [Fact] public async Task TestIndexAccess1() { diff --git a/src/Analyzers/CSharp/Tests/UseCollectionInitializer/UseCollectionInitializerTests_CollectionExpression.cs b/src/Analyzers/CSharp/Tests/UseCollectionInitializer/UseCollectionInitializerTests_CollectionExpression.cs index d856a6b4e1933..e0a54b8ab5dbf 100644 --- a/src/Analyzers/CSharp/Tests/UseCollectionInitializer/UseCollectionInitializerTests_CollectionExpression.cs +++ b/src/Analyzers/CSharp/Tests/UseCollectionInitializer/UseCollectionInitializerTests_CollectionExpression.cs @@ -97,6 +97,170 @@ void M() """); } + [Fact] + public async Task TestOnVariableDeclarator_If1() + { + await TestInRegularAndScriptAsync( + """ + using System.Collections.Generic; + + class C + { + void M(bool b) + { + var c = [|new|] List(); + [|c.Add(|]1); + if (b) + c.Add(2); + } + } + """, + """ + using System.Collections.Generic; + + class C + { + void M() + { + List c = new List + { + 1 + }; + if (b) + c.Add(2); + } + } + """); + } + + [Fact] + public async Task TestOnVariableDeclarator_If2() + { + await TestInRegularAndScriptAsync( + """ + using System.Collections.Generic; + + class C + { + void M(bool b) + { + var c = [|new|] List(); + [|c.Add(|]1); + if (b) + c.Add(2); + else + c.Add(3); + } + } + """, + """ + using System.Collections.Generic; + + class C + { + void M() + { + List c = new List + { + 1 + }; + if (b) + c.Add(2); + else + c.Add(3); + } + } + """); + } + + [Fact] + public async Task TestOnVariableDeclarator_If3() + { + await TestInRegularAndScriptAsync( + """ + using System.Collections.Generic; + + class C + { + void M(bool b) + { + List c = [|new|] List(); + [|c.Add(|]1); + if (b) + { + c.Add(2); + } + } + } + """, + """ + using System.Collections.Generic; + + class C + { + void M() + { + List c = new List + { + 1 + }; + if (b) + { + c.Add(2); + } + } + } + """); + } + + [Fact] + public async Task TestOnVariableDeclarator_If4() + { + await TestInRegularAndScriptAsync( + """ + using System.Collections.Generic; + + class C + { + void M(bool b) + { + List c = [|new|] List(); + [|c.Add(|]1); + if (b) + { + c.Add(2); + } + else + { + c.Add(3); + } + } + } + """, + """ + using System.Collections.Generic; + + class C + { + void M() + { + List c = new List + { + 1 + }; + if (b) + { + c.Add(2); + } + else + { + c.Add(3); + } + } + } + """); + } + [Fact] public async Task TestOnVariableDeclaratorDifferentType() { From e9858ea4db9d8f49d0a246c9ac4c6b25af310cdc Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 28 Jul 2023 11:55:26 -0700 Subject: [PATCH 6/7] Fixes and tests --- .../CSharpUseCollectionInitializerAnalyzer.cs | 7 +- ...CollectionInitializerDiagnosticAnalyzer.cs | 2 +- ...UseCollectionInitializerCodeFixProvider.cs | 94 +++-- ...harpUseObjectInitializerCodeFixProvider.cs | 2 +- .../UseInitializerHelpers.cs | 3 +- ...onInitializerTests_CollectionExpression.cs | 399 ++++++++++++++++-- ...bstractObjectCreationExpressionAnalyzer.cs | 6 +- ...bstractUseCollectionInitializerAnalyzer.cs | 41 +- ...CollectionInitializerDiagnosticAnalyzer.cs | 25 +- .../UseNamedMemberInitializerAnalyzer.cs | 4 +- ...UseCollectionInitializerCodeFixProvider.cs | 2 +- ...isualBasicCollectionInitializerAnalyzer.vb | 5 + 12 files changed, 498 insertions(+), 92 deletions(-) diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerAnalyzer.cs index 9b765929afe4c..940d4397dec4a 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerAnalyzer.cs @@ -21,6 +21,9 @@ internal sealed class CSharpUseCollectionInitializerAnalyzer : AbstractUseCollec VariableDeclaratorSyntax, CSharpUseCollectionInitializerAnalyzer> { + protected override bool IsComplexElementInitializer(SyntaxNode expression) + => expression.IsKind(SyntaxKind.ComplexElementInitializerExpression); + protected override void GetPartsOfForeachStatement( ForEachStatementSyntax statement, out SyntaxToken identifier, @@ -36,11 +39,11 @@ protected override void GetPartsOfIfStatement( IfStatementSyntax statement, out ExpressionSyntax condition, out IEnumerable whenTrueStatements, - out IEnumerable whenFalseStatements) + out IEnumerable? whenFalseStatements) { condition = statement.Condition; whenTrueStatements = ExtractEmbeddedStatements(statement.Statement); - whenFalseStatements = statement.Else != null ? ExtractEmbeddedStatements(statement.Else.Statement) : SpecializedCollections.EmptyEnumerable(); + whenFalseStatements = statement.Else != null ? ExtractEmbeddedStatements(statement.Else.Statement) : null; } private static IEnumerable ExtractEmbeddedStatements(StatementSyntax embeddedStatement) diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerDiagnosticAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerDiagnosticAnalyzer.cs index 448727c596144..d9ed0d600a1eb 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerDiagnosticAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerDiagnosticAnalyzer.cs @@ -44,5 +44,5 @@ protected override ISyntaxFacts GetSyntaxFacts() => CSharpSyntaxFacts.Instance; protected override bool CanUseCollectionExpression(SemanticModel semanticModel, BaseObjectCreationExpressionSyntax objectCreationExpression, CancellationToken cancellationToken) - => UseCollectionExpressionHelpers.CanReplaceWithCollectionExpression(semanticModel, objectCreationExpression, skipVerificationForReplacedNode: false, cancellationToken); + => UseCollectionExpressionHelpers.CanReplaceWithCollectionExpression(semanticModel, objectCreationExpression, skipVerificationForReplacedNode: true, cancellationToken); } diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider.cs index 113eb8952fbc0..036eb0e4a21d5 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider.cs +++ b/src/Analyzers/CSharp/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; @@ -67,7 +69,7 @@ private static ExpressionSyntax GetNewObjectCreation( ImmutableArray> matches) { return useCollectionExpression - ? CreateCollectionExpression(objectCreation, matches, MakeMultiLine(sourceText, objectCreation, matches, wrappingLength)) + ? CreateCollectionExpression(sourceText, objectCreation, wrappingLength, matches) : CreateObjectInitializerExpression(objectCreation, matches); } @@ -81,15 +83,16 @@ private static BaseObjectCreationExpressionSyntax CreateObjectInitializerExpress } private static CollectionExpressionSyntax CreateCollectionExpression( + SourceText sourceText, BaseObjectCreationExpressionSyntax objectCreation, - ImmutableArray> matches, - bool makeMultiLine) + int wrappingLength, + ImmutableArray> matches) { var elements = CreateElements( 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); @@ -145,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 { { A, B }, { C, D } }` then always make multiline. - // Similarly, if we have `new Dictionary { [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 match) - => match.Statement switch + static IEnumerable GetElementComponents(StatementSyntax statement) + { + if (statement is ExpressionStatementSyntax expressionStatement) + { + yield return expressionStatement.Expression; + } + else if (statement is ForEachStatementSyntax foreachStatement) + { + yield return foreachStatement.Expression; + } + else if (statement is IfStatementSyntax ifStatement) { - ExpressionStatementSyntax expressionStatement => expressionStatement.Expression, - ForEachStatementSyntax foreachStatement => foreachStatement.Expression, - _ => throw ExceptionUtilities.Unreachable(), - }; + 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 AddLineBreaks( SeparatedSyntaxList nodes, bool includeFinalLineBreak) where TNode : SyntaxNode @@ -212,7 +221,8 @@ private static SeparatedSyntaxList CreateElements( { using var _ = ArrayBuilder.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++) { @@ -246,6 +256,30 @@ private static SeparatedSyntaxList CreateElements( 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(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/src/Analyzers/CSharp/CodeFixes/UseObjectInitializer/CSharpUseObjectInitializerCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseObjectInitializer/CSharpUseObjectInitializerCodeFixProvider.cs index 7a2364acc67c5..24adaa5291dcd 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseObjectInitializer/CSharpUseObjectInitializerCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseObjectInitializer/CSharpUseObjectInitializerCodeFixProvider.cs @@ -56,7 +56,7 @@ private static SeparatedSyntaxList CreateExpressions( using var _ = ArrayBuilder.GetInstance(out var nodesAndTokens); UseInitializerHelpers.AddExistingItems( - 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 index 727f661e6c596..7b6585f193692 100644 --- 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( BaseObjectCreationExpressionSyntax objectCreation, ArrayBuilder nodesAndTokens, + bool addTrailingComma, Func createElement) where TMatch : struct where TElementSyntax : SyntaxNode @@ -51,7 +52,7 @@ public static void AddExistingItems( // 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(); diff --git a/src/Analyzers/CSharp/Tests/UseCollectionInitializer/UseCollectionInitializerTests_CollectionExpression.cs b/src/Analyzers/CSharp/Tests/UseCollectionInitializer/UseCollectionInitializerTests_CollectionExpression.cs index e0a54b8ab5dbf..b865d8a28b9e5 100644 --- a/src/Analyzers/CSharp/Tests/UseCollectionInitializer/UseCollectionInitializerTests_CollectionExpression.cs +++ b/src/Analyzers/CSharp/Tests/UseCollectionInitializer/UseCollectionInitializerTests_CollectionExpression.cs @@ -97,7 +97,7 @@ void M() """); } - [Fact] + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/69277")] public async Task TestOnVariableDeclarator_If1() { await TestInRegularAndScriptAsync( @@ -108,7 +108,7 @@ class C { void M(bool b) { - var c = [|new|] List(); + List c = [|new|] List(); [|c.Add(|]1); if (b) c.Add(2); @@ -120,14 +120,9 @@ void M(bool b) class C { - void M() + void M(bool b) { - List c = new List - { - 1 - }; - if (b) - c.Add(2); + List c = [1, .. {|CS0173:b ? [2] : []|}]; } } """); @@ -144,7 +139,7 @@ class C { void M(bool b) { - var c = [|new|] List(); + List c = [|new|] List(); [|c.Add(|]1); if (b) c.Add(2); @@ -158,23 +153,86 @@ void M(bool b) class C { - void M() + void M(bool b) { - List c = new List + List c = [1, b ? 2 : 3]; + } + } + """); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/69277")] + public async Task TestOnVariableDeclarator_If3() + { + await TestInRegularAndScriptAsync( + """ + using System.Collections.Generic; + + class C + { + void M(bool b) + { + List c = [|new|] List(); + [|c.Add(|]1); + if (b) { - 1 - }; + c.Add(2); + } + } + } + """, + """ + using System.Collections.Generic; + + class C + { + void M(bool b) + { + List c = [1, .. {|CS0173:b ? [2] : []|}]; + } + } + """); + } + + [Fact] + public async Task TestOnVariableDeclarator_If4() + { + await TestInRegularAndScriptAsync( + """ + using System.Collections.Generic; + + class C + { + void M(bool b) + { + List c = [|new|] List(); + [|c.Add(|]1); if (b) + { c.Add(2); + } else + { c.Add(3); + } + } + } + """, + """ + using System.Collections.Generic; + + class C + { + void M(bool b) + { + List c = [1, b ? 2 : 3]; } } """); } [Fact] - public async Task TestOnVariableDeclarator_If3() + public async Task TestOnVariableDeclarator_If5() { await TestInRegularAndScriptAsync( """ @@ -189,6 +247,7 @@ void M(bool b) if (b) { c.Add(2); + c.Add(3); } } } @@ -198,15 +257,13 @@ void M(bool b) class C { - void M() + void M(bool b) { - List c = new List - { - 1 - }; + List c = [1]; if (b) { c.Add(2); + c.Add(3); } } } @@ -214,7 +271,7 @@ void M() } [Fact] - public async Task TestOnVariableDeclarator_If4() + public async Task TestOnVariableDeclarator_If6() { await TestInRegularAndScriptAsync( """ @@ -233,6 +290,7 @@ void M(bool b) else { c.Add(3); + c.Add(4); } } } @@ -242,12 +300,9 @@ void M(bool b) class C { - void M() + void M(bool b) { - List c = new List - { - 1 - }; + List c = [1]; if (b) { c.Add(2); @@ -255,6 +310,85 @@ void M() else { c.Add(3); + c.Add(4); + } + } + } + """); + } + + [Fact] + public async Task TestOnVariableDeclarator_If7() + { + await TestInRegularAndScriptAsync( + """ + using System.Collections.Generic; + + class C + { + void M(bool b) + { + List c = [|new|] List(); + [|c.Add(|]1); + if (b) + { + } + } + } + """, + """ + using System.Collections.Generic; + + class C + { + void M(bool b) + { + List c = [1]; + if (b) + { + } + } + } + """); + } + + [Fact] + public async Task TestOnVariableDeclarator_If8() + { + await TestInRegularAndScriptAsync( + """ + using System.Collections.Generic; + + class C + { + void M(bool b) + { + List c = [|new|] List(); + [|c.Add(|]1); + if (b) + { + c.Add(2); + } + else + { + } + } + } + """, + """ + using System.Collections.Generic; + + class C + { + void M(bool b) + { + List c = [1]; + if (b) + { + c.Add(2); + } + else + { } } } @@ -1053,9 +1187,21 @@ void M(List[] array) } [Fact] - public async Task TestNotOnNamedArg() + public async Task TestOnNamedArg() { - await TestMissingInRegularAndScriptAsync( + await TestInRegularAndScriptAsync( + """ + using System.Collections.Generic; + + class C + { + void M() + { + List c = [|new|] List(); + c.Add(item: 1); + } + } + """, """ using System.Collections.Generic; @@ -1063,7 +1209,7 @@ class C { void M() { - List c = new List(); + List c = []; c.Add(item: 1); } } @@ -1516,9 +1662,9 @@ static void Main(string[] args) } [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/17823")] - public async Task TestMissingWhenReferencedInInitializer() + public async Task TestWhenReferencedInInitializer() { - await TestMissingInRegularAndScriptAsync( + await TestInRegularAndScriptAsync( """ using System.Collections.Generic; @@ -1526,7 +1672,19 @@ class C { static void M() { - List items = new List(); + List items = [|new|] List(); + items[0] = items[0]; + } + } + """, + """ + using System.Collections.Generic; + + class C + { + static void M() + { + List items = []; items[0] = items[0]; } } @@ -1645,7 +1803,20 @@ void M() [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/18260")] public async Task TestFieldReference() { - await TestMissingInRegularAndScriptAsync( + await TestInRegularAndScriptAsync( + """ + using System.Collections.Generic; + + class C + { + private List myField; + void M() + { + myField = [|new|] List(); + myField.Add(this.myField.Count); + } + } + """, """ using System.Collections.Generic; @@ -1654,7 +1825,7 @@ class C private List myField; void M() { - myField = new List(); + myField = []; myField.Add(this.myField.Count); } } @@ -1966,4 +2137,164 @@ await TestInRegularAndScriptAsync( """, OutputKind.ConsoleApplication); } + + [Fact] + public async Task TestUpdateExistingCollectionInitializerToExpression1() + { + await TestInRegularAndScriptAsync( + """ + using System.Collections.Generic; + + class C + { + void M() + { + List c = [|new|] List(); + } + } + """, + """ + using System.Collections.Generic; + + class C + { + void M() + { + List c = []; + } + } + """); + } + + [Fact] + public async Task TestUpdateExistingCollectionInitializerToExpression2() + { + await TestInRegularAndScriptAsync( + """ + using System.Collections.Generic; + + class C + { + void M() + { + List c = [|new|] List() + { + 1 + }; + } + } + """, + """ + using System.Collections.Generic; + + class C + { + void M() + { + List c = [1 + ]; + } + } + """); + } + + [Fact] + public async Task TestUpdateExistingCollectionInitializerToExpression3() + { + await TestInRegularAndScriptAsync( + """ + using System.Collections.Generic; + + class C + { + void M() + { + List c = [|new|] List() + { + 1, + }; + } + } + """, + """ + using System.Collections.Generic; + + class C + { + void M() + { + List c = [1, + ]; + } + } + """); + } + + [Fact] + public async Task TestUpdateExistingCollectionInitializerToExpression4() + { + await TestInRegularAndScriptAsync( + """ + using System.Collections.Generic; + + class C + { + void M() + { + List c = [|new|] List() + { + 1, + 2 + }; + } + } + """, + """ + using System.Collections.Generic; + + class C + { + void M() + { + List c = [1, + 2 + ]; + } + } + """); + } + + [Fact] + public async Task TestUpdateExistingCollectionInitializerToExpression5() + { + await TestInRegularAndScriptAsync( + """ + using System.Collections.Generic; + + class C + { + void M() + { + List c = [|new|] List() + { + 1, + 2, + }; + } + } + """, + """ + using System.Collections.Generic; + + class C + { + void M() + { + List c = [1, + 2, + ]; + } + } + """); + } } diff --git a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractObjectCreationExpressionAnalyzer.cs b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractObjectCreationExpressionAnalyzer.cs index f476ebe740045..347c1d98c1692 100644 --- a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractObjectCreationExpressionAnalyzer.cs +++ b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractObjectCreationExpressionAnalyzer.cs @@ -39,7 +39,7 @@ protected AbstractObjectCreationExpressionAnalyzer() } protected abstract bool ShouldAnalyze(); - protected abstract void AddMatches(ArrayBuilder matches); + protected abstract bool TryAddMatches(ArrayBuilder matches); public void Initialize( SemanticModel semanticModel, @@ -83,7 +83,9 @@ protected ImmutableArray AnalyzeWorker() } using var _ = ArrayBuilder.GetInstance(out var matches); - AddMatches(matches); + if (!TryAddMatches(matches)) + return default; + return matches.ToImmutable(); } diff --git a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerAnalyzer.cs b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerAnalyzer.cs index c70590c5491d7..b7c8806a80fce 100644 --- a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerAnalyzer.cs +++ b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerAnalyzer.cs @@ -53,8 +53,9 @@ internal abstract class AbstractUseCollectionInitializerAnalyzer< { private static readonly ObjectPool s_pool = SharedPools.Default(); + protected abstract bool IsComplexElementInitializer(SyntaxNode expression); protected abstract void GetPartsOfForeachStatement(TForeachStatementSyntax statement, out SyntaxToken identifier, out TExpressionSyntax expression, out IEnumerable statements); - protected abstract void GetPartsOfIfStatement(TIfStatementSyntax statement, out TExpressionSyntax condition, out IEnumerable whenTrueStatements, out IEnumerable whenFalseStatements); + protected abstract void GetPartsOfIfStatement(TIfStatementSyntax statement, out TExpressionSyntax condition, out IEnumerable whenTrueStatements, out IEnumerable? whenFalseStatements); public static TAnalyzer Allocate() => s_pool.Allocate(); @@ -73,10 +74,28 @@ public ImmutableArray> Analyze( CancellationToken cancellationToken) { this.Initialize(semanticModel, syntaxFacts, objectCreationExpression, areCollectionExpressionsSupported, cancellationToken); - return this.AnalyzeWorker(); + var result = this.AnalyzeWorker(); + + // If analysis failed entirely, immediately bail out. + if (result.IsDefault) + return default; + + // Analysis succeeded, but the result may be empty or non empty. + // + // For collection expressions, it's fine for this result to be empty. In other words, it's ok to offer + // changing `new List() { 1 }` (on its own) to `[1]`. + // + // However, for collection initializers we always want at least one element to add to the initializer. In + // 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 (areCollectionExpressionsSupported) + return result; + + // Downgrade an empty result to a failure for the normal collection-initializer case. + return result.IsEmpty ? default : result; } - protected override void AddMatches(ArrayBuilder> matches) + protected override bool TryAddMatches(ArrayBuilder> matches) { // If containing statement is inside a block (e.g. method), than we need to iterate through its child statements. // If containing statement is in top-level code, than we need to iterate through child statements of containing compilation unit. @@ -98,13 +117,19 @@ protected override void AddMatches(ArrayBuilder> matches if (initializer != null) { var firstInit = _syntaxFacts.GetExpressionsOfObjectCollectionInitializer(initializer).First(); + + // if we have an object creation, and it *already* has an initializer in it (like `new T { { x, y } }`) + // this can't legally become a collection expression. + if (_analyzeForCollectionExpression && this.IsComplexElementInitializer(firstInit)) + return false; + seenIndexAssignment = _syntaxFacts.IsElementAccessInitializer(firstInit); seenInvocation = !seenIndexAssignment; } // An indexer can't be used with a collection expression. So fail out immediately if we see that. if (seenIndexAssignment && _analyzeForCollectionExpression) - return; + return false; foreach (var child in containingBlockOrCompilationUnit.ChildNodesAndTokens()) { @@ -141,9 +166,11 @@ protected override void AddMatches(ArrayBuilder> matches } // Something we didn't understand. Stop here. - return; + break; } + return true; + bool TryProcessExpressionStatement(TExpressionStatementSyntax expressionStatement) { // Can't mix Adds and indexing. @@ -241,7 +268,6 @@ bool TryProcessIfStatement(TIfStatementSyntax ifStatement) this.GetPartsOfIfStatement(ifStatement, out _, out var whenTrue, out var whenFalse); var whenTrueStatements = whenTrue.ToImmutableArray(); - var whenFalseStatements = whenFalse.ToImmutableArray(); if (whenTrueStatements is [TExpressionStatementSyntax trueChildStatement] && TryAnalyzeInvocation( @@ -251,13 +277,14 @@ bool TryProcessIfStatement(TIfStatementSyntax ifStatement) out var instance) && ValuePatternMatches(instance)) { - if (whenTrueStatements.Length == 0) + if (whenFalse is null) { // add the form `.. x ? [y] : []` to the result matches.Add(new Match(ifStatement, UseSpread: true)); return true; } + var whenFalseStatements = whenFalse.ToImmutableArray(); if (whenFalseStatements is [TExpressionStatementSyntax falseChildStatement] && TryAnalyzeInvocation( falseChildStatement, diff --git a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerDiagnosticAnalyzer.cs b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerDiagnosticAnalyzer.cs index 5e574ee96b2a7..191d1bd5cd6f5 100644 --- a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerDiagnosticAnalyzer.cs +++ b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerDiagnosticAnalyzer.cs @@ -156,11 +156,21 @@ private void AnalyzeNode(SyntaxNodeAnalysisContext context, INamedTypeSymbol ien // collection expressions. var syntaxFacts = GetSyntaxFacts(); using var analyzer = GetAnalyzer(); - var result = GetCollectionExpressionMatches() ?? GetCollectionInitializerMatches(); - if (result is null) + + var collectionExpressionMatches = GetCollectionExpressionMatches(); + var collectionInitializerMatches = GetCollectionInitializerMatches(); + + // if both fail, we have nothing to offer. + if (collectionExpressionMatches is null && collectionInitializerMatches is null) return; - var (matches, shouldUseCollectionExpression) = result.Value; + // if one fails, prefer the other. If both succeed, prefer the one with more matches. + var (matches, shouldUseCollectionExpression) = + collectionExpressionMatches is null ? collectionInitializerMatches!.Value : + collectionInitializerMatches is null ? collectionExpressionMatches!.Value : + collectionExpressionMatches.Value.matches.Length >= collectionInitializerMatches.Value.matches.Length + ? collectionExpressionMatches.Value + : collectionInitializerMatches.Value; var nodes = ImmutableArray.Create(containingStatement).AddRange(matches.Select(static m => m.Statement)); if (syntaxFacts.ContainsInterleavedDirective(nodes, cancellationToken)) @@ -187,12 +197,6 @@ private void AnalyzeNode(SyntaxNodeAnalysisContext context, INamedTypeSymbol ien if (matches.IsDefault) return null; - // for collection initializers we always want at least one element to add to the initializer. In other - // words, we don't want to suggest changing `new List()` to `new List() { }` as that's just - // noise. - if (matches.IsEmpty) - return null; - return (matches, shouldUseCollectionExpression: false); } @@ -208,9 +212,6 @@ private void AnalyzeNode(SyntaxNodeAnalysisContext context, INamedTypeSymbol ien if (matches.IsDefault) return null; - // Analysis succeeded. Note: for collection expressions, it's fine for this result to be empty. In - // other words, it's ok to offer changing `new List() { 1 }` (on its own) to `[1]`. - // Check if it would actually be legal to use a collection expression here though. if (!CanUseCollectionExpression(semanticModel, objectCreationExpression, cancellationToken)) return null; diff --git a/src/Analyzers/Core/Analyzers/UseObjectInitializer/UseNamedMemberInitializerAnalyzer.cs b/src/Analyzers/Core/Analyzers/UseObjectInitializer/UseNamedMemberInitializerAnalyzer.cs index f011d650a7c18..1eb985b0a0e60 100644 --- a/src/Analyzers/Core/Analyzers/UseObjectInitializer/UseNamedMemberInitializerAnalyzer.cs +++ b/src/Analyzers/Core/Analyzers/UseObjectInitializer/UseNamedMemberInitializerAnalyzer.cs @@ -57,7 +57,7 @@ protected override bool ShouldAnalyze() return !_syntaxFacts.IsObjectCollectionInitializer(_syntaxFacts.GetInitializerOfBaseObjectCreationExpression(_objectCreationExpression)); } - protected override void AddMatches(ArrayBuilder> matches) + protected override bool TryAddMatches(ArrayBuilder> matches) { // If containing statement is inside a block (e.g. method), than we need to iterate through its child statements. // If containing statement is in top-level code, than we need to iterate through child statements of containing compilation unit. @@ -180,6 +180,8 @@ protected override void AddMatches(ArrayBuilder( statement, leftMemberAccess, rightExpression, typeMember?.Name ?? identifier.ValueText)); } + + return true; } private static bool IsExplicitlyImplemented( diff --git a/src/Analyzers/Core/CodeFixes/UseCollectionInitializer/AbstractUseCollectionInitializerCodeFixProvider.cs b/src/Analyzers/Core/CodeFixes/UseCollectionInitializer/AbstractUseCollectionInitializerCodeFixProvider.cs index 92687c2e3bb15..b570c6c94d3cc 100644 --- a/src/Analyzers/Core/CodeFixes/UseCollectionInitializer/AbstractUseCollectionInitializerCodeFixProvider.cs +++ b/src/Analyzers/Core/CodeFixes/UseCollectionInitializer/AbstractUseCollectionInitializerCodeFixProvider.cs @@ -122,7 +122,7 @@ protected sealed override async Task FixAllAsync( var matches = analyzer.Analyze(semanticModel, syntaxFacts, objectCreation, useCollectionExpression, cancellationToken); - if (matches.IsDefaultOrEmpty) + if (matches.IsDefault) continue; var statement = objectCreation.FirstAncestorOrSelf(); diff --git a/src/Analyzers/VisualBasic/Analyzers/UseCollectionInitializer/VisualBasicCollectionInitializerAnalyzer.vb b/src/Analyzers/VisualBasic/Analyzers/UseCollectionInitializer/VisualBasicCollectionInitializerAnalyzer.vb index 6196f0e80b9f3..5c08ec5f377c4 100644 --- a/src/Analyzers/VisualBasic/Analyzers/UseCollectionInitializer/VisualBasicCollectionInitializerAnalyzer.vb +++ b/src/Analyzers/VisualBasic/Analyzers/UseCollectionInitializer/VisualBasicCollectionInitializerAnalyzer.vb @@ -28,5 +28,10 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.UseCollectionInitializer ' Only called for collection expressions, which VB does not support Throw ExceptionUtilities.Unreachable() End Sub + + Protected Overrides Function IsComplexElementInitializer(expression As SyntaxNode) As Boolean + ' Only called for collection expressions, which VB does not support + Throw ExceptionUtilities.Unreachable() + End Function End Class End Namespace From 027d00c6bc0e8f85210222136140e7e5ddef10a0 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 31 Jul 2023 16:22:58 -0700 Subject: [PATCH 7/7] formattign --- .../CSharpUseCollectionInitializerAnalyzer.cs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerAnalyzer.cs index 940d4397dec4a..e499d8e269022 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerAnalyzer.cs @@ -11,15 +11,15 @@ namespace Microsoft.CodeAnalysis.CSharp.UseCollectionInitializer; internal sealed class CSharpUseCollectionInitializerAnalyzer : AbstractUseCollectionInitializerAnalyzer< ExpressionSyntax, - StatementSyntax, - BaseObjectCreationExpressionSyntax, - MemberAccessExpressionSyntax, - InvocationExpressionSyntax, - ExpressionStatementSyntax, - ForEachStatementSyntax, - IfStatementSyntax, - VariableDeclaratorSyntax, - CSharpUseCollectionInitializerAnalyzer> + StatementSyntax, + BaseObjectCreationExpressionSyntax, + MemberAccessExpressionSyntax, + InvocationExpressionSyntax, + ExpressionStatementSyntax, + ForEachStatementSyntax, + IfStatementSyntax, + VariableDeclaratorSyntax, + CSharpUseCollectionInitializerAnalyzer> { protected override bool IsComplexElementInitializer(SyntaxNode expression) => expression.IsKind(SyntaxKind.ComplexElementInitializerExpression);