diff --git a/src/Analyzers/CSharp/Analyzers/CSharpAnalyzers.projitems b/src/Analyzers/CSharp/Analyzers/CSharpAnalyzers.projitems index 91ed7fdabcf6f..bbe94bbde5581 100644 --- a/src/Analyzers/CSharp/Analyzers/CSharpAnalyzers.projitems +++ b/src/Analyzers/CSharp/Analyzers/CSharpAnalyzers.projitems @@ -83,6 +83,7 @@ + diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForArrayDiagnosticAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForArrayDiagnosticAnalyzer.cs index cfc19db32ee33..6e1842012e3f0 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForArrayDiagnosticAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForArrayDiagnosticAnalyzer.cs @@ -91,7 +91,7 @@ private static void AnalyzeArrayCreationExpression(SyntaxNodeAnalysisContext con ReportArrayCreationDiagnostics(context, syntaxTree, option, arrayCreationExpression); } - public static ImmutableArray TryGetMatches( + public static ImmutableArray> TryGetMatches( SemanticModel semanticModel, ArrayCreationExpressionSyntax expression, CancellationToken cancellationToken) diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForCreateDiagnosticAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForCreateDiagnosticAnalyzer.cs new file mode 100644 index 0000000000000..a62b3c506bc56 --- /dev/null +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForCreateDiagnosticAnalyzer.cs @@ -0,0 +1,284 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Linq; +using System.Threading; +using Microsoft.CodeAnalysis.CodeStyle; +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.Shared.Extensions; +using Microsoft.CodeAnalysis.Text; +using Roslyn.Utilities; + +namespace Microsoft.CodeAnalysis.CSharp.UseCollectionExpression; + +[DiagnosticAnalyzer(LanguageNames.CSharp)] +internal sealed partial class CSharpUseCollectionExpressionForCreateDiagnosticAnalyzer + : AbstractBuiltInCodeStyleDiagnosticAnalyzer +{ + private const string CreateName = nameof(ImmutableArray.Create); + private const string CreateRangeName = nameof(ImmutableArray.CreateRange); + + public const string UnwrapArgument = nameof(UnwrapArgument); + + private static readonly ImmutableDictionary s_unwrapArgumentProperties = + ImmutableDictionary.Empty.Add(UnwrapArgument, UnwrapArgument); + + public override DiagnosticAnalyzerCategory GetAnalyzerCategory() + => DiagnosticAnalyzerCategory.SemanticSpanAnalysis; + + private static readonly DiagnosticDescriptor s_descriptor = CreateDescriptorWithId( + IDEDiagnosticIds.UseCollectionExpressionForCreateDiagnosticId, + EnforceOnBuildValues.UseCollectionExpressionForCreate, + new LocalizableResourceString(nameof(AnalyzersResources.Simplify_collection_initialization), AnalyzersResources.ResourceManager, typeof(AnalyzersResources)), + new LocalizableResourceString(nameof(AnalyzersResources.Collection_initialization_can_be_simplified), AnalyzersResources.ResourceManager, typeof(AnalyzersResources)), + isUnnecessary: false); + + private static readonly DiagnosticDescriptor s_unnecessaryCodeDescriptor = CreateDescriptorWithId( + IDEDiagnosticIds.UseCollectionExpressionForCreateDiagnosticId, + EnforceOnBuildValues.UseCollectionExpressionForCreate, + new LocalizableResourceString(nameof(AnalyzersResources.Simplify_collection_initialization), AnalyzersResources.ResourceManager, typeof(AnalyzersResources)), + new LocalizableResourceString(nameof(AnalyzersResources.Collection_initialization_can_be_simplified), AnalyzersResources.ResourceManager, typeof(AnalyzersResources)), + isUnnecessary: true); + + public CSharpUseCollectionExpressionForCreateDiagnosticAnalyzer() + : base(ImmutableDictionary.Empty + .Add(s_descriptor, CodeStyleOptions2.PreferCollectionExpression) + .Add(s_unnecessaryCodeDescriptor, CodeStyleOptions2.PreferCollectionExpression)) + { + } + + protected override void InitializeWorker(AnalysisContext context) + => context.RegisterCompilationStartAction(context => + { + var compilation = context.Compilation; + if (!compilation.LanguageVersion().SupportsCollectionExpressions()) + return; + + var collectionBuilderAttribute = compilation.CollectionBuilderAttribute(); + if (collectionBuilderAttribute is null) + return; + + // We wrap the SyntaxNodeAction within a CodeBlockStartAction, which allows us to + // get callbacks for object creation expression nodes, but analyze nodes across the entire code block + // and eventually report fading diagnostics with location outside this node. + // Without the containing CodeBlockStartAction, our reported diagnostic would be classified + // as a non-local diagnostic and would not participate in lightbulb for computing code fixes. + context.RegisterCodeBlockStartAction(context => + { + context.RegisterSyntaxNodeAction( + context => AnalyzeInvocationExpression(context, collectionBuilderAttribute), + SyntaxKind.InvocationExpression); + }); + }); + + private static void AnalyzeInvocationExpression( + SyntaxNodeAnalysisContext context, + INamedTypeSymbol collectionBuilderAttribute) + { + var semanticModel = context.SemanticModel; + var compilation = semanticModel.Compilation; + var syntaxTree = semanticModel.SyntaxTree; + var invocationExpression = (InvocationExpressionSyntax)context.Node; + var cancellationToken = context.CancellationToken; + + // no point in analyzing if the option is off. + var option = context.GetAnalyzerOptions().PreferCollectionExpression; + if (!option.Value) + return; + + // Looking for `XXX.Create(...)` + if (invocationExpression.Expression is not MemberAccessExpressionSyntax + { + RawKind: (int)SyntaxKind.SimpleMemberAccessExpression, + Name.Identifier.Value: CreateName or CreateRangeName, + } memberAccessExpression) + { + return; + } + + var createMethod = semanticModel.GetSymbolInfo(memberAccessExpression, cancellationToken).Symbol as IMethodSymbol; + if (createMethod is not { IsStatic: true }) + return; + + var factoryType = semanticModel.GetSymbolInfo(memberAccessExpression.Expression, cancellationToken).Symbol as INamedTypeSymbol; + if (factoryType is null) + return; + + // The pattern is a type like `ImmutableArray` (non-generic), returning an instance of `ImmutableArray`. The + // actual collection type (`ImmutableArray`) has to have a `[CollectionBuilder(...)]` attribute on it that + // then points at the factory type. + var collectionBuilderAttributeData = createMethod.ReturnType.OriginalDefinition + .GetAttributes() + .FirstOrDefault(a => collectionBuilderAttribute.Equals(a.AttributeClass)); + if (collectionBuilderAttributeData?.ConstructorArguments is not [{ Value: ITypeSymbol collectionBuilderType }, { Value: CreateName }]) + return; + + if (!factoryType.OriginalDefinition.Equals(collectionBuilderType.OriginalDefinition)) + return; + + // Ok, this is type that has a collection-builder option available. We can switch over if the current method + // we're calling has one of the following signatures: + // + // `Create()`. Trivial case, can be replaced with `[]`. + // `Create(T), Create(T, T), Create(T, T, T)` etc. + // `Create(params T[])` (passing as individual elements, or an array with an initializer) + // `Create(ReadOnlySpan)` (passing as a stack-alloc with an initializer) + // `Create(IEnumerable)` (passing as something with an initializer. + if (!IsCompatibleSignatureAndArguments( + compilation, invocationExpression, createMethod.OriginalDefinition, + out var unwrapArgument, cancellationToken)) + { + return; + } + + // Make sure we can actually use a collection expression in place of the full invocation. + if (!UseCollectionExpressionHelpers.CanReplaceWithCollectionExpression( + semanticModel, invocationExpression, skipVerificationForReplacedNode: true, cancellationToken)) + { + return; + } + + var locations = ImmutableArray.Create(invocationExpression.GetLocation()); + var properties = unwrapArgument ? s_unwrapArgumentProperties : null; + + context.ReportDiagnostic(DiagnosticHelper.Create( + s_descriptor, + memberAccessExpression.Name.Identifier.GetLocation(), + option.Notification.Severity, + additionalLocations: locations, + properties)); + + var additionalUnnecessaryLocations = ImmutableArray.Create( + syntaxTree.GetLocation(TextSpan.FromBounds( + invocationExpression.SpanStart, + invocationExpression.ArgumentList.OpenParenToken.Span.End)), + invocationExpression.ArgumentList.CloseParenToken.GetLocation()); + + context.ReportDiagnostic(DiagnosticHelper.CreateWithLocationTags( + s_unnecessaryCodeDescriptor, + additionalUnnecessaryLocations[0], + ReportDiagnostic.Default, + additionalLocations: locations, + additionalUnnecessaryLocations: additionalUnnecessaryLocations, + properties)); + } + + private static bool IsCompatibleSignatureAndArguments( + Compilation compilation, + InvocationExpressionSyntax invocationExpression, + IMethodSymbol originalCreateMethod, + out bool unwrapArgument, + CancellationToken cancellationToken) + { + unwrapArgument = false; + + var arguments = invocationExpression.ArgumentList.Arguments; + + // Don't bother offering if any of the arguments are named. It's unlikely for this to occur in practice, and it + // means we do not have to worry about order of operations. + if (arguments.Any(static a => a.NameColon != null)) + return false; + + if (originalCreateMethod.Name is CreateRangeName) + { + // If we have `CreateRange(IEnumerable values)` this is legal if we have an array, or no-arg object creation. + if (originalCreateMethod.Parameters is [ + { + Type: INamedTypeSymbol + { + Name: nameof(IEnumerable), + TypeArguments: [ITypeParameterSymbol { TypeParameterKind: TypeParameterKind.Method }] + } enumerableType + }] && enumerableType.OriginalDefinition.Equals(compilation.IEnumerableOfTType())) + { + var argExpression = arguments[0].Expression; + if (argExpression + is ArrayCreationExpressionSyntax { Initializer: not null } + or ImplicitArrayCreationExpressionSyntax) + { + unwrapArgument = true; + return true; + } + + if (argExpression is ObjectCreationExpressionSyntax objectCreation) + { + // Can't have any arguments, as we cannot preserve them once we grab out all the elements. + if (objectCreation.ArgumentList != null && objectCreation.ArgumentList.Arguments.Count > 0) + return false; + + // If it's got an initializer, it has to be a collection initializer (or an empty object initializer); + if (objectCreation.Initializer.IsKind(SyntaxKind.ObjectCreationExpression) && objectCreation.Initializer.Expressions.Count > 0) + return false; + + unwrapArgument = true; + return true; + } + } + } + else if (originalCreateMethod.Name is CreateName) + { + // `XXX.Create()` can be converted to `[]` + if (originalCreateMethod.Parameters.Length == 0) + return arguments.Count == 0; + + // If we have `Create(T)`, `Create(T, T)` etc., then this is convertible. + if (originalCreateMethod.Parameters.All(static p => p.Type is ITypeParameterSymbol { TypeParameterKind: TypeParameterKind.Method })) + return arguments.Count == originalCreateMethod.Parameters.Length; + + // If we have `Create(params T[])` this is legal if there are multiple arguments. Or a single argument that + // is an array literal. + if (originalCreateMethod.Parameters is [{ IsParams: true, Type: IArrayTypeSymbol { ElementType: ITypeParameterSymbol { TypeParameterKind: TypeParameterKind.Method } } }]) + { + if (arguments.Count >= 2) + return true; + + if (arguments is [{ Expression: ArrayCreationExpressionSyntax { Initializer: not null } or ImplicitArrayCreationExpressionSyntax }]) + { + unwrapArgument = true; + return true; + } + + return false; + } + + // If we have `Create(ReadOnlySpan values)` this is legal if a stack-alloc expression is passed along. + // + // Runtime needs to support inline arrays in order for this to be ok. Otherwise compiler will change the + // stack alloc to a heap alloc, which could be very bad for user perf. + + if (arguments.Count == 1 && + compilation.SupportsRuntimeCapability(RuntimeCapability.InlineArrayTypes) && + originalCreateMethod.Parameters is [ + { + Type: INamedTypeSymbol + { + Name: nameof(Span) or nameof(ReadOnlySpan), + TypeArguments: [ITypeParameterSymbol { TypeParameterKind: TypeParameterKind.Method }] + } spanType + }]) + { + if (spanType.OriginalDefinition.Equals(compilation.SpanOfTType()) || + spanType.OriginalDefinition.Equals(compilation.ReadOnlySpanOfTType())) + { + if (arguments[0].Expression + is StackAllocArrayCreationExpressionSyntax { Initializer: not null } + or ImplicitStackAllocArrayCreationExpressionSyntax) + { + unwrapArgument = true; + return true; + } + } + } + } + + return false; + } +} diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocDiagnosticAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocDiagnosticAnalyzer.cs index 0482fb4872058..2b31bd32bc40a 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocDiagnosticAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocDiagnosticAnalyzer.cs @@ -146,7 +146,7 @@ private static void AnalyzeExplicitStackAllocExpression(SyntaxNodeAnalysisContex additionalUnnecessaryLocations: additionalUnnecessaryLocations)); } - public static ImmutableArray TryGetMatches( + public static ImmutableArray> TryGetMatches( SemanticModel semanticModel, StackAllocArrayCreationExpressionSyntax expression, CancellationToken cancellationToken) diff --git a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CollectionExpressionMatch.cs b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CollectionExpressionMatch.cs index 00c5befd3eb78..8528791948c89 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CollectionExpressionMatch.cs +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CollectionExpressionMatch.cs @@ -2,12 +2,11 @@ // 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.CSharp.Syntax; using Microsoft.CodeAnalysis.UseCollectionInitializer; namespace Microsoft.CodeAnalysis.CSharp.UseCollectionExpression; /// -internal readonly record struct CollectionExpressionMatch( - StatementSyntax Statement, - bool UseSpread); +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 7df0a6f995e8e..916dda0123bb5 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/UseCollectionExpressionHelpers.cs +++ b/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/UseCollectionExpressionHelpers.cs @@ -296,7 +296,7 @@ private static bool ShouldReplaceExistingExpressionEntirely( return false; } - public static ImmutableArray TryGetMatches( + public static ImmutableArray> TryGetMatches( SemanticModel semanticModel, TArrayCreationExpressionSyntax expression, Func getType, @@ -310,7 +310,7 @@ public static ImmutableArray TryGetMatches.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/CodeFixes/CSharpCodeFixes.projitems b/src/Analyzers/CSharp/CodeFixes/CSharpCodeFixes.projitems index 57c3109a58f6a..d4b86494683c8 100644 --- a/src/Analyzers/CSharp/CodeFixes/CSharpCodeFixes.projitems +++ b/src/Analyzers/CSharp/CodeFixes/CSharpCodeFixes.projitems @@ -77,6 +77,7 @@ + diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpCollectionExpressionRewriter.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpCollectionExpressionRewriter.cs index 29a80a5bbb165..b2d8248c762c3 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpCollectionExpressionRewriter.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpCollectionExpressionRewriter.cs @@ -27,15 +27,16 @@ internal static class CSharpCollectionExpressionRewriter /// Creates the final collection-expression [...] that will replace the given expression. /// - public static async Task CreateCollectionExpressionAsync( + public static async Task CreateCollectionExpressionAsync( Document workspaceDocument, CodeActionOptionsProvider fallbackOptions, TParentExpression expressionToReplace, - ImmutableArray matches, + ImmutableArray> matches, Func getInitializer, Func withInitializer, CancellationToken cancellationToken) where TParentExpression : ExpressionSyntax + where TMatchNode : SyntaxNode { // This method is quite complex, but primarily because it wants to perform all the trivia handling itself. // We are moving nodes around in the tree in complex ways, and the formatting engine is just not sufficient @@ -258,7 +259,7 @@ CollectionExpressionSyntax CreateCollectionExpressionWithExistingElements() // 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) @@ -349,7 +350,7 @@ CollectionExpressionSyntax AddMatchesToExistingNonEmptyCollectionExpression( } static CollectionElementSyntax CreateCollectionElement( - CollectionExpressionMatch? match, ExpressionSyntax expression) + CollectionExpressionMatch? match, ExpressionSyntax expression) { return match?.UseSpread is true ? SpreadElement( @@ -359,25 +360,25 @@ static CollectionElementSyntax CreateCollectionElement( } CollectionElementSyntax CreateElement( - CollectionExpressionMatch match, string? preferredIndentation) + CollectionExpressionMatch match, string? preferredIndentation) { - var statement = match.Statement; + var node = match.Node; - if (statement is ExpressionStatementSyntax expressionStatement) + if (node is ExpressionStatementSyntax expressionStatement) { // Create: `x` for `collection.Add(x)` or `.. x` for `collection.AddRange(x)` return CreateCollectionElement( match, ConvertExpression(expressionStatement.Expression, expr => IndentExpression(expressionStatement, expr, preferredIndentation))); } - else if (statement is ForEachStatementSyntax foreachStatement) + else if (node is ForEachStatementSyntax foreachStatement) { // Create: `.. x` for `foreach (var v in x) collection.Add(v)` return CreateCollectionElement( match, IndentExpression(foreachStatement, foreachStatement.Expression, preferredIndentation)); } - else if (statement is IfStatementSyntax ifStatement) + else if (node is IfStatementSyntax ifStatement) { var condition = IndentExpression(ifStatement, ifStatement.Condition, preferredIndentation).Parenthesize(includeElasticTrivia: false); var trueStatement = (ExpressionStatementSyntax)UnwrapEmbeddedStatement(ifStatement.Statement); @@ -403,6 +404,10 @@ CollectionElementSyntax CreateElement( return CreateCollectionElement(match, expression); } } + else if (node is ExpressionSyntax expression) + { + return CreateCollectionElement(match, IndentExpression(parentStatement: null, expression, preferredIndentation)); + } else { throw ExceptionUtilities.Unreachable(); @@ -410,7 +415,7 @@ CollectionElementSyntax CreateElement( } ExpressionSyntax IndentExpression( - StatementSyntax parentStatement, + StatementSyntax? parentStatement, ExpressionSyntax expression, string? preferredIndentation) { @@ -451,7 +456,7 @@ ExpressionSyntax IndentExpression( }); // Now, once we've indented the expression, attempt to move comments on its containing statement to it. - return TransferComments(parentStatement, updatedExpression, preferredIndentation); + return TransferParentStatementComments(parentStatement, updatedExpression, preferredIndentation); } SyntaxToken IndentToken( @@ -503,11 +508,14 @@ SyntaxTrivia GetIndentedWhitespaceTrivia(string preferredIndentation, string fir : preferredIndentation); } - static ExpressionSyntax TransferComments( - StatementSyntax parentStatement, + static ExpressionSyntax TransferParentStatementComments( + StatementSyntax? parentStatement, ExpressionSyntax expression, string preferredIndentation) { + if (parentStatement is null) + return expression; + using var _1 = ArrayBuilder.GetInstance(out var newLeadingTrivia); using var _2 = ArrayBuilder.GetInstance(out var newTrailingTrivia); @@ -569,12 +577,10 @@ string GetIndentationStringForToken(SyntaxToken token) string GetIndentationStringForPosition(int position) { - var tokenLine = document.Text.Lines.GetLineFromPosition(position); - var indentation = position - tokenLine.Start; - var indentationString = new IndentationResult(indentation, offset: 0).GetIndentationString( - document.Text, indentationOptions); - - return indentationString; + var lineContainingPosition = document.Text.Lines.GetLineFromPosition(position); + var lineText = lineContainingPosition.ToString(); + var indentation = lineText.ConvertTabToSpace(formattingOptions.TabSize, initialColumn: 0, endPosition: position - lineContainingPosition.Start); + return indentation.CreateIndentationString(formattingOptions.UseTabs, formattingOptions.TabSize); } bool MakeMultiLineCollectionExpression() @@ -586,17 +592,17 @@ bool MakeMultiLineCollectionExpression() totalLength += expression.Span.Length; } - foreach (var (statement, _) in matches) + foreach (var (node, _) in matches) { // 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 (statement.GetLeadingTrivia().Any(static t => t.IsSingleOrMultiLineComment()) || - statement.GetTrailingTrivia().Any(static t => t.IsSingleOrMultiLineComment())) + if (node.GetLeadingTrivia().Any(static t => t.IsSingleOrMultiLineComment()) || + node.GetTrailingTrivia().Any(static t => t.IsSingleOrMultiLineComment())) { return true; } - foreach (var component in GetElementComponents(statement)) + 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())) @@ -609,23 +615,27 @@ bool MakeMultiLineCollectionExpression() return totalLength > wrappingLength; } - static IEnumerable GetElementComponents(StatementSyntax statement) + static IEnumerable GetElementComponents(TMatchNode node) { - if (statement is ExpressionStatementSyntax expressionStatement) + if (node is ExpressionStatementSyntax expressionStatement) { yield return expressionStatement.Expression; } - else if (statement is ForEachStatementSyntax foreachStatement) + else if (node is ForEachStatementSyntax foreachStatement) { yield return foreachStatement.Expression; } - else if (statement is IfStatementSyntax ifStatement) + else if (node is IfStatementSyntax ifStatement) { yield return ifStatement.Condition; yield return UnwrapEmbeddedStatement(ifStatement.Statement); if (ifStatement.Else != null) yield return UnwrapEmbeddedStatement(ifStatement.Else.Statement); } + else if (node is ExpressionSyntax expression) + { + yield return expression; + } } static StatementSyntax UnwrapEmbeddedStatement(StatementSyntax statement) diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForArrayCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForArrayCodeFixProvider.cs index ef6b1078e3e9e..8315afd325a6b 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForArrayCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForArrayCodeFixProvider.cs @@ -95,7 +95,7 @@ and not ImplicitArrayCreationExpressionSyntax editor.ReplaceNode(arrayCreationExpression, collectionExpression); foreach (var match in matches) - editor.RemoveNode(match.Statement); + editor.RemoveNode(match.Node); } return; @@ -103,13 +103,13 @@ and not ImplicitArrayCreationExpressionSyntax static bool IsOnSingleLine(SourceText sourceText, SyntaxNode node) => sourceText.AreOnSameLine(node.GetFirstToken(), node.GetLastToken()); - ImmutableArray GetMatches(SemanticModel semanticModel, ExpressionSyntax expression) + ImmutableArray> GetMatches(SemanticModel semanticModel, ExpressionSyntax expression) => expression switch { // if we have `new[] { ... }` we have no subsequent matches to add to the collection. All values come // from within the initializer. ImplicitArrayCreationExpressionSyntax - => ImmutableArray.Empty, + => ImmutableArray>.Empty, // we have `stackalloc T[...] ...;` defer to analyzer to find the items that follow that may need to // be added to the collection expression. diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForCreateCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForCreateCodeFixProvider.cs new file mode 100644 index 0000000000000..572789d8b2369 --- /dev/null +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForCreateCodeFixProvider.cs @@ -0,0 +1,111 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Collections.Immutable; +using System.Composition; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Editing; +using Microsoft.CodeAnalysis.Host.Mef; +using Microsoft.CodeAnalysis.Shared.Extensions; +using Roslyn.Utilities; + +namespace Microsoft.CodeAnalysis.CSharp.UseCollectionExpression; + +using static SyntaxFactory; + +[ExportCodeFixProvider(LanguageNames.CSharp, Name = PredefinedCodeFixProviderNames.UseCollectionExpressionForCreate), Shared] +internal partial class CSharpUseCollectionExpressionForCreateCodeFixProvider + : ForkingSyntaxEditorBasedCodeFixProvider +{ + [ImportingConstructor] + [Obsolete(MefConstruction.ImportingConstructorMessage, error: true)] + public CSharpUseCollectionExpressionForCreateCodeFixProvider() + : base(CSharpCodeFixesResources.Use_collection_expression, + IDEDiagnosticIds.UseCollectionExpressionForCreateDiagnosticId) + { + } + + public override ImmutableArray FixableDiagnosticIds { get; } = ImmutableArray.Create(IDEDiagnosticIds.UseCollectionExpressionForCreateDiagnosticId); + + protected override async Task FixAsync( + Document document, + SyntaxEditor editor, + CodeActionOptionsProvider fallbackOptions, + InvocationExpressionSyntax invocationExpression, + ImmutableDictionary properties, + CancellationToken cancellationToken) + { + var unwrapArgument = properties.ContainsKey(CSharpUseCollectionExpressionForCreateDiagnosticAnalyzer.UnwrapArgument); + + // We want to replace `XXX.Create(...)` with the new collection expression. To do this, we go through the + // following steps. First, we replace `XXX.Create(a, b, c)` with `new(a, b, c)` (an dummy object creation + // expression). We then call into our helper which replaces expressions with collection expressions. The reason + // for the dummy object creation expression is that it serves as an actual node the rewriting code can attach an + // initializer to, by which it can figure out appropriate wrapping and indentation for the collection expression + // elements. + + 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 = GetArguments(invocationExpression, unwrapArgument); + + var dummyObjectAnnotation = new SyntaxAnnotation(); + var dummyObjectCreation = ImplicitObjectCreationExpression(ArgumentList(arguments), initializer: null) + .WithTriviaFrom(invocationExpression) + .WithAdditionalAnnotations(dummyObjectAnnotation); + + var newSemanticDocument = await semanticDocument.WithSyntaxRootAsync( + 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 collectionExpression = await CSharpCollectionExpressionRewriter.CreateCollectionExpressionAsync( + newSemanticDocument.Document, + fallbackOptions, + dummyObjectCreation, + matches, + static o => o.Initializer, + static (o, i) => o.WithInitializer(i), + cancellationToken).ConfigureAwait(false); + + editor.ReplaceNode(invocationExpression, collectionExpression); + } + + private static SeparatedSyntaxList GetArguments(InvocationExpressionSyntax invocationExpression, bool unwrapArgument) + { + var arguments = invocationExpression.ArgumentList.Arguments; + + // If we're not unwrapping a singular argument expression, then just pass back all the explicit argument + // expressions the user wrote out. + if (!unwrapArgument) + return arguments; + + Contract.ThrowIfTrue(arguments.Count != 1); + var expression = arguments.Single().Expression; + + var initializer = expression switch + { + ImplicitArrayCreationExpressionSyntax implicitArray => implicitArray.Initializer, + ImplicitStackAllocArrayCreationExpressionSyntax implicitStackAlloc => implicitStackAlloc.Initializer, + ArrayCreationExpressionSyntax arrayCreation => arrayCreation.Initializer, + StackAllocArrayCreationExpressionSyntax stackAllocCreation => stackAllocCreation.Initializer, + ImplicitObjectCreationExpressionSyntax implicitObjectCreation => implicitObjectCreation.Initializer, + ObjectCreationExpressionSyntax objectCreation => objectCreation.Initializer, + _ => throw ExceptionUtilities.Unreachable(), + }; + + return initializer is null + ? default + : SeparatedList(initializer.Expressions.GetWithSeparators().Select( + nodeOrToken => nodeOrToken.IsToken ? nodeOrToken : Argument((ExpressionSyntax)nodeOrToken.AsNode()!))); + } +} diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocCodeFixProvider.cs index 50e1944b8f391..15de9976e8871 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocCodeFixProvider.cs @@ -69,17 +69,17 @@ protected sealed override async Task FixAsync( editor.ReplaceNode(stackAllocExpression, collectionExpression); foreach (var match in matches) - editor.RemoveNode(match.Statement); + editor.RemoveNode(match.Node); return; - ImmutableArray GetMatches() + ImmutableArray> GetMatches() => stackAllocExpression switch { // if we have `stackalloc[] { ... }` we have no subsequent matches to add to the collection. All values come // from within the initializer. ImplicitStackAllocArrayCreationExpressionSyntax - => ImmutableArray.Empty, + => ImmutableArray>.Empty, // we have `stackalloc T[...] ...;` defer to analyzer to find the items that follow that may need to // be added to the collection expression. diff --git a/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionExpression.cs b/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionExpression.cs index 89a98d2a6e58a..19c3f29b3662f 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionExpression.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCollectionInitializer/CSharpUseCollectionInitializerCodeFixProvider_CollectionExpression.cs @@ -29,7 +29,7 @@ private static Task CreateCollectionExpressionAsync( document, fallbackOptions, objectCreation, - matches.SelectAsArray(m => new CollectionExpressionMatch(m.Statement, m.UseSpread)), + matches.SelectAsArray(m => new CollectionExpressionMatch(m.Statement, m.UseSpread)), static objectCreation => objectCreation.Initializer, static (objectCreation, initializer) => objectCreation.WithInitializer(initializer), cancellationToken); diff --git a/src/Analyzers/CSharp/Tests/CSharpAnalyzers.UnitTests.projitems b/src/Analyzers/CSharp/Tests/CSharpAnalyzers.UnitTests.projitems index 2ee27c9a867b4..6da160b10a2a7 100644 --- a/src/Analyzers/CSharp/Tests/CSharpAnalyzers.UnitTests.projitems +++ b/src/Analyzers/CSharp/Tests/CSharpAnalyzers.UnitTests.projitems @@ -98,6 +98,7 @@ + diff --git a/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForCreateTests.cs b/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForCreateTests.cs new file mode 100644 index 0000000000000..3e11d205cd8f0 --- /dev/null +++ b/src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForCreateTests.cs @@ -0,0 +1,816 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System.Threading.Tasks; +using Microsoft.CodeAnalysis.CSharp.UseCollectionExpression; +using Microsoft.CodeAnalysis.Editor.UnitTests.CodeActions; +using Microsoft.CodeAnalysis.Test.Utilities; +using Microsoft.CodeAnalysis.Testing; +using Xunit; + +namespace Microsoft.CodeAnalysis.CSharp.Analyzers.UnitTests.UseCollectionExpression; + +using VerifyCS = CSharpCodeFixVerifier< + CSharpUseCollectionExpressionForCreateDiagnosticAnalyzer, + CSharpUseCollectionExpressionForCreateCodeFixProvider>; + +[Trait(Traits.Feature, Traits.Features.CodeActionsUseCollectionExpression)] +public class UseCollectionExpressionForCreateTests +{ + private const string s_collectionBuilderApi = """ + + namespace System.Runtime.CompilerServices + { + [AttributeUsage(AttributeTargets.All, Inherited = false, AllowMultiple = false)] + public sealed class CollectionBuilderAttribute : Attribute + { + public CollectionBuilderAttribute(Type builderType, string methodName) { } + } + } + """; + + private const string s_basicCollectionApi = """ + + static partial class MyCollection + { + public static MyCollection Create(System.ReadOnlySpan values) => default; + public static MyCollection Create(System.Span values) => default; + + public static MyCollection Create() => default; + public static MyCollection Create(T t1) => default; + public static MyCollection Create(T t1, T t2) => default; + public static MyCollection Create(T t1, T t2, T t3) => default; + public static MyCollection Create(T t1, T t2, T t3, T t4) => default; + public static MyCollection Create(params T[] values) => default; + public static MyCollection CreateRange(System.Collections.Generic.IEnumerable values) => default; + } + + [System.Runtime.CompilerServices.CollectionBuilder(typeof(MyCollection), "Create")] + class MyCollection : System.Collections.Generic.IEnumerable + { + public System.Collections.Generic.IEnumerator GetEnumerator() => default; + System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator() => default; + } + + [System.Runtime.CompilerServices.CollectionBuilder(typeof(MyCollection), "Create")] + interface IMyCollection : System.Collections.Generic.IEnumerable + { + } + """; + + [Fact] + public async Task TestNotInCSharp11() + { + + await new VerifyCS.Test + { + TestCode = """ + using System.Collections.Immutable; + + class C + { + MyCollection i = MyCollection.Create(1, 2, 3); + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + LanguageVersion = LanguageVersion.CSharp11, + ReferenceAssemblies = ReferenceAssemblies.Net.Net80, + }.RunAsync(); + } + + [Fact] + public async Task TestInCSharp12_Net70() + { + await new VerifyCS.Test + { + TestCode = """ + class C + { + MyCollection i = [|MyCollection.[|Create|](|]1, 2, 3); + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + FixedCode = """ + class C + { + MyCollection i = [1, 2, 3]; + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + LanguageVersion = LanguageVersion.CSharp12, + ReferenceAssemblies = ReferenceAssemblies.Net.Net70, + }.RunAsync(); + } + + [Fact] + public async Task TestEmpty() + { + await new VerifyCS.Test + { + TestCode = """ + class C + { + MyCollection i = [|MyCollection.[|Create|](|]); + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + FixedCode = """ + class C + { + MyCollection i = []; + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + LanguageVersion = LanguageVersion.CSharp12, + ReferenceAssemblies = ReferenceAssemblies.Net.Net70, + }.RunAsync(); + } + + [Fact] + public async Task TestOneElement() + { + await new VerifyCS.Test + { + TestCode = """ + class C + { + MyCollection i = [|MyCollection.[|Create|](|]1); + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + FixedCode = """ + class C + { + MyCollection i = [1]; + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + LanguageVersion = LanguageVersion.CSharp12, + ReferenceAssemblies = ReferenceAssemblies.Net.Net70, + }.RunAsync(); + } + + [Fact] + public async Task TestTwoElements() + { + await new VerifyCS.Test + { + TestCode = """ + class C + { + MyCollection i = [|MyCollection.[|Create|](|]1, 2); + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + FixedCode = """ + class C + { + MyCollection i = [1, 2]; + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + LanguageVersion = LanguageVersion.CSharp12, + ReferenceAssemblies = ReferenceAssemblies.Net.Net70, + }.RunAsync(); + } + + [Fact] + public async Task TestThreeElements() + { + await new VerifyCS.Test + { + TestCode = """ + class C + { + MyCollection i = [|MyCollection.[|Create|](|]1, 2, 3); + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + FixedCode = """ + class C + { + MyCollection i = [1, 2, 3]; + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + LanguageVersion = LanguageVersion.CSharp12, + ReferenceAssemblies = ReferenceAssemblies.Net.Net70, + }.RunAsync(); + } + + [Fact] + public async Task TestFourElements() + { + await new VerifyCS.Test + { + TestCode = """ + class C + { + MyCollection i = [|MyCollection.[|Create|](|]1, 2, 3, 4); + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + FixedCode = """ + class C + { + MyCollection i = [1, 2, 3, 4]; + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + LanguageVersion = LanguageVersion.CSharp12, + ReferenceAssemblies = ReferenceAssemblies.Net.Net70, + }.RunAsync(); + } + + [Fact] + public async Task TestParamsWithMultipleElements() + { + await new VerifyCS.Test + { + TestCode = """ + class C + { + MyCollection i = [|MyCollection.[|Create|](|]1, 2, 3, 4, 5); + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + FixedCode = """ + class C + { + MyCollection i = [1, 2, 3, 4, 5]; + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + LanguageVersion = LanguageVersion.CSharp12, + ReferenceAssemblies = ReferenceAssemblies.Net.Net70, + }.RunAsync(); + } + + [Fact] + public async Task TestParamsWithExplicitArrayArgument1() + { + await new VerifyCS.Test + { + TestCode = """ + class C + { + MyCollection i = MyCollection.Create(new int[5]); + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + LanguageVersion = LanguageVersion.CSharp12, + ReferenceAssemblies = ReferenceAssemblies.Net.Net70, + }.RunAsync(); + } + + [Fact] + public async Task TestParamsWithExplicitArrayArgument2() + { + await new VerifyCS.Test + { + TestCode = """ + class C + { + MyCollection i = [|MyCollection.[|Create|](|]new int[] { }); + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + FixedCode = """ + class C + { + MyCollection i = []; + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + LanguageVersion = LanguageVersion.CSharp12, + ReferenceAssemblies = ReferenceAssemblies.Net.Net70, + }.RunAsync(); + } + + [Fact] + public async Task TestParamsWithExplicitArrayArgument3() + { + await new VerifyCS.Test + { + TestCode = """ + class C + { + MyCollection i = [|MyCollection.[|Create|](|]new int[] { 1, 2, 3 }); + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + FixedCode = """ + class C + { + MyCollection i = [1, 2, 3]; + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + LanguageVersion = LanguageVersion.CSharp12, + ReferenceAssemblies = ReferenceAssemblies.Net.Net70, + }.RunAsync(); + } + + [Fact] + public async Task TestParamsWithImplicitArrayArgument1() + { + await new VerifyCS.Test + { + TestCode = """ + class C + { + MyCollection i = [|MyCollection.[|Create|](|]new[] { 1, 2, 3 }); + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + FixedCode = """ + class C + { + MyCollection i = [1, 2, 3]; + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + LanguageVersion = LanguageVersion.CSharp12, + ReferenceAssemblies = ReferenceAssemblies.Net.Net70, + }.RunAsync(); + } + + [Fact] + public async Task TestReadOnlySpan_ExplicitStackAlloc_Net70() + { + await new VerifyCS.Test + { + TestCode = """ + class C + { + MyCollection i = MyCollection.Create(stackalloc int[] { 1 }); + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + LanguageVersion = LanguageVersion.CSharp12, + ReferenceAssemblies = ReferenceAssemblies.Net.Net70, + }.RunAsync(); + } + + [Fact] + public async Task TestReadOnlySpan_ImplicitStackAlloc_Net70() + { + await new VerifyCS.Test + { + TestCode = """ + class C + { + MyCollection i = MyCollection.Create(stackalloc[] { 1 }); + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + LanguageVersion = LanguageVersion.CSharp12, + ReferenceAssemblies = ReferenceAssemblies.Net.Net70, + }.RunAsync(); + } + + [Fact] + public async Task TestReadOnlySpan_ExplicitStackAlloc_Net80_1() + { + await new VerifyCS.Test + { + TestCode = """ + class C + { + MyCollection i = [|MyCollection.[|Create|](|]stackalloc int[] { }); + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + FixedCode = """ + class C + { + MyCollection i = []; + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + LanguageVersion = LanguageVersion.CSharp12, + ReferenceAssemblies = ReferenceAssemblies.Net.Net80, + }.RunAsync(); + } + + [Fact] + public async Task TestReadOnlySpan_ExplicitStackAlloc_Net80_2() + { + await new VerifyCS.Test + { + TestCode = """ + class C + { + MyCollection i = [|MyCollection.[|Create|](|]stackalloc int[] { 1, 2, 3 }); + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + FixedCode = """ + class C + { + MyCollection i = [1, 2, 3]; + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + LanguageVersion = LanguageVersion.CSharp12, + ReferenceAssemblies = ReferenceAssemblies.Net.Net80, + }.RunAsync(); + } + + [Fact] + public async Task TestReadOnlySpan_ImplicitStackAlloc_Net80_1() + { + await new VerifyCS.Test + { + TestCode = """ + class C + { + MyCollection i = [|MyCollection.[|Create|](|]stackalloc[] { 1, 2, 3 }); + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + FixedCode = """ + class C + { + MyCollection i = [1, 2, 3]; + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + LanguageVersion = LanguageVersion.CSharp12, + ReferenceAssemblies = ReferenceAssemblies.Net.Net80, + }.RunAsync(); + } + + [Fact] + public async Task TestCreateRange_Null() + { + await new VerifyCS.Test + { + TestCode = """ + class C + { + MyCollection i = MyCollection.CreateRange(null); + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + LanguageVersion = LanguageVersion.CSharp12, + ReferenceAssemblies = ReferenceAssemblies.Net.Net70, + }.RunAsync(); + } + + [Fact] + public async Task TestCreateRange_ComputedExpression() + { + await new VerifyCS.Test + { + TestCode = """ + using System; + using System.Collections.Generic; + + class C + { + MyCollection i = MyCollection.CreateRange(GetValues()); + + static IEnumerable GetValues() => default; + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + LanguageVersion = LanguageVersion.CSharp12, + ReferenceAssemblies = ReferenceAssemblies.Net.Net70, + }.RunAsync(); + } + + [Fact] + public async Task TestCreateRange_ExplicitArray1() + { + await new VerifyCS.Test + { + TestCode = """ + class C + { + MyCollection i = MyCollection.CreateRange(new int [5]); + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + LanguageVersion = LanguageVersion.CSharp12, + ReferenceAssemblies = ReferenceAssemblies.Net.Net70, + }.RunAsync(); + } + + [Fact] + public async Task TestCreateRange_ExplicitArray2() + { + await new VerifyCS.Test + { + TestCode = """ + class C + { + MyCollection i = [|MyCollection.[|CreateRange|](|]new int[] { }); + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + FixedCode = """ + class C + { + MyCollection i = []; + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + LanguageVersion = LanguageVersion.CSharp12, + ReferenceAssemblies = ReferenceAssemblies.Net.Net70, + }.RunAsync(); + } + + [Fact] + public async Task TestCreateRange_ExplicitArray3() + { + await new VerifyCS.Test + { + TestCode = """ + class C + { + MyCollection i = [|MyCollection.[|CreateRange|](|]new int[] { 1, 2, 3 }); + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + FixedCode = """ + class C + { + MyCollection i = [1, 2, 3]; + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + LanguageVersion = LanguageVersion.CSharp12, + ReferenceAssemblies = ReferenceAssemblies.Net.Net70, + }.RunAsync(); + } + + [Fact] + public async Task TestCreateRange_ImplicitArray1() + { + await new VerifyCS.Test + { + TestCode = """ + class C + { + MyCollection i = [|MyCollection.[|CreateRange|](|]new[] { 1, 2, 3 }); + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + FixedCode = """ + class C + { + MyCollection i = [1, 2, 3]; + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + LanguageVersion = LanguageVersion.CSharp12, + ReferenceAssemblies = ReferenceAssemblies.Net.Net70, + }.RunAsync(); + } + + [Fact] + public async Task TestCreateRange_NewObjectWithArgument() + { + await new VerifyCS.Test + { + TestCode = """ + using System.Collections.Generic; + + class C + { + MyCollection i = MyCollection.CreateRange(new List(capacity: 0)); + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + LanguageVersion = LanguageVersion.CSharp12, + ReferenceAssemblies = ReferenceAssemblies.Net.Net70, + }.RunAsync(); + } + + [Fact] + public async Task TestCreateRange_NewObjectWithArgument2() + { + await new VerifyCS.Test + { + TestCode = """ + using System.Collections.Generic; + + class C + { + MyCollection i = MyCollection.CreateRange(new List(capacity: 0) { 1, 2, 3 }); + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + LanguageVersion = LanguageVersion.CSharp12, + ReferenceAssemblies = ReferenceAssemblies.Net.Net70, + }.RunAsync(); + } + + [Fact] + public async Task TestCreateRange_NewObjectWithoutArgument() + { + await new VerifyCS.Test + { + TestCode = """ + using System.Collections.Generic; + + class C + { + MyCollection i = [|MyCollection.[|CreateRange|](|]new List()); + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + FixedCode = """ + using System.Collections.Generic; + + class C + { + MyCollection i = []; + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + LanguageVersion = LanguageVersion.CSharp12, + ReferenceAssemblies = ReferenceAssemblies.Net.Net70, + }.RunAsync(); + } + + [Fact] + public async Task TestCreateRange_NewObjectWithInitializer1() + { + await new VerifyCS.Test + { + TestCode = """ + using System.Collections.Generic; + + class C + { + MyCollection i = [|MyCollection.[|CreateRange|](|]new List { }); + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + FixedCode = """ + using System.Collections.Generic; + + class C + { + MyCollection i = []; + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + LanguageVersion = LanguageVersion.CSharp12, + ReferenceAssemblies = ReferenceAssemblies.Net.Net70, + }.RunAsync(); + } + + [Fact] + public async Task TestCreateRange_NewObjectWithInitializer2() + { + await new VerifyCS.Test + { + TestCode = """ + using System.Collections.Generic; + + class C + { + MyCollection i = [|MyCollection.[|CreateRange|](|]new List() { }); + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + FixedCode = """ + using System.Collections.Generic; + + class C + { + MyCollection i = []; + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + LanguageVersion = LanguageVersion.CSharp12, + ReferenceAssemblies = ReferenceAssemblies.Net.Net70, + }.RunAsync(); + } + + [Fact] + public async Task TestCreateRange_NewObjectWithInitializer3() + { + await new VerifyCS.Test + { + TestCode = """ + using System.Collections.Generic; + + class C + { + MyCollection i = [|MyCollection.[|CreateRange|](|]new List { 1, 2, 3 }); + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + FixedCode = """ + using System.Collections.Generic; + + class C + { + MyCollection i = [1, 2, 3]; + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + LanguageVersion = LanguageVersion.CSharp12, + ReferenceAssemblies = ReferenceAssemblies.Net.Net70, + }.RunAsync(); + } + + [Fact] + public async Task TestCreateRange_NewObjectWithInitializer4() + { + await new VerifyCS.Test + { + TestCode = """ + using System.Collections.Generic; + + class C + { + MyCollection i = [|MyCollection.[|CreateRange|](|]new List { 1, 2, 3 }); + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + FixedCode = """ + using System.Collections.Generic; + + class C + { + MyCollection i = [1, 2, 3]; + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + LanguageVersion = LanguageVersion.CSharp12, + ReferenceAssemblies = ReferenceAssemblies.Net.Net70, + }.RunAsync(); + } + + [Fact] + public async Task TestCreateRange_NewImplicitObject() + { + await new VerifyCS.Test + { + TestCode = """ + using System.Collections.Generic; + + class C + { + MyCollection i = MyCollection.CreateRange({|CS0144:new() { }|]); + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + LanguageVersion = LanguageVersion.CSharp12, + ReferenceAssemblies = ReferenceAssemblies.Net.Net70, + }.RunAsync(); + } + + [Fact] + public async Task TestInterfaceDestination() + { + await new VerifyCS.Test + { + TestCode = """ + using System.Collections.Generic; + + class C + { + // Will start working once we allow reference conversions. + IMyCollection i = {|CS0266:MyCollection.Create(1, 2, 3)|}; + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + LanguageVersion = LanguageVersion.CSharp12, + ReferenceAssemblies = ReferenceAssemblies.Net.Net70, + }.RunAsync(); + } + + [Fact] + public async Task TestTrivia1() + { + await new VerifyCS.Test + { + TestCode = """ + class C + { + MyCollection i = /*leading*/ [|MyCollection.[|Create|](|]1) /*trailing*/; + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + FixedCode = """ + class C + { + MyCollection i = /*leading*/ [1] /*trailing*/; + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + LanguageVersion = LanguageVersion.CSharp12, + ReferenceAssemblies = ReferenceAssemblies.Net.Net70, + }.RunAsync(); + } + + [Fact] + public async Task TestMultiLine1() + { + await new VerifyCS.Test + { + TestCode = """ + class C + { + MyCollection i = [|MyCollection.[|Create|](|]1 + + 2); + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + FixedCode = """ + class C + { + MyCollection i = + [ + 1 + + 2, + ]; + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + LanguageVersion = LanguageVersion.CSharp12, + ReferenceAssemblies = ReferenceAssemblies.Net.Net70, + }.RunAsync(); + } + + [Fact] + public async Task TestMultiLine2() + { + await new VerifyCS.Test + { + TestCode = """ + class C + { + MyCollection i = [|MyCollection.[|Create|](|]1 + + 2, + 3 + + 4); + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + FixedCode = """ + class C + { + MyCollection i = + [ + 1 + + 2, + 3 + + 4, + ]; + } + """ + s_collectionBuilderApi + s_basicCollectionApi, + LanguageVersion = LanguageVersion.CSharp12, + ReferenceAssemblies = ReferenceAssemblies.Net.Net70, + }.RunAsync(); + } +} diff --git a/src/Analyzers/Core/Analyzers/EnforceOnBuildValues.cs b/src/Analyzers/Core/Analyzers/EnforceOnBuildValues.cs index d82362d23c3ff..24cd5f6009031 100644 --- a/src/Analyzers/Core/Analyzers/EnforceOnBuildValues.cs +++ b/src/Analyzers/Core/Analyzers/EnforceOnBuildValues.cs @@ -93,6 +93,7 @@ internal static class EnforceOnBuildValues public const EnforceOnBuild UseCollectionExpressionForArray = /*IDE0300*/ EnforceOnBuild.Recommended; public const EnforceOnBuild UseCollectionExpressionForEmpty = /*IDE0301*/ EnforceOnBuild.Recommended; public const EnforceOnBuild UseCollectionExpressionForStackAlloc = /*IDE0302*/ EnforceOnBuild.Recommended; + public const EnforceOnBuild UseCollectionExpressionForCreate = /*IDE0303*/ EnforceOnBuild.Recommended; /* EnforceOnBuild.WhenExplicitlyEnabled */ public const EnforceOnBuild RemoveUnnecessaryCast = /*IDE0004*/ EnforceOnBuild.WhenExplicitlyEnabled; // TODO: Move to 'Recommended' OR 'HighlyRecommended' bucket once performance problems are addressed: https://github.com/dotnet/roslyn/issues/43304 diff --git a/src/Analyzers/Core/Analyzers/IDEDiagnosticIds.cs b/src/Analyzers/Core/Analyzers/IDEDiagnosticIds.cs index 0ece9e6dd3f51..786731fa4f67b 100644 --- a/src/Analyzers/Core/Analyzers/IDEDiagnosticIds.cs +++ b/src/Analyzers/Core/Analyzers/IDEDiagnosticIds.cs @@ -195,6 +195,7 @@ internal static class IDEDiagnosticIds public const string UseCollectionExpressionForArrayDiagnosticId = "IDE0300"; public const string UseCollectionExpressionForEmptyDiagnosticId = "IDE0301"; public const string UseCollectionExpressionForStackAllocDiagnosticId = "IDE0302"; + public const string UseCollectionExpressionForCreateDiagnosticId = "IDE0303"; // Analyzer error Ids public const string AnalyzerChangedId = "IDE1001"; diff --git a/src/Analyzers/Core/CodeFixes/PredefinedCodeFixProviderNames.cs b/src/Analyzers/Core/CodeFixes/PredefinedCodeFixProviderNames.cs index 02355be445a5d..0b6bf008e657c 100644 --- a/src/Analyzers/Core/CodeFixes/PredefinedCodeFixProviderNames.cs +++ b/src/Analyzers/Core/CodeFixes/PredefinedCodeFixProviderNames.cs @@ -131,6 +131,7 @@ internal static class PredefinedCodeFixProviderNames public const string UseCoalesceExpressionForNullableTernaryConditionalCheck = nameof(UseCoalesceExpressionForNullableTernaryConditionalCheck); public const string UseCoalesceExpressionForTernaryConditionalCheck = nameof(UseCoalesceExpressionForTernaryConditionalCheck); public const string UseCollectionExpressionForArray = nameof(UseCollectionExpressionForArray); + public const string UseCollectionExpressionForCreate = nameof(UseCollectionExpressionForCreate); public const string UseCollectionExpressionForEmpty = nameof(UseCollectionExpressionForEmpty); public const string UseCollectionExpressionForStackAlloc = nameof(UseCollectionExpressionForStackAlloc); public const string UseCollectionInitializer = nameof(UseCollectionInitializer); diff --git a/src/EditorFeatures/CSharpTest/Formatting/CodeCleanupTests.cs b/src/EditorFeatures/CSharpTest/Formatting/CodeCleanupTests.cs index 8c232b8d8c836..3928ec2061e9b 100644 --- a/src/EditorFeatures/CSharpTest/Formatting/CodeCleanupTests.cs +++ b/src/EditorFeatures/CSharpTest/Formatting/CodeCleanupTests.cs @@ -563,8 +563,8 @@ public void VerifyAllCodeStyleFixersAreSupportedByCodeCleanup(string language) var expectedNumberOfUnsupportedDiagnosticIds = language switch { - LanguageNames.CSharp => 45, - LanguageNames.VisualBasic => 82, + LanguageNames.CSharp => 46, + LanguageNames.VisualBasic => 83, _ => throw ExceptionUtilities.UnexpectedValue(language), }; diff --git a/src/EditorFeatures/Test/Diagnostics/IDEDiagnosticIDConfigurationTests.cs b/src/EditorFeatures/Test/Diagnostics/IDEDiagnosticIDConfigurationTests.cs index ee51585fca8aa..edfff7d14e941 100644 --- a/src/EditorFeatures/Test/Diagnostics/IDEDiagnosticIDConfigurationTests.cs +++ b/src/EditorFeatures/Test/Diagnostics/IDEDiagnosticIDConfigurationTests.cs @@ -473,6 +473,9 @@ public void CSharp_VerifyIDEDiagnosticSeveritiesAreConfigurable() # IDE0302 dotnet_diagnostic.IDE0302.severity = %value% +# IDE0303 +dotnet_diagnostic.IDE0303.severity = %value% + # IDE1005 dotnet_diagnostic.IDE1005.severity = %value% @@ -883,6 +886,7 @@ public void CSharp_VerifyIDECodeStyleOptionsAreConfigurable() ("IDE0300", "dotnet_style_prefer_collection_expression", "true"), ("IDE0301", "dotnet_style_prefer_collection_expression", "true"), ("IDE0302", "dotnet_style_prefer_collection_expression", "true"), + ("IDE0303", "dotnet_style_prefer_collection_expression", "true"), ("IDE1005", "csharp_style_conditional_delegate_call", "true"), ("IDE1006", null, null), ("IDE1007", null, null), diff --git a/src/Features/RulesMissingDocumentation.md b/src/Features/RulesMissingDocumentation.md index 9d7bc698726d5..8dc58f307fa0b 100644 --- a/src/Features/RulesMissingDocumentation.md +++ b/src/Features/RulesMissingDocumentation.md @@ -15,6 +15,7 @@ IDE0290 | | Simplify collection initialization | IDE0301 | | Simplify collection initialization | IDE0302 | | Simplify collection initialization | +IDE0303 | | Simplify collection initialization | IDE1007 | | | IDE2000 | | Avoid multiple blank lines | IDE2001 | | Embedded statements must be on their own line | diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Extensions/ICompilationExtensions.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Extensions/ICompilationExtensions.cs index cb7d849dfdfff..74b8acbe2f4e2 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Extensions/ICompilationExtensions.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Extensions/ICompilationExtensions.cs @@ -186,6 +186,9 @@ public static ImmutableArray GetReferencedAssemblySymbols(this public static INamedTypeSymbol? OnSerializedAttribute(this Compilation compilation) => compilation.GetTypeByMetadataName(typeof(OnSerializedAttribute).FullName!); + public static INamedTypeSymbol? CollectionBuilderAttribute(this Compilation compilation) + => compilation.GetTypeByMetadataName("System.Runtime.CompilerServices.CollectionBuilderAttribute"); + public static INamedTypeSymbol? ComRegisterFunctionAttribute(this Compilation compilation) => compilation.GetTypeByMetadataName(typeof(ComRegisterFunctionAttribute).FullName!); diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/CodeGeneration/ExpressionGenerator.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/CodeGeneration/ExpressionGenerator.cs index 7cd05c1e80dc4..7b8808d3eaa82 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/CodeGeneration/ExpressionGenerator.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/CodeGeneration/ExpressionGenerator.cs @@ -31,8 +31,8 @@ public static ExpressionSyntax GenerateExpression( return GenerateExpression(generator, typedConstant.Type, typedConstant.Value, canUseFieldReference: true); case TypedConstantKind.Type: - return typedConstant.Value is ITypeSymbol - ? SyntaxFactory.TypeOfExpression(((ITypeSymbol)typedConstant.Value).GenerateTypeSyntax()) + return typedConstant.Value is ITypeSymbol typeSymbol + ? SyntaxFactory.TypeOfExpression(typeSymbol.GenerateTypeSyntax()) : GenerateNullLiteral(); case TypedConstantKind.Array: