diff --git a/src/Analyzers/CSharp/Analyzers/CSharpAnalyzers.projitems b/src/Analyzers/CSharp/Analyzers/CSharpAnalyzers.projitems index a13697111e9c7..5f550c427c2b5 100644 --- a/src/Analyzers/CSharp/Analyzers/CSharpAnalyzers.projitems +++ b/src/Analyzers/CSharp/Analyzers/CSharpAnalyzers.projitems @@ -82,7 +82,6 @@ - diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForArrayDiagnosticAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForArrayDiagnosticAnalyzer.cs index eacc1e33f9758..3e1c756b5b5d3 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForArrayDiagnosticAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForArrayDiagnosticAnalyzer.cs @@ -13,6 +13,7 @@ using Microsoft.CodeAnalysis.Shared.CodeStyle; using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.Text; +using Microsoft.CodeAnalysis.UseCollectionExpression; using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.CSharp.UseCollectionExpression; @@ -57,7 +58,7 @@ private void AnalyzeArrayCreationExpression(SyntaxNodeAnalysisContext context, I ReportArrayCreationDiagnostics(context, syntaxTree, option.Notification, arrayCreationExpression, changesSemantics); } - public static ImmutableArray> TryGetMatches( + public static ImmutableArray> TryGetMatches( SemanticModel semanticModel, ArrayCreationExpressionSyntax expression, CollectionExpressionSyntax replacementExpression, @@ -114,7 +115,7 @@ public static ImmutableArray> TryGetM return matches; } - public static ImmutableArray> TryGetMatches( + public static ImmutableArray> TryGetMatches( SemanticModel semanticModel, ImplicitArrayCreationExpressionSyntax expression, CollectionExpressionSyntax replacementExpression, diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForBuilderDiagnosticAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForBuilderDiagnosticAnalyzer.cs index ab5493c28229e..347115c0d6cb5 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForBuilderDiagnosticAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForBuilderDiagnosticAnalyzer.cs @@ -14,6 +14,7 @@ using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.PooledObjects; using Microsoft.CodeAnalysis.Shared.CodeStyle; +using Microsoft.CodeAnalysis.UseCollectionExpression; using Microsoft.CodeAnalysis.UseCollectionInitializer; using Roslyn.Utilities; @@ -159,7 +160,7 @@ void FadeOutCode(SyntaxNodeAnalysisContext context, AnalysisResult analysisResul identifier, initializedSymbol: semanticModel.GetDeclaredSymbol(declarator, cancellationToken)); - using var _ = ArrayBuilder>.GetInstance(out var matches); + using var _ = ArrayBuilder>.GetInstance(out var matches); // Now walk all the statement after the local declaration. using var enumerator = state.GetSubsequentStatements().GetEnumerator(); @@ -249,6 +250,6 @@ public readonly record struct AnalysisResult( Location DiagnosticLocation, LocalDeclarationStatementSyntax LocalDeclarationStatement, InvocationExpressionSyntax CreationExpression, - ImmutableArray> Matches, + ImmutableArray> Matches, bool ChangesSemantics); } diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForFluentDiagnosticAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForFluentDiagnosticAnalyzer.cs index 52084b99e4540..6d999c673739a 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForFluentDiagnosticAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForFluentDiagnosticAnalyzer.cs @@ -15,6 +15,7 @@ using Microsoft.CodeAnalysis.Shared.CodeStyle; using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.Text; +using Microsoft.CodeAnalysis.UseCollectionExpression; using Microsoft.CodeAnalysis.UseCollectionInitializer; using Roslyn.Utilities; @@ -133,9 +134,16 @@ private void AnalyzeMemberAccess(SyntaxNodeAnalysisContext context, INamedTypeSy { // Because we're recursing from top to bottom in the expression tree, we build up the matches in reverse. Right // before returning them, we'll reverse them again to get the proper order. - using var _ = ArrayBuilder>.GetInstance(out var matchesInReverse); - if (!AnalyzeInvocation(text, state, invocation, addMatches ? matchesInReverse : null, out var existingInitializer, cancellationToken)) + using var _1 = ArrayBuilder>.GetInstance(out var preMatchesInReverse); + using var _2 = ArrayBuilder>.GetInstance(out var postMatchesInReverse); + if (!AnalyzeInvocation( + text, state, invocation, + addMatches ? preMatchesInReverse : null, + addMatches ? postMatchesInReverse : null, + out var existingInitializer, cancellationToken)) + { return null; + } if (!CanReplaceWithCollectionExpression( state.SemanticModel, invocation, expressionType, isSingletonInstance: false, allowSemanticsChange, skipVerificationForReplacedNode: true, cancellationToken, out var changesSemantics)) @@ -143,18 +151,23 @@ private void AnalyzeMemberAccess(SyntaxNodeAnalysisContext context, INamedTypeSy return null; } - matchesInReverse.ReverseContents(); - return new AnalysisResult(existingInitializer, invocation, matchesInReverse.ToImmutable(), changesSemantics); + preMatchesInReverse.ReverseContents(); + postMatchesInReverse.ReverseContents(); + return new AnalysisResult(existingInitializer, invocation, preMatchesInReverse.ToImmutable(), postMatchesInReverse.ToImmutable(), changesSemantics); } private static bool AnalyzeInvocation( SourceText text, FluentState state, InvocationExpressionSyntax invocation, - ArrayBuilder>? matchesInReverse, + ArrayBuilder>? preMatchesInReverse, + ArrayBuilder>? postMatchesInReverse, out InitializerExpressionSyntax? existingInitializer, CancellationToken cancellationToken) { + var semanticModel = state.SemanticModel; + var compilation = semanticModel.Compilation; + existingInitializer = null; if (invocation.Expression is not MemberAccessExpressionSyntax memberAccess) return false; @@ -162,17 +175,16 @@ private static bool AnalyzeInvocation( // Topmost invocation must be a syntactic match for one of our collection manipulation forms. At the top level // we don't want to end with a linq method as that would be lazy, and a collection expression will eagerly // realize the collection. - if (!IsMatch(state, memberAccess, invocation, allowLinq: false, matchesInReverse, out var isAdditionMatch, cancellationToken)) + if (!IsMatch(state, memberAccess, invocation, allowLinq: false, postMatchesInReverse, out var isAdditionMatch, cancellationToken)) return false; // We don't want to offer this feature on top of some builder-type. They will commonly end with something like // `builder.ToImmutable()`. We want that case to be handled by the 'ForBuilder' analyzer instead. - var expressionType = state.SemanticModel.GetTypeInfo(memberAccess.Expression, cancellationToken).Type; + var expressionType = semanticModel.GetTypeInfo(memberAccess.Expression, cancellationToken).Type; if (expressionType is null || expressionType.Name.EndsWith("Builder", StringComparison.Ordinal)) return false; - var semanticModel = state.SemanticModel; - var compilation = semanticModel.Compilation; + var ienumerableOfTType = compilation.IEnumerableOfTType(); using var _1 = ArrayBuilder.GetInstance(out var stack); stack.Push(memberAccess.Expression); @@ -187,7 +199,7 @@ private static bool AnalyzeInvocation( // left hand side of the expression. In the inner expressions we can have things like `.Concat/.Append` // calls as the outer expressions will realize the collection. if (current is InvocationExpressionSyntax { Expression: MemberAccessExpressionSyntax currentMemberAccess } currentInvocation && - IsMatch(state, currentMemberAccess, currentInvocation, allowLinq: true, matchesInReverse, out _, cancellationToken)) + IsMatch(state, currentMemberAccess, currentInvocation, allowLinq: true, postMatchesInReverse, out _, cancellationToken)) { copiedData = true; stack.Push(currentMemberAccess.Expression); @@ -224,21 +236,44 @@ private static bool AnalyzeInvocation( { if (objectCreation is not { - ArgumentList: null or { Arguments.Count: 0 }, Initializer: null or { RawKind: (int)SyntaxKind.CollectionInitializerExpression } }) { return false; } + existingInitializer = objectCreation.Initializer; + if (!IsLegalInitializer(objectCreation.Initializer)) return false; if (!IsListLike(current)) return false; - existingInitializer = objectCreation.Initializer; - return true; + if (objectCreation.ArgumentList is { Arguments.Count: 1 }) + { + // Can take a single argument if that argument is itself a collection. + var argumentType = semanticModel.GetTypeInfo(objectCreation.ArgumentList.Arguments[0].Expression, cancellationToken).Type; + if (argumentType is null) + return false; + + if (!Equals(argumentType.OriginalDefinition, ienumerableOfTType) && + !argumentType.AllInterfaces.Any(i => Equals(i.OriginalDefinition, ienumerableOfTType))) + { + return false; + } + + // Add the arguments to the pre-matches. They will execute before the initializer values are added. + AddArgumentsInReverse(preMatchesInReverse, objectCreation.ArgumentList.Arguments, useSpread: true); + return true; + } + else if (objectCreation.ArgumentList is null or { Arguments.Count: 0 }) + { + // Otherwise, we have to have an empty argument list. + return true; + } + + return false; } // Forms like `ImmutableArray.Create(...)` or `ImmutableArray.CreateRange(...)` are fine base cases. @@ -248,7 +283,7 @@ private static bool AnalyzeInvocation( if (!IsListLike(current)) return false; - AddArgumentsInReverse(matchesInReverse, GetArguments(currentInvocationExpression, unwrapArgument), useSpread: false); + AddArgumentsInReverse(postMatchesInReverse, GetArguments(currentInvocationExpression, unwrapArgument), useSpread: false); return true; } @@ -277,13 +312,13 @@ private static bool AnalyzeInvocation( void AddFinalMatch(ExpressionSyntax expression) { - if (matchesInReverse is null) + if (postMatchesInReverse is null) return; // We're only adding one item to the final collection. So we're ending up with `[.. ]`. If this // originally was wrapped over multiple lines in a fluent fashion, and we're down to just a single wrapped // line, then unwrap. - if (matchesInReverse.Count == 0 && + if (postMatchesInReverse.Count == 0 && expression is InvocationExpressionSyntax { Expression: MemberAccessExpressionSyntax memberAccess } innerInvocation && text.Lines.GetLineFromPosition(expression.SpanStart).LineNumber + 1 == text.Lines.GetLineFromPosition(expression.Span.End).LineNumber && memberAccess.Expression.GetTrailingTrivia() @@ -292,7 +327,7 @@ void AddFinalMatch(ExpressionSyntax expression) .All(static t => t.IsWhitespaceOrEndOfLine())) { // Remove any whitespace around the `.`, making the singly-wrapped fluent expression into a single line. - matchesInReverse.Add(new CollectionExpressionMatch( + postMatchesInReverse.Add(new CollectionMatch( Argument(innerInvocation.WithExpression( memberAccess.Update( memberAccess.Expression.WithoutTrailingTrivia(), @@ -302,7 +337,7 @@ void AddFinalMatch(ExpressionSyntax expression) return; } - matchesInReverse.Add(new CollectionExpressionMatch(Argument(expression), UseSpread: true)); + postMatchesInReverse.Add(new CollectionMatch(Argument(expression), UseSpread: true)); } // We only want to offer this feature when the original collection was list-like (as opposed to being something @@ -359,12 +394,13 @@ static bool IsLegalInitializer(InitializerExpressionSyntax? initializer) return false; } } + return true; } } private static void AddArgumentsInReverse( - ArrayBuilder>? matchesInReverse, + ArrayBuilder>? matchesInReverse, SeparatedSyntaxList arguments, bool useSpread) { @@ -388,7 +424,7 @@ private static bool IsMatch( MemberAccessExpressionSyntax memberAccess, InvocationExpressionSyntax invocation, bool allowLinq, - ArrayBuilder>? matchesInReverse, + ArrayBuilder>? matchesInReverse, out bool isAdditionMatch, CancellationToken cancellationToken) { @@ -467,12 +503,15 @@ static bool HasAnySuffix(string name) /// help determine the best collection expression final syntax. /// The location of the code like builder.ToImmutable() that will actually be /// replaced with the collection expression - /// The arguments being added to the collection that will be converted into elements in the - /// final collection expression. + /// The arguments being added to the collection that will be converted into elements in + /// the final collection expression *before* the existing initializer elements. + /// The arguments being added to the collection that will be converted into elements in + /// the final collection expression *after* the existing initializer elements. public readonly record struct AnalysisResult( // Location DiagnosticLocation, InitializerExpressionSyntax? ExistingInitializer, InvocationExpressionSyntax CreationExpression, - ImmutableArray> Matches, + ImmutableArray> PreMatches, + ImmutableArray> PostMatches, bool ChangesSemantics); } diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocDiagnosticAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocDiagnosticAnalyzer.cs index 607899dd00c26..3fea21dfca3c3 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocDiagnosticAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocDiagnosticAnalyzer.cs @@ -9,6 +9,7 @@ using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Shared.CodeStyle; using Microsoft.CodeAnalysis.Text; +using Microsoft.CodeAnalysis.UseCollectionExpression; namespace Microsoft.CodeAnalysis.CSharp.UseCollectionExpression; @@ -119,7 +120,7 @@ private void AnalyzeExplicitStackAllocExpression(SyntaxNodeAnalysisContext conte additionalUnnecessaryLocations: additionalUnnecessaryLocations)); } - public static ImmutableArray> TryGetMatches( + public static ImmutableArray> TryGetMatches( SemanticModel semanticModel, StackAllocArrayCreationExpressionSyntax expression, INamedTypeSymbol? expressionType, diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CollectionExpressionMatch.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CollectionExpressionMatch.cs deleted file mode 100644 index 8528791948c89..0000000000000 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CollectionExpressionMatch.cs +++ /dev/null @@ -1,12 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. -// See the LICENSE file in the project root for more information. - -using Microsoft.CodeAnalysis.UseCollectionInitializer; - -namespace Microsoft.CodeAnalysis.CSharp.UseCollectionExpression; - -/// -internal readonly record struct CollectionExpressionMatch( - TMatchNode Node, - bool UseSpread) where TMatchNode : SyntaxNode; diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/UseCollectionExpressionHelpers.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/UseCollectionExpressionHelpers.cs index bedba7efb662a..f7fab29e21d2e 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/UseCollectionExpressionHelpers.cs +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/UseCollectionExpressionHelpers.cs @@ -17,6 +17,7 @@ using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.Shared.Utilities; using Microsoft.CodeAnalysis.Text; +using Microsoft.CodeAnalysis.UseCollectionExpression; using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.CSharp.UseCollectionExpression; @@ -814,7 +815,7 @@ private static bool ShouldReplaceExistingExpressionEntirely( return false; } - public static ImmutableArray> TryGetMatches( + public static ImmutableArray> TryGetMatches( SemanticModel semanticModel, TArrayCreationExpressionSyntax expression, CollectionExpressionSyntax replacementExpression, @@ -835,7 +836,7 @@ public static ImmutableArray> TryGetM if (getType(expression) is not ArrayTypeSyntax { RankSpecifiers: [{ Sizes: [var size] }, ..] }) return default; - using var _ = ArrayBuilder>.GetInstance(out var matches); + using var _ = ArrayBuilder>.GetInstance(out var matches); var initializer = getInitializer(expression); if (size is OmittedArraySizeExpressionSyntax) diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerAnalyzer.cs index 7c2abecc18d1f..fabc39b46613b 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerAnalyzer.cs @@ -3,10 +3,13 @@ // See the LICENSE file in the project root for more information. using System.Diagnostics.CodeAnalysis; +using System.Linq; using System.Threading; using Microsoft.CodeAnalysis.CSharp.Extensions; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.PooledObjects; +using Microsoft.CodeAnalysis.Shared.Extensions; +using Microsoft.CodeAnalysis.UseCollectionExpression; using Microsoft.CodeAnalysis.UseCollectionInitializer; namespace Microsoft.CodeAnalysis.CSharp.UseCollectionInitializer; @@ -42,136 +45,155 @@ protected override bool HasExistingInvalidInitializerForCollection() }; } - protected override bool ValidateMatchesForCollectionExpression( - ArrayBuilder> matches, CancellationToken cancellationToken) + protected override bool AnalyzeMatchesAndCollectionConstructorForCollectionExpression( + ArrayBuilder> preMatches, + ArrayBuilder> postMatches, + CancellationToken cancellationToken) { // Constructor wasn't called with any arguments. Nothing to validate. var argumentList = _objectCreationExpression.ArgumentList; if (argumentList is null || argumentList.Arguments.Count == 0) return true; - // Anything beyond just a single capacity argument isn't anything we can handle. - if (argumentList.Arguments.Count >= 2) + // Anything beyond just a single capacity argument (or single value to populate the collection with) isn't + // anything we can handle. + if (argumentList.Arguments.Count != 1) return false; - // must be a single `int capacity` constructor. if (this.SemanticModel.GetSymbolInfo(_objectCreationExpression, cancellationToken).Symbol is not IMethodSymbol { MethodKind: MethodKind.Constructor, - Parameters: [{ Type.SpecialType: SpecialType.System_Int32, Name: "capacity" }], + Parameters.Length: 1, } constructor) { return false; } - // The original collection could have been passed elements explicitly in its initializer. Ensure we account for - // that as well. - var individualElementCount = _objectCreationExpression.Initializer?.Expressions.Count ?? 0; - - // Walk the matches, determining what individual elements are added as-is, as well as what values are going to - // be spread into the final collection. We'll then ensure a correspondance between both and the expression the - // user is currently passing to the 'capacity' argument to make sure they're entirely congruent. - using var _1 = ArrayBuilder.GetInstance(out var spreadElements); - foreach (var match in matches) + var ienumerableOfTType = this.SemanticModel.Compilation.IEnumerableOfTType(); + var firstParameter = constructor.Parameters[0]; + if (Equals(firstParameter.Type.OriginalDefinition, ienumerableOfTType) || + firstParameter.Type.AllInterfaces.Any(i => Equals(i.OriginalDefinition, ienumerableOfTType))) { - switch (match.Statement) - { - case ExpressionStatementSyntax { Expression: InvocationExpressionSyntax invocation } expressionStatement: - // x.AddRange(y). Have to make sure we see y.Count in the capacity list. - // x.Add(y, z). Increment the total number of elements by the arg count. - if (match.UseSpread) - spreadElements.Add(invocation.ArgumentList.Arguments[0].Expression); - else - individualElementCount += invocation.ArgumentList.Arguments.Count; - - continue; - - case ForEachStatementSyntax foreachStatement: - // foreach (var v in expr) x.Add(v). Have to make sure we see expr.Count in the capacity list. - spreadElements.Add(foreachStatement.Expression); - continue; - - default: - // Something we don't support (yet). - return false; - } + // Took a single argument that implements IEnumerable. We handle this by spreading that argument as the + // first thing added to the collection. + preMatches.Add(new(argumentList.Arguments[0].Expression, UseSpread: true)); + return true; } + else if (firstParameter is { Type.SpecialType: SpecialType.System_Int32, Name: "capacity" }) + { + // is a single `int capacity` constructor. - // Now, break up an expression like `1 + x.Length + y.Count` into the parts separated by the +'s - var currentArgumentExpression = argumentList.Arguments[0].Expression; - using var _2 = ArrayBuilder.GetInstance(out var expressionPieces); + // The original collection could have been passed elements explicitly in its initializer. Ensure we account for + // that as well. + var individualElementCount = _objectCreationExpression.Initializer?.Expressions.Count ?? 0; - while (true) - { - if (currentArgumentExpression is BinaryExpressionSyntax binaryExpression) + // Walk the matches, determining what individual elements are added as-is, as well as what values are going to + // be spread into the final collection. We'll then ensure a correspondance between both and the expression the + // user is currently passing to the 'capacity' argument to make sure they're entirely congruent. + using var _1 = ArrayBuilder.GetInstance(out var spreadElements); + foreach (var match in postMatches) { - if (binaryExpression.Kind() != SyntaxKind.AddExpression) - return false; - - expressionPieces.Add(binaryExpression.Right); - currentArgumentExpression = binaryExpression.Left; + switch (match.Node) + { + case ExpressionStatementSyntax { Expression: InvocationExpressionSyntax invocation } expressionStatement: + // x.AddRange(y). Have to make sure we see y.Count in the capacity list. + // x.Add(y, z). Increment the total number of elements by the arg count. + if (match.UseSpread) + spreadElements.Add(invocation.ArgumentList.Arguments[0].Expression); + else + individualElementCount += invocation.ArgumentList.Arguments.Count; + + continue; + + case ForEachStatementSyntax foreachStatement: + // foreach (var v in expr) x.Add(v). Have to make sure we see expr.Count in the capacity list. + spreadElements.Add(foreachStatement.Expression); + continue; + + default: + // Something we don't support (yet). + return false; + } } - else + + // Now, break up an expression like `1 + x.Length + y.Count` into the parts separated by the +'s + var currentArgumentExpression = argumentList.Arguments[0].Expression; + using var _2 = ArrayBuilder.GetInstance(out var expressionPieces); + + while (true) { - expressionPieces.Add(currentArgumentExpression); - break; + if (currentArgumentExpression is BinaryExpressionSyntax binaryExpression) + { + if (binaryExpression.Kind() != SyntaxKind.AddExpression) + return false; + + expressionPieces.Add(binaryExpression.Right); + currentArgumentExpression = binaryExpression.Left; + } + else + { + expressionPieces.Add(currentArgumentExpression); + break; + } } - } - // Determine the total constant value provided in the expression. For each constant we see, remove that - // constant from the pieces list. That way the pieces list only corresponds to the values to spread. - var totalConstantValue = 0; - for (var i = expressionPieces.Count - 1; i >= 0; i--) - { - var piece = expressionPieces[i]; - var constant = this.SemanticModel.GetConstantValue(piece, cancellationToken); - if (constant.Value is int value) + // Determine the total constant value provided in the expression. For each constant we see, remove that + // constant from the pieces list. That way the pieces list only corresponds to the values to spread. + var totalConstantValue = 0; + for (var i = expressionPieces.Count - 1; i >= 0; i--) { - totalConstantValue += value; - expressionPieces.RemoveAt(i); + var piece = expressionPieces[i]; + var constant = this.SemanticModel.GetConstantValue(piece, cancellationToken); + if (constant.Value is int value) + { + totalConstantValue += value; + expressionPieces.RemoveAt(i); + } } - } - // If the constant didn't match the number of individual elements to add, we can't update this code. - if (totalConstantValue != individualElementCount) - return false; + // If the constant didn't match the number of individual elements to add, we can't update this code. + if (totalConstantValue != individualElementCount) + return false; - // After removing the constants, we should have an expression for each value we're going to spread. - if (expressionPieces.Count != spreadElements.Count) - return false; + // After removing the constants, we should have an expression for each value we're going to spread. + if (expressionPieces.Count != spreadElements.Count) + return false; - // Now make sure we have a match for each part of `x.Length + y.Length` to an element being spread - // into the collection. - foreach (var piece in expressionPieces) - { - // we support x.Length, x.Count, and x.Count() - var current = piece; - if (piece is InvocationExpressionSyntax invocationExpression) + // Now make sure we have a match for each part of `x.Length + y.Length` to an element being spread + // into the collection. + foreach (var piece in expressionPieces) { - if (invocationExpression.ArgumentList.Arguments.Count != 0) + // we support x.Length, x.Count, and x.Count() + var current = piece; + if (piece is InvocationExpressionSyntax invocationExpression) + { + if (invocationExpression.ArgumentList.Arguments.Count != 0) + return false; + + current = invocationExpression.Expression; + } + + if (current is not MemberAccessExpressionSyntax(SyntaxKind.SimpleMemberAccessExpression) { Name.Identifier.ValueText: "Length" or "Count" } memberAccess) return false; - current = invocationExpression.Expression; - } + current = memberAccess.Expression; - if (current is not MemberAccessExpressionSyntax(SyntaxKind.SimpleMemberAccessExpression) { Name.Identifier.ValueText: "Length" or "Count" } memberAccess) - return false; + // Now see if we have an item we're spreading matching 'x'. + var matchIndex = spreadElements.FindIndex(SyntaxFacts.AreEquivalent, current); + if (matchIndex < 0) + return false; - current = memberAccess.Expression; + spreadElements.RemoveAt(matchIndex); + } - // Now see if we have an item we're spreading matching 'x'. - var matchIndex = spreadElements.FindIndex(SyntaxFacts.AreEquivalent, current); - if (matchIndex < 0) + // If we had any spread elements remaining we can't proceed. + if (spreadElements.Count > 0) return false; - spreadElements.RemoveAt(matchIndex); + // We're all good. The items we found matches up precisely to the capacity provided! + return true; } - // If we had any spread elements remaining we can't proceed. - if (spreadElements.Count > 0) - return false; - - // We're all good. The items we found matches up precisely to the capacity provided! - return true; + return false; } } diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpCollectionExpressionRewriter.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpCollectionExpressionRewriter.cs index c5f2083c7c2a7..23b878b52e152 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpCollectionExpressionRewriter.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpCollectionExpressionRewriter.cs @@ -5,7 +5,10 @@ using System; using System.Collections.Generic; using System.Collections.Immutable; +using System.Diagnostics; using System.Linq; +using System.Linq.Expressions; +using System.Text.RegularExpressions; using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis.CodeActions; @@ -17,6 +20,7 @@ using Microsoft.CodeAnalysis.PooledObjects; using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.Text; +using Microsoft.CodeAnalysis.UseCollectionExpression; using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.CSharp.UseCollectionExpression; @@ -33,7 +37,8 @@ internal static class CSharpCollectionExpressionRewriter public static async Task CreateCollectionExpressionAsync( Document workspaceDocument, TParentExpression expressionToReplace, - ImmutableArray> matches, + ImmutableArray> preMatches, + ImmutableArray> postMatches, Func getInitializer, Func withInitializer, CancellationToken cancellationToken) @@ -80,22 +85,13 @@ CollectionExpressionSyntax CreateCollectionExpressionWithoutExistingElements() // Didn't have an existing initializer (or it was empty). For both cases, just create an entirely // fresh collection expression, and replace the object entirely. - if (matches is [{ Node: ExpressionSyntax expression } match]) + if (preMatches is [{ Node: ExpressionSyntax } preMatch] && postMatches.IsEmpty) { - // Specialize when we're taking some expression (like x.y.ToArray()) and converting to a spreaded - // collection expression. We just want to trivially make that `[.. x.y]` without any specialized - // behavior. In particular, we do not want to generate something like: - // - // [ - // .. x.y, - // ] - // - // For that sort of case. Single element collections should stay closely associated with the original - // expression. - return CollectionExpression([ - match.UseSpread - ? SpreadElement(expression.WithoutTrivia()) - : ExpressionElement(expression.WithoutTrivia())]).WithTriviaFrom(expressionToReplace); + return CreateSingleElementCollection(preMatch); + } + else if (preMatches.IsEmpty && postMatches is [{ Node: ExpressionSyntax } postMatch]) + { + return CreateSingleElementCollection(postMatch); } else if (makeMultiLineCollectionExpression) { @@ -128,7 +124,8 @@ CollectionExpressionSyntax CreateCollectionExpressionWithoutExistingElements() // now create the elements, following that indentation preference. using var _ = ArrayBuilder.GetInstance(out var nodesAndTokens); - CreateAndAddElements(matches, nodesAndTokens, preferredIndentation: elementIndentation, forceTrailingComma: true); + CreateAndAddElements(preMatches, nodesAndTokens, preferredIndentation: elementIndentation, forceTrailingComma: true, moreToCome: true); + CreateAndAddElements(postMatches, nodesAndTokens, preferredIndentation: elementIndentation, forceTrailingComma: true, moreToCome: false); // Add a newline between the last element and the close bracket if we don't already have one. if (nodesAndTokens.Count > 0 && nodesAndTokens.Last().GetTrailingTrivia() is [.., (kind: not SyntaxKind.EndOfLineTrivia)]) @@ -150,7 +147,8 @@ CollectionExpressionSyntax CreateCollectionExpressionWithoutExistingElements() // fresh collection expression, and do a wholesale replacement of the original object creation // expression with it. using var _ = ArrayBuilder.GetInstance(out var nodesAndTokens); - CreateAndAddElements(matches, nodesAndTokens, preferredIndentation: null, forceTrailingComma: false); + CreateAndAddElements(preMatches, nodesAndTokens, preferredIndentation: null, forceTrailingComma: false, moreToCome: true); + CreateAndAddElements(postMatches, nodesAndTokens, preferredIndentation: null, forceTrailingComma: false, moreToCome: false); // Remove any trailing whitespace from the last element/comma and the final close bracket. if (nodesAndTokens.Count > 0) @@ -196,6 +194,25 @@ CollectionExpressionSyntax CreateCollectionExpressionWithoutExistingElements() } } + CollectionExpressionSyntax CreateSingleElementCollection(CollectionMatch match) + { + // Specialize when we're taking some expression (like x.y.ToArray()) and converting to a spreaded + // collection expression. We just want to trivially make that `[.. x.y]` without any specialized + // behavior. In particular, we do not want to generate something like: + // + // [ + // .. x.y, + // ] + // + // For that sort of case. Single element collections should stay closely associated with the original + // expression. + var expression = (ExpressionSyntax)(object)match.Node; + return CollectionExpression([ + match.UseSpread + ? SpreadElement(expression.WithoutTrivia()) + : ExpressionElement(expression.WithoutTrivia())]).WithTriviaFrom(expressionToReplace); + } + CollectionExpressionSyntax CreateCollectionExpressionWithExistingElements() { // If the object creation expression had an initializer (with at least one element in it). Attempt to @@ -325,10 +342,11 @@ SeparatedSyntaxList FixLeadingAndTrailingWhitespace( // Used to we can uniformly add the items correctly with the requested (but optional) indentation. And so that // commas are added properly to the sequence. void CreateAndAddElements( - ImmutableArray> matches, + ImmutableArray> matches, ArrayBuilder nodesAndTokens, string? preferredIndentation, - bool forceTrailingComma) + bool forceTrailingComma, + bool moreToCome) { // If there's no requested indentation, then we want to produce the sequence as: `a, b, c, d`. So just // a space after any comma. If there is desired indentation for an element, then we always follow a comma @@ -344,7 +362,7 @@ void CreateAndAddElements( } if (matches.Length > 0 && forceTrailingComma) - AddCommaIfMissing(last: true); + AddCommaIfMissing(last: !moreToCome); return; @@ -379,6 +397,11 @@ CollectionExpressionSyntax AddMatchesToExistingNonEmptyCollectionExpression( string? preferredIndentation) { using var _ = ArrayBuilder.GetInstance(out var nodesAndTokens); + + // Add any pre-items before the initializer items. + CreateAndAddElements(preMatches, nodesAndTokens, preferredIndentation, forceTrailingComma: true, moreToCome: true); + + // Now add all the initializer items. nodesAndTokens.AddRange(initialCollectionExpression.Elements.GetWithSeparators()); // If there is already a trailing comma before, remove it. We'll add it back at the end. If there is no @@ -397,12 +420,15 @@ CollectionExpressionSyntax AddMatchesToExistingNonEmptyCollectionExpression( nodesAndTokens[^1] = nodesAndTokens[^1].WithTrailingTrivia(); } + // Now add all the post matches in. + // If we're wrapping to multiple lines, and we don't already have a trailing comma, then force one at the // end. This keeps every element consistent with ending the line with a comma, which makes code easier to // maintain. CreateAndAddElements( - matches, nodesAndTokens, preferredIndentation, - forceTrailingComma: preferredIndentation != null && trailingComma == default); + postMatches, nodesAndTokens, preferredIndentation, + forceTrailingComma: preferredIndentation != null && trailingComma == default, + moreToCome: false); if (trailingComma != default) { @@ -431,7 +457,7 @@ static CollectionElementSyntax CreateCollectionElement( } IEnumerable CreateElements( - CollectionExpressionMatch match, string? preferredIndentation) + CollectionMatch match, string? preferredIndentation) { var node = match.Node; @@ -701,7 +727,7 @@ bool MakeMultiLineCollectionExpression() { // If there's already an initializer, and we're not adding anything to it, then just keep the initializer // as-is. No need to convert it to be multi-line if it's currently single-line. - if (initializer != null && matches.Length == 0) + if (initializer != null && preMatches.Length == 0 && postMatches.Length == 0) return false; var totalLength = 0; @@ -711,27 +737,38 @@ bool MakeMultiLineCollectionExpression() totalLength += expression.Span.Length; } - foreach (var (node, _) in matches) + if (CheckForMultiLine(preMatches) || + CheckForMultiLine(postMatches)) { - // if the statement we're replacing has any comments on it, then we need to be multiline to give them an - // appropriate place to go. - if (node.GetLeadingTrivia().Any(static t => t.IsSingleOrMultiLineComment()) || - node.GetTrailingTrivia().Any(static t => t.IsSingleOrMultiLineComment())) - { - return true; - } + return true; + } - foreach (var component in GetElementComponents(node)) + return totalLength > wrappingLength; + + bool CheckForMultiLine(ImmutableArray> matches) + { + foreach (var (node, _) in matches) { - // if any of the expressions we're adding are multiline, then make things multiline. - if (!document.Text.AreOnSameLine(component.GetFirstToken(), component.GetLastToken())) + // if the statement we're replacing has any comments on it, then we need to be multiline to give them an + // appropriate place to go. + if (node.GetLeadingTrivia().Any(static t => t.IsSingleOrMultiLineComment()) || + node.GetTrailingTrivia().Any(static t => t.IsSingleOrMultiLineComment())) + { return true; + } + + foreach (var component in GetElementComponents(node)) + { + // if any of the expressions we're adding are multiline, then make things multiline. + if (!document.Text.AreOnSameLine(component.GetFirstToken(), component.GetLastToken())) + return true; - totalLength += component.Span.Length; + totalLength += component.Span.Length; + } } - } - return totalLength > wrappingLength; + return false; + } } static IEnumerable GetElementComponents(TMatchNode node) diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForArrayCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForArrayCodeFixProvider.cs index 3ca81464e419a..2340ebb0b3fa2 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForArrayCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForArrayCodeFixProvider.cs @@ -7,7 +7,6 @@ using System.Composition; using System.Threading; using System.Threading.Tasks; -using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.CodeFixes; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Diagnostics; @@ -72,6 +71,7 @@ and not ImplicitArrayCreationExpressionSyntax var collectionExpression = await CSharpCollectionExpressionRewriter.CreateCollectionExpressionAsync( document, arrayCreationExpression, + preMatches: [], matches, static e => e switch { @@ -97,7 +97,7 @@ and not ImplicitArrayCreationExpressionSyntax static bool IsOnSingleLine(SourceText sourceText, SyntaxNode node) => sourceText.AreOnSameLine(node.GetFirstToken(), node.GetLastToken()); - ImmutableArray> GetMatches( + ImmutableArray> GetMatches( SemanticModel semanticModel, ExpressionSyntax expression, INamedTypeSymbol? expressionType) => expression switch { diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForBuilderCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForBuilderCodeFixProvider.cs index b3a863bc56df9..65ac94dcec1bd 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForBuilderCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForBuilderCodeFixProvider.cs @@ -8,7 +8,6 @@ using System.Linq; using System.Threading; using System.Threading.Tasks; -using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.CodeFixes; using Microsoft.CodeAnalysis.CSharp.Extensions; using Microsoft.CodeAnalysis.CSharp.Syntax; @@ -18,7 +17,6 @@ using Microsoft.CodeAnalysis.PooledObjects; using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.UseCollectionExpression; -using Microsoft.CodeAnalysis.UseCollectionInitializer; namespace Microsoft.CodeAnalysis.CSharp.UseCollectionExpression; @@ -67,7 +65,8 @@ protected override async Task FixAsync( var collectionExpression = await CreateCollectionExpressionAsync( newDocument, dummyObjectCreation, - analysisResult.Matches.SelectAsArray(m => new CollectionExpressionMatch(m.Statement, m.UseSpread)), + preMatches: [], + analysisResult.Matches, static o => o.Initializer, static (o, i) => o.WithInitializer(i), cancellationToken).ConfigureAwait(false); @@ -98,7 +97,7 @@ static AnalysisResult TrackAnalysisResult(SyntaxNode root, AnalysisResult analys => new(analysisResult.DiagnosticLocation, root.GetCurrentNode(analysisResult.LocalDeclarationStatement)!, root.GetCurrentNode(analysisResult.CreationExpression)!, - analysisResult.Matches.SelectAsArray(m => new Match(root.GetCurrentNode(m.Statement)!, m.UseSpread)), + analysisResult.Matches.SelectAsArray(m => new CollectionMatch(root.GetCurrentNode(m.Node)!, m.UseSpread)), analysisResult.ChangesSemantics); // Creates a new document with all of the relevant nodes in analysisResult tracked so that we can find them diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForCreateCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForCreateCodeFixProvider.cs index 0466efe136cb1..ff99cda43402b 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForCreateCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForCreateCodeFixProvider.cs @@ -62,11 +62,12 @@ protected override async Task FixAsync( semanticDocument.Root.ReplaceNode(invocationExpression, dummyObjectCreation), cancellationToken).ConfigureAwait(false); dummyObjectCreation = (ImplicitObjectCreationExpressionSyntax)newSemanticDocument.Root.GetAnnotatedNodes(dummyObjectAnnotation).Single(); var expressions = dummyObjectCreation.ArgumentList.Arguments.Select(a => a.Expression); - var matches = expressions.SelectAsArray(static e => new CollectionExpressionMatch(e, UseSpread: false)); + var matches = expressions.SelectAsArray(static e => new CollectionMatch(e, UseSpread: false)); var collectionExpression = await CreateCollectionExpressionAsync( newSemanticDocument.Document, dummyObjectCreation, + preMatches: [], matches, static o => o.Initializer, static (o, i) => o.WithInitializer(i), diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForFluentCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForFluentCodeFixProvider.cs index 09314809aa588..18e06fdfdafbb 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForFluentCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForFluentCodeFixProvider.cs @@ -69,7 +69,8 @@ protected override async Task FixAsync( var semanticDocument = await SemanticDocument.CreateAsync(document, cancellationToken).ConfigureAwait(false); // Get the expressions that we're going to fill the new collection expression with. - var arguments = await GetArgumentsAsync(document, analysisResult.Matches, cancellationToken).ConfigureAwait(false); + var allMatches = analysisResult.PreMatches.Concat(analysisResult.PostMatches); + var arguments = await GetArgumentsAsync(document, allMatches, cancellationToken).ConfigureAwait(false); var argumentListTrailingTrivia = analysisResult.ExistingInitializer is null ? default @@ -86,30 +87,31 @@ protected override async Task FixAsync( semanticDocument.Root.ReplaceNode(invocationExpression, dummyObjectCreation), cancellationToken).ConfigureAwait(false); dummyObjectCreation = (ImplicitObjectCreationExpressionSyntax)newSemanticDocument.Root.GetAnnotatedNodes(dummyObjectAnnotation).Single(); - var matches = CreateMatches(dummyObjectCreation.ArgumentList.Arguments, analysisResult.Matches); + var preMatches = CreateMatches(dummyObjectCreation.ArgumentList.Arguments, analysisResult.PreMatches, index: 0); + var postMatches = CreateMatches(dummyObjectCreation.ArgumentList.Arguments, analysisResult.PostMatches, index: preMatches.Length); var collectionExpression = await CreateCollectionExpressionAsync( newSemanticDocument.Document, dummyObjectCreation, - matches, + preMatches, + postMatches, static o => o.Initializer, static (o, i) => o.WithInitializer(i), cancellationToken).ConfigureAwait(false); editor.ReplaceNode(invocationExpression, collectionExpression); - static ImmutableArray> CreateMatches( + static ImmutableArray> CreateMatches( SeparatedSyntaxList arguments, - ImmutableArray> matches) + ImmutableArray> matches, + int index) { - Contract.ThrowIfTrue(arguments.Count != matches.Length); + using var result = TemporaryArray>.Empty; - using var result = TemporaryArray>.Empty; - - for (int i = 0, n = arguments.Count; i < n; i++) + for (int i = 0, n = matches.Length; i < n; i++) { - var argument = arguments[i]; var match = matches[i]; + var argument = arguments[i + index]; // If we're going to spread a collection expression, just take the values *within* that collection expression // and make them arguments to the collection expression we're creating. @@ -138,7 +140,7 @@ static ImmutableArray> CreateMatches static async Task> GetArgumentsAsync( Document document, - ImmutableArray> matches, + ImmutableArray> matches, CancellationToken cancellationToken) { if (matches.IsEmpty) diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocCodeFixProvider.cs index da987b48b87d4..38db892da4846 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocCodeFixProvider.cs @@ -47,6 +47,7 @@ protected sealed override async Task FixAsync( var collectionExpression = await CSharpCollectionExpressionRewriter.CreateCollectionExpressionAsync( document, stackAllocExpression, + preMatches: [], matches, static e => e switch { @@ -69,7 +70,7 @@ protected sealed override async Task FixAsync( return; - ImmutableArray> GetMatches() + ImmutableArray> GetMatches() => stackAllocExpression switch { // if we have `stackalloc[] { ... }` we have no subsequent matches to add to the collection. All values come diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider.cs index 3203bff642c22..e27000bf62d1d 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider.cs @@ -7,9 +7,9 @@ using System.Diagnostics.CodeAnalysis; using System.Threading; using System.Threading.Tasks; -using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.CodeFixes; using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.UseCollectionExpression; using Microsoft.CodeAnalysis.UseCollectionInitializer; namespace Microsoft.CodeAnalysis.CSharp.UseCollectionInitializer; @@ -37,23 +37,14 @@ protected override CSharpUseCollectionInitializerAnalyzer GetAnalyzer() Document document, BaseObjectCreationExpressionSyntax objectCreation, bool useCollectionExpression, - ImmutableArray> matches, + ImmutableArray> preMatches, + ImmutableArray> postMatches, CancellationToken cancellationToken) { - var newObjectCreation = await GetNewObjectCreationAsync( - document, objectCreation, useCollectionExpression, matches, cancellationToken).ConfigureAwait(false); - return (objectCreation, newObjectCreation); - } + ExpressionSyntax newObjectCreation = useCollectionExpression + ? await CreateCollectionExpressionAsync(document, objectCreation, preMatches, postMatches, cancellationToken).ConfigureAwait(false) + : CreateObjectInitializerExpression(objectCreation, postMatches); - private static async Task GetNewObjectCreationAsync( - Document document, - BaseObjectCreationExpressionSyntax objectCreation, - bool useCollectionExpression, - ImmutableArray> matches, - CancellationToken cancellationToken) - { - return useCollectionExpression - ? await CreateCollectionExpressionAsync(document, objectCreation, matches, cancellationToken).ConfigureAwait(false) - : CreateObjectInitializerExpression(objectCreation, matches); + return (objectCreation, newObjectCreation); } } diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionExpression.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionExpression.cs index f84f195875f0a..765047327066e 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionExpression.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionExpression.cs @@ -5,9 +5,9 @@ using System.Collections.Immutable; using System.Threading; using System.Threading.Tasks; -using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.CSharp.UseCollectionExpression; +using Microsoft.CodeAnalysis.UseCollectionExpression; using Microsoft.CodeAnalysis.UseCollectionInitializer; namespace Microsoft.CodeAnalysis.CSharp.UseCollectionInitializer; @@ -21,13 +21,15 @@ internal partial class CSharpUseCollectionInitializerCodeFixProvider private static Task CreateCollectionExpressionAsync( Document document, BaseObjectCreationExpressionSyntax objectCreation, - ImmutableArray> matches, + ImmutableArray> preMatches, + ImmutableArray> postMatches, CancellationToken cancellationToken) { return CSharpCollectionExpressionRewriter.CreateCollectionExpressionAsync( document, objectCreation, - matches.SelectAsArray(m => new CollectionExpressionMatch(m.Statement, m.UseSpread)), + preMatches, + postMatches, static objectCreation => objectCreation.Initializer, static (objectCreation, initializer) => objectCreation.WithInitializer(initializer), cancellationToken); diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionInitializer.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionInitializer.cs index f1d0323603ec6..c8c3ad9b1b932 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionInitializer.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionInitializer.cs @@ -11,6 +11,7 @@ using Microsoft.CodeAnalysis.Formatting; using Microsoft.CodeAnalysis.PooledObjects; using Microsoft.CodeAnalysis.Shared.Extensions; +using Microsoft.CodeAnalysis.UseCollectionExpression; using Microsoft.CodeAnalysis.UseCollectionInitializer; using Roslyn.Utilities; @@ -23,7 +24,7 @@ internal partial class CSharpUseCollectionInitializerCodeFixProvider { private static BaseObjectCreationExpressionSyntax CreateObjectInitializerExpression( BaseObjectCreationExpressionSyntax objectCreation, - ImmutableArray> matches) + ImmutableArray> matches) { var expressions = CreateCollectionInitializerExpressions(); var withLineBreaks = AddLineBreaks(expressions); @@ -35,13 +36,13 @@ SeparatedSyntaxList CreateCollectionInitializerExpressions() { using var _ = ArrayBuilder.GetInstance(out var nodesAndTokens); - UseInitializerHelpers.AddExistingItems, ExpressionSyntax>( + UseInitializerHelpers.AddExistingItems, ExpressionSyntax>( objectCreation, nodesAndTokens, addTrailingComma: matches.Length > 0, static (_, expression) => expression); for (var i = 0; i < matches.Length; i++) { var match = matches[i]; - var statement = (ExpressionStatementSyntax)match.Statement; + var statement = (ExpressionStatementSyntax)match.Node; var trivia = statement.GetLeadingTrivia(); var leadingTrivia = i == 0 ? trivia.WithoutLeadingBlankLines() : trivia; diff --git a/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForFluentTests.cs b/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForFluentTests.cs index 406b13eaa34f1..b3413c13b4dba 100644 --- a/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForFluentTests.cs +++ b/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForFluentTests.cs @@ -2930,4 +2930,140 @@ void M(int[] x) ReferenceAssemblies = ReferenceAssemblies.Net.Net80, }.RunAsync(); } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/72699")] + public async Task TestObjectCreationArgument1() + { + await new VerifyCS.Test + { + TestCode = """ + using System.Linq; + using System.Collections.Generic; + + class C + { + void M(int[] values) + { + List list = new List(values).[|ToList|](); + } + } + """, + FixedCode = """ + using System.Linq; + using System.Collections.Generic; + + class C + { + void M(int[] values) + { + List list = [.. values]; + } + } + """, + LanguageVersion = LanguageVersion.CSharp12, + ReferenceAssemblies = ReferenceAssemblies.Net.Net80, + }.RunAsync(); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/72699")] + public async Task TestObjectCreationArgument2() + { + await new VerifyCS.Test + { + TestCode = """ + using System.Linq; + using System.Collections.Generic; + + class C + { + void M(int[] values) + { + List list = new List(values) { 1, 2, 3 }.[|ToList|](); + } + } + """, + FixedCode = """ + using System.Linq; + using System.Collections.Generic; + + class C + { + void M(int[] values) + { + List list = [.. values, 1, 2, 3]; + } + } + """, + LanguageVersion = LanguageVersion.CSharp12, + ReferenceAssemblies = ReferenceAssemblies.Net.Net80, + }.RunAsync(); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/72699")] + public async Task TestObjectCreationArgument3() + { + await new VerifyCS.Test + { + TestCode = """ + using System.Linq; + using System.Collections.Generic; + + class C + { + void M(int[] values, int[] x) + { + List list = new List(values).Concat(x).[|ToList|](); + } + } + """, + FixedCode = """ + using System.Linq; + using System.Collections.Generic; + + class C + { + void M(int[] values, int[] x) + { + List list = [.. values, .. x]; + } + } + """, + LanguageVersion = LanguageVersion.CSharp12, + ReferenceAssemblies = ReferenceAssemblies.Net.Net80, + }.RunAsync(); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/72699")] + public async Task TestObjectCreationArgument4() + { + await new VerifyCS.Test + { + TestCode = """ + using System.Linq; + using System.Collections.Generic; + + class C + { + void M(int[] values, int[] x) + { + List list = new List(values) { 1, 2, 3 }.Concat(x).[|ToList|](); + } + } + """, + FixedCode = """ + using System.Linq; + using System.Collections.Generic; + + class C + { + void M(int[] values, int[] x) + { + List list = [.. values, 1, 2, 3, .. x]; + } + } + """, + LanguageVersion = LanguageVersion.CSharp12, + ReferenceAssemblies = ReferenceAssemblies.Net.Net80, + }.RunAsync(); + } } diff --git a/src/Analyzers/CSharp/Tests/UseCollectionInitializer/UseCollectionInitializerTests_CollectionExpression.cs b/src/Analyzers/CSharp/Tests/UseCollectionInitializer/UseCollectionInitializerTests_CollectionExpression.cs index 9941e97311d4c..7a19fa362e961 100644 --- a/src/Analyzers/CSharp/Tests/UseCollectionInitializer/UseCollectionInitializerTests_CollectionExpression.cs +++ b/src/Analyzers/CSharp/Tests/UseCollectionInitializer/UseCollectionInitializerTests_CollectionExpression.cs @@ -2,9 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -using System; using System.Diagnostics.CodeAnalysis; -using System.Linq.Expressions; using System.Threading.Tasks; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.UseCollectionInitializer; @@ -198,13 +196,19 @@ class C [Fact] public async Task TestInField4() { - await TestMissingInRegularAndScriptAsync( - """ + await TestInRegularAndScriptAsync(""" + using System.Collections.Generic; + + class C + { + List c = [|new|] List(new[] { 1, 2, 3 }); + } + """, """ using System.Collections.Generic; class C { - List c = new List(new[] { 1, 2, 3 }); + List c = [.. new[] { 1, 2, 3 }]; } """); } @@ -2361,8 +2365,7 @@ static void M() [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/17823")] public async Task TestWhenReferencedInInitializer_LocalVar2() { - await TestMissingInRegularAndScriptAsync( - """ + await TestInRegularAndScriptAsync(""" using System.Collections.Generic; using System.Linq; @@ -2370,7 +2373,19 @@ class C { void M() { - List t = new List(new int[] { 1, 2, 3 }); + List t = [|new|] List(new int[] { 1, 2, 3 }); + t.Add(t.Min() - 1); + } + } + """, """ + using System.Collections.Generic; + using System.Linq; + + class C + { + void M() + { + List t = [.. new int[] { 1, 2, 3 }]; t.Add(t.Min() - 1); } } @@ -2416,7 +2431,7 @@ static void M() [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/18260")] public async Task TestWhenReferencedInInitializer_Assignment2() { - await TestMissingInRegularAndScriptAsync( + await TestInRegularAndScriptAsync( """ using System.Collections.Generic; using System.Linq; @@ -2426,7 +2441,21 @@ class C void M() { List t = null; - t = new List(new int[] { 1, 2, 3 }); + t = [|new|] List(new int[] { 1, 2, 3 }); + t.Add(t.Min() - 1); + } + } + """, + """ + using System.Collections.Generic; + using System.Linq; + + class C + { + void M() + { + List t = null; + t = [.. new int[] { 1, 2, 3 }]; t.Add(t.Min() - 1); } } @@ -4936,7 +4965,7 @@ void M(int[] x, int[] y) { List c = [|new|] List(x); [|c.Add(|]0); - c.AddRange(y); + [|c.AddRange(|]y); } } """, @@ -4947,11 +4976,7 @@ class C { void M(int[] x, int[] y) { - List c = new List(x) - { - 0 - }; - c.AddRange(y); + List c = [.. x, 0, .. y]; } } """); @@ -5720,6 +5745,74 @@ void M() }.RunAsync(); } + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/72699")] + public async Task TestObjectCreationArgument1() + { + await new VerifyCS.Test + { + TestCode = """ + using System.Linq; + using System.Collections.Generic; + + class C + { + void M(int[] values) + { + List list = [|new|] List(values); + } + } + """, + FixedCode = """ + using System.Linq; + using System.Collections.Generic; + + class C + { + void M(int[] values) + { + List list = [.. values]; + } + } + """, + LanguageVersion = LanguageVersion.CSharp12, + ReferenceAssemblies = ReferenceAssemblies.Net.Net80, + }.RunAsync(); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/72699")] + public async Task TestObjectCreationArgument2() + { + await new VerifyCS.Test + { + TestCode = """ + using System.Linq; + using System.Collections.Generic; + + class C + { + void M(int[] values) + { + List list = [|new|] List(values) { 1, 2, 3 }; + } + } + """, + FixedCode = """ + using System.Linq; + using System.Collections.Generic; + + class C + { + void M(int[] values) + { + List list = [.. values, 1, 2, 3]; + } + } + """, + LanguageVersion = LanguageVersion.CSharp12, + ReferenceAssemblies = ReferenceAssemblies.Net.Net80, + }.RunAsync(); + } + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/73362")] public async Task TestWithOverloadResolution1() { diff --git a/src/Analyzers/Core/Analyzers/Analyzers.projitems b/src/Analyzers/Core/Analyzers/Analyzers.projitems index cd595743cd676..ea81bd2ba5653 100644 --- a/src/Analyzers/Core/Analyzers/Analyzers.projitems +++ b/src/Analyzers/Core/Analyzers/Analyzers.projitems @@ -81,6 +81,7 @@ + diff --git a/src/Analyzers/Core/Analyzers/UseCollectionExpression/CollectionExpressionMatch.cs b/src/Analyzers/Core/Analyzers/UseCollectionExpression/CollectionExpressionMatch.cs new file mode 100644 index 0000000000000..db6671311ed8f --- /dev/null +++ b/src/Analyzers/Core/Analyzers/UseCollectionExpression/CollectionExpressionMatch.cs @@ -0,0 +1,19 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +namespace Microsoft.CodeAnalysis.UseCollectionExpression; + +/// +/// Represents statements following an initializer that should be converted into collection-initializer/expression +/// elements. +/// +/// The statement that follows that contains the values to add to the new collection-initializer or +/// collection-expression. Or the expression directly to add. +/// Whether or not a spread (.. x) element should be created for this statement. This +/// is needed as the statement could be cases like expr.Add(x) vs. expr.AddRange(x). This property +/// indicates that the latter should become a spread, without the consumer having to reexamine the statement to see +/// what form it is. +internal readonly record struct CollectionMatch( + TMatchNode Node, + bool UseSpread) where TMatchNode : SyntaxNode; diff --git a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractObjectCreationExpressionAnalyzer.cs b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractObjectCreationExpressionAnalyzer.cs index e1cb4dc271488..33542348cc649 100644 --- a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractObjectCreationExpressionAnalyzer.cs +++ b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractObjectCreationExpressionAnalyzer.cs @@ -43,7 +43,7 @@ internal abstract class AbstractObjectCreationExpressionAnalyzer< protected SemanticModel SemanticModel => this.State.SemanticModel; protected abstract bool ShouldAnalyze(CancellationToken cancellationToken); - protected abstract bool TryAddMatches(ArrayBuilder matches, CancellationToken cancellationToken); + protected abstract bool TryAddMatches(ArrayBuilder preMatches, ArrayBuilder postMatches, CancellationToken cancellationToken); protected abstract bool IsInitializerOfLocalDeclarationStatement( TLocalDeclarationStatementSyntax localDeclarationStatement, TObjectCreationExpressionSyntax rootExpression, [NotNullWhen(true)] out TVariableDeclaratorSyntax? variableDeclarator); @@ -75,16 +75,17 @@ protected void Clear() _analyzeForCollectionExpression = false; } - protected ImmutableArray AnalyzeWorker(CancellationToken cancellationToken) + protected (ImmutableArray preMatches, ImmutableArray postMatches) AnalyzeWorker(CancellationToken cancellationToken) { if (!ShouldAnalyze(cancellationToken)) return default; - using var _ = ArrayBuilder.GetInstance(out var matches); - if (!TryAddMatches(matches, cancellationToken)) + using var _1 = ArrayBuilder.GetInstance(out var preMatches); + using var _2 = ArrayBuilder.GetInstance(out var postMatches); + if (!TryAddMatches(preMatches, postMatches, cancellationToken)) return default; - return matches.ToImmutableAndClear(); + return (preMatches.ToImmutableAndClear(), postMatches.ToImmutableAndClear()); } protected UpdateExpressionState? TryInitializeState( diff --git a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerAnalyzer.cs b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerAnalyzer.cs index 207b25ed408eb..9eafc5d7ab056 100644 --- a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerAnalyzer.cs +++ b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerAnalyzer.cs @@ -8,6 +8,7 @@ using System.Threading; using Microsoft.CodeAnalysis.LanguageService; using Microsoft.CodeAnalysis.PooledObjects; +using Microsoft.CodeAnalysis.UseCollectionExpression; using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.UseCollectionInitializer; @@ -27,7 +28,7 @@ internal abstract class AbstractUseCollectionInitializerAnalyzer< TObjectCreationExpressionSyntax, TLocalDeclarationStatementSyntax, TVariableDeclaratorSyntax, - Match, TAnalyzer> + CollectionMatch, TAnalyzer> where TExpressionSyntax : SyntaxNode where TStatementSyntax : SyntaxNode where TObjectCreationExpressionSyntax : TExpressionSyntax @@ -47,13 +48,18 @@ internal abstract class AbstractUseCollectionInitializerAnalyzer< TVariableDeclaratorSyntax, TAnalyzer>, new() { + public readonly record struct AnalysisResult( + ImmutableArray> PreMatches, + ImmutableArray> PostMatches); + protected abstract bool IsComplexElementInitializer(SyntaxNode expression); protected abstract bool HasExistingInvalidInitializerForCollection(); - protected abstract bool ValidateMatchesForCollectionExpression(ArrayBuilder> matches, CancellationToken cancellationToken); + protected abstract bool AnalyzeMatchesAndCollectionConstructorForCollectionExpression( + ArrayBuilder> preMatches, ArrayBuilder> postMatches, CancellationToken cancellationToken); protected abstract IUpdateExpressionSyntaxHelper SyntaxHelper { get; } - public ImmutableArray> Analyze( + public AnalysisResult Analyze( SemanticModel semanticModel, ISyntaxFacts syntaxFacts, TObjectCreationExpressionSyntax objectCreationExpression, @@ -65,10 +71,10 @@ public ImmutableArray> Analyze( return default; this.Initialize(state.Value, objectCreationExpression, analyzeForCollectionExpression); - var result = this.AnalyzeWorker(cancellationToken); + var (preMatches, postMatches) = this.AnalyzeWorker(cancellationToken); // If analysis failed entirely, immediately bail out. - if (result.IsDefault) + if (preMatches.IsDefault || postMatches.IsDefault) return default; // Analysis succeeded, but the result may be empty or non empty. @@ -80,14 +86,14 @@ public ImmutableArray> Analyze( // other words, we don't want to suggest changing `new List()` to `new List() { }` as that's just // noise. So convert empty results to an invalid result here. if (analyzeForCollectionExpression) - return result; + return new(preMatches, postMatches); // Downgrade an empty result to a failure for the normal collection-initializer case. - return result.IsEmpty ? default : result; + return postMatches.IsEmpty ? default : new(preMatches, postMatches); } protected sealed override bool TryAddMatches( - ArrayBuilder> matches, CancellationToken cancellationToken) + ArrayBuilder> preMatches, ArrayBuilder> postMatches, CancellationToken cancellationToken) { var seenInvocation = false; var seenIndexAssignment = false; @@ -121,17 +127,17 @@ protected sealed override bool TryAddMatches( if (match is null) break; - matches.Add(match.Value); + postMatches.Add(match.Value); } } if (_analyzeForCollectionExpression) - return ValidateMatchesForCollectionExpression(matches, cancellationToken); + return AnalyzeMatchesAndCollectionConstructorForCollectionExpression(preMatches, postMatches, cancellationToken); return true; } - private Match? TryAnalyzeStatement( + private CollectionMatch? TryAnalyzeStatement( TStatementSyntax statement, ref bool seenInvocation, ref bool seenIndexAssignment, CancellationToken cancellationToken) { return _analyzeForCollectionExpression @@ -139,7 +145,7 @@ protected sealed override bool TryAddMatches( : TryAnalyzeStatementForCollectionInitializer(statement, ref seenInvocation, ref seenIndexAssignment, cancellationToken); } - private Match? TryAnalyzeStatementForCollectionInitializer( + private CollectionMatch? TryAnalyzeStatementForCollectionInitializer( TStatementSyntax statement, ref bool seenInvocation, ref bool seenIndexAssignment, CancellationToken cancellationToken) { // At least one of these has to be false. @@ -161,7 +167,7 @@ protected sealed override bool TryAddMatches( this.State.ValuePatternMatches(instance)) { seenInvocation = true; - return new Match(expressionStatement, UseSpread: false); + return new(expressionStatement, UseSpread: false); } } @@ -171,7 +177,7 @@ protected sealed override bool TryAddMatches( this.State.ValuePatternMatches(instance)) { seenIndexAssignment = true; - return new Match(expressionStatement, UseSpread: false); + return new(expressionStatement, UseSpread: false); } } diff --git a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerDiagnosticAnalyzer.cs b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerDiagnosticAnalyzer.cs index a663612d196d2..6b84494d3acfc 100644 --- a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerDiagnosticAnalyzer.cs +++ b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerDiagnosticAnalyzer.cs @@ -11,23 +11,10 @@ using Microsoft.CodeAnalysis.Shared.CodeStyle; using Microsoft.CodeAnalysis.Shared.Collections; using Microsoft.CodeAnalysis.Shared.Extensions; +using Microsoft.CodeAnalysis.UseCollectionExpression; namespace Microsoft.CodeAnalysis.UseCollectionInitializer; -/// -/// Represents statements following an object initializer that should be converted into -/// collection-initializer/expression elements. -/// -/// The statement that follows that contains the values to add to the new -/// collection-initializer or collection-expression -/// Whether or not a spread (.. x) element should be created for this statement. This -/// is needed as the statement could be cases like expr.Add(x) vs. expr.AddRange(x). This property -/// indicates that the latter should become a spread, without the consumer having to reexamine the statement to see -/// what form it is. -internal readonly record struct Match( - TStatementSyntax Statement, - bool UseSpread) where TStatementSyntax : SyntaxNode; - internal abstract partial class AbstractUseCollectionInitializerDiagnosticAnalyzer< TSyntaxKind, TExpressionSyntax, @@ -182,7 +169,7 @@ private void AnalyzeNode( var nodes = containingStatement is null ? ImmutableArray.Empty : [containingStatement]; - nodes = nodes.AddRange(matches.Select(static m => m.Statement)); + nodes = nodes.AddRange(matches.Select(static m => m.Node)); if (syntaxFacts.ContainsInterleavedDirective(nodes, cancellationToken)) return; @@ -205,7 +192,7 @@ private void AnalyzeNode( return; - (ImmutableArray> matches, bool shouldUseCollectionExpression, bool changesSemantics)? GetCollectionInitializerMatches() + (ImmutableArray> matches, bool shouldUseCollectionExpression, bool changesSemantics)? GetCollectionInitializerMatches() { if (containingStatement is null) return null; @@ -213,7 +200,7 @@ private void AnalyzeNode( if (!preferInitializerOption.Value) return null; - var matches = analyzer.Analyze(semanticModel, syntaxFacts, objectCreationExpression, analyzeForCollectionExpression: false, cancellationToken); + var (_, matches) = analyzer.Analyze(semanticModel, syntaxFacts, objectCreationExpression, analyzeForCollectionExpression: false, cancellationToken); // If analysis failed, we can't change this, no matter what. if (matches.IsDefault) @@ -222,7 +209,7 @@ private void AnalyzeNode( return (matches, shouldUseCollectionExpression: false, changesSemantics: false); } - (ImmutableArray> matches, bool shouldUseCollectionExpression, bool changesSemantics)? GetCollectionExpressionMatches() + (ImmutableArray> matches, bool shouldUseCollectionExpression, bool changesSemantics)? GetCollectionExpressionMatches() { if (preferExpressionOption.Value == CollectionExpressionPreference.Never) return null; @@ -231,7 +218,7 @@ private void AnalyzeNode( if (!this.AreCollectionExpressionsSupported(context.Compilation)) return null; - var matches = analyzer.Analyze(semanticModel, syntaxFacts, objectCreationExpression, analyzeForCollectionExpression: true, cancellationToken); + var (_, matches) = analyzer.Analyze(semanticModel, syntaxFacts, objectCreationExpression, analyzeForCollectionExpression: true, cancellationToken); // If analysis failed, we can't change this, no matter what. if (matches.IsDefault) @@ -248,7 +235,7 @@ private void AnalyzeNode( private void FadeOutCode( SyntaxNodeAnalysisContext context, - ImmutableArray> matches, + ImmutableArray> matches, ImmutableArray locations, ImmutableDictionary? properties) { diff --git a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/UpdateExpressionState.cs b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/UpdateExpressionState.cs index d93969a9f41db..48a993be1f27a 100644 --- a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/UpdateExpressionState.cs +++ b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/UpdateExpressionState.cs @@ -9,6 +9,7 @@ using System.Threading; using Microsoft.CodeAnalysis.LanguageService; using Microsoft.CodeAnalysis.Shared.Extensions; +using Microsoft.CodeAnalysis.UseCollectionExpression; using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.UseCollectionInitializer; @@ -337,7 +338,7 @@ private bool TryAnalyzeInvocation( /// includes calls to .Add and .AddRange, as well as foreach statements that update the /// collection, and if statements that conditionally add items to the collection-expression. /// - public Match? TryAnalyzeStatementForCollectionExpression( + public CollectionMatch? TryAnalyzeStatementForCollectionExpression( IUpdateExpressionSyntaxHelper syntaxHelper, TStatementSyntax statement, CancellationToken cancellationToken) @@ -355,7 +356,7 @@ private bool TryAnalyzeInvocation( return null; - Match? TryAnalyzeExpressionStatement(TStatementSyntax expressionStatement) + CollectionMatch? TryAnalyzeExpressionStatement(TStatementSyntax expressionStatement) { var expression = (TExpressionSyntax)@this.SyntaxFacts.GetExpressionOfExpressionStatement(expressionStatement); @@ -363,13 +364,13 @@ private bool TryAnalyzeInvocation( if (@this.TryAnalyzeInvocationForCollectionExpression(expression, allowLinq: false, cancellationToken, out var instance, out var useSpread) && @this.ValuePatternMatches(instance)) { - return new Match(expressionStatement, useSpread); + return new(expressionStatement, useSpread); } return null; } - Match? TryAnalyzeForeachStatement(TStatementSyntax foreachStatement) + CollectionMatch? TryAnalyzeForeachStatement(TStatementSyntax foreachStatement) { syntaxHelper.GetPartsOfForeachStatement(foreachStatement, out var awaitKeyword, out var identifier, out _, out var foreachStatements); if (awaitKeyword != default) @@ -393,13 +394,13 @@ private bool TryAnalyzeInvocation( @this.ValuePatternMatches(instance)) { // `foreach` will become `..expr` when we make it into a collection expression. - return new Match(foreachStatement, UseSpread: true); + return new(foreachStatement, UseSpread: true); } return null; } - Match? TryAnalyzeIfStatement(TStatementSyntax ifStatement) + CollectionMatch? TryAnalyzeIfStatement(TStatementSyntax ifStatement) { // look for the form: // @@ -430,7 +431,7 @@ private bool TryAnalyzeInvocation( { // add the form `.. x ? [y] : []` to the result return @this.SyntaxFacts.SupportsCollectionExpressionNaturalType(ifStatement.SyntaxTree.Options) - ? new Match(ifStatement, UseSpread: true) + ? new(ifStatement, UseSpread: true) : null; } @@ -446,7 +447,7 @@ private bool TryAnalyzeInvocation( @this.ValuePatternMatches(instance)) { // add the form `x ? y : z` to the result - return new Match(ifStatement, UseSpread: false); + return new(ifStatement, UseSpread: false); } } diff --git a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/UseCollectionInitializerHelpers.cs b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/UseCollectionInitializerHelpers.cs index 0860a10ea6eed..48c478b9b0ff1 100644 --- a/src/Analyzers/Core/Analyzers/UseCollectionInitializer/UseCollectionInitializerHelpers.cs +++ b/src/Analyzers/Core/Analyzers/UseCollectionInitializer/UseCollectionInitializerHelpers.cs @@ -7,6 +7,7 @@ using Microsoft.CodeAnalysis.LanguageService; using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.Text; +using Microsoft.CodeAnalysis.UseCollectionExpression; namespace Microsoft.CodeAnalysis.UseCollectionInitializer; @@ -18,12 +19,11 @@ internal static class UseCollectionInitializerHelpers public static readonly ImmutableDictionary UseCollectionExpressionProperties = ImmutableDictionary.Empty.Add(UseCollectionExpressionName, UseCollectionExpressionName); - public static ImmutableArray GetLocationsToFade( + public static ImmutableArray GetLocationsToFade( ISyntaxFacts syntaxFacts, - Match matchInfo) - where TStatementSyntax : SyntaxNode + CollectionMatch matchInfo) { - var match = matchInfo.Statement; + var match = matchInfo.Node; var syntaxTree = match.SyntaxTree; if (syntaxFacts.IsExpressionStatement(match)) { diff --git a/src/Analyzers/Core/Analyzers/UseObjectInitializer/UseNamedMemberInitializerAnalyzer.cs b/src/Analyzers/Core/Analyzers/UseObjectInitializer/UseNamedMemberInitializerAnalyzer.cs index b9ad90458a74b..4550c024a4444 100644 --- a/src/Analyzers/Core/Analyzers/UseObjectInitializer/UseNamedMemberInitializerAnalyzer.cs +++ b/src/Analyzers/Core/Analyzers/UseObjectInitializer/UseNamedMemberInitializerAnalyzer.cs @@ -57,7 +57,7 @@ public ImmutableArray> matches, + ArrayBuilder> preMatches, + ArrayBuilder> postMatches, CancellationToken cancellationToken) { using var _1 = PooledHashSet.GetInstance(out var seenNames); @@ -163,7 +164,7 @@ protected sealed override bool TryAddMatches( if (!seenNames.Add(identifier.ValueText)) break; - matches.Add(new Match( + postMatches.Add(new Match( statement, leftMemberAccess, rightExpression, typeMember?.Name ?? identifier.ValueText)); } diff --git a/src/Analyzers/Core/CodeFixes/UseCollectionInitializer/AbstractUseCollectionInitializerCodeFixProvider.cs b/src/Analyzers/Core/CodeFixes/UseCollectionInitializer/AbstractUseCollectionInitializerCodeFixProvider.cs index 3bd8365d3a7b3..d1c1ed93b6077 100644 --- a/src/Analyzers/Core/CodeFixes/UseCollectionInitializer/AbstractUseCollectionInitializerCodeFixProvider.cs +++ b/src/Analyzers/Core/CodeFixes/UseCollectionInitializer/AbstractUseCollectionInitializerCodeFixProvider.cs @@ -5,7 +5,6 @@ using System.Collections.Immutable; using System.Threading; using System.Threading.Tasks; -using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Editing; using Microsoft.CodeAnalysis.LanguageService; @@ -54,7 +53,9 @@ public sealed override ImmutableArray FixableDiagnosticIds protected abstract TAnalyzer GetAnalyzer(); protected abstract Task<(SyntaxNode oldNode, SyntaxNode newNode)> GetReplacementNodesAsync( - Document document, TObjectCreationExpressionSyntax objectCreation, bool useCollectionExpression, ImmutableArray> matches, CancellationToken cancellationToken); + Document document, TObjectCreationExpressionSyntax objectCreation, bool useCollectionExpression, + ImmutableArray> preMatches, + ImmutableArray> postMatches, CancellationToken cancellationToken); protected sealed override async Task FixAsync( Document document, @@ -75,17 +76,20 @@ protected sealed override async Task FixAsync( using var analyzer = GetAnalyzer(); var useCollectionExpression = properties.ContainsKey(UseCollectionInitializerHelpers.UseCollectionExpressionName) is true; - var matches = analyzer.Analyze( + var (preMatches, postMatches) = analyzer.Analyze( semanticModel, syntaxFacts, objectCreation, useCollectionExpression, cancellationToken); - if (matches.IsDefault) + if (preMatches.IsDefault || postMatches.IsDefault) return; var (oldNode, newNode) = await GetReplacementNodesAsync( - document, objectCreation, useCollectionExpression, matches, cancellationToken).ConfigureAwait(false); + document, objectCreation, useCollectionExpression, preMatches, postMatches, cancellationToken).ConfigureAwait(false); editor.ReplaceNode(oldNode, newNode); - foreach (var match in matches) - editor.RemoveNode(match.Statement, SyntaxRemoveOptions.KeepUnbalancedDirectives); + + // We only need to remove the post-matches. The pre-matches are the arguments in teh object creation, which + // itself got replaced above. + foreach (var match in postMatches) + editor.RemoveNode(match.Node, SyntaxRemoveOptions.KeepUnbalancedDirectives); } } diff --git a/src/Analyzers/Core/CodeFixes/UseObjectInitializer/AbstractUseObjectInitializerCodeFixProvider.cs b/src/Analyzers/Core/CodeFixes/UseObjectInitializer/AbstractUseObjectInitializerCodeFixProvider.cs index e24f92920c2fb..2c81b61a74744 100644 --- a/src/Analyzers/Core/CodeFixes/UseObjectInitializer/AbstractUseObjectInitializerCodeFixProvider.cs +++ b/src/Analyzers/Core/CodeFixes/UseObjectInitializer/AbstractUseObjectInitializerCodeFixProvider.cs @@ -5,7 +5,6 @@ using System.Collections.Immutable; using System.Threading; using System.Threading.Tasks; -using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.CodeFixes; using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Editing; diff --git a/src/Analyzers/VisualBasic/Analyzers/UseCollectionInitializer/VisualBasicCollectionInitializerAnalyzer.vb b/src/Analyzers/VisualBasic/Analyzers/UseCollectionInitializer/VisualBasicCollectionInitializerAnalyzer.vb index e6667bf0464bd..f93006673927f 100644 --- a/src/Analyzers/VisualBasic/Analyzers/UseCollectionInitializer/VisualBasicCollectionInitializerAnalyzer.vb +++ b/src/Analyzers/VisualBasic/Analyzers/UseCollectionInitializer/VisualBasicCollectionInitializerAnalyzer.vb @@ -4,6 +4,7 @@ Imports System.Threading Imports Microsoft.CodeAnalysis.PooledObjects +Imports Microsoft.CodeAnalysis.UseCollectionExpression Imports Microsoft.CodeAnalysis.UseCollectionInitializer Imports Microsoft.CodeAnalysis.VisualBasic.Syntax @@ -37,7 +38,10 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.UseCollectionInitializer Return TypeOf _objectCreationExpression.Initializer Is ObjectMemberInitializerSyntax End Function - Protected Overrides Function ValidateMatchesForCollectionExpression(matches As ArrayBuilder(Of Match(Of StatementSyntax)), cancellationToken As CancellationToken) As Boolean + Protected Overrides Function AnalyzeMatchesAndCollectionConstructorForCollectionExpression( + preMatches As ArrayBuilder(Of CollectionMatch(Of SyntaxNode)), + postMatches As ArrayBuilder(Of CollectionMatch(Of SyntaxNode)), + cancellationToken As CancellationToken) As Boolean ' Only called for collection expressions, which VB does not support Throw ExceptionUtilities.Unreachable() End Function diff --git a/src/Analyzers/VisualBasic/CodeFixes/UseCollectionInitializer/VisualBasicUseCollectionInitializerCodeFixProvider.vb b/src/Analyzers/VisualBasic/CodeFixes/UseCollectionInitializer/VisualBasicUseCollectionInitializerCodeFixProvider.vb index 4d0b85942cbfe..c646b08b34da3 100644 --- a/src/Analyzers/VisualBasic/CodeFixes/UseCollectionInitializer/VisualBasicUseCollectionInitializerCodeFixProvider.vb +++ b/src/Analyzers/VisualBasic/CodeFixes/UseCollectionInitializer/VisualBasicUseCollectionInitializerCodeFixProvider.vb @@ -6,10 +6,10 @@ Imports System.Collections.Immutable Imports System.Composition Imports System.Diagnostics.CodeAnalysis Imports System.Threading -Imports Microsoft.CodeAnalysis.CodeActions Imports Microsoft.CodeAnalysis.CodeFixes Imports Microsoft.CodeAnalysis.Formatting Imports Microsoft.CodeAnalysis.PooledObjects +Imports Microsoft.CodeAnalysis.UseCollectionExpression Imports Microsoft.CodeAnalysis.UseCollectionInitializer Imports Microsoft.CodeAnalysis.VisualBasic.Syntax Imports Microsoft.CodeAnalysis.VisualBasic.UseObjectInitializer @@ -42,21 +42,23 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.UseCollectionInitializer document As Document, objectCreation As ObjectCreationExpressionSyntax, useCollectionExpression As Boolean, - matches As ImmutableArray(Of Match(Of StatementSyntax)), + preMatches As ImmutableArray(Of CollectionMatch(Of SyntaxNode)), + postMatches As ImmutableArray(Of CollectionMatch(Of SyntaxNode)), cancellationToken As CancellationToken) As Task(Of (SyntaxNode, SyntaxNode)) + Contract.ThrowIfFalse(preMatches.IsEmpty) Contract.ThrowIfTrue(useCollectionExpression, "VB does not support collection expressions") Dim statement = objectCreation.FirstAncestorOrSelf(Of StatementSyntax) Dim newStatement = statement.ReplaceNode( objectCreation, - GetNewObjectCreation(objectCreation, matches)) + GetNewObjectCreation(objectCreation, postMatches)) Dim totalTrivia = ArrayBuilder(Of SyntaxTrivia).GetInstance() totalTrivia.AddRange(statement.GetLeadingTrivia()) totalTrivia.Add(SyntaxFactory.ElasticMarker) - For Each match In matches - For Each trivia In match.Statement.GetLeadingTrivia() + For Each match In postMatches + For Each trivia In match.Node.GetLeadingTrivia() If trivia.Kind = SyntaxKind.CommentTrivia Then totalTrivia.Add(trivia) totalTrivia.Add(SyntaxFactory.ElasticMarker) @@ -70,7 +72,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.UseCollectionInitializer Private Shared Function GetNewObjectCreation( objectCreation As ObjectCreationExpressionSyntax, - matches As ImmutableArray(Of Match(Of StatementSyntax))) As ObjectCreationExpressionSyntax + matches As ImmutableArray(Of CollectionMatch(Of SyntaxNode))) As ObjectCreationExpressionSyntax Return UseInitializerHelpers.GetNewObjectCreation( objectCreation, @@ -80,13 +82,13 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.UseCollectionInitializer Private Shared Function CreateCollectionInitializer( objectCreation As ObjectCreationExpressionSyntax, - matches As ImmutableArray(Of Match(Of StatementSyntax))) As CollectionInitializerSyntax + matches As ImmutableArray(Of CollectionMatch(Of SyntaxNode))) As CollectionInitializerSyntax Dim nodesAndTokens = ArrayBuilder(Of SyntaxNodeOrToken).GetInstance() AddExistingItems(objectCreation, nodesAndTokens) For i = 0 To matches.Length - 1 - Dim expressionStatement = DirectCast(matches(i).Statement, ExpressionStatementSyntax) + Dim expressionStatement = DirectCast(matches(i).Node, ExpressionStatementSyntax) Dim newExpression As ExpressionSyntax Dim invocationExpression = DirectCast(expressionStatement.Expression, InvocationExpressionSyntax)