From 1c6f65e93c400a837fbf85199faa0ab208d30d41 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sat, 5 Aug 2023 14:22:22 -0700 Subject: [PATCH 1/7] Extract helper code --- ...ionExpressionForArrayDiagnosticAnalyzer.cs | 1 - ...ionExpressionForEmptyDiagnosticAnalyzer.cs | 1 - ...pressionForStackAllocDiagnosticAnalyzer.cs | 99 +---------------- .../CollectionExpressionMatch.cs | 2 +- .../UseCollectionExpressionHelpers.cs | 104 +++++++++++++++++- ...CollectionInitializerDiagnosticAnalyzer.cs | 4 +- .../CSharpCollectionExpressionRewriter.cs | 4 - ...ectionExpressionForArrayCodeFixProvider.cs | 3 - ...nExpressionForStackAllocCodeFixProvider.cs | 10 -- ...zerCodeFixProvider_CollectionExpression.cs | 1 - 10 files changed, 111 insertions(+), 118 deletions(-) diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForArrayDiagnosticAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForArrayDiagnosticAnalyzer.cs index 03e95b9c8f28..07dcbd461fcb 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForArrayDiagnosticAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForArrayDiagnosticAnalyzer.cs @@ -5,7 +5,6 @@ using System.Collections.Immutable; using System.Diagnostics; using Microsoft.CodeAnalysis.CodeStyle; -using Microsoft.CodeAnalysis.CSharp.Analyzers.UseCollectionExpression; using Microsoft.CodeAnalysis.CSharp.Extensions; using Microsoft.CodeAnalysis.CSharp.Shared.Extensions; using Microsoft.CodeAnalysis.CSharp.Syntax; diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForEmptyDiagnosticAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForEmptyDiagnosticAnalyzer.cs index 282511ee2b81..b6861e1750a3 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForEmptyDiagnosticAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForEmptyDiagnosticAnalyzer.cs @@ -5,7 +5,6 @@ using System; using System.Collections.Immutable; using Microsoft.CodeAnalysis.CodeStyle; -using Microsoft.CodeAnalysis.CSharp.Analyzers.UseCollectionExpression; using Microsoft.CodeAnalysis.CSharp.Extensions; using Microsoft.CodeAnalysis.CSharp.Shared.Extensions; using Microsoft.CodeAnalysis.CSharp.Syntax; diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocDiagnosticAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocDiagnosticAnalyzer.cs index c7fc27144d20..5d05a4c56ae2 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocDiagnosticAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocDiagnosticAnalyzer.cs @@ -3,18 +3,14 @@ // See the LICENSE file in the project root for more information. using System.Collections.Immutable; -using System.Linq; using System.Threading; using Microsoft.CodeAnalysis.CodeStyle; -using Microsoft.CodeAnalysis.CSharp.Analyzers.UseCollectionExpression; using Microsoft.CodeAnalysis.CSharp.Extensions; using Microsoft.CodeAnalysis.CSharp.Shared.Extensions; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Options; -using Microsoft.CodeAnalysis.PooledObjects; using Microsoft.CodeAnalysis.Text; -using Microsoft.CodeAnalysis.UseCollectionExpression; namespace Microsoft.CodeAnalysis.CSharp.UseCollectionExpression; @@ -152,94 +148,11 @@ public static ImmutableArray TryGetMatches( StackAllocArrayCreationExpressionSyntax expression, CancellationToken cancellationToken) { - // has to either be `stackalloc X[]` or `stackalloc X[const]`. - if (expression.Type is not ArrayTypeSyntax { RankSpecifiers: [{ Sizes: [var size] } rankSpecifier] } arrayType) - return default; - - using var _ = ArrayBuilder.GetInstance(out var matches); - - if (size is OmittedArraySizeExpressionSyntax) - { - // `stackalloc int[]` on its own is illegal. Has to either have a size, or an initializer. - if (expression.Initializer is null) - return default; - } - else - { - // if `stackalloc X[val]`, then it `val` has to be a constant value. - if (semanticModel.GetConstantValue(size, cancellationToken).Value is not int sizeValue) - return default; - - if (expression.Initializer != null) - { - // if there is an initializer, then it has to have the right number of elements. - if (sizeValue != expression.Initializer.Expressions.Count) - return default; - } - else - { - // if there is no initializer, we have to be followed by direct statements that initialize the right - // number of elements. - - // This needs to be local variable like `ReadOnlySpan x = stackalloc ... - if (expression.WalkUpParentheses().Parent is not EqualsValueClauseSyntax - { - Parent: VariableDeclaratorSyntax - { - Identifier.ValueText: var variableName, - Parent.Parent: LocalDeclarationStatementSyntax localDeclarationStatement - }, - }) - { - return default; - } - - var currentStatement = localDeclarationStatement.GetNextStatement(); - for (var currentIndex = 0; currentIndex < sizeValue; currentIndex++) - { - // Each following statement needs to of the form: - // - // x[...] = - if (currentStatement is not ExpressionStatementSyntax - { - Expression: AssignmentExpressionSyntax - { - Left: ElementAccessExpressionSyntax - { - Expression: IdentifierNameSyntax { Identifier.ValueText: var elementName }, - ArgumentList.Arguments: [var elementArgument], - } elementAccess, - } - } expressionStatement) - { - return default; - } - - // Ensure we're indexing into the variable created. - if (variableName != elementName) - return default; - - // The indexing value has to equal the corresponding location in the result. - if (semanticModel.GetConstantValue(elementArgument.Expression, cancellationToken).Value is not int indexValue || - indexValue != currentIndex) - { - return default; - } - - // this looks like a good statement, add to the right size of the assignment to track as that's what - // we'll want to put in the final collection expression. - matches.Add(new(expressionStatement, UseSpread: false)); - currentStatement = currentStatement.GetNextStatement(); - } - } - } - - if (!UseCollectionExpressionHelpers.CanReplaceWithCollectionExpression( - semanticModel, expression, skipVerificationForReplacedNode: true, cancellationToken)) - { - return default; - } - - return matches.ToImmutable(); + return UseCollectionExpressionHelpers.TryGetMatches( + semanticModel, + expression, + static e => e.Type, + static e => e.Initializer, + cancellationToken); } } diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CollectionExpressionMatch.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CollectionExpressionMatch.cs index cfd5a8e06561..00c5befd3eb7 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CollectionExpressionMatch.cs +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CollectionExpressionMatch.cs @@ -5,7 +5,7 @@ using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.UseCollectionInitializer; -namespace Microsoft.CodeAnalysis.UseCollectionExpression; +namespace Microsoft.CodeAnalysis.CSharp.UseCollectionExpression; /// internal readonly record struct CollectionExpressionMatch( diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/UseCollectionExpressionHelpers.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/UseCollectionExpressionHelpers.cs index f14ef85900ff..4773df8ce3a6 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/UseCollectionExpressionHelpers.cs +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/UseCollectionExpressionHelpers.cs @@ -3,16 +3,18 @@ // See the LICENSE file in the project root for more information. using System; +using System.Collections.Immutable; using System.Linq; using System.Threading; using Microsoft.CodeAnalysis.CSharp.Extensions; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.CSharp.Utilities; +using Microsoft.CodeAnalysis.PooledObjects; using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.Text; using Roslyn.Utilities; -namespace Microsoft.CodeAnalysis.CSharp.Analyzers.UseCollectionExpression; +namespace Microsoft.CodeAnalysis.CSharp.UseCollectionExpression; using static SyntaxFactory; @@ -293,4 +295,104 @@ private static bool ShouldReplaceExistingExpressionEntirely( // For this we want to remove the 'new' portion, but keep the collection on its own line. return false; } + + public static ImmutableArray TryGetMatches( + SemanticModel semanticModel, + TArrayCreationExpressionSyntax expression, + Func getType, + Func getInitializer, + CancellationToken cancellationToken) + where TArrayCreationExpressionSyntax : ExpressionSyntax + { + // has to either be `stackalloc X[]` or `stackalloc X[const]`. + if (getType(expression) is not ArrayTypeSyntax { RankSpecifiers: [{ Sizes: [var size] } rankSpecifier] } arrayType) + return default; + + using var _ = ArrayBuilder.GetInstance(out var matches); + + var initializer = getInitializer(expression); + if (size is OmittedArraySizeExpressionSyntax) + { + // `stackalloc int[]` on its own is illegal. Has to either have a size, or an initializer. + if (initializer is null) + return default; + } + else + { + // if `stackalloc X[val]`, then it `val` has to be a constant value. + if (semanticModel.GetConstantValue(size, cancellationToken).Value is not int sizeValue) + return default; + + if (initializer != null) + { + // if there is an initializer, then it has to have the right number of elements. + if (sizeValue != initializer.Expressions.Count) + return default; + } + else + { + // if there is no initializer, we have to be followed by direct statements that initialize the right + // number of elements. + + // This needs to be local variable like `ReadOnlySpan x = stackalloc ... + if (expression.WalkUpParentheses().Parent is not EqualsValueClauseSyntax + { + Parent: VariableDeclaratorSyntax + { + Identifier.ValueText: var variableName, + Parent.Parent: LocalDeclarationStatementSyntax localDeclarationStatement + }, + }) + { + return default; + } + + var currentStatement = localDeclarationStatement.GetNextStatement(); + for (var currentIndex = 0; currentIndex < sizeValue; currentIndex++) + { + // Each following statement needs to of the form: + // + // x[...] = + if (currentStatement is not ExpressionStatementSyntax + { + Expression: AssignmentExpressionSyntax + { + Left: ElementAccessExpressionSyntax + { + Expression: IdentifierNameSyntax { Identifier.ValueText: var elementName }, + ArgumentList.Arguments: [var elementArgument], + } elementAccess, + } + } expressionStatement) + { + return default; + } + + // Ensure we're indexing into the variable created. + if (variableName != elementName) + return default; + + // The indexing value has to equal the corresponding location in the result. + if (semanticModel.GetConstantValue(elementArgument.Expression, cancellationToken).Value is not int indexValue || + indexValue != currentIndex) + { + return default; + } + + // this looks like a good statement, add to the right size of the assignment to track as that's what + // we'll want to put in the final collection expression. + matches.Add(new(expressionStatement, UseSpread: false)); + currentStatement = currentStatement.GetNextStatement(); + } + } + } + + if (!CanReplaceWithCollectionExpression( + semanticModel, expression, skipVerificationForReplacedNode: true, cancellationToken)) + { + return default; + } + + return matches.ToImmutable(); + } } diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerDiagnosticAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerDiagnosticAnalyzer.cs index d9ed0d600a1e..0056467a2655 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerDiagnosticAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionInitializer/CSharpUseCollectionInitializerDiagnosticAnalyzer.cs @@ -2,17 +2,15 @@ // 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; using Microsoft.CodeAnalysis.CSharp.LanguageService; using Microsoft.CodeAnalysis.CSharp.Shared.Extensions; using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.CSharp.UseCollectionExpression; using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.LanguageService; using Microsoft.CodeAnalysis.UseCollectionInitializer; -using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.CSharp.UseCollectionInitializer; diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpCollectionExpressionRewriter.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpCollectionExpressionRewriter.cs index fad56cc06432..76e6f594a816 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpCollectionExpressionRewriter.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpCollectionExpressionRewriter.cs @@ -9,20 +9,16 @@ using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis.CodeActions; -using Microsoft.CodeAnalysis.CSharp.Analyzers.UseCollectionExpression; using Microsoft.CodeAnalysis.CSharp.Extensions; using Microsoft.CodeAnalysis.CSharp.Syntax; -using Microsoft.CodeAnalysis.CSharp.UseCollectionInitializer; using Microsoft.CodeAnalysis.Formatting; using Microsoft.CodeAnalysis.Indentation; using Microsoft.CodeAnalysis.PooledObjects; using Microsoft.CodeAnalysis.Shared.Extensions; -using Microsoft.CodeAnalysis.UseCollectionExpression; using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.CSharp.UseCollectionExpression; -using static CSharpCollectionInitializerRewriter; using static SyntaxFactory; internal static class CSharpCollectionExpressionRewriter diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForArrayCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForArrayCodeFixProvider.cs index fb5e40a6178e..e7ec9189952b 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForArrayCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForArrayCodeFixProvider.cs @@ -10,8 +10,6 @@ using System.Threading.Tasks; using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.CodeFixes; -using Microsoft.CodeAnalysis.CSharp.Analyzers.UseCollectionExpression; -using Microsoft.CodeAnalysis.CSharp.Extensions; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Editing; @@ -22,7 +20,6 @@ namespace Microsoft.CodeAnalysis.CSharp.UseCollectionExpression; -using static SyntaxFactory; using static UseCollectionExpressionHelpers; [ExportCodeFixProvider(LanguageNames.CSharp, Name = PredefinedCodeFixProviderNames.UseCollectionExpressionForArray), Shared] diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocCodeFixProvider.cs index ccca695eee01..fcf380fa5f8e 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocCodeFixProvider.cs @@ -11,25 +11,15 @@ using System.Threading.Tasks; using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.CodeFixes; -using Microsoft.CodeAnalysis.CSharp.Analyzers.UseCollectionExpression; -using Microsoft.CodeAnalysis.CSharp.Extensions; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Editing; -using Microsoft.CodeAnalysis.Formatting; using Microsoft.CodeAnalysis.Host.Mef; using Microsoft.CodeAnalysis.LanguageService; -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; - -using static SyntaxFactory; -using static UseCollectionExpressionHelpers; - [ExportCodeFixProvider(LanguageNames.CSharp, Name = PredefinedCodeFixProviderNames.UseCollectionExpressionForStackAlloc), Shared] internal partial class CSharpUseCollectionExpressionForStackAllocCodeFixProvider : SyntaxEditorBasedCodeFixProvider { diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionExpression.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionExpression.cs index d17c815d03fc..89a98d2a6e58 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionExpression.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionExpression.cs @@ -8,7 +8,6 @@ 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 From 93ebb0db68b9d1f9ee730262b7f35e3192ca599b Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sat, 5 Aug 2023 14:28:20 -0700 Subject: [PATCH 2/7] update analyzer --- ...ionExpressionForArrayDiagnosticAnalyzer.cs | 93 ++++++++++++++----- 1 file changed, 70 insertions(+), 23 deletions(-) diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForArrayDiagnosticAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForArrayDiagnosticAnalyzer.cs index 07dcbd461fcb..f378bcf08278 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForArrayDiagnosticAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForArrayDiagnosticAnalyzer.cs @@ -4,6 +4,7 @@ using System.Collections.Immutable; using System.Diagnostics; +using System.Threading; using Microsoft.CodeAnalysis.CodeStyle; using Microsoft.CodeAnalysis.CSharp.Extensions; using Microsoft.CodeAnalysis.CSharp.Shared.Extensions; @@ -59,12 +60,53 @@ private void OnCompilationStart(CompilationStartAnalysisContext context) context.RegisterCodeBlockStartAction(context => { context.RegisterSyntaxNodeAction( - context => AnalyzeArrayInitializer(context), + context => AnalyzeArrayInitializerExpression(context), SyntaxKind.ArrayInitializerExpression); + + context.RegisterSyntaxNodeAction( + context => AnalyzeArrayCreationExpression(context), + SyntaxKind.ArrayCreationExpression); }); } - private static void AnalyzeArrayInitializer(SyntaxNodeAnalysisContext context) + private static void AnalyzeArrayCreationExpression(SyntaxNodeAnalysisContext context) + { + var semanticModel = context.SemanticModel; + var syntaxTree = semanticModel.SyntaxTree; + var arrayCreationExpression = (ArrayCreationExpressionSyntax)context.Node; + var cancellationToken = context.CancellationToken; + + // Don't analyze arrays with initializers here, they're handled in AnalyzeArrayInitializerExpression instead. + if (arrayCreationExpression.Initializer != null) + return; + + // no point in analyzing if the option is off. + var option = context.GetAnalyzerOptions().PreferCollectionExpression; + if (!option.Value) + return; + + // Analyze the statements that follow to see if they can initialize this array. + var matches = TryGetMatches(semanticModel, arrayCreationExpression, cancellationToken); + if (matches.IsDefault) + return; + + ReportArrayCreationDiagnostics(context, syntaxTree, option, arrayCreationExpression); + } + + public static ImmutableArray TryGetMatches( + SemanticModel semanticModel, + ArrayCreationExpressionSyntax expression, + CancellationToken cancellationToken) + { + return UseCollectionExpressionHelpers.TryGetMatches( + semanticModel, + expression, + static e => e.Type, + static e => e.Initializer, + cancellationToken); + } + + private static void AnalyzeArrayInitializerExpression(SyntaxNodeAnalysisContext context) { var semanticModel = context.SemanticModel; var syntaxTree = semanticModel.SyntaxTree; @@ -94,27 +136,7 @@ private static void AnalyzeArrayInitializer(SyntaxNodeAnalysisContext context) if (isConcreteOrImplicitArrayCreation) { - var locations = ImmutableArray.Create(arrayCreationExpression.GetLocation()); - context.ReportDiagnostic(DiagnosticHelper.Create( - s_descriptor, - arrayCreationExpression.GetFirstToken().GetLocation(), - option.Notification.Severity, - additionalLocations: locations, - properties: null)); - - var additionalUnnecessaryLocations = ImmutableArray.Create( - syntaxTree.GetLocation(TextSpan.FromBounds( - arrayCreationExpression.SpanStart, - arrayCreationExpression is ArrayCreationExpressionSyntax arrayCreation - ? arrayCreation.Type.Span.End - : ((ImplicitArrayCreationExpressionSyntax)arrayCreationExpression).CloseBracketToken.Span.End))); - - context.ReportDiagnostic(DiagnosticHelper.CreateWithLocationTags( - s_unnecessaryCodeDescriptor, - additionalUnnecessaryLocations[0], - ReportDiagnostic.Default, - additionalLocations: locations, - additionalUnnecessaryLocations: additionalUnnecessaryLocations)); + ReportArrayCreationDiagnostics(context, syntaxTree, option, arrayCreationExpression); } else { @@ -130,4 +152,29 @@ arrayCreationExpression is ArrayCreationExpressionSyntax arrayCreation properties: null)); } } + + private static void ReportArrayCreationDiagnostics(SyntaxNodeAnalysisContext context, SyntaxTree syntaxTree, CodeStyleOption2 option, ExpressionSyntax expression) + { + var locations = ImmutableArray.Create(expression.GetLocation()); + context.ReportDiagnostic(DiagnosticHelper.Create( + s_descriptor, + expression.GetFirstToken().GetLocation(), + option.Notification.Severity, + additionalLocations: locations, + properties: null)); + + var additionalUnnecessaryLocations = ImmutableArray.Create( + syntaxTree.GetLocation(TextSpan.FromBounds( + expression.SpanStart, + expression is ArrayCreationExpressionSyntax arrayCreationExpression + ? arrayCreationExpression.Type.Span.End + : ((ImplicitArrayCreationExpressionSyntax)expression).CloseBracketToken.Span.End))); + + context.ReportDiagnostic(DiagnosticHelper.CreateWithLocationTags( + s_unnecessaryCodeDescriptor, + additionalUnnecessaryLocations[0], + ReportDiagnostic.Default, + additionalLocations: locations, + additionalUnnecessaryLocations: additionalUnnecessaryLocations)); + } } From 5b4a86c5692a0bf55c4313d477a54790147dd646 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sat, 5 Aug 2023 14:38:09 -0700 Subject: [PATCH 3/7] Share code --- ...ectionExpressionForArrayCodeFixProvider.cs | 218 +++++++++++++----- ...nExpressionForStackAllocCodeFixProvider.cs | 4 +- 2 files changed, 163 insertions(+), 59 deletions(-) diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForArrayCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForArrayCodeFixProvider.cs index e7ec9189952b..958bd50cab7b 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForArrayCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForArrayCodeFixProvider.cs @@ -3,6 +3,7 @@ // See the LICENSE file in the project root for more information. using System; +using System.Collections.Generic; using System.Collections.Immutable; using System.Composition; using System.Linq; @@ -14,6 +15,7 @@ using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Editing; using Microsoft.CodeAnalysis.Host.Mef; +using Microsoft.CodeAnalysis.LanguageService; using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.Text; using Roslyn.Utilities; @@ -42,84 +44,188 @@ public override Task RegisterCodeFixesAsync(CodeFixContext context) return Task.CompletedTask; } - protected override async Task FixAllAsync( + protected sealed override async Task FixAllAsync( Document document, ImmutableArray diagnostics, SyntaxEditor editor, CodeActionOptionsProvider fallbackOptions, CancellationToken cancellationToken) { - var sourceText = await document.GetTextAsync(cancellationToken).ConfigureAwait(false); + var services = document.Project.Solution.Services; + var syntaxFacts = document.GetRequiredLanguageService(); + var originalRoot = editor.OriginalRoot; - foreach (var diagnostic in diagnostics.OrderByDescending(d => d.Location.SourceSpan.Start)) + var arrayExpressions = new Stack(); + foreach (var diagnostic in diagnostics) { - var expression = diagnostic.AdditionalLocations[0].FindNode(getInnermostNodeForTie: true, cancellationToken); - if (expression is InitializerExpressionSyntax initializer) + var expression = (ExpressionSyntax)originalRoot.FindNode( + diagnostic.AdditionalLocations[0].SourceSpan, getInnermostNodeForTie: true); + arrayExpressions.Push(expression); + } + + // We're going to be continually editing this tree. Track all the nodes we + // care about so we can find them across each edit. + var semanticDocument = await SemanticDocument.CreateAsync( + document.WithSyntaxRoot(originalRoot.TrackNodes(arrayExpressions)), + cancellationToken).ConfigureAwait(false); + + while (arrayExpressions.Count > 0) + { + var arrayCreationExpression = semanticDocument.Root.GetCurrentNodes(arrayExpressions.Pop()).Single(); + if (arrayCreationExpression + is not ArrayCreationExpressionSyntax + and not ImplicitArrayCreationExpressionSyntax + and not InitializerExpressionSyntax) { - RewriteInitializerExpression(initializer); + continue; } - else if (expression is ArrayCreationExpressionSyntax arrayCreation) + + var subEditor = new SyntaxEditor(semanticDocument.Root, services); + + if (arrayCreationExpression is InitializerExpressionSyntax initializer) { - RewriteArrayCreationExpression(arrayCreation); + subEditor.ReplaceNode( + initializer, + ConvertInitializerToCollectionExpression( + initializer, + IsOnSingleLine(semanticDocument.Text, initializer))); } - else if (expression is ImplicitArrayCreationExpressionSyntax implicitArrayCreation) + else { - RewriteImplicitArrayCreationExpression(implicitArrayCreation); + var matches = GetMatches(semanticDocument, arrayCreationExpression); + if (matches.IsDefault) + continue; + + var collectionExpression = await CSharpCollectionExpressionRewriter.CreateCollectionExpressionAsync( + semanticDocument.Document, + fallbackOptions, + arrayCreationExpression, + matches, + static e => e switch + { + ArrayCreationExpressionSyntax arrayCreation => arrayCreation.Initializer, + ImplicitArrayCreationExpressionSyntax implicitArrayCreation => implicitArrayCreation.Initializer, + _ => throw ExceptionUtilities.Unreachable(), + }, + static (e, i) => e switch + { + ArrayCreationExpressionSyntax arrayCreation => arrayCreation.WithInitializer(i), + ImplicitArrayCreationExpressionSyntax implicitArrayCreation => implicitArrayCreation.WithInitializer(i), + _ => throw ExceptionUtilities.Unreachable(), + }, + cancellationToken).ConfigureAwait(false); + + subEditor.ReplaceNode(arrayCreationExpression, collectionExpression); + foreach (var match in matches) + subEditor.RemoveNode(match.Statement); } + + semanticDocument = await semanticDocument.WithSyntaxRootAsync( + subEditor.GetChangedRoot(), cancellationToken).ConfigureAwait(false); } + editor.ReplaceNode(originalRoot, semanticDocument.Root); return; static bool IsOnSingleLine(SourceText sourceText, SyntaxNode node) => sourceText.AreOnSameLine(node.GetFirstToken(), node.GetLastToken()); - void RewriteInitializerExpression(InitializerExpressionSyntax initializer) - { - editor.ReplaceNode( - initializer, - (current, _) => ConvertInitializerToCollectionExpression( - (InitializerExpressionSyntax)current, - IsOnSingleLine(sourceText, initializer))); - } - - void RewriteArrayCreationExpression(ArrayCreationExpressionSyntax arrayCreation) - { - Contract.ThrowIfNull(arrayCreation.Initializer); - - editor.ReplaceNode( - arrayCreation, - (current, _) => - { - var currentArrayCreation = (ArrayCreationExpressionSyntax)current; - Contract.ThrowIfNull(currentArrayCreation.Initializer); - - var isOnSingleLine = IsOnSingleLine(sourceText, arrayCreation.Initializer); - var collectionExpression = ConvertInitializerToCollectionExpression( - currentArrayCreation.Initializer, isOnSingleLine); - - return ReplaceWithCollectionExpression( - sourceText, arrayCreation.Initializer, collectionExpression, isOnSingleLine); - }); - } - - void RewriteImplicitArrayCreationExpression(ImplicitArrayCreationExpressionSyntax implicitArrayCreation) + ImmutableArray GetMatches(SemanticDocument document, ExpressionSyntax expression) { - Contract.ThrowIfNull(implicitArrayCreation.Initializer); - - editor.ReplaceNode( - implicitArrayCreation, - (current, _) => - { - var currentArrayCreation = (ImplicitArrayCreationExpressionSyntax)current; - Contract.ThrowIfNull(currentArrayCreation.Initializer); - - var isOnSingleLine = IsOnSingleLine(sourceText, implicitArrayCreation); - var collectionExpression = ConvertInitializerToCollectionExpression( - currentArrayCreation.Initializer, isOnSingleLine); - - return ReplaceWithCollectionExpression( - sourceText, implicitArrayCreation.Initializer, collectionExpression, isOnSingleLine); - }); + switch (expression) + { + case ImplicitArrayCreationExpressionSyntax: + // if we have `new[] { ... }` we have no subsequent matches to add to the collection. All values come + // from within the initializer. + return ImmutableArray.Empty; + + case ArrayCreationExpressionSyntax arrayCreation: + // we have `stackalloc T[...] ...;` defer to analyzer to find the items that follow that may need to + // be added to the collection expression. + return CSharpUseCollectionExpressionForArrayDiagnosticAnalyzer.TryGetMatches( + document.SemanticModel, arrayCreation, cancellationToken); + + default: + // We validated this is unreachable in the caller. + throw ExceptionUtilities.Unreachable(); + } } } + + //protected override async Task FixAllAsync( + // Document document, + // ImmutableArray diagnostics, + // SyntaxEditor editor, + // CodeActionOptionsProvider fallbackOptions, + // CancellationToken cancellationToken) + //{ + // var sourceText = await document.GetTextAsync(cancellationToken).ConfigureAwait(false); + + // foreach (var diagnostic in diagnostics.OrderByDescending(d => d.Location.SourceSpan.Start)) + // { + // var expression = diagnostic.AdditionalLocations[0].FindNode(getInnermostNodeForTie: true, cancellationToken); + // if (expression is InitializerExpressionSyntax initializer) + // { + // RewriteInitializerExpression(initializer); + // } + // else if (expression is ArrayCreationExpressionSyntax arrayCreation) + // { + // RewriteArrayCreationExpression(arrayCreation); + // } + // else if (expression is ImplicitArrayCreationExpressionSyntax implicitArrayCreation) + // { + // RewriteImplicitArrayCreationExpression(implicitArrayCreation); + // } + // } + + // return; + // void RewriteInitializerExpression(InitializerExpressionSyntax initializer) + // { + // editor.ReplaceNode( + // initializer, + // (current, _) => ConvertInitializerToCollectionExpression( + // (InitializerExpressionSyntax)current, + // IsOnSingleLine(sourceText, initializer))); + // } + + // void RewriteArrayCreationExpression(ArrayCreationExpressionSyntax arrayCreation) + // { + // Contract.ThrowIfNull(arrayCreation.Initializer); + + // editor.ReplaceNode( + // arrayCreation, + // (current, _) => + // { + // var currentArrayCreation = (ArrayCreationExpressionSyntax)current; + // Contract.ThrowIfNull(currentArrayCreation.Initializer); + + // var isOnSingleLine = IsOnSingleLine(sourceText, arrayCreation.Initializer); + // var collectionExpression = ConvertInitializerToCollectionExpression( + // currentArrayCreation.Initializer, isOnSingleLine); + + // return ReplaceWithCollectionExpression( + // sourceText, arrayCreation.Initializer, collectionExpression, isOnSingleLine); + // }); + // } + + // void RewriteImplicitArrayCreationExpression(ImplicitArrayCreationExpressionSyntax implicitArrayCreation) + // { + // Contract.ThrowIfNull(implicitArrayCreation.Initializer); + + // editor.ReplaceNode( + // implicitArrayCreation, + // (current, _) => + // { + // var currentArrayCreation = (ImplicitArrayCreationExpressionSyntax)current; + // Contract.ThrowIfNull(currentArrayCreation.Initializer); + + // var isOnSingleLine = IsOnSingleLine(sourceText, implicitArrayCreation); + // var collectionExpression = ConvertInitializerToCollectionExpression( + // currentArrayCreation.Initializer, isOnSingleLine); + + // return ReplaceWithCollectionExpression( + // sourceText, implicitArrayCreation.Initializer, collectionExpression, isOnSingleLine); + // }); + // } + //} } diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocCodeFixProvider.cs index fcf380fa5f8e..78b1d3e6d6fe 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocCodeFixProvider.cs @@ -67,9 +67,7 @@ protected sealed override async Task FixAllAsync( while (stackallocExpressions.Count > 0) { - var originalStackAllocExpression = stackallocExpressions.Pop(); - - var stackAllocExpression = semanticDocument.Root.GetCurrentNodes(originalStackAllocExpression).Single(); + var stackAllocExpression = semanticDocument.Root.GetCurrentNodes(stackallocExpressions.Pop()).Single(); if (stackAllocExpression is not StackAllocArrayCreationExpressionSyntax and not ImplicitStackAllocArrayCreationExpressionSyntax) continue; From a27aa6822d8cea46e8658ec3e09e78113d2240f3 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sat, 5 Aug 2023 14:43:15 -0700 Subject: [PATCH 4/7] Add tests --- .../UseCollectionExpressionForArrayTests.cs | 549 ++++++++++++++++++ 1 file changed, 549 insertions(+) diff --git a/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForArrayTests.cs b/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForArrayTests.cs index acb2c14eaba4..880848983449 100644 --- a/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForArrayTests.cs +++ b/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForArrayTests.cs @@ -1850,4 +1850,553 @@ class C LanguageVersion = LanguageVersionExtensions.CSharpNext, }.RunAsync(); } + + [Fact] + public async Task TestNoInitializer_MultiLine1() + { + await new VerifyCS.Test + { + TestCode = """ + using System; + + class C + { + void M(int i, int j) + { + int[] r = [|[|new|] int[1]|]; + r[0] = 1 + + 2; + } + } + """, + FixedCode = """ + using System; + + class C + { + void M(int i, int j) + { + int[] r = + [ + 1 + + 2 + ]; + } + } + """, + LanguageVersion = LanguageVersionExtensions.CSharpNext, + ReferenceAssemblies = ReferenceAssemblies.Net.Net70, + }.RunAsync(); + } + + [Fact] + public async Task TestNoInitializer_MultiLine2() + { + await new VerifyCS.Test + { + TestCode = """ + using System; + + class C + { + void M(int i, int j) + { + int[] r = [|[|new|] int[2]|]; + r[0] = 1 + + 2; + r[1] = 3 + + 4; + } + } + """, + FixedCode = """ + using System; + + class C + { + void M(int i, int j) + { + int[] r = + [ + 1 + + 2, + 3 + + 4 + ]; + } + } + """, + LanguageVersion = LanguageVersionExtensions.CSharpNext, + ReferenceAssemblies = ReferenceAssemblies.Net.Net70, + }.RunAsync(); + } + + [Fact] + public async Task TestNoInitializer_ZeroSize() + { + await new VerifyCS.Test + { + TestCode = """ + using System; + + class C + { + void M() + { + int[] r = [|[|new|] int[0]|]; + } + } + """, + FixedCode = """ + using System; + + class C + { + void M() + { + int[] r = []; + } + } + """, + LanguageVersion = LanguageVersionExtensions.CSharpNext, + ReferenceAssemblies = ReferenceAssemblies.Net.Net70, + }.RunAsync(); + } + + [Fact] + public async Task TestNoInitializer_NotEnoughFollowingStatements() + { + await new VerifyCS.Test + { + TestCode = """ + using System; + + class C + { + void M() + { + int[] r = new int[1]; + } + } + """, + LanguageVersion = LanguageVersionExtensions.CSharpNext, + ReferenceAssemblies = ReferenceAssemblies.Net.Net70, + }.RunAsync(); + } + + [Fact] + public async Task TestNoInitializer_WrongFollowingStatement() + { + await new VerifyCS.Test + { + TestCode = """ + using System; + + class C + { + void M() + { + int[] r = new int[1]; + return; + } + } + """, + LanguageVersion = LanguageVersionExtensions.CSharpNext, + ReferenceAssemblies = ReferenceAssemblies.Net.Net70, + }.RunAsync(); + } + + [Fact] + public async Task TestNoInitializer_NotLocalStatementInitializer() + { + await new VerifyCS.Test + { + TestCode = """ + using System; + + class C + { + void M() + { + int[] r = Goo(new int[1]); + r[0] = 1; + } + + int[] Goo(int[] input) => default; + } + """, + LanguageVersion = LanguageVersionExtensions.CSharpNext, + ReferenceAssemblies = ReferenceAssemblies.Net.Net70, + }.RunAsync(); + } + + [Fact] + public async Task TestNoInitializer_ExpressionStatementNotAssignment() + { + await new VerifyCS.Test + { + TestCode = """ + using System; + + class C + { + void M(int i) + { + int[] r = new int[1]; + i++; + } + } + """, + LanguageVersion = LanguageVersionExtensions.CSharpNext, + ReferenceAssemblies = ReferenceAssemblies.Net.Net70, + }.RunAsync(); + } + + [Fact] + public async Task TestNoInitializer_AssignmentNotElementAccess() + { + await new VerifyCS.Test + { + TestCode = """ + using System; + + class C + { + void M(int i, int j) + { + int[] r = new int[1]; + i = j; + } + } + """, + LanguageVersion = LanguageVersionExtensions.CSharpNext, + ReferenceAssemblies = ReferenceAssemblies.Net.Net70, + }.RunAsync(); + } + + [Fact] + public async Task TestNoInitializer_ElementAccessNotToIdentifier() + { + await new VerifyCS.Test + { + TestCode = """ + using System; + + class C + { + static int[] array; + + void M(int i, int j) + { + int[] r = new int[1]; + C.array[0] = j; + } + } + """, + LanguageVersion = LanguageVersionExtensions.CSharpNext, + ReferenceAssemblies = ReferenceAssemblies.Net.Net70, + }.RunAsync(); + } + + [Fact] + public async Task TestNoInitializer_IdentifierNotEqualToVariableName() + { + await new VerifyCS.Test + { + TestCode = """ + using System; + + class C + { + static int[] array; + + void M(int i, int j) + { + int[] r = new int[1]; + array[0] = j; + } + } + """, + LanguageVersion = LanguageVersionExtensions.CSharpNext, + ReferenceAssemblies = ReferenceAssemblies.Net.Net70, + }.RunAsync(); + } + + [Fact] + public async Task TestNoInitializer_ArgumentNotConstant() + { + await new VerifyCS.Test + { + TestCode = """ + using System; + + class C + { + void M(int i, int j) + { + int[] r = new int[1]; + r[i] = j; + } + } + """, + LanguageVersion = LanguageVersionExtensions.CSharpNext, + ReferenceAssemblies = ReferenceAssemblies.Net.Net70, + }.RunAsync(); + } + + [Fact] + public async Task TestNoInitializer_ConstantArgumentNotCorrect1() + { + await new VerifyCS.Test + { + TestCode = """ + using System; + + class C + { + void M(int i, int j) + { + int[] r = new int[1]; + r[1] = j; + } + } + """, + LanguageVersion = LanguageVersionExtensions.CSharpNext, + ReferenceAssemblies = ReferenceAssemblies.Net.Net70, + }.RunAsync(); + } + + [Fact] + public async Task TestNoInitializer_ConstantArgumentNotCorrect2() + { + await new VerifyCS.Test + { + TestCode = """ + using System; + + class C + { + void M(int i, int j) + { + int[] r = new int[2]; + r[0] = i; + r[0] = j; + } + } + """, + LanguageVersion = LanguageVersionExtensions.CSharpNext, + ReferenceAssemblies = ReferenceAssemblies.Net.Net70, + }.RunAsync(); + } + + [Fact] + public async Task TestNoInitializer_OneElement() + { + await new VerifyCS.Test + { + TestCode = """ + using System; + + class C + { + void M(int i, int j) + { + int[] r = [|[|new|] int[1]|]; + r[0] = i; + } + } + """, + FixedCode = """ + using System; + + class C + { + void M(int i, int j) + { + int[] r = [i]; + } + } + """, + LanguageVersion = LanguageVersionExtensions.CSharpNext, + ReferenceAssemblies = ReferenceAssemblies.Net.Net70, + }.RunAsync(); + } + + [Fact] + public async Task TestNoInitializer_OneElement_MultipleFollowingStatements() + { + await new VerifyCS.Test + { + TestCode = """ + using System; + + class C + { + void M(int i, int j) + { + int[] r = [|[|new|] int[1]|]; + r[0] = i; + r[0] = j; + } + } + """, + FixedCode = """ + using System; + + class C + { + void M(int i, int j) + { + int[] r = [i]; + r[0] = j; + } + } + """, + LanguageVersion = LanguageVersionExtensions.CSharpNext, + ReferenceAssemblies = ReferenceAssemblies.Net.Net70, + }.RunAsync(); + } + + [Fact] + public async Task TestNoInitializer_TwoElement2() + { + await new VerifyCS.Test + { + TestCode = """ + using System; + + class C + { + void M(int i, int j) + { + int[] r = [|[|new|] int[2]|]; + r[0] = i; + r[1] = j; + } + } + """, + FixedCode = """ + using System; + + class C + { + void M(int i, int j) + { + int[] r = [i, j]; + } + } + """, + LanguageVersion = LanguageVersionExtensions.CSharpNext, + ReferenceAssemblies = ReferenceAssemblies.Net.Net70, + }.RunAsync(); + } + + [Fact] + public async Task TestNoInitializer_TwoElement2_Constant() + { + await new VerifyCS.Test + { + TestCode = """ + using System; + + class C + { + void M(int i, int j) + { + const int v = 1; + int[] r = [|[|new|] int[2]|]; + r[0] = i; + r[v] = j; + } + } + """, + FixedCode = """ + using System; + + class C + { + void M(int i, int j) + { + const int v = 1; + int[] r = [i, j]; + } + } + """, + LanguageVersion = LanguageVersionExtensions.CSharpNext, + ReferenceAssemblies = ReferenceAssemblies.Net.Net70, + }.RunAsync(); + } + + [Fact] + public async Task TestNoInitializer_TwoElement2_SecondWrongIndex() + { + await new VerifyCS.Test + { + TestCode = """ + using System; + + class C + { + void M(int i, int j) + { + int[] r = new int[2]; + r[0] = i; + r[0] = j; + } + } + """, + LanguageVersion = LanguageVersionExtensions.CSharpNext, + ReferenceAssemblies = ReferenceAssemblies.Net.Net70, + }.RunAsync(); + } + + [Fact] + public async Task TestNoInitializer_TwoElement2_SecondNonConstant() + { + await new VerifyCS.Test + { + TestCode = """ + using System; + + class C + { + void M(int i, int j) + { + var v = 1; + int[] r = new int[2]; + r[0] = i; + r[v] = j; + } + } + """, + LanguageVersion = LanguageVersionExtensions.CSharpNext, + ReferenceAssemblies = ReferenceAssemblies.Net.Net70, + }.RunAsync(); + } + + [Fact] + public async Task TestNoInitializer_TwoElement2_SecondWrongDestination() + { + await new VerifyCS.Test + { + TestCode = """ + using System; + + class C + { + static int[] array; + + void M(int i, int j) + { + var v = 1; + int[] r = new int[2]; + r[0] = i; + array[1] = j; + } + } + """, + LanguageVersion = LanguageVersionExtensions.CSharpNext, + ReferenceAssemblies = ReferenceAssemblies.Net.Net70, + }.RunAsync(); + } } From 7c3590d5e839c16b616f0e4fb78a2a906172b4e8 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sat, 5 Aug 2023 15:28:30 -0700 Subject: [PATCH 5/7] Add tests --- .../UseCollectionExpressionForArrayTests.cs | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForArrayTests.cs b/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForArrayTests.cs index 880848983449..e07771821d26 100644 --- a/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForArrayTests.cs +++ b/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForArrayTests.cs @@ -295,6 +295,72 @@ class C }.RunAsync(); } + [Fact] + public async Task TestWithExplicitArray_ExplicitSize() + { + await new VerifyCS.Test + { + TestCode = """ + class C + { + string[] i = [|[|new|] string[1]|] { "" }; + } + """, + FixedCode = """ + class C + { + string[] i = [""]; + } + """, + LanguageVersion = LanguageVersionExtensions.CSharpNext, + }.RunAsync(); + } + + [Fact] + public async Task TestWithExplicitArray_MultiDimensionalArray_ExplicitSizes1() + { + await new VerifyCS.Test + { + TestCode = """ + class C + { + string[,] i = new string[1, 1]; + } + """, + LanguageVersion = LanguageVersionExtensions.CSharpNext, + }.RunAsync(); + } + + [Fact] + public async Task TestWithExplicitArray_MultiDimensionalArray_ExplicitSizes2() + { + await new VerifyCS.Test + { + TestCode = """ + class C + { + string[,] i = new string[1, 1] { { "" } }; + } + """, + LanguageVersion = LanguageVersionExtensions.CSharpNext, + }.RunAsync(); + } + + [Fact] + public async Task TestWithExplicitArray_MultiDimensionalArray_ImplicitSizes1() + { + await new VerifyCS.Test + { + TestCode = """ + class C + { + string[,] i = new string[,] { { "" } }; + } + """, + LanguageVersion = LanguageVersionExtensions.CSharpNext, + }.RunAsync(); + } + [Fact] public async Task TestNotWithIncompatibleImplicitArrays() { From e83ff84829ad351eeb087e8203fb0c5878d7590b Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sat, 5 Aug 2023 15:38:29 -0700 Subject: [PATCH 6/7] Add tests --- .../UseCollectionExpressionHelpers.cs | 2 +- .../UseCollectionExpressionForArrayTests.cs | 79 +++++++++++++++++++ 2 files changed, 80 insertions(+), 1 deletion(-) diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/UseCollectionExpressionHelpers.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/UseCollectionExpressionHelpers.cs index 69b23c19b8ae..7df0a6f995e8 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/UseCollectionExpressionHelpers.cs +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/UseCollectionExpressionHelpers.cs @@ -307,7 +307,7 @@ public static ImmutableArray TryGetMatches.GetInstance(out var matches); diff --git a/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForArrayTests.cs b/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForArrayTests.cs index e07771821d26..28e43129e28a 100644 --- a/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForArrayTests.cs +++ b/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForArrayTests.cs @@ -2465,4 +2465,83 @@ void M(int i, int j) ReferenceAssemblies = ReferenceAssemblies.Net.Net70, }.RunAsync(); } + + [Fact] + public async Task TestNoInitializer_TwoElement_TwoDimensional1() + { + await new VerifyCS.Test + { + TestCode = """ + using System; + + class C + { + void M(int i, int j) + { + int[][] r = [|[|new|] int[2][]|]; + r[0] = null; + r[1] = null; + } + } + """, + FixedCode = """ + using System; + + class C + { + void M(int i, int j) + { + int[][] r = [null, null]; + } + } + """, + LanguageVersion = LanguageVersionExtensions.CSharpNext, + ReferenceAssemblies = ReferenceAssemblies.Net.Net70, + }.RunAsync(); + } + + [Fact] + public async Task TestNoInitializer_TwoElement_TwoDimensional2() + { + await new VerifyCS.Test + { + TestCode = """ + using System; + + class C + { + void M(int i, int j) + { + int[][] r = [|[|new|] int[2][]|]; + r[0] = [|[|new|][]|] { 1 }; + r[1] = [|[|new|] int[]|] { 2 }; + } + } + """, + FixedCode = """ + using System; + + class C + { + void M(int i, int j) + { + int[][] r = [new[] { 1 }, new int[] { 2 }]; + } + } + """, + BatchFixedCode = """ + using System; + + class C + { + void M(int i, int j) + { + int[][] r = [[1], [2]]; + } + } + """, + LanguageVersion = LanguageVersionExtensions.CSharpNext, + ReferenceAssemblies = ReferenceAssemblies.Net.Net70, + }.RunAsync(); + } } From 6b2758c97995a9f0d92848704fd264db95b4dab3 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 8 Aug 2023 15:08:25 -0700 Subject: [PATCH 7/7] Update src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForArrayCodeFixProvider.cs --- ...ectionExpressionForArrayCodeFixProvider.cs | 77 ------------------- 1 file changed, 77 deletions(-) diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForArrayCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForArrayCodeFixProvider.cs index 958bd50cab7b..fbaa5d268d35 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForArrayCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForArrayCodeFixProvider.cs @@ -151,81 +151,4 @@ ImmutableArray GetMatches(SemanticDocument document, } } } - - //protected override async Task FixAllAsync( - // Document document, - // ImmutableArray diagnostics, - // SyntaxEditor editor, - // CodeActionOptionsProvider fallbackOptions, - // CancellationToken cancellationToken) - //{ - // var sourceText = await document.GetTextAsync(cancellationToken).ConfigureAwait(false); - - // foreach (var diagnostic in diagnostics.OrderByDescending(d => d.Location.SourceSpan.Start)) - // { - // var expression = diagnostic.AdditionalLocations[0].FindNode(getInnermostNodeForTie: true, cancellationToken); - // if (expression is InitializerExpressionSyntax initializer) - // { - // RewriteInitializerExpression(initializer); - // } - // else if (expression is ArrayCreationExpressionSyntax arrayCreation) - // { - // RewriteArrayCreationExpression(arrayCreation); - // } - // else if (expression is ImplicitArrayCreationExpressionSyntax implicitArrayCreation) - // { - // RewriteImplicitArrayCreationExpression(implicitArrayCreation); - // } - // } - - // return; - // void RewriteInitializerExpression(InitializerExpressionSyntax initializer) - // { - // editor.ReplaceNode( - // initializer, - // (current, _) => ConvertInitializerToCollectionExpression( - // (InitializerExpressionSyntax)current, - // IsOnSingleLine(sourceText, initializer))); - // } - - // void RewriteArrayCreationExpression(ArrayCreationExpressionSyntax arrayCreation) - // { - // Contract.ThrowIfNull(arrayCreation.Initializer); - - // editor.ReplaceNode( - // arrayCreation, - // (current, _) => - // { - // var currentArrayCreation = (ArrayCreationExpressionSyntax)current; - // Contract.ThrowIfNull(currentArrayCreation.Initializer); - - // var isOnSingleLine = IsOnSingleLine(sourceText, arrayCreation.Initializer); - // var collectionExpression = ConvertInitializerToCollectionExpression( - // currentArrayCreation.Initializer, isOnSingleLine); - - // return ReplaceWithCollectionExpression( - // sourceText, arrayCreation.Initializer, collectionExpression, isOnSingleLine); - // }); - // } - - // void RewriteImplicitArrayCreationExpression(ImplicitArrayCreationExpressionSyntax implicitArrayCreation) - // { - // Contract.ThrowIfNull(implicitArrayCreation.Initializer); - - // editor.ReplaceNode( - // implicitArrayCreation, - // (current, _) => - // { - // var currentArrayCreation = (ImplicitArrayCreationExpressionSyntax)current; - // Contract.ThrowIfNull(currentArrayCreation.Initializer); - - // var isOnSingleLine = IsOnSingleLine(sourceText, implicitArrayCreation); - // var collectionExpression = ConvertInitializerToCollectionExpression( - // currentArrayCreation.Initializer, isOnSingleLine); - - // return ReplaceWithCollectionExpression( - // sourceText, implicitArrayCreation.Initializer, collectionExpression, isOnSingleLine); - // }); - // } - //} }